Roll forward Ruby upb changes now that protobuf Ruby build is fixed (#5866)

* Rolled forward again with "Updated upb from defcleanup branch..."

Revert "Revert "Updated upb from defcleanup branch and modified Ruby to use it (#5539)" (#5848)"

This reverts commit 1568deab.

* A few more merge fixes.

* Updated for defcleanup2 branch.

* Fixed upb to define upb_decode().

* Fixed names of nested messages.

* Revert submodule.

* Set -std=gnu90 and fixed warnings/errors.

Some of our Kokoro tests seem to run with this level of warnings,
and the source strives to be gnu90 compatible.  Enforcing it for
every build removes the possibility of some errors showing up in
Kokoro/Travis tests only.

* Fixed remaining warnings with gnu90 mode.

I tried to match warning flags with what Ruby appears to do
in our Kokoro tests.

* Initialize values registered by rb_gc_register_address().

* Fixed subtle GC bug.

We need to initialize this marked value before creating the instance.

* Truly fix the GC bug.

* Updated upb for mktime() fix.

* Removed XOPEN_SOURCE as we are not using strptime().

* Removed fixed tests from the conformance failure list for Ruby.

* Fixed memory error related to oneof def names.

* Picked up new upb changes re: JSON printing.

* Uncomment concurrent decoding test.

ruby/compatibility_tests/v3.0.0/tests/basic.rb

@@ -67,7 +67,6 @@ module BasicTest
     add_message "BadFieldNames" do
       optional :dup, :int32, 1
       optional :class, :int32, 2
-      optional :"a.b", :int32, 3
     end
 
     add_message "MapMessage" do
@@ -1067,8 +1066,6 @@ module BasicTest
       assert m['class'] == 2
       m['dup'] = 3
       assert m['dup'] == 3
-      m['a.b'] = 4
-      assert m['a.b'] == 4
     end
 
     def test_int_ranges

ruby/ext/google/protobuf_c/extconf.rb

@@ -3,11 +3,9 @@
 require 'mkmf'
 
 if RUBY_PLATFORM =~ /darwin/ || RUBY_PLATFORM =~ /linux/
-  # XOPEN_SOURCE needed for strptime:
-  # https://stackoverflow.com/questions/35234152/strptime-giving-implicit-declaration-and-undefined-reference
-  $CFLAGS += " -std=c99 -O3 -DNDEBUG -D_XOPEN_SOURCE=700"
+  $CFLAGS += " -std=gnu90 -O3 -DNDEBUG -Wall -Wdeclaration-after-statement -Wsign-compare"
 else
-  $CFLAGS += " -std=c99 -O3 -DNDEBUG"
+  $CFLAGS += " -std=gnu90 -O3 -DNDEBUG"
 end
 
 

ruby/ext/google/protobuf_c/map.c

@@ -389,10 +389,7 @@ VALUE Map_index(VALUE _self, VALUE key) {
  * was just inserted.
  */
 VALUE Map_index_set(VALUE _self, VALUE key, VALUE value) {
-  rb_check_frozen(_self);
-
   Map* self = ruby_to_Map(_self);
-
   char keybuf[TABLE_KEY_BUF_LENGTH];
   const char* keyval = NULL;
   size_t length = 0;
@@ -400,6 +397,8 @@ VALUE Map_index_set(VALUE _self, VALUE key, VALUE value) {
   void* mem;
   key = table_key(self, key, keybuf, &keyval, &length);
 
+  rb_check_frozen(_self);
+
   if (TYPE(value) == T_HASH) {
     VALUE args[1] = { value };
     value = rb_class_new_instance(1, args, self->value_type_class);
@@ -448,16 +447,15 @@ VALUE Map_has_key(VALUE _self, VALUE key) {
  * nil if none was present. Throws an exception if the key is of the wrong type.
  */
 VALUE Map_delete(VALUE _self, VALUE key) {
-  rb_check_frozen(_self);
-
   Map* self = ruby_to_Map(_self);
-
   char keybuf[TABLE_KEY_BUF_LENGTH];
   const char* keyval = NULL;
   size_t length = 0;
   upb_value v;
   key = table_key(self, key, keybuf, &keyval, &length);
 
+  rb_check_frozen(_self);
+
   if (upb_strtable_remove2(&self->table, keyval, length, &v)) {
     void* mem = value_memory(&v);
     return native_slot_get(self->value_type, self->value_type_class, mem);
@@ -473,10 +471,10 @@ VALUE Map_delete(VALUE _self, VALUE key) {
  * Removes all entries from the map.
  */
 VALUE Map_clear(VALUE _self) {
-  rb_check_frozen(_self);
-
   Map* self = ruby_to_Map(_self);
 
+  rb_check_frozen(_self);
+
   // Uninit and reinit the table -- this is faster than iterating and doing a
   // delete-lookup on each key.
   upb_strtable_uninit(&self->table);

ruby/ext/google/protobuf_c/protobuf.c

@@ -30,26 +30,10 @@
 
 #include "protobuf.h"
 
-// -----------------------------------------------------------------------------
-// Global map from upb {msg,enum}defs to wrapper Descriptor/EnumDescriptor
-// instances.
-// -----------------------------------------------------------------------------
-
-// This is a hash table from def objects (encoded by converting pointers to
-// Ruby integers) to MessageDef/EnumDef instances (as Ruby values).
-VALUE upb_def_to_ruby_obj_map;
-
 VALUE cError;
 VALUE cParseError;
 VALUE cTypeError;
-
-void add_def_obj(const void* def, VALUE value) {
-  rb_hash_aset(upb_def_to_ruby_obj_map, ULL2NUM((intptr_t)def), value);
-}
-
-VALUE get_def_obj(const void* def) {
-  return rb_hash_aref(upb_def_to_ruby_obj_map, ULL2NUM((intptr_t)def));
-}
+VALUE c_only_cookie = Qnil;
 
 static VALUE cached_empty_string = Qnil;
 static VALUE cached_empty_bytes = Qnil;
@@ -142,8 +126,8 @@ void Init_protobuf_c() {
   kRubyStringASCIIEncoding = rb_usascii_encoding();
   kRubyString8bitEncoding = rb_ascii8bit_encoding();
 
-  rb_gc_register_address(&upb_def_to_ruby_obj_map);
-  upb_def_to_ruby_obj_map = rb_hash_new();
+  rb_gc_register_address(&c_only_cookie);
+  c_only_cookie = rb_class_new_instance(0, NULL, rb_cObject);
 
   rb_gc_register_address(&cached_empty_string);
   rb_gc_register_address(&cached_empty_bytes);

ruby/ext/google/protobuf_c/repeated_field.c

@@ -64,10 +64,11 @@ VALUE RepeatedField_subarray(VALUE _self, long beg, long len) {
   int element_size = native_slot_size(self->field_type);
   upb_fieldtype_t field_type = self->field_type;
   VALUE field_type_class = self->field_type_class;
-
   size_t off = beg * element_size;
   VALUE ary = rb_ary_new2(len);
-  for (int i = beg; i < beg + len; i++, off += element_size) {
+  int i;
+
+  for (i = beg; i < beg + len; i++, off += element_size) {
     void* mem = ((uint8_t *)self->elements) + off;
     VALUE elem = native_slot_get(field_type, field_type_class, mem);
     rb_ary_push(ary, elem);
@@ -88,9 +89,10 @@ VALUE RepeatedField_each(VALUE _self) {
   upb_fieldtype_t field_type = self->field_type;
   VALUE field_type_class = self->field_type_class;
   int element_size = native_slot_size(field_type);
-
   size_t off = 0;
-  for (int i = 0; i < self->size; i++, off += element_size) {
+  int i;
+
+  for (i = 0; i < self->size; i++, off += element_size) {
     void* memory = (void *) (((uint8_t *)self->elements) + off);
     VALUE val = native_slot_get(field_type, field_type_class, memory);
     rb_yield(val);
@@ -169,8 +171,10 @@ VALUE RepeatedField_index_set(VALUE _self, VALUE _index, VALUE val) {
   if (index >= self->size) {
     upb_fieldtype_t field_type = self->field_type;
     int element_size = native_slot_size(field_type);
+    int i;
+
     RepeatedField_reserve(self, index + 1);
-    for (int i = self->size; i <= index; i++) {
+    for (i = self->size; i <= index; i++) {
       void* elem = RepeatedField_memoryat(self, i, element_size);
       native_slot_init(field_type, elem);
     }
@@ -224,7 +228,8 @@ VALUE RepeatedField_push(VALUE _self, VALUE val) {
 }
 
 VALUE RepeatedField_push_vararg(VALUE _self, VALUE args) {
-  for (int i = 0; i < RARRAY_LEN(args); i++) {
+  int i;
+  for (i = 0; i < RARRAY_LEN(args); i++) {
     RepeatedField_push(_self, rb_ary_entry(args, i));
   }
   return _self;
@@ -285,9 +290,11 @@ VALUE RepeatedField_pop_one(VALUE _self) {
  */
 VALUE RepeatedField_replace(VALUE _self, VALUE list) {
   RepeatedField* self = ruby_to_RepeatedField(_self);
+  int i;
+
   Check_Type(list, T_ARRAY);
   self->size = 0;
-  for (int i = 0; i < RARRAY_LEN(list); i++) {
+  for (i = 0; i < RARRAY_LEN(list); i++) {
     RepeatedField_push(_self, rb_ary_entry(list, i));
   }
   return list;
@@ -344,8 +351,10 @@ VALUE RepeatedField_dup(VALUE _self) {
   upb_fieldtype_t field_type = self->field_type;
   size_t elem_size = native_slot_size(field_type);
   size_t off = 0;
+  int i;
+
   RepeatedField_reserve(new_rptfield_self, self->size);
-  for (int i = 0; i < self->size; i++, off += elem_size) {
+  for (i = 0; i < self->size; i++, off += elem_size) {
     void* to_mem = (uint8_t *)new_rptfield_self->elements + off;
     void* from_mem = (uint8_t *)self->elements + off;
     native_slot_dup(field_type, to_mem, from_mem);
@@ -363,8 +372,10 @@ VALUE RepeatedField_deep_copy(VALUE _self) {
   upb_fieldtype_t field_type = self->field_type;
   size_t elem_size = native_slot_size(field_type);
   size_t off = 0;
+  int i;
+
   RepeatedField_reserve(new_rptfield_self, self->size);
-  for (int i = 0; i < self->size; i++, off += elem_size) {
+  for (i = 0; i < self->size; i++, off += elem_size) {
     void* to_mem = (uint8_t *)new_rptfield_self->elements + off;
     void* from_mem = (uint8_t *)self->elements + off;
     native_slot_deep_copy(field_type, to_mem, from_mem);
@@ -384,11 +395,12 @@ VALUE RepeatedField_deep_copy(VALUE _self) {
 VALUE RepeatedField_to_ary(VALUE _self) {
   RepeatedField* self = ruby_to_RepeatedField(_self);
   upb_fieldtype_t field_type = self->field_type;
-
   size_t elem_size = native_slot_size(field_type);
   size_t off = 0;
   VALUE ary = rb_ary_new2(self->size);
-  for (int i = 0; i < self->size; i++, off += elem_size) {
+  int i;
+
+  for (i = 0; i < self->size; i++, off += elem_size) {
     void* mem = ((uint8_t *)self->elements) + off;
     VALUE elem = native_slot_get(field_type, self->field_type_class, mem);
     rb_ary_push(ary, elem);
@@ -434,7 +446,9 @@ VALUE RepeatedField_eq(VALUE _self, VALUE _other) {
     upb_fieldtype_t field_type = self->field_type;
     size_t elem_size = native_slot_size(field_type);
     size_t off = 0;
-    for (int i = 0; i < self->size; i++, off += elem_size) {
+    int i;
+
+    for (i = 0; i < self->size; i++, off += elem_size) {
       void* self_mem = ((uint8_t *)self->elements) + off;
       void* other_mem = ((uint8_t *)other->elements) + off;
       if (!native_slot_eq(field_type, self_mem, other_mem)) {
@@ -459,7 +473,9 @@ VALUE RepeatedField_hash(VALUE _self) {
   VALUE field_type_class = self->field_type_class;
   size_t elem_size = native_slot_size(field_type);
   size_t off = 0;
-  for (int i = 0; i < self->size; i++, off += elem_size) {
+  int i;
+
+  for (i = 0; i < self->size; i++, off += elem_size) {
     void* mem = ((uint8_t *)self->elements) + off;
     VALUE elem = native_slot_get(field_type, field_type_class, mem);
     h = rb_hash_uint(h, NUM2LONG(rb_funcall(elem, hash_sym, 0)));
@@ -481,7 +497,8 @@ VALUE RepeatedField_plus(VALUE _self, VALUE list) {
   VALUE dupped = RepeatedField_dup(_self);
 
   if (TYPE(list) == T_ARRAY) {
-    for (int i = 0; i < RARRAY_LEN(list); i++) {
+    int i;
+    for (i = 0; i < RARRAY_LEN(list); i++) {
       VALUE elem = rb_ary_entry(list, i);
       RepeatedField_push(dupped, elem);
     }
@@ -489,12 +506,14 @@ VALUE RepeatedField_plus(VALUE _self, VALUE list) {
              RTYPEDDATA_TYPE(list) == &RepeatedField_type) {
     RepeatedField* self = ruby_to_RepeatedField(_self);
     RepeatedField* list_rptfield = ruby_to_RepeatedField(list);
+    int i;
+
     if (self->field_type != list_rptfield->field_type ||
         self->field_type_class != list_rptfield->field_type_class) {
       rb_raise(rb_eArgError,
                "Attempt to append RepeatedField with different element type.");
     }
-    for (int i = 0; i < list_rptfield->size; i++) {
+    for (i = 0; i < list_rptfield->size; i++) {
       void* mem = RepeatedField_index_native(list, i);
       RepeatedField_push_native(dupped, mem);
     }
@@ -512,8 +531,10 @@ VALUE RepeatedField_plus(VALUE _self, VALUE list) {
  * concats the passed in array to self.  Returns a Ruby array.
  */
 VALUE RepeatedField_concat(VALUE _self, VALUE list) {
+  int i;
+
   Check_Type(list, T_ARRAY);
-  for (int i = 0; i < RARRAY_LEN(list); i++) {
+  for (i = 0; i < RARRAY_LEN(list); i++) {
     RepeatedField_push(_self, rb_ary_entry(list, i));
   }
   return _self;
@@ -574,10 +595,12 @@ void RepeatedField_init_args(int argc, VALUE* argv,
   }
 
   if (ary != Qnil) {
+    int i;
+
     if (!RB_TYPE_P(ary, T_ARRAY)) {
       rb_raise(rb_eArgError, "Expected array as initialize argument");
     }
-    for (int i = 0; i < RARRAY_LEN(ary); i++) {
+    for (i = 0; i < RARRAY_LEN(ary); i++) {
       RepeatedField_push(_self, rb_ary_entry(ary, i));
     }
   }
@@ -589,8 +612,10 @@ void RepeatedField_mark(void* _self) {
   RepeatedField* self = (RepeatedField*)_self;
   upb_fieldtype_t field_type = self->field_type;
   int element_size = native_slot_size(field_type);
+  int i;
+
   rb_gc_mark(self->field_type_class);
-  for (int i = 0; i < self->size; i++) {
+  for (i = 0; i < self->size; i++) {
     void* memory = (((uint8_t *)self->elements) + i * element_size);
     native_slot_mark(self->field_type, memory);
   }

ruby/ext/google/protobuf_c/storage.c

@@ -181,32 +181,43 @@ void native_slot_set_value_and_case(const char* name,
         value = Qnil;
       } else if (CLASS_OF(value) != type_class) {
         // check for possible implicit conversions
-        VALUE converted_value = NULL;
-        char* field_type_name = rb_class2name(type_class);
+        VALUE converted_value = Qnil;
+        const char* field_type_name = rb_class2name(type_class);
 
         if (strcmp(field_type_name, "Google::Protobuf::Timestamp") == 0 &&
             rb_obj_is_kind_of(value, rb_cTime)) {
           // Time -> Google::Protobuf::Timestamp
           VALUE hash = rb_hash_new();
-          rb_hash_aset(hash, rb_str_new2("seconds"), rb_funcall(value, rb_intern("to_i"), 0));
-          rb_hash_aset(hash, rb_str_new2("nanos"), rb_funcall(value, rb_intern("nsec"), 0));
-          VALUE args[1] = { hash };
+          rb_hash_aset(hash, rb_str_new2("seconds"),
+                       rb_funcall(value, rb_intern("to_i"), 0));
+          rb_hash_aset(hash, rb_str_new2("nanos"),
+                       rb_funcall(value, rb_intern("nsec"), 0));
+          {
+            VALUE args[1] = {hash};
             converted_value = rb_class_new_instance(1, args, type_class);
+          }
         } else if (strcmp(field_type_name, "Google::Protobuf::Duration") == 0 &&
                    rb_obj_is_kind_of(value, rb_cNumeric)) {
           // Numeric -> Google::Protobuf::Duration
           VALUE hash = rb_hash_new();
-          rb_hash_aset(hash, rb_str_new2("seconds"), rb_funcall(value, rb_intern("to_i"), 0));
-          VALUE n_value = rb_funcall(value, rb_intern("remainder"), 1, INT2NUM(1));
-          n_value = rb_funcall(n_value, rb_intern("*"), 1, INT2NUM(1000000000));
+          rb_hash_aset(hash, rb_str_new2("seconds"),
+                       rb_funcall(value, rb_intern("to_i"), 0));
+          {
+            VALUE n_value =
+                rb_funcall(value, rb_intern("remainder"), 1, INT2NUM(1));
+            n_value =
+                rb_funcall(n_value, rb_intern("*"), 1, INT2NUM(1000000000));
             n_value = rb_funcall(n_value, rb_intern("round"), 0);
             rb_hash_aset(hash, rb_str_new2("nanos"), n_value);
+          }
+          {
             VALUE args[1] = { hash };
             converted_value = rb_class_new_instance(1, args, type_class);
           }
+        }
 
         // raise if no suitable conversaion could be found
-        if (converted_value == NULL) {
+        if (converted_value == Qnil) {
           rb_raise(cTypeError,
                    "Invalid type %s to assign to submessage field '%s'.",
                   rb_class2name(CLASS_OF(value)), name);
@@ -462,16 +473,18 @@ static size_t align_up_to(size_t offset, size_t granularity) {
   return (offset + granularity - 1) & ~(granularity - 1);
 }
 
-MessageLayout* create_layout(const upb_msgdef* msgdef) {
+MessageLayout* create_layout(const Descriptor* desc) {
+  const upb_msgdef *msgdef = desc->msgdef;
   MessageLayout* layout = ALLOC(MessageLayout);
   int nfields = upb_msgdef_numfields(msgdef);
   upb_msg_field_iter it;
   upb_msg_oneof_iter oit;
   size_t off = 0;
+  size_t hasbit = 0;
 
+  layout->desc = desc;
   layout->fields = ALLOC_N(MessageField, nfields);
 
-  size_t hasbit = 0;
   for (upb_msg_field_begin(&it, msgdef);
        !upb_msg_field_done(&it);
        upb_msg_field_next(&it)) {
@@ -569,28 +582,25 @@ MessageLayout* create_layout(const upb_msgdef* msgdef) {
   }
 
   layout->size = off;
-
   layout->msgdef = msgdef;
-  upb_msgdef_ref(layout->msgdef, &layout->msgdef);
 
   return layout;
 }
 
 void free_layout(MessageLayout* layout) {
   xfree(layout->fields);
-  upb_msgdef_unref(layout->msgdef, &layout->msgdef);
   xfree(layout);
 }
 
-VALUE field_type_class(const upb_fielddef* field) {
+VALUE field_type_class(const MessageLayout* layout, const upb_fielddef* field) {
   VALUE type_class = Qnil;
   if (upb_fielddef_type(field) == UPB_TYPE_MESSAGE) {
-    VALUE submsgdesc =
-        get_def_obj(upb_fielddef_subdef(field));
+    VALUE submsgdesc = get_msgdef_obj(layout->desc->descriptor_pool,
+                                      upb_fielddef_msgsubdef(field));
     type_class = Descriptor_msgclass(submsgdesc);
   } else if (upb_fielddef_type(field) == UPB_TYPE_ENUM) {
-    VALUE subenumdesc =
-        get_def_obj(upb_fielddef_subdef(field));
+    VALUE subenumdesc = get_enumdef_obj(layout->desc->descriptor_pool,
+                                        upb_fielddef_enumsubdef(field));
     type_class = EnumDescriptor_enummodule(subenumdesc);
   }
   return type_class;
@@ -664,7 +674,7 @@ void layout_clear(MessageLayout* layout,
 
     const upb_fielddef* key_field = map_field_key(field);
     const upb_fielddef* value_field = map_field_value(field);
-    VALUE type_class = field_type_class(value_field);
+    VALUE type_class = field_type_class(layout, value_field);
 
     if (type_class != Qnil) {
       VALUE args[3] = {
@@ -685,7 +695,7 @@ void layout_clear(MessageLayout* layout,
   } else if (upb_fielddef_label(field) == UPB_LABEL_REPEATED) {
     VALUE ary = Qnil;
 
-    VALUE type_class = field_type_class(field);
+    VALUE type_class = field_type_class(layout, field);
 
     if (type_class != Qnil) {
       VALUE args[2] = {
@@ -700,9 +710,9 @@ void layout_clear(MessageLayout* layout,
 
     DEREF(memory, VALUE) = ary;
   } else {
-    native_slot_set(upb_fielddef_name(field),
-                    upb_fielddef_type(field), field_type_class(field),
-                    memory, layout_get_default(field));
+    native_slot_set(upb_fielddef_name(field), upb_fielddef_type(field),
+                    field_type_class(layout, field), memory,
+                    layout_get_default(field));
   }
 }
 
@@ -756,20 +766,19 @@ VALUE layout_get(MessageLayout* layout,
       return layout_get_default(field);
     }
     return native_slot_get(upb_fielddef_type(field),
-                           field_type_class(field),
-                           memory);
+                           field_type_class(layout, field), memory);
   } else if (upb_fielddef_label(field) == UPB_LABEL_REPEATED) {
     return *((VALUE *)memory);
   } else if (!field_set) {
     return layout_get_default(field);
   } else {
     return native_slot_get(upb_fielddef_type(field),
-                           field_type_class(field),
-                           memory);
+                           field_type_class(layout, field), memory);
   }
 }
 
-static void check_repeated_field_type(VALUE val, const upb_fielddef* field) {
+static void check_repeated_field_type(const MessageLayout* layout, VALUE val,
+                                      const upb_fielddef* field) {
   RepeatedField* self;
   assert(upb_fielddef_label(field) == UPB_LABEL_REPEATED);
 
@@ -783,25 +792,13 @@ static void check_repeated_field_type(VALUE val, const upb_fielddef* field) {
     rb_raise(cTypeError, "Repeated field array has wrong element type");
   }
 
-  if (self->field_type == UPB_TYPE_MESSAGE) {
-    if (self->field_type_class !=
-        Descriptor_msgclass(get_def_obj(upb_fielddef_subdef(field)))) {
-      rb_raise(cTypeError,
-               "Repeated field array has wrong message class");
-    }
-  }
-
-
-  if (self->field_type == UPB_TYPE_ENUM) {
-    if (self->field_type_class !=
-        EnumDescriptor_enummodule(get_def_obj(upb_fielddef_subdef(field)))) {
-      rb_raise(cTypeError,
-               "Repeated field array has wrong enum class");
-    }
+  if (self->field_type_class != field_type_class(layout, field)) {
+    rb_raise(cTypeError, "Repeated field array has wrong message/enum class");
   }
 }
 
-static void check_map_field_type(VALUE val, const upb_fielddef* field) {
+static void check_map_field_type(const MessageLayout* layout, VALUE val,
+                                 const upb_fielddef* field) {
   const upb_fielddef* key_field = map_field_key(field);
   const upb_fielddef* value_field = map_field_value(field);
   Map* self;
@@ -818,17 +815,11 @@ static void check_map_field_type(VALUE val, const upb_fielddef* field) {
   if (self->value_type != upb_fielddef_type(value_field)) {
     rb_raise(cTypeError, "Map value type does not match field's value type");
   }
-  if (upb_fielddef_type(value_field) == UPB_TYPE_MESSAGE ||
-      upb_fielddef_type(value_field) == UPB_TYPE_ENUM) {
-    if (self->value_type_class !=
-        get_def_obj(upb_fielddef_subdef(value_field))) {
-      rb_raise(cTypeError,
-               "Map value type has wrong message/enum class");
-    }
+  if (self->value_type_class != field_type_class(layout, value_field)) {
+    rb_raise(cTypeError, "Map value type has wrong message/enum class");
   }
 }
 
-
 void layout_set(MessageLayout* layout,
                 void* storage,
                 const upb_fielddef* field,
@@ -856,20 +847,19 @@ void layout_set(MessageLayout* layout,
       // and case number are altered atomically (w.r.t. the Ruby VM).
       native_slot_set_value_and_case(
           upb_fielddef_name(field),
-          upb_fielddef_type(field), field_type_class(field),
+          upb_fielddef_type(field), field_type_class(layout, field),
           memory, val,
           oneof_case, upb_fielddef_number(field));
     }
   } else if (is_map_field(field)) {
-    check_map_field_type(val, field);
+    check_map_field_type(layout, val, field);
     DEREF(memory, VALUE) = val;
   } else if (upb_fielddef_label(field) == UPB_LABEL_REPEATED) {
-    check_repeated_field_type(val, field);
+    check_repeated_field_type(layout, val, field);
     DEREF(memory, VALUE) = val;
   } else {
-    native_slot_set(upb_fielddef_name(field),
-                    upb_fielddef_type(field), field_type_class(field),
-                    memory, val);
+    native_slot_set(upb_fielddef_name(field), upb_fielddef_type(field),
+                    field_type_class(layout, field), memory, val);
   }
 
   if (layout->fields[upb_fielddef_index(field)].hasbit !=

ruby/lib/google/protobuf.rb

@@ -50,6 +50,72 @@ else
   rescue LoadError
     require 'google/protobuf_c'
   end
+
+  module Google
+    module Protobuf
+      module Internal
+        def self.infer_package(names)
+          # Package is longest common prefix ending in '.', if any.
+          min, max = names.minmax
+          last_common_dot = nil
+          min.size.times { |i|
+            if min[i] != max[i] then break end
+            if min[i] == ?. then last_common_dot = i end
+          }
+          if last_common_dot
+            return min.slice(0, last_common_dot)
+          end
+        end
+
+        class NestingBuilder
+          def initialize(msg_names, enum_names)
+            @to_pos = {nil=>nil}
+            @msg_children = Hash.new { |hash, key| hash[key] = [] }
+            @enum_children = Hash.new { |hash, key| hash[key] = [] }
+
+            msg_names.each_with_index { |name, idx| @to_pos[name] = idx }
+            enum_names.each_with_index { |name, idx| @to_pos[name] = idx }
+
+            msg_names.each { |name| @msg_children[parent(name)] << name }
+            enum_names.each { |name| @enum_children[parent(name)] << name }
+          end
+
+          def build(package)
+            return build_msg(package)
+          end
+
+          private
+          def build_msg(msg)
+            return {
+              :pos => @to_pos[msg],
+              :msgs => @msg_children[msg].map { |child| build_msg(child) },
+              :enums => @enum_children[msg].map { |child| @to_pos[child] },
+            }
+          end
+
+          private
+          def parent(name)
+            idx = name.rindex(?.)
+            if idx
+              return name.slice(0, idx)
+            else
+              return nil
+            end
+          end
+        end
+
+        def self.fixup_descriptor(package, msg_names, enum_names)
+          if package.nil?
+            package = self.infer_package(msg_names + enum_names)
+          end
+
+          nesting = NestingBuilder.new(msg_names, enum_names).build(package)
+
+          return package, nesting
+        end
+      end
+    end
+  end
 end
 
 require 'google/protobuf/repeated_field'

ruby/tests/basic.rb

@@ -17,7 +17,6 @@ module BasicTest
     add_message "BadFieldNames" do
       optional :dup, :int32, 1
       optional :class, :int32, 2
-      optional :"a.b", :int32, 3
     end
   end
 
@@ -285,6 +284,14 @@ module BasicTest
       assert_match(/No such field: not_in_message/, e.message)
     end
 
+    #def test_json_quoted_string
+    #  m = TestMessage.decode_json(%q(
+    #    "optionalInt64": "1",,
+    #  }))
+    #  puts(m)
+    #  assert_equal 1, m.optional_int32
+    #end
+
     def test_to_h
       m = TestMessage.new(:optional_bool => true, :optional_double => -10.100001, :optional_string => 'foo', :repeated_string => ['bar1', 'bar2'], :repeated_msg => [TestMessage2.new(:foo => 100)])
       expected_result = {
@@ -371,11 +378,6 @@ module BasicTest
       assert nil != file_descriptor
       assert_equal "tests/basic_test.proto", file_descriptor.name
       assert_equal :proto3, file_descriptor.syntax
-
-      file_descriptor = BadFieldNames.descriptor.file_descriptor
-      assert nil != file_descriptor
-      assert_equal nil, file_descriptor.name
-      assert_equal :proto3, file_descriptor.syntax
     end
 
     # Ruby 2.5 changed to raise FrozenError instead of RuntimeError

ruby/tests/basic_proto2.rb

@@ -18,7 +18,6 @@ module BasicTestProto2
       add_message "BadFieldNames" do
         optional :dup, :int32, 1
         optional :class, :int32, 2
-        optional :"a.b", :int32, 3
       end
     end
   end

ruby/tests/common_tests.rb

@@ -807,7 +807,7 @@ module CommonTests
                                                         proto_module::TestMessage2.new(:foo => 2)])
     data = proto_module::TestMessage.encode m
     m2 = proto_module::TestMessage.decode data
-    assert m == m2
+    assert_equal m, m2
 
     data = Google::Protobuf.encode m
     m2 = Google::Protobuf.decode(proto_module::TestMessage, data)
@@ -902,8 +902,6 @@ module CommonTests
     assert m['class'] == 2
     m['dup'] = 3
     assert m['dup'] == 3
-    m['a.b'] = 4
-    assert m['a.b'] == 4
   end
 
   def test_int_ranges
@@ -1084,9 +1082,7 @@ module CommonTests
 
     json_text = proto_module::TestMessage.encode_json(m)
     m2 = proto_module::TestMessage.decode_json(json_text)
-    puts m.inspect
-    puts m2.inspect
-    assert m == m2
+    assert_equal m, m2
 
     # Crash case from GitHub issue 283.
     bar = proto_module::Bar.new(msg: "bar")
@@ -1132,7 +1128,7 @@ module CommonTests
 
     actual = proto_module::TestMessage.encode_json(m, :emit_defaults => true)
 
-    assert JSON.parse(actual, :symbolize_names => true) == expected
+    assert_equal expected, JSON.parse(actual, :symbolize_names => true)
   end
 
   def test_json_emit_defaults_submsg
@@ -1167,7 +1163,7 @@ module CommonTests
 
     actual = proto_module::TestMessage.encode_json(m, :emit_defaults => true)
 
-    assert JSON.parse(actual, :symbolize_names => true) == expected
+    assert_equal expected, JSON.parse(actual, :symbolize_names => true)
   end
 
   def test_json_emit_defaults_repeated_submsg
@@ -1201,7 +1197,7 @@ module CommonTests
 
     actual = proto_module::TestMessage.encode_json(m, :emit_defaults => true)
 
-    assert JSON.parse(actual, :symbolize_names => true) == expected
+    assert_equal expected, JSON.parse(actual, :symbolize_names => true)
   end
 
   def value_from_ruby(value)