Merge pull request #312 from haberman/defiter

Added simple offset-based accessors for defs, and deprecated old iterators

generated_for_cmake/upb/json/parser.c

@@ -3295,7 +3295,7 @@ static void json_parser_reset(upb_json_parser *p) {
 
 static upb_json_parsermethod *parsermethod_new(upb_json_codecache *c,
                                                const upb_msgdef *md) {
-  upb_msg_field_iter i;
+  int i, n;
   upb_alloc *alloc = upb_arena_alloc(c->arena);
 
   upb_json_parsermethod *m = upb_malloc(alloc, sizeof(*m));
@@ -3310,10 +3310,9 @@ static upb_json_parsermethod *parsermethod_new(upb_json_codecache *c,
 
   /* Build name_table */
 
-  for(upb_msg_field_begin(&i, md);
+  n = upb_msgdef_fieldcount(md);
-      !upb_msg_field_done(&i);
+  for(i = 0; i < n; i++) {
-      upb_msg_field_next(&i)) {
+    const upb_fielddef *f = upb_msgdef_field(md, i);
-    const upb_fielddef *f = upb_msg_iter_field(&i);
     upb_value v = upb_value_constptr(f);
     const char *name;
 
@@ -3402,7 +3401,7 @@ const upb_json_parsermethod *upb_json_codecache_get(upb_json_codecache *c,
                                                     const upb_msgdef *md) {
   upb_json_parsermethod *m;
   upb_value v;
-  upb_msg_field_iter i;
+  int i, n;
   upb_alloc *alloc = upb_arena_alloc(c->arena);
 
   if (upb_inttable_lookupptr(&c->methods, md, &v)) {
@@ -3417,10 +3416,9 @@ const upb_json_parsermethod *upb_json_codecache_get(upb_json_codecache *c,
 
   /* Populate parser methods for all submessages, so the name tables will
    * be available during parsing. */
-  for(upb_msg_field_begin(&i, md);
+  n = upb_msgdef_fieldcount(md);
-      !upb_msg_field_done(&i);
+  for(i = 0; i < n; i++) {
-      upb_msg_field_next(&i)) {
+    const upb_fielddef *f = upb_msgdef_field(md, i);
-    upb_fielddef *f = upb_msg_iter_field(&i);
 
     if (upb_fielddef_issubmsg(f)) {
       const upb_msgdef *subdef = upb_fielddef_msgsubdef(f);

tests/bindings/lua/test_upb.lua

@@ -37,6 +37,27 @@ function test_def_readers()
   assert_nil(f:containing_oneof())
   assert_equal(m, f:containing_type())
   assert_equal(0, f:default())
+  local message_field_count = 0
+  for field in m:fields() do
+    message_field_count = message_field_count + 1
+  end
+  assert_equal(message_field_count, #m)
+
+  local message_oneof_count = 0
+  for oneof in m:oneofs() do
+    message_oneof_count = message_oneof_count + 1
+  end
+  assert_equal(message_oneof_count, m:oneof_count())
+
+  -- oneof
+  local o = m:lookup_name("oneof_field")
+  assert_equal("oneof_field", o:name())
+  assert_equal(m, o:containing_type())
+  local oneof_field_count = 0
+  for field in o:fields() do
+    oneof_field_count = oneof_field_count + 1
+  end
+  assert_equal(oneof_field_count, #o)
 
   -- enum
   local e = test_messages_proto3['TestAllTypesProto3.NestedEnum']

upb/bindings/lua/def.c

@@ -159,10 +159,11 @@ static int lupb_fielddef_name(lua_State *L) {
 static int lupb_fielddef_number(lua_State *L) {
   const upb_fielddef *f = lupb_fielddef_check(L, 1);
   int32_t num = upb_fielddef_number(f);
-  if (num)
+  if (num) {
     lua_pushinteger(L, num);
-  else
+  } else {
     lua_pushnil(L);
+  }
   return 1;
 }
 
@@ -224,58 +225,72 @@ static int lupb_oneofdef_containingtype(lua_State *L) {
   return 1;
 }
 
-/* lupb_oneofdef_field()
- *
- * Handles:
- *   oneof.field(field_number)
- *   oneof.field(field_name)
- */
 static int lupb_oneofdef_field(lua_State *L) {
   const upb_oneofdef *o = lupb_oneofdef_check(L, 1);
-  const upb_fielddef *f;
+  int32_t idx = lupb_checkint32(L, 2);
+  int count = upb_oneofdef_fieldcount(o);
 
-  switch (lua_type(L, 2)) {
+  if (idx < 0 || idx >= count) {
-    case LUA_TNUMBER:
+    const char *msg = lua_pushfstring(L, "index %d exceeds field count %d",
-      f = upb_oneofdef_itof(o, lua_tointeger(L, 2));
+                                      idx, count);
-      break;
+    return luaL_argerror(L, 2, msg);
-    case LUA_TSTRING:
-      f = upb_oneofdef_ntofz(o, lua_tostring(L, 2));
-      break;
-    default: {
-      const char *msg = lua_pushfstring(L, "number or string expected, got %s",
-                                        luaL_typename(L, 2));
-      return luaL_argerror(L, 2, msg);
-    }
   }
 
-  lupb_wrapper_pushwrapper(L, 1, f, LUPB_FIELDDEF);
+  lupb_wrapper_pushwrapper(L, 1, upb_oneofdef_field(o, idx), LUPB_FIELDDEF);
   return 1;
 }
 
 static int lupb_oneofiter_next(lua_State *L) {
-  upb_oneof_iter *i = lua_touserdata(L, lua_upvalueindex(1));
+  const upb_oneofdef *o = lupb_oneofdef_check(L, lua_upvalueindex(1));
+  int *index = lua_touserdata(L, lua_upvalueindex(2));
   const upb_fielddef *f;
-  if (upb_oneof_done(i)) return 0;
+  if (*index == upb_oneofdef_fieldcount(o)) return 0;
-  f = upb_oneof_iter_field(i);
+  f = upb_oneofdef_field(o, (*index)++);
-  upb_oneof_next(i);
+  lupb_wrapper_pushwrapper(L, lua_upvalueindex(1), f, LUPB_FIELDDEF);
-  lupb_symtab_pushwrapper(L, lua_upvalueindex(2), f, LUPB_FIELDDEF);
   return 1;
 }
 
 static int lupb_oneofdef_fields(lua_State *L) {
-  const upb_oneofdef *o = lupb_oneofdef_check(L, 1);
+  lupb_oneofdef_check(L, 1);
-  upb_oneof_iter *i = lua_newuserdata(L, sizeof(upb_oneof_iter));
+  int *index = lua_newuserdata(L, sizeof(int));
-  lupb_wrapper_pushsymtab(L, 1);
+  *index = 0;
-  upb_oneof_begin(i, o);
 
-  /* Closure upvalues are: iter, symtab. */
+  /* Closure upvalues are: oneofdef, index. */
   lua_pushcclosure(L, &lupb_oneofiter_next, 2);
   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));
+  lua_pushinteger(L, upb_oneofdef_fieldcount(o));
+  return 1;
+}
+
+/* lupb_oneofdef_lookupfield()
+ *
+ * Handles:
+ *   oneof.lookup_field(field_number)
+ *   oneof.lookup_field(field_name)
+ */
+static int lupb_oneofdef_lookupfield(lua_State *L) {
+  const upb_oneofdef *o = lupb_oneofdef_check(L, 1);
+  const upb_fielddef *f;
+
+  switch (lua_type(L, 2)) {
+    case LUA_TNUMBER:
+      f = upb_oneofdef_itof(o, lua_tointeger(L, 2));
+      break;
+    case LUA_TSTRING:
+      f = upb_oneofdef_ntofz(o, lua_tostring(L, 2));
+      break;
+    default: {
+      const char *msg = lua_pushfstring(L, "number or string expected, got %s",
+                                        luaL_typename(L, 2));
+      return luaL_argerror(L, 2, msg);
+    }
+  }
+
+  lupb_wrapper_pushwrapper(L, 1, f, LUPB_FIELDDEF);
   return 1;
 }
 
@@ -289,6 +304,7 @@ static const struct luaL_Reg lupb_oneofdef_m[] = {
   {"containing_type", lupb_oneofdef_containingtype},
   {"field", lupb_oneofdef_field},
   {"fields", lupb_oneofdef_fields},
+  {"lookup_field", lupb_oneofdef_lookupfield},
   {"name", lupb_oneofdef_name},
   {NULL, NULL}
 };
@@ -309,9 +325,15 @@ const upb_msgdef *lupb_msgdef_check(lua_State *L, int narg) {
   return lupb_wrapper_check(L, narg, LUPB_MSGDEF);
 }
 
-static int lupb_msgdef_len(lua_State *L) {
+static int lupb_msgdef_fieldcount(lua_State *L) {
+  const upb_msgdef *m = lupb_msgdef_check(L, 1);
+  lua_pushinteger(L, upb_msgdef_fieldcount(m));
+  return 1;
+}
+
+static int lupb_msgdef_oneofcount(lua_State *L) {
   const upb_msgdef *m = lupb_msgdef_check(L, 1);
-  lua_pushinteger(L, upb_msgdef_numfields(m));
+  lua_pushinteger(L, upb_msgdef_oneofcount(m));
   return 1;
 }
 
@@ -376,23 +398,21 @@ static int lupb_msgdef_name(lua_State *L) {
 }
 
 static int lupb_msgfielditer_next(lua_State *L) {
-  upb_msg_field_iter *i = lua_touserdata(L, lua_upvalueindex(1));
+  const upb_msgdef *m = lupb_msgdef_check(L, lua_upvalueindex(1));
+  int *index = lua_touserdata(L, lua_upvalueindex(2));
   const upb_fielddef *f;
-
+  if (*index == upb_msgdef_fieldcount(m)) return 0;
-  if (upb_msg_field_done(i)) return 0;
+  f = upb_msgdef_field(m, (*index)++);
-  f = upb_msg_iter_field(i);
+  lupb_wrapper_pushwrapper(L, lua_upvalueindex(1), f, LUPB_FIELDDEF);
-  lupb_symtab_pushwrapper(L, lua_upvalueindex(2), f, LUPB_FIELDDEF);
-  upb_msg_field_next(i);
   return 1;
 }
 
 static int lupb_msgdef_fields(lua_State *L) {
-  const upb_msgdef *m = lupb_msgdef_check(L, 1);
+  lupb_msgdef_check(L, 1);
-  upb_msg_field_iter *i = lua_newuserdata(L, sizeof(upb_msg_field_iter));
+  int *index = lua_newuserdata(L, sizeof(int));
-  lupb_wrapper_pushsymtab(L, 1);
+  *index = 0;
-  upb_msg_field_begin(i, m);
 
-  /* Closure upvalues are: iter, symtab. */
+  /* Closure upvalues are: msgdef, index. */
   lua_pushcclosure(L, &lupb_msgfielditer_next, 2);
   return 1;
 }
@@ -411,22 +431,21 @@ static int lupb_msgdef_fullname(lua_State *L) {
 }
 
 static int lupb_msgoneofiter_next(lua_State *L) {
-  upb_msg_oneof_iter *i = lua_touserdata(L, lua_upvalueindex(1));
+  const upb_msgdef *m = lupb_msgdef_check(L, lua_upvalueindex(1));
+  int *index = lua_touserdata(L, lua_upvalueindex(2));
   const upb_oneofdef *o;
-  if (upb_msg_oneof_done(i)) return 0;
+  if (*index == upb_msgdef_oneofcount(m)) return 0;
-  o = upb_msg_iter_oneof(i);
+  o = upb_msgdef_oneof(m, (*index)++);
-  upb_msg_oneof_next(i);
+  lupb_wrapper_pushwrapper(L, lua_upvalueindex(1), o, LUPB_ONEOFDEF);
-  lupb_symtab_pushwrapper(L, lua_upvalueindex(2), o, LUPB_ONEOFDEF);
   return 1;
 }
 
 static int lupb_msgdef_oneofs(lua_State *L) {
-  const upb_msgdef *m = lupb_msgdef_check(L, 1);
+  lupb_msgdef_check(L, 1);
-  upb_msg_oneof_iter *i = lua_newuserdata(L, sizeof(upb_msg_oneof_iter));
+  int *index = lua_newuserdata(L, sizeof(int));
-  lupb_wrapper_pushsymtab(L, 1);
+  *index = 0;
-  upb_msg_oneof_begin(i, m);
 
-  /* Closure upvalues are: iter, symtab. */
+  /* Closure upvalues are: msgdef, index. */
   lua_pushcclosure(L, &lupb_msgoneofiter_next, 2);
   return 1;
 }
@@ -452,7 +471,7 @@ static int lupb_msgdef_tostring(lua_State *L) {
 
 static const struct luaL_Reg lupb_msgdef_mm[] = {
   {"__call", lupb_msg_pushnew},
-  {"__len", lupb_msgdef_len},
+  {"__len", lupb_msgdef_fieldcount},
   {"__tostring", lupb_msgdef_tostring},
   {NULL, NULL}
 };
@@ -460,10 +479,12 @@ static const struct luaL_Reg lupb_msgdef_mm[] = {
 static const struct luaL_Reg lupb_msgdef_m[] = {
   {"field", lupb_msgdef_field},
   {"fields", lupb_msgdef_fields},
+  {"field_count", lupb_msgdef_fieldcount},
   {"file", lupb_msgdef_file},
   {"full_name", lupb_msgdef_fullname},
   {"lookup_name", lupb_msgdef_lookupname},
   {"name", lupb_msgdef_name},
+  {"oneof_count", lupb_msgdef_oneofcount},
   {"oneofs", lupb_msgdef_oneofs},
   {"syntax", lupb_msgdef_syntax},
   {"_map_entry", lupb_msgdef_mapentry},

upb/def.c

@@ -89,7 +89,9 @@ struct upb_enumdef {
 struct upb_oneofdef {
   const upb_msgdef *parent;
   const char *full_name;
-  uint32_t index;
+  int field_count;
+  bool synthetic;
+  const upb_fielddef **fields;
   upb_strtable ntof;
   upb_inttable itof;
 };
@@ -293,37 +295,6 @@ static bool assign_msg_indices(upb_msgdef *m, upb_status *s) {
   return true;
 }
 
-static bool check_oneofs(upb_msgdef *m, upb_status *s) {
-  int i;
-  int first_synthetic = -1;
-  upb_oneofdef *mutable_oneofs = (upb_oneofdef*)m->oneofs;
-
-  for (i = 0; i < m->oneof_count; i++) {
-    mutable_oneofs[i].index = i;
-
-    if (upb_oneofdef_issynthetic(&mutable_oneofs[i])) {
-      if (first_synthetic == -1) {
-        first_synthetic = i;
-      }
-    } else {
-      if (first_synthetic != -1) {
-        upb_status_seterrf(
-            s, "Synthetic oneofs must be after all other oneofs: %s",
-            upb_oneofdef_name(&mutable_oneofs[i]));
-        return false;
-      }
-    }
-  }
-
-  if (first_synthetic == -1) {
-    m->real_oneof_count = m->oneof_count;
-  } else {
-    m->real_oneof_count = first_synthetic;
-  }
-
-  return true;
-}
-
 static void assign_msg_wellknowntype(upb_msgdef *m) {
   const char *name = upb_msgdef_fullname(m);
   if (name == NULL) {
@@ -726,15 +697,32 @@ int upb_msgdef_numrealoneofs(const upb_msgdef *m) {
   return m->real_oneof_count;
 }
 
+int upb_msgdef_fieldcount(const upb_msgdef *m) {
+  return m->field_count;
+}
+
+int upb_msgdef_oneofcount(const upb_msgdef *m) {
+  return m->oneof_count;
+}
+
+int upb_msgdef_realoneofcount(const upb_msgdef *m) {
+  return m->real_oneof_count;
+}
+
 const upb_msglayout *upb_msgdef_layout(const upb_msgdef *m) {
   return m->layout;
 }
 
-const upb_fielddef *_upb_msgdef_field(const upb_msgdef *m, int i) {
+const upb_fielddef *upb_msgdef_field(const upb_msgdef *m, int i) {
-  if (i >= m->field_count) return NULL;
+  UPB_ASSERT(i >= 0 && i < m->field_count);
   return &m->fields[i];
 }
 
+const upb_oneofdef *upb_msgdef_oneof(const upb_msgdef *m, int i) {
+  UPB_ASSERT(i >= 0 && i < m->oneof_count);
+  return &m->oneofs[i];
+}
+
 bool upb_msgdef_mapentry(const upb_msgdef *m) {
   return m->map_entry;
 }
@@ -822,22 +810,25 @@ const upb_msgdef *upb_oneofdef_containingtype(const upb_oneofdef *o) {
   return o->parent;
 }
 
+int upb_oneofdef_fieldcount(const upb_oneofdef *o) {
+  return o->field_count;
+}
+
+const upb_fielddef *upb_oneofdef_field(const upb_oneofdef *o, int i) {
+  UPB_ASSERT(i < o->field_count);
+  return o->fields[i];
+}
+
 int upb_oneofdef_numfields(const upb_oneofdef *o) {
-  return (int)upb_strtable_count(&o->ntof);
+  return o->field_count;
 }
 
 uint32_t upb_oneofdef_index(const upb_oneofdef *o) {
-  return o->index;
+  return o - o->parent->oneofs;
 }
 
 bool upb_oneofdef_issynthetic(const upb_oneofdef *o) {
-  upb_inttable_iter iter;
+  return o->synthetic;
-  const upb_fielddef *f;
-  upb_inttable_begin(&iter, &o->itof);
-  if (upb_oneofdef_numfields(o) != 1) return false;
-  f = upb_value_getptr(upb_inttable_iter_value(&iter));
-  UPB_ASSERT(f);
-  return f->proto3_optional_;
 }
 
 const upb_fielddef *upb_oneofdef_ntof(const upb_oneofdef *o,
@@ -1154,6 +1145,46 @@ static const char *makefullname(const symtab_addctx *ctx, const char *prefix,
   }
 }
 
+static bool finalize_oneofs(symtab_addctx *ctx, upb_msgdef *m) {
+  int i;
+  int synthetic_count = 0;
+  upb_oneofdef *mutable_oneofs = (upb_oneofdef*)m->oneofs;
+
+  for (i = 0; i < m->oneof_count; i++) {
+    upb_oneofdef *o = &mutable_oneofs[i];
+
+    if (o->synthetic && o->field_count != 1) {
+      upb_status_seterrf(
+          ctx->status, "Synthetic oneofs must have one field, not %d: %s",
+          o->field_count, upb_oneofdef_name(o));
+      return false;
+    }
+
+    if (o->synthetic) {
+      synthetic_count++;
+    } else if (synthetic_count != 0) {
+      upb_status_seterrf(
+          ctx->status, "Synthetic oneofs must be after all other oneofs: %s",
+          upb_oneofdef_name(o));
+      return false;
+    }
+
+    o->fields = upb_malloc(ctx->alloc, sizeof(upb_fielddef*) * o->field_count);
+    o->field_count = 0;
+  }
+
+  for (i = 0; i < m->field_count; i++) {
+    const upb_fielddef *f = &m->fields[i];
+    upb_oneofdef *o = (upb_oneofdef*)f->oneof;
+    if (o) {
+      o->fields[o->field_count++] = f;
+    }
+  }
+
+  m->real_oneof_count = m->oneof_count - synthetic_count;
+  return true;
+}
+
 size_t getjsonname(const char *name, char *buf, size_t len) {
   size_t src, dst = 0;
   bool ucase_next = false;
@@ -1270,6 +1301,8 @@ static bool create_oneofdef(
   o = (upb_oneofdef*)&m->oneofs[m->oneof_count++];
   o->parent = m;
   o->full_name = makefullname(ctx, m->full_name, name);
+  o->field_count = 0;
+  o->synthetic = false;
 
   v = pack_def(o, UPB_DEFTYPE_ONEOF);
   CHK_OOM(symtab_add(ctx, o->full_name, v));
@@ -1549,10 +1582,20 @@ static bool create_fielddef(
     oneof = (upb_oneofdef*)&m->oneofs[oneof_index];
     f->oneof = oneof;
 
+    oneof->field_count++;
+    if (f->proto3_optional_) {
+      oneof->synthetic = true;
+    }
     CHK(upb_inttable_insert2(&oneof->itof, f->number_, v, alloc));
     CHK(upb_strtable_insert3(&oneof->ntof, name.data, name.size, v, alloc));
   } else {
     f->oneof = NULL;
+    if (f->proto3_optional_) {
+      upb_status_seterrf(ctx->status,
+                         "field with proto3_optional was not in a oneof (%s)",
+                         f->full_name);
+      return false;
+    }
   }
 
   options = google_protobuf_FieldDescriptorProto_has_options(field_proto) ?
@@ -1692,7 +1735,7 @@ static bool create_msgdef(symtab_addctx *ctx, const char *prefix,
   }
 
   CHK(assign_msg_indices(m, ctx->status));
-  CHK(check_oneofs(m, ctx->status));
+  CHK(finalize_oneofs(ctx, m));
   assign_msg_wellknowntype(m);
   upb_inttable_compact2(&m->itof, ctx->alloc);
 

upb/def.h

@@ -117,9 +117,10 @@ typedef upb_inttable_iter upb_oneof_iter;
 
 const char *upb_oneofdef_name(const upb_oneofdef *o);
 const upb_msgdef *upb_oneofdef_containingtype(const upb_oneofdef *o);
-int upb_oneofdef_numfields(const upb_oneofdef *o);
 uint32_t upb_oneofdef_index(const upb_oneofdef *o);
 bool upb_oneofdef_issynthetic(const upb_oneofdef *o);
+int upb_oneofdef_fieldcount(const upb_oneofdef *o);
+const upb_fielddef *upb_oneofdef_field(const upb_oneofdef *o, int i);
 
 /* Oneof lookups:
  * - ntof:  look up a field by name.
@@ -133,11 +134,8 @@ UPB_INLINE const upb_fielddef *upb_oneofdef_ntofz(const upb_oneofdef *o,
 }
 const upb_fielddef *upb_oneofdef_itof(const upb_oneofdef *o, uint32_t num);
 
-/*  upb_oneof_iter i;
+/* DEPRECATED, slated for removal. */
- *  for(upb_oneof_begin(&i, e); !upb_oneof_done(&i); upb_oneof_next(&i)) {
+int upb_oneofdef_numfields(const upb_oneofdef *o);
- *    // ...
- *  }
- */
 void upb_oneof_begin(upb_oneof_iter *iter, const upb_oneofdef *o);
 void upb_oneof_next(upb_oneof_iter *iter);
 bool upb_oneof_done(upb_oneof_iter *iter);
@@ -145,6 +143,7 @@ upb_fielddef *upb_oneof_iter_field(const upb_oneof_iter *iter);
 void upb_oneof_iter_setdone(upb_oneof_iter *iter);
 bool upb_oneof_iter_isequal(const upb_oneof_iter *iter1,
                             const upb_oneof_iter *iter2);
+/* END DEPRECATED */
 
 /* upb_msgdef *****************************************************************/
 
@@ -170,21 +169,21 @@ typedef upb_strtable_iter upb_msg_oneof_iter;
 const char *upb_msgdef_fullname(const upb_msgdef *m);
 const upb_filedef *upb_msgdef_file(const upb_msgdef *m);
 const char *upb_msgdef_name(const upb_msgdef *m);
-int upb_msgdef_numfields(const upb_msgdef *m);
-int upb_msgdef_numoneofs(const upb_msgdef *m);
-int upb_msgdef_numrealoneofs(const upb_msgdef *m);
 upb_syntax_t upb_msgdef_syntax(const upb_msgdef *m);
 bool upb_msgdef_mapentry(const upb_msgdef *m);
 upb_wellknowntype_t upb_msgdef_wellknowntype(const upb_msgdef *m);
 bool upb_msgdef_iswrapper(const upb_msgdef *m);
 bool upb_msgdef_isnumberwrapper(const upb_msgdef *m);
+int upb_msgdef_fieldcount(const upb_msgdef *m);
+int upb_msgdef_oneofcount(const upb_msgdef *m);
+const upb_fielddef *upb_msgdef_field(const upb_msgdef *m, int i);
+const upb_oneofdef *upb_msgdef_oneof(const upb_msgdef *m, int i);
 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);
 const upb_oneofdef *upb_msgdef_ntoo(const upb_msgdef *m, const char *name,
                                     size_t len);
 const upb_msglayout *upb_msgdef_layout(const upb_msgdef *m);
-const upb_fielddef *_upb_msgdef_field(const upb_msgdef *m, int i);
 
 UPB_INLINE const upb_oneofdef *upb_msgdef_ntooz(const upb_msgdef *m,
                                                const char *name) {
@@ -216,19 +215,10 @@ UPB_INLINE bool upb_msgdef_lookupnamez(const upb_msgdef *m, const char *name,
 const upb_fielddef *upb_msgdef_lookupjsonname(const upb_msgdef *m,
                                               const char *name, size_t len);
 
-/* Iteration over fields and oneofs.  For example:
+/* DEPRECATED, slated for removal */
- *
+int upb_msgdef_numfields(const upb_msgdef *m);
- * upb_msg_field_iter i;
+int upb_msgdef_numoneofs(const upb_msgdef *m);
- * for(upb_msg_field_begin(&i, m);
+int upb_msgdef_numrealoneofs(const upb_msgdef *m);
- *     !upb_msg_field_done(&i);
- *     upb_msg_field_next(&i)) {
- *   upb_fielddef *f = upb_msg_iter_field(&i);
- *   // ...
- * }
- *
- * For C we don't have separate iterators for const and non-const.
- * It is the caller's responsibility to cast the upb_fielddef* to
- * const if the upb_msgdef* is const. */
 void upb_msg_field_begin(upb_msg_field_iter *iter, const upb_msgdef *m);
 void upb_msg_field_next(upb_msg_field_iter *iter);
 bool upb_msg_field_done(const upb_msg_field_iter *iter);
@@ -236,9 +226,6 @@ upb_fielddef *upb_msg_iter_field(const upb_msg_field_iter *iter);
 void upb_msg_field_iter_setdone(upb_msg_field_iter *iter);
 bool upb_msg_field_iter_isequal(const upb_msg_field_iter * iter1,
                                 const upb_msg_field_iter * iter2);
-
-/* Similar to above, we also support iterating through the oneofs in a
- * msgdef. */
 void upb_msg_oneof_begin(upb_msg_oneof_iter * iter, const upb_msgdef *m);
 void upb_msg_oneof_next(upb_msg_oneof_iter * iter);
 bool upb_msg_oneof_done(const upb_msg_oneof_iter *iter);
@@ -246,6 +233,7 @@ const upb_oneofdef *upb_msg_iter_oneof(const upb_msg_oneof_iter *iter);
 void upb_msg_oneof_iter_setdone(upb_msg_oneof_iter * iter);
 bool upb_msg_oneof_iter_isequal(const upb_msg_oneof_iter *iter1,
                                 const upb_msg_oneof_iter *iter2);
+/* END DEPRECATED */
 
 /* upb_enumdef ****************************************************************/
 
@@ -270,11 +258,6 @@ UPB_INLINE bool upb_enumdef_ntoiz(const upb_enumdef *e,
 }
 const char *upb_enumdef_iton(const upb_enumdef *e, int32_t num);
 
-/*  upb_enum_iter i;
- *  for(upb_enum_begin(&i, e); !upb_enum_done(&i); upb_enum_next(&i)) {
- *    // ...
- *  }
- */
 void upb_enum_begin(upb_enum_iter *iter, const upb_enumdef *e);
 void upb_enum_next(upb_enum_iter *iter);
 bool upb_enum_done(upb_enum_iter *iter);

upb/def.hpp

@@ -129,17 +129,17 @@ class OneofDefPtr {
   explicit OneofDefPtr(const upb_oneofdef* ptr) : ptr_(ptr) {}
 
   const upb_oneofdef* ptr() const { return ptr_; }
-  explicit operator bool() { return ptr_ != nullptr; }
+  explicit operator bool() const { return ptr_ != nullptr; }
 
-  // Returns the MessageDef that owns this OneofDef.
+  // Returns the MessageDef that contains this OneofDef.
   MessageDefPtr containing_type() const;
 
-  // Returns the name of this oneof. This is the name used to look up the oneof
+  // Returns the name of this oneof.
-  // by name once added to a message def.
   const char* name() const { return upb_oneofdef_name(ptr_); }
 
-  // Returns the number of fields currently defined in the oneof.
+  // Returns the number of fields in the oneof.
   int field_count() const { return upb_oneofdef_numfields(ptr_); }
+  FieldDefPtr field(int i) const { return FieldDefPtr(upb_oneofdef_field(ptr_, i)); }
 
   // Looks up by name.
   FieldDefPtr FindFieldByName(const char* name, size_t len) const {
@@ -159,40 +159,6 @@ class OneofDefPtr {
     return FieldDefPtr(upb_oneofdef_itof(ptr_, num));
   }
 
-  class const_iterator
-      : public std::iterator<std::forward_iterator_tag, FieldDefPtr> {
-   public:
-    void operator++() { upb_oneof_next(&iter_); }
-
-    FieldDefPtr operator*() const {
-      return FieldDefPtr(upb_oneof_iter_field(&iter_));
-    }
-
-    bool operator!=(const const_iterator& other) const {
-      return !upb_oneof_iter_isequal(&iter_, &other.iter_);
-    }
-
-    bool operator==(const const_iterator& other) const {
-      return upb_oneof_iter_isequal(&iter_, &other.iter_);
-    }
-
-   private:
-    friend class OneofDefPtr;
-
-    const_iterator() {}
-    explicit const_iterator(OneofDefPtr o) { upb_oneof_begin(&iter_, o.ptr()); }
-    static const_iterator end() {
-      const_iterator iter;
-      upb_oneof_iter_setdone(&iter.iter_);
-      return iter;
-    }
-
-    upb_oneof_iter iter_;
-  };
-
-  const_iterator begin() const { return const_iterator(*this); }
-  const_iterator end() const { return const_iterator::end(); }
-
  private:
   const upb_oneofdef* ptr_;
 };
@@ -211,9 +177,11 @@ class MessageDefPtr {
 
   // The number of fields that belong to the MessageDef.
   int field_count() const { return upb_msgdef_numfields(ptr_); }
+  FieldDefPtr field(int i) const { return FieldDefPtr(upb_msgdef_field(ptr_, i)); }
 
   // The number of oneofs that belong to the MessageDef.
   int oneof_count() const { return upb_msgdef_numoneofs(ptr_); }
+  OneofDefPtr oneof(int i) const { return OneofDefPtr(upb_msgdef_oneof(ptr_, i)); }
 
   upb_syntax_t syntax() const { return upb_msgdef_syntax(ptr_); }
 
@@ -258,112 +226,58 @@ class MessageDefPtr {
   // Whether is a number wrapper.
   bool isnumberwrapper() const { return upb_msgdef_isnumberwrapper(ptr_); }
 
-  // Iteration over fields.  The order is undefined.
+ private:
-  class const_field_iterator
+  class FieldIter {
-      : public std::iterator<std::forward_iterator_tag, FieldDefPtr> {
    public:
-    void operator++() { upb_msg_field_next(&iter_); }
+    explicit FieldIter(const upb_msgdef *m, int i) : m_(m), i_(i) {}
-
+    void operator++() { i_++; }
-    FieldDefPtr operator*() const {
-      return FieldDefPtr(upb_msg_iter_field(&iter_));
-    }
 
-    bool operator!=(const const_field_iterator& other) const {
+    FieldDefPtr operator*() { return FieldDefPtr(upb_msgdef_field(m_, i_)); }
-      return !upb_msg_field_iter_isequal(&iter_, &other.iter_);
+    bool operator!=(const FieldIter& other) { return i_ != other.i_; }
-    }
+    bool operator==(const FieldIter& other) { return i_ == other.i_; }
-
-    bool operator==(const const_field_iterator& other) const {
-      return upb_msg_field_iter_isequal(&iter_, &other.iter_);
-    }
 
    private:
-    friend class MessageDefPtr;
+    const upb_msgdef *m_;
-
+    int i_;
-    explicit const_field_iterator() {}
-
-    explicit const_field_iterator(MessageDefPtr msg) {
-      upb_msg_field_begin(&iter_, msg.ptr());
-    }
-
-    static const_field_iterator end() {
-      const_field_iterator iter;
-      upb_msg_field_iter_setdone(&iter.iter_);
-      return iter;
-    }
-
-    upb_msg_field_iter iter_;
   };
 
-  // Iteration over oneofs. The order is undefined.
+  class FieldAccessor {
-  class const_oneof_iterator
-      : public std::iterator<std::forward_iterator_tag, OneofDefPtr> {
    public:
-    void operator++() { upb_msg_oneof_next(&iter_); }
+    explicit FieldAccessor(const upb_msgdef* md) : md_(md) {}
-
+    FieldIter begin() { return FieldIter(md_, 0); }
-    OneofDefPtr operator*() const {
+    FieldIter end() { return FieldIter(md_, upb_msgdef_fieldcount(md_)); }
-      return OneofDefPtr(upb_msg_iter_oneof(&iter_));
-    }
-
-    bool operator!=(const const_oneof_iterator& other) const {
-      return !upb_msg_oneof_iter_isequal(&iter_, &other.iter_);
-    }
-
-    bool operator==(const const_oneof_iterator& other) const {
-      return upb_msg_oneof_iter_isequal(&iter_, &other.iter_);
-    }
 
    private:
-    friend class MessageDefPtr;
+    const upb_msgdef* md_;
-
-    const_oneof_iterator() {}
-
-    explicit const_oneof_iterator(MessageDefPtr msg) {
-      upb_msg_oneof_begin(&iter_, msg.ptr());
-    }
-
-    static const_oneof_iterator end() {
-      const_oneof_iterator iter;
-      upb_msg_oneof_iter_setdone(&iter.iter_);
-      return iter;
-    }
-
-    upb_msg_oneof_iter iter_;
   };
 
-  class ConstFieldAccessor {
+  class OneofIter {
    public:
-    explicit ConstFieldAccessor(const upb_msgdef* md) : md_(md) {}
+    explicit OneofIter(const upb_msgdef *m, int i) : m_(m), i_(i) {}
-    const_field_iterator begin() { return MessageDefPtr(md_).field_begin(); }
+    void operator++() { i_++; }
-    const_field_iterator end() { return MessageDefPtr(md_).field_end(); }
+
+    OneofDefPtr operator*() { return OneofDefPtr(upb_msgdef_oneof(m_, i_)); }
+    bool operator!=(const OneofIter& other) { return i_ != other.i_; }
+    bool operator==(const OneofIter& other) { return i_ == other.i_; }
 
    private:
-    const upb_msgdef* md_;
+    const upb_msgdef *m_;
+    int i_;
   };
 
-  class ConstOneofAccessor {
+  class OneofAccessor {
    public:
-    explicit ConstOneofAccessor(const upb_msgdef* md) : md_(md) {}
+    explicit OneofAccessor(const upb_msgdef* md) : md_(md) {}
-    const_oneof_iterator begin() { return MessageDefPtr(md_).oneof_begin(); }
+    OneofIter begin() { return OneofIter(md_, 0); }
-    const_oneof_iterator end() { return MessageDefPtr(md_).oneof_end(); }
+    OneofIter end() { return OneofIter(md_, upb_msgdef_oneofcount(md_)); }
 
    private:
     const upb_msgdef* md_;
   };
 
-  const_field_iterator field_begin() const {
+ public:
-    return const_field_iterator(*this);
+  FieldAccessor fields() const { return FieldAccessor(ptr()); }
-  }
+  OneofAccessor oneofs() const { return OneofAccessor(ptr()); }
-
-  const_field_iterator field_end() const { return const_field_iterator::end(); }
-
-  const_oneof_iterator oneof_begin() const {
-    return const_oneof_iterator(*this);
-  }
-
-  const_oneof_iterator oneof_end() const { return const_oneof_iterator::end(); }
-
-  ConstFieldAccessor fields() const { return ConstFieldAccessor(ptr()); }
-  ConstOneofAccessor oneofs() const { return ConstOneofAccessor(ptr()); }
 
  private:
   const upb_msgdef* ptr_;

upb/handlers.c

@@ -359,7 +359,7 @@ struct upb_handlercache {
 
 const upb_handlers *upb_handlercache_get(upb_handlercache *c,
                                          const upb_msgdef *md) {
-  upb_msg_field_iter i;
+  int i, n;
   upb_value v;
   upb_handlers *h;
 
@@ -377,10 +377,9 @@ const upb_handlers *upb_handlercache_get(upb_handlercache *c,
 
   /* For each submessage field, get or create a handlers object and set it as
    * the subhandlers. */
-  for(upb_msg_field_begin(&i, md);
+  n = upb_msgdef_fieldcount(md);
-      !upb_msg_field_done(&i);
+  for (i = 0; i < n; i++) {
-      upb_msg_field_next(&i)) {
+    const upb_fielddef *f = upb_msgdef_field(md, i);
-    upb_fielddef *f = upb_msg_iter_field(&i);
 
     if (upb_fielddef_issubmsg(f)) {
       const upb_msgdef *subdef = upb_fielddef_msgsubdef(f);

upb/json/parser.rl

@@ -2858,7 +2858,7 @@ static void json_parser_reset(upb_json_parser *p) {
 
 static upb_json_parsermethod *parsermethod_new(upb_json_codecache *c,
                                                const upb_msgdef *md) {
-  upb_msg_field_iter i;
+  int i, n;
   upb_alloc *alloc = upb_arena_alloc(c->arena);
 
   upb_json_parsermethod *m = upb_malloc(alloc, sizeof(*m));
@@ -2873,10 +2873,9 @@ static upb_json_parsermethod *parsermethod_new(upb_json_codecache *c,
 
   /* Build name_table */
 
-  for(upb_msg_field_begin(&i, md);
+  n = upb_msgdef_fieldcount(md);
-      !upb_msg_field_done(&i);
+  for(i = 0; i < n; i++) {
-      upb_msg_field_next(&i)) {
+    const upb_fielddef *f = upb_msgdef_field(md, i);
-    const upb_fielddef *f = upb_msg_iter_field(&i);
     upb_value v = upb_value_constptr(f);
     const char *name;
 
@@ -2965,7 +2964,7 @@ const upb_json_parsermethod *upb_json_codecache_get(upb_json_codecache *c,
                                                     const upb_msgdef *md) {
   upb_json_parsermethod *m;
   upb_value v;
-  upb_msg_field_iter i;
+  int i, n;
   upb_alloc *alloc = upb_arena_alloc(c->arena);
 
   if (upb_inttable_lookupptr(&c->methods, md, &v)) {
@@ -2980,10 +2979,9 @@ const upb_json_parsermethod *upb_json_codecache_get(upb_json_codecache *c,
 
   /* Populate parser methods for all submessages, so the name tables will
    * be available during parsing. */
-  for(upb_msg_field_begin(&i, md);
+  n = upb_msgdef_fieldcount(md);
-      !upb_msg_field_done(&i);
+  for(i = 0; i < n; i++) {
-      upb_msg_field_next(&i)) {
+    const upb_fielddef *f = upb_msgdef_field(md, i);
-    upb_fielddef *f = upb_msg_iter_field(&i);
 
     if (upb_fielddef_issubmsg(f)) {
       const upb_msgdef *subdef = upb_fielddef_msgsubdef(f);

upb/json/printer.c

@@ -1124,16 +1124,16 @@ void printer_sethandlers_timestamp(const void *closure, upb_handlers *h) {
 
 void printer_sethandlers_value(const void *closure, upb_handlers *h) {
   const upb_msgdef *md = upb_handlers_msgdef(h);
-  upb_msg_field_iter i;
+  int i, n;
 
   upb_handlerattr empty_attr = UPB_HANDLERATTR_INIT;
 
   upb_handlers_setstartmsg(h, printer_startmsg_noframe, &empty_attr);
   upb_handlers_setendmsg(h, printer_endmsg_noframe, &empty_attr);
 
-  upb_msg_field_begin(&i, md);
+  n = upb_msgdef_fieldcount(md);
-  for(; !upb_msg_field_done(&i); upb_msg_field_next(&i)) {
+  for (i = 0; i < n; i++) {
-    const upb_fielddef *f = upb_msg_iter_field(&i);
+    const upb_fielddef *f = upb_msgdef_field(md, i);
 
     switch (upb_fielddef_type(f)) {
       case UPB_TYPE_ENUM:
@@ -1222,7 +1222,7 @@ void printer_sethandlers(const void *closure, upb_handlers *h) {
   const upb_msgdef *md = upb_handlers_msgdef(h);
   bool is_mapentry = upb_msgdef_mapentry(md);
   upb_handlerattr empty_attr = UPB_HANDLERATTR_INIT;
-  upb_msg_field_iter i;
+  int i, n;
   const upb_json_printercache *cache = closure;
   const bool preserve_fieldnames = cache->preserve_fieldnames;
 
@@ -1287,9 +1287,9 @@ void printer_sethandlers(const void *closure, upb_handlers *h) {
     }                                                                         \
     break;
 
-  upb_msg_field_begin(&i, md);
+  n = upb_msgdef_fieldcount(md);
-  for(; !upb_msg_field_done(&i); upb_msg_field_next(&i)) {
+  for (i = 0; i < n; i++) {
-    const upb_fielddef *f = upb_msg_iter_field(&i);
+    const upb_fielddef *f = upb_msgdef_field(md, i);
 
     upb_handlerattr name_attr = UPB_HANDLERATTR_INIT;
     name_attr.handler_data = newstrpc(h, f, preserve_fieldnames);

upb/json_encode.c

@@ -648,10 +648,10 @@ static void jsonenc_msgfields(jsonenc *e, const upb_msg *msg,
 
   if (e->options & UPB_JSONENC_EMITDEFAULTS) {
     /* Iterate over all fields. */
-    upb_msg_field_iter i;
+    int i = 0;
-    for (upb_msg_field_begin(&i, m); !upb_msg_field_done(&i);
+    int n = upb_msgdef_fieldcount(m);
-         upb_msg_field_next(&i)) {
+    for (i = 0; i < n; i++) {
-      f = upb_msg_iter_field(&i);
+      f = upb_msgdef_field(m, i);
       jsonenc_fieldval(e, f, upb_msg_get(msg, f), &first);
     }
   } else {

upb/pb/compile_decoder.c

@@ -701,7 +701,7 @@ static void compile_method(compiler *c, upb_pbdecodermethod *method) {
   const upb_handlers *h;
   const upb_msgdef *md;
   uint32_t* start_pc;
-  upb_msg_field_iter i;
+  int i, n;
   upb_value val;
 
   UPB_ASSERT(method);
@@ -718,10 +718,9 @@ static void compile_method(compiler *c, upb_pbdecodermethod *method) {
   putsel(c, OP_STARTMSG, UPB_STARTMSG_SELECTOR, h);
  label(c, LABEL_FIELD);
   start_pc = c->pc;
-  for(upb_msg_field_begin(&i, md);
+  n = upb_msgdef_fieldcount(md);
-      !upb_msg_field_done(&i);
+  for(i = 0; i < n; i++) {
-      upb_msg_field_next(&i)) {
+    const upb_fielddef *f = upb_msgdef_field(md, i);
-    const upb_fielddef *f = upb_msg_iter_field(&i);
     upb_fieldtype_t type = upb_fielddef_type(f);
 
     if (type == UPB_TYPE_MESSAGE && !(haslazyhandlers(h, f) && c->lazy)) {
@@ -765,7 +764,7 @@ static void compile_method(compiler *c, upb_pbdecodermethod *method) {
  * Generates a new method for every destination handlers reachable from "h". */
 static void find_methods(compiler *c, const upb_handlers *h) {
   upb_value v;
-  upb_msg_field_iter i;
+  int i, n;
   const upb_msgdef *md;
   upb_pbdecodermethod *method;
 
@@ -777,10 +776,9 @@ static void find_methods(compiler *c, const upb_handlers *h) {
 
   /* Find submethods. */
   md = upb_handlers_msgdef(h);
-  for(upb_msg_field_begin(&i, md);
+  n = upb_msgdef_fieldcount(md);
-      !upb_msg_field_done(&i);
+  for (i = 0; i < n; i++) {
-      upb_msg_field_next(&i)) {
+    const upb_fielddef *f = upb_msgdef_field(md, i);
-    const upb_fielddef *f = upb_msg_iter_field(&i);
     const upb_handlers *sub_h;
     if (upb_fielddef_type(f) == UPB_TYPE_MESSAGE &&
         (sub_h = upb_handlers_getsubhandlers(h, f)) != NULL) {

upb/pb/encoder.c

@@ -437,7 +437,7 @@ T(sint64,   int64_t,  upb_zzenc_64, doencode_varint)
 #include <stdio.h>
 static void newhandlers_callback(const void *closure, upb_handlers *h) {
   const upb_msgdef *m;
-  upb_msg_field_iter i;
+  int i, n;
 
   UPB_UNUSED(closure);
 
@@ -446,10 +446,9 @@ static void newhandlers_callback(const void *closure, upb_handlers *h) {
   upb_handlers_setunknown(h, encode_unknown, NULL);
 
   m = upb_handlers_msgdef(h);
-  for(upb_msg_field_begin(&i, m);
+  n = upb_msgdef_fieldcount(m);
-      !upb_msg_field_done(&i);
+  for(i = 0; i < n; i++) {
-      upb_msg_field_next(&i)) {
+    const upb_fielddef *f = upb_msgdef_field(m, i);
-    const upb_fielddef *f = upb_msg_iter_field(&i);
     bool packed = upb_fielddef_isseq(f) && upb_fielddef_isprimitive(f) &&
                   upb_fielddef_packed(f);
     upb_handlerattr attr = UPB_HANDLERATTR_INIT;

upb/pb/textprinter.c

@@ -252,16 +252,15 @@ err:
 
 static void onmreg(const void *c, upb_handlers *h) {
   const upb_msgdef *m = upb_handlers_msgdef(h);
-  upb_msg_field_iter i;
+  int i, n;
   UPB_UNUSED(c);
 
   upb_handlers_setstartmsg(h, textprinter_startmsg, NULL);
   upb_handlers_setendmsg(h, textprinter_endmsg, NULL);
 
-  for(upb_msg_field_begin(&i, m);
+  n = upb_msgdef_fieldcount(m);
-      !upb_msg_field_done(&i);
+  for(i = 0; i < n; i++) {
-      upb_msg_field_next(&i)) {
+    const upb_fielddef *f = upb_msgdef_field(m, i);
-    upb_fielddef *f = upb_msg_iter_field(&i);
     upb_handlerattr attr = UPB_HANDLERATTR_INIT;
     attr.handler_data = f;
     switch (upb_fielddef_type(f)) {

upb/reflection.c

@@ -207,11 +207,12 @@ void upb_msg_clear(upb_msg *msg, const upb_msgdef *m) {
 bool upb_msg_next(const upb_msg *msg, const upb_msgdef *m,
                   const upb_symtab *ext_pool, const upb_fielddef **out_f,
                   upb_msgval *out_val, size_t *iter) {
-  size_t i = *iter;
+  int i = *iter;
+  int n = upb_msgdef_fieldcount(m);
   const upb_msgval zero = {0};
-  const upb_fielddef *f;
   UPB_UNUSED(ext_pool);
-  while ((f = _upb_msgdef_field(m, (int)++i)) != NULL) {
+  while (++i < n) {
+    const upb_fielddef *f = upb_msgdef_field(m, i);
     upb_msgval val = _upb_msg_getraw(msg, f);
 
     /* Skip field if unset or empty. */