From 292ec137d40c1af75994ff2af8cd75227756ea24 Mon Sep 17 00:00:00 2001 From: Ada Zhang Date: Tue, 23 Jun 2026 16:27:18 -0700 Subject: [PATCH] Introduce non-canonical extensions in upb messages. Minimum tagged pointer and extension APIs are added. PiperOrigin-RevId: 936959066 --- upb/message/copy.c | 24 +++-- upb/message/internal/accessors.h | 20 ++++- upb/message/internal/extension.c | 26 ++++-- upb/message/internal/extension.h | 8 ++ upb/message/internal/message.h | 130 +++++++++++++++++++--------- upb/message/map_sorter.c | 6 +- upb/message/message.c | 21 +++-- upb/message/test.cc | 22 +++++ upb/mini_table/internal/extension.h | 6 ++ upb/reflection/message.c | 5 +- 10 files changed, 197 insertions(+), 71 deletions(-) diff --git a/upb/message/copy.c b/upb/message/copy.c index 8c7f2397e9..0d3a849bae 100644 --- a/upb/message/copy.c +++ b/upb/message/copy.c @@ -245,9 +245,10 @@ upb_Message* _upb_Message_Copy(upb_Message* dst, const upb_Message* src, for (size_t i = 0; i < in->size; i++) { upb_TaggedAuxPtr tagged_ptr = in->aux_data[i]; - if (upb_TaggedAuxPtr_IsExtension(tagged_ptr)) { + if (upb_TaggedAuxPtr_IsCanonicalExtension(tagged_ptr)) { // Clone extension - const upb_Extension* msg_ext = upb_TaggedAuxPtr_Extension(tagged_ptr); + const upb_Extension* msg_ext = + upb_TaggedAuxPtr_CanonicalExtension(tagged_ptr); const upb_MiniTableField* field = &msg_ext->ext->UPB_PRIVATE(field); upb_Extension* dst_ext = UPB_PRIVATE(_upb_Message_GetOrCreateExtension)( dst, msg_ext->ext, arena); @@ -267,9 +268,9 @@ upb_Message* _upb_Message_Copy(upb_Message* dst, const upb_Message* src, } dst_ext->data.array_val = cloned_array; } - } else if (upb_TaggedAuxPtr_IsUnknown(tagged_ptr)) { + } else if (upb_TaggedAuxPtr_IsUnknownStringView(tagged_ptr)) { // Clone unknown - upb_StringView* unknown = upb_TaggedAuxPtr_UnknownData(tagged_ptr); + upb_StringView* unknown = upb_TaggedPtrAux_StringViewRepr(tagged_ptr); // Make a copy into destination arena. if (!UPB_PRIVATE(_upb_Message_AddUnknown)( dst, unknown->data, unknown->size, arena, kUpb_AddUnknown_Copy)) { @@ -316,12 +317,12 @@ bool upb_Message_ShallowCopy(upb_Message* dst, const upb_Message* src, for (size_t i = 0; i < in->size; i++) { upb_TaggedAux aux; switch (upb_TaggedAux_Get(in->aux_data[i], &aux)) { - case kUpb_TaggedAuxType_Extension: { + case kUpb_TaggedAuxType_CanonicalExtension: { const upb_Extension* msg_ext = aux.extension; upb_Extension* dst_ext = upb_Arena_Malloc(arena, sizeof(upb_Extension)); if (!dst_ext) return false; *dst_ext = *msg_ext; - dst_in->aux_data[i] = upb_TaggedAuxPtr_MakeExtension(dst_ext); + dst_in->aux_data[i] = upb_TaggedAuxPtr_MakeCanonicalExtension(dst_ext); break; } case kUpb_TaggedAuxType_Unknown: @@ -329,10 +330,19 @@ bool upb_Message_ShallowCopy(upb_Message* dst, const upb_Message* src, upb_StringView* dst_sv = upb_Arena_Malloc(arena, sizeof(upb_StringView)); if (!dst_sv) return false; - *dst_sv = aux.unknown_data; + *dst_sv = *aux.unknown_data; dst_in->aux_data[i] = upb_TaggedAuxPtr_MakeUnknownDataAliased(dst_sv); break; } + case kUpb_TaggedAuxType_NonCanonicalExtension: { + const upb_Extension* msg_ext = aux.extension; + upb_Extension* dst_ext = upb_Arena_Malloc(arena, sizeof(upb_Extension)); + if (!dst_ext) return false; + *dst_ext = *msg_ext; + dst_in->aux_data[i] = + upb_TaggedAuxPtr_MakeNonCanonicalExtension(dst_ext); + break; + } } } diff --git a/upb/message/internal/accessors.h b/upb/message/internal/accessors.h index dba283148f..a1f1eacd49 100644 --- a/upb/message/internal/accessors.h +++ b/upb/message/internal/accessors.h @@ -143,6 +143,7 @@ UPB_INLINE bool UPB_PRIVATE(_upb_Message_IsInitializedShallow)( return (UPB_PRIVATE(_upb_MiniTable_RequiredMask)(m) & ~bits) == 0; } +// LINT.IfChange(message_raw_fields) UPB_INLINE void* UPB_PRIVATE(_upb_Message_MutableDataPtr)( struct upb_Message* msg, const upb_MiniTableField* f) { return (char*)msg + f->UPB_ONLYBITS(offset); @@ -181,6 +182,7 @@ UPB_INLINE_IF_NOT_GCC void UPB_PRIVATE(_upb_MiniTableField_DataCopy)( } UPB_UNREACHABLE(); } +// LINT.ThenChange(//depot/google3/third_party/upb/bits/golang/message.go:message_raw_fields) UPB_INLINE_IF_NOT_GCC bool UPB_PRIVATE(_upb_MiniTableField_DataEquals)( const upb_MiniTableField* f, const void* a, const void* b) { @@ -331,6 +333,19 @@ UPB_API_INLINE bool upb_Message_SetExtension(struct upb_Message* msg, return true; } +UPB_API_INLINE bool UPB_PRIVATE(_upb_Message_SetNonCanonicalExtension)( + struct upb_Message* msg, const upb_MiniTableExtension* e, const void* val, + upb_Arena* a) { + UPB_ASSERT(!upb_Message_IsFrozen(msg)); + UPB_ASSERT(a); + upb_Extension* ext = + UPB_PRIVATE(_upb_Message_CreateNonCanonicalExtension)(msg, e, a); + if (!ext) return false; + UPB_PRIVATE(_upb_MiniTableField_DataCopy) + (&e->UPB_PRIVATE(field), &ext->data, val); + return true; +} + // Sets the value of the given field in the given msg. The return value is true // if the operation completed successfully, or false if memory allocation // failed. @@ -882,8 +897,9 @@ UPB_API_INLINE void upb_Message_ClearExtension( if (!in) return; for (size_t i = 0; i < in->size; i++) { upb_TaggedAuxPtr tagged_ptr = in->aux_data[i]; - if (upb_TaggedAuxPtr_IsExtension(tagged_ptr)) { - const upb_Extension* ext = upb_TaggedAuxPtr_Extension(tagged_ptr); + if (upb_TaggedAuxPtr_IsCanonicalExtension(tagged_ptr)) { + const upb_Extension* ext = + upb_TaggedAuxPtr_CanonicalExtension(tagged_ptr); if (ext->ext == e) { in->aux_data[i] = upb_TaggedAuxPtr_Null(); return; diff --git a/upb/message/internal/extension.c b/upb/message/internal/extension.c index ca8a5865fd..4c207d56ec 100644 --- a/upb/message/internal/extension.c +++ b/upb/message/internal/extension.c @@ -26,8 +26,9 @@ const upb_Extension* UPB_PRIVATE(_upb_Message_Getext)( for (size_t i = 0; i < in->size; i++) { upb_TaggedAuxPtr tagged_ptr = in->aux_data[i]; - if (upb_TaggedAuxPtr_IsExtension(tagged_ptr)) { - const upb_Extension* ext = upb_TaggedAuxPtr_Extension(tagged_ptr); + if (upb_TaggedAuxPtr_IsCanonicalExtension(tagged_ptr)) { + const upb_Extension* ext = + upb_TaggedAuxPtr_CanonicalExtension(tagged_ptr); if (ext->ext == e) { return ext; } @@ -37,8 +38,9 @@ const upb_Extension* UPB_PRIVATE(_upb_Message_Getext)( return NULL; } -upb_Extension* UPB_PRIVATE(_upb_Message_GetOrCreateExtension)( - struct upb_Message* msg, const upb_MiniTableExtension* e, upb_Arena* a) { +UPB_INLINE upb_Extension* _upb_Message_GetOrCreateExtensionInternal( + struct upb_Message* msg, const upb_MiniTableExtension* e, upb_Arena* a, + bool canonical) { UPB_ASSERT(!upb_Message_IsFrozen(msg)); upb_Extension* ext = (upb_Extension*)UPB_PRIVATE(_upb_Message_Getext)(msg, e); if (ext) return ext; @@ -49,6 +51,20 @@ upb_Extension* UPB_PRIVATE(_upb_Message_GetOrCreateExtension)( if (!ext) return NULL; memset(ext, 0, sizeof(upb_Extension)); ext->ext = e; - in->aux_data[in->size++] = upb_TaggedAuxPtr_MakeExtension(ext); + in->aux_data[in->size++] = + canonical ? upb_TaggedAuxPtr_MakeCanonicalExtension(ext) + : upb_TaggedAuxPtr_MakeNonCanonicalExtension(ext); return ext; } + +upb_Extension* UPB_PRIVATE(_upb_Message_GetOrCreateExtension)( + struct upb_Message* msg, const upb_MiniTableExtension* e, upb_Arena* a) { + return _upb_Message_GetOrCreateExtensionInternal(msg, e, a, + /*canonical=*/true); +} + +upb_Extension* UPB_PRIVATE(_upb_Message_CreateNonCanonicalExtension)( + struct upb_Message* msg, const upb_MiniTableExtension* e, upb_Arena* a) { + return _upb_Message_GetOrCreateExtensionInternal(msg, e, a, + /*canonical=*/false); +} diff --git a/upb/message/internal/extension.h b/upb/message/internal/extension.h index d0dc11cf8c..0c551aad23 100644 --- a/upb/message/internal/extension.h +++ b/upb/message/internal/extension.h @@ -45,6 +45,14 @@ UPB_NODISCARD upb_Extension* UPB_PRIVATE(_upb_Message_GetOrCreateExtension)( struct upb_Message* msg, const upb_MiniTableExtension* ext, upb_Arena* arena); +// Adds the given non-canonical extension data to the given message. +// |ext| is copied into the message instance. +// This logically replaces any previously-added extension with this number. +UPB_NODISCARD upb_Extension* UPB_PRIVATE( + _upb_Message_CreateNonCanonicalExtension)(struct upb_Message* msg, + const upb_MiniTableExtension* ext, + upb_Arena* arena); + // Returns an extension for a message with a given mini table, // or NULL if no extension exists with this mini table. const upb_Extension* UPB_PRIVATE(_upb_Message_Getext)( diff --git a/upb/message/internal/message.h b/upb/message/internal/message.h index 59050be0b5..fec705cb69 100644 --- a/upb/message/internal/message.h +++ b/upb/message/internal/message.h @@ -42,15 +42,27 @@ extern const double kUpb_NaN; // Internal members of a upb_Message that track unknown fields and/or // extensions. We can change this without breaking binary compatibility. +// LINT.IfChange(tagged_aux_type) typedef struct upb_TaggedAuxPtr { - // Two lowest bits form a tag: - // 00 - non-aliased unknown data - // 10 - aliased unknown data - // 01 - extension + // Three lowest bits form a tag: + // 000 - non-aliased unknown data (upb_StringView*) + // 100 - aliased unknown data (upb_StringView*) + // 001 - non-canonical extension (upb_Extension*) + // 011 - canonical extension (upb_Extension*) // - // The main semantic difference between aliased and non-aliased unknown data - // is that non-aliased unknown data can be assumed to have the following - // layout: + // Bit 0 (lowest bit): Represents the data format in memory (1 for parsed + // form, 0 for serialized form). + // Bit 1 (middle bit): Represents whether the data is semantically known or + // not (1 for known, 0 for unknown). + // Bit 2 (highest bit): Aliased/Non-aliased (1 for aliased, 0 for + // non-aliased). + // + // Following this tag structure, we can later use tag `010` for lazy + // extensions. + // + // The main semantic difference between aliased and non-aliased + // unknown data is that non-aliased unknown data can be assumed to have the + // following layout: // // [upb_StringView] [data] // @@ -64,55 +76,75 @@ typedef struct upb_TaggedAuxPtr { // For aliased unknown data, this layout is _not_ guaranteed, since the // pointer to the StringView can be anywhere in the allocation, and the // StringView may point to non-data memory. + // + // For a non-canonical extension, its schema is known but not + // the one expected by the message, so it should be treated like an unknown + // field, but is stored as an extension to lazily defer serialization. uintptr_t ptr; } upb_TaggedAuxPtr; -UPB_INLINE bool upb_TaggedAuxPtr_IsExtension(upb_TaggedAuxPtr ptr) { - return ptr.ptr & 1; +// If this returns true, then the entry is semantically known (but may be in +// either parsed or unparsed form). +UPB_INLINE bool upb_TaggedAuxPtr_IsSemanticallyKnown(upb_TaggedAuxPtr ptr) { + return (ptr.ptr & 0x2) != 0; } -UPB_INLINE bool upb_TaggedAuxPtr_IsUnknown(upb_TaggedAuxPtr ptr) { - return (ptr.ptr != 0) && ((ptr.ptr & 1) == 0); +UPB_INLINE bool upb_TaggedAuxPtr_IsCanonicalExtension(upb_TaggedAuxPtr ptr) { + return (ptr.ptr & 3) == 3; +} + +UPB_INLINE bool upb_TaggedAuxPtr_IsNonCanonicalExtension(upb_TaggedAuxPtr ptr) { + return (ptr.ptr & 3) == 1; +} + +// Returns true if the entry is aliased/non-aliased unknown data. +UPB_INLINE bool upb_TaggedAuxPtr_IsUnknownStringView(upb_TaggedAuxPtr ptr) { + return (ptr.ptr != 0) && ((ptr.ptr & 3) == 0); } UPB_INLINE bool upb_TaggedAuxPtr_IsUnknownAliased(upb_TaggedAuxPtr ptr) { - return (ptr.ptr != 0) && ((ptr.ptr & 2) == 2); + return (ptr.ptr != 0) && ((ptr.ptr & 5) == 4); } -UPB_INLINE upb_Extension* upb_TaggedAuxPtr_Extension(upb_TaggedAuxPtr ptr) { - UPB_ASSERT(upb_TaggedAuxPtr_IsExtension(ptr)); - return (upb_Extension*)(ptr.ptr & ~3ULL); +UPB_INLINE upb_Extension* upb_TaggedAuxPtr_CanonicalExtension( + upb_TaggedAuxPtr ptr) { + UPB_ASSERT(upb_TaggedAuxPtr_IsCanonicalExtension(ptr)); + return (upb_Extension*)(ptr.ptr & ~7ULL); } -UPB_INLINE upb_StringView* upb_TaggedAuxPtr_UnknownData(upb_TaggedAuxPtr ptr) { - UPB_ASSERT(!upb_TaggedAuxPtr_IsExtension(ptr)); - return (upb_StringView*)(ptr.ptr & ~3ULL); +UPB_INLINE upb_Extension* upb_TaggedAuxPtr_NonCanonicalExtension( + upb_TaggedAuxPtr ptr) { + UPB_ASSERT(upb_TaggedAuxPtr_IsNonCanonicalExtension(ptr)); + return (upb_Extension*)(ptr.ptr & ~7ULL); } +// Returns a pointer to the aliased or unaliased unknown upb_StringView* data. +UPB_INLINE upb_StringView* upb_TaggedPtrAux_StringViewRepr( + upb_TaggedAuxPtr ptr) { + UPB_ASSERT(upb_TaggedAuxPtr_IsUnknownStringView(ptr)); + return (upb_StringView*)(ptr.ptr & ~7ULL); +} + +// LINT.ThenChange(//depot/google3/third_party/upb/bits/golang/message.go:tagged_aux_type) + typedef enum { - kUpb_TaggedAuxType_Extension, - kUpb_TaggedAuxType_Unknown, - kUpb_TaggedAuxType_AliasedUnknown + kUpb_TaggedAuxType_Unknown = 0, // tag 000 + kUpb_TaggedAuxType_NonCanonicalExtension = 1, // tag 001 + kUpb_TaggedAuxType_CanonicalExtension = 3, // tag 011 + kUpb_TaggedAuxType_AliasedUnknown = 4 // tag 100 } upb_TaggedAuxType; typedef union { upb_Extension* extension; - upb_StringView unknown_data; + const upb_StringView* unknown_data; } upb_TaggedAux; UPB_INLINE upb_TaggedAuxType upb_TaggedAux_Get(upb_TaggedAuxPtr ptr, upb_TaggedAux* data) { - if (upb_TaggedAuxPtr_IsExtension(ptr)) { - data->extension = upb_TaggedAuxPtr_Extension(ptr); - return kUpb_TaggedAuxType_Extension; - } else if (upb_TaggedAuxPtr_IsUnknownAliased(ptr)) { - data->unknown_data = *upb_TaggedAuxPtr_UnknownData(ptr); - return kUpb_TaggedAuxType_AliasedUnknown; - } else { - UPB_ASSERT(upb_TaggedAuxPtr_IsUnknown(ptr)); - data->unknown_data = *upb_TaggedAuxPtr_UnknownData(ptr); - return kUpb_TaggedAuxType_Unknown; - } + uintptr_t untagged = ptr.ptr & ~7ULL; + UPB_ASSERT((untagged & 7) == 0); + memcpy(data, &untagged, sizeof(*data)); + return (upb_TaggedAuxType)(ptr.ptr & 7); } UPB_INLINE upb_TaggedAuxPtr upb_TaggedAuxPtr_Null(void) { @@ -122,7 +154,16 @@ UPB_INLINE upb_TaggedAuxPtr upb_TaggedAuxPtr_Null(void) { } UPB_INLINE upb_TaggedAuxPtr -upb_TaggedAuxPtr_MakeExtension(const upb_Extension* e) { +upb_TaggedAuxPtr_MakeCanonicalExtension(const upb_Extension* e) { + UPB_ASSERT(((uintptr_t)e & 7) == 0); + upb_TaggedAuxPtr ptr; + ptr.ptr = (uintptr_t)e | 3; + return ptr; +} + +UPB_INLINE upb_TaggedAuxPtr +upb_TaggedAuxPtr_MakeNonCanonicalExtension(const upb_Extension* e) { + UPB_ASSERT(((uintptr_t)e & 7) == 0); upb_TaggedAuxPtr ptr; ptr.ptr = (uintptr_t)e | 1; return ptr; @@ -133,6 +174,7 @@ upb_TaggedAuxPtr_MakeExtension(const upb_Extension* e) { // view. UPB_INLINE upb_TaggedAuxPtr upb_TaggedAuxPtr_MakeUnknownData(const upb_StringView* sv) { + UPB_ASSERT(((uintptr_t)sv & 7) == 0); upb_TaggedAuxPtr ptr; ptr.ptr = (uintptr_t)sv; return ptr; @@ -142,8 +184,9 @@ upb_TaggedAuxPtr_MakeUnknownData(const upb_StringView* sv) { // the data it points to. UPB_INLINE upb_TaggedAuxPtr upb_TaggedAuxPtr_MakeUnknownDataAliased(const upb_StringView* sv) { + UPB_ASSERT(((uintptr_t)sv & 7) == 0); upb_TaggedAuxPtr ptr; - ptr.ptr = (uintptr_t)sv | 2; + ptr.ptr = (uintptr_t)sv | 4; return ptr; } @@ -265,8 +308,8 @@ UPB_NODISCARD UPB_INLINE bool UPB_PRIVATE( upb_Message_Internal* in = UPB_PRIVATE(_upb_Message_GetInternal)(msg); if (in && in->size) { upb_TaggedAuxPtr ptr = in->aux_data[in->size - 1]; - if (upb_TaggedAuxPtr_IsUnknown(ptr)) { - upb_StringView* existing = upb_TaggedAuxPtr_UnknownData(ptr); + if (upb_TaggedAuxPtr_IsUnknownStringView(ptr)) { + upb_StringView* existing = upb_TaggedPtrAux_StringViewRepr(ptr); // Fast path if the field we're adding is immediately after the last // added unknown field. // @@ -329,8 +372,8 @@ UPB_INLINE bool upb_Message_NextUnknown(const struct upb_Message* msg, if (in) { while (i < in->size) { upb_TaggedAuxPtr tagged_ptr = in->aux_data[i++]; - if (upb_TaggedAuxPtr_IsUnknown(tagged_ptr)) { - *data = *upb_TaggedAuxPtr_UnknownData(tagged_ptr); + if (upb_TaggedAuxPtr_IsUnknownStringView(tagged_ptr)) { + *data = *upb_TaggedPtrAux_StringViewRepr(tagged_ptr); *iter = i; return true; } @@ -357,8 +400,9 @@ UPB_INLINE bool upb_Message_NextExtension(const struct upb_Message* msg, if (in) { while (i < in->size) { upb_TaggedAuxPtr tagged_ptr = in->aux_data[i++]; - if (upb_TaggedAuxPtr_IsExtension(tagged_ptr)) { - const upb_Extension* ext = upb_TaggedAuxPtr_Extension(tagged_ptr); + if (upb_TaggedAuxPtr_IsCanonicalExtension(tagged_ptr)) { + const upb_Extension* ext = + upb_TaggedAuxPtr_CanonicalExtension(tagged_ptr); // Empty repeated fields or maps semantically don't exist. if (UPB_PRIVATE(_upb_Extension_IsEmpty)(ext)) continue; @@ -385,10 +429,10 @@ UPB_INLINE bool UPB_PRIVATE(_upb_Message_NextExtensionReverse)( while (i < size) { upb_TaggedAuxPtr tagged_ptr = in->aux_data[size - 1 - i]; i++; - if (!upb_TaggedAuxPtr_IsExtension(tagged_ptr)) { + if (!upb_TaggedAuxPtr_IsCanonicalExtension(tagged_ptr)) { continue; } - const upb_Extension* ext = upb_TaggedAuxPtr_Extension(tagged_ptr); + const upb_Extension* ext = upb_TaggedAuxPtr_CanonicalExtension(tagged_ptr); // Empty repeated fields or maps semantically don't exist. if (UPB_PRIVATE(_upb_Extension_IsEmpty)(ext)) continue; diff --git a/upb/message/map_sorter.c b/upb/message/map_sorter.c index 968b583af6..b2b8b153e7 100644 --- a/upb/message/map_sorter.c +++ b/upb/message/map_sorter.c @@ -167,7 +167,7 @@ bool _upb_mapsorter_pushexts(_upb_mapsorter* s, const upb_Message_Internal* in, _upb_sortedmap* sorted) { size_t count = 0; for (size_t i = 0; i < in->size; i++) { - count += upb_TaggedAuxPtr_IsExtension(in->aux_data[i]); + count += upb_TaggedAuxPtr_IsCanonicalExtension(in->aux_data[i]); } if (!_upb_mapsorter_resize(s, sorted, count)) return false; if (count == 0) return true; @@ -175,8 +175,8 @@ bool _upb_mapsorter_pushexts(_upb_mapsorter* s, const upb_Message_Internal* in, (const upb_Extension**)&s->entries[sorted->start]; for (size_t i = 0; i < in->size; i++) { upb_TaggedAuxPtr tagged_ptr = in->aux_data[i]; - if (upb_TaggedAuxPtr_IsExtension(tagged_ptr)) { - *entry++ = upb_TaggedAuxPtr_Extension(tagged_ptr); + if (upb_TaggedAuxPtr_IsCanonicalExtension(tagged_ptr)) { + *entry++ = upb_TaggedAuxPtr_CanonicalExtension(tagged_ptr); } } qsort(&s->entries[sorted->start], count, sizeof(*s->entries), diff --git a/upb/message/message.c b/upb/message/message.c index e013c04fc8..273dc4066f 100644 --- a/upb/message/message.c +++ b/upb/message/message.c @@ -46,8 +46,8 @@ UPB_NOINLINE bool UPB_PRIVATE(_upb_Message_AddUnknownSlowPath)(upb_Message* msg, // this one if (!alias && in && in->size) { upb_TaggedAuxPtr ptr = in->aux_data[in->size - 1]; - if (upb_TaggedAuxPtr_IsUnknown(ptr)) { - upb_StringView* existing = upb_TaggedAuxPtr_UnknownData(ptr); + if (upb_TaggedAuxPtr_IsUnknownStringView(ptr)) { + upb_StringView* existing = upb_TaggedPtrAux_StringViewRepr(ptr); if (!upb_TaggedAuxPtr_IsUnknownAliased(ptr)) { // If part of the existing field was deleted at the beginning, we can // reconstruct it by comparing the address of the end with the address @@ -114,8 +114,8 @@ bool UPB_PRIVATE(_upb_Message_AddUnknownV)(struct upb_Message* msg, upb_Message_Internal* in = UPB_PRIVATE(_upb_Message_GetInternal)(msg); if (in && in->size) { upb_TaggedAuxPtr ptr = in->aux_data[in->size - 1]; - if (upb_TaggedAuxPtr_IsUnknown(ptr)) { - upb_StringView* existing = upb_TaggedAuxPtr_UnknownData(ptr); + if (upb_TaggedAuxPtr_IsUnknownStringView(ptr)) { + upb_StringView* existing = upb_TaggedPtrAux_StringViewRepr(ptr); if (!upb_TaggedAuxPtr_IsUnknownAliased(ptr)) { size_t prev_alloc_size = (existing->data + existing->size) - (char*)existing; @@ -164,7 +164,7 @@ void _upb_Message_DiscardUnknown_shallow(upb_Message* msg) { uint32_t size = 0; for (uint32_t i = 0; i < in->size; i++) { upb_TaggedAuxPtr tagged_ptr = in->aux_data[i]; - if (upb_TaggedAuxPtr_IsExtension(tagged_ptr)) { + if (upb_TaggedAuxPtr_IsSemanticallyKnown(tagged_ptr)) { in->aux_data[size++] = tagged_ptr; } } @@ -181,8 +181,8 @@ upb_Message_DeleteUnknownStatus upb_Message_DeleteUnknown(upb_Message* msg, UPB_ASSERT(in); UPB_ASSERT(*iter <= in->size); upb_TaggedAuxPtr unknown_ptr = in->aux_data[*iter - 1]; - UPB_ASSERT(upb_TaggedAuxPtr_IsUnknown(unknown_ptr)); - upb_StringView* unknown = upb_TaggedAuxPtr_UnknownData(unknown_ptr); + UPB_ASSERT(upb_TaggedAuxPtr_IsUnknownStringView(unknown_ptr)); + upb_StringView* unknown = upb_TaggedPtrAux_StringViewRepr(unknown_ptr); if (unknown->data == data->data && unknown->size == data->size) { // Remove whole field in->aux_data[*iter - 1] = upb_TaggedAuxPtr_Null(); @@ -286,10 +286,13 @@ void upb_Message_Freeze(upb_Message* msg, const upb_MiniTable* m) { uint32_t size = in ? in->size : 0; for (size_t i = 0; i < size; i++) { upb_TaggedAuxPtr tagged_ptr = in->aux_data[i]; - if (!upb_TaggedAuxPtr_IsExtension(tagged_ptr)) { + upb_TaggedAux aux; + upb_TaggedAuxType type = upb_TaggedAux_Get(tagged_ptr, &aux); + if (type != kUpb_TaggedAuxType_CanonicalExtension && + type != kUpb_TaggedAuxType_NonCanonicalExtension) { continue; } - const upb_Extension* ext = upb_TaggedAuxPtr_Extension(tagged_ptr); + const upb_Extension* ext = aux.extension; const upb_MiniTableExtension* e = ext->ext; const upb_MiniTableField* f = &e->UPB_PRIVATE(field); const upb_MiniTable* m2 = upb_MiniTableExtension_GetSubMessage(e); diff --git a/upb/message/test.cc b/upb/message/test.cc index 5bb5abf3d4..74eab57341 100644 --- a/upb/message/test.cc +++ b/upb/message/test.cc @@ -34,6 +34,7 @@ #include "upb/message/accessors.h" #include "upb/message/array.h" #include "upb/message/compare.h" +#include "upb/message/internal/accessors.h" #include "upb/message/map.h" #include "upb/message/message.h" #include "upb/message/test.upb.h" @@ -877,6 +878,27 @@ TEST(MessageTest, Freeze) { ASSERT_TRUE(upb_Map_IsFrozen(map)); ASSERT_TRUE(upb_Message_IsFrozen(UPB_UPCAST(nest))); } + { + upb_test_TestExtensions* ext_msg = upb_test_TestExtensions_new(arena.ptr()); + upb_Message* msg = UPB_UPCAST(ext_msg); + protobuf_test_messages_proto3_TestAllTypesProto3* submsg = + protobuf_test_messages_proto3_TestAllTypesProto3_new(arena.ptr()); + upb_Message* submsg_upcast = UPB_UPCAST(submsg); + + ASSERT_FALSE(upb_Message_IsFrozen(msg)); + ASSERT_FALSE(upb_Message_IsFrozen(submsg_upcast)); + + // Set a non-canonical extension containing a sub-message. + bool set_ext_ok = UPB_PRIVATE(_upb_Message_SetNonCanonicalExtension)( + msg, upb_test_optional_msg_ext_ext, &submsg, arena.ptr()); + ASSERT_TRUE(set_ext_ok); + + // Freezing the parent message should recursively freeze the non-canonical + // extension's sub-message. + upb_Message_Freeze(msg, &upb_0test__TestExtensions_msg_init); + ASSERT_TRUE(upb_Message_IsFrozen(msg)); + ASSERT_TRUE(upb_Message_IsFrozen(submsg_upcast)); + } } /* Tests some somewhat tricky math used in size calculations while encoding */ diff --git a/upb/mini_table/internal/extension.h b/upb/mini_table/internal/extension.h index c0cb8c484e..6b6d03983c 100644 --- a/upb/mini_table/internal/extension.h +++ b/upb/mini_table/internal/extension.h @@ -23,6 +23,12 @@ struct upb_MiniTableExtension { struct upb_MiniTableField UPB_PRIVATE(field); union upb_MiniTableSub UPB_PRIVATE(sub); // NULL unless submsg or proto2 enum + + // A known extendee schema for a canonical extension. For a non-canonical + // extension, it's typically converted from a canonical extension via the + // upb_Message_Convert() API, but is not registered on the extension + // registry of the target message when it gets converted. In this case, the + // `extendee` info is present on the source message before conversion. const struct upb_MiniTable* UPB_PRIVATE(extendee); }; diff --git a/upb/reflection/message.c b/upb/reflection/message.c index 5a29fabe4c..0b326dd209 100644 --- a/upb/reflection/message.c +++ b/upb/reflection/message.c @@ -177,8 +177,9 @@ bool upb_Message_Next(const upb_Message* msg, const upb_MessageDef* m, for (; (i - n) < in->size; i++) { upb_TaggedAuxPtr tagged_ptr = in->aux_data[i - n]; - if (upb_TaggedAuxPtr_IsExtension(tagged_ptr)) { - const upb_Extension* ext = upb_TaggedAuxPtr_Extension(tagged_ptr); + if (upb_TaggedAuxPtr_IsCanonicalExtension(tagged_ptr)) { + const upb_Extension* ext = + upb_TaggedAuxPtr_CanonicalExtension(tagged_ptr); memcpy(out_val, &ext->data, sizeof(*out_val)); *out_f = upb_DefPool_FindExtensionByMiniTable(ext_pool, ext->ext); *iter = i;