Fixed comparison of empty repeated/map extensions.

Repeated/map extensions are semantically equivalent to an extension that is not present at all. We had code paths that were treating them differently, which led to incorrect results.

In particular, we were considering `{.repeated_ext = []}` to be different from `{}` when comparing with `upb_Message_IsEqual()`.  This change fixes this bug so that they will be considered equivalent.

PiperOrigin-RevId: 702072912

pkg/BUILD.bazel

@@ -217,6 +217,7 @@ cc_dist_library(
         "//upb/message:compare",
         "//upb/message:copy",
         "//upb/text",
+        "//upb/text:debug",
         "//upb/util:def_to_proto",
         "//upb/util:required_fields",
         "//upb/wire:byte_size",

upb/message/BUILD

@@ -75,11 +75,9 @@ cc_library(
     deps = [
         ":types",
         "//upb:base",
-        "//upb:eps_copy_input_stream",
         "//upb:mem",
         "//upb:mini_table",
         "//upb:port",
-        "//upb:wire_reader",
         "//upb/base:internal",
         "//upb/hash",
         "//upb/mini_table:internal",
@@ -388,6 +386,8 @@ cc_test(
         "//upb:wire_reader",
         "//upb/test:fuzz_util",
         "//upb/test:test_messages_proto3_upb_proto",
+        "//upb/text:debug",
+        "@com_google_absl//absl/strings",
         "@com_google_googletest//:gtest",
         "@com_google_googletest//:gtest_main",
     ],

upb/message/compare.c

@@ -133,20 +133,18 @@ static bool _upb_Message_ExtensionsAreEqual(const upb_Message* msg1,
                                             const upb_Message* msg2,
                                             const upb_MiniTable* m,
                                             int options) {
-  // Must have identical extension counts.
-  if (upb_Message_ExtensionCount(msg1) != upb_Message_ExtensionCount(msg2)) {
-    return false;
-  }
-
   const upb_MiniTableExtension* e;
   upb_MessageValue val1;
 
   // Iterate over all extensions for msg1, and search msg2 for each extension.
+  size_t count1 = 0;
   size_t iter1 = kUpb_Message_ExtensionBegin;
   while (upb_Message_NextExtension(msg1, &e, &val1, &iter1)) {
     const upb_Extension* ext2 = UPB_PRIVATE(_upb_Message_Getext)(msg2, e);
     if (!ext2) return false;
 
+    count1++;
+
     const upb_MessageValue val2 = ext2->data;
     const upb_MiniTableField* f = &e->UPB_PRIVATE(field);
     const upb_MiniTable* subm = upb_MiniTableField_IsSubMessage(f)
@@ -170,6 +168,11 @@ static bool _upb_Message_ExtensionsAreEqual(const upb_Message* msg1,
     }
     if (!eq) return false;
   }
+
+  // Must have identical extension counts (this catches the case where msg2
+  // has extensions that msg1 doesn't).
+  if (count1 != upb_Message_ExtensionCount(msg2)) return false;
+
   return true;
 }
 

upb/message/internal/extension.h

@@ -11,8 +11,12 @@
 #include <stddef.h>
 
 #include "upb/mem/arena.h"
+#include "upb/message/internal/array.h"
+#include "upb/message/internal/map.h"
+#include "upb/message/internal/types.h"
 #include "upb/message/value.h"
 #include "upb/mini_table/extension.h"
+#include "upb/mini_table/internal/field.h"
 
 // Must be last.
 #include "upb/port/def.inc"
@@ -51,6 +55,19 @@ const upb_Extension* UPB_PRIVATE(_upb_Message_Getexts)(
 const upb_Extension* UPB_PRIVATE(_upb_Message_Getext)(
     const struct upb_Message* msg, const upb_MiniTableExtension* ext);
 
+UPB_INLINE bool UPB_PRIVATE(_upb_Extension_IsEmpty)(const upb_Extension* ext) {
+  switch (
+      UPB_PRIVATE(_upb_MiniTableField_Mode)(&ext->ext->UPB_PRIVATE(field))) {
+    case kUpb_FieldMode_Scalar:
+      return false;
+    case kUpb_FieldMode_Array:
+      return upb_Array_Size(ext->data.array_val) == 0;
+    case kUpb_FieldMode_Map:
+      return _upb_Map_Size(ext->data.map_val) == 0;
+  }
+  UPB_UNREACHABLE();
+}
+
 #ifdef __cplusplus
 } /* extern "C" */
 #endif

upb/message/internal/message.h

@@ -141,12 +141,17 @@ UPB_INLINE bool upb_Message_NextExtension(const struct upb_Message* msg,
   size_t count;
   const upb_Extension* exts = UPB_PRIVATE(_upb_Message_Getexts)(msg, &count);
   size_t i = *iter;
-  if (i < count) {
+  while (i++ < count) {
     // Extensions are stored in reverse wire order, so to iterate in wire order,
     // we need to iterate backwards.
-    *out_e = exts[count - 1 - i].ext;
-    *out_v = exts[count - 1 - i].data;
-    *iter = i + 1;
+    const upb_Extension* ext = &exts[count - i];
+
+    // Empty repeated fields or maps semantically don't exist.
+    if (UPB_PRIVATE(_upb_Extension_IsEmpty)(ext)) continue;
+
+    *out_e = ext->ext;
+    *out_v = ext->data;
+    *iter = i;
     return true;
   }
 
@@ -159,11 +164,16 @@ UPB_INLINE bool UPB_PRIVATE(_upb_Message_NextExtensionReverse)(
   size_t count;
   const upb_Extension* exts = UPB_PRIVATE(_upb_Message_Getexts)(msg, &count);
   size_t i = *iter;
-  if (i < count) {
+  while (i++ < count) {
     // Extensions are stored in reverse wire order
-    *out_e = exts[i].ext;
-    *out_v = exts[i].data;
-    *iter = i + 1;
+    const upb_Extension* ext = &exts[i - 1];
+
+    // Empty repeated fields or maps semantically don't exist.
+    if (UPB_PRIVATE(_upb_Extension_IsEmpty)(ext)) continue;
+
+    *out_e = ext->ext;
+    *out_v = ext->data;
+    *iter = i;
     return true;
   }
 

upb/message/message.c

@@ -117,8 +117,13 @@ bool upb_Message_DeleteUnknown(upb_Message* msg, upb_StringView* data,
 }
 
 size_t upb_Message_ExtensionCount(const upb_Message* msg) {
-  size_t count;
-  UPB_PRIVATE(_upb_Message_Getexts)(msg, &count);
+  const upb_MiniTableExtension* e;
+  upb_MessageValue val;
+  size_t iter = kUpb_Message_ExtensionBegin;
+  size_t count = 0;
+  while (upb_Message_NextExtension(msg, &e, &val, &iter)) {
+    count++;
+  }
   return count;
 }
 

upb/message/test.cc

@@ -15,6 +15,7 @@
 
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
+#include "absl/strings/escaping.h"
 #include "google/protobuf/test_messages_proto3.upb.h"
 #include "upb/base/status.hpp"
 #include "upb/base/string_view.h"
@@ -38,6 +39,7 @@
 #include "upb/reflection/def.hpp"
 #include "upb/reflection/message.h"
 #include "upb/test/fuzz_util.h"
+#include "upb/text/debug_string.h"
 #include "upb/wire/decode.h"
 #include "upb/wire/encode.h"
 #include "upb/wire/eps_copy_input_stream.h"