From 47da2afd52b0f108085439e3dc8ad5236809fbae Mon Sep 17 00:00:00 2001 From: Josh Haberman Date: Sat, 21 Jan 2017 10:47:58 -0800 Subject: Make upb::SymbolTable no longer reference-counted. This transitions it from shared ownership to unique ownership. --- tests/bindings/lua/test_upb.lua | 11 ----------- 1 file changed, 11 deletions(-) (limited to 'tests/bindings') diff --git a/tests/bindings/lua/test_upb.lua b/tests/bindings/lua/test_upb.lua index 65439bc..261328d 100644 --- a/tests/bindings/lua/test_upb.lua +++ b/tests/bindings/lua/test_upb.lua @@ -401,17 +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() -- cgit v1.2.3 From 693b841ec6e2bdb91920b7ea35a70ca30b416741 Mon Sep 17 00:00:00 2001 From: Josh Haberman Date: Mon, 23 Jan 2017 16:06:26 -0800 Subject: Removed all code for adding extensions to upb_symtab. This means extensions can't be used until we implement the replacement APIs for accessing extensions from a symtab. --- tests/bindings/lua/test_upb.lua | 36 ---------- tests/test_def.c | 1 + upb/def.c | 144 ++-------------------------------------- 3 files changed, 5 insertions(+), 176 deletions(-) (limited to 'tests/bindings') diff --git a/tests/bindings/lua/test_upb.lua b/tests/bindings/lua/test_upb.lua index 261328d..1dc0717 100644 --- a/tests/bindings/lua/test_upb.lua +++ b/tests/bindings/lua/test_upb.lua @@ -403,42 +403,6 @@ function test_symtab() assert_equal(msgdef3:field("field5"):subdef(), msgdef2) 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() local function test_for_numeric_type(upb_type, val, too_big, too_small, bad3) local array = upb.Array(upb_type) diff --git a/tests/test_def.c b/tests/test_def.c index 9859ca0..76914c7 100644 --- a/tests/test_def.c +++ b/tests/test_def.c @@ -220,6 +220,7 @@ static void test_replacement_fails() { newdefs[1] = upb_msgdef_upcast_mutable(m2); ok = upb_symtab_add(s, newdefs, 2, &s, &status); ASSERT(ok == false); + upb_status_clear(&status); /* Adding just one is ok. */ ASSERT_STATUS(upb_symtab_add(s, newdefs, 1, &s, &status), &status); diff --git a/upb/def.c b/upb/def.c index 1881dfa..c69e506 100644 --- a/upb/def.c +++ b/upb/def.c @@ -711,43 +711,6 @@ upb_fielddef *upb_fielddef_new(const void *o) { return f; } -static 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_; } @@ -1427,44 +1390,6 @@ err2: return NULL; } -static upb_oneofdef *upb_oneofdef_dup(const upb_oneofdef *o, const void *owner); - -static 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); @@ -1765,24 +1690,6 @@ err2: return NULL; } -static 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) { @@ -2241,57 +2148,14 @@ static bool symtab_add(upb_symtab *s, upb_def *const*defs, size_t n, fullname); goto err; } - upb_def_donateref(def, ref_donor, s); if (!upb_strtable_insert(&addtab, fullname, upb_value_ptr(def))) goto oom_err; - def->came_from_user = true; - } - } - - /* 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); - 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); - 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; } } -- cgit v1.2.3