Merge pull request #77 from haberman/axerefcount

Simplify and remove some code from upb::SymbolTable
pull/13171/head
Joshua Haberman 8 years ago committed by GitHub
commit 72ae34c28f
  1. 47
      tests/bindings/lua/test_upb.lua
  2. 6
      tests/test_cpp.cc
  3. 96
      tests/test_def.c
  4. 57
      upb/bindings/lua/def.c
  5. 2
      upb/bindings/lua/upb.h
  6. 348
      upb/def.c
  7. 95
      upb/def.h
  8. 2
      upb/msg.c

@ -401,53 +401,6 @@ function test_symtab()
local msgdef3 = symtab:lookup("ContainingMessage2")
assert_not_nil(msgdef3)
assert_equal(msgdef3:field("field5"):subdef(), msgdef2)
-- Freeze the symtab and verify that mutating operations are not allowed.
assert_false(symtab:is_frozen())
symtab:freeze()
assert_true(symtab:is_frozen())
assert_error_match("frozen", function() symtab:freeze() end)
assert_error_match("frozen", function()
symtab:add{
upb.MessageDef{full_name = "Foo"}
}
end)
end
function test_symtab_add_extension()
-- Adding an extension at the same time as the extendee.
local symtab = upb.SymbolTable{
upb.MessageDef{full_name = "M1"},
upb.FieldDef{name = "extension1", is_extension = true, number = 1,
type = upb.TYPE_INT32, containing_type_name = "M1"}
}
local m1 = symtab:lookup("M1")
assert_not_nil(m1)
assert_equal(1, #m1)
local f1 = m1:field("extension1")
assert_not_nil(f1)
assert_true(f1:is_extension())
assert_true(f1:is_frozen())
assert_equal(1, f1:number())
-- Adding an extension to an existing extendee.
symtab:add{
upb.FieldDef{name = "extension2", is_extension = true, number = 2,
type = upb.TYPE_INT32, containing_type_name = "M1"}
}
local m1_2 = symtab:lookup("M1")
assert_not_nil(m1_2)
assert_true(m1 ~= m1_2)
assert_equal(2, #m1_2)
local f2 = m1_2:field("extension2")
assert_not_nil(f2)
assert_true(f2:is_extension())
assert_true(f2:is_frozen())
assert_equal(2, f2:number())
end
function test_numeric_array()

@ -154,15 +154,12 @@ static void TestSymbolTable(const char *descriptor_file) {
exit(1);
}
upb::reffed_ptr<upb::SymbolTable> s(upb::SymbolTable::New());
upb::SymbolTable* s = upb::SymbolTable::New();
for (size_t i = 0; i < files.size(); i++) {
ASSERT(s->AddFile(files[i].get(), &status));
}
ASSERT(!s->IsFrozen());
s->Freeze();
ASSERT(s->IsFrozen());
upb::reffed_ptr<const upb::MessageDef> md(s->LookupMessage("C"));
ASSERT(md.get());
@ -180,6 +177,7 @@ static void TestSymbolTable(const char *descriptor_file) {
#endif
ASSERT(md.get());
upb::SymbolTable::Free(s);
}
static void TestCasts1() {

@ -13,13 +13,13 @@
const char *descriptor_file;
static void test_empty_symtab() {
upb_symtab *s = upb_symtab_new(&s);
upb_symtab *s = upb_symtab_new();
upb_symtab_iter i;
for (upb_symtab_begin(&i, s, UPB_DEF_ANY); !upb_symtab_done(&i);
upb_symtab_next(&i)) {
ASSERT(false); /* Should not get here. */
}
upb_symtab_unref(s, &s);
upb_symtab_free(s);
}
static void test_noreftracking() {
@ -39,8 +39,8 @@ static void test_noreftracking() {
upb_msgdef_unref(md, &md);
}
static upb_symtab *load_test_proto(void *owner) {
upb_symtab *s = upb_symtab_new(owner);
static upb_symtab *load_test_proto() {
upb_symtab *s = upb_symtab_new();
upb_status status = UPB_STATUS_INIT;
size_t len;
char *data = upb_readfile(descriptor_file, &len);
@ -64,15 +64,12 @@ static upb_symtab *load_test_proto(void *owner) {
upb_gfree(files);
ASSERT(!upb_symtab_isfrozen(s));
upb_symtab_freeze(s);
ASSERT(upb_symtab_isfrozen(s));
return s;
}
static void test_cycles() {
bool ok;
upb_symtab *s = load_test_proto(&s);
upb_symtab *s = load_test_proto();
const upb_msgdef *m;
const upb_fielddef *f;
const upb_def *def;
@ -84,7 +81,7 @@ static void test_cycles() {
upb_def_ref(def, &def);
ASSERT(def);
ASSERT(upb_def_isfrozen(def));
upb_symtab_unref(s, &s);
upb_symtab_free(s);
/* Message A has only one subfield: "optional B b = 1". */
m = upb_downcast_msgdef(def);
@ -162,18 +159,18 @@ static void test_symbol_resolution() {
upb_msgdef_unref(m3, &m3);
upb_msgdef_unref(m2, &m2);
upb_msgdef_unref(m1, &m1);
upb_symtab_unref(symtab, &symtab);
upb_symtab_free(symtab);
}
static void test_fielddef_unref() {
bool ok;
upb_symtab *s = load_test_proto(&s);
upb_symtab *s = load_test_proto();
const upb_msgdef *md = upb_symtab_lookupmsg(s, "A");
const upb_fielddef *f = upb_msgdef_itof(md, 1);
upb_fielddef_ref(f, &f);
/* Unref symtab; now fielddef is the only thing keeping the msgdef alive. */
upb_symtab_unref(s, &s);
upb_symtab_free(s);
/* Check that md is still alive. */
ok = strcmp(upb_msgdef_fullname(md), "A") == 0;
ASSERT(ok);
@ -210,66 +207,30 @@ static upb_msgdef *upb_msgdef_newnamed(const char *name, void *owner) {
return m;
}
static upb_enumdef *upb_enumdef_newnamed(const char *name, void *owner) {
upb_enumdef *e = upb_enumdef_new(owner);
upb_enumdef_setfullname(e, name, NULL);
return e;
}
static void test_replacement() {
static void test_replacement_fails() {
bool ok;
upb_symtab *s = upb_symtab_new(&s);
upb_enumdef *e2;
upb_msgdef *m2;
upb_enumdef *e;
upb_status status = UPB_STATUS_INIT;
upb_def *newdefs[3];
upb_def *newdefs2[1];
const upb_msgdef *m3;
upb_def *newdefs[2];
upb_msgdef *m = upb_msgdef_newnamed("MyMessage", &s);
upb_msgdef_addfield(m, newfield("field1", 1, UPB_TYPE_ENUM,
UPB_LABEL_OPTIONAL, ".MyEnum", &s),
&s, NULL);
m2 = upb_msgdef_newnamed("MyMessage2", &s);
e = upb_enumdef_newnamed("MyEnum", &s);
ASSERT_STATUS(upb_enumdef_addval(e, "VAL1", 1, &status), &status);
upb_msgdef *m2 = upb_msgdef_newnamed("MyMessage", &s);
newdefs[0] = upb_msgdef_upcast_mutable(m);
newdefs[1] = upb_msgdef_upcast_mutable(m2);
newdefs[2] = upb_enumdef_upcast_mutable(e);
ASSERT_STATUS(upb_symtab_add(s, newdefs, 3, &s, &status), &status);
/* Try adding a new definition of MyEnum, MyMessage should get replaced with
* a new version. */
e2 = upb_enumdef_newnamed("MyEnum", &s);
ASSERT_STATUS(upb_enumdef_addval(e2, "VAL1", 1, &status), &status);
newdefs2[0] = upb_enumdef_upcast_mutable(e2);
ASSERT_STATUS(upb_symtab_add(s, newdefs2, 1, &s, &status), &status);
m3 = upb_symtab_lookupmsg(s, "MyMessage");
ASSERT(m3);
/* Must be different because it points to MyEnum which was replaced. */
ASSERT(m3 != m);
ok = upb_symtab_add(s, newdefs, 2, &s, &status);
ASSERT(ok == false);
upb_status_clear(&status);
m3 = upb_symtab_lookupmsg(s, "MyMessage2");
/* Should be the same because it was not replaced, nor were any defs that
* are reachable from it. */
ASSERT(m3 == m2);
/* Adding just one is ok. */
ASSERT_STATUS(upb_symtab_add(s, newdefs, 1, &s, &status), &status);
upb_symtab_unref(s, &s);
}
/* Adding a conflicting one is not ok. */
newdefs[0] = upb_msgdef_upcast_mutable(m2);
ok = upb_symtab_add(s, newdefs, 1, &s, &status);
ASSERT(ok == false);
static void test_cycles_in_replacement() {
upb_symtab *s = upb_symtab_new(&s);
upb_msgdef *m = upb_msgdef_newnamed("M", &s);
upb_status status = UPB_STATUS_INIT;
upb_msgdef_addfield(m, newfield("m", 1, UPB_TYPE_MESSAGE,
UPB_LABEL_OPTIONAL, ".M", &s),
&s, NULL);
ASSERT_STATUS(upb_symtab_add(s, (upb_def**)&m, 1, &s, &status), &status);
ASSERT_STATUS(upb_symtab_add(s, NULL, 0, &s, &status), &status);
upb_symtab_unref(s, &s);
upb_symtab_free(s);
}
static void test_freeze_free() {
@ -368,7 +329,6 @@ static void test_partial_freeze() {
static void test_descriptor_flags() {
upb_msgdef *m = upb_msgdef_new(&m);
upb_msgdef *m2;
upb_status s = UPB_STATUS_INIT;
ASSERT(upb_msgdef_mapentry(m) == false);
@ -376,10 +336,7 @@ static void test_descriptor_flags() {
ASSERT(upb_ok(&s));
upb_msgdef_setmapentry(m, true);
ASSERT(upb_msgdef_mapentry(m) == true);
m2 = upb_msgdef_dup(m, &m2);
ASSERT(upb_msgdef_mapentry(m2) == true);
upb_msgdef_unref(m, &m);
upb_msgdef_unref(m2, &m2);
}
static void test_mapentry_check() {
@ -413,7 +370,7 @@ static void test_mapentry_check() {
upb_symtab_add(symtab, defs, 2, NULL, &s);
ASSERT(upb_ok(&s));
upb_symtab_unref(symtab, &symtab);
upb_symtab_free(symtab);
upb_msgdef_unref(subm, &subm);
upb_msgdef_unref(m, &m);
}
@ -470,7 +427,7 @@ static void test_oneofs() {
lookup_field = upb_oneofdef_ntofz(o, "field1");
ASSERT(lookup_field != NULL && upb_fielddef_number(lookup_field) == 1);
upb_symtab_unref(symtab, &symtab);
upb_symtab_free(symtab);
upb_oneofdef_unref(o, &o);
}
@ -485,8 +442,7 @@ int run_tests(int argc, char *argv[]) {
test_symbol_resolution();
test_fielddef();
test_fielddef_unref();
test_replacement();
test_cycles_in_replacement();
test_replacement_fails();
test_freeze_free();
test_partial_freeze();
test_noreftracking();

@ -1090,50 +1090,35 @@ static const struct luaL_Reg lupb_filedef_m[] = {
/* lupb_symtab ****************************************************************/
/* Inherits a ref on the symtab.
* Checks that narg is a proper lupb_symtab object. If it is, leaves its
* metatable on the stack for cache lookups/updates. */
const upb_symtab *lupb_symtab_check(lua_State *L, int narg) {
return lupb_refcounted_check(L, narg, LUPB_SYMTAB);
}
static upb_symtab *lupb_symtab_checkmutable(lua_State *L, int narg) {
const upb_symtab *s = lupb_symtab_check(L, narg);
if (upb_symtab_isfrozen(s))
luaL_error(L, "not allowed on frozen value");
return (upb_symtab*)s;
}
void lupb_symtab_pushwrapper(lua_State *L, const upb_symtab *s,
const void *ref_donor) {
lupb_refcounted_pushwrapper(L, upb_symtab_upcast(s), LUPB_SYMTAB, ref_donor,
sizeof(void *));
}
typedef struct {
upb_symtab *symtab;
} lupb_symtab;
void lupb_symtab_pushnewrapper(lua_State *L, const upb_symtab *s,
const void *ref_donor) {
lupb_refcounted_pushnewrapper(L, upb_symtab_upcast(s), LUPB_SYMTAB,
ref_donor);
upb_symtab *lupb_symtab_check(lua_State *L, int narg) {
lupb_symtab *lsymtab = luaL_checkudata(L, narg, LUPB_SYMTAB);
if (!lsymtab->symtab) {
luaL_error(L, "called into dead object");
}
return lsymtab->symtab;
}
static int lupb_symtab_new(lua_State *L) {
upb_symtab *s = upb_symtab_new(&s);
lupb_symtab_pushnewrapper(L, s, &s);
lupb_symtab *lsymtab = lua_newuserdata(L, sizeof(*lsymtab));
lsymtab->symtab = upb_symtab_new();
luaL_getmetatable(L, LUPB_SYMTAB);
lua_setmetatable(L, -2);
return 1;
}
static int lupb_symtab_freeze(lua_State *L) {
upb_symtab_freeze(lupb_symtab_checkmutable(L, 1));
static int lupb_symtab_gc(lua_State *L) {
lupb_symtab *lsymtab = luaL_checkudata(L, 1, LUPB_SYMTAB);
upb_symtab_free(lsymtab->symtab);
lsymtab->symtab = NULL;
return 0;
}
static int lupb_symtab_isfrozen(lua_State *L) {
lua_pushboolean(L, upb_symtab_isfrozen(lupb_symtab_check(L, 1)));
return 1;
}
static int lupb_symtab_add(lua_State *L) {
upb_symtab *s = lupb_symtab_checkmutable(L, 1);
upb_symtab *s = lupb_symtab_check(L, 1);
int n;
upb_def **defs;
@ -1161,7 +1146,7 @@ static int lupb_symtab_add(lua_State *L) {
}
static int lupb_symtab_addfile(lua_State *L) {
upb_symtab *s = lupb_symtab_checkmutable(L, 1);
upb_symtab *s = lupb_symtab_check(L, 1);
upb_filedef *f = lupb_filedef_checkmutable(L, 2);
CHK(upb_symtab_addfile(s, f, &status));
return 0;
@ -1201,14 +1186,12 @@ static const struct luaL_Reg lupb_symtab_m[] = {
{"add", lupb_symtab_add},
{"add_file", lupb_symtab_addfile},
{"defs", lupb_symtab_defs},
{"freeze", lupb_symtab_freeze},
{"is_frozen", lupb_symtab_isfrozen},
{"lookup", lupb_symtab_lookup},
{NULL, NULL}
};
static const struct luaL_Reg lupb_symtab_mm[] = {
{"__gc", lupb_refcounted_gc},
{"__gc", lupb_symtab_gc},
{NULL, NULL}
};

@ -101,7 +101,7 @@ void *lupb_refcounted_check(lua_State *L, int narg, const char *type);
const upb_msgdef *lupb_msgdef_check(lua_State *L, int narg);
const upb_enumdef *lupb_enumdef_check(lua_State *L, int narg);
const upb_fielddef *lupb_fielddef_check(lua_State *L, int narg);
const upb_symtab *lupb_symtab_check(lua_State *L, int narg);
upb_symtab *lupb_symtab_check(lua_State *L, int narg);
void lupb_refcounted_pushnewrapper(lua_State *L, const upb_refcounted *obj,
const char *type, const void *ref_donor);

@ -122,22 +122,6 @@ bool upb_def_setfullname(upb_def *def, const char *fullname, upb_status *s) {
const upb_filedef *upb_def_file(const upb_def *d) { return d->file; }
upb_def *upb_def_dup(const upb_def *def, const void *o) {
switch (def->type) {
case UPB_DEF_MSG:
return upb_msgdef_upcast_mutable(
upb_msgdef_dup(upb_downcast_msgdef(def), o));
case UPB_DEF_FIELD:
return upb_fielddef_upcast_mutable(
upb_fielddef_dup(upb_downcast_fielddef(def), o));
case UPB_DEF_ENUM:
return upb_enumdef_upcast_mutable(
upb_enumdef_dup(upb_downcast_enumdef(def), o));
default:
UPB_UNREACHABLE();
}
}
static bool upb_def_init(upb_def *def, upb_deftype_t type,
const struct upb_refcounted_vtbl *vtbl,
const void *owner) {
@ -394,13 +378,14 @@ bool _upb_def_validate(upb_def *const*defs, size_t n, upb_status *s) {
} else if (def->type == UPB_DEF_FIELD) {
upb_status_seterrmsg(s, "standalone fielddefs can not be frozen");
goto err;
} else if (def->type == UPB_DEF_ENUM) {
if (!upb_validate_enumdef(upb_dyncast_enumdef(def), s)) {
goto err;
}
} else {
/* Set now to detect transitive closure in the second pass. */
def->came_from_user = true;
if (def->type == UPB_DEF_ENUM &&
!upb_validate_enumdef(upb_dyncast_enumdef(def), s)) {
goto err;
}
}
}
@ -493,21 +478,6 @@ err2:
return NULL;
}
upb_enumdef *upb_enumdef_dup(const upb_enumdef *e, const void *owner) {
upb_enum_iter i;
upb_enumdef *new_e = upb_enumdef_new(owner);
if (!new_e) return NULL;
for(upb_enum_begin(&i, e); !upb_enum_done(&i); upb_enum_next(&i)) {
bool success = upb_enumdef_addval(
new_e, upb_enum_iter_name(&i),upb_enum_iter_number(&i), NULL);
if (!success) {
upb_enumdef_unref(new_e, owner);
return NULL;
}
}
return new_e;
}
bool upb_enumdef_freeze(upb_enumdef *e, upb_status *status) {
upb_def *d = upb_enumdef_upcast_mutable(e);
return upb_def_freeze(&d, 1, status);
@ -742,42 +712,6 @@ upb_fielddef *upb_fielddef_new(const void *o) {
return f;
}
upb_fielddef *upb_fielddef_dup(const upb_fielddef *f, const void *owner) {
const char *srcname;
upb_fielddef *newf = upb_fielddef_new(owner);
if (!newf) return NULL;
upb_fielddef_settype(newf, upb_fielddef_type(f));
upb_fielddef_setlabel(newf, upb_fielddef_label(f));
upb_fielddef_setnumber(newf, upb_fielddef_number(f), NULL);
upb_fielddef_setname(newf, upb_fielddef_name(f), NULL);
if (f->default_is_string && f->defaultval.bytes) {
str_t *s = f->defaultval.bytes;
upb_fielddef_setdefaultstr(newf, s->str, s->len, NULL);
} else {
newf->default_is_string = f->default_is_string;
newf->defaultval = f->defaultval;
}
if (f->subdef_is_symbolic) {
srcname = f->sub.name; /* Might be NULL. */
} else {
srcname = f->sub.def ? upb_def_fullname(f->sub.def) : NULL;
}
if (srcname) {
char *newname = upb_gmalloc(strlen(f->sub.def->fullname) + 2);
if (!newname) {
upb_fielddef_unref(newf, owner);
return NULL;
}
strcpy(newname, ".");
strcat(newname, f->sub.def->fullname);
upb_fielddef_setsubdefname(newf, newname, NULL);
upb_gfree(newname);
}
return newf;
}
bool upb_fielddef_typeisset(const upb_fielddef *f) {
return f->type_is_set_;
}
@ -1457,42 +1391,6 @@ err2:
return NULL;
}
upb_msgdef *upb_msgdef_dup(const upb_msgdef *m, const void *owner) {
bool ok;
upb_msg_field_iter i;
upb_msg_oneof_iter o;
upb_msgdef *newm = upb_msgdef_new(owner);
if (!newm) return NULL;
ok = upb_def_setfullname(upb_msgdef_upcast_mutable(newm),
upb_def_fullname(upb_msgdef_upcast(m)),
NULL);
newm->map_entry = m->map_entry;
newm->syntax = m->syntax;
UPB_ASSERT(ok);
for(upb_msg_field_begin(&i, m);
!upb_msg_field_done(&i);
upb_msg_field_next(&i)) {
upb_fielddef *f = upb_fielddef_dup(upb_msg_iter_field(&i), &f);
/* Fields in oneofs are dup'd below. */
if (upb_fielddef_containingoneof(f)) continue;
if (!f || !upb_msgdef_addfield(newm, f, &f, NULL)) {
upb_msgdef_unref(newm, owner);
return NULL;
}
}
for(upb_msg_oneof_begin(&o, m);
!upb_msg_oneof_done(&o);
upb_msg_oneof_next(&o)) {
upb_oneofdef *f = upb_oneofdef_dup(upb_msg_iter_oneof(&o), &f);
if (!f || !upb_msgdef_addoneof(newm, f, &f, NULL)) {
upb_msgdef_unref(newm, owner);
return NULL;
}
}
return newm;
}
bool upb_msgdef_freeze(upb_msgdef *m, upb_status *status) {
upb_def *d = upb_msgdef_upcast_mutable(m);
return upb_def_freeze(&d, 1, status);
@ -1793,23 +1691,6 @@ err2:
return NULL;
}
upb_oneofdef *upb_oneofdef_dup(const upb_oneofdef *o, const void *owner) {
bool ok;
upb_oneof_iter i;
upb_oneofdef *newo = upb_oneofdef_new(owner);
if (!newo) return NULL;
ok = upb_oneofdef_setname(newo, upb_oneofdef_name(o), NULL);
UPB_ASSERT(ok);
for (upb_oneof_begin(&i, o); !upb_oneof_done(&i); upb_oneof_next(&i)) {
upb_fielddef *f = upb_fielddef_dup(upb_oneof_iter_field(&i), &f);
if (!f || !upb_oneofdef_addfield(newo, f, &f, NULL)) {
upb_oneofdef_unref(newo, owner);
return NULL;
}
}
return newo;
}
const char *upb_oneofdef_name(const upb_oneofdef *o) { return o->name; }
bool upb_oneofdef_setname(upb_oneofdef *o, const char *name, upb_status *s) {
@ -2142,8 +2023,7 @@ bool upb_filedef_adddep(upb_filedef *f, const upb_filedef *dep) {
}
}
static void upb_symtab_free(upb_refcounted *r) {
upb_symtab *s = (upb_symtab*)r;
void upb_symtab_free(upb_symtab *s) {
upb_strtable_iter i;
upb_strtable_begin(&i, &s->symtab);
for (; !upb_strtable_done(&i); upb_strtable_next(&i)) {
@ -2154,32 +2034,16 @@ static void upb_symtab_free(upb_refcounted *r) {
upb_gfree(s);
}
upb_symtab *upb_symtab_new(const void *owner) {
static const struct upb_refcounted_vtbl vtbl = {NULL, &upb_symtab_free};
upb_symtab *upb_symtab_new() {
upb_symtab *s = upb_gmalloc(sizeof(*s));
if (!s) {
return NULL;
}
upb_refcounted_init(upb_symtab_upcast_mutable(s), &vtbl, owner);
upb_strtable_init(&s->symtab, UPB_CTYPE_PTR);
return s;
}
void upb_symtab_freeze(upb_symtab *s) {
upb_refcounted *r;
bool ok;
UPB_ASSERT(!upb_symtab_isfrozen(s));
r = upb_symtab_upcast_mutable(s);
/* The symtab does not take ref2's (see refcounted.h) on the defs, because
* defs cannot refer back to the table and therefore cannot create cycles. So
* 0 will suffice for maxdepth here. */
ok = upb_refcounted_freeze(&r, 1, NULL, 0);
UPB_ASSERT(ok);
}
const upb_def *upb_symtab_lookup(const upb_symtab *s, const char *sym) {
upb_value v;
upb_def *ret = upb_strtable_lookup(&s->symtab, sym, &v) ?
@ -2226,115 +2090,7 @@ const upb_def *upb_symtab_resolve(const upb_symtab *s, const char *base,
return ret;
}
/* Starts a depth-first traversal at "def", recursing into any subdefs
* (ie. submessage types). Adds duplicates of existing defs to addtab
* wherever necessary, so that the resulting symtab will be consistent once
* addtab is added.
*
* More specifically, if any def D is found in the DFS that:
*
* 1. can reach a def that is being replaced by something in addtab, AND
*
* 2. is not itself being replaced already (ie. this name doesn't already
* exist in addtab)
*
* ...then a duplicate (new copy) of D will be added to addtab.
*
* Returns true if this happened for any def reachable from "def."
*
* It is slightly tricky to do this correctly in the presence of cycles. If we
* detect that our DFS has hit a cycle, we might not yet know if any SCCs on
* our stack can reach a def in addtab or not. Once we figure this out, that
* answer needs to apply to *all* defs in these SCCs, even if we visited them
* already. So a straight up one-pass cycle-detecting DFS won't work.
*
* To work around this problem, we traverse each SCC (which we already
* computed, since these defs are frozen) as a single node. We first compute
* whether the SCC as a whole can reach any def in addtab, then we dup (or not)
* the entire SCC. This requires breaking the encapsulation of upb_refcounted,
* since that is where we get the data about what SCC we are in. */
static bool upb_resolve_dfs(const upb_def *def, upb_strtable *addtab,
const void *new_owner, upb_inttable *seen,
upb_status *s) {
upb_value v;
bool need_dup;
const upb_def *base;
const void* memoize_key;
/* Memoize results of this function for efficiency (since we're traversing a
* DAG this is not needed to limit the depth of the search).
*
* We memoize by SCC instead of by individual def. */
memoize_key = def->base.group;
if (upb_inttable_lookupptr(seen, memoize_key, &v))
return upb_value_getbool(v);
/* Visit submessages for all messages in the SCC. */
need_dup = false;
base = def;
do {
upb_value v;
const upb_msgdef *m;
UPB_ASSERT(upb_def_isfrozen(def));
if (def->type == UPB_DEF_FIELD) continue;
if (upb_strtable_lookup(addtab, upb_def_fullname(def), &v)) {
need_dup = true;
}
/* For messages, continue the recursion by visiting all subdefs, but only
* ones in different SCCs. */
m = upb_dyncast_msgdef(def);
if (m) {
upb_msg_field_iter i;
for(upb_msg_field_begin(&i, m);
!upb_msg_field_done(&i);
upb_msg_field_next(&i)) {
upb_fielddef *f = upb_msg_iter_field(&i);
const upb_def *subdef;
if (!upb_fielddef_hassubdef(f)) continue;
subdef = upb_fielddef_subdef(f);
/* Skip subdefs in this SCC. */
if (def->base.group == subdef->base.group) continue;
/* |= to avoid short-circuit; we need its side-effects. */
need_dup |= upb_resolve_dfs(subdef, addtab, new_owner, seen, s);
if (!upb_ok(s)) return false;
}
}
} while ((def = (upb_def*)def->base.next) != base);
if (need_dup) {
/* Dup all defs in this SCC that don't already have entries in addtab. */
def = base;
do {
const char *name;
if (def->type == UPB_DEF_FIELD) continue;
name = upb_def_fullname(def);
if (!upb_strtable_lookup(addtab, name, NULL)) {
upb_def *newdef = upb_def_dup(def, new_owner);
if (!newdef) goto oom;
newdef->came_from_user = false;
if (!upb_strtable_insert(addtab, name, upb_value_ptr(newdef)))
goto oom;
}
} while ((def = (upb_def*)def->base.next) != base);
}
upb_inttable_insertptr(seen, memoize_key, upb_value_bool(need_dup));
return need_dup;
oom:
upb_status_seterrmsg(s, "out of memory");
return false;
}
/* TODO(haberman): we need a lot more testing of error conditions.
* The came_from_user stuff in particular is not tested. */
/* TODO(haberman): we need a lot more testing of error conditions. */
static bool symtab_add(upb_symtab *s, upb_def *const*defs, size_t n,
void *ref_donor, upb_refcounted *freeze_also,
upb_status *status) {
@ -2346,13 +2102,11 @@ static bool symtab_add(upb_symtab *s, upb_def *const*defs, size_t n,
upb_def **add_defs = NULL;
size_t add_objs_size;
upb_strtable addtab;
upb_inttable seen;
if (n == 0 && !freeze_also) {
return true;
}
UPB_ASSERT(!upb_symtab_isfrozen(s));
if (!upb_strtable_init(&addtab, UPB_CTYPE_PTR)) {
upb_status_seterrmsg(status, "out of memory");
return false;
@ -2390,74 +2144,23 @@ static bool symtab_add(upb_symtab *s, upb_def *const*defs, size_t n,
upb_status_seterrf(status, "Conflicting defs named '%s'", fullname);
goto err;
}
/* We need this to back out properly, because if there is a failure we
* need to donate the ref back to the caller. */
def->came_from_user = true;
upb_def_donateref(def, ref_donor, s);
if (!upb_strtable_insert(&addtab, fullname, upb_value_ptr(def)))
goto oom_err;
}
}
/* Add standalone fielddefs (ie. extensions) to the appropriate messages.
* If the appropriate message only exists in the existing symtab, duplicate
* it so we have a mutable copy we can add the fields to. */
for (i = 0; i < n; i++) {
upb_def *def = defs[i];
upb_fielddef *f = upb_dyncast_fielddef_mutable(def);
const char *msgname;
upb_value v;
upb_msgdef *m;
if (!f) continue;
msgname = upb_fielddef_containingtypename(f);
/* We validated this earlier in this function. */
UPB_ASSERT(msgname);
/* If the extendee name is absolutely qualified, move past the initial ".".
* TODO(haberman): it is not obvious what it would mean if this was not
* absolutely qualified. */
if (msgname[0] == '.') {
msgname++;
}
if (upb_strtable_lookup(&addtab, msgname, &v)) {
/* Extendee is in the set of defs the user asked us to add. */
m = upb_value_getptr(v);
} else {
/* Need to find and dup the extendee from the existing symtab. */
const upb_msgdef *frozen_m = upb_symtab_lookupmsg(s, msgname);
if (!frozen_m) {
upb_status_seterrf(status,
"Tried to extend message %s that does not exist "
"in this SymbolTable.",
msgname);
if (upb_strtable_lookup(&s->symtab, fullname, NULL)) {
upb_status_seterrf(status, "Symtab already has a def named '%s'",
fullname);
goto err;
}
m = upb_msgdef_dup(frozen_m, s);
if (!m) goto oom_err;
if (!upb_strtable_insert(&addtab, msgname, upb_value_ptr(m))) {
upb_msgdef_unref(m, s);
if (!upb_strtable_insert(&addtab, fullname, upb_value_ptr(def)))
goto oom_err;
}
upb_def_donateref(def, ref_donor, s);
}
if (!upb_msgdef_addfield(m, f, ref_donor, status)) {
if (upb_dyncast_fielddef_mutable(def)) {
/* TODO(haberman): allow adding extensions attached to files. */
upb_status_seterrf(status, "Can't add extensions to symtab.\n");
goto err;
}
}
/* Add dups of any existing def that can reach a def with the same name as
* anything in our "add" set. */
if (!upb_inttable_init(&seen, UPB_CTYPE_BOOL)) goto oom_err;
upb_strtable_begin(&iter, &s->symtab);
for (; !upb_strtable_done(&iter); upb_strtable_next(&iter)) {
upb_def *def = upb_value_getptr(upb_strtable_iter_value(&iter));
upb_resolve_dfs(def, &addtab, s, &seen, status);
if (!upb_ok(status)) goto err;
}
upb_inttable_uninit(&seen);
/* Now using the table, resolve symbolic references for subdefs. */
upb_strtable_begin(&iter, &addtab);
for (; !upb_strtable_done(&iter); upb_strtable_next(&iter)) {
@ -2535,15 +2238,9 @@ static bool symtab_add(upb_symtab *s, upb_def *const*defs, size_t n,
for (i = 0; i < add_n; i++) {
upb_def *def = (upb_def*)add_objs[i];
const char *name = upb_def_fullname(def);
upb_value v;
bool success;
if (upb_strtable_remove(&s->symtab, name, &v)) {
const upb_def *def = upb_value_getptr(v);
upb_def_unref(def, s);
}
success = upb_strtable_insert(&s->symtab, name, upb_value_ptr(def));
UPB_ASSERT(success == true);
UPB_ASSERT(success);
}
upb_gfree(add_defs);
return true;
@ -2551,18 +2248,11 @@ static bool symtab_add(upb_symtab *s, upb_def *const*defs, size_t n,
oom_err:
upb_status_seterrmsg(status, "out of memory");
err: {
/* For defs the user passed in, we need to donate the refs back. For defs
* we dup'd, we need to just unref them. */
/* We need to donate the refs back. */
upb_strtable_begin(&iter, &addtab);
for (; !upb_strtable_done(&iter); upb_strtable_next(&iter)) {
upb_def *def = upb_value_getptr(upb_strtable_iter_value(&iter));
bool came_from_user = def->came_from_user;
def->came_from_user = false;
if (came_from_user) {
upb_def_donateref(def, s, ref_donor);
} else {
upb_def_unref(def, s);
}
upb_def_donateref(def, s, ref_donor);
}
}
upb_strtable_uninit(&addtab);

@ -44,8 +44,7 @@ UPB_DECLARE_DERIVED_TYPE(upb::OneofDef, upb::RefCounted, upb_oneofdef,
upb_refcounted)
UPB_DECLARE_DERIVED_TYPE(upb::FileDef, upb::RefCounted, upb_filedef,
upb_refcounted)
UPB_DECLARE_DERIVED_TYPE(upb::SymbolTable, upb::RefCounted,
upb_symtab, upb_refcounted)
UPB_DECLARE_TYPE(upb::SymbolTable, upb_symtab)
/* The maximum message depth that the type graph can have. This is a resource
@ -80,8 +79,6 @@ class upb::Def {
public:
typedef upb_deftype_t Type;
Def* Dup(const void *owner) const;
/* upb::RefCounted methods like Ref()/Unref(). */
UPB_REFCOUNTED_CPPMETHODS
@ -127,9 +124,6 @@ class upb::Def {
UPB_BEGIN_EXTERN_C
/* Native C API. */
upb_def *upb_def_dup(const upb_def *def, const void *owner);
/* Include upb_refcounted methods like upb_def_ref()/upb_def_unref(). */
UPB_REFCOUNTED_CMETHODS(upb_def, upb_def_upcast)
@ -319,13 +313,6 @@ class upb::FieldDef {
/* Returns NULL if memory allocation failed. */
static reffed_ptr<FieldDef> New();
/* Duplicates the given field, returning NULL if memory allocation failed.
* When a fielddef is duplicated, the subdef (if any) is made symbolic if it
* wasn't already. If the subdef is set but has no name (which is possible
* since msgdefs are not required to have a name) the new fielddef's subdef
* will be unset. */
FieldDef* Dup(const void* owner) const;
/* upb::RefCounted methods like Ref()/Unref(). */
UPB_REFCOUNTED_CPPMETHODS
@ -574,7 +561,6 @@ UPB_BEGIN_EXTERN_C
/* Native C API. */
upb_fielddef *upb_fielddef_new(const void *owner);
upb_fielddef *upb_fielddef_dup(const upb_fielddef *f, const void *owner);
/* Include upb_refcounted methods like upb_fielddef_ref(). */
UPB_REFCOUNTED_CMETHODS(upb_fielddef, upb_fielddef_upcast2)
@ -784,16 +770,6 @@ class upb::MessageDef {
return FindOneofByName(str.c_str(), str.size());
}
/* Returns a new msgdef that is a copy of the given msgdef (and a copy of all
* the fields) but with any references to submessages broken and replaced
* with just the name of the submessage. Returns NULL if memory allocation
* failed.
*
* TODO(haberman): which is more useful, keeping fields resolved or
* unresolving them? If there's no obvious answer, Should this functionality
* just be moved into symtab.c? */
MessageDef* Dup(const void* owner) const;
/* Is this message a map entry? */
void setmapentry(bool map_entry);
bool mapentry() const;
@ -927,7 +903,6 @@ UPB_REFCOUNTED_CMETHODS(upb_msgdef, upb_msgdef_upcast2)
bool upb_msgdef_freeze(upb_msgdef *m, upb_status *status);
upb_msgdef *upb_msgdef_dup(const upb_msgdef *m, const void *owner);
const char *upb_msgdef_fullname(const upb_msgdef *m);
const char *upb_msgdef_name(const upb_msgdef *m);
int upb_msgdef_numoneofs(const upb_msgdef *m);
@ -1077,10 +1052,6 @@ class upb::EnumDef {
* first one that was added. */
const char* FindValueByNumber(int32_t num) const;
/* Returns a new EnumDef with all the same values. The new EnumDef will be
* owned by the given owner. */
EnumDef* Dup(const void* owner) const;
/* Iteration over name/value pairs. The order is undefined.
* Adding an enum val invalidates any iterators.
*
@ -1108,7 +1079,6 @@ UPB_BEGIN_EXTERN_C
/* Native C API. */
upb_enumdef *upb_enumdef_new(const void *owner);
upb_enumdef *upb_enumdef_dup(const upb_enumdef *e, const void *owner);
/* Include upb_refcounted methods like upb_enumdef_ref(). */
UPB_REFCOUNTED_CMETHODS(upb_enumdef, upb_enumdef_upcast2)
@ -1218,10 +1188,6 @@ class upb::OneofDef {
/* Looks up by tag number. */
const FieldDef* FindFieldByNumber(uint32_t num) const;
/* Returns a new OneofDef with all the same fields. The OneofDef will be owned
* by the given owner. */
OneofDef* Dup(const void* owner) const;
/* Iteration over fields. The order is undefined. */
class iterator : public std::iterator<std::forward_iterator_tag, FieldDef*> {
public:
@ -1267,7 +1233,6 @@ UPB_BEGIN_EXTERN_C
/* Native C API. */
upb_oneofdef *upb_oneofdef_new(const void *owner);
upb_oneofdef *upb_oneofdef_dup(const upb_oneofdef *o, const void *owner);
/* Include upb_refcounted methods like upb_oneofdef_ref(). */
UPB_REFCOUNTED_CMETHODS(upb_oneofdef, upb_oneofdef_upcast)
@ -1434,10 +1399,8 @@ class upb::SymbolTable {
public:
/* Returns a new symbol table with a single ref owned by "owner."
* Returns NULL if memory allocation failed. */
static reffed_ptr<SymbolTable> New();
/* Include RefCounted base methods. */
UPB_REFCOUNTED_CPPMETHODS
static SymbolTable* New();
static void Free(upb::SymbolTable* table);
/* For all lookup functions, the returned pointer is not owned by the
* caller; it may be invalidated by any non-const call or unref of the
@ -1475,21 +1438,12 @@ class upb::SymbolTable {
* you ask for an iterator of MessageDef the iterated elements are strongly
* typed as MessageDef*. */
/* Adds the given mutable defs to the symtab, resolving all symbols
* (including enum default values) and finalizing the defs. Only one def per
* name may be in the list, but defs can replace existing defs in the symtab.
/* Adds the given mutable defs to the symtab, resolving all symbols (including
* enum default values) and finalizing the defs. Only one def per name may be
* in the list, and the defs may not duplicate any name already in the symtab.
* All defs must have a name -- anonymous defs are not allowed. Anonymous
* defs can still be frozen by calling upb_def_freeze() directly.
*
* Any existing defs that can reach defs that are being replaced will
* themselves be replaced also, so that the resulting set of defs is fully
* consistent.
*
* This logic implemented in this method is a convenience; ultimately it
* calls some combination of upb_fielddef_setsubdef(), upb_def_dup(), and
* upb_freeze(), any of which the client could call themself. However, since
* the logic for doing so is nontrivial, we provide it here.
*
* The entire operation either succeeds or fails. If the operation fails,
* the symtab is unchanged, false is returned, and status indicates the
* error. The caller passes a ref on all defs to the symtab (even if the
@ -1499,13 +1453,7 @@ class upb::SymbolTable {
* leave the defs themselves partially resolved. Does this matter? If so we
* could do a prepass that ensures that all symbols are resolvable and bail
* if not, so we don't mutate anything until we know the operation will
* succeed.
*
* TODO(haberman): since the defs must be mutable, refining a frozen def
* requires making mutable copies of the entire tree. This is wasteful if
* only a few messages are changing. We may want to add a way of adding a
* tree of frozen defs to the symtab (perhaps an alternate constructor where
* you pass the root of the tree?) */
* succeed. */
bool Add(Def*const* defs, size_t n, void* ref_donor, Status* status);
bool Add(const std::vector<Def*>& defs, void *owner, Status* status) {
@ -1527,11 +1475,8 @@ UPB_BEGIN_EXTERN_C
/* Native C API. */
/* Include refcounted methods like upb_symtab_ref(). */
UPB_REFCOUNTED_CMETHODS(upb_symtab, upb_symtab_upcast)
upb_symtab *upb_symtab_new(const void *owner);
void upb_symtab_freeze(upb_symtab *s);
upb_symtab *upb_symtab_new();
void upb_symtab_free(upb_symtab* s);
const upb_def *upb_symtab_resolve(const upb_symtab *s, const char *base,
const char *sym);
const upb_def *upb_symtab_lookup(const upb_symtab *s, const char *sym);
@ -1562,13 +1507,11 @@ UPB_END_EXTERN_C
#ifdef __cplusplus
/* C++ inline wrappers. */
namespace upb {
inline reffed_ptr<SymbolTable> SymbolTable::New() {
upb_symtab *s = upb_symtab_new(&s);
return reffed_ptr<SymbolTable>(s, &s);
inline SymbolTable* SymbolTable::New() {
return upb_symtab_new();
}
inline void SymbolTable::Freeze() {
return upb_symtab_freeze(this);
inline void SymbolTable::Free(SymbolTable* s) {
upb_symtab_free(s);
}
inline const Def *SymbolTable::Resolve(const char *base,
const char *sym) const {
@ -1600,9 +1543,6 @@ UPB_INLINE const char* upb_safecstr(const std::string& str) {
/* Inline C++ wrappers. */
namespace upb {
inline Def* Def::Dup(const void* owner) const {
return upb_def_dup(this, owner);
}
inline Def::Type Def::def_type() const { return upb_def_type(this); }
inline const char* Def::full_name() const { return upb_def_fullname(this); }
inline const char* Def::name() const { return upb_def_name(this); }
@ -1652,9 +1592,6 @@ inline reffed_ptr<FieldDef> FieldDef::New() {
upb_fielddef *f = upb_fielddef_new(&f);
return reffed_ptr<FieldDef>(f, &f);
}
inline FieldDef* FieldDef::Dup(const void* owner) const {
return upb_fielddef_dup(this, owner);
}
inline const char* FieldDef::full_name() const {
return upb_fielddef_fullname(this);
}
@ -1894,9 +1831,6 @@ inline const OneofDef* MessageDef::FindOneofByName(const char* name,
size_t len) const {
return upb_msgdef_ntoo(this, name, len);
}
inline MessageDef* MessageDef::Dup(const void *owner) const {
return upb_msgdef_dup(this, owner);
}
inline void MessageDef::setmapentry(bool map_entry) {
upb_msgdef_setmapentry(this, map_entry);
}
@ -2066,9 +2000,6 @@ inline bool EnumDef::FindValueByName(const char* name, int32_t *num) const {
inline const char* EnumDef::FindValueByNumber(int32_t num) const {
return upb_enumdef_iton(this, num);
}
inline EnumDef* EnumDef::Dup(const void* owner) const {
return upb_enumdef_dup(this, owner);
}
inline EnumDef::Iterator::Iterator(const EnumDef* e) {
upb_enum_begin(&iter_, e);

@ -350,7 +350,6 @@ upb_msgfactory *upb_msgfactory_new(const upb_symtab *symtab) {
upb_msgfactory *ret = upb_gmalloc(sizeof(*ret));
ret->symtab = symtab;
upb_symtab_ref(ret->symtab, &ret->symtab);
upb_inttable_init(&ret->layouts, UPB_CTYPE_PTR);
upb_inttable_init(&ret->mergehandlers, UPB_CTYPE_CONSTPTR);
@ -373,7 +372,6 @@ void upb_msgfactory_free(upb_msgfactory *f) {
upb_inttable_uninit(&f->layouts);
upb_inttable_uninit(&f->mergehandlers);
upb_symtab_unref(f->symtab, &f->symtab);
upb_gfree(f);
}

Loading…
Cancel
Save