diff --git a/upb/def.c b/upb/def.c index 363d8bb960..3fc8917662 100644 --- a/upb/def.c +++ b/upb/def.c @@ -1623,7 +1623,10 @@ static void make_layout(symtab_addctx* ctx, const upb_MessageDef* m) { l->size = UPB_ALIGN_UP(l->size, 8); /* Sort fields by number. */ - qsort(fields, upb_MessageDef_numfields(m), sizeof(*fields), field_number_cmp); + if (fields) { + qsort(fields, upb_MessageDef_numfields(m), sizeof(*fields), + field_number_cmp); + } assign_layout_indices(m, l, fields); } @@ -2397,6 +2400,12 @@ static int count_bits_debug(uint64_t x) { return n; } +static int compare_int32(const void* a_ptr, const void* b_ptr) { + int32_t a = *(int32_t*)a_ptr; + int32_t b = *(int32_t*)b_ptr; + return a < b ? -1 : (a == b ? 0 : 1); +} + upb_MiniTable_Enum* create_enumlayout(symtab_addctx* ctx, const upb_EnumDef* e) { int n = 0; @@ -2405,7 +2414,7 @@ upb_MiniTable_Enum* create_enumlayout(symtab_addctx* ctx, for (int i = 0; i < e->value_count; i++) { uint32_t val = (uint32_t)e->values[i].number; if (val < 64) { - mask |= 1 << val; + mask |= 1ULL << val; } else { n++; } @@ -2427,6 +2436,17 @@ upb_MiniTable_Enum* create_enumlayout(symtab_addctx* ctx, UPB_ASSERT(p == values + n); } + // Enums can have duplicate values; we must sort+uniq them. + if (values) qsort(values, n, sizeof(*values), &compare_int32); + + int dst = 0; + for (int i = 0; i < n; dst++) { + int32_t val = values[i]; + while (i < n && values[i] == val) i++; // Skip duplicates. + values[dst] = val; + } + n = dst; + UPB_ASSERT(upb_inttable_count(&e->iton) == n + count_bits_debug(mask)); upb_MiniTable_Enum* layout = symtab_alloc(ctx, sizeof(*layout)); @@ -2510,7 +2530,7 @@ static void create_enumdef( if (ctx->layout) { UPB_ASSERT(ctx->enum_count < ctx->layout->enum_count); e->layout = ctx->layout->enums[ctx->enum_count++]; - UPB_ASSERT(n == + UPB_ASSERT(upb_inttable_count(&e->iton) == e->layout->value_count + count_bits_debug(e->layout->mask)); } else { e->layout = create_enumlayout(ctx, e); @@ -3085,7 +3105,8 @@ const upb_FileDef* upb_DefPool_AddFile( /* Include here since we want most of this file to be stdio-free. */ #include -bool _upb_DefPool_LoadDefInit(upb_DefPool* s, const _upb_DefPool_Init* init) { +bool _upb_DefPool_LoadDefInitEx(upb_DefPool* s, const _upb_DefPool_Init* init, + bool rebuild_minitable) { /* Since this function should never fail (it would indicate a bug in upb) we * print errors to stderr instead of returning error status to the user. */ _upb_DefPool_Init** deps = init->deps; @@ -3102,7 +3123,7 @@ bool _upb_DefPool_LoadDefInit(upb_DefPool* s, const _upb_DefPool_Init* init) { arena = upb_Arena_New(); for (; *deps; deps++) { - if (!_upb_DefPool_LoadDefInit(s, *deps)) goto err; + if (!_upb_DefPool_LoadDefInitEx(s, *deps, rebuild_minitable)) goto err; } file = google_protobuf_FileDescriptorProto_parse_ex( @@ -3119,7 +3140,8 @@ bool _upb_DefPool_LoadDefInit(upb_DefPool* s, const _upb_DefPool_Init* init) { goto err; } - if (!_upb_DefPool_AddFile(s, file, init->layout, &status)) { + const upb_MiniTable_File* mt = rebuild_minitable ? NULL : init->layout; + if (!_upb_DefPool_AddFile(s, file, mt, &status)) { goto err; } diff --git a/upb/def.h b/upb/def.h index 8b64a53c1d..2d22896e89 100644 --- a/upb/def.h +++ b/upb/def.h @@ -389,7 +389,15 @@ typedef struct _upb_DefPool_Init { upb_StringView descriptor; /* Serialized descriptor. */ } _upb_DefPool_Init; -bool _upb_DefPool_LoadDefInit(upb_DefPool* s, const _upb_DefPool_Init* init); +// Should only be directly called by tests. This variant lets us suppress +// the use of compiled-in tables, forcing a rebuild of the tables at runtime. +bool _upb_DefPool_LoadDefInitEx(upb_DefPool* s, const _upb_DefPool_Init* init, + bool rebuild_minitable); + +UPB_INLINE bool _upb_DefPool_LoadDefInit(upb_DefPool* s, + const _upb_DefPool_Init* init) { + return _upb_DefPool_LoadDefInitEx(s, init, false); +} #include "upb/port_undef.inc" diff --git a/upb/util/def_to_proto_test.cc b/upb/util/def_to_proto_test.cc index 22c99e5e7b..decc51f88a 100644 --- a/upb/util/def_to_proto_test.cc +++ b/upb/util/def_to_proto_test.cc @@ -125,3 +125,19 @@ TEST(DefToProto, Test) { upb::FileDefPtr file = msgdef.file(); CheckFile(file, file_desc); } + +// Like the previous test, but uses a message layout built at runtime. +TEST(DefToProto, TestRuntimeReflection) { + upb::Arena arena; + upb::SymbolTable symtab; + upb_StringView test_file_desc = + upb_util_def_to_proto_test_proto_upbdefinit.descriptor; + const auto* file_desc = google_protobuf_FileDescriptorProto_parse( + test_file_desc.data, test_file_desc.size, arena.ptr()); + + _upb_DefPool_LoadDefInitEx( + symtab.ptr(), &upb_util_def_to_proto_test_proto_upbdefinit, true); + upb::FileDefPtr file = symtab.FindFileByName( + upb_util_def_to_proto_test_proto_upbdefinit.filename); + CheckFile(file, file_desc); +} diff --git a/upb/util/def_to_proto_test.proto b/upb/util/def_to_proto_test.proto index c278748f88..4888914d3c 100644 --- a/upb/util/def_to_proto_test.proto +++ b/upb/util/def_to_proto_test.proto @@ -69,6 +69,26 @@ enum Enum { NEGATIVE_ONE = -1; } +enum EnumUpper32Value { + UPPER32_VALUE = 40; +} + +enum HasDuplicateValues { + option allow_alias = true; + A = 0; + B = 1; + C = 120; + D = 130; + + G = 120; + F = 1; + E = 0; + H = 121; + I = 121; + J = 121; + K = 121; +} + service Service { rpc Bar(Message) returns (Message); } @@ -77,6 +97,10 @@ extend Message { optional int32 ext = 1001; } +enum Has31 { + VALUE_31 = 31; +} + message PretendMessageSet { option message_set_wire_format = true; // Since this is message_set_wire_format, "max" here means INT32_MAX. diff --git a/upbc/protoc-gen-upb.cc b/upbc/protoc-gen-upb.cc index ff4a5028f1..7bbc154707 100644 --- a/upbc/protoc-gen-upb.cc +++ b/upbc/protoc-gen-upb.cc @@ -95,6 +95,18 @@ std::vector SortedEnums( return enums; } +std::vector SortedUniqueEnumNumbers( + const protobuf::EnumDescriptor* e) { + std::vector values; + for (int i = 0; i < e->value_count(); i++) { + values.push_back(e->value(i)->number()); + } + std::sort(values.begin(), values.end()); + auto last = std::unique(values.begin(), values.end()); + values.erase(last, values.end()); + return values; +} + void AddMessages(const protobuf::Descriptor* message, std::vector* messages) { messages->push_back(message); @@ -1300,23 +1312,19 @@ int WriteEnums(const protobuf::FileDescriptor* file, Output& output) { for (const auto* e : this_file_enums) { uint64_t mask = 0; - absl::flat_hash_set values; - for (int i = 0; i < e->value_count(); i++) { - int32_t number = e->value(i)->number(); + std::vector values; + for (auto number : SortedUniqueEnumNumbers(e)) { if (static_cast(number) < 64) { - mask |= 1 << number; + mask |= 1ULL << number; } else { - values.insert(number); + values.push_back(number); } } - std::vector values_vec(values.begin(), values.end()); - std::sort(values_vec.begin(), values_vec.end()); - if (!values_vec.empty()) { + if (!values.empty()) { values_init = EnumInit(e) + "_values"; - output("static const int32_t $0[$1] = {\n", values_init, - values_vec.size()); - for (auto value : values_vec) { + output("static const int32_t $0[$1] = {\n", values_init, values.size()); + for (int32_t value : values) { output(" $0,\n", value); } output("};\n\n"); @@ -1325,7 +1333,7 @@ int WriteEnums(const protobuf::FileDescriptor* file, Output& output) { output("const upb_MiniTable_Enum $0 = {\n", EnumInit(e)); output(" $0,\n", values_init); output(" 0x$0ULL,\n", absl::Hex(mask)); - output(" $0,\n", values_vec.size()); + output(" $0,\n", values.size()); output("};\n\n"); }