Merge pull request #7402 from haberman/sync-stage

Integrate from Piper for C++, Java, and Python

python/google/protobuf/pyext/message.cc

@@ -1427,8 +1427,7 @@ bool CheckHasPresence(const FieldDescriptor* field_descriptor, bool in_oneof) {
     return false;
   }
 
-  if (field_descriptor->containing_oneof() == NULL &&
-      !field_descriptor->is_singular_with_presence()) {
+  if (!field_descriptor->has_presence()) {
     PyErr_Format(PyExc_ValueError,
                  "Can't test non-optional, non-submessage field \"%s.%s\" for "
                  "presence in proto3.",
@@ -2171,19 +2170,21 @@ static PyObject* RichCompare(CMessage* self, PyObject* other, int opid) {
   // If other is not a message, it cannot be equal.
   if (!PyObject_TypeCheck(other, CMessage_Type)) {
     equals = false;
-  }
-  const google::protobuf::Message* other_message =
-      reinterpret_cast<CMessage*>(other)->message;
-  // If messages don't have the same descriptors, they are not equal.
-  if (equals &&
-      self->message->GetDescriptor() != other_message->GetDescriptor()) {
-    equals = false;
-  }
-  // Check the message contents.
-  if (equals && !google::protobuf::util::MessageDifferencer::Equals(
-          *self->message,
-          *reinterpret_cast<CMessage*>(other)->message)) {
-    equals = false;
+  } else {
+    // Otherwise, we have a CMessage whose message we can inspect.
+    const google::protobuf::Message* other_message =
+        reinterpret_cast<CMessage*>(other)->message;
+    // If messages don't have the same descriptors, they are not equal.
+    if (equals &&
+        self->message->GetDescriptor() != other_message->GetDescriptor()) {
+      equals = false;
+    }
+    // Check the message contents.
+    if (equals &&
+        !google::protobuf::util::MessageDifferencer::Equals(
+            *self->message, *reinterpret_cast<CMessage*>(other)->message)) {
+      equals = false;
+    }
   }
 
   if (equals ^ (opid == Py_EQ)) {

python/google/protobuf/text_format.py

@@ -120,20 +120,21 @@ class TextWriter(object):
     return self._writer.getvalue()
 
 
-def MessageToString(message,
-                    as_utf8=False,
-                    as_one_line=False,
-                    use_short_repeated_primitives=False,
-                    pointy_brackets=False,
-                    use_index_order=False,
-                    float_format=None,
-                    double_format=None,
-                    use_field_number=False,
-                    descriptor_pool=None,
-                    indent=0,
-                    message_formatter=None,
-                    print_unknown_fields=False,
-                    force_colon=False):
+def MessageToString(
+    message,
+    as_utf8=False,
+    as_one_line=False,
+    use_short_repeated_primitives=False,
+    pointy_brackets=False,
+    use_index_order=False,
+    float_format=None,
+    double_format=None,
+    use_field_number=False,
+    descriptor_pool=None,
+    indent=0,
+    message_formatter=None,
+    print_unknown_fields=False,
+    force_colon=False):
   # type: (...) -> str
   """Convert protobuf message to text format.
 
@@ -329,21 +330,22 @@ WIRETYPE_START_GROUP = 3
 class _Printer(object):
   """Text format printer for protocol message."""
 
-  def __init__(self,
-               out,
-               indent=0,
-               as_utf8=False,
-               as_one_line=False,
-               use_short_repeated_primitives=False,
-               pointy_brackets=False,
-               use_index_order=False,
-               float_format=None,
-               double_format=None,
-               use_field_number=False,
-               descriptor_pool=None,
-               message_formatter=None,
-               print_unknown_fields=False,
-               force_colon=False):
+  def __init__(
+      self,
+      out,
+      indent=0,
+      as_utf8=False,
+      as_one_line=False,
+      use_short_repeated_primitives=False,
+      pointy_brackets=False,
+      use_index_order=False,
+      float_format=None,
+      double_format=None,
+      use_field_number=False,
+      descriptor_pool=None,
+      message_formatter=None,
+      print_unknown_fields=False,
+      force_colon=False):
     """Initialize the Printer.
 
     Double values can be formatted compactly with 15 digits of precision

src/google/protobuf/arena_impl.h

@@ -135,6 +135,8 @@ class PROTOBUF_EXPORT ArenaImpl {
   void AddCleanup(void* elem, void (*cleanup)(void*));
 
  private:
+  friend class ArenaBenchmark;
+
   void* AllocateAlignedFallback(size_t n);
   void* AllocateAlignedAndAddCleanupFallback(size_t n, void (*cleanup)(void*));
   void AddCleanupFallback(void* elem, void (*cleanup)(void*));

src/google/protobuf/compiler/command_line_interface.cc

@@ -1105,14 +1105,18 @@ PopulateSingleSimpleDescriptorDatabase(const std::string& descriptor_set_name) {
 bool CommandLineInterface::AllowProto3Optional(
     const FileDescriptor& file) const {
   if (allow_proto3_optional_) return true;
+
   // Whitelist all ads protos. Ads is an early adopter of this feature.
   if (file.name().find("google/ads/googleads") != std::string::npos) {
     return true;
   }
-  if (file.name() == "google/protobuf/unittest_proto3_optional.proto" ||
-      file.name() == "google/protobuf/internal/test_proto3_optional.proto") {
+
+  // Whitelist all protos testing proto3 optional.
+  if (file.name().find("test_proto3_optional") != std::string::npos) {
     return true;
   }
+
+
   return false;
 }
 

src/google/protobuf/compiler/java/java_enum_field_lite.cc

@@ -103,9 +103,6 @@ void SetEnumVariables(const FieldDescriptor* descriptor, int messageBitIndex,
         ".getNumber()";
   }
 
-  // For repeated builders, the underlying list tracks mutability state.
-  (*variables)["is_mutable"] = (*variables)["name"] + "_.isModifiable()";
-
   (*variables)["get_has_field_bit_from_local"] =
       GenerateGetBitFromLocal(builderBitIndex);
   (*variables)["set_has_field_bit_to_local"] =
@@ -572,9 +569,11 @@ void RepeatedImmutableEnumFieldLiteGenerator::GenerateMembers(
   printer->Print(
       variables_,
       "private void ensure$capitalized_name$IsMutable() {\n"
-      "  if (!$is_mutable$) {\n"
+      // Use a temporary to avoid a redundant iget-object.
+      "  com.google.protobuf.Internal.IntList tmp = $name$_;\n"
+      "  if (!tmp.isModifiable()) {\n"
       "    $name$_ =\n"
-      "        com.google.protobuf.GeneratedMessageLite.mutableCopy($name$_);\n"
+      "        com.google.protobuf.GeneratedMessageLite.mutableCopy(tmp);\n"
       "  }\n"
       "}\n");
   WriteFieldAccessorDocComment(printer, descriptor_, LIST_INDEXED_SETTER);

src/google/protobuf/compiler/java/java_message_field_lite.cc

@@ -89,9 +89,6 @@ void SetMessageVariables(const FieldDescriptor* descriptor, int messageBitIndex,
         (*variables)["name"] + "_ != null";
   }
 
-  // For repeated builders, the underlying list tracks mutability state.
-  (*variables)["is_mutable"] = (*variables)["name"] + "_.isModifiable()";
-
   (*variables)["get_has_field_bit_from_local"] =
       GenerateGetBitFromLocal(builderBitIndex);
   (*variables)["set_has_field_bit_to_local"] =
@@ -532,9 +529,11 @@ void RepeatedImmutableMessageFieldLiteGenerator::GenerateMembers(
   printer->Print(
       variables_,
       "private void ensure$capitalized_name$IsMutable() {\n"
-      "  if (!$is_mutable$) {\n"
+      // Use a temporary to avoid a redundant iget-object.
+      "  com.google.protobuf.Internal.ProtobufList<$type$> tmp = $name$_;\n"
+      "  if (!tmp.isModifiable()) {\n"
       "    $name$_ =\n"
-      "        com.google.protobuf.GeneratedMessageLite.mutableCopy($name$_);\n"
+      "        com.google.protobuf.GeneratedMessageLite.mutableCopy(tmp);\n"
       "   }\n"
       "}\n"
       "\n");

src/google/protobuf/compiler/java/java_primitive_field_lite.cc

@@ -164,9 +164,6 @@ void SetPrimitiveVariables(const FieldDescriptor* descriptor,
     }
   }
 
-  // For repeated builders, the underlying list tracks mutability state.
-  (*variables)["is_mutable"] = (*variables)["name"] + "_.isModifiable()";
-
   (*variables)["get_has_field_bit_from_local"] =
       GenerateGetBitFromLocal(builderBitIndex);
   (*variables)["set_has_field_bit_to_local"] =
@@ -511,9 +508,11 @@ void RepeatedImmutablePrimitiveFieldLiteGenerator::GenerateMembers(
   printer->Print(
       variables_,
       "private void ensure$capitalized_name$IsMutable() {\n"
-      "  if (!$is_mutable$) {\n"
+      // Use a temporary to avoid a redundant iget-object.
+      "  $field_list_type$ tmp = $name$_;\n"
+      "  if (!tmp.isModifiable()) {\n"
       "    $name$_ =\n"
-      "        com.google.protobuf.GeneratedMessageLite.mutableCopy($name$_);\n"
+      "        com.google.protobuf.GeneratedMessageLite.mutableCopy(tmp);\n"
       "   }\n"
       "}\n");
 

src/google/protobuf/compiler/java/java_string_field_lite.cc

@@ -104,9 +104,6 @@ void SetPrimitiveVariables(const FieldDescriptor* descriptor,
         "!" + (*variables)["name"] + "_.isEmpty()";
   }
 
-  // For repeated builders, the underlying list tracks mutability state.
-  (*variables)["is_mutable"] = (*variables)["name"] + "_.isModifiable()";
-
   (*variables)["get_has_field_bit_from_local"] =
       GenerateGetBitFromLocal(builderBitIndex);
   (*variables)["set_has_field_bit_to_local"] =
@@ -567,9 +564,12 @@ void RepeatedImmutableStringFieldLiteGenerator::GenerateMembers(
   printer->Print(
       variables_,
       "private void ensure$capitalized_name$IsMutable() {\n"
-      "  if (!$is_mutable$) {\n"
+      // Use a temporary to avoid a redundant iget-object.
+      "  com.google.protobuf.Internal.ProtobufList<java.lang.String> tmp =\n"
+      "      $name$_;"
+      "  if (!tmp.isModifiable()) {\n"
       "    $name$_ =\n"
-      "        com.google.protobuf.GeneratedMessageLite.mutableCopy($name$_);\n"
+      "        com.google.protobuf.GeneratedMessageLite.mutableCopy(tmp);\n"
       "   }\n"
       "}\n");
 

src/google/protobuf/descriptor.h

@@ -693,13 +693,14 @@ class PROTOBUF_EXPORT FieldDescriptor {
   // .proto file. Excludes singular proto3 fields that do not have a label.
   bool has_optional_keyword() const;
 
-  // Returns true if this is a non-oneof field that tracks presence.
-  // This includes all "required" and "optional" fields in the .proto file,
-  // but excludes oneof fields and singular proto3 fields without "optional".
+  // Returns true if this field tracks presence, ie. does the message
+  // distinguish between "unset" and "present with default value."
+  // This includes required, optional, and oneof fields. It excludes maps,
+  // repeated fields, and singular proto3 fields without "optional".
   //
-  // In implementations that use hasbits, this method will probably indicate
-  // whether this field uses a hasbit.
-  bool is_singular_with_presence() const;
+  // For fields where has_presence() == true, the return value of
+  // Reflection::HasField() is semantically meaningful.
+  bool has_presence() const;
 
   // Index of this field within the message's field array, or the file or
   // extension scope's extensions array.
@@ -2182,10 +2183,9 @@ inline const OneofDescriptor* FieldDescriptor::real_containing_oneof() const {
              : nullptr;
 }
 
-inline bool FieldDescriptor::is_singular_with_presence() const {
+inline bool FieldDescriptor::has_presence() const {
   if (is_repeated()) return false;
-  if (real_containing_oneof()) return false;
-  return cpp_type() == CPPTYPE_MESSAGE || proto3_optional_ ||
+  return cpp_type() == CPPTYPE_MESSAGE || containing_oneof() ||
          file()->syntax() == FileDescriptor::SYNTAX_PROTO2;
 }
 

src/google/protobuf/proto3_arena_unittest.cc

@@ -218,7 +218,7 @@ TEST(Proto3OptionalTest, OptionalFieldDescriptor) {
   for (int i = 0; i < d->field_count(); i++) {
     const FieldDescriptor* f = d->field(i);
     EXPECT_TRUE(f->has_optional_keyword()) << f->full_name();
-    EXPECT_TRUE(f->is_singular_with_presence()) << f->full_name();
+    EXPECT_TRUE(f->has_presence()) << f->full_name();
     EXPECT_TRUE(f->containing_oneof()) << f->full_name();
   }
 }
@@ -470,16 +470,8 @@ TEST(Proto3OptionalTest, ReflectiveSwapRoundTrip) {
 TEST(Proto3OptionalTest, PlainFields) {
   const Descriptor* d = TestAllTypes::descriptor();
 
-  for (int i = 0; i < d->field_count(); i++) {
-    const FieldDescriptor* f = d->field(i);
-    EXPECT_FALSE(f->has_optional_keyword()) << f->full_name();
-    if (f->is_optional() && f->cpp_type() != FieldDescriptor::CPPTYPE_MESSAGE) {
-      EXPECT_FALSE(f->is_singular_with_presence()) << f->full_name();
-    }
-  }
-
-  EXPECT_FALSE(
-      d->FindFieldByName("oneof_nested_message")->is_singular_with_presence());
+  EXPECT_FALSE(d->FindFieldByName("optional_int32")->has_presence());
+  EXPECT_TRUE(d->FindFieldByName("oneof_nested_message")->has_presence());
 }
 
 }  // namespace

src/google/protobuf/util/internal/protostream_objectsource.h

@@ -160,6 +160,9 @@ class PROTOBUF_EXPORT ProtoStreamObjectSource : public ObjectSource {
       const google::protobuf::Field& field) const;
 
 
+  // Returns the input stream.
+  io::CodedInputStream* stream() const { return stream_; }
+
  private:
   ProtoStreamObjectSource(io::CodedInputStream* stream,
                           const TypeInfo* typeinfo,
@@ -281,7 +284,7 @@ class PROTOBUF_EXPORT ProtoStreamObjectSource : public ObjectSource {
                                          StringPiece field_name) const;
 
   // Input stream to read from. Ownership rests with the caller.
-  io::CodedInputStream* stream_;
+  mutable io::CodedInputStream* stream_;
 
   // Type information for all the types used in the descriptor. Used to find
   // google::protobuf::Type of nested messages/enums.