Fixed the oneof case for lazy wrappers.

ruby/ext/google/protobuf_c/encode_decode.c

@@ -506,14 +506,22 @@ static void *oneofsubmsg_handler(void *closure,
   // indicating a VALUE is present and expect a valid VALUE. See comment in
   // layout_set() for more detail: basically, the change to the value and the
   // case must be atomic w.r.t. the Ruby VM.
-  DEREF(msg, oneofdata->case_ofs, uint32_t) =
-      oneofdata->oneof_case_num;
+  DEREF(msg, oneofdata->case_ofs, uint32_t) = oneofdata->oneof_case_num;
 
   submsg_rb = DEREF(msg, oneofdata->ofs, VALUE);
   TypedData_Get_Struct(submsg_rb, MessageHeader, &Message_type, submsg);
   return submsg;
 }
 
+static void* oneof_startwrapper(void* closure, const void* hd) {
+  char* msg = closure;
+  const oneof_handlerdata_t *oneofdata = hd;
+
+  DEREF(msg, oneofdata->case_ofs, uint32_t) = oneofdata->oneof_case_num;
+
+  return msg + oneofdata->ofs;
+}
+
 bool is_wrapper(const upb_msgdef* m) {
   switch (upb_msgdef_wellknowntype(m)) {
     case UPB_WELLKNOWN_DOUBLEVALUE:
@@ -805,7 +813,11 @@ static void add_handlers_for_oneof_field(upb_handlers *h,
       break;
     }
     case UPB_TYPE_MESSAGE: {
-      upb_handlers_setstartsubmsg(h, f, oneofsubmsg_handler, &attr);
+      if (is_wrapper(upb_fielddef_msgsubdef(f))) {
+        upb_handlers_setstartsubmsg(h, f, oneof_startwrapper, &attr);
+      } else {
+        upb_handlers_setstartsubmsg(h, f, oneofsubmsg_handler, &attr);
+      }
       break;
     }
   }

ruby/tests/basic_test.proto

@@ -126,11 +126,11 @@ message Wrapper {
   google.protobuf.BytesValue bytes = 9;
   string real_string = 100;
   oneof a_oneof {
-    string oneof_string = 10;
+    string string_in_oneof = 10;
   }
 
-  // Repeated wrappers don't really make sense, but we still need to make sure
-  // they work and don't crash.
+  // Repeated wrappers don't make sense, but we still need to make sure they
+  // work and don't crash.
   repeated google.protobuf.DoubleValue repeated_double = 11;
   repeated google.protobuf.FloatValue repeated_float = 12;
   repeated google.protobuf.Int32Value repeated_int32 = 13;
@@ -141,8 +141,8 @@ message Wrapper {
   repeated google.protobuf.StringValue repeated_string = 18;
   repeated google.protobuf.BytesValue repeated_bytes = 19;
 
-  // Wrappers as map keys don't really make sense, but we still need to make
-  // sure they work and don't crash.
+  // Wrappers as map keys don't make sense, but we still need to make sure they
+  // work and don't crash.
   map<int32, google.protobuf.DoubleValue> map_double = 21;
   map<int32, google.protobuf.FloatValue> map_float = 22;
   map<int32, google.protobuf.Int32Value> map_int32 = 23;
@@ -152,6 +152,21 @@ message Wrapper {
   map<int32, google.protobuf.BoolValue> map_bool = 27;
   map<int32, google.protobuf.StringValue> map_string = 28;
   map<int32, google.protobuf.BytesValue> map_bytes = 29;
+
+  // Wrappers in oneofs don't make sense, but we still need to make sure they
+  // work and don't crash.
+  oneof wrapper_oneof {
+    google.protobuf.DoubleValue oneof_double = 31;
+    google.protobuf.FloatValue oneof_float = 32;
+    google.protobuf.Int32Value oneof_int32 = 33;
+    google.protobuf.Int64Value oneof_int64 = 34;
+    google.protobuf.UInt32Value oneof_uint32 = 35;
+    google.protobuf.UInt64Value oneof_uint64 = 36;
+    google.protobuf.BoolValue oneof_bool = 37;
+    google.protobuf.StringValue oneof_string = 38;
+    google.protobuf.BytesValue oneof_bytes = 39;
+    string oneof_plain_string = 101;
+  }
 }
 
 message TimeMessage {

ruby/tests/basic_test_proto2.proto

@@ -133,7 +133,7 @@ message Wrapper {
   optional google.protobuf.BytesValue bytes = 9;
   optional string real_string = 100;
   oneof a_oneof {
-    string oneof_string = 10;
+    string string_in_oneof = 10;
   }
 
   // Repeated wrappers don't really make sense, but we still need to make sure
@@ -147,6 +147,21 @@ message Wrapper {
   repeated google.protobuf.BoolValue repeated_bool = 17;
   repeated google.protobuf.StringValue repeated_string = 18;
   repeated google.protobuf.BytesValue repeated_bytes = 19;
+
+  // Wrappers in oneofs don't make sense, but we still need to make sure they
+  // work and don't crash.
+  oneof wrapper_oneof {
+    google.protobuf.DoubleValue oneof_double = 31;
+    google.protobuf.FloatValue oneof_float = 32;
+    google.protobuf.Int32Value oneof_int32 = 33;
+    google.protobuf.Int64Value oneof_int64 = 34;
+    google.protobuf.UInt32Value oneof_uint32 = 35;
+    google.protobuf.UInt64Value oneof_uint64 = 36;
+    google.protobuf.BoolValue oneof_bool = 37;
+    google.protobuf.StringValue oneof_string = 38;
+    google.protobuf.BytesValue oneof_bytes = 39;
+    string oneof_plain_string = 101;
+  }
 }
 
 message TimeMessage {

ruby/tests/common_tests.rb

@@ -1376,6 +1376,44 @@ module CommonTests
     assert_equal m5, m
   end
 
+  def test_oneof_wrappers
+    run_test = ->(m) {
+      serialized = proto_module::Wrapper::encode(m)
+      m2 = proto_module::Wrapper::decode(serialized)
+
+      # Encode directly from lazy form.
+      serialized2 = proto_module::Wrapper::encode(m2)
+
+      assert_equal m, m2
+      assert_equal serialized, serialized2
+
+      serialized_json = proto_module::Wrapper::encode_json(m)
+      m3 = proto_module::Wrapper::decode_json(serialized_json)
+      assert_equal m, m3
+    }
+
+    m = proto_module::Wrapper.new()
+
+    run_test.call(m)
+    m.oneof_double_as_value = 2.0
+    run_test.call(m)
+    m.oneof_float_as_value = 4.0
+    run_test.call(m)
+    m.oneof_int32_as_value = 3
+    run_test.call(m)
+    m.oneof_int64_as_value = 5
+    run_test.call(m)
+    m.oneof_uint32_as_value = 6
+    run_test.call(m)
+    m.oneof_uint64_as_value = 7
+    run_test.call(m)
+    m.oneof_string_as_value = 'str'
+    run_test.call(m)
+    m.oneof_bytes_as_value = 'fun'
+    run_test.call(m)
+    puts m
+  end
+
   def test_top_level_wrappers
     # We don't expect anyone to do this, but we should also make sure it does
     # the right thing.
@@ -1383,7 +1421,6 @@ module CommonTests
       m = klass.new(value: val)
       serialized = klass::encode(m)
       m2 = klass::decode(serialized)
-      assert_equal m, m2
 
       # Encode directly from lazy form.
       serialized2 = klass::encode(m2)
@@ -1573,12 +1610,12 @@ module CommonTests
   end
 
   def test_wrappers_only
-    m = proto_module::Wrapper.new(real_string: 'hi', oneof_string: 'there')
+    m = proto_module::Wrapper.new(real_string: 'hi', string_in_oneof: 'there')
 
     assert_raise(NoMethodError) { m.real_string_as_value }
     assert_raise(NoMethodError) { m.as_value }
     assert_raise(NoMethodError) { m._as_value }
-    assert_raise(NoMethodError) { m.oneof_string_as_value }
+    assert_raise(NoMethodError) { m.string_in_oneof_as_value }
 
     m = proto_module::Wrapper.new
     m.string_as_value = 'you'