From 3a758132b402e2c4a346d1feb45d00300fed16e7 Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Wed, 23 Feb 2011 10:14:53 -0800 Subject: Added proper support for enum default values. --- Makefile | 1 - lang_ext/lua/test.lua | 8 ++--- src/upb.c | 2 +- src/upb.h | 1 - src/upb_def.c | 85 +++++++++++++++++++++++++++++++++++---------------- src/upb_def.h | 5 +-- src/upb_msg.c | 2 +- 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. -- cgit v1.2.3