Compare explicit zeroes from prototext in partially.

Right now, zeros are not allowed in explicit prototext comparison, frequently leading to very awkward manual checking. This tracks such explicit zeros, and ensures they remain 0.

PiperOrigin-RevId: 580729313

src/google/protobuf/message.h

@@ -135,6 +135,7 @@ class MapValueConstRef;
 class MapValueRef;
 class MapIterator;
 class MapReflectionTester;
+class TextFormat;
 
 namespace internal {
 struct FuzzPeer;
@@ -1010,6 +1011,7 @@ class PROTOBUF_EXPORT Reflection final {
     return schema_.IsSplit(field);
   }
 
+  friend class google::protobuf::TextFormat;
   friend class FastReflectionBase;
   friend class FastReflectionMessageMutator;
   friend bool internal::IsDescendant(Message& root, const Message& message);

src/google/protobuf/text_format.cc

@@ -295,6 +295,14 @@ const Descriptor* DefaultFinderFindAnyType(const Message& message,
 }
 }  // namespace
 
+const void* TextFormat::Parser::UnsetFieldsMetadata::GetUnsetFieldAddress(
+    const Message& message, const Reflection& reflection,
+    const FieldDescriptor& fd) {
+  // reflection->GetRaw() is a simple cast for any non-repeated type, so for
+  // simplicity we just pass in char as the template argument.
+  return &reflection.GetRaw<char>(message, &fd);
+}
+
 // ===========================================================================
 // Internal class for parsing an ASCII representation of a Protocol Message.
 // This class makes use of the Protocol Message compiler's tokenizer found
@@ -331,7 +339,7 @@ class TextFormat::Parser::ParserImpl {
              bool allow_unknown_extension, bool allow_unknown_enum,
              bool allow_field_number, bool allow_relaxed_whitespace,
              bool allow_partial, int recursion_limit,
-             bool error_on_no_op_fields)
+             UnsetFieldsMetadata* no_op_fields)
       : error_collector_(error_collector),
         finder_(finder),
         parse_info_tree_(parse_info_tree),
@@ -349,7 +357,7 @@ class TextFormat::Parser::ParserImpl {
         recursion_limit_(recursion_limit),
         had_silent_marker_(false),
         had_errors_(false),
-        error_on_no_op_fields_(error_on_no_op_fields) {
+        no_op_fields_(no_op_fields) {
     // For backwards-compatibility with proto1, we need to allow the 'f' suffix
     // for floats.
     tokenizer_.set_allow_f_after_float(true);
@@ -834,19 +842,21 @@ class TextFormat::Parser::ParserImpl {
 // When checking for no-op operations, We verify that both the existing value in
 // the message and the new value are the default. If the existing field value is
 // not the default, setting it to the default should not be treated as a no-op.
-#define SET_FIELD(CPPTYPE, CPPTYPELCASE, VALUE)                   \
-  if (field->is_repeated()) {                                     \
-    reflection->Add##CPPTYPE(message, field, VALUE);              \
-  } else {                                                        \
-    if (error_on_no_op_fields_ && !field->has_presence() &&       \
-        field->default_value_##CPPTYPELCASE() ==                  \
-            reflection->Get##CPPTYPE(*message, field) &&          \
-        field->default_value_##CPPTYPELCASE() == VALUE) {         \
-      ReportError("Input field " + field->full_name() +           \
-                  " did not change resulting proto.");            \
-    } else {                                                      \
-      reflection->Set##CPPTYPE(message, field, std::move(VALUE)); \
-    }                                                             \
+// The pointer of this is kept in no_op_fields_ for bookkeeping.
+#define SET_FIELD(CPPTYPE, CPPTYPELCASE, VALUE)                            \
+  if (field->is_repeated()) {                                              \
+    reflection->Add##CPPTYPE(message, field, VALUE);                       \
+  } else {                                                                 \
+    if (no_op_fields_ && !field->has_presence() &&                         \
+        field->default_value_##CPPTYPELCASE() ==                           \
+            reflection->Get##CPPTYPE(*message, field) &&                   \
+        field->default_value_##CPPTYPELCASE() == VALUE) {                  \
+      no_op_fields_->addresses_.insert(                                    \
+          UnsetFieldsMetadata::GetUnsetFieldAddress(*message, *reflection, \
+                                                    *field));              \
+    } else {                                                               \
+      reflection->Set##CPPTYPE(message, field, std::move(VALUE));          \
+    }                                                                      \
   }
 
     switch (field->cpp_type()) {
@@ -1429,7 +1439,7 @@ class TextFormat::Parser::ParserImpl {
   int recursion_limit_;
   bool had_silent_marker_;
   bool had_errors_;
-  bool error_on_no_op_fields_;
+  UnsetFieldsMetadata* no_op_fields_{};
 
 };
 
@@ -1727,7 +1737,7 @@ bool TextFormat::Parser::Parse(io::ZeroCopyInputStream* input,
                     allow_case_insensitive_field_, allow_unknown_field_,
                     allow_unknown_extension_, allow_unknown_enum_,
                     allow_field_number_, allow_relaxed_whitespace_,
-                    allow_partial_, recursion_limit_, error_on_no_op_fields_);
+                    allow_partial_, recursion_limit_, no_op_fields_);
   return MergeUsingImpl(input, output, &parser);
 }
 
@@ -1752,7 +1762,7 @@ bool TextFormat::Parser::Merge(io::ZeroCopyInputStream* input,
                     allow_case_insensitive_field_, allow_unknown_field_,
                     allow_unknown_extension_, allow_unknown_enum_,
                     allow_field_number_, allow_relaxed_whitespace_,
-                    allow_partial_, recursion_limit_, error_on_no_op_fields_);
+                    allow_partial_, recursion_limit_, no_op_fields_);
   return MergeUsingImpl(input, output, &parser);
 }
 
@@ -1788,7 +1798,7 @@ bool TextFormat::Parser::ParseFieldValueFromString(absl::string_view input,
                     allow_case_insensitive_field_, allow_unknown_field_,
                     allow_unknown_extension_, allow_unknown_enum_,
                     allow_field_number_, allow_relaxed_whitespace_,
-                    allow_partial_, recursion_limit_, error_on_no_op_fields_);
+                    allow_partial_, recursion_limit_, no_op_fields_);
   return parser.ParseField(field, output);
 }
 

src/google/protobuf/text_format.h

@@ -21,6 +21,7 @@
 #include <vector>
 
 #include "absl/container/flat_hash_map.h"
+#include "absl/container/flat_hash_set.h"
 #include "absl/strings/cord.h"
 #include "absl/strings/string_view.h"
 #include "google/protobuf/descriptor.h"
@@ -81,6 +82,9 @@ PROTOBUF_EXPORT enum class Option;
 // Converts a protobuf message to a string with redaction enabled.
 PROTOBUF_EXPORT std::string StringifyMessage(const Message& message,
                                              Option option);
+
+class UnsetFieldsMetadataTextFormatTestUtil;
+class UnsetFieldsMetadataMessageDifferencerTestUtil;
 }  // namespace internal
 
 // This class implements protocol buffer text format, colloquially known as text
@@ -719,11 +723,40 @@ class PROTOBUF_EXPORT TextFormat {
     // the maximum allowed nesting of proto messages.
     void SetRecursionLimit(int limit) { recursion_limit_ = limit; }
 
-    // If called, the parser will report an error if a parsed field had no
+    // Uniquely addresses fields in a message that was explicitly unset in
+    // textproto. Example:
+    // "some_int_field: 0"
+    // where some_int_field is non-optional.
+    //
+    // This class should only be used to pass data between the text_format
+    // parser and the MessageDifferencer.
+    class UnsetFieldsMetadata {
+     public:
+      UnsetFieldsMetadata() = default;
+
+     private:
+      // Return a pointer to the unset field in the given message.
+      static const void* GetUnsetFieldAddress(const Message& message,
+                                              const Reflection& reflection,
+                                              const FieldDescriptor& fd);
+
+      // List of addresses of explicitly unset proto fields.
+      absl::flat_hash_set<const void*> addresses_;
+
+      friend class ::google::protobuf::internal::
+          UnsetFieldsMetadataMessageDifferencerTestUtil;
+      friend class ::google::protobuf::internal::UnsetFieldsMetadataTextFormatTestUtil;
+      friend class ::google::protobuf::util::MessageDifferencer;
+      friend class ::google::protobuf::TextFormat::Parser;
+    };
+
+    // If called, the parser will report the parsed fields that had no
     // effect on the resulting proto (for example, fields with no presence that
-    // were set to their default value).
-    void ErrorOnNoOpFields(bool return_error) {
-      error_on_no_op_fields_ = return_error;
+    // were set to their default value). These can be passed to the Partially()
+    // matcher as an indicator to explicitly check these fields are missing
+    // in the actual.
+    void OutputNoOpFields(UnsetFieldsMetadata* no_op_fields) {
+      no_op_fields_ = no_op_fields;
     }
 
    private:
@@ -748,7 +781,7 @@ class PROTOBUF_EXPORT TextFormat {
     bool allow_relaxed_whitespace_;
     bool allow_singular_overwrites_;
     int recursion_limit_;
-    bool error_on_no_op_fields_ = false;
+    UnsetFieldsMetadata* no_op_fields_ = nullptr;
   };
 
 

src/google/protobuf/text_format_unittest.cc

@@ -15,9 +15,11 @@
 #include <stdlib.h>
 
 #include <atomic>
+#include <cstdint>
 #include <limits>
 #include <memory>
 #include <string>
+#include <vector>
 
 #include "google/protobuf/testing/file.h"
 #include "google/protobuf/testing/file.h"
@@ -34,6 +36,7 @@
 #include "absl/strings/str_format.h"
 #include "absl/strings/str_replace.h"
 #include "absl/strings/substitute.h"
+#include "google/protobuf/descriptor.h"
 #include "google/protobuf/io/tokenizer.h"
 #include "google/protobuf/io/zero_copy_stream_impl.h"
 #include "google/protobuf/map_unittest.pb.h"
@@ -52,10 +55,33 @@
 namespace google {
 namespace protobuf {
 
+namespace internal {
+class UnsetFieldsMetadataTextFormatTestUtil {
+ public:
+  static std::vector<const void*> GetRawAddresses(
+      const TextFormat::Parser::UnsetFieldsMetadata& metadata) {
+    return std::vector<const void*>(metadata.addresses_.begin(),
+                                    metadata.addresses_.end());
+  }
+  static const void* GetAddress(const Message& message,
+                                const Reflection& reflection,
+                                const FieldDescriptor& fd) {
+    return TextFormat::Parser::UnsetFieldsMetadata::GetUnsetFieldAddress(
+        message, reflection, fd);
+  }
+
+  static int32_t NumFields(
+      const TextFormat::Parser::UnsetFieldsMetadata& metadata) {
+    return metadata.addresses_.size();
+  }
+};
+}  // namespace internal
+
 // Can't use an anonymous namespace here due to brokenness of Tru64 compiler.
 namespace text_format_unittest {
 
 using ::google::protobuf::internal::kDebugStringSilentMarker;
+using ::google::protobuf::internal::UnsetFieldsMetadataTextFormatTestUtil;
 using ::testing::AllOf;
 using ::testing::HasSubstr;
 
@@ -997,86 +1023,207 @@ TEST_F(TextFormatTest, ParseUnknownEnumFieldProto3) {
   EXPECT_EQ(-2147483648, proto.repeated_nested_enum(3));
 }
 
-TEST_F(TextFormatTest, ErrorOnNoOpFieldsProto3) {
+TEST_F(TextFormatTest, PopulatesNoOpFields) {
   proto3_unittest::TestAllTypes proto;
   TextFormat::Parser parser;
-  parser.ErrorOnNoOpFields(true);
+  TextFormat::Parser::UnsetFieldsMetadata no_op_fields;
+  parser.OutputNoOpFields(&no_op_fields);
 
   {
+    no_op_fields = {};
     const absl::string_view singular_int_parse_string = "optional_int32: 0";
     EXPECT_TRUE(TextFormat::ParseFromString(singular_int_parse_string, &proto));
-    EXPECT_FALSE(parser.ParseFromString(singular_int_parse_string, &proto));
+    EXPECT_TRUE(parser.ParseFromString(singular_int_parse_string, &proto));
+    EXPECT_EQ(UnsetFieldsMetadataTextFormatTestUtil::NumFields(no_op_fields),
+              1);
   }
   {
+    no_op_fields = {};
     const absl::string_view singular_bool_parse_string = "optional_bool: false";
     EXPECT_TRUE(
         TextFormat::ParseFromString(singular_bool_parse_string, &proto));
-    EXPECT_FALSE(parser.ParseFromString(singular_bool_parse_string, &proto));
+    EXPECT_TRUE(parser.ParseFromString(singular_bool_parse_string, &proto));
+    EXPECT_EQ(UnsetFieldsMetadataTextFormatTestUtil::NumFields(no_op_fields),
+              1);
   }
   {
+    no_op_fields = {};
     const absl::string_view singular_string_parse_string =
         "optional_string: ''";
     EXPECT_TRUE(
         TextFormat::ParseFromString(singular_string_parse_string, &proto));
-    EXPECT_FALSE(parser.ParseFromString(singular_string_parse_string, &proto));
+    EXPECT_TRUE(parser.ParseFromString(singular_string_parse_string, &proto));
+    EXPECT_EQ(UnsetFieldsMetadataTextFormatTestUtil::NumFields(no_op_fields),
+              1);
   }
   {
+    no_op_fields = {};
     const absl::string_view nested_message_parse_string =
         "optional_nested_message { bb: 0 } ";
     EXPECT_TRUE(
         TextFormat::ParseFromString(nested_message_parse_string, &proto));
-    EXPECT_FALSE(parser.ParseFromString(nested_message_parse_string, &proto));
+    EXPECT_TRUE(parser.ParseFromString(nested_message_parse_string, &proto));
+    EXPECT_EQ(UnsetFieldsMetadataTextFormatTestUtil::NumFields(no_op_fields),
+              1);
+  }
+  {
+    no_op_fields = {};
+    const absl::string_view nested_message_parse_string =
+        "optional_nested_message { bb: 1 } ";
+    EXPECT_TRUE(
+        TextFormat::ParseFromString(nested_message_parse_string, &proto));
+    EXPECT_TRUE(parser.ParseFromString(nested_message_parse_string, &proto));
+    EXPECT_EQ(UnsetFieldsMetadataTextFormatTestUtil::NumFields(no_op_fields),
+              0);
   }
   {
+    no_op_fields = {};
     const absl::string_view foreign_message_parse_string =
         "optional_foreign_message { c: 0 } ";
     EXPECT_TRUE(
         TextFormat::ParseFromString(foreign_message_parse_string, &proto));
-    EXPECT_FALSE(parser.ParseFromString(foreign_message_parse_string, &proto));
+    EXPECT_TRUE(parser.ParseFromString(foreign_message_parse_string, &proto));
+    EXPECT_EQ(UnsetFieldsMetadataTextFormatTestUtil::NumFields(no_op_fields),
+              1);
   }
   {
+    no_op_fields = {};
     const absl::string_view nested_enum_parse_string =
         "optional_nested_enum: ZERO ";
     EXPECT_TRUE(TextFormat::ParseFromString(nested_enum_parse_string, &proto));
-    EXPECT_FALSE(parser.ParseFromString(nested_enum_parse_string, &proto));
+    EXPECT_TRUE(parser.ParseFromString(nested_enum_parse_string, &proto));
+    EXPECT_EQ(UnsetFieldsMetadataTextFormatTestUtil::NumFields(no_op_fields),
+              1);
   }
   {
+    no_op_fields = {};
     const absl::string_view foreign_enum_parse_string =
         "optional_foreign_enum: FOREIGN_ZERO ";
     EXPECT_TRUE(TextFormat::ParseFromString(foreign_enum_parse_string, &proto));
-    EXPECT_FALSE(parser.ParseFromString(foreign_enum_parse_string, &proto));
+    EXPECT_TRUE(parser.ParseFromString(foreign_enum_parse_string, &proto));
+    EXPECT_EQ(UnsetFieldsMetadataTextFormatTestUtil::NumFields(no_op_fields),
+              1);
   }
   {
+    no_op_fields = {};
     const absl::string_view string_piece_parse_string =
         "optional_string_piece: '' ";
     EXPECT_TRUE(TextFormat::ParseFromString(string_piece_parse_string, &proto));
-    EXPECT_FALSE(parser.ParseFromString(string_piece_parse_string, &proto));
+    EXPECT_TRUE(parser.ParseFromString(string_piece_parse_string, &proto));
+    EXPECT_EQ(UnsetFieldsMetadataTextFormatTestUtil::NumFields(no_op_fields),
+              1);
   }
   {
+    no_op_fields = {};
     const absl::string_view cord_parse_string = "optional_cord: '' ";
     EXPECT_TRUE(TextFormat::ParseFromString(cord_parse_string, &proto));
-    EXPECT_FALSE(parser.ParseFromString(cord_parse_string, &proto));
+    EXPECT_TRUE(parser.ParseFromString(cord_parse_string, &proto));
+    EXPECT_EQ(UnsetFieldsMetadataTextFormatTestUtil::NumFields(no_op_fields),
+              1);
   }
   {
+    no_op_fields = {};
     // Sanity check that repeated fields work the same.
     const absl::string_view repeated_int32_parse_string = "repeated_int32: 0 ";
     EXPECT_TRUE(
         TextFormat::ParseFromString(repeated_int32_parse_string, &proto));
     EXPECT_TRUE(parser.ParseFromString(repeated_int32_parse_string, &proto));
+    EXPECT_EQ(UnsetFieldsMetadataTextFormatTestUtil::NumFields(no_op_fields),
+              0);
   }
   {
+    no_op_fields = {};
     const absl::string_view repeated_bool_parse_string =
         "repeated_bool: false  ";
     EXPECT_TRUE(
         TextFormat::ParseFromString(repeated_bool_parse_string, &proto));
     EXPECT_TRUE(parser.ParseFromString(repeated_bool_parse_string, &proto));
+    EXPECT_EQ(UnsetFieldsMetadataTextFormatTestUtil::NumFields(no_op_fields),
+              0);
   }
   {
+    no_op_fields = {};
     const absl::string_view repeated_string_parse_string =
         "repeated_string: '' ";
     EXPECT_TRUE(
         TextFormat::ParseFromString(repeated_string_parse_string, &proto));
     EXPECT_TRUE(parser.ParseFromString(repeated_string_parse_string, &proto));
+    EXPECT_EQ(UnsetFieldsMetadataTextFormatTestUtil::NumFields(no_op_fields),
+              0);
+  }
+}
+
+TEST_F(TextFormatTest, FieldsPopulatedCorrectly) {
+  proto3_unittest::TestAllTypes proto;
+  TextFormat::Parser parser;
+  TextFormat::Parser::UnsetFieldsMetadata no_op_fields;
+  parser.OutputNoOpFields(&no_op_fields);
+  {
+    no_op_fields = {};
+    const absl::string_view parse_string = R"pb(
+      optional_int32: 0
+      optional_uint32: 10
+      optional_nested_message { bb: 0 }
+    )pb";
+    EXPECT_TRUE(parser.ParseFromString(parse_string, &proto));
+    ASSERT_EQ(UnsetFieldsMetadataTextFormatTestUtil::NumFields(no_op_fields),
+              2);
+    std::vector<const void*> ptrs =
+        UnsetFieldsMetadataTextFormatTestUtil::GetRawAddresses(no_op_fields);
+    EXPECT_EQ(*static_cast<const int32_t*>(ptrs[0]), 0);
+    EXPECT_EQ(*static_cast<const int32_t*>(ptrs[1]), 0);
+    proto.set_optional_int32(20);
+    proto.mutable_optional_nested_message()->set_bb(30);
+    const std::vector<int32_t> new_values{
+        *static_cast<const int32_t*>(ptrs[0]),
+        *static_cast<const int32_t*>(ptrs[1])};
+    EXPECT_THAT(new_values, testing::UnorderedElementsAre(20, 30));
+  }
+  {
+    no_op_fields = {};
+    const absl::string_view parse_string = R"pb(
+      optional_bool: false
+      optional_uint32: 10
+      optional_nested_message { bb: 20 }
+    )pb";
+    EXPECT_TRUE(parser.ParseFromString(parse_string, &proto));
+    ASSERT_EQ(UnsetFieldsMetadataTextFormatTestUtil::NumFields(no_op_fields),
+              1);
+    std::vector<const void*> ptrs =
+        UnsetFieldsMetadataTextFormatTestUtil::GetRawAddresses(no_op_fields);
+    EXPECT_EQ(*static_cast<const bool*>(ptrs[0]), false);
+    proto.set_optional_bool(true);
+    EXPECT_EQ(*static_cast<const bool*>(ptrs[0]), true);
+  }
+  {
+    // The address returned by the field is a string_view, which is a separate
+    // alocation. Check address directly.
+    no_op_fields = {};
+    const absl::string_view parse_string = "optional_string: \"\"";
+    EXPECT_TRUE(parser.ParseFromString(parse_string, &proto));
+    ASSERT_EQ(UnsetFieldsMetadataTextFormatTestUtil::NumFields(no_op_fields),
+              1);
+    const Reflection* reflection = proto.GetReflection();
+    const FieldDescriptor* field = proto.GetDescriptor()->FindFieldByNumber(14);
+    EXPECT_EQ(
+        UnsetFieldsMetadataTextFormatTestUtil::GetRawAddresses(no_op_fields)[0],
+        UnsetFieldsMetadataTextFormatTestUtil::GetAddress(proto, *reflection,
+                                                          *field));
+  }
+  {
+    // The address returned by the field is a string_view, which is a separate
+    // alocation. Check address directly.
+    no_op_fields = {};
+    const absl::string_view parse_string = "optional_bytes: \"\"";
+    EXPECT_TRUE(parser.ParseFromString(parse_string, &proto));
+    ASSERT_EQ(UnsetFieldsMetadataTextFormatTestUtil::NumFields(no_op_fields),
+              1);
+    const Reflection* reflection = proto.GetReflection();
+    const FieldDescriptor* field = proto.GetDescriptor()->FindFieldByNumber(15);
+    EXPECT_EQ(
+        UnsetFieldsMetadataTextFormatTestUtil::GetRawAddresses(no_op_fields)[0],
+        UnsetFieldsMetadataTextFormatTestUtil::GetAddress(proto, *reflection,
+                                                          *field));
   }
 }
 

src/google/protobuf/util/message_differencer.cc

@@ -696,7 +696,7 @@ bool MessageDifferencer::CompareRequestedFieldsUsingSettings(
       // we are merely checking for a difference in field values,
       // rather than the addition or deletion of fields).
       std::vector<const FieldDescriptor*> fields_union =
-          CombineFields(message1_fields, FULL, message2_fields, FULL);
+          CombineFields(message1, message1_fields, FULL, message2_fields, FULL);
       return CompareWithFieldsInternal(message1, message2, unpacked_any,
                                        fields_union, fields_union,
                                        parent_fields);
@@ -719,8 +719,8 @@ bool MessageDifferencer::CompareRequestedFieldsUsingSettings(
       // but only the intersection for message2.  This way, any fields
       // only present in message2 will be ignored, but any fields only
       // present in message1 will be marked as a difference.
-      std::vector<const FieldDescriptor*> fields_intersection =
-          CombineFields(message1_fields, PARTIAL, message2_fields, PARTIAL);
+      std::vector<const FieldDescriptor*> fields_intersection = CombineFields(
+          message1, message1_fields, PARTIAL, message2_fields, PARTIAL);
       return CompareWithFieldsInternal(message1, message2, unpacked_any,
                                        message1_fields, fields_intersection,
                                        parent_fields);
@@ -728,9 +728,48 @@ bool MessageDifferencer::CompareRequestedFieldsUsingSettings(
   }
 }
 
+namespace {
+bool ValidMissingField(const FieldDescriptor& f) {
+  switch (f.cpp_type()) {
+    case FieldDescriptor::CPPTYPE_INT32:
+    case FieldDescriptor::CPPTYPE_UINT32:
+    case FieldDescriptor::CPPTYPE_INT64:
+    case FieldDescriptor::CPPTYPE_UINT64:
+    case FieldDescriptor::CPPTYPE_FLOAT:
+    case FieldDescriptor::CPPTYPE_DOUBLE:
+    case FieldDescriptor::CPPTYPE_STRING:
+    case FieldDescriptor::CPPTYPE_BOOL:
+    case FieldDescriptor::CPPTYPE_ENUM:
+      return true;
+    default:
+      return false;
+  }
+}
+}  // namespace
+
+bool MessageDifferencer::ShouldCompareNoPresence(
+    const Message& message1, const Reflection& reflection1,
+    const FieldDescriptor* field2) const {
+  const bool compare_no_presence_by_field = force_compare_no_presence_ &&
+                                            !field2->has_presence() &&
+                                            !field2->is_repeated();
+  if (compare_no_presence_by_field) {
+    return true;
+  }
+  const bool compare_no_presence_by_address =
+      !field2->is_repeated() && !field2->has_presence() &&
+      ValidMissingField(*field2) &&
+      require_no_presence_fields_.addresses_.contains(
+          TextFormat::Parser::UnsetFieldsMetadata::GetUnsetFieldAddress(
+              message1, reflection1, *field2));
+  return compare_no_presence_by_address;
+}
+
 std::vector<const FieldDescriptor*> MessageDifferencer::CombineFields(
-    const std::vector<const FieldDescriptor*>& fields1, Scope fields1_scope,
-    const std::vector<const FieldDescriptor*>& fields2, Scope fields2_scope) {
+    const Message& message1, const std::vector<const FieldDescriptor*>& fields1,
+    Scope fields1_scope, const std::vector<const FieldDescriptor*>& fields2,
+    Scope fields2_scope) {
+  const Reflection* reflection1 = message1.GetReflection();
   size_t index1 = 0;
   size_t index2 = 0;
 
@@ -748,8 +787,8 @@ std::vector<const FieldDescriptor*> MessageDifferencer::CombineFields(
     } else if (FieldBefore(field2, field1)) {
       if (fields2_scope == FULL) {
         tmp_message_fields_.push_back(field2);
-      } else if (fields2_scope == PARTIAL && force_compare_no_presence_ &&
-                 !field2->has_presence() && !field2->is_repeated()) {
+      } else if (fields2_scope == PARTIAL &&
+                 ShouldCompareNoPresence(message1, *reflection1, field2)) {
         // In order to make MessageDifferencer play nicely with no-presence
         // fields in unit tests, we want to check if the expected proto
         // (message1) has some fields which are set to their default value but

src/google/protobuf/util/message_differencer.h

@@ -32,6 +32,7 @@
 #include "absl/log/absl_check.h"
 #include "google/protobuf/descriptor.h"  // FieldDescriptor
 #include "google/protobuf/message.h"     // Message
+#include "google/protobuf/text_format.h"
 #include "google/protobuf/unknown_field_set.h"
 #include "google/protobuf/util/field_comparator.h"
 
@@ -581,6 +582,13 @@ class PROTOBUF_EXPORT MessageDifferencer {
   // in the comparison.
   void set_force_compare_no_presence(bool value);
 
+  // If set, the fields in message1 that equal the fields passed here will be
+  // treated as required for comparison, even if they are absent.
+  void set_require_no_presence_fields(
+      const google::protobuf::TextFormat::Parser::UnsetFieldsMetadata& fields) {
+    require_no_presence_fields_ = fields;
+  }
+
   // DEPRECATED. Pass a DefaultFieldComparator instance instead.
   // Sets the type of comparison (as defined in the FloatComparison enumeration
   // above) that is used by this differencer when comparing float (and double)
@@ -768,6 +776,7 @@ class PROTOBUF_EXPORT MessageDifferencer {
   // list.  Fields only present in one of the lists will only appear in the
   // combined list if the corresponding fields_scope option is set to FULL.
   std::vector<const FieldDescriptor*> CombineFields(
+      const Message& message1,
       const std::vector<const FieldDescriptor*>& fields1, Scope fields1_scope,
       const std::vector<const FieldDescriptor*>& fields2, Scope fields2_scope);
 
@@ -919,11 +928,17 @@ class PROTOBUF_EXPORT MessageDifferencer {
       const FieldDescriptor* field,
       const RepeatedFieldComparison& new_comparison);
 
+  // Whether we should still compare the field despite its absence in message1.
+  bool ShouldCompareNoPresence(const Message& message1,
+                               const Reflection& reflection1,
+                               const FieldDescriptor* field2) const;
+
   Reporter* reporter_;
   DefaultFieldComparator default_field_comparator_;
   MessageFieldComparison message_field_comparison_;
   Scope scope_;
   absl::flat_hash_set<const FieldDescriptor*> force_compare_no_presence_fields_;
+  google::protobuf::TextFormat::Parser::UnsetFieldsMetadata require_no_presence_fields_;
   absl::flat_hash_set<std::string> force_compare_failure_triggering_fields_;
   RepeatedFieldComparison repeated_field_comparison_;
 

src/google/protobuf/util/message_differencer_unittest.cc

@@ -29,8 +29,10 @@
 #include "absl/strings/str_split.h"
 #include "absl/strings/string_view.h"
 #include "google/protobuf/any_test.pb.h"
+#include "google/protobuf/descriptor.h"
 #include "google/protobuf/map_test_util.h"
 #include "google/protobuf/map_unittest.pb.h"
+#include "google/protobuf/message.h"
 #include "google/protobuf/test_util.h"
 #include "google/protobuf/text_format.h"
 #include "google/protobuf/unittest.pb.h"
@@ -44,12 +46,31 @@
 namespace google {
 namespace protobuf {
 
+namespace internal {
+class UnsetFieldsMetadataMessageDifferencerTestUtil {
+ public:
+  static void AddExplicitUnsetField(
+      const Message& message, const Reflection& reflection,
+      const FieldDescriptor& fd,
+      TextFormat::Parser::UnsetFieldsMetadata* metadata) {
+    metadata->addresses_.insert(
+        TextFormat::Parser::UnsetFieldsMetadata::GetUnsetFieldAddress(
+            message, reflection, fd));
+  }
+};
+}  // namespace internal
+
 namespace {
 
+using ::google::protobuf::internal::UnsetFieldsMetadataMessageDifferencerTestUtil;
+
 
 proto3_unittest::TestNoPresenceField MakeTestNoPresenceField() {
   proto3_unittest::TestNoPresenceField msg1, msg2;
   msg1.set_no_presence_bool(true);
+  msg1.set_no_presence_bool2(true);
+  msg1.set_no_presence_bool3(true);
+  msg1.set_no_presence_string("yolo");
   msg2 = msg1;
   *msg1.mutable_no_presence_nested() = msg2;
   *msg1.add_no_presence_repeated_nested() = msg2;
@@ -378,6 +399,212 @@ TEST(MessageDifferencerTest,
   EXPECT_FALSE(default_differencer.Compare(msg2, msg1));
 }
 
+TEST(MessageDifferencerTest,
+     PartialEqualityTestBooleanPresenceFieldMissingWithAddress) {
+  util::MessageDifferencer address_differencer;
+  address_differencer.set_scope(util::MessageDifferencer::PARTIAL);
+
+  // This differencer is not setting force_compare_no_presence.
+  util::MessageDifferencer default_differencer;
+  default_differencer.set_scope(util::MessageDifferencer::PARTIAL);
+  default_differencer.set_force_compare_no_presence(false);
+
+  // When clearing a singular no presence field, it will be included in the
+  // comparison.
+  proto3_unittest::TestNoPresenceField msg1 = MakeTestNoPresenceField();
+  proto3_unittest::TestNoPresenceField msg2 = MakeTestNoPresenceField();
+
+  const Reflection* reflection1 = msg1.GetReflection();
+  const FieldDescriptor* fd1 = msg1.GetDescriptor()->FindFieldByNumber(1);
+  TextFormat::Parser::UnsetFieldsMetadata metadata;
+  UnsetFieldsMetadataMessageDifferencerTestUtil::AddExplicitUnsetField(
+      msg1, *reflection1, *fd1, &metadata);
+  address_differencer.set_require_no_presence_fields(metadata);
+
+  EXPECT_TRUE(address_differencer.Compare(msg1, msg2));
+  EXPECT_TRUE(default_differencer.Compare(msg1, msg2));
+
+  msg1.clear_no_presence_bool();
+
+  EXPECT_FALSE(address_differencer.Compare(msg1, msg2));
+  EXPECT_TRUE(default_differencer.Compare(msg1, msg2));
+
+  msg2.clear_no_presence_bool();
+
+  EXPECT_TRUE(address_differencer.Compare(msg1, msg2));
+  EXPECT_TRUE(default_differencer.Compare(msg1, msg2));
+
+  msg1.set_no_presence_bool(true);
+
+  EXPECT_FALSE(default_differencer.Compare(msg1, msg2));
+  EXPECT_FALSE(address_differencer.Compare(msg1, msg2));
+}
+
+TEST(MessageDifferencerTest,
+     PartialEqualityTestStringPresenceFieldMissingWithAddress) {
+  util::MessageDifferencer address_differencer;
+  address_differencer.set_scope(util::MessageDifferencer::PARTIAL);
+
+  // This differencer is not setting force_compare_no_presence.
+  util::MessageDifferencer default_differencer;
+  default_differencer.set_scope(util::MessageDifferencer::PARTIAL);
+  default_differencer.set_force_compare_no_presence(false);
+
+  // When clearing a singular no presence field, it will be included in the
+  // comparison.
+  proto3_unittest::TestNoPresenceField msg1 = MakeTestNoPresenceField();
+  proto3_unittest::TestNoPresenceField msg2 = MakeTestNoPresenceField();
+
+  const Reflection* reflection1 = msg1.GetReflection();
+  const FieldDescriptor* fd1 = msg1.GetDescriptor()->FindFieldByNumber(4);
+  TextFormat::Parser::UnsetFieldsMetadata metadata;
+  UnsetFieldsMetadataMessageDifferencerTestUtil::AddExplicitUnsetField(
+      msg1, *reflection1, *fd1, &metadata);
+  address_differencer.set_require_no_presence_fields(metadata);
+
+  EXPECT_TRUE(address_differencer.Compare(msg1, msg2));
+  EXPECT_TRUE(default_differencer.Compare(msg1, msg2));
+
+  msg1.clear_no_presence_string();
+
+  EXPECT_FALSE(address_differencer.Compare(msg1, msg2));
+  EXPECT_TRUE(default_differencer.Compare(msg1, msg2));
+
+  msg2.clear_no_presence_string();
+
+  EXPECT_TRUE(address_differencer.Compare(msg1, msg2));
+  EXPECT_TRUE(default_differencer.Compare(msg1, msg2));
+
+  msg1.set_no_presence_string("yolo");
+
+  EXPECT_FALSE(default_differencer.Compare(msg1, msg2));
+  EXPECT_FALSE(address_differencer.Compare(msg1, msg2));
+}
+
+// Ensure multiple booleans are addressed distinctly. This is trivially the case
+// now, but tests against possible optimizations in the future to use bitfields.
+TEST(MessageDifferencerTest,
+     PartialEqualityTestTwoBoolsPresenceFieldMissingWithAddress) {
+  util::MessageDifferencer address_differencer;
+  address_differencer.set_scope(util::MessageDifferencer::PARTIAL);
+
+  // This differencer is not setting force_compare_no_presence.
+  util::MessageDifferencer default_differencer;
+  default_differencer.set_scope(util::MessageDifferencer::PARTIAL);
+  default_differencer.set_force_compare_no_presence(false);
+
+  // When clearing a singular no presence field, it will be included in the
+  // comparison.
+  proto3_unittest::TestNoPresenceField msg1 = MakeTestNoPresenceField();
+  proto3_unittest::TestNoPresenceField msg2 = MakeTestNoPresenceField();
+
+  const Reflection* reflection1 = msg1.GetReflection();
+  const FieldDescriptor* fd1 = msg1.GetDescriptor()->FindFieldByNumber(5);
+  TextFormat::Parser::UnsetFieldsMetadata metadata;
+  UnsetFieldsMetadataMessageDifferencerTestUtil::AddExplicitUnsetField(
+      msg1, *reflection1, *fd1, &metadata);
+  address_differencer.set_require_no_presence_fields(metadata);
+
+  EXPECT_TRUE(address_differencer.Compare(msg1, msg2));
+  EXPECT_TRUE(default_differencer.Compare(msg1, msg2));
+
+  // Trigger on bool2.
+  msg1.clear_no_presence_bool2();
+
+  EXPECT_FALSE(address_differencer.Compare(msg1, msg2));
+  EXPECT_TRUE(default_differencer.Compare(msg1, msg2));
+
+  // Triggering on bool2 still ignores bool3.
+  msg1.set_no_presence_bool2(true);
+  msg1.clear_no_presence_bool3();
+
+  EXPECT_TRUE(address_differencer.Compare(msg1, msg2));
+  EXPECT_TRUE(default_differencer.Compare(msg1, msg2));
+}
+
+TEST(MessageDifferencerTest,
+     PartialEqualityTestBooleanNestedMessagePresenceFieldMissingWithAddress) {
+  util::MessageDifferencer address_differencer;
+  address_differencer.set_scope(util::MessageDifferencer::PARTIAL);
+
+  // This differencer is not setting force_compare_no_presence.
+  util::MessageDifferencer default_differencer;
+  default_differencer.set_scope(util::MessageDifferencer::PARTIAL);
+  default_differencer.set_force_compare_no_presence(false);
+
+  // When clearing a singular no presence field, it will be included in the
+  // comparison.
+  proto3_unittest::TestNoPresenceField msg1 = MakeTestNoPresenceField();
+  proto3_unittest::TestNoPresenceField msg2 = MakeTestNoPresenceField();
+
+  const Reflection* reflection1 = msg1.no_presence_nested().GetReflection();
+  const FieldDescriptor* fd1 = msg1.GetDescriptor()->FindFieldByNumber(1);
+  TextFormat::Parser::UnsetFieldsMetadata metadata;
+  UnsetFieldsMetadataMessageDifferencerTestUtil::AddExplicitUnsetField(
+      msg1.no_presence_nested(), *reflection1, *fd1, &metadata);
+  address_differencer.set_require_no_presence_fields(metadata);
+
+  EXPECT_TRUE(address_differencer.Compare(msg1, msg2));
+  EXPECT_TRUE(default_differencer.Compare(msg1, msg2));
+
+  msg1.mutable_no_presence_nested()->clear_no_presence_bool();
+
+  EXPECT_FALSE(address_differencer.Compare(msg1, msg2));
+  EXPECT_TRUE(default_differencer.Compare(msg1, msg2));
+
+  msg2.mutable_no_presence_nested()->clear_no_presence_bool();
+
+  EXPECT_TRUE(address_differencer.Compare(msg1, msg2));
+  EXPECT_TRUE(default_differencer.Compare(msg1, msg2));
+
+  msg1.mutable_no_presence_nested()->set_no_presence_bool(true);
+
+  EXPECT_FALSE(default_differencer.Compare(msg1, msg2));
+  EXPECT_FALSE(address_differencer.Compare(msg1, msg2));
+}
+
+TEST(MessageDifferencerTest,
+     PartialEqualityTestBooleanRepeatedMessagePresenceFieldMissingWithAddress) {
+  util::MessageDifferencer address_differencer;
+  address_differencer.set_scope(util::MessageDifferencer::PARTIAL);
+
+  // This differencer is not setting force_compare_no_presence.
+  util::MessageDifferencer default_differencer;
+  default_differencer.set_scope(util::MessageDifferencer::PARTIAL);
+  default_differencer.set_force_compare_no_presence(false);
+
+  // When clearing a singular no presence field, it will be included in the
+  // comparison.
+  proto3_unittest::TestNoPresenceField msg1 = MakeTestNoPresenceField();
+  proto3_unittest::TestNoPresenceField msg2 = MakeTestNoPresenceField();
+
+  const Reflection* reflection1 =
+      msg1.no_presence_repeated_nested(0).GetReflection();
+  const FieldDescriptor* fd1 = msg1.GetDescriptor()->FindFieldByNumber(1);
+  TextFormat::Parser::UnsetFieldsMetadata metadata;
+  UnsetFieldsMetadataMessageDifferencerTestUtil::AddExplicitUnsetField(
+      msg1.no_presence_repeated_nested(0), *reflection1, *fd1, &metadata);
+  address_differencer.set_require_no_presence_fields(metadata);
+
+  EXPECT_TRUE(address_differencer.Compare(msg1, msg2));
+  EXPECT_TRUE(default_differencer.Compare(msg1, msg2));
+
+  msg1.mutable_no_presence_repeated_nested(0)->clear_no_presence_bool();
+
+  EXPECT_FALSE(address_differencer.Compare(msg1, msg2));
+  EXPECT_TRUE(default_differencer.Compare(msg1, msg2));
+
+  msg2.mutable_no_presence_repeated_nested(0)->clear_no_presence_bool();
+
+  EXPECT_TRUE(address_differencer.Compare(msg1, msg2));
+  EXPECT_TRUE(default_differencer.Compare(msg1, msg2));
+
+  msg1.mutable_no_presence_repeated_nested(0)->set_no_presence_bool(true);
+
+  EXPECT_FALSE(default_differencer.Compare(msg1, msg2));
+  EXPECT_FALSE(address_differencer.Compare(msg1, msg2));
+}
+
 TEST(MessageDifferencerTest,
      PartialEqualityTestExtraFieldNoPresenceForceCompareReporterAware) {
   std::string output;

src/google/protobuf/util/message_differencer_unittest_proto3.proto

@@ -18,4 +18,7 @@ message TestNoPresenceField {
   bool no_presence_bool = 1;
   TestNoPresenceField no_presence_nested = 2;
   repeated TestNoPresenceField no_presence_repeated_nested = 3;
+  string no_presence_string = 4;
+  bool no_presence_bool2 = 5;
+  bool no_presence_bool3 = 6;
 }