Implemented upb_enumvaldef, for storing information about enumvals.

tests/bindings/lua/test_upb.lua

@@ -91,7 +91,7 @@ function test_def_readers()
   -- enum
   local e = test_messages_proto3['TestAllTypesProto3.NestedEnum']
   assert_true(#e > 3 and #e < 10)
-  assert_equal(2, e:value("BAZ"))
+  assert_equal(2, e:value("BAZ"):number())
 end
 
 function test_msg_map()
@@ -717,6 +717,30 @@ function test_descriptor_error()
   assert_nil(symtab:lookup_msg("ABC"))
 end
 
+function test_duplicate_enumval()
+  local symtab = upb.SymbolTable()
+  local file_proto = descriptor.FileDescriptorProto {
+    name = "test.proto",
+    message_type = upb.Array(descriptor.DescriptorProto, {
+      descriptor.DescriptorProto{
+        name = "ABC",
+      },
+    }),
+    enum_type = upb.Array(descriptor.EnumDescriptorProto, {
+      descriptor.EnumDescriptorProto{
+        name = "MyEnum",
+        value = upb.Array(descriptor.EnumValueDescriptorProto, {
+          descriptor.EnumValueDescriptorProto{
+            name = "ABC",
+            number = 1,
+          }
+        }),
+      },
+    })
+  }
+  assert_error(function () symtab:add_file(upb.encode(file_proto)) end)
+end
+
 function test_duplicate_filename_error()
   local symtab = upb.SymbolTable()
   local file = descriptor.FileDescriptorProto()

upb/bindings/lua/def.c

@@ -34,6 +34,7 @@
 #include "upb/def.h"
 
 #define LUPB_ENUMDEF "lupb.enumdef"
+#define LUPB_ENUMVALDEF "lupb.enumvaldef"
 #define LUPB_FIELDDEF "lupb.fielddef"
 #define LUPB_FILEDEF "lupb.filedef"
 #define LUPB_MSGDEF "lupb.msgdef"
@@ -568,29 +569,20 @@ static int lupb_enumdef_file(lua_State *L) {
 /* lupb_enumdef_value()
  *
  * Handles:
- *   enum.value(number) -> name
- *   enum.value(name) -> number
+ *   enum.value(number) -> enumval
+ *   enum.value(name) -> enumval
  */
 static int lupb_enumdef_value(lua_State *L) {
   const upb_enumdef *e = lupb_enumdef_check(L, 1);
+  const upb_enumvaldef *ev;
 
   switch (lua_type(L, 2)) {
-    case LUA_TNUMBER: {
-      int32_t key = lupb_checkint32(L, 2);
-      /* Pushes "nil" for a NULL pointer. */
-      lua_pushstring(L, upb_enumdef_iton(e, key));
+    case LUA_TNUMBER:
+      ev = upb_enumdef_lookupnum(e, lupb_checkint32(L, 2));
       break;
-    }
-    case LUA_TSTRING: {
-      const char *key = lua_tostring(L, 2);
-      int32_t num;
-      if (upb_enumdef_ntoiz(e, key, &num)) {
-        lua_pushinteger(L, num);
-      } else {
-        lua_pushnil(L);
-      }
+    case LUA_TSTRING:
+      ev = upb_enumdef_lookupnamez(e, lua_tostring(L, 2));
       break;
-    }
     default: {
       const char *msg = lua_pushfstring(L, "number or string expected, got %s",
                                         luaL_typename(L, 2));
@@ -598,6 +590,7 @@ static int lupb_enumdef_value(lua_State *L) {
     }
   }
 
+  lupb_wrapper_pushwrapper(L, 1, ev, LUPB_ENUMVALDEF);
   return 1;
 }
 
@@ -634,6 +627,45 @@ static const struct luaL_Reg lupb_enumdef_m[] = {
 };
 
 
+/* lupb_enumvaldef ************************************************************/
+
+const upb_enumvaldef *lupb_enumvaldef_check(lua_State *L, int narg) {
+  return lupb_wrapper_check(L, narg, LUPB_ENUMVALDEF);
+}
+
+static int lupb_enumvaldef_enum(lua_State *L) {
+  const upb_enumvaldef *ev = lupb_enumvaldef_check(L, 1);
+  const upb_enumdef *e = upb_enumvaldef_enum(ev);
+  lupb_wrapper_pushwrapper(L, 1, e, LUPB_ENUMDEF);
+  return 1;
+}
+
+static int lupb_enumvaldef_fullname(lua_State *L) {
+  const upb_enumvaldef *ev = lupb_enumvaldef_check(L, 1);
+  lua_pushstring(L, upb_enumvaldef_fullname(ev));
+  return 1;
+}
+
+static int lupb_enumvaldef_name(lua_State *L) {
+  const upb_enumvaldef *ev = lupb_enumvaldef_check(L, 1);
+  lua_pushstring(L, upb_enumvaldef_name(ev));
+  return 1;
+}
+
+static int lupb_enumvaldef_number(lua_State *L) {
+  const upb_enumvaldef *ev = lupb_enumvaldef_check(L, 1);
+  lupb_pushint32(L, upb_enumvaldef_number(ev));
+  return 1;
+}
+
+static const struct luaL_Reg lupb_enumvaldef_m[] = {
+  {"enum", lupb_enumvaldef_enum},
+  {"full_name", lupb_enumvaldef_fullname},
+  {"name", lupb_enumvaldef_name},
+  {"number", lupb_enumvaldef_number},
+  {NULL, NULL}
+};
+
 /* lupb_filedef ***************************************************************/
 
 const upb_filedef *lupb_filedef_check(lua_State *L, int narg) {
@@ -875,6 +907,13 @@ static int lupb_symtab_lookupenum(lua_State *L) {
   return 1;
 }
 
+static int lupb_symtab_lookupenumval(lua_State *L) {
+  const upb_symtab *s = lupb_symtab_check(L, 1);
+  const upb_enumvaldef *e = upb_symtab_lookupenumval(s, luaL_checkstring(L, 2));
+  lupb_symtab_pushwrapper(L, 1, e, LUPB_ENUMVALDEF);
+  return 1;
+}
+
 static int lupb_symtab_tostring(lua_State *L) {
   const upb_symtab *s = lupb_symtab_check(L, 1);
   lua_pushfstring(L, "<upb.SymbolTable file_count=%d>",
@@ -887,6 +926,7 @@ static const struct luaL_Reg lupb_symtab_m[] = {
   {"add_set", lupb_symtab_addset},
   {"lookup_msg", lupb_symtab_lookupmsg},
   {"lookup_enum", lupb_symtab_lookupenum},
+  {"lookup_enumval", lupb_symtab_lookupenumval},
   {NULL, NULL}
 };
 
@@ -913,6 +953,7 @@ void lupb_def_registertypes(lua_State *L) {
 
   /* Register types. */
   lupb_register_type(L, LUPB_ENUMDEF,  lupb_enumdef_m,  lupb_enumdef_mm);
+  lupb_register_type(L, LUPB_ENUMVALDEF, lupb_enumvaldef_m,  NULL);
   lupb_register_type(L, LUPB_FIELDDEF, lupb_fielddef_m, NULL);
   lupb_register_type(L, LUPB_FILEDEF,  lupb_filedef_m,  NULL);
   lupb_register_type(L, LUPB_MSGDEF,   lupb_msgdef_m,   lupb_msgdef_mm);

upb/bindings/lua/msg.c

@@ -390,6 +390,7 @@ static int lupb_array_checkindex(lua_State *L, int narg, uint32_t max) {
  *   Array(message_type)
  */
 static int lupb_array_new(lua_State *L) {
+  int arg_count = lua_gettop(L);
   lupb_array *larray;
   upb_arena *arena;
 
@@ -411,6 +412,17 @@ static int lupb_array_new(lua_State *L) {
   larray->arr = upb_array_new(arena, larray->type);
   lupb_cacheset(L, larray->arr);
 
+  if (arg_count > 1) {
+    /* Set initial fields from table. */
+    int msg = arg_count + 1;
+    lua_pushnil(L);
+    while (lua_next(L, 2) != 0) {
+      lua_pushvalue(L, -2);  /* now stack is key, val, key */
+      lua_insert(L, -3);  /* now stack is key, key, val */
+      lua_settable(L, msg);
+    }
+  }
+
   return 1;
 }
 

upb/def.c

@@ -101,9 +101,17 @@ struct upb_enumdef {
   const char *full_name;
   upb_strtable ntoi;
   upb_inttable iton;
+  const upb_enumvaldef *values;
+  int value_count;
   int32_t defaultval;
 };
 
+struct upb_enumvaldef {
+  const upb_enumdef *enum_;
+  const char *full_name;
+  int32_t number;
+};
+
 struct upb_oneofdef {
   const upb_msgdef *parent;
   const char *full_name;
@@ -147,6 +155,7 @@ typedef enum {
   /* Only inside symtab table. */
   UPB_DEFTYPE_MSG = 1,
   UPB_DEFTYPE_ENUM = 2,
+  UPB_DEFTYPE_ENUMVAL = 3,
 
   /* Only inside message table. */
   UPB_DEFTYPE_ONEOF = 1,
@@ -270,10 +279,31 @@ const upb_filedef *upb_enumdef_file(const upb_enumdef *e) {
 }
 
 int32_t upb_enumdef_default(const upb_enumdef *e) {
-  UPB_ASSERT(upb_enumdef_iton(e, e->defaultval));
+  UPB_ASSERT(upb_enumdef_lookupnum(e, e->defaultval));
   return e->defaultval;
 }
 
+const upb_enumvaldef *upb_enumdef_lookupname(const upb_enumdef *def,
+                                             const char *name, size_t len) {
+  upb_value v;
+  return upb_strtable_lookup2(&def->ntoi, name, len, &v)
+             ? upb_value_getconstptr(v)
+             : NULL;
+}
+
+const upb_enumvaldef *upb_enumdef_lookupnum(const upb_enumdef *def, int32_t num) {
+  upb_value v;
+  return upb_inttable_lookup(&def->iton, num, &v) ? upb_value_getconstptr(v)
+                                                  : NULL;
+}
+
+const upb_enumvaldef *upb_enumdef_value(const upb_enumdef *e, int i) {
+  UPB_ASSERT(i >= 0 && i < e->value_count);
+  return &e->values[i];
+}
+
+// Deprecated functions.
+
 int upb_enumdef_numvals(const upb_enumdef *e) {
   return (int)upb_strtable_count(&e->ntoi);
 }
@@ -286,21 +316,6 @@ void upb_enum_begin(upb_enum_iter *i, const upb_enumdef *e) {
 void upb_enum_next(upb_enum_iter *iter) { upb_strtable_next(iter); }
 bool upb_enum_done(upb_enum_iter *iter) { return upb_strtable_done(iter); }
 
-bool upb_enumdef_ntoi(const upb_enumdef *def, const char *name,
-                      size_t len, int32_t *num) {
-  upb_value v;
-  if (!upb_strtable_lookup2(&def->ntoi, name, len, &v)) {
-    return false;
-  }
-  if (num) *num = upb_value_getint32(v);
-  return true;
-}
-
-const char *upb_enumdef_iton(const upb_enumdef *def, int32_t num) {
-  upb_value v;
-  return upb_inttable_lookup(&def->iton, num, &v) ? upb_value_getcstr(v) : NULL;
-}
-
 const char *upb_enum_iter_name(upb_enum_iter *iter) {
   return upb_strtable_iter_key(iter).data;
 }
@@ -310,6 +325,25 @@ int32_t upb_enum_iter_number(upb_enum_iter *iter) {
 }
 
 
+/* upb_enumvaldef *************************************************************/
+
+const upb_enumdef *upb_enumvaldef_enum(const upb_enumvaldef *ev) {
+  return ev->enum_;
+}
+
+const char *upb_enumvaldef_fullname(const upb_enumvaldef *ev) {
+  return ev->full_name;
+}
+
+const char *upb_enumvaldef_name(const upb_enumvaldef *ev) {
+  return shortdefname(ev->full_name);
+}
+
+int32_t upb_enumvaldef_number(const upb_enumvaldef *ev) {
+  return ev->number;
+}
+
+
 /* upb_fielddef ***************************************************************/
 
 const char *upb_fielddef_fullname(const upb_fielddef *f) {
@@ -877,6 +911,14 @@ const upb_enumdef *upb_symtab_lookupenum(const upb_symtab *s, const char *sym) {
       unpack_def(v, UPB_DEFTYPE_ENUM) : NULL;
 }
 
+const upb_enumvaldef *upb_symtab_lookupenumval(const upb_symtab *s,
+                                               const char *sym) {
+  upb_value v;
+  return upb_strtable_lookup(&s->syms, sym, &v)
+             ? unpack_def(v, UPB_DEFTYPE_ENUMVAL)
+             : NULL;
+}
+
 const upb_filedef *upb_symtab_lookupfile(const upb_symtab *s, const char *name) {
   upb_value v;
   return upb_strtable_lookup(&s->files, name, &v) ? upb_value_getconstptr(v)
@@ -1217,7 +1259,9 @@ static void make_layout(symtab_addctx *ctx, const upb_msgdef *m) {
 }
 
 static char *strviewdup(symtab_addctx *ctx, upb_strview view) {
-  return upb_strdup2(view.data, view.size, ctx->arena);
+  char *ret = upb_strdup2(view.data, view.size, ctx->arena);
+  CHK_OOM(ret);
+  return ret;
 }
 
 static bool streql2(const char *a, size_t n, const char *b) {
@@ -1325,6 +1369,8 @@ static char* makejsonname(symtab_addctx *ctx, const char* name) {
 }
 
 static void symtab_add(symtab_addctx *ctx, const char *name, upb_value v) {
+  // TODO: table should support an operation "tryinsert" to avoid the double
+  // lookup.
   if (upb_strtable_lookup(&ctx->symtab->syms, name, NULL)) {
     symtab_errf(ctx, "duplicate symbol '%s'", name);
   }
@@ -1433,11 +1479,11 @@ static void parse_default(symtab_addctx *ctx, const char *str, size_t len,
     }
     case UPB_TYPE_ENUM: {
       const upb_enumdef *e = f->sub.enumdef;
-      int32_t val;
-      if (!upb_enumdef_ntoi(e, str, len, &val)) {
+      const upb_enumvaldef *ev = upb_enumdef_lookupname(e, str, len);
+      if (!ev) {
         goto invalid;
       }
-      f->defaultval.sint = val;
+      f->defaultval.sint = ev->number;
       break;
     }
     case UPB_TYPE_INT64: {
@@ -1721,6 +1767,8 @@ static void create_enumdef(
 
   e->file = ctx->file;
   e->defaultval = 0;
+  e->value_count = 0;
+  e->values = symtab_alloc(ctx, sizeof(*e->values) * n);
 
   if (n == 0) {
     symtab_errf(ctx, "enums must contain at least one value (%s)",
@@ -1728,27 +1776,26 @@ static void create_enumdef(
   }
 
   for (i = 0; i < n; i++) {
-    const google_protobuf_EnumValueDescriptorProto *value = values[i];
-    upb_strview name = google_protobuf_EnumValueDescriptorProto_name(value);
-    char *name2 = strviewdup(ctx, name);
-    int32_t num = google_protobuf_EnumValueDescriptorProto_number(value);
-    upb_value v = upb_value_int32(num);
+    const google_protobuf_EnumValueDescriptorProto *val_proto = values[i];
+    upb_enumvaldef *val = (upb_enumvaldef*)&e->values[e->value_count++];
+    upb_strview name = google_protobuf_EnumValueDescriptorProto_name(val_proto);
+    upb_value v = upb_value_constptr(val);
+
+    val->enum_ = e;
+    val->full_name = makefullname(ctx, prefix, name);
+    val->number = google_protobuf_EnumValueDescriptorProto_number(val_proto);
+    symtab_add(ctx, val->full_name, pack_def(val, UPB_DEFTYPE_ENUMVAL));
 
-    if (i == 0 && e->file->syntax == UPB_SYNTAX_PROTO3 && num != 0) {
+    if (i == 0 && e->file->syntax == UPB_SYNTAX_PROTO3 && val->number != 0) {
       symtab_errf(ctx, "for proto3, the first enum value must be zero (%s)",
                   e->full_name);
     }
 
-    if (upb_strtable_lookup(&e->ntoi, name2, NULL)) {
-      symtab_errf(ctx, "duplicate enum label '%s'", name2);
-    }
-
-    CHK_OOM(name2)
-    CHK_OOM(upb_strtable_insert(&e->ntoi, name2, strlen(name2), v, ctx->arena));
+    CHK_OOM(upb_strtable_insert(&e->ntoi, name.data, name.size, v, ctx->arena));
 
-    if (!upb_inttable_lookup(&e->iton, num, NULL)) {
-      upb_value v = upb_value_cstr(name2);
-      CHK_OOM(upb_inttable_insert(&e->iton, num, v, ctx->arena));
+    // Multiple enumerators can have the same number, first one wins.
+    if (!upb_inttable_lookup(&e->iton, val->number, NULL)) {
+      CHK_OOM(upb_inttable_insert(&e->iton, val->number, v, ctx->arena));
     }
   }
 

upb/def.h

@@ -54,6 +54,8 @@ extern "C" {
 
 struct upb_enumdef;
 typedef struct upb_enumdef upb_enumdef;
+struct upb_enumvaldef;
+typedef struct upb_enumvaldef upb_enumvaldef;
 struct upb_fielddef;
 typedef struct upb_fielddef upb_fielddef;
 struct upb_filedef;
@@ -264,26 +266,34 @@ const char *upb_enumdef_fullname(const upb_enumdef *e);
 const char *upb_enumdef_name(const upb_enumdef *e);
 const upb_filedef *upb_enumdef_file(const upb_enumdef *e);
 int32_t upb_enumdef_default(const upb_enumdef *e);
-int upb_enumdef_numvals(const upb_enumdef *e);
+int upb_enumdef_valuecount(const upb_enumdef *e);
+const upb_enumvaldef *upb_enumdef_value(const upb_enumdef *e, int i);
 
-/* Enum lookups:
- * - ntoi:  look up a name with specified length.
- * - ntoiz: look up a name provided as a null-terminated string.
- * - iton:  look up an integer, returning the name as a null-terminated
- *          string. */
-bool upb_enumdef_ntoi(const upb_enumdef *e, const char *name, size_t len,
-                      int32_t *num);
-UPB_INLINE bool upb_enumdef_ntoiz(const upb_enumdef *e,
-                                  const char *name, int32_t *num) {
-  return upb_enumdef_ntoi(e, name, strlen(name), num);
-}
-const char *upb_enumdef_iton(const upb_enumdef *e, int32_t num);
+const upb_enumvaldef *upb_enumdef_lookupname(const upb_enumdef *e,
+                                             const char *name, size_t len);
+const upb_enumvaldef *upb_enumdef_lookupnum(const upb_enumdef *e, int32_t num);
 
+/* DEPRECATED, slated for removal */
+int upb_enumdef_numvals(const upb_enumdef *e);
 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);
 const char *upb_enum_iter_name(upb_enum_iter *iter);
 int32_t upb_enum_iter_number(upb_enum_iter *iter);
+/* END DEPRECATED */
+
+// Convenience wrapper.
+UPB_INLINE const upb_enumvaldef *upb_enumdef_lookupnamez(const upb_enumdef *e,
+                                                         const char *name) {
+  return upb_enumdef_lookupname(e, name, strlen(name));
+}
+
+/* upb_enumvaldef *************************************************************/
+
+const char *upb_enumvaldef_fullname(const upb_enumvaldef *e);
+const char *upb_enumvaldef_name(const upb_enumvaldef *e);
+int32_t upb_enumvaldef_number(const upb_enumvaldef *e);
+const upb_enumdef *upb_enumvaldef_enum(const upb_enumvaldef *e);
 
 /* upb_filedef ****************************************************************/
 
@@ -308,6 +318,8 @@ const upb_msgdef *upb_symtab_lookupmsg(const upb_symtab *s, const char *sym);
 const upb_msgdef *upb_symtab_lookupmsg2(
     const upb_symtab *s, const char *sym, size_t len);
 const upb_enumdef *upb_symtab_lookupenum(const upb_symtab *s, const char *sym);
+const upb_enumvaldef *upb_symtab_lookupenumval(const upb_symtab *s,
+                                               const char *sym);
 const upb_filedef *upb_symtab_lookupfile(const upb_symtab *s, const char *name);
 const upb_filedef *upb_symtab_lookupfile2(
     const upb_symtab *s, const char *name, size_t len);

upb/def.hpp

@@ -312,6 +312,19 @@ class MessageDefPtr {
   const upb_msgdef* ptr_;
 };
 
+class EnumValDefPtr {
+ public:
+  EnumValDefPtr() : ptr_(nullptr) {}
+  explicit EnumValDefPtr(const upb_enumvaldef* ptr) : ptr_(ptr) {}
+
+  int32_t number() const { return upb_enumvaldef_number(ptr_); }
+  const char *full_name() const { return upb_enumvaldef_fullname(ptr_); }
+  const char *name() const { return upb_enumvaldef_name(ptr_); }
+
+ private:
+  const upb_enumvaldef* ptr_;
+};
+
 class EnumDefPtr {
  public:
   EnumDefPtr() : ptr_(nullptr) {}
@@ -335,15 +348,15 @@ class EnumDefPtr {
   int value_count() const { return upb_enumdef_numvals(ptr_); }
 
   // Lookups from name to integer, returning true if found.
-  bool FindValueByName(const char* name, int32_t* num) const {
-    return upb_enumdef_ntoiz(ptr_, name, num);
+  const EnumValDefPtr FindValueByName(const char* name) const {
+    return EnumValDefPtr(upb_enumdef_lookupnamez(ptr_, name));
   }
 
   // Finds the name corresponding to the given number, or NULL if none was
   // found.  If more than one name corresponds to this number, returns the
   // first one that was added.
-  const char* FindValueByNumber(int32_t num) const {
-    return upb_enumdef_iton(ptr_, num);
+  const EnumValDefPtr FindValueByNumber(int32_t num) const {
+    return EnumValDefPtr(upb_enumdef_lookupnum(ptr_, num));
   }
 
   // Iteration over name/value pairs.  The order is undefined.

upb/json_decode.c

@@ -814,10 +814,13 @@ static upb_msgval jsondec_strfield(jsondec *d, const upb_fielddef *f) {
 static upb_msgval jsondec_enum(jsondec *d, const upb_fielddef *f) {
   switch (jsondec_peek(d)) {
     case JD_STRING: {
-      const upb_enumdef *e = upb_fielddef_enumsubdef(f);
       upb_strview str = jsondec_string(d);
+      const upb_enumdef *e = upb_fielddef_enumsubdef(f);
+      const upb_enumvaldef *ev = upb_enumdef_lookupname(e, str.data, str.size);
       upb_msgval val;
-      if (!upb_enumdef_ntoi(e, str.data, str.size, &val.int32_val)) {
+      if (ev) {
+        val.int32_val = upb_enumvaldef_number(ev);
+      } else {
         if (d->options & UPB_JSONDEC_IGNOREUNKNOWN) {
           val.int32_val = 0;
         } else {

upb/json_encode.c

@@ -203,10 +203,10 @@ static void jsonenc_enum(int32_t val, const upb_fielddef *f, jsonenc *e) {
   if (strcmp(upb_enumdef_fullname(e_def), "google.protobuf.NullValue") == 0) {
     jsonenc_putstr(e, "null");
   } else {
-    const char *name = upb_enumdef_iton(e_def, val);
+    const upb_enumvaldef *ev = upb_enumdef_lookupnum(e_def, val);
 
-    if (name) {
-      jsonenc_printf(e, "\"%s\"", name);
+    if (ev) {
+      jsonenc_printf(e, "\"%s\"", upb_enumvaldef_name(ev));
     } else {
       jsonenc_printf(e, "%" PRId32, val);
     }

upb/text_encode.c

@@ -102,10 +102,10 @@ static void txtenc_endfield(txtenc *e) {
 
 static void txtenc_enum(int32_t val, const upb_fielddef *f, txtenc *e) {
   const upb_enumdef *e_def = upb_fielddef_enumsubdef(f);
-  const char *name = upb_enumdef_iton(e_def, val);
+  const upb_enumvaldef *ev = upb_enumdef_lookupnum(e_def, val);
 
-  if (name) {
-    txtenc_printf(e, "%s", name);
+  if (ev) {
+    txtenc_printf(e, "%s", upb_enumvaldef_name(ev));
   } else {
     txtenc_printf(e, "%" PRId32, val);
   }