From 565b28c0d1d04008f3af87629e059a9f96ed73e1 Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Mon, 26 Dec 2022 18:44:24 -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: 497871886 --- .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, 254 insertions(+), 181 deletions(-) diff --git a/.github/workflows/bazel_tests.yml b/.github/workflows/bazel_tests.yml index b0affa1859..efe6fe5e69 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-12, os: ubuntu-22.04, flags: "-c opt" } + - { NAME: "GCC Optimized", BAZEL: bazel, CC: gcc, os: ubuntu-20.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,10 +53,6 @@ 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 99bfaa2402..835601c549 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 fdb1bd206c..a957f26ab8 100644 --- a/upb/collections/array.c +++ b/upb/collections/array.c @@ -32,22 +32,23 @@ // Must be last. #include "upb/port/def.inc" -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), +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 */ }; upb_Array* upb_Array_New(upb_Arena* a, upb_CType type) { - return _upb_Array_New(a, 4, _upb_Array_CTypeSizeLg2(type)); + return _upb_Array_New(a, 4, _upb_CTypeo_sizelg2[type]); } size_t upb_Array_Size(const upb_Array* arr) { return arr->size; } @@ -141,3 +142,35 @@ 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 0c126eb642..e1a4cefde2 100644 --- a/upb/collections/array_internal.h +++ b/upb/collections/array_internal.h @@ -48,14 +48,8 @@ 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_Array_ElementSizeLg2(arr); // Check assertion. + UPB_ASSERT((arr->data & 7) <= 4); return (void*)(arr->data & ~(uintptr_t)7); } @@ -74,15 +68,8 @@ 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); @@ -96,6 +83,12 @@ 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); @@ -110,21 +103,100 @@ UPB_INLINE bool _upb_Array_ResizeUninitialized(upb_Array* arr, size_t size, return true; } -// 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_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; + } +} + +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); +} + #ifdef __cplusplus } /* extern "C" */ #endif diff --git a/upb/message/accessors.c b/upb/message/accessors.c index 4359441a33..83ca4040ec 100644 --- a/upb/message/accessors.c +++ b/upb/message/accessors.c @@ -38,6 +38,38 @@ // 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 94da35b231..c40b9ddca8 100644 --- a/upb/message/accessors.h +++ b/upb/message/accessors.h @@ -30,7 +30,6 @@ #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" @@ -69,25 +68,12 @@ 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; @@ -105,6 +91,10 @@ 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); @@ -123,36 +113,6 @@ 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 @@ -301,14 +261,12 @@ 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_MiniTableField_CheckIsMap(field); + UPB_ASSUME(upb_IsRepeatedOrMap(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; @@ -573,7 +531,6 @@ 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); @@ -582,43 +539,22 @@ 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_INLINE upb_Array* upb_Message_GetOrCreateMutableArray( - upb_Message* msg, const upb_MiniTableField* field, upb_Arena* arena) { - _upb_MiniTableField_CheckIsArray(field); +UPB_API_INLINE upb_Array* upb_Message_GetOrCreateMutableArray( + upb_Message* msg, const upb_MiniTableField* field, upb_CType ctype, + upb_Arena* arena) { upb_Array* array = upb_Message_GetMutableArray(msg, field); if (!array) { - array = _upb_Array_New(arena, 4, _upb_MiniTable_ElementSizeLg2(field)); - // Check again due to: https://godbolt.org/z/7WfaoKG1r - _upb_MiniTableField_CheckIsArray(field); + array = upb_Array_New(arena, ctype); _upb_Message_SetField(msg, field, &array, arena); } return array; } -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); -} +void* upb_Message_ResizeArray(upb_Message* msg, const upb_MiniTableField* field, + size_t len, upb_Arena* arena); UPB_API_INLINE bool upb_MiniTableField_IsClosedEnum( const upb_MiniTableField* field) { @@ -627,7 +563,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_MiniTableField_CheckIsMap(field); + UPB_ASSUME(upb_FieldMode_Get(field) == kUpb_FieldMode_Map); 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 e2b8f6bcc2..f58531a5a6 100644 --- a/upb/mini_table/field_internal.h +++ b/upb/mini_table/field_internal.h @@ -75,8 +75,6 @@ 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; @@ -95,20 +93,6 @@ 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 265dfe3c83..51ea2b72c0 100644 --- a/upbc/file_layout.h +++ b/upbc/file_layout.h @@ -180,6 +180,23 @@ 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 4c27f53233..71d34963e0 100644 --- a/upbc/protoc-gen-upb.cc +++ b/upbc/protoc-gen-upb.cc @@ -137,6 +137,34 @@ 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"; @@ -490,20 +518,12 @@ void GenerateRepeatedGetters(const protobuf::FieldDescriptor* field, Output& output) { output( R"cc( - 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; - } + UPB_INLINE $0 const* $1_$2(const $1* msg, size_t* len) { + return ($0 const*)_upb_array_accessor(msg, $3, len); } )cc", CTypeConst(field), msg_name, ResolveFieldName(field, field_names), - FieldInitializer(layout, field)); + layout.GetFieldOffset(field)); } void GenerateScalarGetters(const protobuf::FieldDescriptor* field, @@ -598,58 +618,41 @@ 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* 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; - } + UPB_INLINE $0* $1_mutable_$2($1* msg, size_t* len) { + return ($0*)_upb_array_mutable_accessor(msg, $3, len); } )cc", - CType(field), msg_name, resolved_name, FieldInitializer(layout, field)); + CType(field), msg_name, resolved_name, layout.GetFieldOffset(field)); output( R"cc( - 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); + 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); } )cc", - CType(field), msg_name, resolved_name, FieldInitializer(layout, field)); + CType(field), msg_name, resolved_name, layout.GetFieldOffset(field), + SizeLg2(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); - if (!arr || !sub) return NULL; - _upb_Array_Set(arr, arr->size - 1, &sub, sizeof(sub)); + bool ok = _upb_Array_Append_accessor2(msg, $4, $5, &sub, arena); + if (!ok) return NULL; return sub; } )cc", MessageName(field->message_type()), msg_name, resolved_name, - MessageInit(field->message_type()), FieldInitializer(layout, field)); + MessageInit(field->message_type()), layout.GetFieldOffset(field), + SizeLg2(field)); } else { output( R"cc( UPB_INLINE bool $1_add_$2($1* msg, $0 val, upb_Arena* 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; + return _upb_Array_Append_accessor2(msg, $3, $4, &val, arena); } )cc", - CType(field), msg_name, resolved_name, FieldInitializer(layout, field)); + CType(field), msg_name, resolved_name, layout.GetFieldOffset(field), + SizeLg2(field)); } }