Put oneofs in the same table as fields. (#60)

* Put oneofs in the same table as fields.

Oneofs and fields are not allowed to have names that conflict,
so we might as well put them all in the same table.  This also
allows an efficient operation that looks for both fields and
oneofs in a single lookup.

Added support for OneofDef to Lua to allow testing of this.

* Addressed PR comments.

Makefile

@@ -109,6 +109,7 @@ dep:
 clean_leave_profile:
 	@rm -rf obj lib
 	@rm -f tests/google_message?.h
+	@rm -f tests/json/test.upbdefs.o
 	@rm -f $(TESTS) tests/testmain.o tests/t.*
 	@rm -rf tools/upbc deps
 	@rm -rf upb/bindings/python/build
@@ -455,8 +456,8 @@ LUATESTS = \
 .PHONY: clean_lua testlua lua
 
 testlua: lua
-	@set -e  # Abort on error.
+	@set -e; \
-	@for test in $(LUATESTS) ; do \
+	for test in $(LUATESTS) ; do \
 	  echo LUA $$test; \
 	  LUA_PATH="third_party/lunit/?.lua;upb/bindings/lua/?.lua" \
 	    LUA_CPATH=upb/bindings/lua/?.so \

tests/bindings/lua/test_upb.lua

@@ -16,6 +16,18 @@ function iter_to_array(iter)
   return arr
 end
 
+function test_msgdef()
+  local f2 = upb.FieldDef{name = "field2", number = 1, type = upb.TYPE_INT32}
+  local o = upb.OneofDef{name = "field1", fields = {f2}}
+  local f = upb.FieldDef{name = "field3", number = 2, type = upb.TYPE_INT32}
+
+  local m = upb.MessageDef{fields = {o, f}}
+
+  assert_equal(f, m:lookup_name("field3"))
+  assert_equal(o, m:lookup_name("field1"))
+  assert_equal(f2, m:lookup_name("field2"))
+end
+
 function test_fielddef()
   local f = upb.FieldDef()
   assert_false(f:is_frozen())
@@ -314,6 +326,18 @@ function test_msgdef_errors()
     }
   end)
 
+  assert_error(function()
+    -- Duplicate field name.
+    upb.MessageDef{
+      fields = {
+        upb.OneofDef{name = "field1", fields = {
+          upb.FieldDef{name = "field2", number = 1, type = upb.TYPE_INT32},
+        }},
+        upb.FieldDef{name = "field2", number = 2, type = upb.TYPE_INT32}
+      }
+    }
+  end)
+
   -- attempt to set a name with embedded NULLs.
   assert_error_match("names cannot have embedded NULLs", function()
     md:set_full_name("abc\0def")

upb/bindings/lua/upb.c

@@ -31,14 +31,16 @@
 #include "upb/shim/shim.h"
 
 /* Lua metatable types. */
-#define LUPB_MSG "lupb.msg"
-#define LUPB_ARRAY "lupb.array"
-#define LUPB_MSGDEF "lupb.msgdef"
 #define LUPB_ENUMDEF "lupb.enumdef"
 #define LUPB_FIELDDEF "lupb.fielddef"
 #define LUPB_FILEDEF "lupb.filedef"
+#define LUPB_MSGDEF "lupb.msgdef"
+#define LUPB_ONEOFDEF "lupb.oneof"
 #define LUPB_SYMTAB "lupb.symtab"
 
+#define LUPB_ARRAY "lupb.array"
+#define LUPB_MSG "lupb.msg"
+
 /* Other table constants. */
 #define LUPB_OBJCACHE "lupb.objcache"
 
@@ -509,6 +511,11 @@ static int lupb_def_setfullname(lua_State *L) {
 
 /* lupb_fielddef **************************************************************/
 
+void lupb_fielddef_pushwrapper(lua_State *L, const upb_fielddef *f,
+                               const void *ref_donor) {
+  lupb_def_pushwrapper(L, upb_fielddef_upcast(f), ref_donor);
+}
+
 const upb_fielddef *lupb_fielddef_check(lua_State *L, int narg) {
   return lupb_refcounted_check(L, narg, LUPB_FIELDDEF);
 }
@@ -530,8 +537,7 @@ static int lupb_fielddef_new(lua_State *L) {
 
 static int lupb_fielddef_containingtype(lua_State *L) {
   const upb_fielddef *f = lupb_fielddef_check(L, 1);
-  lupb_def_pushwrapper(L, upb_msgdef_upcast(upb_fielddef_containingtype(f)),
+  lupb_msgdef_pushwrapper(L, upb_fielddef_containingtype(f), NULL);
-                       NULL);
   return 1;
 }
 
@@ -857,6 +863,102 @@ static const struct luaL_Reg lupb_fielddef_m[] = {
 };
 
 
+/* lupb_oneofdef **************************************************************/
+
+void lupb_oneofdef_pushwrapper(lua_State *L, const upb_oneofdef *o,
+                               const void *ref_donor) {
+  lupb_refcounted_pushwrapper(L, upb_oneofdef_upcast(o), LUPB_ONEOFDEF,
+                              ref_donor, sizeof(void *));
+}
+
+const upb_oneofdef *lupb_oneofdef_check(lua_State *L, int narg) {
+  return lupb_refcounted_check(L, narg, LUPB_ONEOFDEF);
+}
+
+static upb_oneofdef *lupb_oneofdef_checkmutable(lua_State *L, int narg) {
+  const upb_oneofdef *o = lupb_oneofdef_check(L, narg);
+  if (upb_oneofdef_isfrozen(o))
+    luaL_error(L, "not allowed on frozen value");
+  return (upb_oneofdef*)o;
+}
+
+static int lupb_oneofdef_new(lua_State *L) {
+  upb_oneofdef *o = upb_oneofdef_new(&o);
+  lupb_refcounted_pushnewrapper(L, upb_oneofdef_upcast(o), LUPB_ONEOFDEF, &o);
+  return 1;
+}
+
+/* Getters */
+
+static int lupb_oneofdef_containingtype(lua_State *L) {
+  const upb_oneofdef *o = lupb_oneofdef_check(L, 1);
+  lupb_def_pushwrapper(L, upb_msgdef_upcast(upb_oneofdef_containingtype(o)),
+                       NULL);
+  return 1;
+}
+
+static int lupb_oneofdef_field(lua_State *L) {
+  const upb_oneofdef *o = lupb_oneofdef_check(L, 1);
+  int type = lua_type(L, 2);
+  const upb_fielddef *f;
+  if (type == LUA_TNUMBER) {
+    f = upb_oneofdef_itof(o, lua_tointeger(L, 2));
+  } else if (type == LUA_TSTRING) {
+    f = upb_oneofdef_ntofz(o, lua_tostring(L, 2));
+  } else {
+    const char *msg = lua_pushfstring(L, "number or string expected, got %s",
+                                      luaL_typename(L, 2));
+    return luaL_argerror(L, 2, msg);
+  }
+
+  lupb_def_pushwrapper(L, upb_fielddef_upcast(f), NULL);
+  return 1;
+}
+
+static int lupb_oneofdef_len(lua_State *L) {
+  const upb_oneofdef *o = lupb_oneofdef_check(L, 1);
+  lua_pushinteger(L, upb_oneofdef_numfields(o));
+  return 1;
+}
+
+static int lupb_oneofdef_name(lua_State *L) {
+  const upb_oneofdef *o = lupb_oneofdef_check(L, 1);
+  lua_pushstring(L, upb_oneofdef_name(o));
+  return 1;
+}
+
+/* Setters */
+
+static int lupb_oneofdef_add(lua_State *L) {
+  upb_oneofdef *o = lupb_oneofdef_checkmutable(L, 1);
+  upb_fielddef *f = lupb_fielddef_checkmutable(L, 2);
+  CHK(upb_oneofdef_addfield(o, f, NULL, &status));
+  return 0;
+}
+
+static int lupb_oneofdef_setname(lua_State *L) {
+  upb_oneofdef *o = lupb_oneofdef_checkmutable(L, 1);
+  CHK(upb_oneofdef_setname(o, lupb_checkname(L, 2), &status));
+  return 0;
+}
+
+static const struct luaL_Reg lupb_oneofdef_m[] = {
+  {"containing_type", lupb_oneofdef_containingtype},
+  {"field", lupb_oneofdef_field},
+  {"name", lupb_oneofdef_name},
+
+  {"add", lupb_oneofdef_add},
+  {"set_name", lupb_oneofdef_setname},
+
+  {NULL, NULL}
+};
+
+static const struct luaL_Reg lupb_oneofdef_mm[] = {
+  {"__len", lupb_oneofdef_len},
+  {NULL, NULL}
+};
+
+
 /* lupb_msgdef ****************************************************************/
 
 typedef struct {
@@ -874,6 +976,11 @@ static size_t lupb_msgdef_sizeof() {
   return sizeof(lupb_msgdef);
 }
 
+void lupb_msgdef_pushwrapper(lua_State *L, const upb_msgdef *m,
+                             const void *ref_donor) {
+  lupb_def_pushwrapper(L, upb_msgdef_upcast(m), ref_donor);
+}
+
 const upb_msgdef *lupb_msgdef_check(lua_State *L, int narg) {
   return lupb_refcounted_check(L, narg, LUPB_MSGDEF);
 }
@@ -912,8 +1019,15 @@ static void lupb_msgdef_init(lua_State *L) {
 
 static int lupb_msgdef_add(lua_State *L) {
   upb_msgdef *m = lupb_msgdef_checkmutable(L, 1);
-  upb_fielddef *f = lupb_fielddef_checkmutable(L, 2);
+
-  CHK(upb_msgdef_addfield(m, f, NULL, &status));
+  /* Both oneofs and fields can be added. */
+  if (luaL_testudata(L, 2, LUPB_FIELDDEF)) {
+    upb_fielddef *f = lupb_fielddef_checkmutable(L, 2);
+    CHK(upb_msgdef_addfield(m, f, NULL, &status));
+  } else if (luaL_testudata(L, 2, LUPB_ONEOFDEF)) {
+    upb_oneofdef *o = lupb_oneofdef_checkmutable(L, 2);
+    CHK(upb_msgdef_addoneof(m, o, NULL, &status));
+  }
   return 0;
 }
 
@@ -941,7 +1055,21 @@ static int lupb_msgdef_field(lua_State *L) {
   return 1;
 }
 
-static int lupb_msgiter_next(lua_State *L) {
+static int lupb_msgdef_lookupname(lua_State *L) {
+  const upb_msgdef *m = lupb_msgdef_check(L, 1);
+  const upb_fielddef *f;
+  const upb_oneofdef *o;
+  if (!upb_msgdef_lookupnamez(m, lua_tostring(L, 2), &f, &o)) {
+    lua_pushnil(L);
+  } else if (o) {
+    lupb_oneofdef_pushwrapper(L, o, NULL);
+  } else {
+    lupb_fielddef_pushwrapper(L, f, NULL);
+  }
+  return 1;
+}
+
+static int lupb_msgfielditer_next(lua_State *L) {
   upb_msg_field_iter *i = lua_touserdata(L, lua_upvalueindex(1));
   if (upb_msg_field_done(i)) return 0;
   lupb_def_pushwrapper(L, upb_fielddef_upcast(upb_msg_iter_field(i)), NULL);
@@ -955,7 +1083,25 @@ static int lupb_msgdef_fields(lua_State *L) {
   upb_msg_field_begin(i, m);
   /* Need to guarantee that the msgdef outlives the iter. */
   lua_pushvalue(L, 1);
-  lua_pushcclosure(L, &lupb_msgiter_next, 2);
+  lua_pushcclosure(L, &lupb_msgfielditer_next, 2);
+  return 1;
+}
+
+static int lupb_msgoneofiter_next(lua_State *L) {
+  upb_msg_oneof_iter *i = lua_touserdata(L, lua_upvalueindex(1));
+  if (upb_msg_oneof_done(i)) return 0;
+  lupb_oneofdef_pushwrapper(L, upb_msg_iter_oneof(i), NULL);
+  upb_msg_oneof_next(i);
+  return 1;
+}
+
+static int lupb_msgdef_oneofs(lua_State *L) {
+  const upb_msgdef *m = lupb_msgdef_check(L, 1);
+  upb_msg_oneof_iter *i = lua_newuserdata(L, sizeof(upb_msg_oneof_iter));
+  upb_msg_oneof_begin(i, m);
+  /* Need to guarantee that the msgdef outlives the iter. */
+  lua_pushvalue(L, 1);
+  lua_pushcclosure(L, &lupb_msgoneofiter_next, 2);
   return 1;
 }
 
@@ -982,6 +1128,8 @@ static const struct luaL_Reg lupb_msgdef_m[] = {
   {"add", lupb_msgdef_add},
   {"field", lupb_msgdef_field},
   {"fields", lupb_msgdef_fields},
+  {"lookup_name", lupb_msgdef_lookupname},
+  {"oneofs", lupb_msgdef_oneofs},
   {"syntax", lupb_msgdef_syntax},
   {"_map_entry", lupb_msgdef_mapentry},
 
@@ -1900,6 +2048,7 @@ static const struct luaL_Reg lupb_toplevel_m[] = {
   {"FileDef", lupb_filedef_new},
   {"Message", lupb_msg_new},
   {"MessageDef", lupb_msgdef_new},
+  {"OneofDef", lupb_oneofdef_new},
   {"SymbolTable", lupb_symtab_new},
   {"freeze", lupb_freeze},
   {"load_descriptor", lupb_loaddescriptor},
@@ -1953,6 +2102,7 @@ int luaopen_upb_c(lua_State *L) {
   lupb_register_type(L, LUPB_ENUMDEF,  lupb_enumdef_m,  lupb_enumdef_mm,  true);
   lupb_register_type(L, LUPB_FIELDDEF, lupb_fielddef_m, NULL,             true);
   lupb_register_type(L, LUPB_FILEDEF,  lupb_filedef_m,  lupb_filedef_mm,  true);
+  lupb_register_type(L, LUPB_ONEOFDEF, lupb_oneofdef_m, lupb_oneofdef_mm, true);
   lupb_register_type(L, LUPB_SYMTAB,   lupb_symtab_m,   NULL,             true);
 
   /* Refcounted but with custom __gc. */

upb/bindings/lua/upb.h

@@ -94,6 +94,8 @@ bool lupb_def_pushwrapper(lua_State *L, const upb_def *def,
                           const void *ref_donor);
 void lupb_def_pushnewrapper(lua_State *L, const upb_def *def,
                             const void *ref_donor);
+void lupb_msgdef_pushwrapper(lua_State *L, const upb_msgdef *m,
+                             const void *ref_donor);
 void lupb_symtab_pushwrapper(lua_State *L, const upb_symtab *s,
                              const void *ref_donor);
 void lupb_symtab_pushnewrapper(lua_State *L, const upb_symtab *s,

upb/bindings/lua/upb.lua

@@ -66,6 +66,7 @@ end
 local RealFieldDef = upb.FieldDef
 local RealEnumDef = upb.EnumDef
 local RealMessageDef = upb.MessageDef
+local RealOneofDef = upb.OneofDef
 local RealSymbolTable = upb.SymbolTable
 
 -- FieldDef constructor; a wrapper around the real constructor that can
@@ -136,6 +137,26 @@ upb.EnumDef = function(init)
   return e
 end
 
+-- OneofDef constructor; a wrapper around the real constructor that can
+-- set initial properties.
+--
+-- User can specify initialization values like so:
+--   upb.OneofDef{name="foo", fields={...}}
+upb.OneofDef = function(init)
+  local o = RealOneofDef()
+
+  if init then
+    for _, val in pairs(init.fields or {}) do
+      o:add(val)
+    end
+    init.fields = nil
+
+    set_named(o, init)
+  end
+
+  return o
+end
+
 -- SymbolTable constructor; a wrapper around the real constructor that can
 -- add an initial set of defs.
 upb.SymbolTable = function(defs)

upb/def.c

@@ -66,6 +66,22 @@ static bool upb_isident(const char *str, size_t len, bool full, upb_status *s) {
   return !start;
 }
 
+static bool upb_isoneof(const upb_refcounted *def) {
+  return def->vtbl == &upb_oneofdef_vtbl;
+}
+
+static bool upb_isfield(const upb_refcounted *def) {
+  return def->vtbl == &upb_fielddef_vtbl;
+}
+
+static const upb_oneofdef *upb_trygetoneof(const upb_refcounted *def) {
+  return upb_isoneof(def) ? (const upb_oneofdef*)def : NULL;
+}
+
+static const upb_fielddef *upb_trygetfield(const upb_refcounted *def) {
+  return upb_isfield(def) ? (const upb_fielddef*)def : NULL;
+}
+
 
 /* upb_def ********************************************************************/
 
@@ -437,12 +453,17 @@ static void upb_enumdef_free(upb_refcounted *r) {
   upb_gfree(e);
 }
 
+const struct upb_refcounted_vtbl upb_enumdef_vtbl = {NULL, &upb_enumdef_free};
+
 upb_enumdef *upb_enumdef_new(const void *owner) {
-  static const struct upb_refcounted_vtbl vtbl = {NULL, &upb_enumdef_free};
   upb_enumdef *e = upb_gmalloc(sizeof(*e));
   if (!e) return NULL;
-  if (!upb_def_init(upb_enumdef_upcast_mutable(e), UPB_DEF_ENUM, &vtbl, owner))
+
+  if (!upb_def_init(upb_enumdef_upcast_mutable(e), UPB_DEF_ENUM,
+                    &upb_enumdef_vtbl, owner)) {
     goto err2;
+  }
+
   if (!upb_strtable_init(&e->ntoi, UPB_CTYPE_INT32)) goto err2;
   if (!upb_inttable_init(&e->iton, UPB_CTYPE_CSTR)) goto err1;
   return e;
@@ -663,11 +684,13 @@ static bool enumdefaultint32(const upb_fielddef *f, int32_t *val) {
   return false;
 }
 
+const struct upb_refcounted_vtbl upb_fielddef_vtbl = {visitfield, freefield};
+
 upb_fielddef *upb_fielddef_new(const void *o) {
-  static const struct upb_refcounted_vtbl vtbl = {visitfield, freefield};
   upb_fielddef *f = upb_gmalloc(sizeof(*f));
   if (!f) return NULL;
-  if (!upb_def_init(upb_fielddef_upcast_mutable(f), UPB_DEF_FIELD, &vtbl, o)) {
+  if (!upb_def_init(upb_fielddef_upcast_mutable(f), UPB_DEF_FIELD,
+                    &upb_fielddef_vtbl, o)) {
     upb_gfree(f);
     return NULL;
   }
@@ -1378,31 +1401,32 @@ static void visitmsg(const upb_refcounted *r, upb_refcounted_visit *visit,
 
 static void freemsg(upb_refcounted *r) {
   upb_msgdef *m = (upb_msgdef*)r;
-  upb_strtable_uninit(&m->ntoo);
   upb_strtable_uninit(&m->ntof);
   upb_inttable_uninit(&m->itof);
   upb_def_uninit(upb_msgdef_upcast_mutable(m));
   upb_gfree(m);
 }
 
+const struct upb_refcounted_vtbl upb_msgdef_vtbl = {visitmsg, freemsg};
+
 upb_msgdef *upb_msgdef_new(const void *owner) {
-  static const struct upb_refcounted_vtbl vtbl = {visitmsg, freemsg};
   upb_msgdef *m = upb_gmalloc(sizeof(*m));
   if (!m) return NULL;
-  if (!upb_def_init(upb_msgdef_upcast_mutable(m), UPB_DEF_MSG, &vtbl, owner))
+
+  if (!upb_def_init(upb_msgdef_upcast_mutable(m), UPB_DEF_MSG, &upb_msgdef_vtbl,
+                    owner)) {
     goto err2;
-  if (!upb_inttable_init(&m->itof, UPB_CTYPE_PTR)) goto err3;
+  }
-  if (!upb_strtable_init(&m->ntof, UPB_CTYPE_PTR)) goto err2;
+
-  if (!upb_strtable_init(&m->ntoo, UPB_CTYPE_PTR)) goto err1;
+  if (!upb_inttable_init(&m->itof, UPB_CTYPE_PTR)) goto err2;
+  if (!upb_strtable_init(&m->ntof, UPB_CTYPE_PTR)) goto err1;
   m->map_entry = false;
   m->syntax = UPB_SYNTAX_PROTO2;
   return m;
 
 err1:
-  upb_strtable_uninit(&m->ntof);
-err2:
   upb_inttable_uninit(&m->itof);
-err3:
+err2:
   upb_gfree(m);
   return NULL;
 }
@@ -1484,9 +1508,11 @@ static bool check_field_add(const upb_msgdef *m, const upb_fielddef *f,
   } else if (upb_fielddef_name(f) == NULL || upb_fielddef_number(f) == 0) {
     upb_status_seterrmsg(s, "field name or number were not set");
     return false;
-  } else if (upb_msgdef_ntofz(m, upb_fielddef_name(f)) ||
+  } else if (upb_msgdef_itof(m, upb_fielddef_number(f))) {
-             upb_msgdef_itof(m, upb_fielddef_number(f))) {
+    upb_status_seterrmsg(s, "duplicate field number");
-    upb_status_seterrmsg(s, "duplicate field name or number for field");
+    return false;
+  } else if (upb_strtable_lookup(&m->ntof, upb_fielddef_name(f), NULL)) {
+    upb_status_seterrmsg(s, "name conflicts with existing field or oneof");
     return false;
   }
   return true;
@@ -1547,8 +1573,8 @@ bool upb_msgdef_addoneof(upb_msgdef *m, upb_oneofdef *o, const void *ref_donor,
   } else if (upb_oneofdef_name(o) == NULL) {
     upb_status_seterrmsg(s, "oneofdef name was not set");
     return false;
-  } else if (upb_msgdef_ntooz(m, upb_oneofdef_name(o))) {
+  } else if (upb_strtable_lookup(&m->ntof, upb_oneofdef_name(o), NULL)) {
-    upb_status_seterrmsg(s, "duplicate oneof name");
+    upb_status_seterrmsg(s, "name conflicts with existing field or oneof");
     return false;
   }
 
@@ -1565,7 +1591,7 @@ bool upb_msgdef_addoneof(upb_msgdef *m, upb_oneofdef *o, const void *ref_donor,
 
   /* Add oneof itself first. */
   o->parent = m;
-  upb_strtable_insert(&m->ntoo, upb_oneofdef_name(o), upb_value_ptr(o));
+  upb_strtable_insert(&m->ntof, upb_oneofdef_name(o), upb_value_ptr(o));
   upb_ref2(o, m);
   upb_ref2(m, o);
 
@@ -1589,23 +1615,47 @@ const upb_fielddef *upb_msgdef_itof(const upb_msgdef *m, uint32_t i) {
 const upb_fielddef *upb_msgdef_ntof(const upb_msgdef *m, const char *name,
                                     size_t len) {
   upb_value val;
-  return upb_strtable_lookup2(&m->ntof, name, len, &val) ?
+
-      upb_value_getptr(val) : NULL;
+  if (!upb_strtable_lookup2(&m->ntof, name, len, &val)) {
+    return NULL;
+  }
+
+  return upb_trygetfield(upb_value_getptr(val));
 }
 
 const upb_oneofdef *upb_msgdef_ntoo(const upb_msgdef *m, const char *name,
                                     size_t len) {
   upb_value val;
-  return upb_strtable_lookup2(&m->ntoo, name, len, &val) ?
+
-      upb_value_getptr(val) : NULL;
+  if (!upb_strtable_lookup2(&m->ntof, name, len, &val)) {
+    return NULL;
+  }
+
+  return upb_trygetoneof(upb_value_getptr(val));
+}
+
+bool upb_msgdef_lookupname(const upb_msgdef *m, const char *name, size_t len,
+                           const upb_fielddef **f, const upb_oneofdef **o) {
+  upb_value val;
+
+  if (!upb_strtable_lookup2(&m->ntof, name, len, &val)) {
+    return false;
+  }
+
+  *o = upb_trygetoneof(upb_value_getptr(val));
+  *f = upb_trygetfield(upb_value_getptr(val));
+  assert((*o != NULL) ^ (*f != NULL));  /* Exactly one of the two should be set. */
+  return true;
 }
 
 int upb_msgdef_numfields(const upb_msgdef *m) {
-  return upb_strtable_count(&m->ntof);
+  /* The number table contains only fields. */
+  return upb_inttable_count(&m->itof);
 }
 
 int upb_msgdef_numoneofs(const upb_msgdef *m) {
-  return upb_strtable_count(&m->ntoo);
+  /* The name table includes oneofs, and the number table does not. */
+  return upb_strtable_count(&m->ntof) - upb_inttable_count(&m->itof);
 }
 
 void upb_msgdef_setmapentry(upb_msgdef *m, bool map_entry) {
@@ -1636,10 +1686,21 @@ void upb_msg_field_iter_setdone(upb_msg_field_iter *iter) {
 }
 
 void upb_msg_oneof_begin(upb_msg_oneof_iter *iter, const upb_msgdef *m) {
-  upb_strtable_begin(iter, &m->ntoo);
+  upb_strtable_begin(iter, &m->ntof);
+  /* We need to skip past any initial fields. */
+  while (!upb_strtable_done(iter) &&
+         !upb_isoneof(upb_value_getptr(upb_strtable_iter_value(iter)))) {
+    upb_strtable_next(iter);
+  }
 }
 
-void upb_msg_oneof_next(upb_msg_oneof_iter *iter) { upb_strtable_next(iter); }
+void upb_msg_oneof_next(upb_msg_oneof_iter *iter) {
+  /* We need to skip past fields to return only oneofs. */
+  do {
+    upb_strtable_next(iter);
+  } while (!upb_strtable_done(iter) &&
+           !upb_isoneof(upb_value_getptr(upb_strtable_iter_value(iter))));
+}
 
 bool upb_msg_oneof_done(const upb_msg_oneof_iter *iter) {
   return upb_strtable_done(iter);
@@ -1676,16 +1737,26 @@ static void freeoneof(upb_refcounted *r) {
   upb_gfree(o);
 }
 
+const struct upb_refcounted_vtbl upb_oneofdef_vtbl = {visitoneof, freeoneof};
+
 upb_oneofdef *upb_oneofdef_new(const void *owner) {
-  static const struct upb_refcounted_vtbl vtbl = {visitoneof, freeoneof};
   upb_oneofdef *o = upb_gmalloc(sizeof(*o));
+
+  if (!o) {
+    return NULL;
+  }
+
   o->parent = NULL;
-  if (!o) return NULL;
-  if (!upb_refcounted_init(upb_oneofdef_upcast_mutable(o), &vtbl, owner))
-    goto err2;
   o->name = NULL;
+
+  if (!upb_refcounted_init(upb_oneofdef_upcast_mutable(o), &upb_oneofdef_vtbl,
+                           owner)) {
+    goto err2;
+  }
+
   if (!upb_inttable_init(&o->itof, UPB_CTYPE_PTR)) goto err2;
   if (!upb_strtable_init(&o->ntof, UPB_CTYPE_PTR)) goto err1;
+
   return o;
 
 err1:
@@ -1879,8 +1950,9 @@ static void freefiledef(upb_refcounted *r) {
   upb_gfree(f);
 }
 
+const struct upb_refcounted_vtbl upb_filedef_vtbl = {visitfiledef, freefiledef};
+
 upb_filedef *upb_filedef_new(const void *owner) {
-  static const struct upb_refcounted_vtbl vtbl = {visitfiledef, freefiledef};
   upb_filedef *f = upb_gmalloc(sizeof(*f));
 
   if (!f) {
@@ -1891,7 +1963,8 @@ upb_filedef *upb_filedef_new(const void *owner) {
   f->name = NULL;
   f->syntax = UPB_SYNTAX_PROTO2;
 
-  if (!upb_refcounted_init(upb_filedef_upcast_mutable(f), &vtbl, owner)) {
+  if (!upb_refcounted_init(upb_filedef_upcast_mutable(f), &upb_filedef_vtbl,
+                           owner)) {
     goto err;
   }
 

upb/def.h

@@ -668,6 +668,10 @@ UPB_END_EXTERN_C
 typedef upb_inttable_iter upb_msg_field_iter;
 typedef upb_strtable_iter upb_msg_oneof_iter;
 
+/* Well-known field tag numbers for map-entry messages. */
+#define UPB_MAPENTRY_KEY   1
+#define UPB_MAPENTRY_VALUE 2
+
 #ifdef __cplusplus
 
 /* Structure that describes a single .proto message type.
@@ -921,16 +925,20 @@ 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);
 upb_syntax_t upb_msgdef_syntax(const upb_msgdef *m);
-bool upb_msgdef_setfullname(upb_msgdef *m, const char *fullname, upb_status *s);
 
-upb_msgdef *upb_msgdef_dup(const upb_msgdef *m, const void *owner);
 bool upb_msgdef_addfield(upb_msgdef *m, upb_fielddef *f, const void *ref_donor,
                          upb_status *s);
 bool upb_msgdef_addoneof(upb_msgdef *m, upb_oneofdef *o, const void *ref_donor,
                          upb_status *s);
+bool upb_msgdef_setfullname(upb_msgdef *m, const char *fullname, upb_status *s);
+void upb_msgdef_setmapentry(upb_msgdef *m, bool map_entry);
+bool upb_msgdef_mapentry(const upb_msgdef *m);
+bool upb_msgdef_setsyntax(upb_msgdef *m, upb_syntax_t syntax);
 
 /* Field lookup in a couple of different variations:
  *   - itof = int to field
@@ -972,19 +980,21 @@ UPB_INLINE upb_oneofdef *upb_msgdef_ntoo_mutable(upb_msgdef *m,
   return (upb_oneofdef *)upb_msgdef_ntoo(m, name, len);
 }
 
-void upb_msgdef_setmapentry(upb_msgdef *m, bool map_entry);
+/* Lookup of either field or oneof by name.  Returns whether either was found.
-bool upb_msgdef_mapentry(const upb_msgdef *m);
+ * If the return is true, then the found def will be set, and the non-found
-bool upb_msgdef_setsyntax(upb_msgdef *m, upb_syntax_t syntax);
+ * one set to NULL. */
+bool upb_msgdef_lookupname(const upb_msgdef *m, const char *name, size_t len,
+                           const upb_fielddef **f, const upb_oneofdef **o);
 
-/* Well-known field tag numbers for map-entry messages. */
+UPB_INLINE bool upb_msgdef_lookupnamez(const upb_msgdef *m, const char *name,
-#define UPB_MAPENTRY_KEY   1
+                                       const upb_fielddef **f,
-#define UPB_MAPENTRY_VALUE 2
+                                       const upb_oneofdef **o) {
-
+  return upb_msgdef_lookupname(m, name, strlen(name), f, o);
-const upb_oneofdef *upb_msgdef_findoneof(const upb_msgdef *m,
+}
-                                          const char *name);
-int upb_msgdef_numoneofs(const upb_msgdef *m);
 
-/* upb_msg_field_iter i;
+/* Iteration over fields and oneofs.  For example:
+ *
+ * upb_msg_field_iter i;
  * for(upb_msg_field_begin(&i, m);
  *     !upb_msg_field_done(&i);
  *     upb_msg_field_next(&i)) {

upb/refcounted.h

@@ -103,10 +103,11 @@ struct upb_refcounted {
 
 #ifdef UPB_DEBUG_REFS
 extern upb_alloc upb_alloc_debugrefs;
-#define UPB_REFCOUNT_INIT(refs, ref2s) \
+#define UPB_REFCOUNT_INIT(vtbl, refs, ref2s) \
-    {&static_refcount, NULL, NULL, 0, true, refs, ref2s}
+    {&static_refcount, NULL, vtbl, 0, true, refs, ref2s}
 #else
-#define UPB_REFCOUNT_INIT(refs, ref2s) {&static_refcount, NULL, NULL, 0, true}
+#define UPB_REFCOUNT_INIT(vtbl, refs, ref2s) \
+    {&static_refcount, NULL, vtbl, 0, true}
 #endif
 
 UPB_BEGIN_EXTERN_C

upb/structdefs.int.h

@@ -43,8 +43,8 @@ struct upb_def {
   bool came_from_user;
 };
 
-#define UPB_DEF_INIT(name, type, refs, ref2s) \
+#define UPB_DEF_INIT(name, type, vtbl, refs, ref2s) \
-    { UPB_REFCOUNT_INIT(refs, ref2s), name, NULL, type, false }
+    { UPB_REFCOUNT_INIT(vtbl, refs, ref2s), name, NULL, type, false }
 
 
 /* upb_fielddef ***************************************************************/
@@ -84,12 +84,14 @@ struct upb_fielddef {
   uint32_t index_;
 };
 
+extern const struct upb_refcounted_vtbl upb_fielddef_vtbl;
+
 #define UPB_FIELDDEF_INIT(label, type, intfmt, tagdelim, is_extension, lazy,   \
                           packed, name, num, msgdef, subdef, selector_base,    \
                           index, defaultval, refs, ref2s)                      \
   {                                                                            \
-    UPB_DEF_INIT(name, UPB_DEF_FIELD, refs, ref2s), defaultval, {msgdef},      \
+    UPB_DEF_INIT(name, UPB_DEF_FIELD, &upb_fielddef_vtbl, refs, ref2s),        \
-        {subdef}, NULL, false, false,                                          \
+        defaultval, {msgdef}, {subdef}, NULL, false, false,                    \
         type == UPB_TYPE_STRING || type == UPB_TYPE_BYTES, true, is_extension, \
         lazy, packed, intfmt, tagdelim, type, label, num, selector_base, index \
   }
@@ -105,10 +107,7 @@ struct upb_msgdef {
 
   /* Tables for looking up fields by number and name. */
   upb_inttable itof;  /* int to field */
-  upb_strtable ntof;  /* name to field */
+  upb_strtable ntof;  /* name to field/oneof */
-
-  /* Tables for looking up oneofs by name. */
-  upb_strtable ntoo;  /* name to oneof */
 
   /* Is this a map-entry message? */
   bool map_entry;
@@ -119,14 +118,15 @@ struct upb_msgdef {
   /* TODO(haberman): proper extension ranges (there can be multiple). */
 };
 
+extern const struct upb_refcounted_vtbl upb_msgdef_vtbl;
+
 /* TODO: also support static initialization of the oneofs table. This will be
  * needed if we compile in descriptors that contain oneofs. */
 #define UPB_MSGDEF_INIT(name, selector_count, submsg_field_count, itof, ntof, \
                         map_entry, syntax, refs, ref2s)                       \
   {                                                                           \
-    UPB_DEF_INIT(name, UPB_DEF_MSG, refs, ref2s), selector_count,             \
+    UPB_DEF_INIT(name, UPB_DEF_MSG, &upb_fielddef_vtbl, refs, ref2s),         \
-        submsg_field_count, itof, ntof,                                       \
+        selector_count, submsg_field_count, itof, ntof, map_entry, syntax     \
-        UPB_EMPTY_STRTABLE_INIT(UPB_CTYPE_PTR), map_entry, syntax             \
   }
 
 
@@ -140,8 +140,11 @@ struct upb_enumdef {
   int32_t defaultval;
 };
 
+extern const struct upb_refcounted_vtbl upb_enumdef_vtbl;
+
 #define UPB_ENUMDEF_INIT(name, ntoi, iton, defaultval, refs, ref2s) \
-  { UPB_DEF_INIT(name, UPB_DEF_ENUM, refs, ref2s), ntoi, iton, defaultval }
+  { UPB_DEF_INIT(name, UPB_DEF_ENUM, &upb_enumdef_vtbl, refs, ref2s), ntoi,    \
+    iton, defaultval }
 
 
 /* upb_oneofdef ***************************************************************/
@@ -155,8 +158,10 @@ struct upb_oneofdef {
   const upb_msgdef *parent;
 };
 
+extern const struct upb_refcounted_vtbl upb_oneofdef_vtbl;
+
 #define UPB_ONEOFDEF_INIT(name, ntof, itof, refs, ref2s) \
-  { UPB_REFCOUNT_INIT(refs, ref2s), name, ntof, itof }
+  { UPB_REFCOUNT_INIT(&upb_oneofdef_vtbl, refs, ref2s), name, ntof, itof }
 
 
 /* upb_symtab *****************************************************************/
@@ -167,9 +172,6 @@ struct upb_symtab {
   upb_strtable symtab;
 };
 
-#define UPB_SYMTAB_INIT(symtab, refs, ref2s) \
-  { UPB_REFCOUNT_INIT(refs, ref2s), symtab }
-
 struct upb_filedef {
   upb_refcounted base;
 
@@ -181,4 +183,6 @@ struct upb_filedef {
   upb_inttable deps;
 };
 
+extern const struct upb_refcounted_vtbl upb_filedef_vtbl;
+
 #endif  /* UPB_STATICINIT_H_ */

upb/table.int.h

@@ -261,13 +261,18 @@ typedef struct {
 } upb_table;
 
 #ifdef NDEBUG
-#define UPB_TABLE_INIT(count, mask, ctype, size_lg2, entries) \
+#  define UPB_TABLE_INIT(count, mask, ctype, size_lg2, entries) \
-  {count, mask, ctype, size_lg2, entries}
+     {count, mask, ctype, size_lg2, entries}
 #else
+#  ifdef UPB_DEBUG_REFS
 /* At the moment the only mutable tables we statically initialize are debug
  * ref tables. */
-#define UPB_TABLE_INIT(count, mask, ctype, size_lg2, entries) \
+#    define UPB_TABLE_INIT(count, mask, ctype, size_lg2, entries) \
-  {count, mask, ctype, size_lg2, entries, &upb_alloc_debugrefs}
+       {count, mask, ctype, size_lg2, entries, &upb_alloc_debugrefs}
+#  else
+#    define UPB_TABLE_INIT(count, mask, ctype, size_lg2, entries) \
+       {count, mask, ctype, size_lg2, entries, NULL}
+#  endif
 #endif
 
 typedef struct {