Added proper support for enum default values.

7 files changed, 67 insertions, 37 deletions

diff --git a/Makefile b/Makefile
index 13ce46a..008cc85 100644
--- a/Makefile
+++ b/Makefile
@@ -62,7 +62,6 @@ $(ALLSRC): perf-cppflags
 # The core library -- the absolute minimum you must compile in to successfully
 # bootstrap.
 CORE= \
-  src/descriptor.c \
   src/upb.c \
   src/upb_table.c \
   src/upb_string.c \
diff --git a/lang_ext/lua/test.lua b/lang_ext/lua/test.lua
index 67c955e..072f646 100644
--- a/lang_ext/lua/test.lua
+++ b/lang_ext/lua/test.lua
@@ -23,12 +23,12 @@ SpeedMessage1 = symtab:lookup("benchmarks.SpeedMessage1")
 print(SpeedMessage1:name())
 
 msg = SpeedMessage1()
---print(msg.field1)
+print(msg.foo)
 -- print(msg.field129)
 -- print(msg.field271)
-print(msg.field15.field15)
-msg.field15.field15 = "my override"
-print(msg.field15.field15)
+--print(msg.field15.field15)
+--msg.field15.field15 = "my override"
+--print(msg.field15.field15)
 -- print(msg.field1)
 -- print(msg.field1)
 -- msg.field1 = "YEAH BABY!"
diff --git a/src/upb.c b/src/upb.c
index 897ca4e..6bb1ccc 100644
--- a/src/upb.c
+++ b/src/upb.c
@@ -33,7 +33,7 @@ const upb_type_info upb_types[] = {
   TYPE_INFO(UPB_WIRE_TYPE_DELIMITED,   void*,     1, MESSAGE)   // MESSAGE
   TYPE_INFO(UPB_WIRE_TYPE_DELIMITED,   void*,     1, STRING)    // BYTES
   TYPE_INFO(UPB_WIRE_TYPE_VARINT,      uint32_t,  1, UINT32)    // UINT32
-  TYPE_INFO(UPB_WIRE_TYPE_VARINT,      uint32_t,  1, ENUM)      // ENUM
+  TYPE_INFO(UPB_WIRE_TYPE_VARINT,      uint32_t,  1, INT32)     // ENUM
   TYPE_INFO(UPB_WIRE_TYPE_32BIT,       int32_t,   1, INT32)     // SFIXED32
   TYPE_INFO(UPB_WIRE_TYPE_64BIT,       int64_t,   1, INT64)     // SFIXED64
   TYPE_INFO(UPB_WIRE_TYPE_VARINT,      int32_t,   1, INT32)     // SINT32
diff --git a/src/upb.h b/src/upb.h
index fe576eb..c8d707d 100644
--- a/src/upb.h
+++ b/src/upb.h
@@ -200,7 +200,6 @@ typedef struct {
 UPB_VALUE_ACCESSORS(double, _double, double, UPB_TYPE(DOUBLE));
 UPB_VALUE_ACCESSORS(float, _float, float, UPB_TYPE(FLOAT));
 UPB_VALUE_ACCESSORS(int32, int32, int32_t, UPB_TYPE(INT32));
-UPB_VALUE_ACCESSORS(enumval, int32, int32_t, UPB_TYPE(ENUM));
 UPB_VALUE_ACCESSORS(int64, int64, int64_t, UPB_TYPE(INT64));
 UPB_VALUE_ACCESSORS(uint32, uint32, uint32_t, UPB_TYPE(UINT32));
 UPB_VALUE_ACCESSORS(uint64, uint64, uint64_t, UPB_TYPE(UINT64));
diff --git a/src/upb_def.c b/src/upb_def.c
index d77e29a..b12ed14 100644
--- a/src/upb_def.c
+++ b/src/upb_def.c
@@ -495,9 +495,14 @@ static upb_flow_t upb_enumdef_EnumValueDescriptorProto_endmsg(void *_b) {
     upb_seterr(&b->status, UPB_ERROR, "Enum value missing name or number.");
     return UPB_BREAK;
   }
+  upb_enumdef *e = upb_downcast_enumdef(upb_defbuilder_last(b));
+  if (upb_inttable_count(&e->iton) == 0) {
+    // The default value of an enum (in the absence of an explicit default) is
+    // its first listed value.
+    e->default_value = b->number;
+  }
   upb_ntoi_ent ntoi_ent = {{b->name, 0}, b->number};
   upb_iton_ent iton_ent = {0, b->name};
-  upb_enumdef *e = upb_downcast_enumdef(upb_defbuilder_last(b));
   upb_strtable_insert(&e->ntoi, &ntoi_ent.e);
   upb_inttable_insert(&e->iton, b->number, &iton_ent);
   // We don't unref "name" because we pass our ref to the iton entry of the
@@ -530,8 +535,16 @@ static upb_flow_t upb_enumdef_EnumDescriptorProto_startmsg(void *_b) {
 }
 
 static upb_flow_t upb_enumdef_EnumDescriptorProto_endmsg(void *_b) {
-  (void)_b;
-  assert(upb_defbuilder_last((upb_defbuilder*)_b)->fqname != NULL);
+  upb_defbuilder *b = _b;
+  upb_enumdef *e = upb_downcast_enumdef(upb_defbuilder_last(b));
+  if (upb_defbuilder_last((upb_defbuilder*)_b)->fqname == NULL) {
+    upb_seterr(&b->status, UPB_ERROR, "Enum had no name.");
+    return UPB_BREAK;
+  }
+  if (upb_inttable_count(&e->iton) == 0) {
+    upb_seterr(&b->status, UPB_ERROR, "Enum had no values.");
+    return UPB_BREAK;
+  }
   return UPB_CONTINUE;
 }
 
@@ -593,11 +606,18 @@ upb_string *upb_enumdef_iton(upb_enumdef *def, upb_enumval_t num) {
   return e ? e->string : NULL;
 }
 
+bool upb_enumdef_ntoi(upb_enumdef *def, upb_string *name, upb_enumval_t *num) {
+  upb_ntoi_ent *e = (upb_ntoi_ent*)upb_strtable_lookup(&def->ntoi, name);
+  if (!e) return false;
+  *num = e->value;
+  return true;
+}
+
 
 /* upb_fielddef ***************************************************************/
 
 static void upb_fielddef_free(upb_fielddef *f) {
-  if (upb_isstring(f) || f->type == UPB_TYPE(ENUM)) {
+  if (upb_isstring(f)) {
     upb_string_unref(upb_value_getstr(f->default_value));
   } else if (upb_issubmsg(f)) {
     upb_msg *m = upb_value_getmsg(f->default_value);
@@ -617,6 +637,36 @@ static void upb_fielddef_free(upb_fielddef *f) {
   free(f);
 }
 
+static bool upb_fielddef_resolve(upb_fielddef *f, upb_def *def, upb_status *s) {
+  if(f->owned) upb_def_unref(f->def);
+  f->def = def;
+  // We will later make the ref unowned if it is a part of a cycle.
+  f->owned = true;
+  upb_def_ref(def);
+  if (upb_issubmsg(f)) {
+    upb_msgdef *md = upb_downcast_msgdef(def);
+    upb_value_setmsg(&f->default_value, upb_msg_getref(md->default_message));
+  } else if (f->type == UPB_TYPE(ENUM)) {
+    upb_string *str = upb_value_getstr(f->default_value);
+    assert(str);  // Should point to either a real default or the empty string.
+    upb_enumdef *e = upb_downcast_enumdef(f->def);
+    upb_enumval_t val;
+    if (str == upb_emptystring()) {
+      upb_value_setint32(&f->default_value, e->default_value);
+    } else {
+      bool success = upb_enumdef_ntoi(e, str, &val);
+      upb_string_unref(str);
+      if (!success) {
+        upb_seterr(s, UPB_ERROR, "Default enum value (" UPB_STRFMT ") is not a "
+                   "member of the enum", UPB_STRARG(str));
+        return false;
+      }
+      upb_value_setint32(&f->default_value, val);
+    }
+  }
+  return true;
+}
+
 static upb_flow_t upb_fielddef_startmsg(void *_b) {
   upb_defbuilder *b = _b;
   upb_fielddef *f = malloc(sizeof(*f));
@@ -645,10 +695,7 @@ static bool upb_fielddef_setdefault(upb_string *dstr, upb_value *d, int type) {
   } else if (type == UPB_TYPE(MESSAGE) || type == UPB_TYPE(GROUP)) {
     // We don't expect to get a default value.
     upb_string_unref(dstr);
-    if (dstr != NULL) {
-      printf("Returning false because I got a default string for a message!\n");
-      success = false;
-    }
+    if (dstr != NULL) success = false;
   } else {
     // The strto* functions need the string to be NULL-terminated.
     char *strz = upb_string_isempty(dstr) ? NULL : upb_string_newcstr(dstr);
@@ -724,9 +771,6 @@ static bool upb_fielddef_setdefault(upb_string *dstr, upb_value *d, int type) {
           success = false;
         break;
     }
-    if (!success) {
-      printf("Returning false on the int conversion path, was trying to convert: %s, type=%d\n", strz, type);
-    }
     free(strz);
   }
   return success;
@@ -761,10 +805,10 @@ static upb_flow_t upb_fielddef_value(void *_b, upb_fielddef *f, upb_value val) {
   upb_defbuilder *b = _b;
   switch(f->number) {
     case GOOGLE_PROTOBUF_FIELDDESCRIPTORPROTO_TYPE_FIELDNUM:
-      b->f->type = upb_value_getenumval(val);
+      b->f->type = upb_value_getint32(val);
       break;
     case GOOGLE_PROTOBUF_FIELDDESCRIPTORPROTO_LABEL_FIELDNUM:
-      b->f->label = upb_value_getenumval(val);
+      b->f->label = upb_value_getint32(val);
       break;
     case GOOGLE_PROTOBUF_FIELDDESCRIPTORPROTO_NUMBER_FIELDNUM:
       b->f->number = upb_value_getint32(val);
@@ -966,19 +1010,6 @@ static void upb_msgdef_free(upb_msgdef *m)
   free(m);
 }
 
-static void upb_msgdef_resolve(upb_msgdef *m, upb_fielddef *f, upb_def *def) {
-  (void)m;
-  if(f->owned) upb_def_unref(f->def);
-  f->def = def;
-  // We will later make the ref unowned if it is a part of a cycle.
-  f->owned = true;
-  upb_def_ref(def);
-  if (upb_issubmsg(f)) {
-    upb_msgdef *md = upb_downcast_msgdef(def);
-    upb_value_setmsg(&f->default_value, upb_msg_getref(md->default_message));
-  }
-}
-
 upb_msg_iter upb_msg_begin(upb_msgdef *m) {
   return upb_inttable_begin(&m->itof);
 }
@@ -1133,7 +1164,7 @@ bool upb_resolverefs(upb_strtable *tmptab, upb_strtable *symtab,
         upb_seterr(status, UPB_ERROR, "Unexpected type");
         return false;
       }
-      upb_msgdef_resolve(m, f, found->def);
+      if (!upb_fielddef_resolve(f, found->def, status)) return false;
     }
   }
 
diff --git a/src/upb_def.h b/src/upb_def.h
index 3f79895..4a2bcbd 100644
--- a/src/upb_def.h
+++ b/src/upb_def.h
@@ -227,10 +227,13 @@ INLINE upb_fielddef *upb_msg_iter_field(upb_msg_iter iter) {
 
 /* upb_enumdef ****************************************************************/
 
+typedef int32_t upb_enumval_t;
+
 typedef struct _upb_enumdef {
   upb_def base;
   upb_strtable ntoi;
   upb_inttable iton;
+  upb_enumval_t default_value;  // The first value listed in the enum.
 } upb_enumdef;
 
 typedef struct {
@@ -243,8 +246,6 @@ typedef struct {
   upb_string *string;
 } upb_iton_ent;
 
-typedef int32_t upb_enumval_t;
-
 // Lookups from name to integer and vice-versa.
 bool upb_enumdef_ntoi(upb_enumdef *e, upb_string *name, upb_enumval_t *num);
 // Caller does not own a ref on the returned string.
diff --git a/src/upb_msg.c b/src/upb_msg.c
index 27170f5..951b5e0 100644
--- a/src/upb_msg.c
+++ b/src/upb_msg.c
@@ -146,7 +146,7 @@ INLINE void upb_msg_sethas(upb_msg *msg, upb_fielddef *f) {
 }
 
 void upb_msg_set(upb_msg *msg, upb_fielddef *f, upb_value val) {
-  assert(val.type == upb_field_valuetype(f));
+  assert(val.type == upb_types[upb_field_valuetype(f)].inmemory_type);
   upb_valueptr ptr = _upb_msg_getptr(msg, f);
   if (upb_field_ismm(f)) {
     // Unref any previous value we may have had there.