Browse Source

Merge branch 'sync-piper' into sync-stage

Joshua Haberman 5 years ago
parent
commit
3dcf368c2f
32 changed files with 304 additions and 738 deletions
  1. 0 3
      cmake/tests.cmake
  2. 17 6
      conformance/ConformanceJava.java
  3. 0 1
      conformance/failure_list_java.txt
  4. 1 1
      java/util/src/main/java/com/google/protobuf/util/Timestamps.java
  5. 0 2
      python/setup.py
  6. 0 9
      src/Makefile.am
  7. 1 24
      src/google/protobuf/arena_unittest.cc
  8. 4 12
      src/google/protobuf/compiler/cpp/cpp_message.cc
  9. 0 64
      src/google/protobuf/compiler/cpp/cpp_unittest.inc
  10. 4 5
      src/google/protobuf/descriptor_database.cc
  11. 3 1
      src/google/protobuf/extension_set_inl.h
  12. 49 2
      src/google/protobuf/generated_message_reflection.cc
  13. 8 0
      src/google/protobuf/generated_message_reflection.h
  14. 1 2
      src/google/protobuf/generated_message_table_driven_lite.h
  15. 76 204
      src/google/protobuf/map.h
  16. 36 39
      src/google/protobuf/map_lite_unittest.proto
  17. 0 5
      src/google/protobuf/map_test_util_impl.h
  18. 33 38
      src/google/protobuf/map_unittest.proto
  19. 16 0
      src/google/protobuf/message.h
  20. 27 15
      src/google/protobuf/message_lite.h
  21. 2 2
      src/google/protobuf/message_unittest.inc
  22. 4 2
      src/google/protobuf/port_def.inc
  23. 1 0
      src/google/protobuf/port_undef.inc
  24. 6 4
      src/google/protobuf/reflection_ops.cc
  25. 1 1
      src/google/protobuf/struct.pb.h
  26. 7 1
      src/google/protobuf/text_format.cc
  27. 2 0
      src/google/protobuf/text_format.h
  28. 2 5
      src/google/protobuf/unittest_arena.proto
  29. 0 206
      src/google/protobuf/unittest_no_arena.proto
  30. 0 39
      src/google/protobuf/unittest_no_arena_import.proto
  31. 0 44
      src/google/protobuf/unittest_no_arena_lite.proto
  32. 3 1
      src/google/protobuf/wire_format.cc

+ 0 - 3
cmake/tests.cmake

@@ -33,7 +33,6 @@ set(lite_test_protos
   google/protobuf/unittest_import_lite.proto
   google/protobuf/unittest_import_public_lite.proto
   google/protobuf/unittest_lite.proto
-  google/protobuf/unittest_no_arena_lite.proto
 )
 
 set(tests_protos
@@ -56,8 +55,6 @@ set(tests_protos
   google/protobuf/unittest_lite_imports_nonlite.proto
   google/protobuf/unittest_mset.proto
   google/protobuf/unittest_mset_wire_format.proto
-  google/protobuf/unittest_no_arena.proto
-  google/protobuf/unittest_no_arena_import.proto
   google/protobuf/unittest_no_field_presence.proto
   google/protobuf/unittest_no_generic_services.proto
   google/protobuf/unittest_optimize_for.proto

+ 17 - 6
conformance/ConformanceJava.java

@@ -236,8 +236,10 @@ class ConformanceJava {
 
   private Conformance.ConformanceResponse doTest(Conformance.ConformanceRequest request) {
     com.google.protobuf.AbstractMessage testMessage;
-    boolean isProto3 = request.getMessageType().equals("protobuf_test_messages.proto3.TestAllTypesProto3");
-    boolean isProto2 = request.getMessageType().equals("protobuf_test_messages.proto2.TestAllTypesProto2");
+    boolean isProto3 =
+        request.getMessageType().equals("protobuf_test_messages.proto3.TestAllTypesProto3");
+    boolean isProto2 =
+        request.getMessageType().equals("protobuf_test_messages.proto2.TestAllTypesProto2");
 
     switch (request.getPayloadCase()) {
       case PROTOBUF_PAYLOAD: {
@@ -264,15 +266,24 @@ class ConformanceJava {
       }
       case JSON_PAYLOAD: {
         try {
-          TestMessagesProto3.TestAllTypesProto3.Builder builder =
-              TestMessagesProto3.TestAllTypesProto3.newBuilder();
           JsonFormat.Parser parser = JsonFormat.parser().usingTypeRegistry(typeRegistry);
           if (request.getTestCategory()
               == Conformance.TestCategory.JSON_IGNORE_UNKNOWN_PARSING_TEST) {
             parser = parser.ignoringUnknownFields();
           }
-          parser.merge(request.getJsonPayload(), builder);
-          testMessage = builder.build();
+          if (isProto3) {
+            TestMessagesProto3.TestAllTypesProto3.Builder builder =
+                TestMessagesProto3.TestAllTypesProto3.newBuilder();
+            parser.merge(request.getJsonPayload(), builder);
+            testMessage = builder.build();
+          } else if (isProto2) {
+            TestMessagesProto2.TestAllTypesProto2.Builder builder =
+                TestMessagesProto2.TestAllTypesProto2.newBuilder();
+            parser.merge(request.getJsonPayload(), builder);
+            testMessage = builder.build();
+          } else {
+            throw new RuntimeException("Protobuf request doesn't have specific payload type.");
+          }
         } catch (InvalidProtocolBufferException e) {
           return Conformance.ConformanceResponse.newBuilder().setParseError(e.getMessage()).build();
         }

+ 0 - 1
conformance/failure_list_java.txt

@@ -46,4 +46,3 @@ Required.Proto3.ProtobufInput.PrematureEofInDelimitedDataForKnownNonRepeatedValu
 Required.Proto3.ProtobufInput.PrematureEofInDelimitedDataForKnownRepeatedValue.MESSAGE
 Required.Proto2.ProtobufInput.PrematureEofInDelimitedDataForKnownNonRepeatedValue.MESSAGE
 Required.Proto2.ProtobufInput.PrematureEofInDelimitedDataForKnownRepeatedValue.MESSAGE
-Required.Proto2.JsonInput.StoresDefaultPrimitive.Validator

+ 1 - 1
java/util/src/main/java/com/google/protobuf/util/Timestamps.java

@@ -92,7 +92,7 @@ public final class Timestamps {
     SimpleDateFormat sdf = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss", Locale.ENGLISH);
     GregorianCalendar calendar = new GregorianCalendar(TimeZone.getTimeZone("UTC"));
     // We use Proleptic Gregorian Calendar (i.e., Gregorian calendar extends
-    // backwards to year one) for timestamp formating.
+    // backwards to year one) for timestamp formatting.
     calendar.setGregorianChange(new Date(Long.MIN_VALUE));
     sdf.setCalendar(calendar);
     return sdf;

+ 0 - 2
python/setup.py

@@ -82,8 +82,6 @@ def GenerateUnittestProtos():
   generate_proto("../src/google/protobuf/test_messages_proto3.proto", False)
   generate_proto("../src/google/protobuf/test_messages_proto2.proto", False)
   generate_proto("../src/google/protobuf/unittest_arena.proto", False)
-  generate_proto("../src/google/protobuf/unittest_no_arena.proto", False)
-  generate_proto("../src/google/protobuf/unittest_no_arena_import.proto", False)
   generate_proto("../src/google/protobuf/unittest.proto", False)
   generate_proto("../src/google/protobuf/unittest_custom_options.proto", False)
   generate_proto("../src/google/protobuf/unittest_import.proto", False)

+ 0 - 9
src/Makefile.am

@@ -497,9 +497,6 @@ protoc_inputs =                                                   \
   google/protobuf/unittest_lite.proto                             \
   google/protobuf/unittest_mset.proto                             \
   google/protobuf/unittest_mset_wire_format.proto                 \
-  google/protobuf/unittest_no_arena_lite.proto                    \
-  google/protobuf/unittest_no_arena_import.proto                  \
-  google/protobuf/unittest_no_arena.proto                         \
   google/protobuf/unittest_no_field_presence.proto                \
   google/protobuf/unittest_no_generic_services.proto              \
   google/protobuf/unittest_optimize_for.proto                     \
@@ -573,8 +570,6 @@ protoc_lite_outputs =                                          \
   google/protobuf/map_lite_unittest.pb.h                       \
   google/protobuf/unittest_lite.pb.cc                          \
   google/protobuf/unittest_lite.pb.h                           \
-  google/protobuf/unittest_no_arena_lite.pb.cc                 \
-  google/protobuf/unittest_no_arena_lite.pb.h                  \
   google/protobuf/unittest_import_lite.pb.cc                   \
   google/protobuf/unittest_import_lite.pb.h                    \
   google/protobuf/unittest_import_public_lite.pb.cc            \
@@ -620,10 +615,6 @@ protoc_outputs =                                                  \
   google/protobuf/unittest_mset.pb.h                              \
   google/protobuf/unittest_mset_wire_format.pb.cc                 \
   google/protobuf/unittest_mset_wire_format.pb.h                  \
-  google/protobuf/unittest_no_arena_import.pb.cc                  \
-  google/protobuf/unittest_no_arena_import.pb.h                   \
-  google/protobuf/unittest_no_arena.pb.cc                         \
-  google/protobuf/unittest_no_arena.pb.h                          \
   google/protobuf/unittest_no_field_presence.pb.cc                \
   google/protobuf/unittest_no_field_presence.pb.h                 \
   google/protobuf/unittest_no_generic_services.pb.cc              \

+ 1 - 24
src/google/protobuf/arena_unittest.cc

@@ -44,7 +44,6 @@
 #include <google/protobuf/test_util.h>
 #include <google/protobuf/unittest.pb.h>
 #include <google/protobuf/unittest_arena.pb.h>
-#include <google/protobuf/unittest_no_arena.pb.h>
 #include <google/protobuf/io/coded_stream.h>
 #include <google/protobuf/io/zero_copy_stream_impl_lite.h>
 #include <google/protobuf/descriptor.h>
@@ -66,7 +65,6 @@ using protobuf_unittest::TestAllExtensions;
 using protobuf_unittest::TestAllTypes;
 using protobuf_unittest::TestEmptyMessage;
 using protobuf_unittest::TestOneof2;
-using protobuf_unittest_no_arena::TestNoArenaMessage;
 
 namespace google {
 namespace protobuf {
@@ -159,14 +157,12 @@ class MustBeConstructedWithOneThroughEight {
 TEST(ArenaTest, ArenaConstructable) {
   EXPECT_TRUE(Arena::is_arena_constructable<TestAllTypes>::type::value);
   EXPECT_TRUE(Arena::is_arena_constructable<const TestAllTypes>::type::value);
-  EXPECT_FALSE(Arena::is_arena_constructable<TestNoArenaMessage>::type::value);
   EXPECT_FALSE(Arena::is_arena_constructable<Arena>::type::value);
 }
 
 TEST(ArenaTest, DestructorSkippable) {
   EXPECT_TRUE(Arena::is_destructor_skippable<TestAllTypes>::type::value);
   EXPECT_TRUE(Arena::is_destructor_skippable<const TestAllTypes>::type::value);
-  EXPECT_FALSE(Arena::is_destructor_skippable<TestNoArenaMessage>::type::value);
   EXPECT_FALSE(Arena::is_destructor_skippable<Arena>::type::value);
 }
 
@@ -465,13 +461,6 @@ TEST(ArenaTest, SetAllocatedMessage) {
   nested->set_bb(118);
   arena_message->set_allocated_optional_nested_message(nested);
   EXPECT_EQ(118, arena_message->optional_nested_message().bb());
-
-  TestNoArenaMessage no_arena_message;
-  EXPECT_FALSE(no_arena_message.has_arena_message());
-  no_arena_message.set_allocated_arena_message(NULL);
-  EXPECT_FALSE(no_arena_message.has_arena_message());
-  no_arena_message.set_allocated_arena_message(new ArenaMessage);
-  EXPECT_TRUE(no_arena_message.has_arena_message());
 }
 
 TEST(ArenaTest, ReleaseMessage) {
@@ -676,15 +665,8 @@ TEST(ArenaTest, AddAllocatedWithReflection) {
   ArenaMessage* arena1_message = Arena::CreateMessage<ArenaMessage>(&arena1);
   const Reflection* r = arena1_message->GetReflection();
   const Descriptor* d = arena1_message->GetDescriptor();
-  const FieldDescriptor* fd =
-      d->FindFieldByName("repeated_import_no_arena_message");
-  // Message with cc_enable_arenas = false;
-  r->AddMessage(arena1_message, fd);
-  r->AddMessage(arena1_message, fd);
-  r->AddMessage(arena1_message, fd);
-  EXPECT_EQ(3, r->FieldSize(*arena1_message, fd));
   // Message with cc_enable_arenas = true;
-  fd = d->FindFieldByName("repeated_nested_message");
+  const FieldDescriptor* fd = d->FindFieldByName("repeated_nested_message");
   r->AddMessage(arena1_message, fd);
   r->AddMessage(arena1_message, fd);
   r->AddMessage(arena1_message, fd);
@@ -1334,11 +1316,6 @@ TEST(ArenaTest, GetArenaShouldReturnNullForNonArenaAllocatedMessages) {
 }
 
 TEST(ArenaTest, GetArenaShouldReturnNullForNonArenaCompatibleTypes) {
-  TestNoArenaMessage message;
-  const TestNoArenaMessage* const_pointer_to_message = &message;
-  EXPECT_EQ(nullptr, Arena::GetArena(&message));
-  EXPECT_EQ(nullptr, Arena::GetArena(const_pointer_to_message));
-
   // Test that GetArena returns nullptr for types that have a GetArena method
   // that doesn't return Arena*.
   struct {

+ 4 - 12
src/google/protobuf/compiler/cpp/cpp_message.cc

@@ -1035,7 +1035,7 @@ void MessageGenerator::GenerateClassDefinition(io::Printer* printer) {
         "    ::$proto_ns$::internal::WireFormatLite::$val_wire_type$,\n"
         "    $default_enum_value$ > SuperType;\n"
         "  $classname$();\n"
-        "  $classname$(::$proto_ns$::Arena* arena);\n"
+        "  explicit $classname$(::$proto_ns$::Arena* arena);\n"
         "  void MergeFrom(const $classname$& other);\n"
         "  static const $classname$* internal_default_instance() { return "
         "reinterpret_cast<const "
@@ -2408,18 +2408,13 @@ std::pair<size_t, size_t> MessageGenerator::GenerateOffsets(
   } else {
     format("~0u,  // no _weak_field_map_\n");
   }
-  int num_stripped = 0;
-  for (auto field : FieldRange(descriptor_)) {
-    if (!IsFieldUsed(field, options_)) {
-      num_stripped++;
-    }
-  }
   const int kNumGenericOffsets = 5;  // the number of fixed offsets above
   const size_t offsets = kNumGenericOffsets + descriptor_->field_count() +
-                         descriptor_->real_oneof_decl_count() - num_stripped;
+                         descriptor_->real_oneof_decl_count();
   size_t entries = offsets;
   for (auto field : FieldRange(descriptor_)) {
     if (!IsFieldUsed(field, options_)) {
+      format("~0u,  // stripped\n");
       continue;
     }
     if (field->real_containing_oneof() || field->options().weak()) {
@@ -2450,11 +2445,8 @@ std::pair<size_t, size_t> MessageGenerator::GenerateOffsets(
         "0,\n"
         "1,\n");
   } else if (!has_bit_indices_.empty()) {
-    entries += has_bit_indices_.size() - num_stripped;
+    entries += has_bit_indices_.size();
     for (int i = 0; i < has_bit_indices_.size(); i++) {
-      if (!IsFieldUsed(descriptor_->field(i), options_)) {
-        continue;
-      }
       const std::string index =
           has_bit_indices_[i] >= 0 ? StrCat(has_bit_indices_[i]) : "~0u";
       format("$1$,\n", index);

+ 0 - 64
src/google/protobuf/compiler/cpp/cpp_unittest.inc

@@ -48,7 +48,6 @@
 #include <vector>
 
 #include <google/protobuf/compiler/cpp/cpp_unittest.h>
-#include <google/protobuf/unittest_no_arena.pb.h>
 #include <google/protobuf/stubs/strutil.h>
 #if !defined(GOOGLE_PROTOBUF_CMAKE_BUILD) && !defined(_MSC_VER)
 // We exclude this large proto from cmake build because it's too large for
@@ -402,69 +401,6 @@ TEST(GENERATED_MESSAGE_TEST_NAME, StringCharStarLength) {
   EXPECT_EQ("wx", message.repeated_string(0));
 }
 
-TEST(GENERATED_MESSAGE_TEST_NAME, StringMove) {
-  // Verify that we trigger the move behavior on a scalar setter.
-  protobuf_unittest_no_arena::TestAllTypes message;
-  {
-    std::string tmp(32, 'a');
-
-    const char* old_data = tmp.data();
-    message.set_optional_string(std::move(tmp));
-    const char* new_data = message.optional_string().data();
-
-    EXPECT_EQ(old_data, new_data);
-    EXPECT_EQ(std::string(32, 'a'), message.optional_string());
-
-    std::string tmp2(32, 'b');
-    old_data = tmp2.data();
-    message.set_optional_string(std::move(tmp2));
-    new_data = message.optional_string().data();
-
-    EXPECT_EQ(old_data, new_data);
-    EXPECT_EQ(std::string(32, 'b'), message.optional_string());
-  }
-
-  // Verify that we trigger the move behavior on a oneof setter.
-  {
-    std::string tmp(32, 'a');
-
-    const char* old_data = tmp.data();
-    message.set_oneof_string(std::move(tmp));
-    const char* new_data = message.oneof_string().data();
-
-    EXPECT_EQ(old_data, new_data);
-    EXPECT_EQ(std::string(32, 'a'), message.oneof_string());
-
-    std::string tmp2(32, 'b');
-    old_data = tmp2.data();
-    message.set_oneof_string(std::move(tmp2));
-    new_data = message.oneof_string().data();
-
-    EXPECT_EQ(old_data, new_data);
-    EXPECT_EQ(std::string(32, 'b'), message.oneof_string());
-  }
-
-  // Verify that we trigger the move behavior on a repeated setter.
-  {
-    std::string tmp(32, 'a');
-
-    const char* old_data = tmp.data();
-    message.add_repeated_string(std::move(tmp));
-    const char* new_data = message.repeated_string(0).data();
-
-    EXPECT_EQ(old_data, new_data);
-    EXPECT_EQ(std::string(32, 'a'), message.repeated_string(0));
-
-    std::string tmp2(32, 'b');
-    old_data = tmp2.data();
-    message.set_repeated_string(0, std::move(tmp2));
-    new_data = message.repeated_string(0).data();
-
-    EXPECT_EQ(old_data, new_data);
-    EXPECT_EQ(std::string(32, 'b'), message.repeated_string(0));
-  }
-}
-
 
 TEST(GENERATED_MESSAGE_TEST_NAME, CopyFrom) {
   UNITTEST::TestAllTypes message1, message2;

+ 4 - 5
src/google/protobuf/descriptor_database.cc

@@ -165,8 +165,8 @@ bool ValidateSymbolName(StringPiece name) {
 // symbol name.  Since upper_bound() returns the *first* key that sorts
 // *greater* than the input, we want the element immediately before that.
 template <typename Container, typename Key>
-typename Container::const_iterator FindLastLessOrEqual(Container* container,
-                                                       const Key& key) {
+typename Container::const_iterator FindLastLessOrEqual(
+    const Container* container, const Key& key) {
   auto iter = container->upper_bound(key);
   if (iter != container->begin()) --iter;
   return iter;
@@ -174,9 +174,8 @@ typename Container::const_iterator FindLastLessOrEqual(Container* container,
 
 // As above, but using std::upper_bound instead.
 template <typename Container, typename Key, typename Cmp>
-typename Container::const_iterator FindLastLessOrEqual(Container* container,
-                                                       const Key& key,
-                                                       const Cmp& cmp) {
+typename Container::const_iterator FindLastLessOrEqual(
+    const Container* container, const Key& key, const Cmp& cmp) {
   auto iter = std::upper_bound(container->begin(), container->end(), key, cmp);
   if (iter != container->begin()) --iter;
   return iter;

+ 3 - 1
src/google/protobuf/extension_set_inl.h

@@ -207,6 +207,7 @@ const char* ExtensionSet::ParseMessageSetItemTmpl(
     internal::InternalMetadata* metadata, internal::ParseContext* ctx) {
   std::string payload;
   uint32 type_id = 0;
+  bool payload_read = false;
   while (!ctx->Done(&ptr)) {
     uint32 tag = static_cast<uint8>(*ptr++);
     if (tag == WireFormatLite::kMessageSetTypeIdTag) {
@@ -214,7 +215,7 @@ const char* ExtensionSet::ParseMessageSetItemTmpl(
       ptr = ParseBigVarint(ptr, &tmp);
       GOOGLE_PROTOBUF_PARSER_ASSERT(ptr);
       type_id = tmp;
-      if (!payload.empty()) {
+      if (payload_read) {
         ExtensionInfo extension;
         bool was_packed_on_wire;
         if (!FindExtension(2, type_id, containing_type, ctx, &extension,
@@ -253,6 +254,7 @@ const char* ExtensionSet::ParseMessageSetItemTmpl(
         GOOGLE_PROTOBUF_PARSER_ASSERT(ptr);
         ptr = ctx->ReadString(ptr, size, &payload);
         GOOGLE_PROTOBUF_PARSER_ASSERT(ptr);
+        payload_read = true;
       }
     } else {
       ptr = ReadTag(ptr - 1, &tag);

+ 49 - 2
src/google/protobuf/generated_message_reflection.cc

@@ -171,6 +171,12 @@ static void ReportReflectionUsageEnumTypeError(
              << value->full_name();
 }
 
+inline void CheckInvalidAccess(const internal::ReflectionSchema& schema,
+                               const FieldDescriptor* field) {
+  GOOGLE_CHECK(!schema.IsFieldStripped(field))
+      << "invalid access to a stripped field " << field->full_name();
+}
+
 #define USAGE_CHECK(CONDITION, METHOD, ERROR_DESCRIPTION) \
   if (!(CONDITION))                                       \
   ReportReflectionUsageError(descriptor_, field, #METHOD, ERROR_DESCRIPTION)
@@ -352,6 +358,8 @@ size_t Reflection::SpaceUsedLong(const Message& message) const {
 
 void Reflection::SwapField(Message* message1, Message* message2,
                            const FieldDescriptor* field) const {
+  CheckInvalidAccess(schema_, field);
+
   if (field->is_repeated()) {
     switch (field->cpp_type()) {
 #define SWAP_ARRAYS(CPPTYPE, TYPE)                                 \
@@ -649,6 +657,7 @@ void Reflection::Swap(Message* message1, Message* message2) const {
   for (int i = 0; i <= last_non_weak_field_index_; i++) {
     const FieldDescriptor* field = descriptor_->field(i);
     if (schema_.InRealOneof(field)) continue;
+    if (schema_.IsFieldStripped(field)) continue;
     SwapField(message1, message2, field);
   }
   const int oneof_decl_count = descriptor_->oneof_decl_count();
@@ -694,6 +703,7 @@ void Reflection::SwapFields(
   const int fields_size = static_cast<int>(fields.size());
   for (int i = 0; i < fields_size; i++) {
     const FieldDescriptor* field = fields[i];
+    CheckInvalidAccess(schema_, field);
     if (field->is_extension()) {
       MutableExtensionSet(message1)->SwapExtension(
           MutableExtensionSet(message2), field->number());
@@ -725,6 +735,7 @@ bool Reflection::HasField(const Message& message,
                           const FieldDescriptor* field) const {
   USAGE_CHECK_MESSAGE_TYPE(HasField);
   USAGE_CHECK_SINGULAR(HasField);
+  CheckInvalidAccess(schema_, field);
 
   if (field->is_extension()) {
     return GetExtensionSet(message).Has(field->number());
@@ -741,6 +752,7 @@ int Reflection::FieldSize(const Message& message,
                           const FieldDescriptor* field) const {
   USAGE_CHECK_MESSAGE_TYPE(FieldSize);
   USAGE_CHECK_REPEATED(FieldSize);
+  CheckInvalidAccess(schema_, field);
 
   if (field->is_extension()) {
     return GetExtensionSet(message).ExtensionSize(field->number());
@@ -785,6 +797,7 @@ int Reflection::FieldSize(const Message& message,
 void Reflection::ClearField(Message* message,
                             const FieldDescriptor* field) const {
   USAGE_CHECK_MESSAGE_TYPE(ClearField);
+  CheckInvalidAccess(schema_, field);
 
   if (field->is_extension()) {
     MutableExtensionSet(message)->ClearExtension(field->number());
@@ -899,6 +912,7 @@ void Reflection::RemoveLast(Message* message,
                             const FieldDescriptor* field) const {
   USAGE_CHECK_MESSAGE_TYPE(RemoveLast);
   USAGE_CHECK_REPEATED(RemoveLast);
+  CheckInvalidAccess(schema_, field);
 
   if (field->is_extension()) {
     MutableExtensionSet(message)->RemoveLast(field->number());
@@ -946,6 +960,7 @@ void Reflection::RemoveLast(Message* message,
 Message* Reflection::ReleaseLast(Message* message,
                                  const FieldDescriptor* field) const {
   USAGE_CHECK_ALL(ReleaseLast, REPEATED, MESSAGE);
+  CheckInvalidAccess(schema_, field);
 
   if (field->is_extension()) {
     return static_cast<Message*>(
@@ -966,6 +981,7 @@ void Reflection::SwapElements(Message* message, const FieldDescriptor* field,
                               int index1, int index2) const {
   USAGE_CHECK_MESSAGE_TYPE(Swap);
   USAGE_CHECK_REPEATED(Swap);
+  CheckInvalidAccess(schema_, field);
 
   if (field->is_extension()) {
     MutableExtensionSet(message)->SwapElements(field->number(), index1, index2);
@@ -1030,8 +1046,9 @@ bool CreateUnknownEnumValues(const FieldDescriptor* field) {
 }  // namespace internal
 using internal::CreateUnknownEnumValues;
 
-void Reflection::ListFields(const Message& message,
-                            std::vector<const FieldDescriptor*>* output) const {
+void Reflection::ListFieldsMayFailOnStripped(
+    const Message& message, bool should_fail,
+    std::vector<const FieldDescriptor*>* output) const {
   output->clear();
 
   // Optimization:  The default instance never has any fields set.
@@ -1048,6 +1065,9 @@ void Reflection::ListFields(const Message& message,
   output->reserve(descriptor_->field_count());
   for (int i = 0; i <= last_non_weak_field_index_; i++) {
     const FieldDescriptor* field = descriptor_->field(i);
+    if (!should_fail && schema_.IsFieldStripped(field)) {
+      continue;
+    }
     if (field->is_repeated()) {
       if (FieldSize(message, field) > 0) {
         output->push_back(field);
@@ -1080,6 +1100,16 @@ void Reflection::ListFields(const Message& message,
   std::sort(output->begin(), output->end(), FieldNumberSorter());
 }
 
+void Reflection::ListFields(const Message& message,
+                            std::vector<const FieldDescriptor*>* output) const {
+  ListFieldsMayFailOnStripped(message, true, output);
+}
+
+void Reflection::ListFieldsOmitStripped(
+    const Message& message, std::vector<const FieldDescriptor*>* output) const {
+  ListFieldsMayFailOnStripped(message, false, output);
+}
+
 // -------------------------------------------------------------------
 
 #undef DEFINE_PRIMITIVE_ACCESSORS
@@ -1449,6 +1479,7 @@ const Message& Reflection::GetMessage(const Message& message,
                                       const FieldDescriptor* field,
                                       MessageFactory* factory) const {
   USAGE_CHECK_ALL(GetMessage, SINGULAR, MESSAGE);
+  CheckInvalidAccess(schema_, field);
 
   if (factory == nullptr) factory = message_factory_;
 
@@ -1468,6 +1499,7 @@ Message* Reflection::MutableMessage(Message* message,
                                     const FieldDescriptor* field,
                                     MessageFactory* factory) const {
   USAGE_CHECK_ALL(MutableMessage, SINGULAR, MESSAGE);
+  CheckInvalidAccess(schema_, field);
 
   if (factory == nullptr) factory = message_factory_;
 
@@ -1503,6 +1535,7 @@ void Reflection::UnsafeArenaSetAllocatedMessage(
     Message* message, Message* sub_message,
     const FieldDescriptor* field) const {
   USAGE_CHECK_ALL(SetAllocatedMessage, SINGULAR, MESSAGE);
+  CheckInvalidAccess(schema_, field);
 
   if (field->is_extension()) {
     MutableExtensionSet(message)->UnsafeArenaSetAllocatedMessage(
@@ -1534,6 +1567,8 @@ void Reflection::UnsafeArenaSetAllocatedMessage(
 
 void Reflection::SetAllocatedMessage(Message* message, Message* sub_message,
                                      const FieldDescriptor* field) const {
+  CheckInvalidAccess(schema_, field);
+
   // If message and sub-message are in different memory ownership domains
   // (different arenas, or one is on heap and one is not), then we may need to
   // do a copy.
@@ -1562,6 +1597,7 @@ Message* Reflection::UnsafeArenaReleaseMessage(Message* message,
                                                const FieldDescriptor* field,
                                                MessageFactory* factory) const {
   USAGE_CHECK_ALL(ReleaseMessage, SINGULAR, MESSAGE);
+  CheckInvalidAccess(schema_, field);
 
   if (factory == nullptr) factory = message_factory_;
 
@@ -1590,6 +1626,8 @@ Message* Reflection::UnsafeArenaReleaseMessage(Message* message,
 Message* Reflection::ReleaseMessage(Message* message,
                                     const FieldDescriptor* field,
                                     MessageFactory* factory) const {
+  CheckInvalidAccess(schema_, field);
+
   Message* released = UnsafeArenaReleaseMessage(message, field, factory);
   if (GetArena(message) != nullptr && released != nullptr) {
     Message* copy_from_arena = released->New();
@@ -1603,6 +1641,7 @@ const Message& Reflection::GetRepeatedMessage(const Message& message,
                                               const FieldDescriptor* field,
                                               int index) const {
   USAGE_CHECK_ALL(GetRepeatedMessage, REPEATED, MESSAGE);
+  CheckInvalidAccess(schema_, field);
 
   if (field->is_extension()) {
     return static_cast<const Message&>(
@@ -1623,6 +1662,7 @@ Message* Reflection::MutableRepeatedMessage(Message* message,
                                             const FieldDescriptor* field,
                                             int index) const {
   USAGE_CHECK_ALL(MutableRepeatedMessage, REPEATED, MESSAGE);
+  CheckInvalidAccess(schema_, field);
 
   if (field->is_extension()) {
     return static_cast<Message*>(
@@ -1643,6 +1683,7 @@ Message* Reflection::MutableRepeatedMessage(Message* message,
 Message* Reflection::AddMessage(Message* message, const FieldDescriptor* field,
                                 MessageFactory* factory) const {
   USAGE_CHECK_ALL(AddMessage, REPEATED, MESSAGE);
+  CheckInvalidAccess(schema_, field);
 
   if (factory == nullptr) factory = message_factory_;
 
@@ -1685,6 +1726,7 @@ void Reflection::AddAllocatedMessage(Message* message,
                                      const FieldDescriptor* field,
                                      Message* new_entry) const {
   USAGE_CHECK_ALL(AddAllocatedMessage, REPEATED, MESSAGE);
+  CheckInvalidAccess(schema_, field);
 
   if (field->is_extension()) {
     MutableExtensionSet(message)->AddAllocatedMessage(field, new_entry);
@@ -1706,6 +1748,8 @@ void* Reflection::MutableRawRepeatedField(Message* message,
                                           int ctype,
                                           const Descriptor* desc) const {
   USAGE_CHECK_REPEATED("MutableRawRepeatedField");
+  CheckInvalidAccess(schema_, field);
+
   if (field->cpp_type() != cpptype &&
       (field->cpp_type() != FieldDescriptor::CPPTYPE_ENUM ||
        cpptype != FieldDescriptor::CPPTYPE_INT32))
@@ -1931,6 +1975,9 @@ bool Reflection::HasBit(const Message& message,
     return IsIndexInHasBitSet(GetHasBits(message), schema_.HasBitIndex(field));
   }
 
+  // Intentionally check here because HasBitIndex(field) != -1 means valid.
+  CheckInvalidAccess(schema_, field);
+
   // proto3: no has-bits. All fields present except messages, which are
   // present only if their message-field pointer is non-null.
   if (field->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE) {

+ 8 - 0
src/google/protobuf/generated_message_reflection.h

@@ -212,6 +212,14 @@ struct ReflectionSchema {
            OffsetValue(offsets_[field->index()], field->type());
   }
 
+  bool IsFieldStripped(const FieldDescriptor* field) const {
+    return false;
+  }
+
+  bool IsMessageStripped(const Descriptor* descriptor) const {
+    return false;
+  }
+
 
   bool HasWeakFields() const { return weak_field_map_offset_ > 0; }
 

+ 1 - 2
src/google/protobuf/generated_message_table_driven_lite.h

@@ -31,10 +31,9 @@
 #ifndef GOOGLE_PROTOBUF_GENERATED_MESSAGE_TABLE_DRIVEN_LITE_H__
 #define GOOGLE_PROTOBUF_GENERATED_MESSAGE_TABLE_DRIVEN_LITE_H__
 
-#include <google/protobuf/generated_message_table_driven.h>
-
 #include <google/protobuf/io/zero_copy_stream_impl_lite.h>
 #include <google/protobuf/extension_set.h>
+#include <google/protobuf/generated_message_table_driven.h>
 #include <google/protobuf/implicit_weak_message.h>
 #include <google/protobuf/inlined_string_field.h>
 #include <google/protobuf/repeated_field.h>

+ 76 - 204
src/google/protobuf/map.h

@@ -41,7 +41,7 @@
 #include <initializer_list>
 #include <iterator>
 #include <limits>  // To support Visual Studio 2008
-#include <set>
+#include <map>
 #include <type_traits>
 #include <utility>
 
@@ -181,39 +181,10 @@ class MapAllocator {
   Arena* const arena_;
 };
 
-template <typename Key>
-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;
-};
+template <typename T>
+using KeyForTree =
+    typename std::conditional<std::is_scalar<T>::value, T,
+                              std::reference_wrapper<const T>>::type;
 
 }  // namespace internal
 
@@ -313,31 +284,9 @@ class Map {
   }
 
  private:
-  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 TrivialKey& k, value_type* v) : k_(k), v_(v) {}
-
-    const TrivialKey& key() const { return k_; }
-    TrivialKey& key() { return k_; }
-    value_type* value() const { return v_; }
-    value_type*& value() { return v_; }
-
-   private:
-    TrivialKey k_;
-    value_type* v_;
-  };
+  void Init() { elements_ = Arena::CreateMessage<InnerMap>(arena_, 0); }
 
-  using Allocator = internal::MapAllocator<KeyValuePair>;
+  using Allocator = internal::MapAllocator<void*>;
 
   // InnerMap is a generic hash-based map.  It doesn't contain any
   // protocol-buffer-specific logic.  It is a chaining hash map with the
@@ -368,14 +317,11 @@ 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.
+  // 10. InnerMap uses KeyForTree<Key> when using the Tree representation, which
+  //    is either `Key`, if Key is a scalar, or `reference_wrapper<const Key>`
+  //    otherwise. This avoids unncessary copies of string keys, for example.
   class InnerMap : private hasher {
    public:
-    using Value = value_type*;
-
     explicit InnerMap(size_type n) : InnerMap(nullptr, n) {}
     InnerMap(Arena* arena, size_type n)
         : hasher(),
@@ -386,10 +332,6 @@ class Map {
       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() {
@@ -404,27 +346,25 @@ class Map {
 
     // Linked-list nodes, as one would expect for a chaining hash table.
     struct Node {
-      KeyValuePair kv;
+      value_type kv;
       Node* next;
     };
 
-    // This is safe only if the given pointer is known to point to a Key that is
-    // part of a Node.
-    static Node* NodePtrFromKeyPtr(TrivialKey* k) {
-      return reinterpret_cast<Node*>(k);
-    }
-
-    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<TrivialKey*>::other;
-    using Tree = std::set<TrivialKey*, internal::DerefCompare<TrivialKey>,
-                          KeyPtrAllocator>;
+    // Trees. The payload type is a copy of Key, so that we can query the tree
+    // with Keys that are not in any particular data structure.
+    // The value is a void* pointing to Node. We use void* instead of Node* to
+    // avoid code bloat. That way there is only one instantiation of the tree
+    // class per key type.
+    using TreeAllocator = typename Allocator::template rebind<
+        std::pair<const internal::KeyForTree<Key>, void*>>::other;
+    using Tree = std::map<internal::KeyForTree<Key>, void*, std::less<Key>,
+                          TreeAllocator>;
     using TreeIterator = typename Tree::iterator;
 
+    static Node* NodeFromTreeIterator(TreeIterator it) {
+      return static_cast<Node*>(it->second);
+    }
+
     // iterator and const_iterator are instantiations of iterator_base.
     template <typename KeyValueType>
     class iterator_base {
@@ -456,7 +396,7 @@ class Map {
           : node_(n), m_(m), bucket_index_(index) {}
 
       iterator_base(TreeIterator tree_it, const InnerMap* m, size_type index)
-          : node_(NodePtrFromKeyPtr(*tree_it)), m_(m), bucket_index_(index) {
+          : node_(NodeFromTreeIterator(tree_it)), m_(m), bucket_index_(index) {
         // Invariant: iterators that use buckets with trees have an even
         // bucket_index_.
         GOOGLE_DCHECK_EQ(bucket_index_ % 2, 0u);
@@ -476,7 +416,7 @@ class Map {
           } else if (m_->TableEntryIsTree(bucket_index_)) {
             Tree* tree = static_cast<Tree*>(m_->table_[bucket_index_]);
             GOOGLE_DCHECK(!tree->empty());
-            node_ = NodePtrFromKeyPtr(*tree->begin());
+            node_ = NodeFromTreeIterator(tree->begin());
             break;
           }
         }
@@ -504,7 +444,7 @@ class Map {
             if (++tree_it == tree->end()) {
               SearchFrom(bucket_index_ + 2);
             } else {
-              node_ = NodePtrFromKeyPtr(*tree_it);
+              node_ = NodeFromTreeIterator(tree_it);
             }
           }
         } else {
@@ -542,8 +482,8 @@ 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 TrivialKey.
-        iterator_base i(m_->find(*KeyPtrFromNodePtr(node_), it));
+        // find-like method that compares Node* instead of the key.
+        iterator_base i(m_->find(node_->kv.first, it));
         bucket_index_ = i.bucket_index_;
         return m_->TableEntryIsList(bucket_index_);
       }
@@ -554,8 +494,8 @@ class Map {
     };
 
    public:
-    using iterator = iterator_base<KeyValuePair>;
-    using const_iterator = iterator_base<const KeyValuePair>;
+    using iterator = iterator_base<value_type>;
+    using const_iterator = iterator_base<const value_type>;
 
     iterator begin() { return iterator(this); }
     iterator end() { return iterator(); }
@@ -578,7 +518,7 @@ class Map {
           table_[b] = table_[b + 1] = nullptr;
           typename Tree::iterator tree_it = tree->begin();
           do {
-            Node* node = NodePtrFromKeyPtr(*tree_it);
+            Node* node = NodeFromTreeIterator(tree_it);
             typename Tree::iterator next = tree_it;
             ++next;
             tree->erase(tree_it);
@@ -601,31 +541,13 @@ class Map {
     size_type size() const { return num_elements_; }
     bool empty() const { return size() == 0; }
 
-    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) {
-      std::pair<const_iterator, size_type> p = FindHelper(kv.key());
-      // Case 1: key was already present.
-      if (p.first.node_ != nullptr)
-        return std::make_pair(iterator(p.first), false);
-      // Case 2: insert.
-      if (ResizeIfLoadIsOutOfRange(num_elements_ + 1)) {
-        p = FindHelper(kv.key());
-      }
-      const size_type b = p.second;  // bucket number
-      Node* node = Alloc<Node>(1);
-      alloc_.construct(&node->kv, kv);
-      iterator result = InsertUnique(b, node);
-      ++num_elements_;
-      return std::make_pair(result, true);
-    }
+    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(); }
 
-    // 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 TrivialKey& k) {
+    // Insert the key into the map, if not present. In that case, the value will
+    // be value initialized.
+    std::pair<iterator, bool> insert(const Key& k) {
       std::pair<const_iterator, size_type> p = FindHelper(k);
       // Case 1: key was already present.
       if (p.first.node_ != nullptr)
@@ -635,21 +557,22 @@ class Map {
         p = FindHelper(k);
       }
       const size_type b = p.second;  // bucket number
-      Node* node = Alloc<Node>(1);
-      using KeyAllocator =
-          typename Allocator::template rebind<TrivialKey>::other;
-      KeyAllocator(alloc_).construct(&node->kv.key(), k);
+      Node* node;
+      if (alloc_.arena() == nullptr) {
+        node = new Node{value_type(k), nullptr};
+      } else {
+        node = Alloc<Node>(1);
+        Arena::CreateInArenaStorage(const_cast<Key*>(&node->kv.first),
+                                    alloc_.arena(), k);
+        Arena::CreateInArenaStorage(&node->kv.second, alloc_.arena());
+      }
+
       iterator result = InsertUnique(b, node);
       ++num_elements_;
       return std::make_pair(result, true);
     }
 
-    // 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_type& operator[](const Key& k) { return *insert(k).first; }
 
     void erase(iterator it) {
       GOOGLE_DCHECK_EQ(it.m_, this);
@@ -665,7 +588,7 @@ class Map {
       } else {
         GOOGLE_DCHECK(TableEntryIsTree(b));
         Tree* tree = static_cast<Tree*>(table_[b]);
-        tree->erase(*tree_it);
+        tree->erase(tree_it);
         if (tree->empty()) {
           // Force b to be the minimum of b and b ^ 1.  This is important
           // only because we want index_of_first_non_null_ to be correct.
@@ -685,19 +608,19 @@ class Map {
     }
 
    private:
-    const_iterator find(const TrivialKey& k, TreeIterator* it) const {
+    const_iterator find(const Key& k, TreeIterator* it) const {
       return FindHelper(k, it).first;
     }
-    std::pair<const_iterator, size_type> FindHelper(const TrivialKey& k) const {
+    std::pair<const_iterator, size_type> FindHelper(const Key& k) const {
       return FindHelper(k, nullptr);
     }
-    std::pair<const_iterator, size_type> FindHelper(const TrivialKey& k,
+    std::pair<const_iterator, size_type> FindHelper(const Key& k,
                                                     TreeIterator* it) const {
       size_type b = BucketNumber(k);
       if (TableEntryIsNonEmptyList(b)) {
         Node* node = static_cast<Node*>(table_[b]);
         do {
-          if (IsMatch(*KeyPtrFromNodePtr(node), k)) {
+          if (IsMatch(node->kv.first, k)) {
             return std::make_pair(const_iterator(node, this, b), b);
           } else {
             node = node->next;
@@ -707,8 +630,7 @@ class Map {
         GOOGLE_DCHECK_EQ(table_[b], table_[b ^ 1]);
         b &= ~static_cast<size_t>(1);
         Tree* tree = static_cast<Tree*>(table_[b]);
-        TrivialKey* key = const_cast<TrivialKey*>(&k);
-        typename Tree::iterator tree_it = tree->find(key);
+        auto tree_it = tree->find(k);
         if (tree_it != tree->end()) {
           if (it != nullptr) *it = tree_it;
           return std::make_pair(const_iterator(tree_it, this, b), b);
@@ -729,7 +651,7 @@ class Map {
       // or whatever.  But it's probably cheap enough to recompute that here;
       // it's likely that we're inserting into an empty or short list.
       iterator result;
-      GOOGLE_DCHECK(find(*KeyPtrFromNodePtr(node)) == end());
+      GOOGLE_DCHECK(find(node->kv.first) == end());
       if (TableEntryIsEmpty(b)) {
         result = InsertUniqueInList(b, node);
       } else if (TableEntryIsNonEmptyList(b)) {
@@ -768,7 +690,7 @@ class Map {
       // Maintain the invariant that node->next is null for all Nodes in Trees.
       node->next = nullptr;
       return iterator(
-          static_cast<Tree*>(table_[b])->insert(KeyPtrFromNodePtr(node)).first,
+          static_cast<Tree*>(table_[b])->insert({node->kv.first, node}).first,
           this, b & ~static_cast<size_t>(1));
     }
 
@@ -836,7 +758,7 @@ class Map {
       Node* node = static_cast<Node*>(table[index]);
       do {
         Node* next = node->next;
-        InsertUnique(BucketNumber(*KeyPtrFromNodePtr(node)), node);
+        InsertUnique(BucketNumber(node->kv.first), node);
         node = next;
       } while (node != nullptr);
     }
@@ -845,8 +767,8 @@ class Map {
       Tree* tree = static_cast<Tree*>(table[index]);
       typename Tree::iterator tree_it = tree->begin();
       do {
-        Node* node = NodePtrFromKeyPtr(*tree_it);
-        InsertUnique(BucketNumber(**tree_it), node);
+        InsertUnique(BucketNumber(tree_it->first),
+                     NodeFromTreeIterator(tree_it));
       } while (++tree_it != tree->end());
       DestroyTree(tree);
     }
@@ -888,15 +810,9 @@ class Map {
 
     void TreeConvert(size_type b) {
       GOOGLE_DCHECK(!TableEntryIsTree(b) && !TableEntryIsTree(b ^ 1));
-      typename Allocator::template rebind<Tree>::other tree_allocator(alloc_);
-      Tree* tree = tree_allocator.allocate(1);
-      // We want to use the three-arg form of construct, if it exists, but we
-      // create a temporary and use the two-arg construct that's known to exist.
-      // It's clunky, but the compiler should be able to generate more-or-less
-      // the same code.
-      tree_allocator.construct(
-          tree, Tree(typename Tree::key_compare(), KeyPtrAllocator(alloc_)));
-      // Now the tree is ready to use.
+      Tree* tree =
+          Arena::Create<Tree>(alloc_.arena(), typename Tree::key_compare(),
+                              typename Tree::allocator_type(alloc_));
       size_type count = CopyListToTree(b, tree) + CopyListToTree(b ^ 1, tree);
       GOOGLE_DCHECK_EQ(count, tree->size());
       table_[b] = table_[b ^ 1] = static_cast<void*>(tree);
@@ -908,7 +824,7 @@ class Map {
       size_type count = 0;
       Node* node = static_cast<Node*>(table_[b]);
       while (node != nullptr) {
-        tree->insert(KeyPtrFromNodePtr(node));
+        tree->insert({node->kv.first, node});
         ++count;
         Node* next = node->next;
         node->next = nullptr;
@@ -932,14 +848,12 @@ class Map {
       return count >= kMaxLength;
     }
 
-    size_type BucketNumber(const TrivialKey& k) const {
+    size_type BucketNumber(const Key& k) const {
       size_type h = hash_function()(k);
       return (h + seed_) & (num_buckets_ - 1);
     }
 
-    bool IsMatch(const TrivialKey& k0, const TrivialKey& k1) const {
-      return k0 == k1;
-    }
+    bool IsMatch(const Key& k0, const Key& k1) const { return k0 == k1; }
 
     // Return a power of two no less than max(kMinTableSize, n).
     // Assumes either n < kMinTableSize or n is a power of two.
@@ -964,14 +878,15 @@ class Map {
     }
 
     void DestroyNode(Node* node) {
-      alloc_.destroy(&node->kv);
-      Dealloc<Node>(node, 1);
+      if (alloc_.arena() == nullptr) {
+        delete node;
+      }
     }
 
     void DestroyTree(Tree* tree) {
-      typename Allocator::template rebind<Tree>::other tree_allocator(alloc_);
-      tree_allocator.destroy(tree);
-      tree_allocator.deallocate(tree, 1);
+      if (alloc_.arena() == nullptr) {
+        delete tree;
+      }
     }
 
     void** CreateEmptyTable(size_type n) {
@@ -1022,7 +937,7 @@ class Map {
     const_iterator() {}
     explicit const_iterator(const InnerIt& it) : it_(it) {}
 
-    const_reference operator*() const { return *it_->value(); }
+    const_reference operator*() const { return *it_; }
     const_pointer operator->() const { return &(operator*()); }
 
     const_iterator& operator++() {
@@ -1055,7 +970,7 @@ class Map {
     iterator() {}
     explicit iterator(const InnerIt& it) : it_(it) {}
 
-    reference operator*() const { return *it_->value(); }
+    reference operator*() const { return *it_; }
     pointer operator->() const { return &(operator*()); }
 
     iterator& operator++() {
@@ -1098,18 +1013,7 @@ class Map {
   bool empty() const { return size() == 0; }
 
   // Element access
-  T& operator[](const key_type& 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_);
-    }
-    return (*value)->second;
-  }
+  T& operator[](const key_type& key) { return (*elements_)[key].second; }
   const T& at(const key_type& key) const {
     const_iterator it = find(key);
     GOOGLE_CHECK(it != end()) << "key not found: " << key;
@@ -1157,9 +1061,7 @@ class Map {
     std::pair<typename InnerMap::iterator, bool> p =
         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;
+      p.first->second = value.second;
     }
     return std::pair<iterator, bool>(iterator(p.first), p.second);
   }
@@ -1187,12 +1089,8 @@ class Map {
     }
   }
   iterator erase(iterator pos) {
-    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) {
@@ -1235,32 +1133,6 @@ class Map {
     default_enum_value_ = default_enum_value;
   }
 
-  value_type* CreateValueTypeInternal(const Key& key) {
-    if (arena_ == nullptr) {
-      return new value_type(key);
-    } else {
-      value_type* value = reinterpret_cast<value_type*>(
-          Arena::CreateArray<uint8>(arena_, sizeof(value_type)));
-      Arena::CreateInArenaStorage(const_cast<Key*>(&value->first), arena_, key);
-      Arena::CreateInArenaStorage(&value->second, arena_);
-      return value;
-    }
-  }
-
-  value_type* CreateValueTypeInternal(const value_type& value) {
-    if (arena_ == nullptr) {
-      return new value_type(value);
-    } else {
-      value_type* p = reinterpret_cast<value_type*>(
-          Arena::CreateArray<uint8>(arena_, sizeof(value_type)));
-      Arena::CreateInArenaStorage(const_cast<Key*>(&p->first), arena_,
-                                  value.first);
-      Arena::CreateInArenaStorage(&p->second, arena_);
-      p->second = value.second;
-      return p;
-    }
-  }
-
   Arena* arena_;
   int default_enum_value_;
   InnerMap* elements_;

+ 36 - 39
src/google/protobuf/map_lite_unittest.proto

@@ -30,55 +30,52 @@
 
 syntax = "proto2";
 
-option cc_enable_arenas = true;
-option optimize_for = LITE_RUNTIME;
+package protobuf_unittest;
 
 import "google/protobuf/unittest_lite.proto";
-import "google/protobuf/unittest_no_arena_lite.proto";
 
-package protobuf_unittest;
+option cc_enable_arenas = true;
+option optimize_for = LITE_RUNTIME;
 
 message TestMapLite {
-  map<int32   , int32   > map_int32_int32       = 1;
-  map<int64   , int64   > map_int64_int64       = 2;
-  map<uint32  , uint32  > map_uint32_uint32     = 3;
-  map<uint64  , uint64  > map_uint64_uint64     = 4;
-  map<sint32  , sint32  > map_sint32_sint32     = 5;
-  map<sint64  , sint64  > map_sint64_sint64     = 6;
-  map<fixed32 , fixed32 > map_fixed32_fixed32   = 7;
-  map<fixed64 , fixed64 > map_fixed64_fixed64   = 8;
+  map<int32, int32> map_int32_int32 = 1;
+  map<int64, int64> map_int64_int64 = 2;
+  map<uint32, uint32> map_uint32_uint32 = 3;
+  map<uint64, uint64> map_uint64_uint64 = 4;
+  map<sint32, sint32> map_sint32_sint32 = 5;
+  map<sint64, sint64> map_sint64_sint64 = 6;
+  map<fixed32, fixed32> map_fixed32_fixed32 = 7;
+  map<fixed64, fixed64> map_fixed64_fixed64 = 8;
   map<sfixed32, sfixed32> map_sfixed32_sfixed32 = 9;
   map<sfixed64, sfixed64> map_sfixed64_sfixed64 = 10;
-  map<int32   , float   > map_int32_float       = 11;
-  map<int32   , double  > map_int32_double      = 12;
-  map<bool    , bool    > map_bool_bool         = 13;
-  map<string  , string  > map_string_string     = 14;
-  map<int32   , bytes   > map_int32_bytes       = 15;
-  map<int32   , MapEnumLite> map_int32_enum     = 16;
-  map<int32   , ForeignMessageLite> map_int32_foreign_message = 17;
+  map<int32, float> map_int32_float = 11;
+  map<int32, double> map_int32_double = 12;
+  map<bool, bool> map_bool_bool = 13;
+  map<string, string> map_string_string = 14;
+  map<int32, bytes> map_int32_bytes = 15;
+  map<int32, MapEnumLite> map_int32_enum = 16;
+  map<int32, ForeignMessageLite> map_int32_foreign_message = 17;
   map<int32, int32> teboring = 18;
 }
 
 message TestArenaMapLite {
-  map<int32   , int32   > map_int32_int32       = 1;
-  map<int64   , int64   > map_int64_int64       = 2;
-  map<uint32  , uint32  > map_uint32_uint32     = 3;
-  map<uint64  , uint64  > map_uint64_uint64     = 4;
-  map<sint32  , sint32  > map_sint32_sint32     = 5;
-  map<sint64  , sint64  > map_sint64_sint64     = 6;
-  map<fixed32 , fixed32 > map_fixed32_fixed32   = 7;
-  map<fixed64 , fixed64 > map_fixed64_fixed64   = 8;
+  map<int32, int32> map_int32_int32 = 1;
+  map<int64, int64> map_int64_int64 = 2;
+  map<uint32, uint32> map_uint32_uint32 = 3;
+  map<uint64, uint64> map_uint64_uint64 = 4;
+  map<sint32, sint32> map_sint32_sint32 = 5;
+  map<sint64, sint64> map_sint64_sint64 = 6;
+  map<fixed32, fixed32> map_fixed32_fixed32 = 7;
+  map<fixed64, fixed64> map_fixed64_fixed64 = 8;
   map<sfixed32, sfixed32> map_sfixed32_sfixed32 = 9;
   map<sfixed64, sfixed64> map_sfixed64_sfixed64 = 10;
-  map<int32   , float   > map_int32_float       = 11;
-  map<int32   , double  > map_int32_double      = 12;
-  map<bool    , bool    > map_bool_bool         = 13;
-  map<string  , string  > map_string_string     = 14;
-  map<int32   , bytes   > map_int32_bytes       = 15;
-  map<int32   , MapEnumLite> map_int32_enum     = 16;
-  map<int32   , ForeignMessageArenaLite> map_int32_foreign_message = 17;
-  map<int32, .protobuf_unittest_no_arena.ForeignMessageLite>
-      map_int32_foreign_message_no_arena = 18;
+  map<int32, float> map_int32_float = 11;
+  map<int32, double> map_int32_double = 12;
+  map<bool, bool> map_bool_bool = 13;
+  map<string, string> map_string_string = 14;
+  map<int32, bytes> map_int32_bytes = 15;
+  map<int32, MapEnumLite> map_int32_enum = 16;
+  map<int32, ForeignMessageArenaLite> map_int32_foreign_message = 17;
 }
 
 // Test embedded message with required fields
@@ -107,9 +104,9 @@ enum Proto2MapEnumLite {
 }
 
 enum Proto2MapEnumPlusExtraLite {
-  E_PROTO2_MAP_ENUM_FOO_LITE   = 0;
-  E_PROTO2_MAP_ENUM_BAR_LITE   = 1;
-  E_PROTO2_MAP_ENUM_BAZ_LITE   = 2;
+  E_PROTO2_MAP_ENUM_FOO_LITE = 0;
+  E_PROTO2_MAP_ENUM_BAR_LITE = 1;
+  E_PROTO2_MAP_ENUM_BAZ_LITE = 2;
   E_PROTO2_MAP_ENUM_EXTRA_LITE = 3;
 }
 

+ 0 - 5
src/google/protobuf/map_test_util_impl.h

@@ -167,7 +167,6 @@ void MapTestUtilImpl::SetArenaMapFields(MapMessage* message) {
   (*message->mutable_map_int32_bytes())[0] = "0";
   (*message->mutable_map_int32_enum())[0] = enum_value0;
   (*message->mutable_map_int32_foreign_message())[0].set_c(0);
-  (*message->mutable_map_int32_foreign_message_no_arena())[0].set_c(0);
 
   // Add second element
   (*message->mutable_map_int32_int32())[1] = 1;
@@ -187,7 +186,6 @@ void MapTestUtilImpl::SetArenaMapFields(MapMessage* message) {
   (*message->mutable_map_int32_bytes())[1] = "1";
   (*message->mutable_map_int32_enum())[1] = enum_value1;
   (*message->mutable_map_int32_foreign_message())[1].set_c(1);
-  (*message->mutable_map_int32_foreign_message_no_arena())[1].set_c(1);
 }
 
 template <typename MapMessage>
@@ -333,7 +331,6 @@ void MapTestUtilImpl::ExpectArenaMapFieldsSet(const MapMessage& message) {
   EXPECT_EQ(2, message.map_int32_bytes().size());
   EXPECT_EQ(2, message.map_int32_enum().size());
   EXPECT_EQ(2, message.map_int32_foreign_message().size());
-  EXPECT_EQ(2, message.map_int32_foreign_message_no_arena().size());
 
   EXPECT_EQ(0, message.map_int32_int32().at(0));
   EXPECT_EQ(0, message.map_int64_int64().at(0));
@@ -352,7 +349,6 @@ void MapTestUtilImpl::ExpectArenaMapFieldsSet(const MapMessage& message) {
   EXPECT_EQ("0", message.map_int32_bytes().at(0));
   EXPECT_EQ(enum_value0, message.map_int32_enum().at(0));
   EXPECT_EQ(0, message.map_int32_foreign_message().at(0).c());
-  EXPECT_EQ(0, message.map_int32_foreign_message_no_arena().at(0).c());
 
   EXPECT_EQ(1, message.map_int32_int32().at(1));
   EXPECT_EQ(1, message.map_int64_int64().at(1));
@@ -371,7 +367,6 @@ void MapTestUtilImpl::ExpectArenaMapFieldsSet(const MapMessage& message) {
   EXPECT_EQ("1", message.map_int32_bytes().at(1));
   EXPECT_EQ(enum_value1, message.map_int32_enum().at(1));
   EXPECT_EQ(1, message.map_int32_foreign_message().at(1).c());
-  EXPECT_EQ(1, message.map_int32_foreign_message_no_arena().at(1).c());
 }
 
 template <typename EnumType, EnumType enum_value, typename MapMessage>

+ 33 - 38
src/google/protobuf/map_unittest.proto

@@ -33,7 +33,6 @@ syntax = "proto3";
 option cc_enable_arenas = true;
 
 import "google/protobuf/unittest.proto";
-import "google/protobuf/unittest_no_arena.proto";
 
 // We don't put this in a package within proto2 because we need to make sure
 // that the generated code doesn't depend on being in the proto2 namespace.
@@ -42,25 +41,25 @@ package protobuf_unittest;
 
 // Tests maps.
 message TestMap {
-  map<int32   , int32   > map_int32_int32       = 1;
-  map<int64   , int64   > map_int64_int64       = 2;
-  map<uint32  , uint32  > map_uint32_uint32     = 3;
-  map<uint64  , uint64  > map_uint64_uint64     = 4;
-  map<sint32  , sint32  > map_sint32_sint32     = 5;
-  map<sint64  , sint64  > map_sint64_sint64     = 6;
-  map<fixed32 , fixed32 > map_fixed32_fixed32   = 7;
-  map<fixed64 , fixed64 > map_fixed64_fixed64   = 8;
+  map<int32, int32> map_int32_int32 = 1;
+  map<int64, int64> map_int64_int64 = 2;
+  map<uint32, uint32> map_uint32_uint32 = 3;
+  map<uint64, uint64> map_uint64_uint64 = 4;
+  map<sint32, sint32> map_sint32_sint32 = 5;
+  map<sint64, sint64> map_sint64_sint64 = 6;
+  map<fixed32, fixed32> map_fixed32_fixed32 = 7;
+  map<fixed64, fixed64> map_fixed64_fixed64 = 8;
   map<sfixed32, sfixed32> map_sfixed32_sfixed32 = 9;
   map<sfixed64, sfixed64> map_sfixed64_sfixed64 = 10;
-  map<int32   , float   > map_int32_float       = 11;
-  map<int32   , double  > map_int32_double      = 12;
-  map<bool    , bool    > map_bool_bool         = 13;
-  map<string  , string  > map_string_string     = 14;
-  map<int32   , bytes   > map_int32_bytes       = 15;
-  map<int32   , MapEnum > map_int32_enum        = 16;
-  map<int32   , ForeignMessage> map_int32_foreign_message = 17;
-  map<string  , ForeignMessage> map_string_foreign_message = 18;
-  map<int32   , TestAllTypes> map_int32_all_types = 19;
+  map<int32, float> map_int32_float = 11;
+  map<int32, double> map_int32_double = 12;
+  map<bool, bool> map_bool_bool = 13;
+  map<string, string> map_string_string = 14;
+  map<int32, bytes> map_int32_bytes = 15;
+  map<int32, MapEnum> map_int32_enum = 16;
+  map<int32, ForeignMessage> map_int32_foreign_message = 17;
+  map<string, ForeignMessage> map_string_foreign_message = 18;
+  map<int32, TestAllTypes> map_int32_all_types = 19;
 }
 
 message TestMapSubmessage {
@@ -90,33 +89,29 @@ message TestRequiredMessageMap {
 }
 
 message TestArenaMap {
-  map<int32   , int32   > map_int32_int32       = 1;
-  map<int64   , int64   > map_int64_int64       = 2;
-  map<uint32  , uint32  > map_uint32_uint32     = 3;
-  map<uint64  , uint64  > map_uint64_uint64     = 4;
-  map<sint32  , sint32  > map_sint32_sint32     = 5;
-  map<sint64  , sint64  > map_sint64_sint64     = 6;
-  map<fixed32 , fixed32 > map_fixed32_fixed32   = 7;
-  map<fixed64 , fixed64 > map_fixed64_fixed64   = 8;
+  map<int32, int32> map_int32_int32 = 1;
+  map<int64, int64> map_int64_int64 = 2;
+  map<uint32, uint32> map_uint32_uint32 = 3;
+  map<uint64, uint64> map_uint64_uint64 = 4;
+  map<sint32, sint32> map_sint32_sint32 = 5;
+  map<sint64, sint64> map_sint64_sint64 = 6;
+  map<fixed32, fixed32> map_fixed32_fixed32 = 7;
+  map<fixed64, fixed64> map_fixed64_fixed64 = 8;
   map<sfixed32, sfixed32> map_sfixed32_sfixed32 = 9;
   map<sfixed64, sfixed64> map_sfixed64_sfixed64 = 10;
-  map<int32   , float   > map_int32_float       = 11;
-  map<int32   , double  > map_int32_double      = 12;
-  map<bool    , bool    > map_bool_bool         = 13;
-  map<string  , string  > map_string_string     = 14;
-  map<int32   , bytes   > map_int32_bytes       = 15;
-  map<int32   , MapEnum > map_int32_enum        = 16;
-  map<int32   , ForeignMessage> map_int32_foreign_message = 17;
-  map<int32, .protobuf_unittest_no_arena.ForeignMessage>
-      map_int32_foreign_message_no_arena = 18;
+  map<int32, float> map_int32_float = 11;
+  map<int32, double> map_int32_double = 12;
+  map<bool, bool> map_bool_bool = 13;
+  map<string, string> map_string_string = 14;
+  map<int32, bytes> map_int32_bytes = 15;
+  map<int32, MapEnum> map_int32_enum = 16;
+  map<int32, ForeignMessage> map_int32_foreign_message = 17;
 }
 
 // Previously, message containing enum called Type cannot be used as value of
 // map field.
 message MessageContainingEnumCalledType {
-  enum Type {
-    TYPE_FOO = 0;
-  }
+  enum Type { TYPE_FOO = 0; }
   map<string, MessageContainingEnumCalledType> type = 1;
 }
 

+ 16 - 0
src/google/protobuf/message.h

@@ -911,6 +911,22 @@ class PROTOBUF_EXPORT Reflection final {
   const internal::RepeatedFieldAccessor* RepeatedFieldAccessor(
       const FieldDescriptor* field) const;
 
+  // Lists all fields of the message which are currently set, except for unknown
+  // fields and stripped fields. See ListFields for details.
+  void ListFieldsOmitStripped(
+      const Message& message,
+      std::vector<const FieldDescriptor*>* output) const;
+
+  bool IsMessageStripped(const Descriptor* descriptor) const {
+    return schema_.IsMessageStripped(descriptor);
+  }
+
+  friend class TextFormat;
+
+  void ListFieldsMayFailOnStripped(
+      const Message& message, bool should_fail,
+      std::vector<const FieldDescriptor*>* output) const;
+
   const Descriptor* const descriptor_;
   const internal::ReflectionSchema schema_;
   const DescriptorPool* const descriptor_pool_;

+ 27 - 15
src/google/protobuf/message_lite.h

@@ -267,28 +267,35 @@ class PROTOBUF_EXPORT MessageLite {
   // format.  A successful return does not indicate the entire input is
   // consumed, ensure you call ConsumedEntireMessage() to check that if
   // applicable.
-  bool ParseFromCodedStream(io::CodedInputStream* input);
+  PROTOBUF_ATTRIBUTE_REINITIALIZES bool ParseFromCodedStream(
+      io::CodedInputStream* input);
   // Like ParseFromCodedStream(), but accepts messages that are missing
   // required fields.
-  bool ParsePartialFromCodedStream(io::CodedInputStream* input);
+  PROTOBUF_ATTRIBUTE_REINITIALIZES bool ParsePartialFromCodedStream(
+      io::CodedInputStream* input);
   // Read a protocol buffer from the given zero-copy input stream.  If
   // successful, the entire input will be consumed.
-  bool ParseFromZeroCopyStream(io::ZeroCopyInputStream* input);
+  PROTOBUF_ATTRIBUTE_REINITIALIZES bool ParseFromZeroCopyStream(
+      io::ZeroCopyInputStream* input);
   // Like ParseFromZeroCopyStream(), but accepts messages that are missing
   // required fields.
-  bool ParsePartialFromZeroCopyStream(io::ZeroCopyInputStream* input);
+  PROTOBUF_ATTRIBUTE_REINITIALIZES bool ParsePartialFromZeroCopyStream(
+      io::ZeroCopyInputStream* input);
   // Parse a protocol buffer from a file descriptor.  If successful, the entire
   // input will be consumed.
-  bool ParseFromFileDescriptor(int file_descriptor);
+  PROTOBUF_ATTRIBUTE_REINITIALIZES bool ParseFromFileDescriptor(
+      int file_descriptor);
   // Like ParseFromFileDescriptor(), but accepts messages that are missing
   // required fields.
-  bool ParsePartialFromFileDescriptor(int file_descriptor);
+  PROTOBUF_ATTRIBUTE_REINITIALIZES bool ParsePartialFromFileDescriptor(
+      int file_descriptor);
   // Parse a protocol buffer from a C++ istream.  If successful, the entire
   // input will be consumed.
-  bool ParseFromIstream(std::istream* input);
+  PROTOBUF_ATTRIBUTE_REINITIALIZES bool ParseFromIstream(std::istream* input);
   // Like ParseFromIstream(), but accepts messages that are missing
   // required fields.
-  bool ParsePartialFromIstream(std::istream* input);
+  PROTOBUF_ATTRIBUTE_REINITIALIZES bool ParsePartialFromIstream(
+      std::istream* input);
   // Read a protocol buffer from the given zero-copy input stream, expecting
   // the message to be exactly "size" bytes long.  If successful, exactly
   // this many bytes will have been consumed from the input.
@@ -297,25 +304,30 @@ class PROTOBUF_EXPORT MessageLite {
   // Like ParseFromBoundedZeroCopyStream(), but accepts messages that are
   // missing required fields.
   bool MergeFromBoundedZeroCopyStream(io::ZeroCopyInputStream* input, int size);
-  bool ParseFromBoundedZeroCopyStream(io::ZeroCopyInputStream* input, int size);
+  PROTOBUF_ATTRIBUTE_REINITIALIZES bool ParseFromBoundedZeroCopyStream(
+      io::ZeroCopyInputStream* input, int size);
   // Like ParseFromBoundedZeroCopyStream(), but accepts messages that are
   // missing required fields.
-  bool ParsePartialFromBoundedZeroCopyStream(io::ZeroCopyInputStream* input,
-                                             int size);
+  PROTOBUF_ATTRIBUTE_REINITIALIZES bool ParsePartialFromBoundedZeroCopyStream(
+      io::ZeroCopyInputStream* input, int size);
   // Parses a protocol buffer contained in a string. Returns true on success.
   // This function takes a string in the (non-human-readable) binary wire
   // format, matching the encoding output by MessageLite::SerializeToString().
   // If you'd like to convert a human-readable string into a protocol buffer
   // object, see google::protobuf::TextFormat::ParseFromString().
-  bool ParseFromString(const std::string& data);
+  PROTOBUF_ATTRIBUTE_REINITIALIZES bool ParseFromString(
+      const std::string& data);
   // Like ParseFromString(), but accepts messages that are missing
   // required fields.
-  bool ParsePartialFromString(const std::string& data);
+  PROTOBUF_ATTRIBUTE_REINITIALIZES bool ParsePartialFromString(
+      const std::string& data);
   // Parse a protocol buffer contained in an array of bytes.
-  bool ParseFromArray(const void* data, int size);
+  PROTOBUF_ATTRIBUTE_REINITIALIZES bool ParseFromArray(const void* data,
+                                                       int size);
   // Like ParseFromArray(), but accepts messages that are missing
   // required fields.
-  bool ParsePartialFromArray(const void* data, int size);
+  PROTOBUF_ATTRIBUTE_REINITIALIZES bool ParsePartialFromArray(const void* data,
+                                                              int size);
 
 
   // Reads a protocol buffer from the stream and merges it into this

+ 2 - 2
src/google/protobuf/message_unittest.inc

@@ -35,11 +35,11 @@
 // This file needs to be included as .inc as it depends on certain macros being
 // defined prior to its inclusion.
 
-#include <google/protobuf/message.h>
-
 #include <fcntl.h>
 #include <sys/stat.h>
 #include <sys/types.h>
+
+#include <google/protobuf/message.h>
 #ifndef _MSC_VER
 #include <unistd.h>
 #endif

+ 4 - 2
src/google/protobuf/port_def.inc

@@ -586,12 +586,14 @@ PROTOBUF_EXPORT_TEMPLATE_TEST(DEFAULT, __declspec(dllimport));
 #endif  // _MSC_VER
 
 #if defined(__clang__) || defined(__GNUC__) || defined(_MSC_VER)
-// Don't let the YES/NO Objective-C Macros interfere with proto identifiers with
-// the same name.
+// Don't let Objective-C Macros interfere with proto identifiers with the same 
+// name.
 #pragma push_macro("YES")
 #undef YES
 #pragma push_macro("NO")
 #undef NO
+#pragma push_macro("DEBUG")
+#undef DEBUG
 #endif // defined(__clang__) || defined(__GNUC__) || defined(_MSC_VER)
 
 #if defined(__clang__)

+ 1 - 0
src/google/protobuf/port_undef.inc

@@ -110,6 +110,7 @@
 #if defined(__clang__) || defined(__GNUC__) || defined(_MSC_VER)
 #pragma pop_macro("YES")
 #pragma pop_macro("NO")
+#pragma pop_macro("DEBUG")
 #endif // defined(__clang__) || defined(__GNUC__) || defined(_MSC_VER)
 
 #if defined(__clang__)

+ 6 - 4
src/google/protobuf/reflection_ops.cc

@@ -85,7 +85,7 @@ void ReflectionOps::Merge(const Message& from, Message* to) {
                           google::protobuf::MessageFactory::generated_factory());
 
   std::vector<const FieldDescriptor*> fields;
-  from_reflection->ListFields(from, &fields);
+  from_reflection->ListFieldsOmitStripped(from, &fields);
   for (int i = 0; i < fields.size(); i++) {
     const FieldDescriptor* field = fields[i];
 
@@ -180,7 +180,7 @@ void ReflectionOps::Clear(Message* message) {
   const Reflection* reflection = GetReflectionOrDie(*message);
 
   std::vector<const FieldDescriptor*> fields;
-  reflection->ListFields(*message, &fields);
+  reflection->ListFieldsOmitStripped(*message, &fields);
   for (int i = 0; i < fields.size(); i++) {
     reflection->ClearField(message, fields[i]);
   }
@@ -268,7 +268,9 @@ bool ReflectionOps::IsInitialized(const Message& message) {
 
   // Check that sub-messages are initialized.
   std::vector<const FieldDescriptor*> fields;
-  reflection->ListFields(message, &fields);
+  // Should be safe to skip stripped fields because required fields are not
+  // stripped.
+  reflection->ListFieldsOmitStripped(message, &fields);
   for (int i = 0; i < fields.size(); i++) {
     const FieldDescriptor* field = fields[i];
     if (field->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE) {
@@ -399,7 +401,7 @@ void ReflectionOps::FindInitializationErrors(const Message& message,
 
   // Check sub-messages.
   std::vector<const FieldDescriptor*> fields;
-  reflection->ListFields(message, &fields);
+  reflection->ListFieldsOmitStripped(message, &fields);
   for (int i = 0; i < fields.size(); i++) {
     const FieldDescriptor* field = fields[i];
     if (field->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE) {

+ 1 - 1
src/google/protobuf/struct.pb.h

@@ -118,7 +118,7 @@ public:
     ::PROTOBUF_NAMESPACE_ID::internal::WireFormatLite::TYPE_MESSAGE,
     0 > SuperType;
   Struct_FieldsEntry_DoNotUse();
-  Struct_FieldsEntry_DoNotUse(::PROTOBUF_NAMESPACE_ID::Arena* arena);
+  explicit Struct_FieldsEntry_DoNotUse(::PROTOBUF_NAMESPACE_ID::Arena* arena);
   void MergeFrom(const Struct_FieldsEntry_DoNotUse& other);
   static const Struct_FieldsEntry_DoNotUse* internal_default_instance() { return reinterpret_cast<const Struct_FieldsEntry_DoNotUse*>(&_Struct_FieldsEntry_DoNotUse_default_instance_); }
   static bool ValidateKey(std::string* s) {

+ 7 - 1
src/google/protobuf/text_format.cc

@@ -1803,6 +1803,9 @@ class FastFieldValuePrinterUtf8Escaping
 
 }  // namespace
 
+const char* const TextFormat::Printer::kDoNotParse =
+    "DO NOT PARSE: fields may be stripped and missing.\n";
+
 TextFormat::Printer::Printer()
     : initial_indent_level_(0),
       single_line_mode_(false),
@@ -2019,7 +2022,10 @@ void TextFormat::Printer::Print(const Message& message,
     fields.push_back(descriptor->field(0));
     fields.push_back(descriptor->field(1));
   } else {
-    reflection->ListFields(message, &fields);
+    reflection->ListFieldsOmitStripped(message, &fields);
+    if (reflection->IsMessageStripped(message.GetDescriptor())) {
+      generator->Print(kDoNotParse, std::strlen(kDoNotParse));
+    }
   }
 
   if (print_message_fields_in_index_order_) {

+ 2 - 0
src/google/protobuf/text_format.h

@@ -369,6 +369,8 @@ class PROTOBUF_EXPORT TextFormat {
     // output to the OutputStream (see text_format.cc for implementation).
     class TextGenerator;
 
+    static const char* const kDoNotParse;
+
     // Internal Print method, used for writing to the OutputStream via
     // the TextGenerator class.
     void Print(const Message& message, TextGenerator* generator) const;

+ 2 - 5
src/google/protobuf/unittest_arena.proto

@@ -30,8 +30,6 @@
 
 syntax = "proto2";
 
-import "google/protobuf/unittest_no_arena_import.proto";
-
 package proto2_arena_unittest;
 
 option cc_enable_arenas = true;
@@ -41,6 +39,5 @@ message NestedMessage {
 }
 
 message ArenaMessage {
-  repeated NestedMessage  repeated_nested_message = 1;
-  repeated ImportNoArenaNestedMessage  repeated_import_no_arena_message = 2;
-};
+  repeated NestedMessage repeated_nested_message = 1;
+}

+ 0 - 206
src/google/protobuf/unittest_no_arena.proto

@@ -1,206 +0,0 @@
-// 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)
-//  Based on original Protocol Buffers design by
-//  Sanjay Ghemawat, Jeff Dean, and others.
-//
-// This proto file contains copies of TestAllTypes and friends, but with arena
-// support disabled in code generation. It allows us to test the performance
-// impact against baseline (non-arena) google.protobuf.
-
-syntax = "proto2";
-
-// Some generic_services option(s) added automatically.
-// See:  http://go/proto2-generic-services-default
-option cc_generic_services = true;     // auto-added
-option java_generic_services = true;   // auto-added
-option py_generic_services = true;     // auto-added
-option cc_enable_arenas = false;
-option objc_class_prefix = "NOARN";
-
-import "google/protobuf/unittest_import.proto";
-import "google/protobuf/unittest_arena.proto";
-
-// We don't put this in a package within proto2 because we need to make sure
-// that the generated code doesn't depend on being in the proto2 namespace.
-// In test_util.h we do "using namespace unittest = protobuf_unittest".
-package protobuf_unittest_no_arena;
-
-// Protos optimized for SPEED use a strict superset of the generated code
-// of equivalent ones optimized for CODE_SIZE, so we should optimize all our
-// tests for speed unless explicitly testing code size optimization.
-option optimize_for = SPEED;
-
-option java_outer_classname = "UnittestProto";
-
-// This proto includes every type of field in both singular and repeated
-// forms.
-message TestAllTypes {
-  message NestedMessage {
-    // The field name "b" fails to compile in proto1 because it conflicts with
-    // a local variable named "b" in one of the generated methods.  Doh.
-    // This file needs to compile in proto1 to test backwards-compatibility.
-    optional int32 bb = 1;
-  }
-
-  enum NestedEnum {
-    FOO = 1;
-    BAR = 2;
-    BAZ = 3;
-    NEG = -1;  // Intentionally negative.
-  }
-
-  // Singular
-  optional    int32 optional_int32    =  1;
-  optional    int64 optional_int64    =  2;
-  optional   uint32 optional_uint32   =  3;
-  optional   uint64 optional_uint64   =  4;
-  optional   sint32 optional_sint32   =  5;
-  optional   sint64 optional_sint64   =  6;
-  optional  fixed32 optional_fixed32  =  7;
-  optional  fixed64 optional_fixed64  =  8;
-  optional sfixed32 optional_sfixed32 =  9;
-  optional sfixed64 optional_sfixed64 = 10;
-  optional    float optional_float    = 11;
-  optional   double optional_double   = 12;
-  optional     bool optional_bool     = 13;
-  optional   string optional_string   = 14;
-  optional    bytes optional_bytes    = 15;
-
-  optional group OptionalGroup = 16 {
-    optional int32 a = 17;
-  }
-
-  optional NestedMessage                        optional_nested_message  = 18;
-  optional ForeignMessage                       optional_foreign_message = 19;
-  optional protobuf_unittest_import.ImportMessage optional_import_message  = 20;
-
-  optional NestedEnum                           optional_nested_enum     = 21;
-  optional ForeignEnum                          optional_foreign_enum    = 22;
-  optional protobuf_unittest_import.ImportEnum    optional_import_enum     = 23;
-
-  optional string optional_string_piece = 24 [ctype=STRING_PIECE];
-  optional string optional_cord = 25 [ctype=CORD];
-
-  // Defined in unittest_import_public.proto
-  optional protobuf_unittest_import.PublicImportMessage
-      optional_public_import_message = 26;
-
-  optional NestedMessage optional_message = 27 [lazy=true];
-
-  // Repeated
-  repeated    int32 repeated_int32    = 31;
-  repeated    int64 repeated_int64    = 32;
-  repeated   uint32 repeated_uint32   = 33;
-  repeated   uint64 repeated_uint64   = 34;
-  repeated   sint32 repeated_sint32   = 35;
-  repeated   sint64 repeated_sint64   = 36;
-  repeated  fixed32 repeated_fixed32  = 37;
-  repeated  fixed64 repeated_fixed64  = 38;
-  repeated sfixed32 repeated_sfixed32 = 39;
-  repeated sfixed64 repeated_sfixed64 = 40;
-  repeated    float repeated_float    = 41;
-  repeated   double repeated_double   = 42;
-  repeated     bool repeated_bool     = 43;
-  repeated   string repeated_string   = 44;
-  repeated    bytes repeated_bytes    = 45;
-
-  repeated group RepeatedGroup = 46 {
-    optional int32 a = 47;
-  }
-
-  repeated NestedMessage                        repeated_nested_message  = 48;
-  repeated ForeignMessage                       repeated_foreign_message = 49;
-  repeated protobuf_unittest_import.ImportMessage repeated_import_message  = 50;
-
-  repeated NestedEnum                           repeated_nested_enum     = 51;
-  repeated ForeignEnum                          repeated_foreign_enum    = 52;
-  repeated protobuf_unittest_import.ImportEnum    repeated_import_enum     = 53;
-
-  repeated string repeated_string_piece = 54 [ctype=STRING_PIECE];
-  repeated string repeated_cord = 55 [ctype=CORD];
-
-  repeated NestedMessage repeated_lazy_message = 57 [lazy=true];
-
-  // Singular with defaults
-  optional    int32 default_int32    = 61 [default =  41    ];
-  optional    int64 default_int64    = 62 [default =  42    ];
-  optional   uint32 default_uint32   = 63 [default =  43    ];
-  optional   uint64 default_uint64   = 64 [default =  44    ];
-  optional   sint32 default_sint32   = 65 [default = -45    ];
-  optional   sint64 default_sint64   = 66 [default =  46    ];
-  optional  fixed32 default_fixed32  = 67 [default =  47    ];
-  optional  fixed64 default_fixed64  = 68 [default =  48    ];
-  optional sfixed32 default_sfixed32 = 69 [default =  49    ];
-  optional sfixed64 default_sfixed64 = 70 [default = -50    ];
-  optional    float default_float    = 71 [default =  51.5  ];
-  optional   double default_double   = 72 [default =  52e3  ];
-  optional     bool default_bool     = 73 [default = true   ];
-  optional   string default_string   = 74 [default = "hello"];
-  optional    bytes default_bytes    = 75 [default = "world"];
-
-  optional NestedEnum  default_nested_enum  = 81 [default = BAR        ];
-  optional ForeignEnum default_foreign_enum = 82 [default = FOREIGN_BAR];
-  optional protobuf_unittest_import.ImportEnum
-      default_import_enum = 83 [default = IMPORT_BAR];
-
-  optional string default_string_piece = 84 [ctype=STRING_PIECE,default="abc"];
-  optional string default_cord = 85 [ctype=CORD,default="123"];
-
-  // For oneof test
-  oneof oneof_field {
-    uint32 oneof_uint32 = 111;
-    NestedMessage oneof_nested_message = 112;
-    string oneof_string = 113;
-    bytes oneof_bytes = 114;
-    NestedMessage lazy_oneof_nested_message = 115 [lazy=true];
-  }
-}
-
-// Define these after TestAllTypes to make sure the compiler can handle
-// that.
-message ForeignMessage {
-  optional int32 c = 1;
-}
-
-enum ForeignEnum {
-  FOREIGN_FOO = 4;
-  FOREIGN_BAR = 5;
-  FOREIGN_BAZ = 6;
-}
-
-message TestNoArenaMessage {
-  optional proto2_arena_unittest.ArenaMessage arena_message = 1;
-};
-
-message OneString {
-  optional string data = 1;
-}

+ 0 - 39
src/google/protobuf/unittest_no_arena_import.proto

@@ -1,39 +0,0 @@
-// 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.
-
-syntax = "proto2";
-
-package proto2_arena_unittest;
-
-option cc_enable_arenas = false;
-
-message ImportNoArenaNestedMessage {
-  optional int32 d = 1;
-}

+ 0 - 44
src/google/protobuf/unittest_no_arena_lite.proto

@@ -1,44 +0,0 @@
-// 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.
-
-syntax = "proto2";
-
-option optimize_for = LITE_RUNTIME;
-
-// We don't put this in a package within proto2 because we need to make sure
-// that the generated code doesn't depend on being in the proto2 namespace.
-// In test_util.h we do "using namespace unittest = protobuf_unittest".
-package protobuf_unittest_no_arena;
-
-option cc_enable_arenas = false;
-
-message ForeignMessageLite {
-  optional int32 c = 1;
-}

+ 3 - 1
src/google/protobuf/wire_format.cc

@@ -661,6 +661,7 @@ struct WireFormat::MessageSetParser {
     auto metadata = reflection->MutableInternalMetadata(msg);
     std::string payload;
     uint32 type_id = 0;
+    bool payload_read = false;
     while (!ctx->Done(&ptr)) {
       // We use 64 bit tags in order to allow typeid's that span the whole
       // range of 32 bit numbers.
@@ -670,7 +671,7 @@ struct WireFormat::MessageSetParser {
         ptr = ParseBigVarint(ptr, &tmp);
         GOOGLE_PROTOBUF_PARSER_ASSERT(ptr);
         type_id = tmp;
-        if (!payload.empty()) {
+        if (payload_read) {
           const FieldDescriptor* field;
           if (ctx->data().pool == nullptr) {
             field = reflection->FindKnownExtensionByNumber(type_id);
@@ -706,6 +707,7 @@ struct WireFormat::MessageSetParser {
           GOOGLE_PROTOBUF_PARSER_ASSERT(ptr);
           ptr = ctx->ReadString(ptr, size, &payload);
           GOOGLE_PROTOBUF_PARSER_ASSERT(ptr);
+          payload_read = true;
         } else {
           // We're now parsing the payload
           const FieldDescriptor* field = nullptr;