From f3316e2d7d3fd7c33b79d18ff258c6c188447501 Mon Sep 17 00:00:00 2001 From: Eric Salo Date: Wed, 24 Aug 2022 13:46:10 -0700 Subject: [PATCH 01/12] remove upb_String from the public tokenizer api upb_String is a hack which exists because the original C++ tokenizer got to assume the existence of C++ strings, so at least for now the C tokenizer needs a rough equivalent. But this should be a purely internal implementation detail, not part of the visible surface. PiperOrigin-RevId: 469814074 --- upb/io/tokenizer.c | 36 +++++++++++++++++++++--------------- upb/io/tokenizer.h | 16 +++++----------- upb/io/tokenizer_test.cc | 36 ++++++++++++++---------------------- 3 files changed, 40 insertions(+), 48 deletions(-) diff --git a/upb/io/tokenizer.c b/upb/io/tokenizer.c index 41ba7d9245..e3acd4dad7 100644 --- a/upb/io/tokenizer.c +++ b/upb/io/tokenizer.c @@ -30,6 +30,7 @@ #include #include "upb/internal/unicode.h" +#include "upb/io/string.h" #include "upb/io/strtod.h" // Must be included last. @@ -871,23 +872,25 @@ static const char* FetchUnicodePoint(const char* ptr, uint32_t* code_point) { } // The text string must begin and end with single or double quote characters. -void upb_Parse_StringAppend(const char* text, upb_String* output) { +upb_StringView upb_Parse_String(const char* text, upb_Arena* arena) { const size_t size = strlen(text); - // Reminder: text[0] is always a quote character. (If text is - // empty, it's invalid, so we'll just return). + upb_String output; + upb_String_Init(&output, arena); + + // Reminder: text[0] is always a quote character. + // (If text is empty, it's invalid, so we'll just return). if (size == 0) { fprintf(stderr, "Tokenizer::ParseStringAppend() passed text that could not" " have been tokenized as a string: %s", text); UPB_ASSERT(0); - return; } // Reserve room for new string. - const size_t new_len = size + upb_String_Size(output); - upb_String_Reserve(output, new_len); + const size_t new_len = size + upb_String_Size(&output); + upb_String_Reserve(&output, new_len); // Loop through the string copying characters to "output" and // interpreting escape sequences. Note that any invalid escape @@ -909,7 +912,7 @@ void upb_Parse_StringAppend(const char* text, upb_String* output) { ++ptr; code = code * 8 + DigitValue(*ptr); } - upb_String_PushBack(output, (char)code); + upb_String_PushBack(&output, (char)code); } else if (*ptr == 'x') { // A hex escape. May zero, one, or two digits. (The zero case @@ -923,29 +926,32 @@ void upb_Parse_StringAppend(const char* text, upb_String* output) { ++ptr; code = code * 16 + DigitValue(*ptr); } - upb_String_PushBack(output, (char)code); + upb_String_PushBack(&output, (char)code); } else if (*ptr == 'u' || *ptr == 'U') { uint32_t unicode; const char* end = FetchUnicodePoint(ptr, &unicode); if (end == ptr) { // Failure: Just dump out what we saw, don't try to parse it. - upb_String_PushBack(output, *ptr); + upb_String_PushBack(&output, *ptr); } else { - AppendUTF8(unicode, output); + AppendUTF8(unicode, &output); ptr = end - 1; // Because we're about to ++ptr. } } else { // Some other escape code. - upb_String_PushBack(output, TranslateEscape(*ptr)); + upb_String_PushBack(&output, TranslateEscape(*ptr)); } } else if (*ptr == text[0] && ptr[1] == '\0') { // Ignore final quote matching the starting quote. } else { - upb_String_PushBack(output, *ptr); + upb_String_PushBack(&output, *ptr); } } + + return upb_StringView_FromDataAndSize(upb_String_Data(&output), + upb_String_Size(&output)); } static bool AllInClass(bool (*f)(char), const char* text, int size) { @@ -955,11 +961,11 @@ static bool AllInClass(bool (*f)(char), const char* text, int size) { return true; } -bool upb_Tokenizer_IsIdentifier(const char* text, int size) { +bool upb_Tokenizer_IsIdentifier(const char* data, int size) { // Mirrors IDENTIFIER definition in Tokenizer::Next() above. if (size == 0) return false; - if (!upb_Tokenizer_IsLetter(text[0])) return false; - if (!AllInClass(upb_Tokenizer_IsAlphanumeric, text + 1, size - 1)) + if (!upb_Tokenizer_IsLetter(data[0])) return false; + if (!AllInClass(upb_Tokenizer_IsAlphanumeric, data + 1, size - 1)) return false; return true; } diff --git a/upb/io/tokenizer.h b/upb/io/tokenizer.h index d0d4f4d2b4..ed6ca5f434 100644 --- a/upb/io/tokenizer.h +++ b/upb/io/tokenizer.h @@ -30,8 +30,8 @@ #ifndef UPB_IO_TOKENIZER_H_ #define UPB_IO_TOKENIZER_H_ -#include "upb/io/string.h" #include "upb/io/zero_copy_input_stream.h" +#include "upb/string_view.h" #include "upb/upb.h" // Must be included last. @@ -123,6 +123,9 @@ int upb_Tokenizer_Line(const upb_Tokenizer* t); int upb_Tokenizer_TextSize(const upb_Tokenizer* t); const char* upb_Tokenizer_TextData(const upb_Tokenizer* t); +// External helper: validate an identifier. +bool upb_Tokenizer_IsIdentifier(const char* data, int size); + // Parses a TYPE_INTEGER token. Returns false if the result would be // greater than max_value. Otherwise, returns true and sets *output to the // result. If the text is not from a Token of type TYPE_INTEGER originally @@ -135,19 +138,10 @@ bool upb_Parse_Integer(const char* text, uint64_t max_value, uint64_t* output); // result is undefined (possibly an assert failure). double upb_Parse_Float(const char* text); -// Identical to ParseString (below), but appends to output. -void upb_Parse_StringAppend(const char* text, upb_String* output); - // Parses a TYPE_STRING token. This never fails, so long as the text actually // comes from a TYPE_STRING token parsed by Tokenizer. If it doesn't, the // result is undefined (possibly an assert failure). -UPB_INLINE void upb_Parse_String(const char* text, upb_String* output) { - upb_String_Clear(output); - upb_Parse_StringAppend(text, output); -} - -// External helper: validate an identifier. -bool upb_Tokenizer_IsIdentifier(const char* text, int size); +upb_StringView upb_Parse_String(const char* text, upb_Arena* arena); #ifdef __cplusplus } /* extern "C" */ diff --git a/upb/io/tokenizer_test.cc b/upb/io/tokenizer_test.cc index aa40b10f54..a4ed94337c 100644 --- a/upb/io/tokenizer_test.cc +++ b/upb/io/tokenizer_test.cc @@ -32,6 +32,7 @@ #include "absl/strings/str_format.h" #include "upb/internal/unicode.h" #include "upb/io/chunked_input_stream.h" +#include "upb/io/string.h" #include "upb/upb.hpp" // Must be last. @@ -998,35 +999,30 @@ TEST_F(TokenizerTest, ParseString) { }; upb::Arena arena; - upb_String result; - upb_String_Init(&result, arena.ptr()); for (int i = 0; i < sizeof(inputs) / sizeof(inputs[0]); i++) { - upb_Parse_String(inputs[i].data(), &result); - EXPECT_TRUE(StringEquals(upb_String_Data(&result), outputs[i].data())); + auto sv = upb_Parse_String(inputs[i].data(), arena.ptr()); + EXPECT_TRUE(StringEquals(sv.data, outputs[i].data())); } // Test invalid strings that will never be tokenized as strings. #ifdef GTEST_HAS_DEATH_TEST // death tests do not work on Windows yet EXPECT_DEBUG_DEATH( - upb_Parse_String("", &result), + upb_Parse_String("", arena.ptr()), "passed text that could not have been tokenized as a string"); #endif // GTEST_HAS_DEATH_TEST } TEST_F(TokenizerTest, ParseStringAppend) { - // Check that ParseString and ParseStringAppend differ. upb::Arena arena; upb_String output; upb_String_Init(&output, arena.ptr()); upb_String_Assign(&output, "stuff+", 6); - - upb_Parse_StringAppend("'hello'", &output); + auto sv = upb_Parse_String("'hello'", arena.ptr()); + EXPECT_TRUE(StringEquals(sv.data, "hello")); + upb_String_Append(&output, sv.data, sv.size); EXPECT_TRUE(StringEquals(upb_String_Data(&output), "stuff+hello")); - - upb_Parse_String("'hello'", &output); - EXPECT_TRUE(StringEquals(upb_String_Data(&output), "hello")); } // ------------------------------------------------------------------- @@ -1172,14 +1168,11 @@ static const char* kParseBenchmark[] = { TEST(Benchmark, ParseStringAppendAccumulate) { upb::Arena arena; - upb_String output; - upb_String_Init(&output, arena.ptr()); size_t outsize = 0; int benchmark_len = arraysize(kParseBenchmark); for (int i = 0; i < benchmark_len; i++) { - upb_Parse_StringAppend(kParseBenchmark[i], &output); - outsize += upb_String_Size(&output); - upb_String_Clear(&output); + auto sv = upb_Parse_String(kParseBenchmark[i], arena.ptr()); + outsize += sv.size; } EXPECT_NE(0, outsize); } @@ -1190,7 +1183,8 @@ TEST(Benchmark, ParseStringAppend) { upb_String_Init(&output, arena.ptr()); int benchmark_len = arraysize(kParseBenchmark); for (int i = 0; i < benchmark_len; i++) { - upb_Parse_StringAppend(kParseBenchmark[i], &output); + auto sv = upb_Parse_String(kParseBenchmark[i], arena.ptr()); + upb_String_Append(&output, sv.data, sv.size); } EXPECT_NE(0, upb_String_Size(&output)); } @@ -1217,12 +1211,10 @@ static std::string DisplayHex(const std::string& data) { static void ExpectFormat(const std::string& expectation, const std::string& formatted) { upb::Arena arena; - upb_String output; - upb_String_Init(&output, arena.ptr()); - upb_Parse_String(formatted.data(), &output); - EXPECT_EQ(strcmp(upb_String_Data(&output), expectation.data()), 0) + auto sv = upb_Parse_String(formatted.data(), arena.ptr()); + EXPECT_EQ(strcmp(sv.data, expectation.data()), 0) << ": Incorrectly parsed " << formatted << ":\nGot " - << DisplayHex(output.data_) << "\nExpected " << DisplayHex(expectation); + << DisplayHex(sv.data) << "\nExpected " << DisplayHex(expectation); } TEST(TokenizerHandlesUnicode, BMPCodes) { From ce32d9d68fa1da0ce46938c42bfb36073bc20391 Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Wed, 24 Aug 2022 15:49:52 -0700 Subject: [PATCH 02/12] Fix code generation for infinity default value on float/double fields. PiperOrigin-RevId: 469843901 --- upb/msg.c | 5 +++++ upb/msg_internal.h | 3 +++ upbc/protoc-gen-upb.cc | 36 ++++++++++++++++++++++++++++-------- 3 files changed, 36 insertions(+), 8 deletions(-) diff --git a/upb/msg.c b/upb/msg.c index ffc0f4f4d7..714cc79208 100644 --- a/upb/msg.c +++ b/upb/msg.c @@ -27,6 +27,8 @@ #include "upb/msg.h" +#include + #include "upb/msg_internal.h" // Must be last. @@ -309,3 +311,6 @@ bool _upb_mapsorter_pushmap(_upb_mapsorter* s, upb_FieldType key_type, qsort(&s->entries[sorted->start], map_size, sizeof(*s->entries), compar); return true; } + +const float kUpb_FltInfinity = INFINITY; +const double kUpb_Infinity = INFINITY; diff --git a/upb/msg_internal.h b/upb/msg_internal.h index a260b81842..8c18a86dca 100644 --- a/upb/msg_internal.h +++ b/upb/msg_internal.h @@ -62,6 +62,9 @@ UPB_INLINE uint64_t _upb_UInt64_FromULL(unsigned long long v) { return (uint64_t)v; } +extern const float kUpb_FltInfinity; +extern const double kUpb_Infinity; + /** upb_MiniTable *************************************************************/ /* upb_MiniTable represents the memory layout of a given upb_MessageDef. The diff --git a/upbc/protoc-gen-upb.cc b/upbc/protoc-gen-upb.cc index 68c578e865..6d588aefc4 100644 --- a/upbc/protoc-gen-upb.cc +++ b/upbc/protoc-gen-upb.cc @@ -57,12 +57,12 @@ inline std::vector FieldHotnessOrder( for (int i = 0; i < message->field_count(); i++) { fields.push_back(message->field(i)); } - std::sort(fields.begin(), fields.end(), - [](const google::protobuf::FieldDescriptor* a, - const google::protobuf::FieldDescriptor* b) { - return std::make_pair(!a->is_required(), a->number()) < - std::make_pair(!b->is_required(), b->number()); - }); + std::sort( + fields.begin(), fields.end(), + [](const google::protobuf::FieldDescriptor* a, const google::protobuf::FieldDescriptor* b) { + return std::make_pair(!a->is_required(), a->number()) < + std::make_pair(!b->is_required(), b->number()); + }); return fields; } @@ -190,6 +190,26 @@ bool HasNonZeroDefault(const protobuf::FieldDescriptor* field) { return false; } +std::string FloatToCLiteral(float value) { + if (value == std::numeric_limits::infinity()) { + return "kUpb_FltInfinity"; + } else if (value == -std::numeric_limits::infinity()) { + return "-kUpb_FltInfinity"; + } else { + return absl::StrCat(value); + } +} + +std::string DoubleToCLiteral(double value) { + if (value == std::numeric_limits::infinity()) { + return "kUpb_Infinity"; + } else if (value == -std::numeric_limits::infinity()) { + return "-kUpb_Infinity"; + } else { + return absl::StrCat(value); + } +} + std::string FieldDefault(const protobuf::FieldDescriptor* field) { switch (field->cpp_type()) { case protobuf::FieldDescriptor::CPPTYPE_MESSAGE: @@ -210,9 +230,9 @@ std::string FieldDefault(const protobuf::FieldDescriptor* field) { return absl::Substitute("_upb_UInt64_FromULL($0ull)", field->default_value_uint64()); case protobuf::FieldDescriptor::CPPTYPE_FLOAT: - return absl::StrCat(field->default_value_float()); + return FloatToCLiteral(field->default_value_float()); case protobuf::FieldDescriptor::CPPTYPE_DOUBLE: - return absl::StrCat(field->default_value_double()); + return DoubleToCLiteral(field->default_value_double()); case protobuf::FieldDescriptor::CPPTYPE_BOOL: return field->default_value_bool() ? "true" : "false"; case protobuf::FieldDescriptor::CPPTYPE_ENUM: From 4e979b8d13efa461e6399e08c4ee70bee38961aa Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Wed, 24 Aug 2022 16:31:04 -0700 Subject: [PATCH 03/12] Add support for dashes in proto file names. PiperOrigin-RevId: 469853045 --- upbc/common.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/upbc/common.cc b/upbc/common.cc index 9299eb0674..a63813948c 100644 --- a/upbc/common.cc +++ b/upbc/common.cc @@ -43,7 +43,7 @@ std::string StripExtension(absl::string_view fname) { } std::string ToCIdent(absl::string_view str) { - return absl::StrReplaceAll(str, {{".", "_"}, {"/", "_"}}); + return absl::StrReplaceAll(str, {{".", "_"}, {"/", "_"}, {"-", "_"}}); } std::string ToPreproc(absl::string_view str) { From 1135746e42ecbaf8aafbe790d985155c1be7aa63 Mon Sep 17 00:00:00 2001 From: Eric Salo Date: Thu, 25 Aug 2022 11:39:35 -0700 Subject: [PATCH 04/12] start replumbing def.c Give the def types their own array allocators and also implement some simple array constructors for the enum and enum value defs. PiperOrigin-RevId: 470042521 --- upb/def.c | 237 +++++++++++++++++++++++++++++++++--------------------- 1 file changed, 145 insertions(+), 92 deletions(-) diff --git a/upb/def.c b/upb/def.c index 4db6bdd9be..ee9f5b5c14 100644 --- a/upb/def.c +++ b/upb/def.c @@ -36,7 +36,7 @@ #include "upb/mini_table.h" #include "upb/reflection.h" -/* Must be last. */ +// Must be last. #include "upb/port_def.inc" typedef struct { @@ -377,6 +377,40 @@ static void assign_msg_wellknowntype(upb_MessageDef* m) { } } +typedef struct { + upb_DefPool* symtab; + upb_FileDef* file; /* File we are building. */ + upb_Arena* arena; /* Allocate defs here. */ + upb_Arena* tmp_arena; /* For temporary allocations. */ + const upb_MiniTable_File* layout; /* NULL if we should build layouts. */ + int enum_count; /* Count of enums built so far. */ + int msg_count; /* Count of messages built so far. */ + int ext_count; /* Count of extensions built so far. */ + upb_Status* status; /* Record errors here. */ + jmp_buf err; /* longjmp() on error. */ +} symtab_addctx; + +UPB_NORETURN UPB_NOINLINE UPB_PRINTF(2, 3) static void symtab_errf( + symtab_addctx* ctx, const char* fmt, ...) { + va_list argp; + va_start(argp, fmt); + upb_Status_VSetErrorFormat(ctx->status, fmt, argp); + va_end(argp); + UPB_LONGJMP(ctx->err, 1); +} + +UPB_NORETURN UPB_NOINLINE static void symtab_oomerr(symtab_addctx* ctx) { + upb_Status_setoom(ctx->status); + UPB_LONGJMP(ctx->err, 1); +} + +void* symtab_alloc(symtab_addctx* ctx, size_t bytes) { + if (bytes == 0) return NULL; + void* ret = upb_Arena_Malloc(ctx->arena, bytes); + if (!ret) symtab_oomerr(ctx); + return ret; +} + /* upb_EnumDef ****************************************************************/ const google_protobuf_EnumOptions* upb_EnumDef_Options(const upb_EnumDef* e) { @@ -479,8 +513,7 @@ uint32_t upb_EnumValueDef_Index(const upb_EnumValueDef* ev) { return ev - ev->parent->values; } -/* upb_ExtensionRange - * ***************************************************************/ +/* upb_ExtensionRange *********************************************************/ const google_protobuf_ExtensionRangeOptions* upb_ExtensionRange_Options( const upb_ExtensionRange* r) { @@ -497,6 +530,12 @@ int32_t upb_ExtensionRange_Start(const upb_ExtensionRange* e) { int32_t upb_ExtensionRange_End(const upb_ExtensionRange* e) { return e->end; } +// Allocate sufficient storage to contain an array of |n| extension ranges. +static upb_ExtensionRange* _upb_ExtensionRange_Alloc(symtab_addctx* ctx, + int n) { + return symtab_alloc(ctx, sizeof(upb_ExtensionRange) * n); +} + /* upb_FieldDef ***************************************************************/ const google_protobuf_FieldOptions* upb_FieldDef_Options( @@ -709,8 +748,12 @@ bool upb_FieldDef_checkdescriptortype(int32_t type) { return between(type, 1, 18); } -/* upb_MessageDef - * *****************************************************************/ +// Allocate sufficient storage to contain an array of |n| field defs. +static upb_FieldDef* _upb_FieldDef_Alloc(symtab_addctx* ctx, int n) { + return symtab_alloc(ctx, sizeof(upb_FieldDef) * n); +} + +/* upb_MessageDef *************************************************************/ const google_protobuf_MessageOptions* upb_MessageDef_Options( const upb_MessageDef* m) { @@ -879,6 +922,11 @@ upb_WellKnown upb_MessageDef_WellKnownType(const upb_MessageDef* m) { return m->well_known_type; } +// Allocate sufficient storage to contain an array of |n| message defs. +static upb_MessageDef* _upb_MessageDef_Alloc(symtab_addctx* ctx, int n) { + return symtab_alloc(ctx, sizeof(upb_MessageDef) * n); +} + /* upb_OneofDef ***************************************************************/ const google_protobuf_OneofOptions* upb_OneofDef_Options( @@ -930,6 +978,11 @@ const upb_FieldDef* upb_OneofDef_LookupNumber(const upb_OneofDef* o, : NULL; } +// Allocate sufficient storage to contain an array of |n| oneof defs. +static upb_OneofDef* _upb_OneofDef_Alloc(symtab_addctx* ctx, int n) { + return symtab_alloc(ctx, sizeof(upb_OneofDef) * n); +} + /* upb_FileDef ****************************************************************/ const google_protobuf_FileOptions* upb_FileDef_Options(const upb_FileDef* f) { @@ -1017,6 +1070,11 @@ const upb_ServiceDef* upb_FileDef_Service(const upb_FileDef* f, int i) { const upb_DefPool* upb_FileDef_Pool(const upb_FileDef* f) { return f->symtab; } +// Allocate sufficient storage to contain one file def. +static upb_FileDef* _upb_FileDef_Alloc(symtab_addctx* ctx) { + return symtab_alloc(ctx, sizeof(upb_FileDef)); +} + /* upb_MethodDef **************************************************************/ const google_protobuf_MethodOptions* upb_MethodDef_Options(const upb_MethodDef* m) { @@ -1057,6 +1115,11 @@ bool upb_MethodDef_ServerStreaming(const upb_MethodDef* m) { return m->server_streaming; } +// Allocate sufficient storage to contain an array of |n| method defs. +static upb_MethodDef* _upb_MethodDef_Alloc(symtab_addctx* ctx, int n) { + return symtab_alloc(ctx, sizeof(upb_MethodDef) * n); +} + /* upb_ServiceDef *************************************************************/ const google_protobuf_ServiceOptions* upb_ServiceDef_Options(const upb_ServiceDef* s) { @@ -1099,6 +1162,11 @@ const upb_MethodDef* upb_ServiceDef_FindMethodByName(const upb_ServiceDef* s, return NULL; } +// Allocate sufficient storage to contain an array of |n| service defs. +static upb_ServiceDef* _upb_ServiceDef_Alloc(symtab_addctx* ctx, int n) { + return symtab_alloc(ctx, sizeof(upb_ServiceDef) * n); +} + /* upb_DefPool ****************************************************************/ void upb_DefPool_Free(upb_DefPool* s) { @@ -1275,40 +1343,6 @@ const upb_FileDef* upb_DefPool_FindFileContainingSymbol(const upb_DefPool* s, symtab_oomerr(ctx); \ } -typedef struct { - upb_DefPool* symtab; - upb_FileDef* file; /* File we are building. */ - upb_Arena* arena; /* Allocate defs here. */ - upb_Arena* tmp_arena; /* For temporary allocations. */ - const upb_MiniTable_File* layout; /* NULL if we should build layouts. */ - int enum_count; /* Count of enums built so far. */ - int msg_count; /* Count of messages built so far. */ - int ext_count; /* Count of extensions built so far. */ - upb_Status* status; /* Record errors here. */ - jmp_buf err; /* longjmp() on error. */ -} symtab_addctx; - -UPB_NORETURN UPB_NOINLINE UPB_PRINTF(2, 3) static void symtab_errf( - symtab_addctx* ctx, const char* fmt, ...) { - va_list argp; - va_start(argp, fmt); - upb_Status_VSetErrorFormat(ctx->status, fmt, argp); - va_end(argp); - UPB_LONGJMP(ctx->err, 1); -} - -UPB_NORETURN UPB_NOINLINE static void symtab_oomerr(symtab_addctx* ctx) { - upb_Status_setoom(ctx->status); - UPB_LONGJMP(ctx->err, 1); -} - -void* symtab_alloc(symtab_addctx* ctx, size_t bytes) { - if (bytes == 0) return NULL; - void* ret = upb_Arena_Malloc(ctx->arena, bytes); - if (!ret) symtab_oomerr(ctx); - return ret; -} - // We want to copy the options verbatim into the destination options proto. // We use serialize+parse as our deep copy. #define SET_OPTIONS(target, desc_type, options_type, proto) \ @@ -2426,7 +2460,7 @@ static void create_service( methods = google_protobuf_ServiceDescriptorProto_method(svc_proto, &n); s->method_count = n; - s->methods = symtab_alloc(ctx, sizeof(*s->methods) * n); + s->methods = _upb_MethodDef_Alloc(ctx, n); SET_OPTIONS(s->opts, ServiceDescriptorProto, ServiceOptions, svc_proto); @@ -2487,42 +2521,59 @@ static upb_MiniTable_Enum* create_enumlayout(symtab_addctx* ctx, 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_EnumDef* e, upb_EnumValueDef* v) { upb_StringView name = google_protobuf_EnumValueDescriptorProto_name(val_proto); - upb_value v = upb_value_constptr(val); + upb_value val = upb_value_constptr(v); - val->parent = e; /* Must happen prior to symtab_add(). */ - val->full_name = makefullname(ctx, prefix, name); - val->number = google_protobuf_EnumValueDescriptorProto_number(val_proto); - symtab_add(ctx, val->full_name, pack_def(val, UPB_DEFTYPE_ENUMVAL)); + v->parent = e; // Must happen prior to symtab_add() + v->full_name = makefullname(ctx, prefix, name); + v->number = google_protobuf_EnumValueDescriptorProto_number(val_proto); + symtab_add(ctx, v->full_name, pack_def(v, UPB_DEFTYPE_ENUMVAL)); - SET_OPTIONS(val->opts, EnumValueDescriptorProto, EnumValueOptions, val_proto); + SET_OPTIONS(v->opts, EnumValueDescriptorProto, EnumValueOptions, val_proto); - if (i == 0 && e->file->syntax == kUpb_Syntax_Proto3 && val->number != 0) { - symtab_errf(ctx, "for proto3, the first enum value must be zero (%s)", - e->full_name); + CHK_OOM(upb_strtable_insert(&e->ntoi, name.data, name.size, val, ctx->arena)); + + // Multiple enumerators can have the same number, first one wins. + if (!upb_inttable_lookup(&e->iton, v->number, NULL)) { + CHK_OOM(upb_inttable_insert(&e->iton, v->number, val, ctx->arena)); } +} - CHK_OOM(upb_strtable_insert(&e->ntoi, name.data, name.size, v, ctx->arena)); +// Allocate and initialize an array of |n| enum value defs. +// TODO(b/243726666): This will eventually become the (only) public constructor. +static upb_EnumValueDef* _upb_EnumValueDefs_New( + symtab_addctx* ctx, const char* prefix, int n, + const google_protobuf_EnumValueDescriptorProto* const* protos, upb_EnumDef* e) { + upb_EnumValueDef* v = symtab_alloc(ctx, sizeof(upb_EnumValueDef) * n); - // Multiple enumerators can have the same number, first one wins. - if (!upb_inttable_lookup(&e->iton, val->number, NULL)) { - CHK_OOM(upb_inttable_insert(&e->iton, val->number, v, ctx->arena)); + bool is_sorted = true; + uint32_t previous = 0; + for (size_t i = 0; i < n; i++) { + create_enumvaldef(ctx, prefix, protos[i], e, &v[i]); + + const uint32_t current = v[i].number; + if (previous > current) is_sorted = false; + previous = current; } + e->is_sorted = is_sorted; + + if (ctx->file->syntax == kUpb_Syntax_Proto3 && n > 0 && v[0].number != 0) { + symtab_errf(ctx, "for proto3, the first enum value must be zero (%s)", + e->full_name); + } + + return v; } -static void create_enumdef( - symtab_addctx* ctx, const char* prefix, - const google_protobuf_EnumDescriptorProto* enum_proto, - const upb_MessageDef* containing_type, const upb_EnumDef* _e) { - upb_EnumDef* e = (upb_EnumDef*)_e; +static void create_enumdef(symtab_addctx* ctx, const char* prefix, + const google_protobuf_EnumDescriptorProto* enum_proto, + upb_EnumDef* e) { const google_protobuf_EnumValueDescriptorProto* const* values; upb_StringView name; - size_t i, n; + size_t n; - e->file = ctx->file; /* Must happen prior to symtab_add() */ - e->containing_type = containing_type; + e->file = ctx->file; // Must happen prior to symtab_add() name = google_protobuf_EnumDescriptorProto_name(enum_proto); check_ident(ctx, name, false); @@ -2535,9 +2586,8 @@ static void create_enumdef( CHK_OOM(upb_inttable_init(&e->iton, ctx->arena)); e->defaultval = 0; - e->is_sorted = true; e->value_count = n; - e->values = symtab_alloc(ctx, sizeof(*e->values) * n); + e->values = _upb_EnumValueDefs_New(ctx, prefix, n, values, e); if (n == 0) { symtab_errf(ctx, "enums must contain at least one value (%s)", @@ -2546,14 +2596,6 @@ static void create_enumdef( SET_OPTIONS(e->opts, EnumDescriptorProto, EnumOptions, enum_proto); - uint32_t previous = 0; - for (i = 0; i < n; i++) { - create_enumvaldef(ctx, prefix, values[i], e, i); - const uint32_t current = e->values[i].number; - if (previous > current) e->is_sorted = false; - previous = current; - } - upb_inttable_compact(&e->iton, ctx->arena); if (e->file->syntax == kUpb_Syntax_Proto2) { @@ -2570,6 +2612,24 @@ static void create_enumdef( } } +// Allocate and initialize an array of |n| enum defs. +// TODO(b/243726666): This will eventually become the (only) public constructor. +static upb_EnumDef* _upb_EnumDefs_New( + symtab_addctx* ctx, int n, const google_protobuf_EnumDescriptorProto* const* protos, + const upb_MessageDef* containing_type) { + // If a containing type is defined then get the full name from that. + // Otherwise use the package name from the file def. + const char* name = + containing_type ? containing_type->full_name : ctx->file->package; + + upb_EnumDef* e = symtab_alloc(ctx, sizeof(upb_EnumDef) * n); + for (size_t i = 0; i < n; i++) { + create_enumdef(ctx, name, protos[i], &e[i]); + e[i].containing_type = containing_type; + } + return e; +} + static void msgdef_create_nested( symtab_addctx* ctx, const google_protobuf_DescriptorProto* msg_proto, upb_MessageDef* m); @@ -2616,20 +2676,20 @@ static void create_msgdef(symtab_addctx* ctx, const char* prefix, SET_OPTIONS(m->opts, DescriptorProto, MessageOptions, msg_proto); m->oneof_count = n_oneof; - m->oneofs = symtab_alloc(ctx, sizeof(*m->oneofs) * n_oneof); + m->oneofs = _upb_OneofDef_Alloc(ctx, n_oneof); for (i = 0; i < n_oneof; i++) { create_oneofdef(ctx, m, oneofs[i], &m->oneofs[i]); } m->field_count = n_field; - m->fields = symtab_alloc(ctx, sizeof(*m->fields) * n_field); + m->fields = _upb_FieldDef_Alloc(ctx, n_field); for (i = 0; i < n_field; i++) { create_fielddef(ctx, m->full_name, m, fields[i], &m->fields[i], /* is_extension= */ false); } m->ext_range_count = n_ext_range; - m->ext_ranges = symtab_alloc(ctx, sizeof(*m->ext_ranges) * n_ext_range); + m->ext_ranges = _upb_ExtensionRange_Alloc(ctx, n_ext_range); for (i = 0; i < n_ext_range; i++) { const google_protobuf_DescriptorProto_ExtensionRange* r = ext_ranges[i]; upb_ExtensionRange* r_def = (upb_ExtensionRange*)&m->ext_ranges[i]; @@ -2668,16 +2728,12 @@ static void msgdef_create_nested( const google_protobuf_EnumDescriptorProto* const* enums = google_protobuf_DescriptorProto_enum_type(msg_proto, &n); m->nested_enum_count = n; - m->nested_enums = symtab_alloc(ctx, sizeof(*m->nested_enums) * n); - for (size_t i = 0; i < n; i++) { - m->nested_enum_count = i + 1; - create_enumdef(ctx, m->full_name, enums[i], m, &m->nested_enums[i]); - } + m->nested_enums = _upb_EnumDefs_New(ctx, n, enums, m); const google_protobuf_FieldDescriptorProto* const* exts = google_protobuf_DescriptorProto_extension(msg_proto, &n); m->nested_ext_count = n; - m->nested_exts = symtab_alloc(ctx, sizeof(*m->nested_exts) * n); + m->nested_exts = _upb_FieldDef_Alloc(ctx, n); for (size_t i = 0; i < n; i++) { create_fielddef(ctx, m->full_name, m, exts[i], &m->nested_exts[i], /* is_extension= */ true); @@ -2687,7 +2743,7 @@ static void msgdef_create_nested( const google_protobuf_DescriptorProto* const* msgs = google_protobuf_DescriptorProto_nested_type(msg_proto, &n); m->nested_msg_count = n; - m->nested_msgs = symtab_alloc(ctx, sizeof(*m->nested_msgs) * n); + m->nested_msgs = _upb_MessageDef_Alloc(ctx, n); for (size_t i = 0; i < n; i++) { create_msgdef(ctx, m->full_name, msgs[i], m, &m->nested_msgs[i]); } @@ -2975,18 +3031,15 @@ static void build_filedef( mutable_weak_deps[i] = weak_deps[i]; } - /* Create enums. */ + // Create enums. enums = google_protobuf_FileDescriptorProto_enum_type(file_proto, &n); file->top_lvl_enum_count = n; - file->top_lvl_enums = symtab_alloc(ctx, sizeof(*file->top_lvl_enums) * n); - for (i = 0; i < n; i++) { - create_enumdef(ctx, file->package, enums[i], NULL, &file->top_lvl_enums[i]); - } + file->top_lvl_enums = _upb_EnumDefs_New(ctx, n, enums, NULL); /* Create extensions. */ exts = google_protobuf_FileDescriptorProto_extension(file_proto, &n); file->top_lvl_ext_count = n; - file->top_lvl_exts = symtab_alloc(ctx, sizeof(*file->top_lvl_exts) * n); + file->top_lvl_exts = _upb_FieldDef_Alloc(ctx, n); for (i = 0; i < n; i++) { create_fielddef(ctx, file->package, NULL, exts[i], &file->top_lvl_exts[i], /* is_extension= */ true); @@ -2996,7 +3049,7 @@ static void build_filedef( /* Create messages. */ msgs = google_protobuf_FileDescriptorProto_message_type(file_proto, &n); file->top_lvl_msg_count = n; - file->top_lvl_msgs = symtab_alloc(ctx, sizeof(*file->top_lvl_msgs) * n); + file->top_lvl_msgs = _upb_MessageDef_Alloc(ctx, n); for (i = 0; i < n; i++) { create_msgdef(ctx, file->package, msgs[i], NULL, &file->top_lvl_msgs[i]); } @@ -3004,7 +3057,7 @@ static void build_filedef( /* Create services. */ services = google_protobuf_FileDescriptorProto_service(file_proto, &n); file->service_count = n; - file->services = symtab_alloc(ctx, sizeof(*file->services) * n); + file->services = _upb_ServiceDef_Alloc(ctx, n); for (i = 0; i < n; i++) { create_service(ctx, services[i], &file->services[i]); ((upb_ServiceDef*)&file->services[i])->index = i; @@ -3104,7 +3157,7 @@ static const upb_FileDef* _upb_DefPool_AddFile( ctx.file = NULL; } } else { - ctx.file = symtab_alloc(&ctx, sizeof(*ctx.file)); + ctx.file = _upb_FileDef_Alloc(&ctx); build_filedef(&ctx, ctx.file, file_proto); upb_strtable_insert(&s->files, name.data, name.size, pack_def(ctx.file, UPB_DEFTYPE_FILE), ctx.arena); From 5b46a55a46e31bac36e9823ef0a1b66f05af47ad Mon Sep 17 00:00:00 2001 From: Eric Salo Date: Fri, 26 Aug 2022 13:30:52 -0700 Subject: [PATCH 05/12] try to be better about using accessors in def.c There are many places within def.c where a function which implements a method for struct A ends up directly accessing fields within struct B even though accessor functions for these fields are defined. So, this is a first pass at trying to clean that up a bit. Yes, we are adding function calls to a lot of code paths by doing this but in the unlikely event that this adds unacceptable overhead we can deal with it then. PiperOrigin-RevId: 470321129 --- upb/def.c | 59 ++++++++++++++++++++++++++++++++----------------------- 1 file changed, 34 insertions(+), 25 deletions(-) diff --git a/upb/def.c b/upb/def.c index ee9f5b5c14..5c53888faa 100644 --- a/upb/def.c +++ b/upb/def.c @@ -323,11 +323,9 @@ uint32_t field_rank(const upb_FieldDef* f) { } 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; + const uint32_t A = upb_EnumValueDef_Number(*(const upb_EnumValueDef**)a); + const uint32_t B = upb_EnumValueDef_Number(*(const upb_EnumValueDef**)b); + return (A < B) ? -1 : (A > B); } static void upb_Status_setoom(upb_Status* status) { @@ -686,7 +684,8 @@ const upb_MiniTable_Field* upb_FieldDef_MiniTable(const upb_FieldDef* f) { const upb_MiniTable_Extension* _upb_FieldDef_ExtensionMiniTable( const upb_FieldDef* f) { UPB_ASSERT(upb_FieldDef_IsExtension(f)); - return f->file->ext_layouts[f->layout_index]; + const upb_FileDef* file = upb_FieldDef_File(f); + return file->ext_layouts[f->layout_index]; } bool _upb_FieldDef_IsProto3Optional(const upb_FieldDef* f) { @@ -732,8 +731,9 @@ bool upb_FieldDef_HasSubDef(const upb_FieldDef* f) { bool upb_FieldDef_HasPresence(const upb_FieldDef* f) { if (upb_FieldDef_IsRepeated(f)) return false; + const upb_FileDef* file = upb_FieldDef_File(f); return upb_FieldDef_IsSubMessage(f) || upb_FieldDef_ContainingOneof(f) || - f->file->syntax == kUpb_Syntax_Proto2; + upb_FileDef_Syntax(file) == kUpb_Syntax_Proto2; } static bool between(int32_t x, int32_t low, int32_t high) { @@ -781,7 +781,7 @@ const char* upb_MessageDef_Name(const upb_MessageDef* m) { } upb_Syntax upb_MessageDef_Syntax(const upb_MessageDef* m) { - return m->file->syntax; + return upb_FileDef_Syntax(m->file); } const upb_FieldDef* upb_MessageDef_FindFieldByNumber(const upb_MessageDef* m, @@ -1467,16 +1467,22 @@ static uint8_t map_descriptortype(const upb_FieldDef* f) { uint8_t type = upb_FieldDef_Type(f); /* See TableDescriptorType() in upbc/generator.cc for details and * rationale of these exceptions. */ - if (type == kUpb_FieldType_String && f->file->syntax == kUpb_Syntax_Proto2) { - return kUpb_FieldType_Bytes; - } else if (type == kUpb_FieldType_Enum && - (f->sub.enumdef->file->syntax == kUpb_Syntax_Proto3 || - UPB_TREAT_PROTO2_ENUMS_LIKE_PROTO3 || - // TODO(https://github.com/protocolbuffers/upb/issues/541): - // fix map enum values to check for unknown enum values and put - // them in the unknown field set. - upb_MessageDef_IsMapEntry(upb_FieldDef_ContainingType(f)))) { - return kUpb_FieldType_Int32; + if (type == kUpb_FieldType_String) { + const upb_FileDef* file = upb_FieldDef_File(f); + const upb_Syntax syntax = upb_FileDef_Syntax(file); + + if (syntax == kUpb_Syntax_Proto2) return kUpb_FieldType_Bytes; + } else if (type == kUpb_FieldType_Enum) { + const upb_FileDef* file = upb_EnumDef_File(f->sub.enumdef); + const upb_Syntax syntax = upb_FileDef_Syntax(file); + + if (syntax == kUpb_Syntax_Proto3 || UPB_TREAT_PROTO2_ENUMS_LIKE_PROTO3 || + // TODO(https://github.com/protocolbuffers/upb/issues/541): + // fix map enum values to check for unknown enum values and put + // them in the unknown field set. + upb_MessageDef_IsMapEntry(upb_FieldDef_ContainingType(f))) { + return kUpb_FieldType_Int32; + } } return type; } @@ -1541,12 +1547,12 @@ static void make_layout(symtab_addctx* ctx, const upb_MessageDef* m) { /* Count sub-messages. */ for (size_t i = 0; i < field_count; i++) { - const upb_FieldDef* f = &m->fields[i]; + const upb_FieldDef* f = upb_MessageDef_Field(m, i); if (upb_FieldDef_IsSubMessage(f)) { sublayout_count++; } if (upb_FieldDef_CType(f) == kUpb_CType_Enum && - f->sub.enumdef->file->syntax == kUpb_Syntax_Proto2) { + upb_FileDef_Syntax(f->sub.enumdef->file) == kUpb_Syntax_Proto2) { sublayout_count++; } } @@ -2235,8 +2241,10 @@ static void set_default_default(symtab_addctx* ctx, upb_FieldDef* f) { case kUpb_CType_Bool: f->defaultval.boolean = false; break; - case kUpb_CType_Enum: - f->defaultval.sint = f->sub.enumdef->values[0].number; + case kUpb_CType_Enum: { + const upb_EnumValueDef* v = upb_EnumDef_Value(f->sub.enumdef, 0); + f->defaultval.sint = upb_EnumValueDef_Number(v); + } case kUpb_CType_Message: break; } @@ -2558,7 +2566,8 @@ static upb_EnumValueDef* _upb_EnumValueDefs_New( } e->is_sorted = is_sorted; - if (ctx->file->syntax == kUpb_Syntax_Proto3 && n > 0 && v[0].number != 0) { + if (upb_FileDef_Syntax(ctx->file) == kUpb_Syntax_Proto3 && n > 0 && + v[0].number != 0) { symtab_errf(ctx, "for proto3, the first enum value must be zero (%s)", e->full_name); } @@ -2598,7 +2607,7 @@ static void create_enumdef(symtab_addctx* ctx, const char* prefix, upb_inttable_compact(&e->iton, ctx->arena); - if (e->file->syntax == kUpb_Syntax_Proto2) { + if (upb_FileDef_Syntax(e->file) == kUpb_Syntax_Proto2) { if (ctx->layout) { UPB_ASSERT(ctx->enum_count < ctx->layout->enum_count); e->layout = ctx->layout->enums[ctx->enum_count++]; @@ -2850,7 +2859,7 @@ static void resolve_default( upb_StringView defaultval = google_protobuf_FieldDescriptorProto_default_value(field_proto); - if (f->file->syntax == kUpb_Syntax_Proto3) { + if (upb_FileDef_Syntax(f->file) == kUpb_Syntax_Proto3) { symtab_errf(ctx, "proto3 fields cannot have explicit defaults (%s)", f->full_name); } From 52c9b986920b2eeb7a30cece39126c1e45339ed7 Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Fri, 26 Aug 2022 14:35:38 -0700 Subject: [PATCH 06/12] Resolve field name/accessor name conflicts. PiperOrigin-RevId: 470335824 --- upb/msg_test.proto | 5 ++ upbc/BUILD | 19 +++++++ upbc/names.cc | 81 +++++++++++++++++++++++++++ upbc/names.h | 51 +++++++++++++++++ upbc/protoc-gen-upb.cc | 123 +++++++++++++++++++++++++---------------- 5 files changed, 230 insertions(+), 49 deletions(-) create mode 100644 upbc/names.cc create mode 100644 upbc/names.h diff --git a/upb/msg_test.proto b/upb/msg_test.proto index 237f132381..3b45d13109 100644 --- a/upb/msg_test.proto +++ b/upb/msg_test.proto @@ -193,3 +193,8 @@ message TestMapFieldExtra { } map map_field = 1; } + +message TestNameConflict { + map map_field = 1; + optional bool clear_map_field = 2; +} diff --git a/upbc/BUILD b/upbc/BUILD index a1c6e2413e..c77740789c 100644 --- a/upbc/BUILD +++ b/upbc/BUILD @@ -114,6 +114,24 @@ cc_library( visibility = ["//third_party/upb/protos_generator:__pkg__"], ) +cc_library( + name = "names", + srcs = [ + "names.cc", + ], + hdrs = [ + "names.h", + ], + copts = UPB_DEFAULT_CPPOPTS, + visibility = ["//third_party/upb/protos_generator:__pkg__"], + deps = [ + "@com_google_absl//absl/container:flat_hash_map", + "@com_google_absl//absl/strings", + "@com_google_protobuf//:protobuf", + "@com_google_protobuf//src/google/protobuf/compiler:code_generator", + ], +) + cc_binary( name = "protoc-gen-upb", srcs = ["protoc-gen-upb.cc"], @@ -122,6 +140,7 @@ cc_binary( deps = [ ":common", ":file_layout", + ":names", "//:mini_table", "//:port", "//:upb", diff --git a/upbc/names.cc b/upbc/names.cc new file mode 100644 index 0000000000..a2a98dfe0a --- /dev/null +++ b/upbc/names.cc @@ -0,0 +1,81 @@ +/* + * Copyright (c) 2007-2021, Google LLC + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * * Neither the name of Google LLC nor the + * names of its contributors may be used to endorse or promote products + * derived from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL Google LLC BE LIABLE FOR ANY DIRECT, + * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +#include "upbc/names.h" + +#include + +#include "google/protobuf/descriptor.h" +#include "absl/strings/match.h" + +namespace upbc { + +namespace protobuf = ::google::protobuf; + +// List of generated accessor prefixes to check against. +// Example: +// optional repeated string phase = 236; +// optional bool clear_phase = 237; +static constexpr absl::string_view kAccessorPrefixes[] = { + "clear_", + "delete_", + "add_", + "resize_", +}; + +std::string ResolveFieldName(const protobuf::FieldDescriptor* field, + const NameToFieldDescriptorMap& field_names) { + absl::string_view field_name = field->name(); + for (const auto prefix : kAccessorPrefixes) { + // If field name starts with a prefix such as clear_ and the proto + // contains a field name with trailing end, depending on type of field + // (repeated, map, message) we have a conflict to resolve. + if (absl::StartsWith(field_name, prefix)) { + auto match = field_names.find(field_name.substr(prefix.size())); + if (match != field_names.end()) { + const auto* candidate = match->second; + if (candidate->is_repeated() || candidate->is_map()) { + return absl::StrCat(field_name, "_"); + } + } + } + } + return std::string(field_name); +} + +// Returns field map by name to use for conflict checks. +NameToFieldDescriptorMap CreateFieldNameMap( + const protobuf::Descriptor* message) { + NameToFieldDescriptorMap field_names; + for (int i = 0; i < message->field_count(); i++) { + const protobuf::FieldDescriptor* field = message->field(i); + field_names.emplace(field->name(), field); + } + return field_names; +} + +} // namespace upbc diff --git a/upbc/names.h b/upbc/names.h new file mode 100644 index 0000000000..4f3ba06ed5 --- /dev/null +++ b/upbc/names.h @@ -0,0 +1,51 @@ +/* + * Copyright (c) 2009-2021, Google LLC + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * * Neither the name of Google LLC nor the + * names of its contributors may be used to endorse or promote products + * derived from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL Google LLC BE LIABLE FOR ANY DIRECT, + * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +#ifndef UPB_PROTOS_GENERATOR_NAMES_H +#define UPB_PROTOS_GENERATOR_NAMES_H + +#include + +#include "google/protobuf/descriptor.h" +#include "absl/container/flat_hash_map.h" + +namespace upbc { + +using NameToFieldDescriptorMap = + absl::flat_hash_map; + +// Returns field name by resolving naming conflicts across +// proto field names (such as clear_ prefixes). +std::string ResolveFieldName(const google::protobuf::FieldDescriptor* field, + const NameToFieldDescriptorMap& field_names); + +// Returns field map by name to use for conflict checks. +NameToFieldDescriptorMap CreateFieldNameMap(const google::protobuf::Descriptor* message); + +} // namespace upbc + +#endif // UPB_PROTOS_GENERATOR_NAMES_H diff --git a/upbc/protoc-gen-upb.cc b/upbc/protoc-gen-upb.cc index 6d588aefc4..b418670fed 100644 --- a/upbc/protoc-gen-upb.cc +++ b/upbc/protoc-gen-upb.cc @@ -38,6 +38,7 @@ #include "upb/upb.hpp" #include "upbc/common.h" #include "upbc/file_layout.h" +#include "upbc/names.h" // Must be last. #include "upb/port_def.inc" @@ -54,7 +55,9 @@ namespace protobuf = ::google::protobuf; inline std::vector FieldHotnessOrder( const google::protobuf::Descriptor* message) { std::vector fields; - for (int i = 0; i < message->field_count(); i++) { + size_t field_count = message->field_count(); + fields.reserve(field_count); + for (size_t i = 0; i < field_count; i++) { fields.push_back(message->field(i)); } std::sort( @@ -400,7 +403,9 @@ void GenerateOneofInHeader(const protobuf::OneofDescriptor* oneof, void GenerateHazzer(const protobuf::FieldDescriptor* field, const FileLayout& layout, absl::string_view msg_name, + const NameToFieldDescriptorMap& field_names, Output& output) { + std::string resolved_name = ResolveFieldName(field, field_names); if (layout.HasHasbit(field)) { output( R"cc( @@ -408,7 +413,7 @@ void GenerateHazzer(const protobuf::FieldDescriptor* field, return _upb_hasbit(msg, $2); } )cc", - msg_name, field->name(), layout.GetHasbitIndex(field)); + msg_name, resolved_name, layout.GetHasbitIndex(field)); } else if (field->real_containing_oneof()) { output( R"cc( @@ -416,7 +421,7 @@ void GenerateHazzer(const protobuf::FieldDescriptor* field, return _upb_getoneofcase(msg, $2) == $3; } )cc", - msg_name, field->name(), + msg_name, resolved_name, layout.GetOneofCaseOffset(field->real_containing_oneof()), field->number()); } else if (field->message_type()) { @@ -426,19 +431,20 @@ void GenerateHazzer(const protobuf::FieldDescriptor* field, return _upb_has_submsg_nohasbit(msg, $2); } )cc", - msg_name, field->name(), layout.GetFieldOffset(field)); + msg_name, resolved_name, layout.GetFieldOffset(field)); } } void GenerateClear(const protobuf::FieldDescriptor* field, const FileLayout& layout, absl::string_view msg_name, + const NameToFieldDescriptorMap& field_names, Output& output) { if (field == field->containing_type()->map_key() || field == field->containing_type()->map_value()) { // Cannot be cleared. return; } - + std::string resolved_name = ResolveFieldName(field, field_names); if (field->real_containing_oneof()) { const protobuf::OneofDescriptor* oneof = field->real_containing_oneof(); std::string oneof_fullname = ToCIdent(oneof->full_name()); @@ -452,7 +458,7 @@ void GenerateClear(const protobuf::FieldDescriptor* field, UPB_WRITE_ONEOF(msg, $2, $3, $7, $4, $6_NOT_SET); } )cc", - msg_name, field->name(), CType(field), layout.GetFieldOffset(field), + msg_name, resolved_name, CType(field), layout.GetFieldOffset(field), layout.GetOneofCaseOffset(field->real_containing_oneof()), field->number(), oneof_fullname, default_value); } else { @@ -465,7 +471,7 @@ void GenerateClear(const protobuf::FieldDescriptor* field, _upb_clearhas(msg, $3); } )cc", - msg_name, field->name(), layout.GetFieldOffset(field), + msg_name, resolved_name, layout.GetFieldOffset(field), layout.GetHasbitIndex(field)); } else { output( @@ -474,7 +480,7 @@ void GenerateClear(const protobuf::FieldDescriptor* field, *UPB_PTR_AT(msg, $2, const upb_Message*) = NULL; } )cc", - msg_name, field->name(), layout.GetFieldOffset(field)); + msg_name, resolved_name, layout.GetFieldOffset(field)); } } else if (layout.HasHasbit(field)) { if (field->cpp_type() == protobuf::FieldDescriptor::CPPTYPE_STRING) { @@ -485,7 +491,7 @@ void GenerateClear(const protobuf::FieldDescriptor* field, _upb_clearhas(msg, $4); } )cc", - msg_name, field->name(), CType(field), layout.GetFieldOffset(field), + msg_name, resolved_name, CType(field), layout.GetFieldOffset(field), layout.GetHasbitIndex(field)); } else { output( @@ -495,7 +501,7 @@ void GenerateClear(const protobuf::FieldDescriptor* field, _upb_clearhas(msg, $4); } )cc", - msg_name, field->name(), CType(field), layout.GetFieldOffset(field), + msg_name, resolved_name, CType(field), layout.GetFieldOffset(field), layout.GetHasbitIndex(field)); } } else { @@ -506,7 +512,7 @@ void GenerateClear(const protobuf::FieldDescriptor* field, *UPB_PTR_AT(msg, $3, $2) = upb_StringView_FromDataAndSize(NULL, 0); } )cc", - msg_name, field->name(), CType(field), + msg_name, resolved_name, CType(field), layout.GetFieldOffset(field)); } else { output( @@ -515,7 +521,7 @@ void GenerateClear(const protobuf::FieldDescriptor* field, *UPB_PTR_AT(msg, $3, $2) = 0; } )cc", - msg_name, field->name(), CType(field), layout.GetFieldOffset(field), + msg_name, resolved_name, CType(field), layout.GetFieldOffset(field), layout.GetHasbitIndex(field)); } } @@ -524,7 +530,9 @@ void GenerateClear(const protobuf::FieldDescriptor* field, void GenerateRepeatedClear(const protobuf::FieldDescriptor* field, const FileLayout& layout, absl::string_view msg_name, + const NameToFieldDescriptorMap& field_names, Output& output) { + std::string resolved_name = ResolveFieldName(field, field_names); if (layout.HasHasbit(field)) { output( R"cc( @@ -533,7 +541,7 @@ void GenerateRepeatedClear(const protobuf::FieldDescriptor* field, _upb_clearhas(msg, $4); } )cc", - msg_name, field->name(), layout.GetFieldOffset(field), + msg_name, resolved_name, layout.GetFieldOffset(field), layout.GetHasbitIndex(field)); } else { output( @@ -542,30 +550,32 @@ void GenerateRepeatedClear(const protobuf::FieldDescriptor* field, _upb_array_detach(msg, $2); } )cc", - msg_name, field->name(), layout.GetFieldOffset(field)); + msg_name, resolved_name, layout.GetFieldOffset(field)); } } void GenerateMapGetters(const protobuf::FieldDescriptor* field, const FileLayout& layout, absl::string_view msg_name, + const NameToFieldDescriptorMap& field_names, Output& output) { const protobuf::Descriptor* entry = field->message_type(); const protobuf::FieldDescriptor* key = entry->FindFieldByNumber(1); const protobuf::FieldDescriptor* val = entry->FindFieldByNumber(2); + std::string resolved_name = ResolveFieldName(field, field_names); output( R"cc( UPB_INLINE size_t $0_$1_size(const $0* msg) { return _upb_msg_map_size(msg, $2); } )cc", - msg_name, field->name(), layout.GetFieldOffset(field)); + msg_name, resolved_name, layout.GetFieldOffset(field)); output( R"cc( UPB_INLINE bool $0_$1_get(const $0* msg, $2 key, $3* val) { return _upb_msg_map_get(msg, $4, &key, $5, val, $6); } )cc", - msg_name, field->name(), CType(key), CType(val), + msg_name, resolved_name, CType(key), CType(val), layout.GetFieldOffset(field), key->cpp_type() == protobuf::FieldDescriptor::CPPTYPE_STRING ? "0" @@ -579,7 +589,7 @@ void GenerateMapGetters(const protobuf::FieldDescriptor* field, return ($0)_upb_msg_map_next(msg, $3, iter); } )cc", - CTypeConst(field), msg_name, field->name(), layout.GetFieldOffset(field)); + CTypeConst(field), msg_name, resolved_name, layout.GetFieldOffset(field)); } void GenerateMapEntryGetters(const protobuf::FieldDescriptor* field, @@ -600,18 +610,22 @@ void GenerateMapEntryGetters(const protobuf::FieldDescriptor* field, void GenerateRepeatedGetters(const protobuf::FieldDescriptor* field, const FileLayout& layout, - absl::string_view msg_name, Output& output) { + absl::string_view msg_name, + const NameToFieldDescriptorMap& field_names, + 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); } )cc", - CTypeConst(field), msg_name, field->name(), layout.GetFieldOffset(field)); + CTypeConst(field), msg_name, ResolveFieldName(field, field_names), + layout.GetFieldOffset(field)); } void GenerateOneofGetters(const protobuf::FieldDescriptor* field, const FileLayout& layout, absl::string_view msg_name, + const NameToFieldDescriptorMap& field_names, Output& output) { output( R"cc( @@ -619,14 +633,17 @@ void GenerateOneofGetters(const protobuf::FieldDescriptor* field, return UPB_READ_ONEOF(msg, $0, $3, $4, $5, $6); } )cc", - CTypeConst(field), msg_name, field->name(), layout.GetFieldOffset(field), + CTypeConst(field), msg_name, ResolveFieldName(field, field_names), + layout.GetFieldOffset(field), layout.GetOneofCaseOffset(field->real_containing_oneof()), field->number(), FieldDefault(field)); } void GenerateScalarGetters(const protobuf::FieldDescriptor* field, const FileLayout& layout, absl::string_view msg_name, + const NameToFieldDescriptorMap& field_names, Output& output) { + std::string resolved_name = ResolveFieldName(field, field_names); if (HasNonZeroDefault(field)) { output( R"cc( @@ -634,7 +651,7 @@ void GenerateScalarGetters(const protobuf::FieldDescriptor* field, return $1_has_$2(msg) ? *UPB_PTR_AT(msg, $3, $0) : $4; } )cc", - CTypeConst(field), msg_name, field->name(), + CTypeConst(field), msg_name, resolved_name, layout.GetFieldOffset(field), FieldDefault(field)); } else { output( @@ -643,45 +660,48 @@ void GenerateScalarGetters(const protobuf::FieldDescriptor* field, return *UPB_PTR_AT(msg, $3, $0); } )cc", - CTypeConst(field), msg_name, field->name(), + CTypeConst(field), msg_name, resolved_name, layout.GetFieldOffset(field)); } } void GenerateGetters(const protobuf::FieldDescriptor* field, const FileLayout& layout, absl::string_view msg_name, + const NameToFieldDescriptorMap& field_names, Output& output) { if (field->is_map()) { - GenerateMapGetters(field, layout, msg_name, output); + GenerateMapGetters(field, layout, msg_name, field_names, output); } else if (field->containing_type()->options().map_entry()) { GenerateMapEntryGetters(field, msg_name, output); } else if (field->is_repeated()) { - GenerateRepeatedGetters(field, layout, msg_name, output); + GenerateRepeatedGetters(field, layout, msg_name, field_names, output); } else if (field->real_containing_oneof()) { - GenerateOneofGetters(field, layout, msg_name, output); + GenerateOneofGetters(field, layout, msg_name, field_names, output); } else { - GenerateScalarGetters(field, layout, msg_name, output); + GenerateScalarGetters(field, layout, msg_name, field_names, output); } } void GenerateMapSetters(const protobuf::FieldDescriptor* field, const FileLayout& layout, absl::string_view msg_name, + const NameToFieldDescriptorMap& field_names, Output& output) { const protobuf::Descriptor* entry = field->message_type(); const protobuf::FieldDescriptor* key = entry->FindFieldByNumber(1); const protobuf::FieldDescriptor* val = entry->FindFieldByNumber(2); + std::string resolved_name = ResolveFieldName(field, field_names); output( R"cc( UPB_INLINE void $0_$1_clear($0* msg) { _upb_msg_map_clear(msg, $2); } )cc", - msg_name, field->name(), layout.GetFieldOffset(field)); + msg_name, resolved_name, layout.GetFieldOffset(field)); output( R"cc( UPB_INLINE bool $0_$1_set($0* msg, $2 key, $3 val, upb_Arena* a) { return _upb_msg_map_set(msg, $4, &key, $5, &val, $6, a); } )cc", - msg_name, field->name(), CType(key), CType(val), + msg_name, resolved_name, CType(key), CType(val), layout.GetFieldOffset(field), key->cpp_type() == protobuf::FieldDescriptor::CPPTYPE_STRING ? "0" @@ -695,7 +715,7 @@ void GenerateMapSetters(const protobuf::FieldDescriptor* field, return _upb_msg_map_delete(msg, $3, &key, $4); } )cc", - msg_name, field->name(), CType(key), layout.GetFieldOffset(field), + msg_name, resolved_name, CType(key), layout.GetFieldOffset(field), key->cpp_type() == protobuf::FieldDescriptor::CPPTYPE_STRING ? "0" : "sizeof(key)"); @@ -705,26 +725,29 @@ void GenerateMapSetters(const protobuf::FieldDescriptor* field, return ($0)_upb_msg_map_next(msg, $3, iter); } )cc", - CType(field), msg_name, field->name(), layout.GetFieldOffset(field)); + CType(field), msg_name, resolved_name, layout.GetFieldOffset(field)); } void GenerateRepeatedSetters(const protobuf::FieldDescriptor* field, const FileLayout& layout, - absl::string_view msg_name, Output& output) { + absl::string_view msg_name, + const NameToFieldDescriptorMap& field_names, + Output& output) { + 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); } )cc", - CType(field), msg_name, field->name(), layout.GetFieldOffset(field)); + CType(field), msg_name, resolved_name, layout.GetFieldOffset(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); } )cc", - CType(field), msg_name, field->name(), layout.GetFieldOffset(field), + CType(field), msg_name, resolved_name, layout.GetFieldOffset(field), SizeLg2(field)); if (field->cpp_type() == protobuf::FieldDescriptor::CPPTYPE_MESSAGE) { output( @@ -736,7 +759,7 @@ void GenerateRepeatedSetters(const protobuf::FieldDescriptor* field, return sub; } )cc", - MessageName(field->message_type()), msg_name, field->name(), + MessageName(field->message_type()), msg_name, resolved_name, MessageInit(field->message_type()), layout.GetFieldOffset(field), SizeLg2(field)); } else { @@ -746,23 +769,25 @@ void GenerateRepeatedSetters(const protobuf::FieldDescriptor* field, return _upb_Array_Append_accessor2(msg, $3, $4, &val, arena); } )cc", - CType(field), msg_name, field->name(), layout.GetFieldOffset(field), + CType(field), msg_name, resolved_name, layout.GetFieldOffset(field), SizeLg2(field)); } } void GenerateNonRepeatedSetters(const protobuf::FieldDescriptor* field, const FileLayout& layout, - absl::string_view msg_name, Output& output) { + absl::string_view msg_name, + const NameToFieldDescriptorMap& field_names, + Output& output) { if (field == field->containing_type()->map_key()) { // Key cannot be mutated. return; } - + std::string resolved_name = ResolveFieldName(field, field_names); // The common function signature for all setters. Varying // implementations follow. output("UPB_INLINE void $0_set_$1($0 *msg, $2 value) {\n", msg_name, - field->name(), CType(field)); + resolved_name, CType(field)); if (field == field->containing_type()->map_value()) { output( @@ -804,20 +829,21 @@ void GenerateNonRepeatedSetters(const protobuf::FieldDescriptor* field, return sub; } )cc", - MessageName(field->message_type()), msg_name, field->name(), + MessageName(field->message_type()), msg_name, resolved_name, MessageInit(field->message_type())); } } void GenerateSetters(const protobuf::FieldDescriptor* field, const FileLayout& layout, absl::string_view msg_name, + const NameToFieldDescriptorMap& field_names, Output& output) { if (field->is_map()) { - GenerateMapSetters(field, layout, msg_name, output); + GenerateMapSetters(field, layout, msg_name, field_names, output); } else if (field->is_repeated()) { - GenerateRepeatedSetters(field, layout, msg_name, output); + GenerateRepeatedSetters(field, layout, msg_name, field_names, output); } else { - GenerateNonRepeatedSetters(field, layout, msg_name, output); + GenerateNonRepeatedSetters(field, layout, msg_name, field_names, output); } } @@ -825,7 +851,6 @@ void GenerateMessageInHeader(const protobuf::Descriptor* message, const FileLayout& layout, Output& output) { output("/* $0 */\n\n", message->full_name()); std::string msg_name = ToCIdent(message->full_name()); - if (!message->options().map_entry()) { GenerateMessageFunctionsInHeader(message, output); } @@ -834,20 +859,21 @@ void GenerateMessageInHeader(const protobuf::Descriptor* message, GenerateOneofInHeader(message->oneof_decl(i), layout, msg_name, output); } + auto field_names = CreateFieldNameMap(message); for (auto field : FieldNumberOrder(message)) { - GenerateHazzer(field, layout, msg_name, output); + GenerateHazzer(field, layout, msg_name, field_names, output); if (field->is_repeated()) { - GenerateRepeatedClear(field, layout, msg_name, output); + GenerateRepeatedClear(field, layout, msg_name, field_names, output); } else { - GenerateClear(field, layout, msg_name, output); + GenerateClear(field, layout, msg_name, field_names, output); } - GenerateGetters(field, layout, msg_name, output); + GenerateGetters(field, layout, msg_name, field_names, output); } output("\n"); for (auto field : FieldNumberOrder(message)) { - GenerateSetters(field, layout, msg_name, output); + GenerateSetters(field, layout, msg_name, field_names, output); } output("\n"); @@ -954,7 +980,6 @@ void WriteHeader(const FileLayout& layout, Output& output) { } output("\n"); - for (auto message : this_file_messages) { GenerateMessageInHeader(message, layout, output); } From 11aa037bfdcc6e5dcfbf4645f37e50bd33e1131b Mon Sep 17 00:00:00 2001 From: Eric Salo Date: Fri, 26 Aug 2022 23:22:54 -0700 Subject: [PATCH 07/12] add some more accessor calls to def.c PiperOrigin-RevId: 470404251 --- upb/def.c | 38 ++++++++++++++++++-------------------- 1 file changed, 18 insertions(+), 20 deletions(-) diff --git a/upb/def.c b/upb/def.c index 5c53888faa..06f3f4441f 100644 --- a/upb/def.c +++ b/upb/def.c @@ -678,7 +678,8 @@ const upb_EnumDef* upb_FieldDef_EnumSubDef(const upb_FieldDef* f) { const upb_MiniTable_Field* upb_FieldDef_MiniTable(const upb_FieldDef* f) { UPB_ASSERT(!upb_FieldDef_IsExtension(f)); - return &f->msgdef->layout->fields[f->layout_index]; + const upb_MiniTable* layout = upb_MessageDef_MiniTable(f->msgdef); + return &layout->fields[f->layout_index]; } const upb_MiniTable_Extension* _upb_FieldDef_ExtensionMiniTable( @@ -1149,7 +1150,7 @@ int upb_ServiceDef_MethodCount(const upb_ServiceDef* s) { } const upb_MethodDef* upb_ServiceDef_Method(const upb_ServiceDef* s, int i) { - return i < 0 || i >= s->method_count ? NULL : &s->methods[i]; + return (i < 0 || i >= s->method_count) ? NULL : &s->methods[i]; } const upb_MethodDef* upb_ServiceDef_FindMethodByName(const upb_ServiceDef* s, @@ -1176,22 +1177,18 @@ void upb_DefPool_Free(upb_DefPool* s) { upb_DefPool* upb_DefPool_New(void) { upb_DefPool* s = upb_gmalloc(sizeof(*s)); - - if (!s) { - return NULL; - } + if (!s) return NULL; s->arena = upb_Arena_New(); s->bytes_loaded = 0; - if (!upb_strtable_init(&s->syms, 32, s->arena) || - !upb_strtable_init(&s->files, 4, s->arena) || - !upb_inttable_init(&s->exts, s->arena)) { - goto err; - } + if (!upb_strtable_init(&s->syms, 32, s->arena)) goto err; + if (!upb_strtable_init(&s->files, 4, s->arena)) goto err; + if (!upb_inttable_init(&s->exts, s->arena)) goto err; s->extreg = upb_ExtensionRegistry_New(s->arena); if (!s->extreg) goto err; + return s; err: @@ -1537,7 +1534,7 @@ static void fill_fieldlayout(upb_MiniTable_Field* field, /* This function is the dynamic equivalent of message_layout.{cc,h} in upbc. * It computes a dynamic layout for all of the fields in |m|. */ static void make_layout(symtab_addctx* ctx, const upb_MessageDef* m) { - upb_MiniTable* l = (upb_MiniTable*)m->layout; + upb_MiniTable* l = (upb_MiniTable*)upb_MessageDef_MiniTable(m); size_t field_count = upb_MessageDef_numfields(m); size_t sublayout_count = 0; upb_MiniTable_Sub* subs; @@ -2261,7 +2258,7 @@ static void create_fielddef( const char* shortname; int32_t field_number; - f->file = ctx->file; /* Must happen prior to symtab_add(). */ + f->file = ctx->file; // Must happen prior to symtab_add() if (!google_protobuf_FieldDescriptorProto_has_name(field_proto)) { symtab_errf(ctx, "field has no name"); @@ -2359,8 +2356,9 @@ static void create_fielddef( CHK_OOM(upb_inttable_insert(&m->itof, field_number, v, ctx->arena)); if (ctx->layout) { - const upb_MiniTable_Field* fields = m->layout->fields; - int count = m->layout->field_count; + const upb_MiniTable* mt = upb_MessageDef_MiniTable(m); + const upb_MiniTable_Field* fields = mt->fields; + const int count = mt->field_count; bool found = false; for (int i = 0; i < count; i++) { if (fields[i].number == field_number) { @@ -2378,7 +2376,7 @@ static void create_fielddef( symtab_add(ctx, full_name, pack_def(f, UPB_DEFTYPE_EXT)); f->layout_index = ctx->ext_count++; if (ctx->layout) { - UPB_ASSERT(ctx->file->ext_layouts[f->layout_index]->field.number == + UPB_ASSERT(_upb_FieldDef_ExtensionMiniTable(f)->field.number == field_number); } } @@ -2446,7 +2444,7 @@ static void create_fielddef( /* Repeated fields default to packed for proto3 only. */ f->packed_ = upb_FieldDef_IsPrimitive(f) && f->label_ == kUpb_Label_Repeated && - f->file->syntax == kUpb_Syntax_Proto3; + upb_FileDef_Syntax(f->file) == kUpb_Syntax_Proto3; } } @@ -2458,7 +2456,7 @@ static void create_service( const google_protobuf_MethodDescriptorProto* const* methods; size_t i, n; - s->file = ctx->file; /* Must happen prior to symtab_add. */ + s->file = ctx->file; // Must happen prior to symtab_add() name = google_protobuf_ServiceDescriptorProto_name(svc_proto); check_ident(ctx, name, false); @@ -2654,7 +2652,7 @@ static void create_msgdef(symtab_addctx* ctx, const char* prefix, size_t i, n_oneof, n_field, n_ext_range; upb_StringView name; - m->file = ctx->file; /* Must happen prior to symtab_add(). */ + m->file = ctx->file; // Must happen prior to symtab_add() m->containing_type = containing_type; name = google_protobuf_DescriptorProto_name(msg_proto); @@ -2833,7 +2831,7 @@ static void resolve_extension( (unsigned)f->number_, f->full_name, f->msgdef->full_name); } - const upb_MiniTable_Extension* ext = ctx->file->ext_layouts[f->layout_index]; + const upb_MiniTable_Extension* ext = _upb_FieldDef_ExtensionMiniTable(f); if (ctx->layout) { UPB_ASSERT(upb_FieldDef_Number(f) == ext->field.number); } else { From daac8dce8fda41e529550aac93053a69cb3bd5a7 Mon Sep 17 00:00:00 2001 From: Eric Salo Date: Sun, 28 Aug 2022 08:29:58 -0700 Subject: [PATCH 08/12] add more constructors to def.c PiperOrigin-RevId: 470566093 --- upb/def.c | 168 ++++++++++++++++++++++++++++-------------------------- 1 file changed, 87 insertions(+), 81 deletions(-) diff --git a/upb/def.c b/upb/def.c index 06f3f4441f..ca1f57e006 100644 --- a/upb/def.c +++ b/upb/def.c @@ -923,11 +923,6 @@ upb_WellKnown upb_MessageDef_WellKnownType(const upb_MessageDef* m) { return m->well_known_type; } -// Allocate sufficient storage to contain an array of |n| message defs. -static upb_MessageDef* _upb_MessageDef_Alloc(symtab_addctx* ctx, int n) { - return symtab_alloc(ctx, sizeof(upb_MessageDef) * n); -} - /* upb_OneofDef ***************************************************************/ const google_protobuf_OneofOptions* upb_OneofDef_Options( @@ -979,11 +974,6 @@ const upb_FieldDef* upb_OneofDef_LookupNumber(const upb_OneofDef* o, : NULL; } -// Allocate sufficient storage to contain an array of |n| oneof defs. -static upb_OneofDef* _upb_OneofDef_Alloc(symtab_addctx* ctx, int n) { - return symtab_alloc(ctx, sizeof(upb_OneofDef) * n); -} - /* upb_FileDef ****************************************************************/ const google_protobuf_FileOptions* upb_FileDef_Options(const upb_FileDef* f) { @@ -1071,11 +1061,6 @@ const upb_ServiceDef* upb_FileDef_Service(const upb_FileDef* f, int i) { const upb_DefPool* upb_FileDef_Pool(const upb_FileDef* f) { return f->symtab; } -// Allocate sufficient storage to contain one file def. -static upb_FileDef* _upb_FileDef_Alloc(symtab_addctx* ctx) { - return symtab_alloc(ctx, sizeof(upb_FileDef)); -} - /* upb_MethodDef **************************************************************/ const google_protobuf_MethodOptions* upb_MethodDef_Options(const upb_MethodDef* m) { @@ -1116,11 +1101,6 @@ bool upb_MethodDef_ServerStreaming(const upb_MethodDef* m) { return m->server_streaming; } -// Allocate sufficient storage to contain an array of |n| method defs. -static upb_MethodDef* _upb_MethodDef_Alloc(symtab_addctx* ctx, int n) { - return symtab_alloc(ctx, sizeof(upb_MethodDef) * n); -} - /* upb_ServiceDef *************************************************************/ const google_protobuf_ServiceOptions* upb_ServiceDef_Options(const upb_ServiceDef* s) { @@ -1163,11 +1143,6 @@ const upb_MethodDef* upb_ServiceDef_FindMethodByName(const upb_ServiceDef* s, return NULL; } -// Allocate sufficient storage to contain an array of |n| service defs. -static upb_ServiceDef* _upb_ServiceDef_Alloc(symtab_addctx* ctx, int n) { - return symtab_alloc(ctx, sizeof(upb_ServiceDef) * n); -} - /* upb_DefPool ****************************************************************/ void upb_DefPool_Free(upb_DefPool* s) { @@ -1957,6 +1932,17 @@ static void create_oneofdef( CHK_OOM(upb_strtable_init(&o->ntof, 4, ctx->arena)); } +// Allocate and initialize an array of |n| oneof defs. +static upb_OneofDef* _upb_OneofDefs_New( + symtab_addctx* ctx, int n, const google_protobuf_OneofDescriptorProto* const* protos, + upb_MessageDef* m) { + upb_OneofDef* o = symtab_alloc(ctx, sizeof(upb_OneofDef) * n); + for (int i = 0; i < n; i++) { + create_oneofdef(ctx, m, protos[i], &o[i]); + } + return o; +} + static str_t* newstr(symtab_addctx* ctx, const char* data, size_t len) { str_t* ret = symtab_alloc(ctx, sizeof(*ret) + len); CHK_OOM(ret); @@ -2448,13 +2434,44 @@ static void create_fielddef( } } -static void create_service( - symtab_addctx* ctx, const google_protobuf_ServiceDescriptorProto* svc_proto, - const upb_ServiceDef* _s) { - upb_ServiceDef* s = (upb_ServiceDef*)_s; +static void create_method(symtab_addctx* ctx, + const google_protobuf_MethodDescriptorProto* method_proto, + upb_ServiceDef* s, upb_MethodDef* m) { + upb_StringView name = google_protobuf_MethodDescriptorProto_name(method_proto); + + m->service = s; + m->full_name = makefullname(ctx, s->full_name, name); + m->client_streaming = + google_protobuf_MethodDescriptorProto_client_streaming(method_proto); + m->server_streaming = + google_protobuf_MethodDescriptorProto_server_streaming(method_proto); + m->input_type = symtab_resolve( + ctx, m->full_name, m->full_name, + google_protobuf_MethodDescriptorProto_input_type(method_proto), UPB_DEFTYPE_MSG); + m->output_type = symtab_resolve( + ctx, m->full_name, m->full_name, + google_protobuf_MethodDescriptorProto_output_type(method_proto), UPB_DEFTYPE_MSG); + + SET_OPTIONS(m->opts, MethodDescriptorProto, MethodOptions, method_proto); +} + +// Allocate and initialize an array of |n| method defs. +static upb_MethodDef* _upb_MethodDefs_New( + symtab_addctx* ctx, int n, + const google_protobuf_MethodDescriptorProto* const* protos, upb_ServiceDef* s) { + upb_MethodDef* m = symtab_alloc(ctx, sizeof(upb_MethodDef) * n); + for (int i = 0; i < n; i++) { + create_method(ctx, protos[i], s, &m[i]); + m[i].index = i; + } + return m; +} + +static void create_service(symtab_addctx* ctx, + const google_protobuf_ServiceDescriptorProto* svc_proto, + upb_ServiceDef* s) { upb_StringView name; - const google_protobuf_MethodDescriptorProto* const* methods; - size_t i, n; + size_t n; s->file = ctx->file; // Must happen prior to symtab_add() @@ -2463,37 +2480,24 @@ static void create_service( s->full_name = makefullname(ctx, ctx->file->package, name); symtab_add(ctx, s->full_name, pack_def(s, UPB_DEFTYPE_SERVICE)); - methods = google_protobuf_ServiceDescriptorProto_method(svc_proto, &n); - + const google_protobuf_MethodDescriptorProto* const* methods = + google_protobuf_ServiceDescriptorProto_method(svc_proto, &n); s->method_count = n; - s->methods = _upb_MethodDef_Alloc(ctx, n); + s->methods = _upb_MethodDefs_New(ctx, n, methods, s); SET_OPTIONS(s->opts, ServiceDescriptorProto, ServiceOptions, svc_proto); +} - for (i = 0; i < n; i++) { - const google_protobuf_MethodDescriptorProto* method_proto = methods[i]; - upb_MethodDef* m = (upb_MethodDef*)&s->methods[i]; - upb_StringView name = - google_protobuf_MethodDescriptorProto_name(method_proto); - - m->service = s; - m->full_name = makefullname(ctx, s->full_name, name); - m->index = i; - m->client_streaming = - google_protobuf_MethodDescriptorProto_client_streaming(method_proto); - m->server_streaming = - google_protobuf_MethodDescriptorProto_server_streaming(method_proto); - m->input_type = symtab_resolve( - ctx, m->full_name, m->full_name, - google_protobuf_MethodDescriptorProto_input_type(method_proto), - UPB_DEFTYPE_MSG); - m->output_type = symtab_resolve( - ctx, m->full_name, m->full_name, - google_protobuf_MethodDescriptorProto_output_type(method_proto), - UPB_DEFTYPE_MSG); - - SET_OPTIONS(m->opts, MethodDescriptorProto, MethodOptions, method_proto); +// Allocate and initialize an array of |n| service defs. +static upb_ServiceDef* _upb_ServiceDefs_New( + symtab_addctx* ctx, int n, + const google_protobuf_ServiceDescriptorProto* const* protos) { + upb_ServiceDef* s = symtab_alloc(ctx, sizeof(upb_ServiceDef) * n); + for (int i = 0; i < n; i++) { + create_service(ctx, protos[i], &s[i]); + s[i].index = i; } + return s; } static int count_bits_debug(uint64_t x) { @@ -2683,10 +2687,7 @@ static void create_msgdef(symtab_addctx* ctx, const char* prefix, SET_OPTIONS(m->opts, DescriptorProto, MessageOptions, msg_proto); m->oneof_count = n_oneof; - m->oneofs = _upb_OneofDef_Alloc(ctx, n_oneof); - for (i = 0; i < n_oneof; i++) { - create_oneofdef(ctx, m, oneofs[i], &m->oneofs[i]); - } + m->oneofs = _upb_OneofDefs_New(ctx, n_oneof, oneofs, m); m->field_count = n_field; m->fields = _upb_FieldDef_Alloc(ctx, n_field); @@ -2727,6 +2728,19 @@ static void create_msgdef(symtab_addctx* ctx, const char* prefix, msgdef_create_nested(ctx, msg_proto, m); } +// Allocate and initialize an array of |n| message defs. +static upb_MessageDef* _upb_MessageDefs_New( + symtab_addctx* ctx, int n, const google_protobuf_DescriptorProto* const* protos, + const upb_MessageDef* containing_type) { + const char* name = + containing_type ? containing_type->full_name : ctx->file->package; + upb_MessageDef* m = symtab_alloc(ctx, sizeof(upb_MessageDef) * n); + for (int i = 0; i < n; i++) { + create_msgdef(ctx, name, protos[i], containing_type, &m[i]); + } + return m; +} + static void msgdef_create_nested( symtab_addctx* ctx, const google_protobuf_DescriptorProto* msg_proto, upb_MessageDef* m) { @@ -2750,10 +2764,7 @@ static void msgdef_create_nested( const google_protobuf_DescriptorProto* const* msgs = google_protobuf_DescriptorProto_nested_type(msg_proto, &n); m->nested_msg_count = n; - m->nested_msgs = _upb_MessageDef_Alloc(ctx, n); - for (size_t i = 0; i < n; i++) { - create_msgdef(ctx, m->full_name, msgs[i], m, &m->nested_msgs[i]); - } + m->nested_msgs = _upb_MessageDefs_New(ctx, n, msgs, m); } static void resolve_subdef(symtab_addctx* ctx, const char* prefix, @@ -2926,9 +2937,12 @@ static int count_exts_in_msg(const google_protobuf_DescriptorProto* msg_proto) { return ext_count; } -static void build_filedef( - symtab_addctx* ctx, upb_FileDef* file, - const google_protobuf_FileDescriptorProto* file_proto) { +// Allocate and initialize one file def, and add it to the context object. +static void _upb_FileDef_Create(symtab_addctx* ctx, + const google_protobuf_FileDescriptorProto* file_proto) { + upb_FileDef* file = symtab_alloc(ctx, sizeof(upb_FileDef)); + ctx->file = file; + const google_protobuf_DescriptorProto* const* msgs; const google_protobuf_EnumDescriptorProto* const* enums; const google_protobuf_FieldDescriptorProto* const* exts; @@ -3053,22 +3067,15 @@ static void build_filedef( ((upb_FieldDef*)&file->top_lvl_exts[i])->index_ = i; } - /* Create messages. */ + // Create messages. msgs = google_protobuf_FileDescriptorProto_message_type(file_proto, &n); file->top_lvl_msg_count = n; - file->top_lvl_msgs = _upb_MessageDef_Alloc(ctx, n); - for (i = 0; i < n; i++) { - create_msgdef(ctx, file->package, msgs[i], NULL, &file->top_lvl_msgs[i]); - } + file->top_lvl_msgs = _upb_MessageDefs_New(ctx, n, msgs, NULL); - /* Create services. */ + // Create services. services = google_protobuf_FileDescriptorProto_service(file_proto, &n); file->service_count = n; - file->services = _upb_ServiceDef_Alloc(ctx, n); - for (i = 0; i < n; i++) { - create_service(ctx, services[i], &file->services[i]); - ((upb_ServiceDef*)&file->services[i])->index = i; - } + file->services = _upb_ServiceDefs_New(ctx, n, services); /* Now that all names are in the table, build layouts and resolve refs. */ for (i = 0; i < (size_t)file->top_lvl_ext_count; i++) { @@ -3164,8 +3171,7 @@ static const upb_FileDef* _upb_DefPool_AddFile( ctx.file = NULL; } } else { - ctx.file = _upb_FileDef_Alloc(&ctx); - build_filedef(&ctx, ctx.file, file_proto); + _upb_FileDef_Create(&ctx, file_proto); upb_strtable_insert(&s->files, name.data, name.size, pack_def(ctx.file, UPB_DEFTYPE_FILE), ctx.arena); UPB_ASSERT(upb_Status_IsOk(status)); From 15b2402144a08c3dcbfab33608e1da06336c02bb Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Sun, 28 Aug 2022 08:41:50 -0700 Subject: [PATCH 09/12] Removed unused `_upb_DefPool_registerlayout()` function. This function was introduced in https://github.com/protocolbuffers/upb/pull/426 but it appears it was never used. I am not sure what the purpose was, but in any case it is not needed. With this function removed, we no longer need to tag pointers for the DefPool "files" table. PiperOrigin-RevId: 470567000 --- upb/def.c | 35 +++++++---------------------------- 1 file changed, 7 insertions(+), 28 deletions(-) diff --git a/upb/def.c b/upb/def.c index ca1f57e006..d2225f619a 100644 --- a/upb/def.c +++ b/upb/def.c @@ -239,7 +239,6 @@ typedef enum { /* Only inside file table. */ UPB_DEFTYPE_FILE = 0, - UPB_DEFTYPE_LAYOUT = 1 } upb_deftype_t; #define FIELD_TYPE_UNSPECIFIED 0 @@ -1208,9 +1207,8 @@ const upb_EnumValueDef* upb_DefPool_FindEnumByNameval(const upb_DefPool* s, const upb_FileDef* upb_DefPool_FindFileByName(const upb_DefPool* s, const char* name) { upb_value v; - return upb_strtable_lookup(&s->files, name, &v) - ? unpack_def(v, UPB_DEFTYPE_FILE) - : NULL; + return upb_strtable_lookup(&s->files, name, &v) ? upb_value_getconstptr(v) + : NULL; } const upb_FileDef* upb_DefPool_FindFileByNameWithSize(const upb_DefPool* s, @@ -1218,7 +1216,7 @@ const upb_FileDef* upb_DefPool_FindFileByNameWithSize(const upb_DefPool* s, size_t len) { upb_value v; return upb_strtable_lookup2(&s->files, name, len, &v) - ? unpack_def(v, UPB_DEFTYPE_FILE) + ? upb_value_getconstptr(v) : NULL; } @@ -3131,20 +3129,9 @@ static const upb_FileDef* _upb_DefPool_AddFile( upb_value v; if (upb_strtable_lookup2(&s->files, name.data, name.size, &v)) { - if (unpack_def(v, UPB_DEFTYPE_FILE)) { - upb_Status_SetErrorFormat(status, "duplicate file name (%.*s)", - UPB_STRINGVIEW_ARGS(name)); - return NULL; - } - const upb_MiniTable_File* registered = unpack_def(v, UPB_DEFTYPE_LAYOUT); - UPB_ASSERT(registered); - if (layout && layout != registered) { - upb_Status_SetErrorFormat( - status, "tried to build with a different layout (filename=%.*s)", - UPB_STRINGVIEW_ARGS(name)); - return NULL; - } - layout = registered; + upb_Status_SetErrorFormat(status, "duplicate file name (%.*s)", + UPB_STRINGVIEW_ARGS(name)); + return NULL; } ctx.symtab = s; @@ -3173,7 +3160,7 @@ static const upb_FileDef* _upb_DefPool_AddFile( } else { _upb_FileDef_Create(&ctx, file_proto); upb_strtable_insert(&s->files, name.data, name.size, - pack_def(ctx.file, UPB_DEFTYPE_FILE), ctx.arena); + upb_value_constptr(ctx.file), ctx.arena); UPB_ASSERT(upb_Status_IsOk(status)); upb_Arena_Fuse(s->arena, ctx.arena); } @@ -3266,14 +3253,6 @@ const upb_FieldDef* upb_DefPool_FindExtensionByNumber(const upb_DefPool* s, return ext ? _upb_DefPool_FindExtensionByMiniTable(s, ext) : NULL; } -bool _upb_DefPool_registerlayout(upb_DefPool* s, const char* filename, - const upb_MiniTable_File* file) { - if (upb_DefPool_FindFileByName(s, filename)) return false; - upb_value v = pack_def(file, UPB_DEFTYPE_LAYOUT); - return upb_strtable_insert(&s->files, filename, strlen(filename), v, - s->arena); -} - const upb_ExtensionRegistry* upb_DefPool_ExtensionRegistry( const upb_DefPool* s) { return s->extreg; From 682b519aac03dc2cd22cf36cedd6be72cc75a176 Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Sun, 28 Aug 2022 16:52:06 +0000 Subject: [PATCH 10/12] Fixed GitHub Actions tests for pull requests that come from other repos. Previously GitHub Actions would fail if they did not have access to repository secrets. The code attempted to have graceful fallback, but the graceful fallback was not working properly. This PR fixes the graceful fallback: - Bazel tests will use read-only caching instead of read-write. - We unfortunately need to skip the Python wheel tests, as they require our private Docker image to run. --- .github/workflows/bazel_tests.yml | 6 +++--- .github/workflows/python_tests.yml | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/bazel_tests.yml b/.github/workflows/bazel_tests.yml index 8416481ae8..2f612053f5 100644 --- a/.github/workflows/bazel_tests.yml +++ b/.github/workflows/bazel_tests.yml @@ -36,13 +36,13 @@ jobs: with: credentials_json: ${{ secrets.GOOGLE_CREDENTIALS }} export_environment_variables: true - if: ${{ github.repository_owner == 'protocolbuffers' }} + if: ${{ github.event.pull_request.head.repo.full_name == 'protocolbuffers/upb' }} - name: Set up Bazel read/write caching run: echo "BAZEL_CACHE_AUTH=--google_default_credentials" >> $GITHUB_ENV - if: ${{ github.repository_owner == 'protocolbuffers' }} + if: ${{ github.event.pull_request.head.repo.full_name == 'protocolbuffers/upb' }} - name: Set up Bazel read-only caching run: echo "BAZEL_CACHE_AUTH=--remote_upload_local_results=false" >> $GITHUB_ENV - if: ${{ github.repository_owner != 'protocolbuffers' }} + if: ${{ github.event.pull_request.head.repo.full_name != 'protocolbuffers/upb' }} - name: Setup Python venv run: rm -rf /tmp/venv && python3 -m venv /tmp/venv - name: Install dependencies diff --git a/.github/workflows/python_tests.yml b/.github/workflows/python_tests.yml index 810563598f..68b7ac43c1 100644 --- a/.github/workflows/python_tests.yml +++ b/.github/workflows/python_tests.yml @@ -13,7 +13,7 @@ jobs: build_wheels: name: Build Wheels runs-on: ubuntu-latest - if: ${{ github.repository_owner == 'protocolbuffers' }} + if: ${{ github.event.pull_request.head.repo.full_name == 'protocolbuffers/upb' }} env: DOCKER_IMAGE: gcr.io/protobuf-build/bazel/linux@sha256:2bfd061284eff8234f2fcca16d71d43c69ccf3a22206628b54c204a6a9aac277 BAZEL_CACHE: --remote_cache=https://storage.googleapis.com/protobuf-bazel-cache --google_default_credentials From 250321e63bf401e9b15ef4f602b436978af4eab7 Mon Sep 17 00:00:00 2001 From: Eric Salo Date: Sun, 28 Aug 2022 14:02:51 -0700 Subject: [PATCH 11/12] more misc tweaks to def.c: - renamed symtab_addctx as upb_AddDefCtx - simplified _upb_DefPool_AddFile() slightly - replaced upb_Status_setoom() with kOutOfMemory - deleted all references to UPB_DEFTYPE_LAYOUT and UPB_DEFTYPE_FILE PiperOrigin-RevId: 470592649 --- upb/def.c | 186 ++++++++++++++++++++++++++---------------------------- 1 file changed, 89 insertions(+), 97 deletions(-) diff --git a/upb/def.c b/upb/def.c index d2225f619a..a24cf91dee 100644 --- a/upb/def.c +++ b/upb/def.c @@ -53,6 +53,8 @@ typedef struct { static const char opt_default_buf[_UPB_MAXOPT_SIZE + sizeof(void*)] = {0}; static const char* opt_default = &opt_default_buf[sizeof(void*)]; +static const char kOutOfMemory[] = "out of memory"; + struct upb_FieldDef { const google_protobuf_FieldOptions* opts; const upb_FileDef* file; @@ -236,9 +238,6 @@ typedef enum { UPB_DEFTYPE_FIELD = 0, UPB_DEFTYPE_ONEOF = 1, UPB_DEFTYPE_FIELD_JSONNAME = 2, - - /* Only inside file table. */ - UPB_DEFTYPE_FILE = 0, } upb_deftype_t; #define FIELD_TYPE_UNSPECIFIED 0 @@ -327,10 +326,6 @@ static int cmp_values(const void* a, const void* b) { return (A < B) ? -1 : (A > B); } -static void upb_Status_setoom(upb_Status* status) { - upb_Status_SetErrorMessage(status, "out of memory"); -} - static void assign_msg_wellknowntype(upb_MessageDef* m) { const char* name = upb_MessageDef_FullName(m); if (name == NULL) { @@ -379,16 +374,16 @@ typedef struct { upb_FileDef* file; /* File we are building. */ upb_Arena* arena; /* Allocate defs here. */ upb_Arena* tmp_arena; /* For temporary allocations. */ + upb_Status* status; /* Record errors here. */ const upb_MiniTable_File* layout; /* NULL if we should build layouts. */ int enum_count; /* Count of enums built so far. */ int msg_count; /* Count of messages built so far. */ int ext_count; /* Count of extensions built so far. */ - upb_Status* status; /* Record errors here. */ jmp_buf err; /* longjmp() on error. */ -} symtab_addctx; +} upb_AddDefCtx; UPB_NORETURN UPB_NOINLINE UPB_PRINTF(2, 3) static void symtab_errf( - symtab_addctx* ctx, const char* fmt, ...) { + upb_AddDefCtx* ctx, const char* fmt, ...) { va_list argp; va_start(argp, fmt); upb_Status_VSetErrorFormat(ctx->status, fmt, argp); @@ -396,12 +391,12 @@ UPB_NORETURN UPB_NOINLINE UPB_PRINTF(2, 3) static void symtab_errf( UPB_LONGJMP(ctx->err, 1); } -UPB_NORETURN UPB_NOINLINE static void symtab_oomerr(symtab_addctx* ctx) { - upb_Status_setoom(ctx->status); +UPB_NORETURN UPB_NOINLINE static void symtab_oomerr(upb_AddDefCtx* ctx) { + upb_Status_SetErrorMessage(ctx->status, kOutOfMemory); UPB_LONGJMP(ctx->err, 1); } -void* symtab_alloc(symtab_addctx* ctx, size_t bytes) { +void* symtab_alloc(upb_AddDefCtx* ctx, size_t bytes) { if (bytes == 0) return NULL; void* ret = upb_Arena_Malloc(ctx->arena, bytes); if (!ret) symtab_oomerr(ctx); @@ -528,7 +523,7 @@ int32_t upb_ExtensionRange_Start(const upb_ExtensionRange* e) { int32_t upb_ExtensionRange_End(const upb_ExtensionRange* e) { return e->end; } // Allocate sufficient storage to contain an array of |n| extension ranges. -static upb_ExtensionRange* _upb_ExtensionRange_Alloc(symtab_addctx* ctx, +static upb_ExtensionRange* _upb_ExtensionRange_Alloc(upb_AddDefCtx* ctx, int n) { return symtab_alloc(ctx, sizeof(upb_ExtensionRange) * n); } @@ -749,7 +744,7 @@ bool upb_FieldDef_checkdescriptortype(int32_t type) { } // Allocate sufficient storage to contain an array of |n| field defs. -static upb_FieldDef* _upb_FieldDef_Alloc(symtab_addctx* ctx, int n) { +static upb_FieldDef* _upb_FieldDef_Alloc(upb_AddDefCtx* ctx, int n) { return symtab_alloc(ctx, sizeof(upb_FieldDef) * n); } @@ -1327,7 +1322,7 @@ const upb_FileDef* upb_DefPool_FindFileContainingSymbol(const upb_DefPool* s, target = (const google_protobuf_##options_type*)opt_default; \ } -static void check_ident(symtab_addctx* ctx, upb_StringView name, bool full) { +static void check_ident(upb_AddDefCtx* ctx, upb_StringView name, bool full) { const char* str = name.data; size_t len = name.size; bool start = true; @@ -1395,7 +1390,7 @@ static uint8_t upb_msg_fielddefsize(const upb_FieldDef* f) { } } -static uint32_t upb_MiniTable_place(symtab_addctx* ctx, upb_MiniTable* l, +static uint32_t upb_MiniTable_place(upb_AddDefCtx* ctx, upb_MiniTable* l, size_t size, const upb_MessageDef* m) { size_t ofs = UPB_ALIGN_UP(l->size, size); size_t next = ofs + size; @@ -1506,7 +1501,7 @@ static void fill_fieldlayout(upb_MiniTable_Field* field, /* This function is the dynamic equivalent of message_layout.{cc,h} in upbc. * It computes a dynamic layout for all of the fields in |m|. */ -static void make_layout(symtab_addctx* ctx, const upb_MessageDef* m) { +static void make_layout(upb_AddDefCtx* ctx, const upb_MessageDef* m) { upb_MiniTable* l = (upb_MiniTable*)upb_MessageDef_MiniTable(m); size_t field_count = upb_MessageDef_numfields(m); size_t sublayout_count = 0; @@ -1701,7 +1696,7 @@ static void make_layout(symtab_addctx* ctx, const upb_MessageDef* m) { assign_layout_indices(m, l, fields); } -static char* strviewdup(symtab_addctx* ctx, upb_StringView view) { +static char* strviewdup(upb_AddDefCtx* ctx, upb_StringView view) { char* ret = upb_strdup2(view.data, view.size, ctx->arena); CHK_OOM(ret); return ret; @@ -1715,7 +1710,7 @@ static bool streql_view(upb_StringView view, const char* b) { return streql2(view.data, view.size, b); } -static const char* makefullname(symtab_addctx* ctx, const char* prefix, +static const char* makefullname(upb_AddDefCtx* ctx, const char* prefix, upb_StringView name) { if (prefix) { /* ret = prefix + '.' + name; */ @@ -1731,7 +1726,7 @@ static const char* makefullname(symtab_addctx* ctx, const char* prefix, } } -static void finalize_oneofs(symtab_addctx* ctx, upb_MessageDef* m) { +static void finalize_oneofs(upb_AddDefCtx* ctx, upb_MessageDef* m) { int i; int synthetic_count = 0; upb_OneofDef* mutable_oneofs = (upb_OneofDef*)m->oneofs; @@ -1806,7 +1801,7 @@ size_t getjsonname(const char* name, char* buf, size_t len) { #undef WRITE } -static char* makejsonname(symtab_addctx* ctx, const char* name) { +static char* makejsonname(upb_AddDefCtx* ctx, const char* name) { size_t size = getjsonname(name, NULL, 0); char* json_name = symtab_alloc(ctx, size); getjsonname(name, json_name, size); @@ -1816,7 +1811,7 @@ static char* makejsonname(symtab_addctx* ctx, const char* name) { /* Adds a symbol |v| to the symtab, which must be a def pointer previously * packed with pack_def(). The def's pointer to upb_FileDef* must be set before * adding, so we know which entries to remove if building this file fails. */ -static void symtab_add(symtab_addctx* ctx, const char* name, upb_value v) { +static void symtab_add(upb_AddDefCtx* ctx, const char* name, upb_value v) { // TODO: table should support an operation "tryinsert" to avoid the double // lookup. if (upb_strtable_lookup(&ctx->symtab->syms, name, NULL)) { @@ -1843,7 +1838,7 @@ static bool remove_component(char* base, size_t* len) { /* Given a symbol and the base symbol inside which it is defined, find the * symbol's definition in t. */ -static const void* symtab_resolveany(symtab_addctx* ctx, +static const void* symtab_resolveany(upb_AddDefCtx* ctx, const char* from_name_dbg, const char* base, upb_StringView sym, upb_deftype_t* type) { @@ -1888,7 +1883,7 @@ notfound: UPB_STRINGVIEW_ARGS(sym)); } -static const void* symtab_resolve(symtab_addctx* ctx, const char* from_name_dbg, +static const void* symtab_resolve(upb_AddDefCtx* ctx, const char* from_name_dbg, const char* base, upb_StringView sym, upb_deftype_t type) { upb_deftype_t found_type; @@ -1903,10 +1898,9 @@ static const void* symtab_resolve(symtab_addctx* ctx, const char* from_name_dbg, return ret; } -static void create_oneofdef( - symtab_addctx* ctx, upb_MessageDef* m, - const google_protobuf_OneofDescriptorProto* oneof_proto, - const upb_OneofDef* _o) { +static void create_oneofdef(upb_AddDefCtx* ctx, upb_MessageDef* m, + const google_protobuf_OneofDescriptorProto* oneof_proto, + const upb_OneofDef* _o) { upb_OneofDef* o = (upb_OneofDef*)_o; upb_StringView name = google_protobuf_OneofDescriptorProto_name(oneof_proto); upb_value v; @@ -1932,7 +1926,7 @@ static void create_oneofdef( // Allocate and initialize an array of |n| oneof defs. static upb_OneofDef* _upb_OneofDefs_New( - symtab_addctx* ctx, int n, const google_protobuf_OneofDescriptorProto* const* protos, + upb_AddDefCtx* ctx, int n, const google_protobuf_OneofDescriptorProto* const* protos, upb_MessageDef* m) { upb_OneofDef* o = symtab_alloc(ctx, sizeof(upb_OneofDef) * n); for (int i = 0; i < n; i++) { @@ -1941,7 +1935,7 @@ static upb_OneofDef* _upb_OneofDefs_New( return o; } -static str_t* newstr(symtab_addctx* ctx, const char* data, size_t len) { +static str_t* newstr(upb_AddDefCtx* ctx, const char* data, size_t len) { str_t* ret = symtab_alloc(ctx, sizeof(*ret) + len); CHK_OOM(ret); ret->len = len; @@ -1958,7 +1952,7 @@ static bool upb_DefPool_TryGetChar(const char** src, const char* end, return true; } -static char upb_DefPool_TryGetHexDigit(symtab_addctx* ctx, +static char upb_DefPool_TryGetHexDigit(upb_AddDefCtx* ctx, const upb_FieldDef* f, const char** src, const char* end) { char ch; @@ -1974,7 +1968,7 @@ static char upb_DefPool_TryGetHexDigit(symtab_addctx* ctx, return -1; } -static char upb_DefPool_ParseHexEscape(symtab_addctx* ctx, +static char upb_DefPool_ParseHexEscape(upb_AddDefCtx* ctx, const upb_FieldDef* f, const char** src, const char* end) { char hex_digit = upb_DefPool_TryGetHexDigit(ctx, f, src, end); @@ -2006,7 +2000,7 @@ char upb_DefPool_TryGetOctalDigit(const char** src, const char* end) { return -1; } -static char upb_DefPool_ParseOctalEscape(symtab_addctx* ctx, +static char upb_DefPool_ParseOctalEscape(upb_AddDefCtx* ctx, const upb_FieldDef* f, const char** src, const char* end) { char ch = 0; @@ -2019,7 +2013,7 @@ static char upb_DefPool_ParseOctalEscape(symtab_addctx* ctx, return ch; } -static char upb_DefPool_ParseEscape(symtab_addctx* ctx, const upb_FieldDef* f, +static char upb_DefPool_ParseEscape(upb_AddDefCtx* ctx, const upb_FieldDef* f, const char** src, const char* end) { char ch; if (!upb_DefPool_TryGetChar(src, end, &ch)) { @@ -2067,7 +2061,7 @@ static char upb_DefPool_ParseEscape(symtab_addctx* ctx, const upb_FieldDef* f, symtab_errf(ctx, "Unknown escape sequence: \\%c", ch); } -static str_t* unescape(symtab_addctx* ctx, const upb_FieldDef* f, +static str_t* unescape(upb_AddDefCtx* ctx, const upb_FieldDef* f, const char* data, size_t len) { // Size here is an upper bound; escape sequences could ultimately shrink it. str_t* ret = symtab_alloc(ctx, sizeof(*ret) + len); @@ -2088,7 +2082,7 @@ static str_t* unescape(symtab_addctx* ctx, const upb_FieldDef* f, return ret; } -static void parse_default(symtab_addctx* ctx, const char* str, size_t len, +static void parse_default(upb_AddDefCtx* ctx, const char* str, size_t len, upb_FieldDef* f) { char* end; char nullz[64]; @@ -2201,7 +2195,7 @@ invalid: str, upb_FieldDef_FullName(f), (int)upb_FieldDef_Type(f)); } -static void set_default_default(symtab_addctx* ctx, upb_FieldDef* f) { +static void set_default_default(upb_AddDefCtx* ctx, upb_FieldDef* f) { switch (upb_FieldDef_CType(f)) { case kUpb_CType_Int32: case kUpb_CType_Int64: @@ -2231,10 +2225,10 @@ static void set_default_default(symtab_addctx* ctx, upb_FieldDef* f) { } } -static void create_fielddef( - symtab_addctx* ctx, const char* prefix, upb_MessageDef* m, - const google_protobuf_FieldDescriptorProto* field_proto, - const upb_FieldDef* _f, bool is_extension) { +static void create_fielddef(upb_AddDefCtx* ctx, const char* prefix, + upb_MessageDef* m, + const google_protobuf_FieldDescriptorProto* field_proto, + const upb_FieldDef* _f, bool is_extension) { upb_FieldDef* f = (upb_FieldDef*)_f; upb_StringView name; const char* full_name; @@ -2432,7 +2426,7 @@ static void create_fielddef( } } -static void create_method(symtab_addctx* ctx, +static void create_method(upb_AddDefCtx* ctx, const google_protobuf_MethodDescriptorProto* method_proto, upb_ServiceDef* s, upb_MethodDef* m) { upb_StringView name = google_protobuf_MethodDescriptorProto_name(method_proto); @@ -2455,7 +2449,7 @@ static void create_method(symtab_addctx* ctx, // Allocate and initialize an array of |n| method defs. static upb_MethodDef* _upb_MethodDefs_New( - symtab_addctx* ctx, int n, + upb_AddDefCtx* ctx, int n, const google_protobuf_MethodDescriptorProto* const* protos, upb_ServiceDef* s) { upb_MethodDef* m = symtab_alloc(ctx, sizeof(upb_MethodDef) * n); for (int i = 0; i < n; i++) { @@ -2465,7 +2459,7 @@ static upb_MethodDef* _upb_MethodDefs_New( return m; } -static void create_service(symtab_addctx* ctx, +static void create_service(upb_AddDefCtx* ctx, const google_protobuf_ServiceDescriptorProto* svc_proto, upb_ServiceDef* s) { upb_StringView name; @@ -2488,7 +2482,7 @@ static void create_service(symtab_addctx* ctx, // Allocate and initialize an array of |n| service defs. static upb_ServiceDef* _upb_ServiceDefs_New( - symtab_addctx* ctx, int n, + upb_AddDefCtx* ctx, int n, const google_protobuf_ServiceDescriptorProto* const* protos) { upb_ServiceDef* s = symtab_alloc(ctx, sizeof(upb_ServiceDef) * n); for (int i = 0; i < n; i++) { @@ -2514,7 +2508,7 @@ static int compare_int32(const void* a_ptr, const void* b_ptr) { return a < b ? -1 : (a == b ? 0 : 1); } -static upb_MiniTable_Enum* create_enumlayout(symtab_addctx* ctx, +static upb_MiniTable_Enum* create_enumlayout(upb_AddDefCtx* ctx, const upb_EnumDef* e) { const char* desc = _upb_EnumDef_MiniDescriptor(e, ctx->tmp_arena); if (!desc) symtab_errf(ctx, "OOM while building enum MiniDescriptor"); @@ -2527,7 +2521,7 @@ static upb_MiniTable_Enum* create_enumlayout(symtab_addctx* ctx, return layout; } -static void create_enumvaldef(symtab_addctx* ctx, const char* prefix, +static void create_enumvaldef(upb_AddDefCtx* ctx, const char* prefix, const google_protobuf_EnumValueDescriptorProto* val_proto, upb_EnumDef* e, upb_EnumValueDef* v) { upb_StringView name = google_protobuf_EnumValueDescriptorProto_name(val_proto); @@ -2551,7 +2545,7 @@ static void create_enumvaldef(symtab_addctx* ctx, const char* prefix, // Allocate and initialize an array of |n| enum value defs. // TODO(b/243726666): This will eventually become the (only) public constructor. static upb_EnumValueDef* _upb_EnumValueDefs_New( - symtab_addctx* ctx, const char* prefix, int n, + upb_AddDefCtx* ctx, const char* prefix, int n, const google_protobuf_EnumValueDescriptorProto* const* protos, upb_EnumDef* e) { upb_EnumValueDef* v = symtab_alloc(ctx, sizeof(upb_EnumValueDef) * n); @@ -2575,7 +2569,7 @@ static upb_EnumValueDef* _upb_EnumValueDefs_New( return v; } -static void create_enumdef(symtab_addctx* ctx, const char* prefix, +static void create_enumdef(upb_AddDefCtx* ctx, const char* prefix, const google_protobuf_EnumDescriptorProto* enum_proto, upb_EnumDef* e) { const google_protobuf_EnumValueDescriptorProto* const* values; @@ -2624,7 +2618,7 @@ static void create_enumdef(symtab_addctx* ctx, const char* prefix, // Allocate and initialize an array of |n| enum defs. // TODO(b/243726666): This will eventually become the (only) public constructor. static upb_EnumDef* _upb_EnumDefs_New( - symtab_addctx* ctx, int n, const google_protobuf_EnumDescriptorProto* const* protos, + upb_AddDefCtx* ctx, int n, const google_protobuf_EnumDescriptorProto* const* protos, const upb_MessageDef* containing_type) { // If a containing type is defined then get the full name from that. // Otherwise use the package name from the file def. @@ -2639,11 +2633,11 @@ static upb_EnumDef* _upb_EnumDefs_New( return e; } -static void msgdef_create_nested( - symtab_addctx* ctx, const google_protobuf_DescriptorProto* msg_proto, - upb_MessageDef* m); +static void msgdef_create_nested(upb_AddDefCtx* ctx, + const google_protobuf_DescriptorProto* msg_proto, + upb_MessageDef* m); -static void create_msgdef(symtab_addctx* ctx, const char* prefix, +static void create_msgdef(upb_AddDefCtx* ctx, const char* prefix, const google_protobuf_DescriptorProto* msg_proto, const upb_MessageDef* containing_type, const upb_MessageDef* _m) { @@ -2728,7 +2722,7 @@ static void create_msgdef(symtab_addctx* ctx, const char* prefix, // Allocate and initialize an array of |n| message defs. static upb_MessageDef* _upb_MessageDefs_New( - symtab_addctx* ctx, int n, const google_protobuf_DescriptorProto* const* protos, + upb_AddDefCtx* ctx, int n, const google_protobuf_DescriptorProto* const* protos, const upb_MessageDef* containing_type) { const char* name = containing_type ? containing_type->full_name : ctx->file->package; @@ -2739,9 +2733,9 @@ static upb_MessageDef* _upb_MessageDefs_New( return m; } -static void msgdef_create_nested( - symtab_addctx* ctx, const google_protobuf_DescriptorProto* msg_proto, - upb_MessageDef* m) { +static void msgdef_create_nested(upb_AddDefCtx* ctx, + const google_protobuf_DescriptorProto* msg_proto, + upb_MessageDef* m) { size_t n; const google_protobuf_EnumDescriptorProto* const* enums = @@ -2765,7 +2759,7 @@ static void msgdef_create_nested( m->nested_msgs = _upb_MessageDefs_New(ctx, n, msgs, m); } -static void resolve_subdef(symtab_addctx* ctx, const char* prefix, +static void resolve_subdef(upb_AddDefCtx* ctx, const char* prefix, upb_FieldDef* f) { const google_protobuf_FieldDescriptorProto* field_proto = f->sub.unresolved; upb_StringView name = @@ -2811,9 +2805,9 @@ static void resolve_subdef(symtab_addctx* ctx, const char* prefix, } } -static void resolve_extension( - symtab_addctx* ctx, const char* prefix, upb_FieldDef* f, - const google_protobuf_FieldDescriptorProto* field_proto) { +static void resolve_extension(upb_AddDefCtx* ctx, const char* prefix, + upb_FieldDef* f, + const google_protobuf_FieldDescriptorProto* field_proto) { if (!google_protobuf_FieldDescriptorProto_has_extendee(field_proto)) { symtab_errf(ctx, "extension for field '%s' had no extendee", f->full_name); } @@ -2857,9 +2851,8 @@ static void resolve_extension( upb_value_constptr(f), ctx->arena)); } -static void resolve_default( - symtab_addctx* ctx, upb_FieldDef* f, - const google_protobuf_FieldDescriptorProto* field_proto) { +static void resolve_default(upb_AddDefCtx* ctx, upb_FieldDef* f, + const google_protobuf_FieldDescriptorProto* field_proto) { // Have to delay resolving of the default value until now because of the enum // case, since enum defaults are specified with a label. if (google_protobuf_FieldDescriptorProto_has_default_value(field_proto)) { @@ -2884,7 +2877,7 @@ static void resolve_default( } } -static void resolve_fielddef(symtab_addctx* ctx, const char* prefix, +static void resolve_fielddef(upb_AddDefCtx* ctx, const char* prefix, upb_FieldDef* f) { // We have to stash this away since resolve_subdef() may overwrite it. const google_protobuf_FieldDescriptorProto* field_proto = f->sub.unresolved; @@ -2897,7 +2890,7 @@ static void resolve_fielddef(symtab_addctx* ctx, const char* prefix, } } -static void resolve_msgdef(symtab_addctx* ctx, upb_MessageDef* m) { +static void resolve_msgdef(upb_AddDefCtx* ctx, upb_MessageDef* m) { for (int i = 0; i < m->field_count; i++) { resolve_fielddef(ctx, m->full_name, (upb_FieldDef*)&m->fields[i]); } @@ -2936,7 +2929,7 @@ static int count_exts_in_msg(const google_protobuf_DescriptorProto* msg_proto) { } // Allocate and initialize one file def, and add it to the context object. -static void _upb_FileDef_Create(symtab_addctx* ctx, +static void _upb_FileDef_Create(upb_AddDefCtx* ctx, const google_protobuf_FileDescriptorProto* file_proto) { upb_FileDef* file = symtab_alloc(ctx, sizeof(upb_FileDef)); ctx->file = file; @@ -3124,34 +3117,33 @@ static void remove_filedef(upb_DefPool* s, upb_FileDef* file) { static const upb_FileDef* _upb_DefPool_AddFile( upb_DefPool* s, const google_protobuf_FileDescriptorProto* file_proto, const upb_MiniTable_File* layout, upb_Status* status) { - symtab_addctx ctx; - upb_StringView name = google_protobuf_FileDescriptorProto_name(file_proto); - upb_value v; - - if (upb_strtable_lookup2(&s->files, name.data, name.size, &v)) { - upb_Status_SetErrorFormat(status, "duplicate file name (%.*s)", - UPB_STRINGVIEW_ARGS(name)); - return NULL; - } - - ctx.symtab = s; - ctx.layout = layout; - ctx.msg_count = 0; - ctx.enum_count = 0; - ctx.ext_count = 0; - ctx.status = status; - ctx.file = NULL; - ctx.arena = upb_Arena_New(); - ctx.tmp_arena = upb_Arena_New(); + const upb_StringView name = google_protobuf_FileDescriptorProto_name(file_proto); + + // Determine whether we already know about this file. + { + upb_value v; + if (upb_strtable_lookup2(&s->files, name.data, name.size, &v)) { + upb_Status_SetErrorFormat(status, "duplicate file name (%.*s)", + UPB_STRINGVIEW_ARGS(name)); + return NULL; + } + } + + upb_AddDefCtx ctx = { + .symtab = s, + .layout = layout, + .msg_count = 0, + .enum_count = 0, + .ext_count = 0, + .status = status, + .file = NULL, + .arena = upb_Arena_New(), + .tmp_arena = upb_Arena_New(), + }; if (!ctx.arena || !ctx.tmp_arena) { - if (ctx.arena) upb_Arena_Free(ctx.arena); - if (ctx.tmp_arena) upb_Arena_Free(ctx.tmp_arena); - upb_Status_setoom(status); - return NULL; - } - - if (UPB_UNLIKELY(UPB_SETJMP(ctx.err))) { + upb_Status_SetErrorMessage(status, kOutOfMemory); + } else if (UPB_SETJMP(ctx.err)) { UPB_ASSERT(!upb_Status_IsOk(status)); if (ctx.file) { remove_filedef(s, ctx.file); @@ -3165,8 +3157,8 @@ static const upb_FileDef* _upb_DefPool_AddFile( upb_Arena_Fuse(s->arena, ctx.arena); } - upb_Arena_Free(ctx.arena); - upb_Arena_Free(ctx.tmp_arena); + if (ctx.arena) upb_Arena_Free(ctx.arena); + if (ctx.tmp_arena) upb_Arena_Free(ctx.tmp_arena); return ctx.file; } From 02ef81247a4e16d946003692e34475e82eb4200a Mon Sep 17 00:00:00 2001 From: Eric Salo Date: Mon, 29 Aug 2022 13:52:20 -0700 Subject: [PATCH 12/12] rename some symtab functions to follow the upb style guide: - check_ident() is now _upb_AddDefCtx_CheckIdent() - makefullname() is now _upb_AddDefCtx_MakeFullName() - symtab_add() is now _upb_AddDefCtx_Add() - symtab_alloc() is now _upb_AddDefCtx_Alloc() - symtab_errf() is now _upb_AddDefCtx_Errf() - symtab_oomerr() is now _upb_AddDefCtx_OomErr() PiperOrigin-RevId: 470806376 --- upb/def.c | 319 +++++++++++++++++++++++++++++------------------------- 1 file changed, 171 insertions(+), 148 deletions(-) diff --git a/upb/def.c b/upb/def.c index a24cf91dee..7ee261eef0 100644 --- a/upb/def.c +++ b/upb/def.c @@ -53,8 +53,6 @@ typedef struct { static const char opt_default_buf[_UPB_MAXOPT_SIZE + sizeof(void*)] = {0}; static const char* opt_default = &opt_default_buf[sizeof(void*)]; -static const char kOutOfMemory[] = "out of memory"; - struct upb_FieldDef { const google_protobuf_FieldOptions* opts; const upb_FileDef* file; @@ -382,7 +380,7 @@ typedef struct { jmp_buf err; /* longjmp() on error. */ } upb_AddDefCtx; -UPB_NORETURN UPB_NOINLINE UPB_PRINTF(2, 3) static void symtab_errf( +UPB_NORETURN UPB_NOINLINE UPB_PRINTF(2, 3) static void _upb_AddDefCtx_Errf( upb_AddDefCtx* ctx, const char* fmt, ...) { va_list argp; va_start(argp, fmt); @@ -391,15 +389,16 @@ UPB_NORETURN UPB_NOINLINE UPB_PRINTF(2, 3) static void symtab_errf( UPB_LONGJMP(ctx->err, 1); } -UPB_NORETURN UPB_NOINLINE static void symtab_oomerr(upb_AddDefCtx* ctx) { - upb_Status_SetErrorMessage(ctx->status, kOutOfMemory); +UPB_NORETURN UPB_NOINLINE static void _upb_AddDefCtx_OomErr( + upb_AddDefCtx* ctx) { + upb_Status_SetErrorMessage(ctx->status, "out of memory"); UPB_LONGJMP(ctx->err, 1); } -void* symtab_alloc(upb_AddDefCtx* ctx, size_t bytes) { +void* _upb_AddDefCtx_Alloc(upb_AddDefCtx* ctx, size_t bytes) { if (bytes == 0) return NULL; void* ret = upb_Arena_Malloc(ctx->arena, bytes); - if (!ret) symtab_oomerr(ctx); + if (!ret) _upb_AddDefCtx_OomErr(ctx); return ret; } @@ -525,7 +524,7 @@ int32_t upb_ExtensionRange_End(const upb_ExtensionRange* e) { return e->end; } // Allocate sufficient storage to contain an array of |n| extension ranges. static upb_ExtensionRange* _upb_ExtensionRange_Alloc(upb_AddDefCtx* ctx, int n) { - return symtab_alloc(ctx, sizeof(upb_ExtensionRange) * n); + return _upb_AddDefCtx_Alloc(ctx, sizeof(upb_ExtensionRange) * n); } /* upb_FieldDef ***************************************************************/ @@ -745,7 +744,7 @@ bool upb_FieldDef_checkdescriptortype(int32_t type) { // Allocate sufficient storage to contain an array of |n| field defs. static upb_FieldDef* _upb_FieldDef_Alloc(upb_AddDefCtx* ctx, int n) { - return symtab_alloc(ctx, sizeof(upb_FieldDef) * n); + return _upb_AddDefCtx_Alloc(ctx, sizeof(upb_FieldDef) * n); } /* upb_MessageDef *************************************************************/ @@ -1303,9 +1302,9 @@ const upb_FileDef* upb_DefPool_FindFileContainingSymbol(const upb_DefPool* s, * this code is used to directly build defs from Ruby (for example) we do need * to validate important constraints like uniqueness of names and numbers. */ -#define CHK_OOM(x) \ - if (!(x)) { \ - symtab_oomerr(ctx); \ +#define CHK_OOM(x) \ + if (!(x)) { \ + _upb_AddDefCtx_OomErr(ctx); \ } // We want to copy the options verbatim into the destination options proto. @@ -1322,7 +1321,8 @@ const upb_FileDef* upb_DefPool_FindFileContainingSymbol(const upb_DefPool* s, target = (const google_protobuf_##options_type*)opt_default; \ } -static void check_ident(upb_AddDefCtx* ctx, upb_StringView name, bool full) { +static void _upb_AddDefCtx_CheckIdent(upb_AddDefCtx* ctx, upb_StringView name, + bool full) { const char* str = name.data; size_t len = name.size; bool start = true; @@ -1331,12 +1331,13 @@ static void check_ident(upb_AddDefCtx* ctx, upb_StringView name, bool full) { char c = str[i]; if (c == '.') { if (start || !full) { - symtab_errf(ctx, "invalid name: unexpected '.' (%.*s)", (int)len, str); + _upb_AddDefCtx_Errf(ctx, "invalid name: unexpected '.' (%.*s)", + (int)len, str); } start = true; } else if (start) { if (!upb_isletter(c)) { - symtab_errf( + _upb_AddDefCtx_Errf( ctx, "invalid name: path components must start with a letter (%.*s)", (int)len, str); @@ -1344,13 +1345,14 @@ static void check_ident(upb_AddDefCtx* ctx, upb_StringView name, bool full) { start = false; } else { if (!upb_isalphanum(c)) { - symtab_errf(ctx, "invalid name: non-alphanumeric character (%.*s)", - (int)len, str); + _upb_AddDefCtx_Errf(ctx, + "invalid name: non-alphanumeric character (%.*s)", + (int)len, str); } } } if (start) { - symtab_errf(ctx, "invalid name: empty part (%.*s)", (int)len, str); + _upb_AddDefCtx_Errf(ctx, "invalid name: empty part (%.*s)", (int)len, str); } } @@ -1396,8 +1398,9 @@ static uint32_t upb_MiniTable_place(upb_AddDefCtx* ctx, upb_MiniTable* l, size_t next = ofs + size; if (next > UINT16_MAX) { - symtab_errf(ctx, "size of message %s exceeded max size of %zu bytes", - upb_MessageDef_FullName(m), (size_t)UINT16_MAX); + _upb_AddDefCtx_Errf(ctx, + "size of message %s exceeded max size of %zu bytes", + upb_MessageDef_FullName(m), (size_t)UINT16_MAX); } l->size = next; @@ -1522,8 +1525,8 @@ static void make_layout(upb_AddDefCtx* ctx, const upb_MessageDef* m) { } } - fields = symtab_alloc(ctx, field_count * sizeof(*fields)); - subs = symtab_alloc(ctx, sublayout_count * sizeof(*subs)); + fields = _upb_AddDefCtx_Alloc(ctx, field_count * sizeof(*fields)); + subs = _upb_AddDefCtx_Alloc(ctx, sublayout_count * sizeof(*subs)); l->field_count = upb_MessageDef_numfields(m); l->fields = fields; @@ -1598,8 +1601,8 @@ static void make_layout(upb_AddDefCtx* ctx, const upb_MessageDef* m) { if (upb_FieldDef_Label(f) == kUpb_Label_Required) { field->presence = ++hasbit; if (hasbit >= 63) { - symtab_errf(ctx, "Message with >=63 required fields: %s", - upb_MessageDef_FullName(m)); + _upb_AddDefCtx_Errf(ctx, "Message with >=63 required fields: %s", + upb_MessageDef_FullName(m)); } l->required_count++; } @@ -1664,7 +1667,8 @@ static void make_layout(upb_AddDefCtx* ctx, const upb_MessageDef* m) { if (upb_OneofDef_IsSynthetic(o)) continue; if (o->field_count == 0) { - symtab_errf(ctx, "Oneof must have at least one field (%s)", o->full_name); + _upb_AddDefCtx_Errf(ctx, "Oneof must have at least one field (%s)", + o->full_name); } /* Calculate field size: the max of all field sizes. */ @@ -1710,12 +1714,13 @@ static bool streql_view(upb_StringView view, const char* b) { return streql2(view.data, view.size, b); } -static const char* makefullname(upb_AddDefCtx* ctx, const char* prefix, - upb_StringView name) { +static const char* _upb_AddDefCtx_MakeFullName(upb_AddDefCtx* ctx, + const char* prefix, + upb_StringView name) { if (prefix) { /* ret = prefix + '.' + name; */ size_t n = strlen(prefix); - char* ret = symtab_alloc(ctx, n + name.size + 2); + char* ret = _upb_AddDefCtx_Alloc(ctx, n + name.size + 2); strcpy(ret, prefix); ret[n] = '.'; memcpy(&ret[n + 1], name.data, name.size); @@ -1735,18 +1740,21 @@ static void finalize_oneofs(upb_AddDefCtx* ctx, upb_MessageDef* m) { upb_OneofDef* o = &mutable_oneofs[i]; if (o->synthetic && o->field_count != 1) { - symtab_errf(ctx, "Synthetic oneofs must have one field, not %d: %s", - o->field_count, upb_OneofDef_Name(o)); + _upb_AddDefCtx_Errf(ctx, + "Synthetic oneofs must have one field, not %d: %s", + o->field_count, upb_OneofDef_Name(o)); } if (o->synthetic) { synthetic_count++; } else if (synthetic_count != 0) { - symtab_errf(ctx, "Synthetic oneofs must be after all other oneofs: %s", - upb_OneofDef_Name(o)); + _upb_AddDefCtx_Errf(ctx, + "Synthetic oneofs must be after all other oneofs: %s", + upb_OneofDef_Name(o)); } - o->fields = symtab_alloc(ctx, sizeof(upb_FieldDef*) * o->field_count); + o->fields = + _upb_AddDefCtx_Alloc(ctx, sizeof(upb_FieldDef*) * o->field_count); o->field_count = 0; } @@ -1803,19 +1811,20 @@ size_t getjsonname(const char* name, char* buf, size_t len) { static char* makejsonname(upb_AddDefCtx* ctx, const char* name) { size_t size = getjsonname(name, NULL, 0); - char* json_name = symtab_alloc(ctx, size); + char* json_name = _upb_AddDefCtx_Alloc(ctx, size); getjsonname(name, json_name, size); return json_name; } /* Adds a symbol |v| to the symtab, which must be a def pointer previously - * packed with pack_def(). The def's pointer to upb_FileDef* must be set before + * packed with pack_def(). The def's pointer to upb_FileDef* must be set before * adding, so we know which entries to remove if building this file fails. */ -static void symtab_add(upb_AddDefCtx* ctx, const char* name, upb_value v) { +static void _upb_AddDefCtx_Add(upb_AddDefCtx* ctx, const char* name, + upb_value v) { // TODO: table should support an operation "tryinsert" to avoid the double // lookup. if (upb_strtable_lookup(&ctx->symtab->syms, name, NULL)) { - symtab_errf(ctx, "duplicate symbol '%s'", name); + _upb_AddDefCtx_Errf(ctx, "duplicate symbol '%s'", name); } size_t len = strlen(name); CHK_OOM(upb_strtable_insert(&ctx->symtab->syms, name, len, v, @@ -1879,8 +1888,8 @@ static const void* symtab_resolveany(upb_AddDefCtx* ctx, return unpack_def(v, *type); notfound: - symtab_errf(ctx, "couldn't resolve name '" UPB_STRINGVIEW_FORMAT "'", - UPB_STRINGVIEW_ARGS(sym)); + _upb_AddDefCtx_Errf(ctx, "couldn't resolve name '" UPB_STRINGVIEW_FORMAT "'", + UPB_STRINGVIEW_ARGS(sym)); } static const void* symtab_resolve(upb_AddDefCtx* ctx, const char* from_name_dbg, @@ -1890,10 +1899,10 @@ static const void* symtab_resolve(upb_AddDefCtx* ctx, const char* from_name_dbg, const void* ret = symtab_resolveany(ctx, from_name_dbg, base, sym, &found_type); if (ret && found_type != type) { - symtab_errf(ctx, - "type mismatch when resolving %s: couldn't find " - "name " UPB_STRINGVIEW_FORMAT " with type=%d", - from_name_dbg, UPB_STRINGVIEW_ARGS(sym), (int)type); + _upb_AddDefCtx_Errf(ctx, + "type mismatch when resolving %s: couldn't find " + "name " UPB_STRINGVIEW_FORMAT " with type=%d", + from_name_dbg, UPB_STRINGVIEW_ARGS(sym), (int)type); } return ret; } @@ -1906,7 +1915,7 @@ static void create_oneofdef(upb_AddDefCtx* ctx, upb_MessageDef* m, upb_value v; o->parent = m; - o->full_name = makefullname(ctx, m->full_name, name); + o->full_name = _upb_AddDefCtx_MakeFullName(ctx, m->full_name, name); o->field_count = 0; o->synthetic = false; @@ -1914,7 +1923,7 @@ static void create_oneofdef(upb_AddDefCtx* ctx, upb_MessageDef* m, upb_value existing_v; if (upb_strtable_lookup2(&m->ntof, name.data, name.size, &existing_v)) { - symtab_errf(ctx, "duplicate oneof name (%s)", o->full_name); + _upb_AddDefCtx_Errf(ctx, "duplicate oneof name (%s)", o->full_name); } v = pack_def(o, UPB_DEFTYPE_ONEOF); @@ -1928,7 +1937,7 @@ static void create_oneofdef(upb_AddDefCtx* ctx, upb_MessageDef* m, static upb_OneofDef* _upb_OneofDefs_New( upb_AddDefCtx* ctx, int n, const google_protobuf_OneofDescriptorProto* const* protos, upb_MessageDef* m) { - upb_OneofDef* o = symtab_alloc(ctx, sizeof(upb_OneofDef) * n); + upb_OneofDef* o = _upb_AddDefCtx_Alloc(ctx, sizeof(upb_OneofDef) * n); for (int i = 0; i < n; i++) { create_oneofdef(ctx, m, protos[i], &o[i]); } @@ -1936,7 +1945,7 @@ static upb_OneofDef* _upb_OneofDefs_New( } static str_t* newstr(upb_AddDefCtx* ctx, const char* data, size_t len) { - str_t* ret = symtab_alloc(ctx, sizeof(*ret) + len); + str_t* ret = _upb_AddDefCtx_Alloc(ctx, sizeof(*ret) + len); CHK_OOM(ret); ret->len = len; if (len) memcpy(ret->str, data, len); @@ -1973,9 +1982,9 @@ static char upb_DefPool_ParseHexEscape(upb_AddDefCtx* ctx, const char* end) { char hex_digit = upb_DefPool_TryGetHexDigit(ctx, f, src, end); if (hex_digit < 0) { - symtab_errf(ctx, - "\\x cannot be followed by non-hex digit in field '%s' default", - upb_FieldDef_FullName(f)); + _upb_AddDefCtx_Errf( + ctx, "\\x cannot be followed by non-hex digit in field '%s' default", + upb_FieldDef_FullName(f)); return 0; } unsigned int ret = hex_digit; @@ -1983,8 +1992,8 @@ static char upb_DefPool_ParseHexEscape(upb_AddDefCtx* ctx, ret = (ret << 4) | hex_digit; } if (ret > 0xff) { - symtab_errf(ctx, "Value of hex escape in field %s exceeds 8 bits", - upb_FieldDef_FullName(f)); + _upb_AddDefCtx_Errf(ctx, "Value of hex escape in field %s exceeds 8 bits", + upb_FieldDef_FullName(f)); return 0; } return ret; @@ -2017,8 +2026,8 @@ static char upb_DefPool_ParseEscape(upb_AddDefCtx* ctx, const upb_FieldDef* f, const char** src, const char* end) { char ch; if (!upb_DefPool_TryGetChar(src, end, &ch)) { - symtab_errf(ctx, "unterminated escape sequence in field %s", - upb_FieldDef_FullName(f)); + _upb_AddDefCtx_Errf(ctx, "unterminated escape sequence in field %s", + upb_FieldDef_FullName(f)); return 0; } switch (ch) { @@ -2058,13 +2067,13 @@ static char upb_DefPool_ParseEscape(upb_AddDefCtx* ctx, const upb_FieldDef* f, *src -= 1; return upb_DefPool_ParseOctalEscape(ctx, f, src, end); } - symtab_errf(ctx, "Unknown escape sequence: \\%c", ch); + _upb_AddDefCtx_Errf(ctx, "Unknown escape sequence: \\%c", ch); } static str_t* unescape(upb_AddDefCtx* ctx, const upb_FieldDef* f, const char* data, size_t len) { // Size here is an upper bound; escape sequences could ultimately shrink it. - str_t* ret = symtab_alloc(ctx, sizeof(*ret) + len); + str_t* ret = _upb_AddDefCtx_Alloc(ctx, sizeof(*ret) + len); char* dst = &ret->str[0]; const char* src = data; const char* end = data + len; @@ -2097,7 +2106,7 @@ static void parse_default(upb_AddDefCtx* ctx, const char* str, size_t len, case kUpb_CType_Float: /* Standard C number parsing functions expect null-terminated strings. */ if (len >= sizeof(nullz) - 1) { - symtab_errf(ctx, "Default too long: %.*s", (int)len, str); + _upb_AddDefCtx_Errf(ctx, "Default too long: %.*s", (int)len, str); } memcpy(nullz, str, len); nullz[len] = '\0'; @@ -2184,15 +2193,16 @@ static void parse_default(upb_AddDefCtx* ctx, const char* str, size_t len, break; case kUpb_CType_Message: /* Should not have a default value. */ - symtab_errf(ctx, "Message should not have a default (%s)", - upb_FieldDef_FullName(f)); + _upb_AddDefCtx_Errf(ctx, "Message should not have a default (%s)", + upb_FieldDef_FullName(f)); } return; invalid: - symtab_errf(ctx, "Invalid default '%.*s' for field %s of type %d", (int)len, - str, upb_FieldDef_FullName(f), (int)upb_FieldDef_Type(f)); + _upb_AddDefCtx_Errf(ctx, "Invalid default '%.*s' for field %s of type %d", + (int)len, str, upb_FieldDef_FullName(f), + (int)upb_FieldDef_Type(f)); } static void set_default_default(upb_AddDefCtx* ctx, upb_FieldDef* f) { @@ -2236,15 +2246,15 @@ static void create_fielddef(upb_AddDefCtx* ctx, const char* prefix, const char* shortname; int32_t field_number; - f->file = ctx->file; // Must happen prior to symtab_add() + f->file = ctx->file; // Must happen prior to _upb_AddDefCtx_Add() if (!google_protobuf_FieldDescriptorProto_has_name(field_proto)) { - symtab_errf(ctx, "field has no name"); + _upb_AddDefCtx_Errf(ctx, "field has no name"); } name = google_protobuf_FieldDescriptorProto_name(field_proto); - check_ident(ctx, name, false); - full_name = makefullname(ctx, prefix, name); + _upb_AddDefCtx_CheckIdent(ctx, name, false); + full_name = _upb_AddDefCtx_MakeFullName(ctx, prefix, name); shortname = shortdefname(full_name); if (google_protobuf_FieldDescriptorProto_has_json_name(field_proto)) { @@ -2278,14 +2288,15 @@ static void create_fielddef(upb_AddDefCtx* ctx, const char* prefix, case kUpb_FieldType_Group: case kUpb_FieldType_Enum: if (!has_type_name) { - symtab_errf(ctx, "field of type %d requires type name (%s)", - (int)f->type_, full_name); + _upb_AddDefCtx_Errf(ctx, "field of type %d requires type name (%s)", + (int)f->type_, full_name); } break; default: if (has_type_name) { - symtab_errf(ctx, "invalid type for field with type_name set (%s, %d)", - full_name, (int)f->type_); + _upb_AddDefCtx_Errf( + ctx, "invalid type for field with type_name set (%s, %d)", + full_name, (int)f->type_); } } } else if (has_type_name) { @@ -2299,7 +2310,7 @@ static void create_fielddef(upb_AddDefCtx* ctx, const char* prefix, size_t json_size; if (field_number <= 0 || field_number > kUpb_MaxFieldNumber) { - symtab_errf(ctx, "invalid field number (%u)", field_number); + _upb_AddDefCtx_Errf(ctx, "invalid field number (%u)", field_number); } f->index_ = f - m->fields; @@ -2312,7 +2323,7 @@ static void create_fielddef(upb_AddDefCtx* ctx, const char* prefix, json_size = strlen(json_name); if (upb_strtable_lookup(&m->ntof, shortname, &existing_v)) { - symtab_errf(ctx, "duplicate field name (%s)", shortname); + _upb_AddDefCtx_Errf(ctx, "duplicate field name (%s)", shortname); } CHK_OOM(upb_strtable_insert(&m->ntof, name.data, name.size, field_v, @@ -2320,7 +2331,7 @@ static void create_fielddef(upb_AddDefCtx* ctx, const char* prefix, if (strcmp(shortname, json_name) != 0) { if (upb_strtable_lookup(&m->ntof, json_name, &v)) { - symtab_errf(ctx, "duplicate json_name (%s)", json_name); + _upb_AddDefCtx_Errf(ctx, "duplicate json_name (%s)", json_name); } else { CHK_OOM(upb_strtable_insert(&m->ntof, json_name, json_size, json_v, ctx->arena)); @@ -2328,7 +2339,7 @@ static void create_fielddef(upb_AddDefCtx* ctx, const char* prefix, } if (upb_inttable_lookup(&m->itof, field_number, NULL)) { - symtab_errf(ctx, "duplicate field number (%u)", field_number); + _upb_AddDefCtx_Errf(ctx, "duplicate field number (%u)", field_number); } CHK_OOM(upb_inttable_insert(&m->itof, field_number, v, ctx->arena)); @@ -2351,7 +2362,7 @@ static void create_fielddef(upb_AddDefCtx* ctx, const char* prefix, /* extension field. */ f->is_extension_ = true; f->scope.extension_scope = m; - symtab_add(ctx, full_name, pack_def(f, UPB_DEFTYPE_EXT)); + _upb_AddDefCtx_Add(ctx, full_name, pack_def(f, UPB_DEFTYPE_EXT)); f->layout_index = ctx->ext_count++; if (ctx->layout) { UPB_ASSERT(_upb_FieldDef_ExtensionMiniTable(f)->field.number == @@ -2360,12 +2371,13 @@ static void create_fielddef(upb_AddDefCtx* ctx, const char* prefix, } if (f->type_ < kUpb_FieldType_Double || f->type_ > kUpb_FieldType_SInt64) { - symtab_errf(ctx, "invalid type for field %s (%d)", f->full_name, f->type_); + _upb_AddDefCtx_Errf(ctx, "invalid type for field %s (%d)", f->full_name, + f->type_); } if (f->label_ < kUpb_Label_Optional || f->label_ > kUpb_Label_Repeated) { - symtab_errf(ctx, "invalid label for field %s (%d)", f->full_name, - f->label_); + _upb_AddDefCtx_Errf(ctx, "invalid label for field %s (%d)", f->full_name, + f->label_); } /* We can't resolve the subdef or (in the case of extensions) the containing @@ -2375,7 +2387,8 @@ static void create_fielddef(upb_AddDefCtx* ctx, const char* prefix, if (f->label_ == kUpb_Label_Required && f->file->syntax == kUpb_Syntax_Proto3) { - symtab_errf(ctx, "proto3 fields cannot be required (%s)", f->full_name); + _upb_AddDefCtx_Errf(ctx, "proto3 fields cannot be required (%s)", + f->full_name); } if (google_protobuf_FieldDescriptorProto_has_oneof_index(field_proto)) { @@ -2384,17 +2397,17 @@ static void create_fielddef(upb_AddDefCtx* ctx, const char* prefix, upb_value v = upb_value_constptr(f); if (upb_FieldDef_Label(f) != kUpb_Label_Optional) { - symtab_errf(ctx, "fields in oneof must have OPTIONAL label (%s)", - f->full_name); + _upb_AddDefCtx_Errf(ctx, "fields in oneof must have OPTIONAL label (%s)", + f->full_name); } if (!m) { - symtab_errf(ctx, "oneof_index provided for extension field (%s)", - f->full_name); + _upb_AddDefCtx_Errf(ctx, "oneof_index provided for extension field (%s)", + f->full_name); } if (oneof_index >= m->oneof_count) { - symtab_errf(ctx, "oneof_index out of range (%s)", f->full_name); + _upb_AddDefCtx_Errf(ctx, "oneof_index out of range (%s)", f->full_name); } oneof = (upb_OneofDef*)&m->oneofs[oneof_index]; @@ -2409,8 +2422,9 @@ static void create_fielddef(upb_AddDefCtx* ctx, const char* prefix, upb_strtable_insert(&oneof->ntof, name.data, name.size, v, ctx->arena)); } else { if (f->proto3_optional_) { - symtab_errf(ctx, "field with proto3_optional was not in a oneof (%s)", - f->full_name); + _upb_AddDefCtx_Errf(ctx, + "field with proto3_optional was not in a oneof (%s)", + f->full_name); } } @@ -2432,7 +2446,7 @@ static void create_method(upb_AddDefCtx* ctx, upb_StringView name = google_protobuf_MethodDescriptorProto_name(method_proto); m->service = s; - m->full_name = makefullname(ctx, s->full_name, name); + m->full_name = _upb_AddDefCtx_MakeFullName(ctx, s->full_name, name); m->client_streaming = google_protobuf_MethodDescriptorProto_client_streaming(method_proto); m->server_streaming = @@ -2451,7 +2465,7 @@ static void create_method(upb_AddDefCtx* ctx, static upb_MethodDef* _upb_MethodDefs_New( upb_AddDefCtx* ctx, int n, const google_protobuf_MethodDescriptorProto* const* protos, upb_ServiceDef* s) { - upb_MethodDef* m = symtab_alloc(ctx, sizeof(upb_MethodDef) * n); + upb_MethodDef* m = _upb_AddDefCtx_Alloc(ctx, sizeof(upb_MethodDef) * n); for (int i = 0; i < n; i++) { create_method(ctx, protos[i], s, &m[i]); m[i].index = i; @@ -2465,12 +2479,12 @@ static void create_service(upb_AddDefCtx* ctx, upb_StringView name; size_t n; - s->file = ctx->file; // Must happen prior to symtab_add() + s->file = ctx->file; // Must happen prior to _upb_AddDefCtx_Add() name = google_protobuf_ServiceDescriptorProto_name(svc_proto); - check_ident(ctx, name, false); - s->full_name = makefullname(ctx, ctx->file->package, name); - symtab_add(ctx, s->full_name, pack_def(s, UPB_DEFTYPE_SERVICE)); + _upb_AddDefCtx_CheckIdent(ctx, name, false); + s->full_name = _upb_AddDefCtx_MakeFullName(ctx, ctx->file->package, name); + _upb_AddDefCtx_Add(ctx, s->full_name, pack_def(s, UPB_DEFTYPE_SERVICE)); const google_protobuf_MethodDescriptorProto* const* methods = google_protobuf_ServiceDescriptorProto_method(svc_proto, &n); @@ -2484,7 +2498,7 @@ static void create_service(upb_AddDefCtx* ctx, static upb_ServiceDef* _upb_ServiceDefs_New( upb_AddDefCtx* ctx, int n, const google_protobuf_ServiceDescriptorProto* const* protos) { - upb_ServiceDef* s = symtab_alloc(ctx, sizeof(upb_ServiceDef) * n); + upb_ServiceDef* s = _upb_AddDefCtx_Alloc(ctx, sizeof(upb_ServiceDef) * n); for (int i = 0; i < n; i++) { create_service(ctx, protos[i], &s[i]); s[i].index = i; @@ -2511,13 +2525,13 @@ static int compare_int32(const void* a_ptr, const void* b_ptr) { static upb_MiniTable_Enum* create_enumlayout(upb_AddDefCtx* ctx, const upb_EnumDef* e) { const char* desc = _upb_EnumDef_MiniDescriptor(e, ctx->tmp_arena); - if (!desc) symtab_errf(ctx, "OOM while building enum MiniDescriptor"); + if (!desc) _upb_AddDefCtx_Errf(ctx, "OOM while building enum MiniDescriptor"); upb_Status status; upb_MiniTable_Enum* layout = upb_MiniTable_BuildEnum(desc, strlen(desc), ctx->arena, &status); if (!layout) - symtab_errf(ctx, "Error building enum MiniTable: %s", status.msg); + _upb_AddDefCtx_Errf(ctx, "Error building enum MiniTable: %s", status.msg); return layout; } @@ -2527,10 +2541,10 @@ static void create_enumvaldef(upb_AddDefCtx* ctx, const char* prefix, upb_StringView name = google_protobuf_EnumValueDescriptorProto_name(val_proto); upb_value val = upb_value_constptr(v); - v->parent = e; // Must happen prior to symtab_add() - v->full_name = makefullname(ctx, prefix, name); + v->parent = e; // Must happen prior to _upb_AddDefCtx_Add() + v->full_name = _upb_AddDefCtx_MakeFullName(ctx, prefix, name); v->number = google_protobuf_EnumValueDescriptorProto_number(val_proto); - symtab_add(ctx, v->full_name, pack_def(v, UPB_DEFTYPE_ENUMVAL)); + _upb_AddDefCtx_Add(ctx, v->full_name, pack_def(v, UPB_DEFTYPE_ENUMVAL)); SET_OPTIONS(v->opts, EnumValueDescriptorProto, EnumValueOptions, val_proto); @@ -2547,7 +2561,7 @@ static void create_enumvaldef(upb_AddDefCtx* ctx, const char* prefix, static upb_EnumValueDef* _upb_EnumValueDefs_New( upb_AddDefCtx* ctx, const char* prefix, int n, const google_protobuf_EnumValueDescriptorProto* const* protos, upb_EnumDef* e) { - upb_EnumValueDef* v = symtab_alloc(ctx, sizeof(upb_EnumValueDef) * n); + upb_EnumValueDef* v = _upb_AddDefCtx_Alloc(ctx, sizeof(upb_EnumValueDef) * n); bool is_sorted = true; uint32_t previous = 0; @@ -2562,8 +2576,9 @@ static upb_EnumValueDef* _upb_EnumValueDefs_New( if (upb_FileDef_Syntax(ctx->file) == kUpb_Syntax_Proto3 && n > 0 && v[0].number != 0) { - symtab_errf(ctx, "for proto3, the first enum value must be zero (%s)", - e->full_name); + _upb_AddDefCtx_Errf(ctx, + "for proto3, the first enum value must be zero (%s)", + e->full_name); } return v; @@ -2576,13 +2591,13 @@ static void create_enumdef(upb_AddDefCtx* ctx, const char* prefix, upb_StringView name; size_t n; - e->file = ctx->file; // Must happen prior to symtab_add() + e->file = ctx->file; // Must happen prior to _upb_AddDefCtx_Add() name = google_protobuf_EnumDescriptorProto_name(enum_proto); - check_ident(ctx, name, false); + _upb_AddDefCtx_CheckIdent(ctx, name, false); - e->full_name = makefullname(ctx, prefix, name); - symtab_add(ctx, e->full_name, pack_def(e, UPB_DEFTYPE_ENUM)); + e->full_name = _upb_AddDefCtx_MakeFullName(ctx, prefix, name); + _upb_AddDefCtx_Add(ctx, e->full_name, pack_def(e, UPB_DEFTYPE_ENUM)); values = google_protobuf_EnumDescriptorProto_value(enum_proto, &n); CHK_OOM(upb_strtable_init(&e->ntoi, n, ctx->arena)); @@ -2593,8 +2608,8 @@ static void create_enumdef(upb_AddDefCtx* ctx, const char* prefix, e->values = _upb_EnumValueDefs_New(ctx, prefix, n, values, e); if (n == 0) { - symtab_errf(ctx, "enums must contain at least one value (%s)", - e->full_name); + _upb_AddDefCtx_Errf(ctx, "enums must contain at least one value (%s)", + e->full_name); } SET_OPTIONS(e->opts, EnumDescriptorProto, EnumOptions, enum_proto); @@ -2625,7 +2640,7 @@ static upb_EnumDef* _upb_EnumDefs_New( const char* name = containing_type ? containing_type->full_name : ctx->file->package; - upb_EnumDef* e = symtab_alloc(ctx, sizeof(upb_EnumDef) * n); + upb_EnumDef* e = _upb_AddDefCtx_Alloc(ctx, sizeof(upb_EnumDef) * n); for (size_t i = 0; i < n; i++) { create_enumdef(ctx, name, protos[i], &e[i]); e[i].containing_type = containing_type; @@ -2648,14 +2663,14 @@ static void create_msgdef(upb_AddDefCtx* ctx, const char* prefix, size_t i, n_oneof, n_field, n_ext_range; upb_StringView name; - m->file = ctx->file; // Must happen prior to symtab_add() + m->file = ctx->file; // Must happen prior to _upb_AddDefCtx_Add() m->containing_type = containing_type; name = google_protobuf_DescriptorProto_name(msg_proto); - check_ident(ctx, name, false); + _upb_AddDefCtx_CheckIdent(ctx, name, false); - m->full_name = makefullname(ctx, prefix, name); - symtab_add(ctx, m->full_name, pack_def(m, UPB_DEFTYPE_MSG)); + m->full_name = _upb_AddDefCtx_MakeFullName(ctx, prefix, name); + _upb_AddDefCtx_Add(ctx, m->full_name, pack_def(m, UPB_DEFTYPE_MSG)); oneofs = google_protobuf_DescriptorProto_oneof_decl(msg_proto, &n_oneof); fields = google_protobuf_DescriptorProto_field(msg_proto, &n_field); @@ -2672,8 +2687,8 @@ static void create_msgdef(upb_AddDefCtx* ctx, const char* prefix, UPB_ASSERT(n_field == m->layout->field_count); } else { /* Allocate now (to allow cross-linking), populate later. */ - m->layout = - symtab_alloc(ctx, sizeof(*m->layout) + sizeof(_upb_FastTable_Entry)); + m->layout = _upb_AddDefCtx_Alloc( + ctx, sizeof(*m->layout) + sizeof(_upb_FastTable_Entry)); } SET_OPTIONS(m->opts, DescriptorProto, MessageOptions, msg_proto); @@ -2704,8 +2719,9 @@ static void create_msgdef(upb_AddDefCtx* ctx, const char* prefix, // none of the fields overlap with the extension ranges, but we are just // sanity checking here. if (start < 1 || end <= start || end > max) { - symtab_errf(ctx, "Extension range (%d, %d) is invalid, message=%s\n", - (int)start, (int)end, m->full_name); + _upb_AddDefCtx_Errf(ctx, + "Extension range (%d, %d) is invalid, message=%s\n", + (int)start, (int)end, m->full_name); } r_def->start = start; @@ -2726,7 +2742,7 @@ static upb_MessageDef* _upb_MessageDefs_New( const upb_MessageDef* containing_type) { const char* name = containing_type ? containing_type->full_name : ctx->file->package; - upb_MessageDef* m = symtab_alloc(ctx, sizeof(upb_MessageDef) * n); + upb_MessageDef* m = _upb_AddDefCtx_Alloc(ctx, sizeof(upb_MessageDef) * n); for (int i = 0; i < n; i++) { create_msgdef(ctx, name, protos[i], containing_type, &m[i]); } @@ -2784,8 +2800,8 @@ static void resolve_subdef(upb_AddDefCtx* ctx, const char* prefix, // this being a group. break; default: - symtab_errf(ctx, "Couldn't resolve type name for field %s", - f->full_name); + _upb_AddDefCtx_Errf(ctx, "Couldn't resolve type name for field %s", + f->full_name); } } case kUpb_FieldType_Message: @@ -2809,7 +2825,8 @@ static void resolve_extension(upb_AddDefCtx* ctx, const char* prefix, upb_FieldDef* f, const google_protobuf_FieldDescriptorProto* field_proto) { if (!google_protobuf_FieldDescriptorProto_has_extendee(field_proto)) { - symtab_errf(ctx, "extension for field '%s' had no extendee", f->full_name); + _upb_AddDefCtx_Errf(ctx, "extension for field '%s' had no extendee", + f->full_name); } upb_StringView name = google_protobuf_FieldDescriptorProto_extendee(field_proto); @@ -2828,10 +2845,11 @@ static void resolve_extension(upb_AddDefCtx* ctx, const char* prefix, } if (!found) { - symtab_errf(ctx, - "field number %u in extension %s has no extension range in " - "message %s", - (unsigned)f->number_, f->full_name, f->msgdef->full_name); + _upb_AddDefCtx_Errf( + ctx, + "field number %u in extension %s has no extension range in " + "message %s", + (unsigned)f->number_, f->full_name, f->msgdef->full_name); } const upb_MiniTable_Extension* ext = _upb_FieldDef_ExtensionMiniTable(f); @@ -2860,13 +2878,15 @@ static void resolve_default(upb_AddDefCtx* ctx, upb_FieldDef* f, google_protobuf_FieldDescriptorProto_default_value(field_proto); if (upb_FileDef_Syntax(f->file) == kUpb_Syntax_Proto3) { - symtab_errf(ctx, "proto3 fields cannot have explicit defaults (%s)", - f->full_name); + _upb_AddDefCtx_Errf(ctx, + "proto3 fields cannot have explicit defaults (%s)", + f->full_name); } if (upb_FieldDef_IsSubMessage(f)) { - symtab_errf(ctx, "message fields cannot have explicit defaults (%s)", - f->full_name); + _upb_AddDefCtx_Errf(ctx, + "message fields cannot have explicit defaults (%s)", + f->full_name); } parse_default(ctx, defaultval.data, defaultval.size, f); @@ -2931,7 +2951,7 @@ static int count_exts_in_msg(const google_protobuf_DescriptorProto* msg_proto) { // Allocate and initialize one file def, and add it to the context object. static void _upb_FileDef_Create(upb_AddDefCtx* ctx, const google_protobuf_FileDescriptorProto* file_proto) { - upb_FileDef* file = symtab_alloc(ctx, sizeof(upb_FileDef)); + upb_FileDef* file = _upb_AddDefCtx_Alloc(ctx, sizeof(upb_FileDef)); ctx->file = file; const google_protobuf_DescriptorProto* const* msgs; @@ -2958,29 +2978,30 @@ static void _upb_FileDef_Create(upb_AddDefCtx* ctx, /* We are using the ext layouts that were passed in. */ file->ext_layouts = ctx->layout->exts; if (ctx->layout->ext_count != file->ext_count) { - symtab_errf(ctx, "Extension count did not match layout (%d vs %d)", - ctx->layout->ext_count, file->ext_count); + _upb_AddDefCtx_Errf(ctx, + "Extension count did not match layout (%d vs %d)", + ctx->layout->ext_count, file->ext_count); } } else { /* We are building ext layouts from scratch. */ file->ext_layouts = - symtab_alloc(ctx, sizeof(*file->ext_layouts) * file->ext_count); + _upb_AddDefCtx_Alloc(ctx, sizeof(*file->ext_layouts) * file->ext_count); upb_MiniTable_Extension* ext = - symtab_alloc(ctx, sizeof(*ext) * file->ext_count); + _upb_AddDefCtx_Alloc(ctx, sizeof(*ext) * file->ext_count); for (int i = 0; i < file->ext_count; i++) { file->ext_layouts[i] = &ext[i]; } } if (!google_protobuf_FileDescriptorProto_has_name(file_proto)) { - symtab_errf(ctx, "File has no name"); + _upb_AddDefCtx_Errf(ctx, "File has no name"); } file->name = strviewdup(ctx, google_protobuf_FileDescriptorProto_name(file_proto)); upb_StringView package = google_protobuf_FileDescriptorProto_package(file_proto); if (package.size) { - check_ident(ctx, package, true); + _upb_AddDefCtx_CheckIdent(ctx, package, true); file->package = strviewdup(ctx, package); } else { file->package = NULL; @@ -2994,8 +3015,8 @@ static void _upb_FileDef_Create(upb_AddDefCtx* ctx, } else if (streql_view(syntax, "proto3")) { file->syntax = kUpb_Syntax_Proto3; } else { - symtab_errf(ctx, "Invalid syntax '" UPB_STRINGVIEW_FORMAT "'", - UPB_STRINGVIEW_ARGS(syntax)); + _upb_AddDefCtx_Errf(ctx, "Invalid syntax '" UPB_STRINGVIEW_FORMAT "'", + UPB_STRINGVIEW_ARGS(syntax)); } } else { file->syntax = kUpb_Syntax_Proto2; @@ -3007,38 +3028,40 @@ static void _upb_FileDef_Create(upb_AddDefCtx* ctx, /* Verify dependencies. */ strs = google_protobuf_FileDescriptorProto_dependency(file_proto, &n); file->dep_count = n; - file->deps = symtab_alloc(ctx, sizeof(*file->deps) * n); + file->deps = _upb_AddDefCtx_Alloc(ctx, sizeof(*file->deps) * n); for (i = 0; i < n; i++) { upb_StringView str = strs[i]; file->deps[i] = upb_DefPool_FindFileByNameWithSize(ctx->symtab, str.data, str.size); if (!file->deps[i]) { - symtab_errf(ctx, - "Depends on file '" UPB_STRINGVIEW_FORMAT - "', but it has not been loaded", - UPB_STRINGVIEW_ARGS(str)); + _upb_AddDefCtx_Errf(ctx, + "Depends on file '" UPB_STRINGVIEW_FORMAT + "', but it has not been loaded", + UPB_STRINGVIEW_ARGS(str)); } } public_deps = google_protobuf_FileDescriptorProto_public_dependency(file_proto, &n); file->public_dep_count = n; - file->public_deps = symtab_alloc(ctx, sizeof(*file->public_deps) * n); + file->public_deps = _upb_AddDefCtx_Alloc(ctx, sizeof(*file->public_deps) * n); int32_t* mutable_public_deps = (int32_t*)file->public_deps; for (i = 0; i < n; i++) { if (public_deps[i] >= file->dep_count) { - symtab_errf(ctx, "public_dep %d is out of range", (int)public_deps[i]); + _upb_AddDefCtx_Errf(ctx, "public_dep %d is out of range", + (int)public_deps[i]); } mutable_public_deps[i] = public_deps[i]; } weak_deps = google_protobuf_FileDescriptorProto_weak_dependency(file_proto, &n); file->weak_dep_count = n; - file->weak_deps = symtab_alloc(ctx, sizeof(*file->weak_deps) * n); + file->weak_deps = _upb_AddDefCtx_Alloc(ctx, sizeof(*file->weak_deps) * n); int32_t* mutable_weak_deps = (int32_t*)file->weak_deps; for (i = 0; i < n; i++) { if (weak_deps[i] >= file->dep_count) { - symtab_errf(ctx, "weak_dep %d is out of range", (int)weak_deps[i]); + _upb_AddDefCtx_Errf(ctx, "weak_dep %d is out of range", + (int)weak_deps[i]); } mutable_weak_deps[i] = weak_deps[i]; } @@ -3141,14 +3164,14 @@ static const upb_FileDef* _upb_DefPool_AddFile( .tmp_arena = upb_Arena_New(), }; - if (!ctx.arena || !ctx.tmp_arena) { - upb_Status_SetErrorMessage(status, kOutOfMemory); - } else if (UPB_SETJMP(ctx.err)) { + if (UPB_SETJMP(ctx.err)) { UPB_ASSERT(!upb_Status_IsOk(status)); if (ctx.file) { remove_filedef(s, ctx.file); ctx.file = NULL; } + } else if (!ctx.arena || !ctx.tmp_arena) { + _upb_AddDefCtx_OomErr(&ctx); } else { _upb_FileDef_Create(&ctx, file_proto); upb_strtable_insert(&s->files, name.data, name.size,