From ee49a8d7df43163f0c552b837f7a204d74036a4e Mon Sep 17 00:00:00 2001 From: Joshua Haberman <jhaberman@gmail.com> Date: Sun, 6 Dec 2020 17:24:30 -0800 Subject: [PATCH] Added an accessor to get the symtab from a filedef. This matches an API already present in proto2 (const DescriptorPool* FileDescriptor::pool()). However there is a slightly subtle implication here. In proto2, the relationship between Descriptor and MessageFactory is 1:many. You can create as many DynamicMessageFactory instances as you want, and each one will have its own independent DynamicMessage prototype and computed layout for the same underlying Descriptor. In practice the layouts will all be the same, but one thing that could be distinct is that each can have its own extension pool, which is a DescriptorPool that will be searched for extensions when parsing. In contrast, upb does not have a separate "message factory" abstraction. That means that each upb_msgdef has a single distinct layout, in other words a 1:1 correspondence between descriptor and layout. This means that there is no way to create multiple message types for the same descriptor that have distinct extension pools. If you want a different set of extensions, you must create a separate upb_symtab with a distinct set of descriptors. This change further entrenches that upb_filedef:upb_symtab is a 1:1 relationship. A single upb_filedef cannot be a member of multiple symbol tables. In practice this was already true (there is no way to add a single filedef to multiple symbol tables) but this change codifies this 1:1 relationship. --- tests/bindings/lua/test_upb.lua | 14 ++++++++++++++ upb/bindings/lua/def.c | 12 ++++++++++++ upb/def.c | 8 +++++++- upb/def.h | 1 + 4 files changed, 34 insertions(+), 1 deletion(-) diff --git a/tests/bindings/lua/test_upb.lua b/tests/bindings/lua/test_upb.lua index 64696a52a0..b966fcc4be 100644 --- a/tests/bindings/lua/test_upb.lua +++ b/tests/bindings/lua/test_upb.lua @@ -651,6 +651,20 @@ function test_foo() assert_equal(set.file[1].name, "google/protobuf/descriptor.proto") end +function test_descriptor() + local symtab = upb.SymbolTable() + local file_proto = descriptor.FileDescriptorProto { + name = "test.proto", + message_type = upb.Array(descriptor.DescriptorProto, { + descriptor.DescriptorProto{ + name = "ABC", + }, + }) + } + local file = symtab:add_file(upb.encode(file_proto)) + assert_equal(file:symtab(), symtab) +end + function test_descriptor_error() local symtab = upb.SymbolTable() local file = descriptor.FileDescriptorProto() diff --git a/upb/bindings/lua/def.c b/upb/bindings/lua/def.c index ca277c8354..5a4d42c4ac 100644 --- a/upb/bindings/lua/def.c +++ b/upb/bindings/lua/def.c @@ -668,6 +668,13 @@ static int lupb_filedef_package(lua_State *L) { return 1; } +static int lupb_filedef_symtab(lua_State *L) { + const upb_filedef *f = lupb_filedef_check(L, 1); + const upb_symtab *symtab = upb_filedef_symtab(f); + lupb_wrapper_pushwrapper(L, 1, symtab, LUPB_SYMTAB); + return 1; +} + static int lupb_filedef_syntax(lua_State *L) { const upb_filedef *f = lupb_filedef_check(L, 1); lua_pushnumber(L, upb_filedef_syntax(f)); @@ -683,6 +690,7 @@ static const struct luaL_Reg lupb_filedef_m[] = { {"msgcount", lupb_filedef_msgcount}, {"name", lupb_filedef_name}, {"package", lupb_filedef_package}, + {"symtab", lupb_filedef_symtab}, {"syntax", lupb_filedef_syntax}, {NULL, NULL} }; @@ -761,6 +769,10 @@ static int lupb_symtab_new(lua_State *L) { lua_setfield(L, -2, "__mode"); lua_setmetatable(L, -2); + /* Put the symtab itself in the cache metatable. */ + lua_pushvalue(L, -2); + lua_rawsetp(L, -2, lsymtab->symtab); + /* Set the cache as our userval. */ lua_setiuservalue(L, -2, LUPB_CACHE_INDEX); diff --git a/upb/def.c b/upb/def.c index 600153e8d5..4ba553a69c 100644 --- a/upb/def.c +++ b/upb/def.c @@ -93,17 +93,18 @@ struct upb_filedef { const char *package; const char *phpprefix; const char *phpnamespace; - upb_syntax_t syntax; const upb_filedef **deps; const upb_msgdef *msgs; const upb_enumdef *enums; const upb_fielddef *exts; + const upb_symtab *symtab; int dep_count; int msg_count; int enum_count; int ext_count; + upb_syntax_t syntax; }; struct upb_symtab { @@ -823,6 +824,10 @@ const upb_enumdef *upb_filedef_enum(const upb_filedef *f, int i) { return i < 0 || i >= f->enum_count ? NULL : &f->enums[i]; } +const upb_symtab *upb_filedef_symtab(const upb_filedef *f) { + return f->symtab; +} + void upb_symtab_free(upb_symtab *s) { upb_arena_free(s->arena); upb_gfree(s); @@ -2093,6 +2098,7 @@ static const upb_filedef *_upb_symtab_addfile( file->msg_count = 0; file->enum_count = 0; file->ext_count = 0; + file->symtab = s; if (UPB_UNLIKELY(UPB_SETJMP(ctx.err))) { UPB_ASSERT(!upb_ok(status)); diff --git a/upb/def.h b/upb/def.h index 6b73ef5be3..7206ec0d3e 100644 --- a/upb/def.h +++ b/upb/def.h @@ -277,6 +277,7 @@ int upb_filedef_enumcount(const upb_filedef *f); const upb_filedef *upb_filedef_dep(const upb_filedef *f, int i); const upb_msgdef *upb_filedef_msg(const upb_filedef *f, int i); const upb_enumdef *upb_filedef_enum(const upb_filedef *f, int i); +const upb_symtab *upb_filedef_symtab(const upb_filedef *f); /* upb_symtab *****************************************************************/