From 28bc460dc95bc25a8392d56c55cdfdf0bcee6dda Mon Sep 17 00:00:00 2001 From: Eric Salo Date: Fri, 15 Jul 2022 09:59:27 -0700 Subject: [PATCH] create _upb_EnumDef_MiniDescriptor() delete upb_EnumDef_IsSorted() We now have a simple internal function for returning a mini descriptor directly from an enum def. PiperOrigin-RevId: 461208352 --- upb/def.c | 45 ++++++++++++-------- upb/def.h | 4 +- upb/mini_descriptor.c | 79 +++++++++++------------------------ upb/mini_descriptor.h | 14 +++++-- upbc/code_generator_request.c | 8 ++-- 5 files changed, 69 insertions(+), 81 deletions(-) diff --git a/upb/def.c b/upb/def.c index c1a9d211e6..4a1254d7be 100644 --- a/upb/def.c +++ b/upb/def.c @@ -322,10 +322,12 @@ uint32_t field_rank(const upb_FieldDef* f) { return ret; } -int cmp_fields(const void* p1, const void* p2) { - const upb_FieldDef* f1 = *(upb_FieldDef* const*)p1; - const upb_FieldDef* f2 = *(upb_FieldDef* const*)p2; - return field_rank(f1) - field_rank(f2); +static int cmp_values(const void* a, const void* b) { + const upb_EnumValueDef* A = *(const upb_EnumValueDef**)a; + const upb_EnumValueDef* B = *(const upb_EnumValueDef**)b; + if ((uint32_t)A->number < (uint32_t)B->number) return -1; + if ((uint32_t)A->number > (uint32_t)B->number) return 1; + return 0; } static void upb_Status_setoom(upb_Status* status) { @@ -404,8 +406,6 @@ int32_t upb_EnumDef_Default(const upb_EnumDef* e) { int upb_EnumDef_ValueCount(const upb_EnumDef* e) { return e->value_count; } -bool upb_EnumDef_IsSorted(const upb_EnumDef* e) { return e->is_sorted; } - const upb_EnumValueDef* upb_EnumDef_FindValueByNameWithSize( const upb_EnumDef* def, const char* name, size_t len) { upb_value v; @@ -432,6 +432,21 @@ const upb_EnumValueDef* upb_EnumDef_Value(const upb_EnumDef* e, int i) { return &e->values[i]; } +const char* _upb_EnumDef_MiniDescriptor(const upb_EnumDef* e, upb_Arena* a) { + if (e->is_sorted) return upb_MiniDescriptor_EncodeEnum(e, NULL, a); + + const upb_EnumValueDef** sorted = (const upb_EnumValueDef**)upb_Arena_Malloc( + a, e->value_count * sizeof(void*)); + if (!sorted) return NULL; + + for (size_t i = 0; i < e->value_count; i++) { + sorted[i] = upb_EnumDef_Value(e, i); + } + qsort(sorted, e->value_count, sizeof(void*), cmp_values); + + return upb_MiniDescriptor_EncodeEnum(e, sorted, a); +} + /* upb_EnumValueDef ***********************************************************/ const google_protobuf_EnumValueOptions* upb_EnumValueDef_Options( @@ -2459,26 +2474,22 @@ static int compare_int32(const void* a_ptr, const void* b_ptr) { static upb_MiniTable_Enum* create_enumlayout(symtab_addctx* ctx, const upb_EnumDef* e) { - char* data; - size_t size; - bool ok = upb_MiniDescriptor_EncodeEnum(e, &data, &size, ctx->tmp_arena); - CHK_OOM(ok); + const char* desc = _upb_EnumDef_MiniDescriptor(e, ctx->tmp_arena); + if (!desc) symtab_errf(ctx, "OOM while building enum MiniDescriptor"); upb_Status status; upb_MiniTable_Enum* layout = - upb_MiniTable_BuildEnum(data, size, ctx->arena, &status); + upb_MiniTable_BuildEnum(desc, strlen(desc), ctx->arena, &status); if (!layout) symtab_errf(ctx, "Error building enum MiniTable: %s", status.msg); return layout; } -static void create_enumvaldef( - symtab_addctx* ctx, const char* prefix, - const google_protobuf_EnumValueDescriptorProto* val_proto, upb_EnumDef* e, - int i) { +static void create_enumvaldef(symtab_addctx* ctx, const char* prefix, + const google_protobuf_EnumValueDescriptorProto* val_proto, + upb_EnumDef* e, int i) { upb_EnumValueDef* val = (upb_EnumValueDef*)&e->values[i]; - upb_StringView name = - google_protobuf_EnumValueDescriptorProto_name(val_proto); + upb_StringView name = google_protobuf_EnumValueDescriptorProto_name(val_proto); upb_value v = upb_value_constptr(val); val->parent = e; /* Must happen prior to symtab_add(). */ diff --git a/upb/def.h b/upb/def.h index 8dd696d80e..5fd5fb703b 100644 --- a/upb/def.h +++ b/upb/def.h @@ -268,7 +268,6 @@ const upb_FileDef* upb_EnumDef_File(const upb_EnumDef* e); const upb_MessageDef* upb_EnumDef_ContainingType(const upb_EnumDef* e); int32_t upb_EnumDef_Default(const upb_EnumDef* e); int upb_EnumDef_ValueCount(const upb_EnumDef* e); -bool upb_EnumDef_IsSorted(const upb_EnumDef* e); const upb_EnumValueDef* upb_EnumDef_Value(const upb_EnumDef* e, int i); const upb_EnumValueDef* upb_EnumDef_FindValueByNameWithSize( @@ -283,6 +282,9 @@ UPB_INLINE const upb_EnumValueDef* upb_EnumDef_FindValueByName( return upb_EnumDef_FindValueByNameWithSize(e, name, strlen(name)); } +// Builds and returns a mini descriptor, or NULL if OOM. +const char* _upb_EnumDef_MiniDescriptor(const upb_EnumDef* e, upb_Arena* a); + /* upb_EnumValueDef ***********************************************************/ const google_protobuf_EnumValueOptions* upb_EnumValueDef_Options( diff --git a/upb/mini_descriptor.c b/upb/mini_descriptor.c index ba2f02252b..9be560af45 100644 --- a/upb/mini_descriptor.c +++ b/upb/mini_descriptor.c @@ -113,19 +113,6 @@ static uint64_t upb_Message_Modifiers(const upb_MessageDef* m) { /******************************************************************************/ -// Sort by enum value. -static int upb_MiniDescriptor_CompareEnums(const void* a, const void* b) { - const upb_EnumValueDef* A = *(void**)a; - const upb_EnumValueDef* B = *(void**)b; - if ((uint32_t)upb_EnumValueDef_Number(A) < - (uint32_t)upb_EnumValueDef_Number(B)) - return -1; - if ((uint32_t)upb_EnumValueDef_Number(A) > - (uint32_t)upb_EnumValueDef_Number(B)) - return 1; - return 0; -} - // Sort by field number. static int upb_MiniDescriptor_CompareFields(const void* a, const void* b) { const upb_FieldDef* A = *(void**)a; @@ -135,55 +122,39 @@ static int upb_MiniDescriptor_CompareFields(const void* a, const void* b) { return 0; } -bool upb_MiniDescriptor_EncodeEnum(const upb_EnumDef* e, char** data, - size_t* size, upb_Arena* a) { - const size_t len = upb_EnumDef_ValueCount(e); - +const char* upb_MiniDescriptor_EncodeEnum(const upb_EnumDef* e, + const upb_EnumValueDef** sorted, + upb_Arena* a) { DescState s; upb_DescState_Init(&s); - // Duplicate values are allowed but we only encode each value once. - uint32_t previous = 0; - upb_MtDataEncoder_StartEnum(&s.e); - if (upb_EnumDef_IsSorted(e)) { - // The enum is well behaved so no need to copy/sort the pointers here. - for (size_t i = 0; i < len; i++) { - const uint32_t current = upb_EnumValueDef_Number(upb_EnumDef_Value(e, i)); - if (i != 0 && previous == current) continue; + const size_t value_count = upb_EnumDef_ValueCount(e); - if (!upb_DescState_Grow(&s, a)) return false; - s.ptr = upb_MtDataEncoder_PutEnumValue(&s.e, s.ptr, current); - previous = current; - } - } else { - // The enum fields are unsorted. - const upb_EnumValueDef** sorted = - (const upb_EnumValueDef**)upb_Arena_Malloc(a, len * sizeof(void*)); - if (!sorted) return false; - - for (size_t i = 0; i < len; i++) { - sorted[i] = upb_EnumDef_Value(e, i); - } - qsort(sorted, len, sizeof(void*), upb_MiniDescriptor_CompareEnums); + // Duplicate values are allowed but we only encode each value once. + uint32_t previous = 0; - for (size_t i = 0; i < len; i++) { - const uint32_t current = upb_EnumValueDef_Number(sorted[i]); - if (i != 0 && previous == current) continue; + for (size_t i = 0; i < value_count; i++) { + const uint32_t current = + upb_EnumValueDef_Number(sorted ? sorted[i] : upb_EnumDef_Value(e, i)); + if (i != 0 && previous == current) continue; - if (!upb_DescState_Grow(&s, a)) return false; - s.ptr = upb_MtDataEncoder_PutEnumValue(&s.e, s.ptr, current); - previous = current; - } + if (!upb_DescState_Grow(&s, a)) return false; + s.ptr = upb_MtDataEncoder_PutEnumValue(&s.e, s.ptr, current); + previous = current; } if (!upb_DescState_Grow(&s, a)) return false; s.ptr = upb_MtDataEncoder_EndEnum(&s.e, s.ptr); - *data = s.buf; - *size = s.ptr - s.buf; - return true; + // NULL-terminate the mini descriptor so we can return it as a C string. + // There will always be room for this in the encoder buffer because + // kUpb_MtDataEncoder_MinSize is overkill for upb_MtDataEncoder_EndEnum(). + UPB_ASSERT(s.ptr < s.buf + s.bufsize); + *s.ptr++ = '\0'; + + return s.buf; } bool upb_MiniDescriptor_EncodeField(const upb_FieldDef* f, char** data, @@ -213,23 +184,23 @@ bool upb_MiniDescriptor_EncodeMessage(const upb_MessageDef* m, char** data, upb_DescState_Init(&s); // Make a copy. - const size_t len = upb_MessageDef_FieldCount(m); + const size_t field_count = upb_MessageDef_FieldCount(m); const upb_FieldDef** sorted = - (const upb_FieldDef**)upb_Arena_Malloc(a, len * sizeof(void*)); + (const upb_FieldDef**)upb_Arena_Malloc(a, field_count * sizeof(void*)); if (!sorted) return false; // Sort the copy. - for (size_t i = 0; i < len; i++) { + for (size_t i = 0; i < field_count; i++) { sorted[i] = upb_MessageDef_Field(m, i); } - qsort(sorted, len, sizeof(void*), upb_MiniDescriptor_CompareFields); + qsort(sorted, field_count, sizeof(void*), upb_MiniDescriptor_CompareFields); // Start encoding. if (!upb_DescState_Grow(&s, a)) return false; upb_MtDataEncoder_StartMessage(&s.e, s.ptr, upb_Message_Modifiers(m)); // Encode the fields. - for (size_t i = 0; i < len; i++) { + for (size_t i = 0; i < field_count; i++) { const upb_FieldDef* field_def = sorted[i]; const upb_FieldType type = upb_FieldDef_Type(field_def); const int number = upb_FieldDef_Number(field_def); diff --git a/upb/mini_descriptor.h b/upb/mini_descriptor.h index 73fbae3b29..50f55f384b 100644 --- a/upb/mini_descriptor.h +++ b/upb/mini_descriptor.h @@ -40,15 +40,21 @@ extern "C" { /** upb_MiniDescriptor ********************************************************/ -// All of these functions return true on success, false on failure. -// Failure always means an OOM error. +// Creates and returns a mini descriptor string for an enum, or NULL on error. +// If the values in the enum happen to be defined in ascending order (when cast +// to uint32_t) then |sorted| should be NULL. Otherwise it must point to an +// array containing pointers to the enum values in sorted order. +const char* upb_MiniDescriptor_EncodeEnum(const upb_EnumDef* e, + const upb_EnumValueDef** sorted, + upb_Arena* a); -bool upb_MiniDescriptor_EncodeEnum(const upb_EnumDef* e, char** data, - size_t* size, upb_Arena* a); +// TODO(salo): Change the following two functions to match the above function. +// Returns true on success, false on error (OOM). bool upb_MiniDescriptor_EncodeField(const upb_FieldDef* f, char** data, size_t* size, upb_Arena* a); +// Returns true on success, false on error (OOM). bool upb_MiniDescriptor_EncodeMessage(const upb_MessageDef* m, char** data, size_t* size, upb_Arena* a); diff --git a/upbc/code_generator_request.c b/upbc/code_generator_request.c index 1ac0c4e683..54f05d0c4d 100644 --- a/upbc/code_generator_request.c +++ b/upbc/code_generator_request.c @@ -85,12 +85,10 @@ static void upbc_State_Emit(upbc_State* s, const char* name, const char* data, static void upbc_Scrape_Message(upbc_State*, const upb_MessageDef*); static void upbc_Scrape_Enum(upbc_State* s, const upb_EnumDef* e) { - char* data; - size_t size; - bool ok = upb_MiniDescriptor_EncodeEnum(e, &data, &size, s->arena); - if (!ok) upbc_Error(s, __func__, "could not encode enum"); + const char* desc = _upb_EnumDef_MiniDescriptor(e, s->arena); + if (!desc) upbc_Error(s, __func__, "could not encode enum"); - upbc_State_Emit(s, upb_EnumDef_FullName(e), data, size); + upbc_State_Emit(s, upb_EnumDef_FullName(e), desc, strlen(desc)); } static void upbc_Scrape_Extension(upbc_State* s, const upb_FieldDef* f) {