瀏覽代碼

Merge branch 'sync-piper' into sync-integrate

Joshua Haberman 5 年之前
父節點
當前提交
4692661ee4

+ 1 - 2
conformance/conformance_python.py

@@ -157,8 +157,7 @@ def do_test(request):
 
     elif request.requested_output_format == conformance_pb2.JSON:
       try:
-        response.json_payload = json_format.MessageToJson(
-            test_message, float_precision=None)
+        response.json_payload = json_format.MessageToJson(test_message)
       except Exception as e:
         response.serialize_error = str(e)
         return response

二進制
csharp/src/Google.Protobuf.Test/testprotos.pb


+ 29 - 2
python/google/protobuf/internal/json_format_test.py

@@ -36,6 +36,7 @@ __author__ = 'jieluo@google.com (Jie Luo)'
 
 import json
 import math
+import struct
 import sys
 
 try:
@@ -821,15 +822,41 @@ class JsonFormatTest(JsonFormatBase):
   def testFloatPrecision(self):
     message = json_format_proto3_pb2.TestMessage()
     message.float_value = 1.123456789
-    # Default to 8 valid digits.
+    # Set to 8 valid digits.
     text = '{\n  "floatValue": 1.1234568\n}'
     self.assertEqual(
-        json_format.MessageToJson(message), text)
+        json_format.MessageToJson(message, float_precision=8), text)
     # Set to 7 valid digits.
     text = '{\n  "floatValue": 1.123457\n}'
     self.assertEqual(
         json_format.MessageToJson(message, float_precision=7), text)
 
+    # Default float_precision will automatic print shortest float.
+    message.float_value = 1.1000000011
+    text = '{\n  "floatValue": 1.1\n}'
+    self.assertEqual(
+        json_format.MessageToJson(message), text)
+    message.float_value = 1.00000075e-36
+    text = '{\n  "floatValue": 1.00000075e-36\n}'
+    self.assertEqual(
+        json_format.MessageToJson(message), text)
+    message.float_value = 12345678912345e+11
+    text = '{\n  "floatValue": 1.234568e+24\n}'
+    self.assertEqual(
+        json_format.MessageToJson(message), text)
+
+    # Test a bunch of data and check json encode/decode do not
+    # lose precision
+    value_list = [0x00, 0xD8, 0x6E, 0x00]
+    msg2 = json_format_proto3_pb2.TestMessage()
+    for a in range(0, 256):
+      value_list[3] = a
+      for b in range(0, 256):
+        value_list[0] = b
+        byte_array = bytearray(value_list)
+        message.float_value = struct.unpack('<f', byte_array)[0]
+        self.CheckParseBack(message, msg2)
+
   def testParseEmptyText(self):
     self.CheckError('',
                     r'Failed to load JSON: (Expecting value)|(No JSON).')

+ 9 - 4
python/google/protobuf/internal/type_checkers.py

@@ -64,6 +64,14 @@ from google.protobuf import descriptor
 
 _FieldDescriptor = descriptor.FieldDescriptor
 
+
+def TruncateToFourByteFloat(original):
+  if ctypes:
+    return ctypes.c_float(original).value
+  else:
+    return struct.unpack('<f', struct.pack('<f', original))[0]
+
+
 def SupportsOpenEnums(field_descriptor):
   return field_descriptor.containing_type.syntax == "proto3"
 
@@ -262,10 +270,7 @@ class FloatValueChecker(object):
     if converted_value < _FLOAT_MIN:
       return _NEG_INF
 
-    if ctypes:
-      return ctypes.c_float(converted_value).value
-    else:
-      return struct.unpack('<f', struct.pack('<f', converted_value))[0]
+    return TruncateToFourByteFloat(converted_value)
 
   def DefaultValue(self):
     return 0.0

+ 28 - 7
python/google/protobuf/json_format.py

@@ -60,6 +60,7 @@ import sys
 
 import six
 
+from google.protobuf.internal import type_checkers
 from google.protobuf import descriptor
 from google.protobuf import symbol_database
 
@@ -104,7 +105,7 @@ def MessageToJson(
     sort_keys=False,
     use_integers_for_enums=False,
     descriptor_pool=None,
-    float_precision=8):
+    float_precision=None):
   """Converts protobuf message to JSON format.
 
   Args:
@@ -123,7 +124,6 @@ def MessageToJson(
     descriptor_pool: A Descriptor Pool for resolving types. If None use the
         default.
     float_precision: If set, use this to specify float field valid digits.
-        Otherwise, 8 valid digits is used (default '.8g').
 
   Returns:
     A string containing the JSON formatted protocol buffer message.
@@ -143,7 +143,7 @@ def MessageToDict(
     preserving_proto_field_name=False,
     use_integers_for_enums=False,
     descriptor_pool=None,
-    float_precision=8):
+    float_precision=None):
   """Converts protobuf message to a dictionary.
 
   When the dictionary is encoded to JSON, it conforms to proto3 JSON spec.
@@ -161,7 +161,6 @@ def MessageToDict(
     descriptor_pool: A Descriptor Pool for resolving types. If None use the
         default.
     float_precision: If set, use this to specify float field valid digits.
-        Otherwise, 8 valid digits is used (default '.8g').
 
   Returns:
     A dict representation of the protocol buffer message.
@@ -284,6 +283,25 @@ class _Printer(object):
 
   def _FieldToJsonObject(self, field, value):
     """Converts field value according to Proto3 JSON Specification."""
+
+    def _ToShortestFloat(original):
+      """Returns the shortest float that has same value in wire."""
+      # Return the original value if it is not truncated. This may happen
+      # if someone mixes this code with an old protobuf runtime.
+      if type_checkers.TruncateToFourByteFloat(original) != original:
+        return original
+      # All 4 byte floats have between 6 and 9 significant digits, so we
+      # start with 6 as the lower bound.
+      # It has to be iterative because use '.9g' directly can not get rid
+      # of the noises for most values. For example if set a float_field=0.9
+      # use '.9g' will print 0.899999976.
+      precision = 6
+      rounded = float('{0:.{1}g}'.format(original, precision))
+      while type_checkers.TruncateToFourByteFloat(rounded) != original:
+        precision += 1
+        rounded = float('{0:.{1}g}'.format(original, precision))
+      return rounded
+
     if field.cpp_type == descriptor.FieldDescriptor.CPPTYPE_MESSAGE:
       return self._MessageToJsonObject(value)
     elif field.cpp_type == descriptor.FieldDescriptor.CPPTYPE_ENUM:
@@ -315,9 +333,12 @@ class _Printer(object):
           return _INFINITY
       if math.isnan(value):
         return _NAN
-      if (self.float_format and
-          field.cpp_type == descriptor.FieldDescriptor.CPPTYPE_FLOAT):
-        return float(format(value, self.float_format))
+      if field.cpp_type == descriptor.FieldDescriptor.CPPTYPE_FLOAT:
+        if self.float_format:
+          return float(format(value, self.float_format))
+        else:
+          return _ToShortestFloat(value)
+
     return value
 
   def _AnyMessageToJsonObject(self, message):

+ 8 - 12
src/google/protobuf/arena.h

@@ -219,14 +219,15 @@ struct ArenaOptions {
 // any special requirements on the type T, and will invoke the object's
 // destructor when the arena is destroyed.
 //
-// The arena message allocation protocol, required by CreateMessage<T>, is as
-// follows:
+// The arena message allocation protocol, required by
+// CreateMessage<T>(Arena* arena, Args&&... args), is as follows:
 //
-// - The type T must have (at least) two constructors: a constructor with no
-//   arguments, called when a T is allocated on the heap; and a constructor with
-//   a Arena* argument, called when a T is allocated on an arena. If the
-//   second constructor is called with a NULL arena pointer, it must be
-//   equivalent to invoking the first (no-argument) constructor.
+// - The type T must have (at least) two constructors: a constructor callable
+//   with `args` (without `arena`), called when a T is allocated on the heap;
+//   and a constructor callable with `Arena* arena, Args&&... args`, called when
+//   a T is allocated on an arena. If the second constructor is called with a
+//   NULL arena pointer, it must be equivalent to invoking the first
+//   (`args`-only) constructor.
 //
 // - The type T must have a particular type trait: a nested type
 //   |InternalArenaConstructable_|. This is usually a typedef to |void|. If no
@@ -239,11 +240,6 @@ struct ArenaOptions {
 //   present on the type, then its destructor is always called when the
 //   containing arena is destroyed.
 //
-// - One- and two-user-argument forms of CreateMessage<T>() also exist that
-//   forward these constructor arguments to T's constructor: for example,
-//   CreateMessage<T>(Arena*, arg1, arg2) forwards to a constructor T(Arena*,
-//   arg1, arg2).
-//
 // This protocol is implemented by all arena-enabled proto2 message classes as
 // well as protobuf container types like RepeatedPtrField and Map. The protocol
 // is internal to protobuf and is not guaranteed to be stable. Non-proto types

+ 15 - 2
src/google/protobuf/compiler/cpp/cpp_helpers.h

@@ -429,9 +429,22 @@ inline bool HasFieldPresence(const FileDescriptor* file) {
 }
 
 inline bool HasHasbit(const FieldDescriptor* field) {
-  // TODO(haberman): remove, and give some proto3 fields hasbits.
+  // TODO(haberman): remove, so proto3 optional fields have hasbits.
   if (field->file()->syntax() == FileDescriptor::SYNTAX_PROTO3) return false;
-  return field->is_singular_with_presence();
+
+  // This predicate includes proto3 message fields only if they have "optional".
+  //   Foo submsg1 = 1;           // HasHasbit() == false
+  //   optional Foo submsg2 = 2;  // HasHasbit() == true
+  // This is slightly odd, as adding "optional" to a singular proto3 field does
+  // not change the semantics or API. However whenever any field in a message
+  // has a hasbit, it forces reflection to include hasbit offsets for *all*
+  // fields, even if almost all of them are set to -1 (no hasbit). So to avoid
+  // causing a sudden size regression for ~all proto3 messages, we give proto3
+  // message fields a hasbit only if "optional" is present. If the user is
+  // explicitly writing "optional", it is likely they are writing it on
+  // primitive fields also.
+  return (field->has_optional_keyword() || field->is_required()) &&
+         !field->options().weak();
 }
 
 // Returns true if 'enum' semantics are such that unknown values are preserved

+ 82 - 85
src/google/protobuf/compiler/cpp/cpp_message.cc

@@ -69,6 +69,8 @@ using internal::WireFormatLite;
 
 namespace {
 
+static constexpr int kNoHasbit = -1;
+
 // Create an expression that evaluates to
 //  "for all i, (_has_bits_[i] & masks[i]) == masks[i]"
 // masks is allowed to be shorter than _has_bits_, but at least one element of
@@ -239,7 +241,8 @@ bool HasHasMethod(const FieldDescriptor* field) {
   }
   // For message types without true field presence, only fields with a message
   // type have a has_$name$() method.
-  return field->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE;
+  return field->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE ||
+         field->has_optional_keyword();
 }
 
 // Collects map entry message type information.
@@ -329,10 +332,16 @@ bool TableDrivenParsingEnabled(const Descriptor* descriptor,
   // Consider table-driven parsing.  We only do this if:
   // - We have has_bits for fields.  This avoids a check on every field we set
   //   when are present (the common case).
-  if (!HasFieldPresence(descriptor->file())) {
-    return false;
+  bool has_hasbit = false;
+  for (int i = 0; i < descriptor->field_count(); i++) {
+    if (HasHasbit(descriptor->field(i))) {
+      has_hasbit = true;
+      break;
+    }
   }
 
+  if (!has_hasbit) return false;
+
   const double table_sparseness = 0.5;
   int max_field_number = 0;
   for (auto field : FieldRange(descriptor)) {
@@ -488,7 +497,7 @@ void ColdChunkSkipper::OnStartChunk(int chunk, int cached_has_word_index,
                                     const std::string& from,
                                     io::Printer* printer) {
   Formatter format(printer, variables_);
-  if (!access_info_map_ || has_bit_indices_.empty()) {
+  if (!access_info_map_) {
     return;
   } else if (chunk < limit_chunk_) {
     // We are already inside a run of cold chunks.
@@ -595,17 +604,18 @@ MessageGenerator::MessageGenerator(
 
   message_layout_helper_->OptimizeLayout(&optimized_order_, options_);
 
-  if (HasFieldPresence(descriptor_->file())) {
-    // We use -1 as a sentinel.
-    has_bit_indices_.resize(descriptor_->field_count(), -1);
-    for (auto field : optimized_order_) {
-      // Skip fields that do not have has bits.
-      if (field->is_repeated()) {
-        continue;
+  // This message has hasbits iff one or more fields need one.
+  for (auto field : optimized_order_) {
+    if (HasHasbit(field)) {
+      if (has_bit_indices_.empty()) {
+        has_bit_indices_.resize(descriptor_->field_count(), kNoHasbit);
       }
-
       has_bit_indices_[field->index()] = max_has_bit_index_++;
     }
+  }
+
+
+  if (!has_bit_indices_.empty()) {
     field_generators_.SetHasBitIndices(has_bit_indices_);
   }
 
@@ -635,17 +645,18 @@ size_t MessageGenerator::HasBitsSize() const {
 }
 
 int MessageGenerator::HasBitIndex(const FieldDescriptor* field) const {
-  return has_bit_indices_.empty() ? -1 : has_bit_indices_[field->index()];
+  return has_bit_indices_.empty() ? kNoHasbit
+                                  : has_bit_indices_[field->index()];
 }
 
 int MessageGenerator::HasByteIndex(const FieldDescriptor* field) const {
   int hasbit = HasBitIndex(field);
-  return hasbit == -1 ? -1 : hasbit / 8;
+  return hasbit == kNoHasbit ? kNoHasbit : hasbit / 8;
 }
 
 int MessageGenerator::HasWordIndex(const FieldDescriptor* field) const {
   int hasbit = HasBitIndex(field);
-  return hasbit == -1 ? -1 : hasbit / 32;
+  return hasbit == kNoHasbit ? kNoHasbit : hasbit / 32;
 }
 
 void MessageGenerator::AddGenerators(
@@ -784,11 +795,9 @@ void MessageGenerator::GenerateSingularFieldHasBits(
         "}\n");
     return;
   }
-  if (HasFieldPresence(descriptor_->file())) {
-    // N.B.: without field presence, we do not use has-bits or generate
-    // has_$name$() methods.
-    int has_bit_index = has_bit_indices_[field->index()];
-    GOOGLE_CHECK_GE(has_bit_index, 0);
+  if (HasHasbit(field)) {
+    int has_bit_index = HasBitIndex(field);
+    GOOGLE_CHECK_NE(has_bit_index, kNoHasbit);
 
     format.Set("has_array_index", has_bit_index / 32);
     format.Set("has_mask",
@@ -813,27 +822,25 @@ void MessageGenerator::GenerateSingularFieldHasBits(
         "$annotate_accessor$"
         "  return _internal_has_$name$();\n"
         "}\n");
-  } else {
+  } else if (field->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE) {
     // Message fields have a has_$name$() method.
-    if (field->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE) {
-      if (IsLazy(field, options_)) {
-        format(
-            "inline bool $classname$::_internal_has_$name$() const {\n"
-            "  return !$name$_.IsCleared();\n"
-            "}\n");
-      } else {
-        format(
-            "inline bool $classname$::_internal_has_$name$() const {\n"
-            "  return this != internal_default_instance() "
-            "&& $name$_ != nullptr;\n"
-            "}\n");
-      }
+    if (IsLazy(field, options_)) {
+      format(
+          "inline bool $classname$::_internal_has_$name$() const {\n"
+          "  return !$name$_.IsCleared();\n"
+          "}\n");
+    } else {
       format(
-          "inline bool $classname$::has_$name$() const {\n"
-          "$annotate_accessor$"
-          "  return _internal_has_$name$();\n"
+          "inline bool $classname$::_internal_has_$name$() const {\n"
+          "  return this != internal_default_instance() "
+          "&& $name$_ != nullptr;\n"
           "}\n");
     }
+    format(
+        "inline bool $classname$::has_$name$() const {\n"
+        "$annotate_accessor$"
+        "  return _internal_has_$name$();\n"
+        "}\n");
   }
 }
 
@@ -926,16 +933,12 @@ void MessageGenerator::GenerateFieldClear(const FieldDescriptor* field,
     format("}\n");
   } else {
     field_generators_.get(field).GenerateClearingCode(format.printer());
-    if (HasFieldPresence(descriptor_->file())) {
-      if (!field->is_repeated() && !field->options().weak()) {
-        int has_bit_index = has_bit_indices_[field->index()];
-        GOOGLE_CHECK_GE(has_bit_index, 0);
-
-        format.Set("has_array_index", has_bit_index / 32);
-        format.Set("has_mask",
-                   strings::Hex(1u << (has_bit_index % 32), strings::ZERO_PAD_8));
-        format("_has_bits_[$has_array_index$] &= ~0x$has_mask$u;\n");
-      }
+    if (HasHasbit(field)) {
+      int has_bit_index = HasBitIndex(field);
+      format.Set("has_array_index", has_bit_index / 32);
+      format.Set("has_mask",
+                 strings::Hex(1u << (has_bit_index % 32), strings::ZERO_PAD_8));
+      format("_has_bits_[$has_array_index$] &= ~0x$has_mask$u;\n");
     }
   }
 
@@ -1540,7 +1543,7 @@ void MessageGenerator::GenerateClassDefinition(io::Printer* printer) {
         "typedef void DestructorSkippable_;\n");
   }
 
-  if (HasFieldPresence(descriptor_->file())) {
+  if (!has_bit_indices_.empty()) {
     // _has_bits_ is frequently accessed, so to reduce code size and improve
     // speed, it should be close to the start of the object. Placing
     // _cached_size_ together with _has_bits_ improves cache locality despite
@@ -1686,8 +1689,8 @@ bool MessageGenerator::GenerateParseTable(io::Printer* printer, size_t offset,
       "$3$,\n",
       offset, aux_offset, max_field_number);
 
-  if (!HasFieldPresence(descriptor_->file())) {
-    // If we don't have field presence, then _has_bits_ does not exist.
+  if (has_bit_indices_.empty()) {
+    // If no fields have hasbits, then _has_bits_ does not exist.
     format("-1,\n");
   } else {
     format("PROTOBUF_FIELD_OFFSET($classtype$, _has_bits_),\n");
@@ -1725,10 +1728,9 @@ bool MessageGenerator::GenerateParseTable(io::Printer* printer, size_t offset,
 void MessageGenerator::GenerateSchema(io::Printer* printer, int offset,
                                       int has_offset) {
   Formatter format(printer, variables_);
-  has_offset =
-      HasFieldPresence(descriptor_->file()) || IsMapEntryMessage(descriptor_)
-          ? offset + has_offset
-          : -1;
+  has_offset = !has_bit_indices_.empty() || IsMapEntryMessage(descriptor_)
+                   ? offset + has_offset
+                   : -1;
 
   format("{ $1$, $2$, sizeof($classtype$)},\n", offset, has_offset);
 }
@@ -1763,13 +1765,12 @@ uint32 CalcFieldNum(const FieldGenerator& generator,
   } else if (field->is_repeated()) {
     return internal::FieldMetadata::CalculateType(
         type, internal::FieldMetadata::kRepeated);
-  } else if (!HasFieldPresence(field->file()) &&
-             field->containing_oneof() == NULL && !is_a_map) {
+  } else if (HasHasbit(field) || field->containing_oneof() || is_a_map) {
     return internal::FieldMetadata::CalculateType(
-        type, internal::FieldMetadata::kNoPresence);
+        type, internal::FieldMetadata::kPresence);
   } else {
     return internal::FieldMetadata::CalculateType(
-        type, internal::FieldMetadata::kPresence);
+        type, internal::FieldMetadata::kNoPresence);
   }
 }
 
@@ -1897,8 +1898,7 @@ int MessageGenerator::GenerateFieldMetadata(io::Printer* printer) {
             "::internal::LazyFieldSerializer";
       if (field->containing_oneof()) {
         ptr += "OneOf";
-      } else if (!HasFieldPresence(descriptor_->file()) ||
-                 has_bit_indices_[field->index()] == -1) {
+      } else if (!HasHasbit(field)) {
         ptr += "NoPresence";
       }
       ptr += ")";
@@ -1921,8 +1921,7 @@ int MessageGenerator::GenerateFieldMetadata(io::Printer* printer) {
           " PROTOBUF_FIELD_OFFSET($classtype$, _oneof_case_) + "
           "$oneofoffset$, $2$, $3$},\n",
           tag, type, ptr);
-    } else if (HasFieldPresence(descriptor_->file()) &&
-               has_bit_indices_[field->index()] != -1) {
+    } else if (HasHasbit(field)) {
       format.Set("hasbitsoffset", has_bit_indices_[field->index()]);
       format(
           "{PROTOBUF_FIELD_OFFSET($classtype$, $field_name$_), "
@@ -2073,7 +2072,7 @@ void MessageGenerator::GenerateClassMethods(io::Printer* printer) {
       "class $classname$::_Internal {\n"
       " public:\n");
   format.Indent();
-  if (HasFieldPresence(descriptor_->file()) && HasBitsSize() != 0) {
+  if (!has_bit_indices_.empty()) {
     format(
         "using HasBits = decltype(std::declval<$classname$>()._has_bits_);\n");
   }
@@ -2082,10 +2081,8 @@ void MessageGenerator::GenerateClassMethods(io::Printer* printer) {
     if (!IsFieldUsed(field, options_)) {
       continue;
     }
-    if (HasFieldPresence(descriptor_->file()) && !field->is_repeated() &&
-        !field->options().weak() && !field->containing_oneof()) {
-      int has_bit_index = has_bit_indices_[field->index()];
-      GOOGLE_CHECK_GE(has_bit_index, 0);
+    if (HasHasbit(field)) {
+      int has_bit_index = HasBitIndex(field);
       format(
           "static void set_has_$1$(HasBits* has_bits) {\n"
           "  (*has_bits)[$2$] |= $3$u;\n"
@@ -2093,7 +2090,7 @@ void MessageGenerator::GenerateClassMethods(io::Printer* printer) {
           FieldName(field), has_bit_index / 32, (1u << (has_bit_index % 32)));
     }
   }
-  if (num_required_fields_ > 0 && HasFieldPresence(descriptor_->file())) {
+  if (num_required_fields_ > 0) {
     const std::vector<uint32> masks_for_has_bits = RequiredFieldsBitMask();
     format(
         "static bool MissingRequiredFields(const HasBits& has_bits) "
@@ -2392,7 +2389,7 @@ std::pair<size_t, size_t> MessageGenerator::GenerateOffsets(
     io::Printer* printer) {
   Formatter format(printer, variables_);
 
-  if (HasFieldPresence(descriptor_->file()) || IsMapEntryMessage(descriptor_)) {
+  if (!has_bit_indices_.empty() || IsMapEntryMessage(descriptor_)) {
     format("PROTOBUF_FIELD_OFFSET($classtype$, _has_bits_),\n");
   } else {
     format("~0u,  // no _has_bits_\n");
@@ -2451,7 +2448,7 @@ std::pair<size_t, size_t> MessageGenerator::GenerateOffsets(
     format(
         "0,\n"
         "1,\n");
-  } else if (HasFieldPresence(descriptor_->file())) {
+  } else if (!has_bit_indices_.empty()) {
     entries += has_bit_indices_.size() - num_stripped;
     for (int i = 0; i < has_bit_indices_.size(); i++) {
       if (!IsFieldUsed(descriptor_->field(i), options_)) {
@@ -2723,10 +2720,8 @@ void MessageGenerator::GenerateStructors(io::Printer* printer) {
     format.Indent();
     format.Indent();
 
-    if (HasFieldPresence(descriptor_->file())) {
-      if (!IsProto2MessageSet(descriptor_, options_)) {
-        format(",\n_has_bits_(from._has_bits_)");
-      }
+    if (!has_bit_indices_.empty()) {
+      format(",\n_has_bits_(from._has_bits_)");
     }
 
     std::vector<bool> processed(optimized_order_.size(), false);
@@ -2916,7 +2911,7 @@ void MessageGenerator::GenerateClear(io::Printer* printer) {
     // We can omit the if() for chunk size 1, or if our fields do not have
     // hasbits. I don't understand the rationale for the last part of the
     // condition, but it matches the old logic.
-    const bool have_outer_if = HasBitIndex(chunk.front()) != -1 &&
+    const bool have_outer_if = HasBitIndex(chunk.front()) != kNoHasbit &&
                                chunk.size() > 1 &&
                                (memset_end != chunk.back() || merge_zero_init);
 
@@ -2962,7 +2957,7 @@ void MessageGenerator::GenerateClear(io::Printer* printer) {
       //
       // TODO(kenton):  Let the CppFieldGenerator decide this somehow.
       bool have_enclosing_if =
-          HasBitIndex(field) != -1 &&
+          HasBitIndex(field) != kNoHasbit &&
           (field->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE ||
            field->cpp_type() == FieldDescriptor::CPPTYPE_STRING);
 
@@ -2999,7 +2994,7 @@ void MessageGenerator::GenerateClear(io::Printer* printer) {
     format("_weak_field_map_.ClearAll();\n");
   }
 
-  if (HasFieldPresence(descriptor_->file())) {
+  if (!has_bit_indices_.empty()) {
     // Step 5: Everything else.
     format("_has_bits_.Clear();\n");
   }
@@ -3075,7 +3070,7 @@ void MessageGenerator::GenerateSwap(io::Printer* printer) {
         "_internal_metadata_.Swap<$unknown_fields_type$>(&other->_internal_"
         "metadata_);\n");
 
-    if (HasFieldPresence(descriptor_->file())) {
+    if (!has_bit_indices_.empty()) {
       for (int i = 0; i < HasBitsSize() / 4; ++i) {
         format("swap(_has_bits_[$1$], other->_has_bits_[$1$]);\n", i);
       }
@@ -3218,7 +3213,8 @@ void MessageGenerator::GenerateClassSpecificMergeFrom(io::Printer* printer) {
 
   for (int chunk_index = 0; chunk_index < chunks.size(); chunk_index++) {
     const std::vector<const FieldDescriptor*>& chunk = chunks[chunk_index];
-    bool have_outer_if = chunk.size() > 1 && HasByteIndex(chunk.front()) != -1;
+    bool have_outer_if =
+        chunk.size() > 1 && HasByteIndex(chunk.front()) != kNoHasbit;
     cold_skipper.OnStartChunk(chunk_index, cached_has_word_index, "from.",
                               printer);
 
@@ -3466,9 +3462,9 @@ void MessageGenerator::GenerateSerializeOneField(io::Printer* printer,
 
   bool have_enclosing_if = false;
   if (field->options().weak()) {
-  } else if (!field->is_repeated() && HasFieldPresence(descriptor_->file())) {
+  } else if (HasHasbit(field)) {
     // Attempt to use the state of cached_has_bits, if possible.
-    int has_bit_index = has_bit_indices_[field->index()];
+    int has_bit_index = HasBitIndex(field);
     if (cached_has_bits_index == has_bit_index / 32) {
       const std::string mask =
           StrCat(strings::Hex(1u << (has_bit_index % 32), strings::ZERO_PAD_8));
@@ -3480,7 +3476,7 @@ void MessageGenerator::GenerateSerializeOneField(io::Printer* printer,
 
     format.Indent();
     have_enclosing_if = true;
-  } else if (!HasFieldPresence(descriptor_->file())) {
+  } else if (field->is_optional() && !HasHasbit(field)) {
     have_enclosing_if = EmitFieldNonDefaultCondition(printer, "this->", field);
   }
 
@@ -3561,7 +3557,7 @@ void MessageGenerator::GenerateSerializeWithCachedSizesBody(
         : mg_(mg),
           format_(printer),
           eager_(!HasFieldPresence(mg->descriptor_->file())),
-          cached_has_bit_index_(-1) {}
+          cached_has_bit_index_(kNoHasbit) {}
 
     ~LazySerializerEmitter() { Flush(); }
 
@@ -3841,7 +3837,8 @@ void MessageGenerator::GenerateByteSize(io::Printer* printer) {
 
   for (int chunk_index = 0; chunk_index < chunks.size(); chunk_index++) {
     const std::vector<const FieldDescriptor*>& chunk = chunks[chunk_index];
-    const bool have_outer_if = chunk.size() > 1 && HasWordIndex(chunk[0]) != -1;
+    const bool have_outer_if =
+        chunk.size() > 1 && HasWordIndex(chunk[0]) != kNoHasbit;
     cold_skipper.OnStartChunk(chunk_index, cached_has_word_index, "", printer);
 
     if (have_outer_if) {
@@ -3983,7 +3980,7 @@ void MessageGenerator::GenerateIsInitialized(io::Printer* printer) {
         "}\n\n");
   }
 
-  if (num_required_fields_ > 0 && HasFieldPresence(descriptor_->file())) {
+  if (num_required_fields_ > 0) {
     format(
         "if (_Internal::MissingRequiredFields(_has_bits_))"
         " return false;\n");

+ 0 - 30
src/google/protobuf/compiler/zip_writer.cc

@@ -28,36 +28,6 @@
 // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-// Protocol Buffers - Google's data interchange format
-// Copyright 2008 Google Inc.  All rights reserved.
-// https://developers.google.com/protocol-buffers/
-//
-// Redistribution and use in source and binary forms, with or without
-// modification, are permitted provided that the following conditions are
-// met:
-//
-//     * Redistributions of source code must retain the above copyright
-// notice, this list of conditions and the following disclaimer.
-//     * Redistributions in binary form must reproduce the above
-// copyright notice, this list of conditions and the following disclaimer
-// in the documentation and/or other materials provided with the
-// distribution.
-//     * Neither the name of Google Inc. nor the names of its
-// contributors may be used to endorse or promote products derived from
-// this software without specific prior written permission.
-//
-// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
-// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
-// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
-// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
-// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
-// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
-// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
-// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
-// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
-// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
-// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-
 // Author: ambrose@google.com (Ambrose Feinstein),
 //         kenton@google.com (Kenton Varda)
 //

+ 0 - 30
src/google/protobuf/compiler/zip_writer.h

@@ -28,36 +28,6 @@
 // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-// Protocol Buffers - Google's data interchange format
-// Copyright 2008 Google Inc.  All rights reserved.
-// https://developers.google.com/protocol-buffers/
-//
-// Redistribution and use in source and binary forms, with or without
-// modification, are permitted provided that the following conditions are
-// met:
-//
-//     * Redistributions of source code must retain the above copyright
-// notice, this list of conditions and the following disclaimer.
-//     * Redistributions in binary form must reproduce the above
-// copyright notice, this list of conditions and the following disclaimer
-// in the documentation and/or other materials provided with the
-// distribution.
-//     * Neither the name of Google Inc. nor the names of its
-// contributors may be used to endorse or promote products derived from
-// this software without specific prior written permission.
-//
-// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
-// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
-// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
-// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
-// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
-// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
-// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
-// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
-// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
-// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
-// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-
 // Author: kenton@google.com (Kenton Varda)
 
 #include <vector>

+ 32 - 57
src/google/protobuf/descriptor.pb.cc

@@ -567,13 +567,12 @@ static const ::PROTOBUF_NAMESPACE_ID::EnumDescriptor* file_level_enum_descriptor
 static constexpr ::PROTOBUF_NAMESPACE_ID::ServiceDescriptor const** file_level_service_descriptors_google_2fprotobuf_2fdescriptor_2eproto = nullptr;
 
 const ::PROTOBUF_NAMESPACE_ID::uint32 TableStruct_google_2fprotobuf_2fdescriptor_2eproto::offsets[] PROTOBUF_SECTION_VARIABLE(protodesc_cold) = {
-  PROTOBUF_FIELD_OFFSET(PROTOBUF_NAMESPACE_ID::FileDescriptorSet, _has_bits_),
+  ~0u,  // no _has_bits_
   PROTOBUF_FIELD_OFFSET(PROTOBUF_NAMESPACE_ID::FileDescriptorSet, _internal_metadata_),
   ~0u,  // no _extensions_
   ~0u,  // no _oneof_case_
   ~0u,  // no _weak_field_map_
   PROTOBUF_FIELD_OFFSET(PROTOBUF_NAMESPACE_ID::FileDescriptorSet, file_),
-  ~0u,
   PROTOBUF_FIELD_OFFSET(PROTOBUF_NAMESPACE_ID::FileDescriptorProto, _has_bits_),
   PROTOBUF_FIELD_OFFSET(PROTOBUF_NAMESPACE_ID::FileDescriptorProto, _internal_metadata_),
   ~0u,  // no _extensions_
@@ -648,13 +647,12 @@ const ::PROTOBUF_NAMESPACE_ID::uint32 TableStruct_google_2fprotobuf_2fdescriptor
   1,
   ~0u,
   ~0u,
-  PROTOBUF_FIELD_OFFSET(PROTOBUF_NAMESPACE_ID::ExtensionRangeOptions, _has_bits_),
+  ~0u,  // no _has_bits_
   PROTOBUF_FIELD_OFFSET(PROTOBUF_NAMESPACE_ID::ExtensionRangeOptions, _internal_metadata_),
   PROTOBUF_FIELD_OFFSET(PROTOBUF_NAMESPACE_ID::ExtensionRangeOptions, _extensions_),
   ~0u,  // no _oneof_case_
   ~0u,  // no _weak_field_map_
   PROTOBUF_FIELD_OFFSET(PROTOBUF_NAMESPACE_ID::ExtensionRangeOptions, uninterpreted_option_),
-  ~0u,
   PROTOBUF_FIELD_OFFSET(PROTOBUF_NAMESPACE_ID::FieldDescriptorProto, _has_bits_),
   PROTOBUF_FIELD_OFFSET(PROTOBUF_NAMESPACE_ID::FieldDescriptorProto, _internal_metadata_),
   ~0u,  // no _extensions_
@@ -835,13 +833,12 @@ const ::PROTOBUF_NAMESPACE_ID::uint32 TableStruct_google_2fprotobuf_2fdescriptor
   3,
   4,
   ~0u,
-  PROTOBUF_FIELD_OFFSET(PROTOBUF_NAMESPACE_ID::OneofOptions, _has_bits_),
+  ~0u,  // no _has_bits_
   PROTOBUF_FIELD_OFFSET(PROTOBUF_NAMESPACE_ID::OneofOptions, _internal_metadata_),
   PROTOBUF_FIELD_OFFSET(PROTOBUF_NAMESPACE_ID::OneofOptions, _extensions_),
   ~0u,  // no _oneof_case_
   ~0u,  // no _weak_field_map_
   PROTOBUF_FIELD_OFFSET(PROTOBUF_NAMESPACE_ID::OneofOptions, uninterpreted_option_),
-  ~0u,
   PROTOBUF_FIELD_OFFSET(PROTOBUF_NAMESPACE_ID::EnumOptions, _has_bits_),
   PROTOBUF_FIELD_OFFSET(PROTOBUF_NAMESPACE_ID::EnumOptions, _internal_metadata_),
   PROTOBUF_FIELD_OFFSET(PROTOBUF_NAMESPACE_ID::EnumOptions, _extensions_),
@@ -925,13 +922,12 @@ const ::PROTOBUF_NAMESPACE_ID::uint32 TableStruct_google_2fprotobuf_2fdescriptor
   0,
   1,
   ~0u,
-  PROTOBUF_FIELD_OFFSET(PROTOBUF_NAMESPACE_ID::SourceCodeInfo, _has_bits_),
+  ~0u,  // no _has_bits_
   PROTOBUF_FIELD_OFFSET(PROTOBUF_NAMESPACE_ID::SourceCodeInfo, _internal_metadata_),
   ~0u,  // no _extensions_
   ~0u,  // no _oneof_case_
   ~0u,  // no _weak_field_map_
   PROTOBUF_FIELD_OFFSET(PROTOBUF_NAMESPACE_ID::SourceCodeInfo, location_),
-  ~0u,
   PROTOBUF_FIELD_OFFSET(PROTOBUF_NAMESPACE_ID::GeneratedCodeInfo_Annotation, _has_bits_),
   PROTOBUF_FIELD_OFFSET(PROTOBUF_NAMESPACE_ID::GeneratedCodeInfo_Annotation, _internal_metadata_),
   ~0u,  // no _extensions_
@@ -945,42 +941,41 @@ const ::PROTOBUF_NAMESPACE_ID::uint32 TableStruct_google_2fprotobuf_2fdescriptor
   0,
   1,
   2,
-  PROTOBUF_FIELD_OFFSET(PROTOBUF_NAMESPACE_ID::GeneratedCodeInfo, _has_bits_),
+  ~0u,  // no _has_bits_
   PROTOBUF_FIELD_OFFSET(PROTOBUF_NAMESPACE_ID::GeneratedCodeInfo, _internal_metadata_),
   ~0u,  // no _extensions_
   ~0u,  // no _oneof_case_
   ~0u,  // no _weak_field_map_
   PROTOBUF_FIELD_OFFSET(PROTOBUF_NAMESPACE_ID::GeneratedCodeInfo, annotation_),
-  ~0u,
 };
 static const ::PROTOBUF_NAMESPACE_ID::internal::MigrationSchema schemas[] PROTOBUF_SECTION_VARIABLE(protodesc_cold) = {
-  { 0, 6, sizeof(PROTOBUF_NAMESPACE_ID::FileDescriptorSet)},
-  { 7, 24, sizeof(PROTOBUF_NAMESPACE_ID::FileDescriptorProto)},
-  { 36, 44, sizeof(PROTOBUF_NAMESPACE_ID::DescriptorProto_ExtensionRange)},
-  { 47, 54, sizeof(PROTOBUF_NAMESPACE_ID::DescriptorProto_ReservedRange)},
-  { 56, 71, sizeof(PROTOBUF_NAMESPACE_ID::DescriptorProto)},
-  { 81, 87, sizeof(PROTOBUF_NAMESPACE_ID::ExtensionRangeOptions)},
-  { 88, 104, sizeof(PROTOBUF_NAMESPACE_ID::FieldDescriptorProto)},
-  { 115, 122, sizeof(PROTOBUF_NAMESPACE_ID::OneofDescriptorProto)},
-  { 124, 131, sizeof(PROTOBUF_NAMESPACE_ID::EnumDescriptorProto_EnumReservedRange)},
-  { 133, 143, sizeof(PROTOBUF_NAMESPACE_ID::EnumDescriptorProto)},
-  { 148, 156, sizeof(PROTOBUF_NAMESPACE_ID::EnumValueDescriptorProto)},
-  { 159, 167, sizeof(PROTOBUF_NAMESPACE_ID::ServiceDescriptorProto)},
-  { 170, 181, sizeof(PROTOBUF_NAMESPACE_ID::MethodDescriptorProto)},
-  { 187, 213, sizeof(PROTOBUF_NAMESPACE_ID::FileOptions)},
-  { 234, 244, sizeof(PROTOBUF_NAMESPACE_ID::MessageOptions)},
-  { 249, 261, sizeof(PROTOBUF_NAMESPACE_ID::FieldOptions)},
-  { 268, 274, sizeof(PROTOBUF_NAMESPACE_ID::OneofOptions)},
-  { 275, 283, sizeof(PROTOBUF_NAMESPACE_ID::EnumOptions)},
-  { 286, 293, sizeof(PROTOBUF_NAMESPACE_ID::EnumValueOptions)},
-  { 295, 302, sizeof(PROTOBUF_NAMESPACE_ID::ServiceOptions)},
-  { 304, 312, sizeof(PROTOBUF_NAMESPACE_ID::MethodOptions)},
-  { 315, 322, sizeof(PROTOBUF_NAMESPACE_ID::UninterpretedOption_NamePart)},
-  { 324, 336, sizeof(PROTOBUF_NAMESPACE_ID::UninterpretedOption)},
-  { 343, 353, sizeof(PROTOBUF_NAMESPACE_ID::SourceCodeInfo_Location)},
-  { 358, 364, sizeof(PROTOBUF_NAMESPACE_ID::SourceCodeInfo)},
-  { 365, 374, sizeof(PROTOBUF_NAMESPACE_ID::GeneratedCodeInfo_Annotation)},
-  { 378, 384, sizeof(PROTOBUF_NAMESPACE_ID::GeneratedCodeInfo)},
+  { 0, -1, sizeof(PROTOBUF_NAMESPACE_ID::FileDescriptorSet)},
+  { 6, 23, sizeof(PROTOBUF_NAMESPACE_ID::FileDescriptorProto)},
+  { 35, 43, sizeof(PROTOBUF_NAMESPACE_ID::DescriptorProto_ExtensionRange)},
+  { 46, 53, sizeof(PROTOBUF_NAMESPACE_ID::DescriptorProto_ReservedRange)},
+  { 55, 70, sizeof(PROTOBUF_NAMESPACE_ID::DescriptorProto)},
+  { 80, -1, sizeof(PROTOBUF_NAMESPACE_ID::ExtensionRangeOptions)},
+  { 86, 102, sizeof(PROTOBUF_NAMESPACE_ID::FieldDescriptorProto)},
+  { 113, 120, sizeof(PROTOBUF_NAMESPACE_ID::OneofDescriptorProto)},
+  { 122, 129, sizeof(PROTOBUF_NAMESPACE_ID::EnumDescriptorProto_EnumReservedRange)},
+  { 131, 141, sizeof(PROTOBUF_NAMESPACE_ID::EnumDescriptorProto)},
+  { 146, 154, sizeof(PROTOBUF_NAMESPACE_ID::EnumValueDescriptorProto)},
+  { 157, 165, sizeof(PROTOBUF_NAMESPACE_ID::ServiceDescriptorProto)},
+  { 168, 179, sizeof(PROTOBUF_NAMESPACE_ID::MethodDescriptorProto)},
+  { 185, 211, sizeof(PROTOBUF_NAMESPACE_ID::FileOptions)},
+  { 232, 242, sizeof(PROTOBUF_NAMESPACE_ID::MessageOptions)},
+  { 247, 259, sizeof(PROTOBUF_NAMESPACE_ID::FieldOptions)},
+  { 266, -1, sizeof(PROTOBUF_NAMESPACE_ID::OneofOptions)},
+  { 272, 280, sizeof(PROTOBUF_NAMESPACE_ID::EnumOptions)},
+  { 283, 290, sizeof(PROTOBUF_NAMESPACE_ID::EnumValueOptions)},
+  { 292, 299, sizeof(PROTOBUF_NAMESPACE_ID::ServiceOptions)},
+  { 301, 309, sizeof(PROTOBUF_NAMESPACE_ID::MethodOptions)},
+  { 312, 319, sizeof(PROTOBUF_NAMESPACE_ID::UninterpretedOption_NamePart)},
+  { 321, 333, sizeof(PROTOBUF_NAMESPACE_ID::UninterpretedOption)},
+  { 340, 350, sizeof(PROTOBUF_NAMESPACE_ID::SourceCodeInfo_Location)},
+  { 355, -1, sizeof(PROTOBUF_NAMESPACE_ID::SourceCodeInfo)},
+  { 361, 370, sizeof(PROTOBUF_NAMESPACE_ID::GeneratedCodeInfo_Annotation)},
+  { 374, -1, sizeof(PROTOBUF_NAMESPACE_ID::GeneratedCodeInfo)},
 };
 
 static ::PROTOBUF_NAMESPACE_ID::Message const * const file_default_instances[] = {
@@ -1384,7 +1379,6 @@ void FileDescriptorSet::InitAsDefaultInstance() {
 }
 class FileDescriptorSet::_Internal {
  public:
-  using HasBits = decltype(std::declval<FileDescriptorSet>()._has_bits_);
 };
 
 FileDescriptorSet::FileDescriptorSet(::PROTOBUF_NAMESPACE_ID::Arena* arena)
@@ -1396,7 +1390,6 @@ FileDescriptorSet::FileDescriptorSet(::PROTOBUF_NAMESPACE_ID::Arena* arena)
 }
 FileDescriptorSet::FileDescriptorSet(const FileDescriptorSet& from)
   : ::PROTOBUF_NAMESPACE_ID::Message(),
-      _has_bits_(from._has_bits_),
       file_(from.file_) {
   _internal_metadata_.MergeFrom<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(from._internal_metadata_);
   // @@protoc_insertion_point(copy_constructor:google.protobuf.FileDescriptorSet)
@@ -1438,7 +1431,6 @@ void FileDescriptorSet::Clear() {
   (void) cached_has_bits;
 
   file_.Clear();
-  _has_bits_.Clear();
   _internal_metadata_.Clear<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>();
 }
 
@@ -1577,7 +1569,6 @@ bool FileDescriptorSet::IsInitialized() const {
 void FileDescriptorSet::InternalSwap(FileDescriptorSet* other) {
   using std::swap;
   _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_);
-  swap(_has_bits_[0], other->_has_bits_[0]);
   file_.InternalSwap(&other->file_);
 }
 
@@ -3369,7 +3360,6 @@ void ExtensionRangeOptions::InitAsDefaultInstance() {
 }
 class ExtensionRangeOptions::_Internal {
  public:
-  using HasBits = decltype(std::declval<ExtensionRangeOptions>()._has_bits_);
 };
 
 ExtensionRangeOptions::ExtensionRangeOptions(::PROTOBUF_NAMESPACE_ID::Arena* arena)
@@ -3382,7 +3372,6 @@ ExtensionRangeOptions::ExtensionRangeOptions(::PROTOBUF_NAMESPACE_ID::Arena* are
 }
 ExtensionRangeOptions::ExtensionRangeOptions(const ExtensionRangeOptions& from)
   : ::PROTOBUF_NAMESPACE_ID::Message(),
-      _has_bits_(from._has_bits_),
       uninterpreted_option_(from.uninterpreted_option_) {
   _internal_metadata_.MergeFrom<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(from._internal_metadata_);
   _extensions_.MergeFrom(from._extensions_);
@@ -3426,7 +3415,6 @@ void ExtensionRangeOptions::Clear() {
 
   _extensions_.Clear();
   uninterpreted_option_.Clear();
-  _has_bits_.Clear();
   _internal_metadata_.Clear<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>();
 }
 
@@ -3583,7 +3571,6 @@ void ExtensionRangeOptions::InternalSwap(ExtensionRangeOptions* other) {
   using std::swap;
   _extensions_.Swap(&other->_extensions_);
   _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_);
-  swap(_has_bits_[0], other->_has_bits_[0]);
   uninterpreted_option_.InternalSwap(&other->uninterpreted_option_);
 }
 
@@ -7985,7 +7972,6 @@ void OneofOptions::InitAsDefaultInstance() {
 }
 class OneofOptions::_Internal {
  public:
-  using HasBits = decltype(std::declval<OneofOptions>()._has_bits_);
 };
 
 OneofOptions::OneofOptions(::PROTOBUF_NAMESPACE_ID::Arena* arena)
@@ -7998,7 +7984,6 @@ OneofOptions::OneofOptions(::PROTOBUF_NAMESPACE_ID::Arena* arena)
 }
 OneofOptions::OneofOptions(const OneofOptions& from)
   : ::PROTOBUF_NAMESPACE_ID::Message(),
-      _has_bits_(from._has_bits_),
       uninterpreted_option_(from.uninterpreted_option_) {
   _internal_metadata_.MergeFrom<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(from._internal_metadata_);
   _extensions_.MergeFrom(from._extensions_);
@@ -8042,7 +8027,6 @@ void OneofOptions::Clear() {
 
   _extensions_.Clear();
   uninterpreted_option_.Clear();
-  _has_bits_.Clear();
   _internal_metadata_.Clear<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>();
 }
 
@@ -8199,7 +8183,6 @@ void OneofOptions::InternalSwap(OneofOptions* other) {
   using std::swap;
   _extensions_.Swap(&other->_extensions_);
   _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_);
-  swap(_has_bits_[0], other->_has_bits_[0]);
   uninterpreted_option_.InternalSwap(&other->uninterpreted_option_);
 }
 
@@ -10473,7 +10456,6 @@ void SourceCodeInfo::InitAsDefaultInstance() {
 }
 class SourceCodeInfo::_Internal {
  public:
-  using HasBits = decltype(std::declval<SourceCodeInfo>()._has_bits_);
 };
 
 SourceCodeInfo::SourceCodeInfo(::PROTOBUF_NAMESPACE_ID::Arena* arena)
@@ -10485,7 +10467,6 @@ SourceCodeInfo::SourceCodeInfo(::PROTOBUF_NAMESPACE_ID::Arena* arena)
 }
 SourceCodeInfo::SourceCodeInfo(const SourceCodeInfo& from)
   : ::PROTOBUF_NAMESPACE_ID::Message(),
-      _has_bits_(from._has_bits_),
       location_(from.location_) {
   _internal_metadata_.MergeFrom<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(from._internal_metadata_);
   // @@protoc_insertion_point(copy_constructor:google.protobuf.SourceCodeInfo)
@@ -10527,7 +10508,6 @@ void SourceCodeInfo::Clear() {
   (void) cached_has_bits;
 
   location_.Clear();
-  _has_bits_.Clear();
   _internal_metadata_.Clear<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>();
 }
 
@@ -10665,7 +10645,6 @@ bool SourceCodeInfo::IsInitialized() const {
 void SourceCodeInfo::InternalSwap(SourceCodeInfo* other) {
   using std::swap;
   _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_);
-  swap(_has_bits_[0], other->_has_bits_[0]);
   location_.InternalSwap(&other->location_);
 }
 
@@ -11021,7 +11000,6 @@ void GeneratedCodeInfo::InitAsDefaultInstance() {
 }
 class GeneratedCodeInfo::_Internal {
  public:
-  using HasBits = decltype(std::declval<GeneratedCodeInfo>()._has_bits_);
 };
 
 GeneratedCodeInfo::GeneratedCodeInfo(::PROTOBUF_NAMESPACE_ID::Arena* arena)
@@ -11033,7 +11011,6 @@ GeneratedCodeInfo::GeneratedCodeInfo(::PROTOBUF_NAMESPACE_ID::Arena* arena)
 }
 GeneratedCodeInfo::GeneratedCodeInfo(const GeneratedCodeInfo& from)
   : ::PROTOBUF_NAMESPACE_ID::Message(),
-      _has_bits_(from._has_bits_),
       annotation_(from.annotation_) {
   _internal_metadata_.MergeFrom<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(from._internal_metadata_);
   // @@protoc_insertion_point(copy_constructor:google.protobuf.GeneratedCodeInfo)
@@ -11075,7 +11052,6 @@ void GeneratedCodeInfo::Clear() {
   (void) cached_has_bits;
 
   annotation_.Clear();
-  _has_bits_.Clear();
   _internal_metadata_.Clear<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>();
 }
 
@@ -11213,7 +11189,6 @@ bool GeneratedCodeInfo::IsInitialized() const {
 void GeneratedCodeInfo::InternalSwap(GeneratedCodeInfo* other) {
   using std::swap;
   _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_);
-  swap(_has_bits_[0], other->_has_bits_[0]);
   annotation_.InternalSwap(&other->annotation_);
 }
 

+ 5 - 10
src/google/protobuf/descriptor.pb.h

@@ -477,9 +477,8 @@ class PROTOBUF_EXPORT FileDescriptorSet PROTOBUF_FINAL :
   template <typename T> friend class ::PROTOBUF_NAMESPACE_ID::Arena::InternalHelper;
   typedef void InternalArenaConstructable_;
   typedef void DestructorSkippable_;
-  ::PROTOBUF_NAMESPACE_ID::internal::HasBits<1> _has_bits_;
-  mutable ::PROTOBUF_NAMESPACE_ID::internal::CachedSize _cached_size_;
   ::PROTOBUF_NAMESPACE_ID::RepeatedPtrField< PROTOBUF_NAMESPACE_ID::FileDescriptorProto > file_;
+  mutable ::PROTOBUF_NAMESPACE_ID::internal::CachedSize _cached_size_;
   friend struct ::TableStruct_google_2fprotobuf_2fdescriptor_2eproto;
 };
 // -------------------------------------------------------------------
@@ -1757,9 +1756,8 @@ class PROTOBUF_EXPORT ExtensionRangeOptions PROTOBUF_FINAL :
   template <typename T> friend class ::PROTOBUF_NAMESPACE_ID::Arena::InternalHelper;
   typedef void InternalArenaConstructable_;
   typedef void DestructorSkippable_;
-  ::PROTOBUF_NAMESPACE_ID::internal::HasBits<1> _has_bits_;
-  mutable ::PROTOBUF_NAMESPACE_ID::internal::CachedSize _cached_size_;
   ::PROTOBUF_NAMESPACE_ID::RepeatedPtrField< PROTOBUF_NAMESPACE_ID::UninterpretedOption > uninterpreted_option_;
+  mutable ::PROTOBUF_NAMESPACE_ID::internal::CachedSize _cached_size_;
   friend struct ::TableStruct_google_2fprotobuf_2fdescriptor_2eproto;
 };
 // -------------------------------------------------------------------
@@ -4853,9 +4851,8 @@ class PROTOBUF_EXPORT OneofOptions PROTOBUF_FINAL :
   template <typename T> friend class ::PROTOBUF_NAMESPACE_ID::Arena::InternalHelper;
   typedef void InternalArenaConstructable_;
   typedef void DestructorSkippable_;
-  ::PROTOBUF_NAMESPACE_ID::internal::HasBits<1> _has_bits_;
-  mutable ::PROTOBUF_NAMESPACE_ID::internal::CachedSize _cached_size_;
   ::PROTOBUF_NAMESPACE_ID::RepeatedPtrField< PROTOBUF_NAMESPACE_ID::UninterpretedOption > uninterpreted_option_;
+  mutable ::PROTOBUF_NAMESPACE_ID::internal::CachedSize _cached_size_;
   friend struct ::TableStruct_google_2fprotobuf_2fdescriptor_2eproto;
 };
 // -------------------------------------------------------------------
@@ -6508,9 +6505,8 @@ class PROTOBUF_EXPORT SourceCodeInfo PROTOBUF_FINAL :
   template <typename T> friend class ::PROTOBUF_NAMESPACE_ID::Arena::InternalHelper;
   typedef void InternalArenaConstructable_;
   typedef void DestructorSkippable_;
-  ::PROTOBUF_NAMESPACE_ID::internal::HasBits<1> _has_bits_;
-  mutable ::PROTOBUF_NAMESPACE_ID::internal::CachedSize _cached_size_;
   ::PROTOBUF_NAMESPACE_ID::RepeatedPtrField< PROTOBUF_NAMESPACE_ID::SourceCodeInfo_Location > location_;
+  mutable ::PROTOBUF_NAMESPACE_ID::internal::CachedSize _cached_size_;
   friend struct ::TableStruct_google_2fprotobuf_2fdescriptor_2eproto;
 };
 // -------------------------------------------------------------------
@@ -6884,9 +6880,8 @@ class PROTOBUF_EXPORT GeneratedCodeInfo PROTOBUF_FINAL :
   template <typename T> friend class ::PROTOBUF_NAMESPACE_ID::Arena::InternalHelper;
   typedef void InternalArenaConstructable_;
   typedef void DestructorSkippable_;
-  ::PROTOBUF_NAMESPACE_ID::internal::HasBits<1> _has_bits_;
-  mutable ::PROTOBUF_NAMESPACE_ID::internal::CachedSize _cached_size_;
   ::PROTOBUF_NAMESPACE_ID::RepeatedPtrField< PROTOBUF_NAMESPACE_ID::GeneratedCodeInfo_Annotation > annotation_;
+  mutable ::PROTOBUF_NAMESPACE_ID::internal::CachedSize _cached_size_;
   friend struct ::TableStruct_google_2fprotobuf_2fdescriptor_2eproto;
 };
 // ===================================================================

+ 95 - 39
src/google/protobuf/map.h

@@ -41,6 +41,7 @@
 #include <iterator>
 #include <limits>  // To support Visual Studio 2008
 #include <set>
+#include <type_traits>
 #include <utility>
 
 #include <google/protobuf/stubs/common.h>
@@ -184,6 +185,35 @@ struct DerefCompare {
   bool operator()(const Key* n0, const Key* n1) const { return *n0 < *n1; }
 };
 
+// This class is used to get trivially destructible views of std::string and
+// MapKey, which are the only non-trivially destructible allowed key types.
+template <typename Key>
+class KeyView {
+ public:
+  KeyView(const Key& key) : key_(&key) {}  // NOLINT(runtime/explicit)
+
+  const Key& get() const { return *key_; }
+  // Allows implicit conversions to `const Key&`, which allows us to use the
+  // hasher defined for Key.
+  operator const Key&() const { return get(); }  // NOLINT(runtime/explicit)
+
+  bool operator==(const KeyView& other) const { return get() == other.get(); }
+  bool operator==(const Key& other) const { return get() == other; }
+  bool operator<(const KeyView& other) const { return get() < other.get(); }
+  bool operator<(const Key& other) const { return get() < other; }
+
+ private:
+  const Key* key_;
+};
+
+// Allows the InnerMap type to support skippable destruction.
+template <typename Key>
+struct GetTrivialKey {
+  using type =
+      typename std::conditional<std::is_trivially_destructible<Key>::value, Key,
+                                KeyView<Key>>::type;
+};
+
 }  // namespace internal
 
 // This is the class for Map's internal value_type. Instead of using
@@ -282,26 +312,27 @@ class Map {
   }
 
  private:
-  void Init() {
-    elements_ =
-        Arena::Create<InnerMap>(arena_, 0u, hasher(), Allocator(arena_));
-  }
-
-  // InnerMap's key type is Key and its value type is value_type*.  We use a
-  // custom class here and for Node, below, to ensure that k_ is at offset 0,
-  // allowing safe conversion from pointer to Node to pointer to Key, and vice
-  // versa when appropriate.
+  void Init() { elements_ = Arena::CreateMessage<InnerMap>(arena_, 0u); }
+
+  // InnerMap's key type is TrivialKey and its value type is value_type*.  We
+  // use a custom class here and for Node, below, to ensure that k_ is at offset
+  // 0, allowing safe conversion from pointer to Node to pointer to TrivialKey,
+  // and vice versa when appropriate.  We use GetTrivialKey to adapt Key to
+  // be a trivially destructible view if Key is not already trivially
+  // destructible.  This view points into the Key inside v_ once it's
+  // initialized.
+  using TrivialKey = typename internal::GetTrivialKey<Key>::type;
   class KeyValuePair {
    public:
-    KeyValuePair(const Key& k, value_type* v) : k_(k), v_(v) {}
+    KeyValuePair(const TrivialKey& k, value_type* v) : k_(k), v_(v) {}
 
-    const Key& key() const { return k_; }
-    Key& key() { return k_; }
+    const TrivialKey& key() const { return k_; }
+    TrivialKey& key() { return k_; }
     value_type* value() const { return v_; }
     value_type*& value() { return v_; }
 
    private:
-    Key k_;
+    TrivialKey k_;
     value_type* v_;
   };
 
@@ -336,19 +367,28 @@ class Map {
   // 8. Mutations to a map do not invalidate the map's iterators, pointers to
   //    elements, or references to elements.
   // 9. Except for erase(iterator), any non-const method can reorder iterators.
+  // 10. InnerMap's key is TrivialKey, which is either Key, if Key is trivially
+  //    destructible, or a trivially destructible view of Key otherwise. This
+  //    allows InnerMap's destructor to be skipped when InnerMap is
+  //    arena-allocated.
   class InnerMap : private hasher {
    public:
     using Value = value_type*;
 
-    InnerMap(size_type n, hasher h, Allocator alloc)
-        : hasher(h),
+    explicit InnerMap(size_type n) : InnerMap(nullptr, n) {}
+    InnerMap(Arena* arena, size_type n)
+        : hasher(),
           num_elements_(0),
           seed_(Seed()),
           table_(nullptr),
-          alloc_(alloc) {
+          alloc_(arena) {
       n = TableSize(n);
       table_ = CreateEmptyTable(n);
       num_buckets_ = index_of_first_non_null_ = n;
+      static_assert(
+          std::is_trivially_destructible<KeyValuePair>::value,
+          "We require KeyValuePair to be trivially destructible so that we can "
+          "skip InnerMap's destructor when it's arena allocated.");
     }
 
     ~InnerMap() {
@@ -369,17 +409,19 @@ class Map {
 
     // This is safe only if the given pointer is known to point to a Key that is
     // part of a Node.
-    static Node* NodePtrFromKeyPtr(Key* k) {
+    static Node* NodePtrFromKeyPtr(TrivialKey* k) {
       return reinterpret_cast<Node*>(k);
     }
 
-    static Key* KeyPtrFromNodePtr(Node* node) { return &node->kv.key(); }
+    static TrivialKey* KeyPtrFromNodePtr(Node* node) { return &node->kv.key(); }
 
     // Trees.  The payload type is pointer to Key, so that we can query the tree
     // with Keys that are not in any particular data structure.  When we insert,
     // though, the pointer is always pointing to a Key that is inside a Node.
-    using KeyPtrAllocator = typename Allocator::template rebind<Key*>::other;
-    using Tree = std::set<Key*, internal::DerefCompare<Key>, KeyPtrAllocator>;
+    using KeyPtrAllocator =
+        typename Allocator::template rebind<TrivialKey*>::other;
+    using Tree = std::set<TrivialKey*, internal::DerefCompare<TrivialKey>,
+                          KeyPtrAllocator>;
     using TreeIterator = typename Tree::iterator;
 
     // iterator and const_iterator are instantiations of iterator_base.
@@ -499,7 +541,7 @@ class Map {
         // Well, bucket_index_ still might be correct, but probably
         // not.  Revalidate just to be sure.  This case is rare enough that we
         // don't worry about potential optimizations, such as having a custom
-        // find-like method that compares Node* instead of const Key&.
+        // find-like method that compares Node* instead of TrivialKey.
         iterator_base i(m_->find(*KeyPtrFromNodePtr(node_), it));
         bucket_index_ = i.bucket_index_;
         return m_->TableEntryIsList(bucket_index_);
@@ -558,9 +600,9 @@ class Map {
     size_type size() const { return num_elements_; }
     bool empty() const { return size() == 0; }
 
-    iterator find(const Key& k) { return iterator(FindHelper(k).first); }
-    const_iterator find(const Key& k) const { return find(k, nullptr); }
-    bool contains(const Key& k) const { return find(k) != end(); }
+    iterator find(const TrivialKey& k) { return iterator(FindHelper(k).first); }
+    const_iterator find(const TrivialKey& k) const { return find(k, nullptr); }
+    bool contains(const TrivialKey& k) const { return find(k) != end(); }
 
     // In traditional C++ style, this performs "insert if not present."
     std::pair<iterator, bool> insert(const KeyValuePair& kv) {
@@ -582,7 +624,7 @@ class Map {
 
     // The same, but if an insertion is necessary then the value portion of the
     // inserted key-value pair is left uninitialized.
-    std::pair<iterator, bool> insert(const Key& k) {
+    std::pair<iterator, bool> insert(const TrivialKey& k) {
       std::pair<const_iterator, size_type> p = FindHelper(k);
       // Case 1: key was already present.
       if (p.first.node_ != nullptr)
@@ -593,16 +635,19 @@ class Map {
       }
       const size_type b = p.second;  // bucket number
       Node* node = Alloc<Node>(1);
-      using KeyAllocator = typename Allocator::template rebind<Key>::other;
+      using KeyAllocator =
+          typename Allocator::template rebind<TrivialKey>::other;
       KeyAllocator(alloc_).construct(&node->kv.key(), k);
       iterator result = InsertUnique(b, node);
       ++num_elements_;
       return std::make_pair(result, true);
     }
 
-    Value& operator[](const Key& k) {
+    // Returns iterator so that outer map can update the TrivialKey to point to
+    // the Key inside value_type in case TrivialKey is a view type.
+    iterator operator[](const TrivialKey& k) {
       KeyValuePair kv(k, Value());
-      return insert(kv).first->value();
+      return insert(kv).first;
     }
 
     void erase(iterator it) {
@@ -639,13 +684,13 @@ class Map {
     }
 
    private:
-    const_iterator find(const Key& k, TreeIterator* it) const {
+    const_iterator find(const TrivialKey& k, TreeIterator* it) const {
       return FindHelper(k, it).first;
     }
-    std::pair<const_iterator, size_type> FindHelper(const Key& k) const {
+    std::pair<const_iterator, size_type> FindHelper(const TrivialKey& k) const {
       return FindHelper(k, nullptr);
     }
-    std::pair<const_iterator, size_type> FindHelper(const Key& k,
+    std::pair<const_iterator, size_type> FindHelper(const TrivialKey& k,
                                                     TreeIterator* it) const {
       size_type b = BucketNumber(k);
       if (TableEntryIsNonEmptyList(b)) {
@@ -661,7 +706,7 @@ class Map {
         GOOGLE_DCHECK_EQ(table_[b], table_[b ^ 1]);
         b &= ~static_cast<size_t>(1);
         Tree* tree = static_cast<Tree*>(table_[b]);
-        Key* key = const_cast<Key*>(&k);
+        TrivialKey* key = const_cast<TrivialKey*>(&k);
         typename Tree::iterator tree_it = tree->find(key);
         if (tree_it != tree->end()) {
           if (it != nullptr) *it = tree_it;
@@ -886,14 +931,13 @@ class Map {
       return count >= kMaxLength;
     }
 
-    size_type BucketNumber(const Key& k) const {
-      // We inherit from hasher, so one-arg operator() provides a hash function.
-      size_type h = (*const_cast<InnerMap*>(this))(k);
+    size_type BucketNumber(const TrivialKey& k) const {
+      size_type h = hash_function()(k);
       return (h + seed_) & (num_buckets_ - 1);
     }
 
-    bool IsMatch(const Key& k0, const Key& k1) const {
-      return std::equal_to<Key>()(k0, k1);
+    bool IsMatch(const TrivialKey& k0, const TrivialKey& k1) const {
+      return k0 == k1;
     }
 
     // Return a power of two no less than max(kMinTableSize, n).
@@ -949,6 +993,10 @@ class Map {
       return s;
     }
 
+    friend class Arena;
+    using InternalArenaConstructable_ = void;
+    using DestructorSkippable_ = void;
+
     size_type num_elements_;
     size_type num_buckets_;
     size_type seed_;
@@ -1050,9 +1098,12 @@ class Map {
 
   // Element access
   T& operator[](const key_type& key) {
-    value_type** value = &(*elements_)[key];
+    typename InnerMap::iterator it = (*elements_)[key];
+    value_type** value = &it->value();
     if (*value == nullptr) {
       *value = CreateValueTypeInternal(key);
+      // We need to update the key in case it's a view type.
+      it->key() = (*value)->first;
       internal::MapValueInitializer<is_proto_enum<T>::value, T>::Initialize(
           (*value)->second, default_enum_value_);
     }
@@ -1106,6 +1157,8 @@ class Map {
         elements_->insert(value.first);
     if (p.second) {
       p.first->value() = CreateValueTypeInternal(value);
+      // We need to update the key in case it's a view type.
+      p.first->key() = p.first->value()->first;
     }
     return std::pair<iterator, bool>(iterator(p.first), p.second);
   }
@@ -1133,9 +1186,12 @@ class Map {
     }
   }
   iterator erase(iterator pos) {
-    if (arena_ == nullptr) delete pos.operator->();
+    value_type* value = pos.operator->();
     iterator i = pos++;
     elements_->erase(i.it_);
+    // Note: we need to delete the value after erasing from the inner map
+    // because the inner map's key may be a view of the value's key.
+    if (arena_ == nullptr) delete value;
     return pos;
   }
   void erase(iterator first, iterator last) {

+ 206 - 206
src/google/protobuf/map_field.h

@@ -56,8 +56,213 @@
 namespace google {
 namespace protobuf {
 class DynamicMessage;
-class MapKey;
 class MapIterator;
+
+#define TYPE_CHECK(EXPECTEDTYPE, METHOD)                                   \
+  if (type() != EXPECTEDTYPE) {                                            \
+    GOOGLE_LOG(FATAL) << "Protocol Buffer map usage error:\n"                     \
+               << METHOD << " type does not match\n"                       \
+               << "  Expected : "                                          \
+               << FieldDescriptor::CppTypeName(EXPECTEDTYPE) << "\n"       \
+               << "  Actual   : " << FieldDescriptor::CppTypeName(type()); \
+  }
+
+// MapKey is an union type for representing any possible
+// map key.
+class PROTOBUF_EXPORT MapKey {
+ public:
+  MapKey() : type_(0) {}
+  MapKey(const MapKey& other) : type_(0) { CopyFrom(other); }
+
+  MapKey& operator=(const MapKey& other) {
+    CopyFrom(other);
+    return *this;
+  }
+
+  ~MapKey() {
+    if (type_ == FieldDescriptor::CPPTYPE_STRING) {
+      val_.string_value_.Destruct();
+    }
+  }
+
+  FieldDescriptor::CppType type() const {
+    if (type_ == 0) {
+      GOOGLE_LOG(FATAL) << "Protocol Buffer map usage error:\n"
+                 << "MapKey::type MapKey is not initialized. "
+                 << "Call set methods to initialize MapKey.";
+    }
+    return (FieldDescriptor::CppType)type_;
+  }
+
+  void SetInt64Value(int64 value) {
+    SetType(FieldDescriptor::CPPTYPE_INT64);
+    val_.int64_value_ = value;
+  }
+  void SetUInt64Value(uint64 value) {
+    SetType(FieldDescriptor::CPPTYPE_UINT64);
+    val_.uint64_value_ = value;
+  }
+  void SetInt32Value(int32 value) {
+    SetType(FieldDescriptor::CPPTYPE_INT32);
+    val_.int32_value_ = value;
+  }
+  void SetUInt32Value(uint32 value) {
+    SetType(FieldDescriptor::CPPTYPE_UINT32);
+    val_.uint32_value_ = value;
+  }
+  void SetBoolValue(bool value) {
+    SetType(FieldDescriptor::CPPTYPE_BOOL);
+    val_.bool_value_ = value;
+  }
+  void SetStringValue(std::string val) {
+    SetType(FieldDescriptor::CPPTYPE_STRING);
+    *val_.string_value_.get_mutable() = std::move(val);
+  }
+
+  int64 GetInt64Value() const {
+    TYPE_CHECK(FieldDescriptor::CPPTYPE_INT64, "MapKey::GetInt64Value");
+    return val_.int64_value_;
+  }
+  uint64 GetUInt64Value() const {
+    TYPE_CHECK(FieldDescriptor::CPPTYPE_UINT64, "MapKey::GetUInt64Value");
+    return val_.uint64_value_;
+  }
+  int32 GetInt32Value() const {
+    TYPE_CHECK(FieldDescriptor::CPPTYPE_INT32, "MapKey::GetInt32Value");
+    return val_.int32_value_;
+  }
+  uint32 GetUInt32Value() const {
+    TYPE_CHECK(FieldDescriptor::CPPTYPE_UINT32, "MapKey::GetUInt32Value");
+    return val_.uint32_value_;
+  }
+  bool GetBoolValue() const {
+    TYPE_CHECK(FieldDescriptor::CPPTYPE_BOOL, "MapKey::GetBoolValue");
+    return val_.bool_value_;
+  }
+  const std::string& GetStringValue() const {
+    TYPE_CHECK(FieldDescriptor::CPPTYPE_STRING, "MapKey::GetStringValue");
+    return val_.string_value_.get();
+  }
+
+  bool operator<(const MapKey& other) const {
+    if (type_ != other.type_) {
+      // We could define a total order that handles this case, but
+      // there currently no need.  So, for now, fail.
+      GOOGLE_LOG(FATAL) << "Unsupported: type mismatch";
+    }
+    switch (type()) {
+      case FieldDescriptor::CPPTYPE_DOUBLE:
+      case FieldDescriptor::CPPTYPE_FLOAT:
+      case FieldDescriptor::CPPTYPE_ENUM:
+      case FieldDescriptor::CPPTYPE_MESSAGE:
+        GOOGLE_LOG(FATAL) << "Unsupported";
+        return false;
+      case FieldDescriptor::CPPTYPE_STRING:
+        return val_.string_value_.get() < other.val_.string_value_.get();
+      case FieldDescriptor::CPPTYPE_INT64:
+        return val_.int64_value_ < other.val_.int64_value_;
+      case FieldDescriptor::CPPTYPE_INT32:
+        return val_.int32_value_ < other.val_.int32_value_;
+      case FieldDescriptor::CPPTYPE_UINT64:
+        return val_.uint64_value_ < other.val_.uint64_value_;
+      case FieldDescriptor::CPPTYPE_UINT32:
+        return val_.uint32_value_ < other.val_.uint32_value_;
+      case FieldDescriptor::CPPTYPE_BOOL:
+        return val_.bool_value_ < other.val_.bool_value_;
+    }
+    return false;
+  }
+
+  bool operator==(const MapKey& other) const {
+    if (type_ != other.type_) {
+      // To be consistent with operator<, we don't allow this either.
+      GOOGLE_LOG(FATAL) << "Unsupported: type mismatch";
+    }
+    switch (type()) {
+      case FieldDescriptor::CPPTYPE_DOUBLE:
+      case FieldDescriptor::CPPTYPE_FLOAT:
+      case FieldDescriptor::CPPTYPE_ENUM:
+      case FieldDescriptor::CPPTYPE_MESSAGE:
+        GOOGLE_LOG(FATAL) << "Unsupported";
+        break;
+      case FieldDescriptor::CPPTYPE_STRING:
+        return val_.string_value_.get() == other.val_.string_value_.get();
+      case FieldDescriptor::CPPTYPE_INT64:
+        return val_.int64_value_ == other.val_.int64_value_;
+      case FieldDescriptor::CPPTYPE_INT32:
+        return val_.int32_value_ == other.val_.int32_value_;
+      case FieldDescriptor::CPPTYPE_UINT64:
+        return val_.uint64_value_ == other.val_.uint64_value_;
+      case FieldDescriptor::CPPTYPE_UINT32:
+        return val_.uint32_value_ == other.val_.uint32_value_;
+      case FieldDescriptor::CPPTYPE_BOOL:
+        return val_.bool_value_ == other.val_.bool_value_;
+    }
+    GOOGLE_LOG(FATAL) << "Can't get here.";
+    return false;
+  }
+
+  void CopyFrom(const MapKey& other) {
+    SetType(other.type());
+    switch (type_) {
+      case FieldDescriptor::CPPTYPE_DOUBLE:
+      case FieldDescriptor::CPPTYPE_FLOAT:
+      case FieldDescriptor::CPPTYPE_ENUM:
+      case FieldDescriptor::CPPTYPE_MESSAGE:
+        GOOGLE_LOG(FATAL) << "Unsupported";
+        break;
+      case FieldDescriptor::CPPTYPE_STRING:
+        *val_.string_value_.get_mutable() = other.val_.string_value_.get();
+        break;
+      case FieldDescriptor::CPPTYPE_INT64:
+        val_.int64_value_ = other.val_.int64_value_;
+        break;
+      case FieldDescriptor::CPPTYPE_INT32:
+        val_.int32_value_ = other.val_.int32_value_;
+        break;
+      case FieldDescriptor::CPPTYPE_UINT64:
+        val_.uint64_value_ = other.val_.uint64_value_;
+        break;
+      case FieldDescriptor::CPPTYPE_UINT32:
+        val_.uint32_value_ = other.val_.uint32_value_;
+        break;
+      case FieldDescriptor::CPPTYPE_BOOL:
+        val_.bool_value_ = other.val_.bool_value_;
+        break;
+    }
+  }
+
+ private:
+  template <typename K, typename V>
+  friend class internal::TypeDefinedMapFieldBase;
+  friend class ::PROTOBUF_NAMESPACE_ID::MapIterator;
+  friend class internal::DynamicMapField;
+
+  union KeyValue {
+    KeyValue() {}
+    internal::ExplicitlyConstructed<std::string> string_value_;
+    int64 int64_value_;
+    int32 int32_value_;
+    uint64 uint64_value_;
+    uint32 uint32_value_;
+    bool bool_value_;
+  } val_;
+
+  void SetType(FieldDescriptor::CppType type) {
+    if (type_ == type) return;
+    if (type_ == FieldDescriptor::CPPTYPE_STRING) {
+      val_.string_value_.Destruct();
+    }
+    type_ = type;
+    if (type_ == FieldDescriptor::CPPTYPE_STRING) {
+      val_.string_value_.DefaultConstruct();
+    }
+  }
+
+  // type_ is 0 or a valid FieldDescriptor::CppType.
+  int type_;
+};
+
 namespace internal {
 
 class ContendedMapCleanTest;
@@ -374,211 +579,6 @@ class PROTOBUF_EXPORT DynamicMapField
 
 }  // namespace internal
 
-#define TYPE_CHECK(EXPECTEDTYPE, METHOD)                                   \
-  if (type() != EXPECTEDTYPE) {                                            \
-    GOOGLE_LOG(FATAL) << "Protocol Buffer map usage error:\n"                     \
-               << METHOD << " type does not match\n"                       \
-               << "  Expected : "                                          \
-               << FieldDescriptor::CppTypeName(EXPECTEDTYPE) << "\n"       \
-               << "  Actual   : " << FieldDescriptor::CppTypeName(type()); \
-  }
-
-// MapKey is an union type for representing any possible
-// map key.
-class PROTOBUF_EXPORT MapKey {
- public:
-  MapKey() : type_(0) {}
-  MapKey(const MapKey& other) : type_(0) { CopyFrom(other); }
-
-  MapKey& operator=(const MapKey& other) {
-    CopyFrom(other);
-    return *this;
-  }
-
-  ~MapKey() {
-    if (type_ == FieldDescriptor::CPPTYPE_STRING) {
-      val_.string_value_.Destruct();
-    }
-  }
-
-  FieldDescriptor::CppType type() const {
-    if (type_ == 0) {
-      GOOGLE_LOG(FATAL) << "Protocol Buffer map usage error:\n"
-                 << "MapKey::type MapKey is not initialized. "
-                 << "Call set methods to initialize MapKey.";
-    }
-    return (FieldDescriptor::CppType)type_;
-  }
-
-  void SetInt64Value(int64 value) {
-    SetType(FieldDescriptor::CPPTYPE_INT64);
-    val_.int64_value_ = value;
-  }
-  void SetUInt64Value(uint64 value) {
-    SetType(FieldDescriptor::CPPTYPE_UINT64);
-    val_.uint64_value_ = value;
-  }
-  void SetInt32Value(int32 value) {
-    SetType(FieldDescriptor::CPPTYPE_INT32);
-    val_.int32_value_ = value;
-  }
-  void SetUInt32Value(uint32 value) {
-    SetType(FieldDescriptor::CPPTYPE_UINT32);
-    val_.uint32_value_ = value;
-  }
-  void SetBoolValue(bool value) {
-    SetType(FieldDescriptor::CPPTYPE_BOOL);
-    val_.bool_value_ = value;
-  }
-  void SetStringValue(std::string val) {
-    SetType(FieldDescriptor::CPPTYPE_STRING);
-    *val_.string_value_.get_mutable() = std::move(val);
-  }
-
-  int64 GetInt64Value() const {
-    TYPE_CHECK(FieldDescriptor::CPPTYPE_INT64, "MapKey::GetInt64Value");
-    return val_.int64_value_;
-  }
-  uint64 GetUInt64Value() const {
-    TYPE_CHECK(FieldDescriptor::CPPTYPE_UINT64, "MapKey::GetUInt64Value");
-    return val_.uint64_value_;
-  }
-  int32 GetInt32Value() const {
-    TYPE_CHECK(FieldDescriptor::CPPTYPE_INT32, "MapKey::GetInt32Value");
-    return val_.int32_value_;
-  }
-  uint32 GetUInt32Value() const {
-    TYPE_CHECK(FieldDescriptor::CPPTYPE_UINT32, "MapKey::GetUInt32Value");
-    return val_.uint32_value_;
-  }
-  bool GetBoolValue() const {
-    TYPE_CHECK(FieldDescriptor::CPPTYPE_BOOL, "MapKey::GetBoolValue");
-    return val_.bool_value_;
-  }
-  const std::string& GetStringValue() const {
-    TYPE_CHECK(FieldDescriptor::CPPTYPE_STRING, "MapKey::GetStringValue");
-    return val_.string_value_.get();
-  }
-
-  bool operator<(const MapKey& other) const {
-    if (type_ != other.type_) {
-      // We could define a total order that handles this case, but
-      // there currently no need.  So, for now, fail.
-      GOOGLE_LOG(FATAL) << "Unsupported: type mismatch";
-    }
-    switch (type()) {
-      case FieldDescriptor::CPPTYPE_DOUBLE:
-      case FieldDescriptor::CPPTYPE_FLOAT:
-      case FieldDescriptor::CPPTYPE_ENUM:
-      case FieldDescriptor::CPPTYPE_MESSAGE:
-        GOOGLE_LOG(FATAL) << "Unsupported";
-        return false;
-      case FieldDescriptor::CPPTYPE_STRING:
-        return val_.string_value_.get() < other.val_.string_value_.get();
-      case FieldDescriptor::CPPTYPE_INT64:
-        return val_.int64_value_ < other.val_.int64_value_;
-      case FieldDescriptor::CPPTYPE_INT32:
-        return val_.int32_value_ < other.val_.int32_value_;
-      case FieldDescriptor::CPPTYPE_UINT64:
-        return val_.uint64_value_ < other.val_.uint64_value_;
-      case FieldDescriptor::CPPTYPE_UINT32:
-        return val_.uint32_value_ < other.val_.uint32_value_;
-      case FieldDescriptor::CPPTYPE_BOOL:
-        return val_.bool_value_ < other.val_.bool_value_;
-    }
-    return false;
-  }
-
-  bool operator==(const MapKey& other) const {
-    if (type_ != other.type_) {
-      // To be consistent with operator<, we don't allow this either.
-      GOOGLE_LOG(FATAL) << "Unsupported: type mismatch";
-    }
-    switch (type()) {
-      case FieldDescriptor::CPPTYPE_DOUBLE:
-      case FieldDescriptor::CPPTYPE_FLOAT:
-      case FieldDescriptor::CPPTYPE_ENUM:
-      case FieldDescriptor::CPPTYPE_MESSAGE:
-        GOOGLE_LOG(FATAL) << "Unsupported";
-        break;
-      case FieldDescriptor::CPPTYPE_STRING:
-        return val_.string_value_.get() == other.val_.string_value_.get();
-      case FieldDescriptor::CPPTYPE_INT64:
-        return val_.int64_value_ == other.val_.int64_value_;
-      case FieldDescriptor::CPPTYPE_INT32:
-        return val_.int32_value_ == other.val_.int32_value_;
-      case FieldDescriptor::CPPTYPE_UINT64:
-        return val_.uint64_value_ == other.val_.uint64_value_;
-      case FieldDescriptor::CPPTYPE_UINT32:
-        return val_.uint32_value_ == other.val_.uint32_value_;
-      case FieldDescriptor::CPPTYPE_BOOL:
-        return val_.bool_value_ == other.val_.bool_value_;
-    }
-    GOOGLE_LOG(FATAL) << "Can't get here.";
-    return false;
-  }
-
-  void CopyFrom(const MapKey& other) {
-    SetType(other.type());
-    switch (type_) {
-      case FieldDescriptor::CPPTYPE_DOUBLE:
-      case FieldDescriptor::CPPTYPE_FLOAT:
-      case FieldDescriptor::CPPTYPE_ENUM:
-      case FieldDescriptor::CPPTYPE_MESSAGE:
-        GOOGLE_LOG(FATAL) << "Unsupported";
-        break;
-      case FieldDescriptor::CPPTYPE_STRING:
-        *val_.string_value_.get_mutable() = other.val_.string_value_.get();
-        break;
-      case FieldDescriptor::CPPTYPE_INT64:
-        val_.int64_value_ = other.val_.int64_value_;
-        break;
-      case FieldDescriptor::CPPTYPE_INT32:
-        val_.int32_value_ = other.val_.int32_value_;
-        break;
-      case FieldDescriptor::CPPTYPE_UINT64:
-        val_.uint64_value_ = other.val_.uint64_value_;
-        break;
-      case FieldDescriptor::CPPTYPE_UINT32:
-        val_.uint32_value_ = other.val_.uint32_value_;
-        break;
-      case FieldDescriptor::CPPTYPE_BOOL:
-        val_.bool_value_ = other.val_.bool_value_;
-        break;
-    }
-  }
-
- private:
-  template <typename K, typename V>
-  friend class internal::TypeDefinedMapFieldBase;
-  friend class ::PROTOBUF_NAMESPACE_ID::MapIterator;
-  friend class internal::DynamicMapField;
-
-  union KeyValue {
-    KeyValue() {}
-    internal::ExplicitlyConstructed<std::string> string_value_;
-    int64 int64_value_;
-    int32 int32_value_;
-    uint64 uint64_value_;
-    uint32 uint32_value_;
-    bool bool_value_;
-  } val_;
-
-  void SetType(FieldDescriptor::CppType type) {
-    if (type_ == type) return;
-    if (type_ == FieldDescriptor::CPPTYPE_STRING) {
-      val_.string_value_.Destruct();
-    }
-    type_ = type;
-    if (type_ == FieldDescriptor::CPPTYPE_STRING) {
-      val_.string_value_.DefaultConstruct();
-    }
-  }
-
-  // type_ is 0 or a valid FieldDescriptor::CppType.
-  int type_;
-};
-
 // MapValueRef points to a map value.
 class PROTOBUF_EXPORT MapValueRef {
  public:

+ 0 - 30
src/google/protobuf/test_messages_proto2.proto

@@ -1,33 +1,3 @@
-// Protocol Buffers - Google's data interchange format
-// Copyright 2008 Google Inc.  All rights reserved.
-// https://developers.google.com/protocol-buffers/
-//
-// Redistribution and use in source and binary forms, with or without
-// modification, are permitted provided that the following conditions are
-// met:
-//
-//     * Redistributions of source code must retain the above copyright
-// notice, this list of conditions and the following disclaimer.
-//     * Redistributions in binary form must reproduce the above
-// copyright notice, this list of conditions and the following disclaimer
-// in the documentation and/or other materials provided with the
-// distribution.
-//     * Neither the name of Google Inc. nor the names of its
-// contributors may be used to endorse or promote products derived from
-// this software without specific prior written permission.
-//
-// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
-// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
-// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
-// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
-// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
-// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
-// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
-// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
-// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
-// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
-// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-
 // Protocol Buffers - Google's data interchange format
 // Copyright 2008 Google Inc.  All rights reserved.
 // https://developers.google.com/protocol-buffers/

+ 0 - 30
src/google/protobuf/test_messages_proto3.proto

@@ -1,33 +1,3 @@
-// Protocol Buffers - Google's data interchange format
-// Copyright 2008 Google Inc.  All rights reserved.
-// https://developers.google.com/protocol-buffers/
-//
-// Redistribution and use in source and binary forms, with or without
-// modification, are permitted provided that the following conditions are
-// met:
-//
-//     * Redistributions of source code must retain the above copyright
-// notice, this list of conditions and the following disclaimer.
-//     * Redistributions in binary form must reproduce the above
-// copyright notice, this list of conditions and the following disclaimer
-// in the documentation and/or other materials provided with the
-// distribution.
-//     * Neither the name of Google Inc. nor the names of its
-// contributors may be used to endorse or promote products derived from
-// this software without specific prior written permission.
-//
-// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
-// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
-// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
-// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
-// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
-// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
-// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
-// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
-// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
-// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
-// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-
 // Protocol Buffers - Google's data interchange format
 // Copyright 2008 Google Inc.  All rights reserved.
 // https://developers.google.com/protocol-buffers/

+ 21 - 15
src/google/protobuf/util/internal/expecting_objectwriter.h

@@ -71,21 +71,27 @@ class MockObjectWriter : public ObjectWriter {
  public:
   MockObjectWriter() {}
 
-  MOCK_METHOD1(StartObject, ObjectWriter*(StringPiece));
-  MOCK_METHOD0(EndObject, ObjectWriter*());
-  MOCK_METHOD1(StartList, ObjectWriter*(StringPiece));
-  MOCK_METHOD0(EndList, ObjectWriter*());
-  MOCK_METHOD2(RenderBool, ObjectWriter*(StringPiece, bool));
-  MOCK_METHOD2(RenderInt32, ObjectWriter*(StringPiece, int32));
-  MOCK_METHOD2(RenderUint32, ObjectWriter*(StringPiece, uint32));
-  MOCK_METHOD2(RenderInt64, ObjectWriter*(StringPiece, int64));
-  MOCK_METHOD2(RenderUint64, ObjectWriter*(StringPiece, uint64));
-  MOCK_METHOD2(RenderDouble, ObjectWriter*(StringPiece, double));
-  MOCK_METHOD2(RenderFloat, ObjectWriter*(StringPiece, float));
-  MOCK_METHOD2(RenderString,
-               ObjectWriter*(StringPiece, StringPiece));
-  MOCK_METHOD2(RenderBytes, ObjectWriter*(StringPiece, StringPiece));
-  MOCK_METHOD1(RenderNull, ObjectWriter*(StringPiece));
+  MOCK_METHOD(ObjectWriter*, StartObject, (StringPiece), (override));
+  MOCK_METHOD(ObjectWriter*, EndObject, (), (override));
+  MOCK_METHOD(ObjectWriter*, StartList, (StringPiece), (override));
+  MOCK_METHOD(ObjectWriter*, EndList, (), (override));
+  MOCK_METHOD(ObjectWriter*, RenderBool, (StringPiece, bool), (override));
+  MOCK_METHOD(ObjectWriter*, RenderInt32, (StringPiece, int32),
+              (override));
+  MOCK_METHOD(ObjectWriter*, RenderUint32, (StringPiece, uint32),
+              (override));
+  MOCK_METHOD(ObjectWriter*, RenderInt64, (StringPiece, int64),
+              (override));
+  MOCK_METHOD(ObjectWriter*, RenderUint64, (StringPiece, uint64),
+              (override));
+  MOCK_METHOD(ObjectWriter*, RenderDouble, (StringPiece, double),
+              (override));
+  MOCK_METHOD(ObjectWriter*, RenderFloat, (StringPiece, float),
+              (override));
+  MOCK_METHOD(ObjectWriter*, RenderString,
+              (StringPiece, StringPiece), (override));
+  MOCK_METHOD(ObjectWriter*, RenderBytes, (StringPiece, StringPiece));
+  MOCK_METHOD(ObjectWriter*, RenderNull, (StringPiece), (override));
 };
 
 class ExpectingObjectWriter : public ObjectWriter {

+ 12 - 8
src/google/protobuf/util/internal/mock_error_listener.h

@@ -46,14 +46,18 @@ class MockErrorListener : public ErrorListener {
   MockErrorListener() {}
   virtual ~MockErrorListener() {}
 
-  MOCK_METHOD3(InvalidName,
-               void(const LocationTrackerInterface& loc,
-                    StringPiece unknown_name, StringPiece message));
-  MOCK_METHOD3(InvalidValue,
-               void(const LocationTrackerInterface& loc,
-                    StringPiece type_name, StringPiece value));
-  MOCK_METHOD2(MissingField, void(const LocationTrackerInterface& loc,
-                                  StringPiece missing_name));
+  MOCK_METHOD(void, InvalidName,
+              (const LocationTrackerInterface& loc,
+               StringPiece unknown_name, StringPiece message),
+              (override));
+  MOCK_METHOD(void, InvalidValue,
+              (const LocationTrackerInterface& loc, StringPiece type_name,
+               StringPiece value),
+              (override));
+  MOCK_METHOD(void, MissingField,
+              (const LocationTrackerInterface& loc,
+               StringPiece missing_name),
+              (override));
 };
 
 }  // namespace converter