From 7f0d3bb83dbb7590f3008182a8b27ecf9f0220d3 Mon Sep 17 00:00:00 2001 From: Josh Haberman Date: Tue, 8 Mar 2016 16:21:37 -0800 Subject: Make OneofDef stop deriving from Def. It is clear now that oneofs are different form other defs in one important way: they are not top-level constructs that can stand on their own in a SymbolTable, for example. If you are iterating over a list of Defs in a SymbolTable, there is no chance you will run into a oneof. To reflect this reality, OneofDef no longer derives from Def, and the UPB_DEF_ONEOF is no longer an enum value that needs to be handled. --- upb/def.c | 26 +++++++++++++------------- upb/def.h | 14 ++++++++------ upb/structdefs.int.h | 5 +++-- 3 files changed, 24 insertions(+), 21 deletions(-) diff --git a/upb/def.c b/upb/def.c index abb5b05..59d892d 100644 --- a/upb/def.c +++ b/upb/def.c @@ -538,7 +538,7 @@ static void visitfield(const upb_refcounted *r, upb_refcounted_visit *visit, visit(r, upb_msgdef_upcast2(upb_fielddef_containingtype(f)), closure); } if (upb_fielddef_containingoneof(f)) { - visit(r, upb_oneofdef_upcast2(upb_fielddef_containingoneof(f)), closure); + visit(r, upb_oneofdef_upcast(upb_fielddef_containingoneof(f)), closure); } if (upb_fielddef_subdef(f)) { visit(r, upb_def_upcast(upb_fielddef_subdef(f)), closure); @@ -1300,7 +1300,7 @@ static void visitmsg(const upb_refcounted *r, upb_refcounted_visit *visit, !upb_msg_oneof_done(&o); upb_msg_oneof_next(&o)) { upb_oneofdef *f = upb_msg_iter_oneof(&o); - visit(r, upb_oneofdef_upcast2(f), closure); + visit(r, upb_oneofdef_upcast(f), closure); } } @@ -1588,7 +1588,7 @@ static void freeoneof(upb_refcounted *r) { upb_oneofdef *o = (upb_oneofdef*)r; upb_strtable_uninit(&o->ntof); upb_inttable_uninit(&o->itof); - upb_def_uninit(upb_oneofdef_upcast_mutable(o)); + free((void*)o->name); free(o); } @@ -1597,9 +1597,9 @@ upb_oneofdef *upb_oneofdef_new(const void *owner) { upb_oneofdef *o = malloc(sizeof(*o)); o->parent = NULL; if (!o) return NULL; - if (!upb_def_init(upb_oneofdef_upcast_mutable(o), UPB_DEF_ONEOF, &vtbl, - owner)) + if (!upb_refcounted_init(upb_oneofdef_upcast_mutable(o), &vtbl, owner)) goto err2; + o->name = NULL; if (!upb_inttable_init(&o->itof, UPB_CTYPE_PTR)) goto err2; if (!upb_strtable_init(&o->ntof, UPB_CTYPE_PTR)) goto err1; return o; @@ -1616,8 +1616,7 @@ upb_oneofdef *upb_oneofdef_dup(const upb_oneofdef *o, const void *owner) { upb_oneof_iter i; upb_oneofdef *newo = upb_oneofdef_new(owner); if (!newo) return NULL; - ok = upb_def_setfullname(upb_oneofdef_upcast_mutable(newo), - upb_def_fullname(upb_oneofdef_upcast(o)), NULL); + ok = upb_oneofdef_setname(newo, upb_oneofdef_name(o), NULL); UPB_ASSERT_VAR(ok, 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); @@ -1629,17 +1628,18 @@ upb_oneofdef *upb_oneofdef_dup(const upb_oneofdef *o, const void *owner) { return newo; } -const char *upb_oneofdef_name(const upb_oneofdef *o) { - return upb_def_fullname(upb_oneofdef_upcast(o)); -} +const char *upb_oneofdef_name(const upb_oneofdef *o) { return o->name; } -bool upb_oneofdef_setname(upb_oneofdef *o, const char *fullname, - upb_status *s) { +bool upb_oneofdef_setname(upb_oneofdef *o, const char *name, upb_status *s) { + assert(!upb_oneofdef_isfrozen(o)); if (upb_oneofdef_containingtype(o)) { upb_status_seterrmsg(s, "oneof already added to a message"); return false; } - return upb_def_setfullname(upb_oneofdef_upcast_mutable(o), fullname, s); + if (!upb_isident(name, strlen(name), true, s)) return false; + free((void*)o->name); + o->name = upb_strdup(name); + return true; } const upb_msgdef *upb_oneofdef_containingtype(const upb_oneofdef *o) { diff --git a/upb/def.h b/upb/def.h index 6d303f8..f060b3f 100644 --- a/upb/def.h +++ b/upb/def.h @@ -37,6 +37,8 @@ class OneofDef; #endif UPB_DECLARE_DERIVED_TYPE(upb::Def, upb::RefCounted, upb_def, upb_refcounted) +UPB_DECLARE_DERIVED_TYPE(upb::OneofDef, upb::RefCounted, upb_oneofdef, + upb_refcounted) /* The maximum message depth that the type graph can have. This is a resource * limit for the C stack since we sometimes need to recursively traverse the @@ -48,15 +50,16 @@ UPB_DECLARE_DERIVED_TYPE(upb::Def, upb::RefCounted, upb_def, upb_refcounted) #define UPB_MAX_MESSAGE_DEPTH 64 -/* upb::Def: base class for defs *********************************************/ +/* upb::Def: base class for top-level defs ***********************************/ -/* All the different kind of defs we support. These correspond 1:1 with - * declarations in a .proto file. */ +/* All the different kind of defs that can be defined at the top-level and put + * in a SymbolTable or appear in a FileDef::defs() list. This excludes some + * defs (like oneofs and files). It only includes fields because they can be + * defined as extensions. */ typedef enum { UPB_DEF_MSG, UPB_DEF_FIELD, UPB_DEF_ENUM, - UPB_DEF_ONEOF, UPB_DEF_SERVICE, /* Not yet implemented. */ UPB_DEF_ANY = -1 /* Wildcard for upb_symtab_get*() */ } upb_deftype_t; @@ -190,7 +193,6 @@ UPB_END_EXTERN_C UPB_DECLARE_DEF_TYPE(upb::FieldDef, fielddef, FIELD) UPB_DECLARE_DEF_TYPE(upb::MessageDef, msgdef, MSG) UPB_DECLARE_DEF_TYPE(upb::EnumDef, enumdef, ENUM) -UPB_DECLARE_DEF_TYPE(upb::OneofDef, oneofdef, ONEOF) #undef UPB_DECLARE_DEF_TYPE #undef UPB_DEF_CASTS @@ -1226,7 +1228,7 @@ 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_upcast2) +UPB_REFCOUNTED_CMETHODS(upb_oneofdef, upb_oneofdef_upcast) const char *upb_oneofdef_name(const upb_oneofdef *o); bool upb_oneofdef_setname(upb_oneofdef *o, const char *name, upb_status *s); diff --git a/upb/structdefs.int.h b/upb/structdefs.int.h index b7b36fa..467963b 100644 --- a/upb/structdefs.int.h +++ b/upb/structdefs.int.h @@ -150,15 +150,16 @@ struct upb_enumdef { /* upb_oneofdef ***************************************************************/ struct upb_oneofdef { - upb_def base; + upb_refcounted base; + const char *name; upb_strtable ntof; upb_inttable itof; const upb_msgdef *parent; }; #define UPB_ONEOFDEF_INIT(name, ntof, itof, refs, ref2s) \ - { UPB_DEF_INIT(name, UPB_DEF_ENUM, refs, ref2s), ntof, itof } + { UPB_REFCOUNT_INIT(refs, ref2s), name, ntof, itof } /* upb_symtab *****************************************************************/ -- cgit v1.2.3