diff --git a/pkg/BUILD.bazel b/pkg/BUILD.bazel index 792f11c155..49cfbd33fd 100644 --- a/pkg/BUILD.bazel +++ b/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", diff --git a/upb/message/BUILD b/upb/message/BUILD index 0089596af5..de904bd61e 100644 --- a/upb/message/BUILD +++ b/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", ], diff --git a/upb/message/compare.c b/upb/message/compare.c index 6963ea5bdd..5266e0c830 100644 --- a/upb/message/compare.c +++ b/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; } diff --git a/upb/message/internal/extension.h b/upb/message/internal/extension.h index 13f5b77d2f..0cfa1a9a62 100644 --- a/upb/message/internal/extension.h +++ b/upb/message/internal/extension.h @@ -11,8 +11,12 @@ #include #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 diff --git a/upb/message/internal/message.h b/upb/message/internal/message.h index 67fca81808..681c30fa5c 100644 --- a/upb/message/internal/message.h +++ b/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; } diff --git a/upb/message/message.c b/upb/message/message.c index 14b7eeb5ba..117b014fe4 100644 --- a/upb/message/message.c +++ b/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; } diff --git a/upb/message/test.cc b/upb/message/test.cc index 041a82d374..ed46c8c56e 100644 --- a/upb/message/test.cc +++ b/upb/message/test.cc @@ -15,6 +15,7 @@ #include #include +#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"