From 48ad4bbd0f0a795723ff9713c1faea2460920c54 Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Mon, 26 Dec 2022 17:32:44 -0800 Subject: [PATCH] Unified accessor API for repeated field getters and setters. This CL eliminates the last remaining callers of GetFieldOffset(), therefore opening the door to a more principled bootstrapping process. PiperOrigin-RevId: 497864910 --- .github/workflows/bazel_tests.yml | 6 +- protos/protos_internal_test.cc | 2 +- upb/collections/array.c | 59 ++++----------- upb/collections/array_internal.h | 122 ++++++------------------------ upb/message/accessors.c | 32 -------- upb/message/accessors.h | 94 +++++++++++++++++++---- upb/mini_table/field_internal.h | 16 ++++ upbc/file_layout.h | 17 ----- upbc/protoc-gen-upb.cc | 87 ++++++++++----------- 9 files changed, 181 insertions(+), 254 deletions(-) diff --git a/.github/workflows/bazel_tests.yml b/.github/workflows/bazel_tests.yml index efe6fe5e69..b0affa1859 100644 --- a/.github/workflows/bazel_tests.yml +++ b/.github/workflows/bazel_tests.yml @@ -24,7 +24,7 @@ jobs: include: - { NAME: "Fastbuild", BAZEL: bazel, CC: clang, os: ubuntu-20.04, flags: "" } - { NAME: "Optmized", BAZEL: bazel, CC: clang, os: ubuntu-20.04, flags: "-c opt" } # Some warnings only fire with -c opt - - { NAME: "GCC Optimized", BAZEL: bazel, CC: gcc, os: ubuntu-20.04, flags: "-c opt" } + - { NAME: "GCC Optimized", BAZEL: bazel, CC: gcc-12, os: ubuntu-22.04, flags: "-c opt" } - { NAME: "FastTable", BAZEL: bazel, CC: clang, os: ubuntu-20.04, flags: "--//:fasttable_enabled=true -- -cmake:test_generated_files" } - { NAME: "ASAN", BAZEL: bazel, CC: clang, os: ubuntu-20.04, flags: "--config=asan -c dbg -- -benchmarks:benchmark -python/..." } - { NAME: "UBSAN", BAZEL: bazel, CC: clang, os: ubuntu-20.04, flags: "--config=ubsan -c dbg -- -benchmarks:benchmark -python/... -lua/...", install: "libunwind-dev" } @@ -53,6 +53,10 @@ jobs: wget -O $FILENAME https://github.com/bazelbuild/bazel/releases/download/$VERSION/${{ matrix.BAZEL }} chmod a+x $FILENAME if: ${{ matrix.BAZEL != 'bazel' }} + - name: Check tool versions + run: | + ${{ matrix.CC }} --version + ${{ matrix.BAZEL }} --version - name: Set up Bazel read/write caching run: echo "BAZEL_CACHE_AUTH=--google_default_credentials" >> $GITHUB_ENV if: ${{ github.event.pull_request.head.repo.full_name == 'protocolbuffers/upb' }} diff --git a/protos/protos_internal_test.cc b/protos/protos_internal_test.cc index 835601c549..99bfaa2402 100644 --- a/protos/protos_internal_test.cc +++ b/protos/protos_internal_test.cc @@ -40,9 +40,9 @@ TEST(CppGeneratedCode, InternalMoveMessage) { upb_Arena* source_arena = upb_Arena_New(); protos_generator_test_TestModel* message = protos_generator_test_TestModel_new(source_arena); + ASSERT_NE(message, nullptr); protos_generator_test_TestModel_set_int_value_with_default(message, 123); - EXPECT_NE(message, nullptr); // Move ownership. TestModel model = protos::internal::MoveMessage(message, source_arena); diff --git a/upb/collections/array.c b/upb/collections/array.c index a957f26ab8..fdb1bd206c 100644 --- a/upb/collections/array.c +++ b/upb/collections/array.c @@ -32,23 +32,22 @@ // Must be last. #include "upb/port/def.inc" -static const char _upb_CTypeo_sizelg2[12] = { - 0, - 0, /* kUpb_CType_Bool */ - 2, /* kUpb_CType_Float */ - 2, /* kUpb_CType_Int32 */ - 2, /* kUpb_CType_UInt32 */ - 2, /* kUpb_CType_Enum */ - UPB_SIZE(2, 3), /* kUpb_CType_Message */ - 3, /* kUpb_CType_Double */ - 3, /* kUpb_CType_Int64 */ - 3, /* kUpb_CType_UInt64 */ - UPB_SIZE(3, 4), /* kUpb_CType_String */ - UPB_SIZE(3, 4), /* kUpb_CType_Bytes */ +const char _upb_Array_CTypeSizeLg2Table[] = { + [kUpb_CType_Bool] = 0, + [kUpb_CType_Float] = 2, + [kUpb_CType_Int32] = 2, + [kUpb_CType_UInt32] = 2, + [kUpb_CType_Enum] = 2, + [kUpb_CType_Message] = UPB_SIZE(2, 3), + [kUpb_CType_Double] = 3, + [kUpb_CType_Int64] = 3, + [kUpb_CType_UInt64] = 3, + [kUpb_CType_String] = UPB_SIZE(3, 4), + [kUpb_CType_Bytes] = UPB_SIZE(3, 4), }; upb_Array* upb_Array_New(upb_Arena* a, upb_CType type) { - return _upb_Array_New(a, 4, _upb_CTypeo_sizelg2[type]); + return _upb_Array_New(a, 4, _upb_Array_CTypeSizeLg2(type)); } size_t upb_Array_Size(const upb_Array* arr) { return arr->size; } @@ -142,35 +141,3 @@ bool _upb_array_realloc(upb_Array* arr, size_t min_capacity, upb_Arena* arena) { arr->capacity = new_capacity; return true; } - -static upb_Array* getorcreate_array(upb_Array** arr_ptr, int elem_size_lg2, - upb_Arena* arena) { - upb_Array* arr = *arr_ptr; - if (!arr) { - arr = _upb_Array_New(arena, 4, elem_size_lg2); - if (!arr) return NULL; - *arr_ptr = arr; - } - return arr; -} - -void* _upb_Array_Resize_fallback(upb_Array** arr_ptr, size_t size, - int elem_size_lg2, upb_Arena* arena) { - upb_Array* arr = getorcreate_array(arr_ptr, elem_size_lg2, arena); - return arr && _upb_Array_ResizeUninitialized(arr, size, arena) - ? _upb_array_ptr(arr) - : NULL; -} - -bool _upb_Array_Append_fallback(upb_Array** arr_ptr, const void* value, - int elem_size_lg2, upb_Arena* arena) { - upb_Array* arr = getorcreate_array(arr_ptr, elem_size_lg2, arena); - if (!arr) return false; - - size_t elems = arr->size; - if (!_upb_Array_ResizeUninitialized(arr, elems + 1, arena)) return false; - - char* data = _upb_array_ptr(arr); - memcpy(data + (elems << elem_size_lg2), value, 1 << elem_size_lg2); - return true; -} diff --git a/upb/collections/array_internal.h b/upb/collections/array_internal.h index e1a4cefde2..0c126eb642 100644 --- a/upb/collections/array_internal.h +++ b/upb/collections/array_internal.h @@ -48,8 +48,14 @@ struct upb_Array { }; // LINT.ThenChange(GoogleInternalName1) +UPB_INLINE size_t _upb_Array_ElementSizeLg2(const upb_Array* arr) { + size_t ret = arr->data & 7; + UPB_ASSERT(ret <= 4); + return ret; +} + UPB_INLINE const void* _upb_array_constptr(const upb_Array* arr) { - UPB_ASSERT((arr->data & 7) <= 4); + _upb_Array_ElementSizeLg2(arr); // Check assertion. return (void*)(arr->data & ~(uintptr_t)7); } @@ -68,8 +74,15 @@ UPB_INLINE uintptr_t _upb_tag_arrptr(void* ptr, int elem_size_lg2) { return (uintptr_t)ptr | (unsigned)elem_size_lg2; } +extern const char _upb_Array_CTypeSizeLg2Table[]; + +UPB_INLINE size_t _upb_Array_CTypeSizeLg2(upb_CType ctype) { + return _upb_Array_CTypeSizeLg2Table[ctype]; +} + UPB_INLINE upb_Array* _upb_Array_New(upb_Arena* a, size_t init_capacity, int elem_size_lg2) { + UPB_ASSERT(elem_size_lg2 <= 4); const size_t arr_size = UPB_ALIGN_UP(sizeof(upb_Array), UPB_MALLOC_ALIGN); const size_t bytes = arr_size + (init_capacity << elem_size_lg2); upb_Array* arr = (upb_Array*)upb_Arena_Malloc(a, bytes); @@ -83,12 +96,6 @@ UPB_INLINE upb_Array* _upb_Array_New(upb_Arena* a, size_t init_capacity, // Resizes the capacity of the array to be at least min_size. bool _upb_array_realloc(upb_Array* arr, size_t min_size, upb_Arena* arena); -// Fallback functions for when the accessors require a resize. -void* _upb_Array_Resize_fallback(upb_Array** arr_ptr, size_t size, - int elem_size_lg2, upb_Arena* arena); -bool _upb_Array_Append_fallback(upb_Array** arr_ptr, const void* value, - int elem_size_lg2, upb_Arena* arena); - UPB_INLINE bool _upb_array_reserve(upb_Array* arr, size_t size, upb_Arena* arena) { if (arr->capacity < size) return _upb_array_realloc(arr, size, arena); @@ -103,98 +110,19 @@ UPB_INLINE bool _upb_Array_ResizeUninitialized(upb_Array* arr, size_t size, return true; } -UPB_INLINE void _upb_array_detach(const void* msg, size_t ofs) { - *UPB_PTR_AT(msg, ofs, upb_Array*) = NULL; -} - -UPB_INLINE const void* _upb_array_accessor(const void* msg, size_t ofs, - size_t* size) { - const upb_Array* arr = *UPB_PTR_AT(msg, ofs, const upb_Array*); - if (arr) { - if (size) *size = arr->size; - return _upb_array_constptr(arr); - } else { - if (size) *size = 0; - return NULL; - } -} - -UPB_INLINE void* _upb_array_mutable_accessor(void* msg, size_t ofs, - size_t* size) { - upb_Array* arr = *UPB_PTR_AT(msg, ofs, upb_Array*); - if (arr) { - if (size) *size = arr->size; - return _upb_array_ptr(arr); - } else { - if (size) *size = 0; - return NULL; - } +// This function is intended for situations where elem_size is compile-time +// constant or a known expression of the form (1 << lg2), so that the expression +// i*elem_size does not result in an actual multiplication. +UPB_INLINE void _upb_Array_Set(upb_Array* arr, size_t i, const void* data, + size_t elem_size) { + UPB_ASSERT(i < arr->size); + UPB_ASSERT(elem_size == 1U << _upb_Array_ElementSizeLg2(arr)); + char* arr_data = (char*)_upb_array_ptr(arr); + memcpy(arr_data + (i * elem_size), data, elem_size); } -UPB_INLINE void* _upb_Array_Resize_accessor2(void* msg, size_t ofs, size_t size, - int elem_size_lg2, - upb_Arena* arena) { - upb_Array** arr_ptr = UPB_PTR_AT(msg, ofs, upb_Array*); - upb_Array* arr = *arr_ptr; - if (!arr || arr->capacity < size) { - return _upb_Array_Resize_fallback(arr_ptr, size, elem_size_lg2, arena); - } - arr->size = size; - return _upb_array_ptr(arr); -} - -UPB_INLINE bool _upb_Array_Append_accessor2(void* msg, size_t ofs, - int elem_size_lg2, - const void* value, - upb_Arena* arena) { - upb_Array** arr_ptr = UPB_PTR_AT(msg, ofs, upb_Array*); - size_t elem_size = 1 << elem_size_lg2; - upb_Array* arr = *arr_ptr; - void* ptr; - if (!arr || arr->size == arr->capacity) { - return _upb_Array_Append_fallback(arr_ptr, value, elem_size_lg2, arena); - } - ptr = _upb_array_ptr(arr); - memcpy(UPB_PTR_AT(ptr, arr->size * elem_size, char), value, elem_size); - arr->size++; - return true; -} - -// Used by old generated code, remove once all code has been regenerated. -UPB_INLINE int _upb_sizelg2(upb_CType type) { - switch (type) { - case kUpb_CType_Bool: - return 0; - case kUpb_CType_Float: - case kUpb_CType_Int32: - case kUpb_CType_UInt32: - case kUpb_CType_Enum: - return 2; - case kUpb_CType_Message: - return UPB_SIZE(2, 3); - case kUpb_CType_Double: - case kUpb_CType_Int64: - case kUpb_CType_UInt64: - return 3; - case kUpb_CType_String: - case kUpb_CType_Bytes: - return UPB_SIZE(3, 4); - } - UPB_UNREACHABLE(); -} - -UPB_INLINE void* _upb_Array_Resize_accessor(void* msg, size_t ofs, size_t size, - upb_CType type, upb_Arena* arena) { - return _upb_Array_Resize_accessor2(msg, ofs, size, _upb_sizelg2(type), arena); -} - -UPB_INLINE bool _upb_Array_Append_accessor(void* msg, size_t ofs, - size_t elem_size, upb_CType type, - const void* value, - upb_Arena* arena) { - (void)elem_size; - return _upb_Array_Append_accessor2(msg, ofs, _upb_sizelg2(type), value, - arena); +UPB_INLINE void _upb_array_detach(const void* msg, size_t ofs) { + *UPB_PTR_AT(msg, ofs, upb_Array*) = NULL; } #ifdef __cplusplus diff --git a/upb/message/accessors.c b/upb/message/accessors.c index 83ca4040ec..4359441a33 100644 --- a/upb/message/accessors.c +++ b/upb/message/accessors.c @@ -38,38 +38,6 @@ // Must be last. #include "upb/port/def.inc" -// Maps descriptor type to elem_size_lg2. -static int _upb_MiniTableField_CTypeLg2Size(const upb_MiniTableField* f) { - static const uint8_t sizes[] = { - -1, /* invalid descriptor type */ - 3, /* DOUBLE */ - 2, /* FLOAT */ - 3, /* INT64 */ - 3, /* UINT64 */ - 2, /* INT32 */ - 3, /* FIXED64 */ - 2, /* FIXED32 */ - 0, /* BOOL */ - UPB_SIZE(3, 4), /* STRING */ - UPB_SIZE(2, 3), /* GROUP */ - UPB_SIZE(2, 3), /* MESSAGE */ - UPB_SIZE(3, 4), /* BYTES */ - 2, /* UINT32 */ - 2, /* ENUM */ - 2, /* SFIXED32 */ - 3, /* SFIXED64 */ - 2, /* SINT32 */ - 3, /* SINT64 */ - }; - return sizes[f->descriptortype]; -} - -void* upb_Message_ResizeArray(upb_Message* msg, const upb_MiniTableField* field, - size_t len, upb_Arena* arena) { - return _upb_Array_Resize_accessor2( - msg, field->offset, len, _upb_MiniTableField_CTypeLg2Size(field), arena); -} - typedef struct { const char* ptr; uint64_t val; diff --git a/upb/message/accessors.h b/upb/message/accessors.h index c40b9ddca8..94da35b231 100644 --- a/upb/message/accessors.h +++ b/upb/message/accessors.h @@ -30,6 +30,7 @@ #include "upb/base/descriptor_constants.h" #include "upb/collections/array.h" +#include "upb/collections/array_internal.h" #include "upb/collections/map.h" #include "upb/collections/map_internal.h" #include "upb/message/extension_internal.h" @@ -68,12 +69,25 @@ UPB_INLINE void _upb_Message_SetPresence(upb_Message* msg, } } +#if defined(__GNUC__) && !defined(__clang__) +// GCC raises incorrect warnings in these functions. It thinks that we are +// overrunning buffers, but we carefully write the functions in this file to +// guarantee that this is impossible. GCC gets this wrong due it its failure +// to perform constant propagation as we expect: +// - https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108217 +// - https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108226 +// +// Unfortunately this also indicates that GCC is not optimizing away the +// switch() in cases where it should be, compromising the performance. +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Warray-bounds" +#pragma GCC diagnostic ignored "-Wstringop-overflow" +#pragma GCC diagnostic ignored "-Wstringop-overread" +#endif + UPB_INLINE bool _upb_MiniTable_ValueIsNonZero(const void* default_val, const upb_MiniTableField* field) { char zero[16] = {0}; - if (upb_IsRepeatedOrMap(field)) { - return memcmp(&zero, default_val, sizeof(void*)); - } switch (_upb_MiniTableField_GetRep(field)) { case kUpb_FieldRep_1Byte: return memcmp(&zero, default_val, 1) != 0; @@ -91,10 +105,6 @@ UPB_INLINE bool _upb_MiniTable_ValueIsNonZero(const void* default_val, UPB_INLINE void _upb_MiniTable_CopyFieldData(void* to, const void* from, const upb_MiniTableField* field) { - if (upb_IsRepeatedOrMap(field)) { - memcpy(to, from, sizeof(void*)); - return; - } switch (_upb_MiniTableField_GetRep(field)) { case kUpb_FieldRep_1Byte: memcpy(to, from, 1); @@ -113,6 +123,36 @@ UPB_INLINE void _upb_MiniTable_CopyFieldData(void* to, const void* from, UPB_UNREACHABLE(); } +#if defined(__GNUC__) && !defined(__clang__) +#pragma GCC diagnostic pop +#endif + +UPB_INLINE size_t +_upb_MiniTable_ElementSizeLg2(const upb_MiniTableField* field) { + const unsigned char table[] = { + 0, + 3, // kUpb_FieldType_Double = 1, + 2, // kUpb_FieldType_Float = 2, + 3, // kUpb_FieldType_Int64 = 3, + 3, // kUpb_FieldType_UInt64 = 4, + 2, // kUpb_FieldType_Int32 = 5, + 3, // kUpb_FieldType_Fixed64 = 6, + 2, // kUpb_FieldType_Fixed32 = 7, + 0, // kUpb_FieldType_Bool = 8, + UPB_SIZE(3, 4), // kUpb_FieldType_String = 9, + UPB_SIZE(2, 3), // kUpb_FieldType_Group = 10, + UPB_SIZE(2, 3), // kUpb_FieldType_Message = 11, + UPB_SIZE(3, 4), // kUpb_FieldType_Bytes = 12, + 2, // kUpb_FieldType_UInt32 = 13, + 2, // kUpb_FieldType_Enum = 14, + 2, // kUpb_FieldType_SFixed32 = 15, + 3, // kUpb_FieldType_SFixed64 = 16, + 2, // kUpb_FieldType_SInt32 = 17, + 3, // kUpb_FieldType_SInt64 = 18, + }; + return table[field->descriptortype]; +} + // Here we define universal getter/setter functions for message fields. // These look very branchy and inefficient, but as long as the MiniTableField // values are known at compile time, all the branches are optimized away and @@ -261,12 +301,14 @@ UPB_INLINE void _upb_Message_ClearNonExtensionField( UPB_INLINE upb_Map* _upb_MiniTable_GetOrCreateMutableMap( upb_Message* msg, const upb_MiniTableField* field, size_t key_size, size_t val_size, upb_Arena* arena) { - UPB_ASSUME(upb_IsRepeatedOrMap(field)); + _upb_MiniTableField_CheckIsMap(field); upb_Map* map = NULL; upb_Map* default_map_value = NULL; _upb_Message_GetNonExtensionField(msg, field, &default_map_value, &map); if (!map) { map = _upb_Map_New(arena, key_size, val_size); + // Check again due to: https://godbolt.org/z/7WfaoKG1r + _upb_MiniTableField_CheckIsMap(field); _upb_Message_SetNonExtensionField(msg, field, &map); } return map; @@ -531,6 +573,7 @@ UPB_API_INLINE upb_Message* upb_MiniTable_GetMutableMessage( UPB_API_INLINE const upb_Array* upb_Message_GetArray( const upb_Message* msg, const upb_MiniTableField* field) { + _upb_MiniTableField_CheckIsArray(field); const upb_Array* ret; const upb_Array* default_val = NULL; _upb_Message_GetNonExtensionField(msg, field, &default_val, &ret); @@ -539,22 +582,43 @@ UPB_API_INLINE const upb_Array* upb_Message_GetArray( UPB_API_INLINE upb_Array* upb_Message_GetMutableArray( upb_Message* msg, const upb_MiniTableField* field) { + _upb_MiniTableField_CheckIsArray(field); return (upb_Array*)upb_Message_GetArray(msg, field); } -UPB_API_INLINE upb_Array* upb_Message_GetOrCreateMutableArray( - upb_Message* msg, const upb_MiniTableField* field, upb_CType ctype, - upb_Arena* arena) { +UPB_INLINE upb_Array* upb_Message_GetOrCreateMutableArray( + upb_Message* msg, const upb_MiniTableField* field, upb_Arena* arena) { + _upb_MiniTableField_CheckIsArray(field); upb_Array* array = upb_Message_GetMutableArray(msg, field); if (!array) { - array = upb_Array_New(arena, ctype); + array = _upb_Array_New(arena, 4, _upb_MiniTable_ElementSizeLg2(field)); + // Check again due to: https://godbolt.org/z/7WfaoKG1r + _upb_MiniTableField_CheckIsArray(field); _upb_Message_SetField(msg, field, &array, arena); } return array; } -void* upb_Message_ResizeArray(upb_Message* msg, const upb_MiniTableField* field, - size_t len, upb_Arena* arena); +UPB_INLINE upb_Array* upb_Message_ResizeArrayUninitialized( + upb_Message* msg, const upb_MiniTableField* field, size_t size, + upb_Arena* arena) { + _upb_MiniTableField_CheckIsArray(field); + upb_Array* arr = upb_Message_GetOrCreateMutableArray(msg, field, arena); + if (!arr || !_upb_Array_ResizeUninitialized(arr, size, arena)) return NULL; + return arr; +} + +// TODO: remove, migrate users to upb_Message_ResizeArrayUninitialized(), which +// has the same semantics but a clearer name. Alternatively, if users want an +// initialized variant, we can also offer that. +UPB_API_INLINE void* upb_Message_ResizeArray(upb_Message* msg, + const upb_MiniTableField* field, + size_t size, upb_Arena* arena) { + _upb_MiniTableField_CheckIsArray(field); + upb_Array* arr = + upb_Message_ResizeArrayUninitialized(msg, field, size, arena); + return _upb_array_ptr(arr); +} UPB_API_INLINE bool upb_MiniTableField_IsClosedEnum( const upb_MiniTableField* field) { @@ -563,7 +627,7 @@ UPB_API_INLINE bool upb_MiniTableField_IsClosedEnum( UPB_API_INLINE const upb_Map* upb_Message_GetMap( const upb_Message* msg, const upb_MiniTableField* field) { - UPB_ASSUME(upb_FieldMode_Get(field) == kUpb_FieldMode_Map); + _upb_MiniTableField_CheckIsMap(field); const upb_Map* ret; const upb_Map* default_val = NULL; _upb_Message_GetNonExtensionField(msg, field, &default_val, &ret); diff --git a/upb/mini_table/field_internal.h b/upb/mini_table/field_internal.h index f58531a5a6..e2b8f6bcc2 100644 --- a/upb/mini_table/field_internal.h +++ b/upb/mini_table/field_internal.h @@ -75,6 +75,8 @@ typedef enum { kUpb_FieldRep_StringView = 2, kUpb_FieldRep_8Byte = 3, + kUpb_FieldRep_NativePointer = + UPB_SIZE(kUpb_FieldRep_4Byte, kUpb_FieldRep_8Byte), kUpb_FieldRep_Max = kUpb_FieldRep_8Byte, } upb_FieldRep; @@ -93,6 +95,20 @@ UPB_INLINE upb_FieldMode upb_FieldMode_Get(const upb_MiniTableField* field) { return (upb_FieldMode)(field->mode & 3); } +UPB_INLINE void _upb_MiniTableField_CheckIsArray( + const upb_MiniTableField* field) { + UPB_ASSUME(_upb_MiniTableField_GetRep(field) == kUpb_FieldRep_NativePointer); + UPB_ASSUME(upb_FieldMode_Get(field) == kUpb_FieldMode_Array); + UPB_ASSUME(field->presence == 0); +} + +UPB_INLINE void _upb_MiniTableField_CheckIsMap( + const upb_MiniTableField* field) { + UPB_ASSUME(_upb_MiniTableField_GetRep(field) == kUpb_FieldRep_NativePointer); + UPB_ASSUME(upb_FieldMode_Get(field) == kUpb_FieldMode_Map); + UPB_ASSUME(field->presence == 0); +} + UPB_INLINE bool upb_IsRepeatedOrMap(const upb_MiniTableField* field) { // This works because upb_FieldMode has no value 3. return !(field->mode & kUpb_FieldMode_Scalar); diff --git a/upbc/file_layout.h b/upbc/file_layout.h index 51ea2b72c0..265dfe3c83 100644 --- a/upbc/file_layout.h +++ b/upbc/file_layout.h @@ -180,23 +180,6 @@ class FileLayout { return layout64_.GetEnumTable(d); } - std::string GetFieldOffset(const protobuf::FieldDescriptor* f) const { - const upb_MiniTableField* f_32 = upb_MiniTable_FindFieldByNumber( - GetMiniTable32(f->containing_type()), f->number()); - const upb_MiniTableField* f_64 = upb_MiniTable_FindFieldByNumber( - GetMiniTable64(f->containing_type()), f->number()); - return UpbSize(f_32->offset, f_64->offset); - } - - std::string GetOneofCaseOffset(const protobuf::OneofDescriptor* o) const { - const protobuf::FieldDescriptor* f = o->field(0); - const upb_MiniTableField* f_32 = upb_MiniTable_FindFieldByNumber( - GetMiniTable32(f->containing_type()), f->number()); - const upb_MiniTableField* f_64 = upb_MiniTable_FindFieldByNumber( - GetMiniTable64(f->containing_type()), f->number()); - return UpbSize(~f_32->presence, ~f_64->presence); - } - std::string GetMessageSize(const protobuf::Descriptor* d) const { return UpbSize(GetMiniTable32(d)->size, GetMiniTable64(d)->size); } diff --git a/upbc/protoc-gen-upb.cc b/upbc/protoc-gen-upb.cc index 71d34963e0..4c27f53233 100644 --- a/upbc/protoc-gen-upb.cc +++ b/upbc/protoc-gen-upb.cc @@ -137,34 +137,6 @@ std::string CTypeInternal(const protobuf::FieldDescriptor* field, } } -std::string SizeLg2(const protobuf::FieldDescriptor* field) { - switch (field->cpp_type()) { - case protobuf::FieldDescriptor::CPPTYPE_MESSAGE: - return "UPB_SIZE(2, 3)"; - case protobuf::FieldDescriptor::CPPTYPE_ENUM: - return std::to_string(2); - case protobuf::FieldDescriptor::CPPTYPE_BOOL: - return std::to_string(0); - case protobuf::FieldDescriptor::CPPTYPE_FLOAT: - return std::to_string(2); - case protobuf::FieldDescriptor::CPPTYPE_INT32: - return std::to_string(2); - case protobuf::FieldDescriptor::CPPTYPE_UINT32: - return std::to_string(2); - case protobuf::FieldDescriptor::CPPTYPE_DOUBLE: - return std::to_string(3); - case protobuf::FieldDescriptor::CPPTYPE_INT64: - return std::to_string(3); - case protobuf::FieldDescriptor::CPPTYPE_UINT64: - return std::to_string(3); - case protobuf::FieldDescriptor::CPPTYPE_STRING: - return "UPB_SIZE(3, 4)"; - default: - fprintf(stderr, "Unexpected type"); - abort(); - } -} - std::string FloatToCLiteral(float value) { if (value == std::numeric_limits::infinity()) { return "kUpb_FltInfinity"; @@ -518,12 +490,20 @@ void GenerateRepeatedGetters(const protobuf::FieldDescriptor* field, Output& output) { output( R"cc( - UPB_INLINE $0 const* $1_$2(const $1* msg, size_t* len) { - return ($0 const*)_upb_array_accessor(msg, $3, len); + UPB_INLINE $0 const* $1_$2(const $1* msg, size_t* size) { + const upb_MiniTableField field = $3; + const upb_Array* arr = upb_Message_GetArray(msg, &field); + if (arr) { + if (size) *size = arr->size; + return ($0 const*)_upb_array_constptr(arr); + } else { + if (size) *size = 0; + return NULL; + } } )cc", CTypeConst(field), msg_name, ResolveFieldName(field, field_names), - layout.GetFieldOffset(field)); + FieldInitializer(layout, field)); } void GenerateScalarGetters(const protobuf::FieldDescriptor* field, @@ -618,41 +598,58 @@ void GenerateRepeatedSetters(const protobuf::FieldDescriptor* field, std::string resolved_name = ResolveFieldName(field, field_names); output( R"cc( - UPB_INLINE $0* $1_mutable_$2($1* msg, size_t* len) { - return ($0*)_upb_array_mutable_accessor(msg, $3, len); + UPB_INLINE $0* $1_mutable_$2($1* msg, size_t* size) { + upb_MiniTableField field = $3; + upb_Array* arr = upb_Message_GetMutableArray(msg, &field); + if (arr) { + if (size) *size = arr->size; + return ($0*)_upb_array_ptr(arr); + } else { + if (size) *size = 0; + return NULL; + } } )cc", - CType(field), msg_name, resolved_name, layout.GetFieldOffset(field)); + CType(field), msg_name, resolved_name, FieldInitializer(layout, field)); output( R"cc( - UPB_INLINE $0* $1_resize_$2($1* msg, size_t len, upb_Arena* arena) { - return ($0*)_upb_Array_Resize_accessor2(msg, $3, len, $4, arena); + UPB_INLINE $0* $1_resize_$2($1* msg, size_t size, upb_Arena* arena) { + upb_MiniTableField field = $3; + return ($0*)upb_Message_ResizeArray(msg, &field, size, arena); } )cc", - CType(field), msg_name, resolved_name, layout.GetFieldOffset(field), - SizeLg2(field)); + CType(field), msg_name, resolved_name, FieldInitializer(layout, field)); if (field->cpp_type() == protobuf::FieldDescriptor::CPPTYPE_MESSAGE) { output( R"cc( UPB_INLINE struct $0* $1_add_$2($1* msg, upb_Arena* arena) { + upb_MiniTableField field = $4; + upb_Array* arr = upb_Message_GetOrCreateMutableArray(msg, &field, arena); + if (!arr || !_upb_Array_ResizeUninitialized(arr, arr->size + 1, arena)) { + return NULL; + } struct $0* sub = (struct $0*)_upb_Message_New(&$3, arena); - bool ok = _upb_Array_Append_accessor2(msg, $4, $5, &sub, arena); - if (!ok) return NULL; + if (!arr || !sub) return NULL; + _upb_Array_Set(arr, arr->size - 1, &sub, sizeof(sub)); return sub; } )cc", MessageName(field->message_type()), msg_name, resolved_name, - MessageInit(field->message_type()), layout.GetFieldOffset(field), - SizeLg2(field)); + MessageInit(field->message_type()), FieldInitializer(layout, field)); } else { output( R"cc( UPB_INLINE bool $1_add_$2($1* msg, $0 val, upb_Arena* arena) { - return _upb_Array_Append_accessor2(msg, $3, $4, &val, arena); + upb_MiniTableField field = $3; + upb_Array* arr = upb_Message_GetOrCreateMutableArray(msg, &field, arena); + if (!arr || !_upb_Array_ResizeUninitialized(arr, arr->size + 1, arena)) { + return false; + } + _upb_Array_Set(arr, arr->size - 1, &val, sizeof(val)); + return true; } )cc", - CType(field), msg_name, resolved_name, layout.GetFieldOffset(field), - SizeLg2(field)); + CType(field), msg_name, resolved_name, FieldInitializer(layout, field)); } }