浏览代码

Implemented proto3 presence for Ruby. (#7406)

* WIP.

* WIP.

* Builds and runs. Tests need to be updated to test presence.

* Ruby: proto3 presence is passing all tests.

* Fixed a bug where empty messages has the wrong oneof count.
Joshua Haberman 5 年之前
父节点
当前提交
6b759688a1

+ 1 - 1
ruby/Rakefile

@@ -124,7 +124,7 @@ file "tests/test_ruby_package_proto2.rb" => "tests/test_ruby_package_proto2.prot
 end
 
 file "tests/basic_test.rb" => "tests/basic_test.proto" do |file_task|
-  sh "../src/protoc -I../src -I. --ruby_out=. tests/basic_test.proto"
+  sh "../src/protoc --experimental_allow_proto3_optional -I../src -I. --ruby_out=. tests/basic_test.proto"
 end
 
 file "tests/basic_test_proto2.rb" => "tests/basic_test_proto2.proto" do |file_task|

+ 94 - 6
ruby/ext/google/protobuf_c/defs.c

@@ -1100,7 +1100,7 @@ VALUE FieldDescriptor_get(VALUE _self, VALUE msg_rb) {
  *     FieldDescriptor.has?(message) => boolean
  *
  * Returns whether the value is set on the given message. Raises an
- * exception when calling with proto syntax 3.
+ * exception when calling for fields that do not have presence.
  */
 VALUE FieldDescriptor_has(VALUE _self, VALUE msg_rb) {
   DEFINE_SELF(FieldDescriptor, self, _self);
@@ -1434,6 +1434,7 @@ void MessageBuilderContext_register(VALUE module) {
   rb_define_method(klass, "initialize",
                    MessageBuilderContext_initialize, 2);
   rb_define_method(klass, "optional", MessageBuilderContext_optional, -1);
+  rb_define_method(klass, "proto3_optional", MessageBuilderContext_proto3_optional, -1);
   rb_define_method(klass, "required", MessageBuilderContext_required, -1);
   rb_define_method(klass, "repeated", MessageBuilderContext_repeated, -1);
   rb_define_method(klass, "map", MessageBuilderContext_map, -1);
@@ -1469,7 +1470,8 @@ VALUE MessageBuilderContext_initialize(VALUE _self,
 
 static void msgdef_add_field(VALUE msgbuilder_rb, upb_label_t label, VALUE name,
                              VALUE type, VALUE number, VALUE type_class,
-                             VALUE options, int oneof_index) {
+                             VALUE options, int oneof_index,
+                             bool proto3_optional) {
   DEFINE_SELF(MessageBuilderContext, self, msgbuilder_rb);
   FileBuilderContext* file_context =
       ruby_to_FileBuilderContext(self->file_builder);
@@ -1489,6 +1491,10 @@ static void msgdef_add_field(VALUE msgbuilder_rb, upb_label_t label, VALUE name,
   google_protobuf_FieldDescriptorProto_set_type(
       field_proto, (int)ruby_to_descriptortype(type));
 
+  if (proto3_optional) {
+    google_protobuf_FieldDescriptorProto_set_proto3_optional(field_proto, true);
+  }
+
   if (type_class != Qnil) {
     Check_Type(type_class, T_STRING);
 
@@ -1574,7 +1580,38 @@ VALUE MessageBuilderContext_optional(int argc, VALUE* argv, VALUE _self) {
   }
 
   msgdef_add_field(_self, UPB_LABEL_OPTIONAL, name, type, number, type_class,
-                   options, -1);
+                   options, -1, false);
+
+  return Qnil;
+}
+
+/*
+ * call-seq:
+ *     MessageBuilderContext.proto3_optional(name, type, number,
+ *                                           type_class = nil, options = nil)
+ *
+ * Defines a true proto3 optional field (that tracks presence) on this message
+ * type with the given type, tag number, and type class (for message and enum
+ * fields). The type must be a Ruby symbol (as accepted by
+ * FieldDescriptor#type=) and the type_class must be a string, if present (as
+ * accepted by FieldDescriptor#submsg_name=).
+ */
+VALUE MessageBuilderContext_proto3_optional(int argc, VALUE* argv,
+                                            VALUE _self) {
+  VALUE name, type, number;
+  VALUE type_class, options = Qnil;
+
+  rb_scan_args(argc, argv, "32", &name, &type, &number, &type_class, &options);
+
+  // Allow passing (name, type, number, options) or
+  // (name, type, number, type_class, options)
+  if (argc == 4 && RB_TYPE_P(type_class, T_HASH)) {
+    options = type_class;
+    type_class = Qnil;
+  }
+
+  msgdef_add_field(_self, UPB_LABEL_OPTIONAL, name, type, number, type_class,
+                   options, -1, true);
 
   return Qnil;
 }
@@ -1607,7 +1644,7 @@ VALUE MessageBuilderContext_required(int argc, VALUE* argv, VALUE _self) {
   }
 
   msgdef_add_field(_self, UPB_LABEL_REQUIRED, name, type, number, type_class,
-                   options, -1);
+                   options, -1, false);
 
   return Qnil;
 }
@@ -1633,7 +1670,7 @@ VALUE MessageBuilderContext_repeated(int argc, VALUE* argv, VALUE _self) {
   type_class = (argc > 3) ? argv[3] : Qnil;
 
   msgdef_add_field(_self, UPB_LABEL_REPEATED, name, type, number, type_class,
-                   Qnil, -1);
+                   Qnil, -1, false);
 
   return Qnil;
 }
@@ -1758,6 +1795,56 @@ VALUE MessageBuilderContext_oneof(VALUE _self, VALUE name) {
   return Qnil;
 }
 
+void MessageBuilderContext_add_synthetic_oneofs(VALUE _self) {
+  DEFINE_SELF(MessageBuilderContext, self, _self);
+  FileBuilderContext* file_context =
+      ruby_to_FileBuilderContext(self->file_builder);
+  size_t field_count, oneof_count;
+  google_protobuf_FieldDescriptorProto** fields =
+      google_protobuf_DescriptorProto_mutable_field(self->msg_proto, &field_count);
+  const google_protobuf_OneofDescriptorProto*const* oneofs =
+      google_protobuf_DescriptorProto_oneof_decl(self->msg_proto, &oneof_count);
+  VALUE names = rb_hash_new();
+  VALUE underscore = rb_str_new2("_");
+  size_t i;
+
+  // We have to build a set of all names, to ensure that synthetic oneofs are
+  // not creating conflicts.
+  for (i = 0; i < field_count; i++) {
+    upb_strview name = google_protobuf_FieldDescriptorProto_name(fields[i]);
+    rb_hash_aset(names, rb_str_new(name.data, name.size), Qtrue);
+  }
+  for (i = 0; i < oneof_count; i++) {
+    upb_strview name = google_protobuf_OneofDescriptorProto_name(oneofs[i]);
+    rb_hash_aset(names, rb_str_new(name.data, name.size), Qtrue);
+  }
+
+  for (i = 0; i < field_count; i++) {
+    google_protobuf_OneofDescriptorProto* oneof_proto;
+    VALUE oneof_name;
+    upb_strview field_name;
+
+    if (!google_protobuf_FieldDescriptorProto_proto3_optional(fields[i])) {
+      continue;
+    }
+
+    // Prepend '_' until we are no longer conflicting.
+    field_name = google_protobuf_FieldDescriptorProto_name(fields[i]);
+    oneof_name = rb_str_new(field_name.data, field_name.size);
+    while (rb_hash_lookup(names, oneof_name) != Qnil) {
+      oneof_name = rb_str_plus(underscore, oneof_name);
+    }
+
+    rb_hash_aset(names, oneof_name, Qtrue);
+    google_protobuf_FieldDescriptorProto_set_oneof_index(fields[i],
+                                                         oneof_count++);
+    oneof_proto = google_protobuf_DescriptorProto_add_oneof_decl(
+        self->msg_proto, file_context->arena);
+    google_protobuf_OneofDescriptorProto_set_name(
+        oneof_proto, FileBuilderContext_strdup(self->file_builder, oneof_name));
+  }
+}
+
 // -----------------------------------------------------------------------------
 // OneofBuilderContext.
 // -----------------------------------------------------------------------------
@@ -1829,7 +1916,7 @@ VALUE OneofBuilderContext_optional(int argc, VALUE* argv, VALUE _self) {
   rb_scan_args(argc, argv, "32", &name, &type, &number, &type_class, &options);
 
   msgdef_add_field(self->message_builder, UPB_LABEL_OPTIONAL, name, type,
-                   number, type_class, options, self->oneof_index);
+                   number, type_class, options, self->oneof_index, false);
 
   return Qnil;
 }
@@ -2033,6 +2120,7 @@ VALUE FileBuilderContext_add_message(VALUE _self, VALUE name) {
   VALUE ctx = rb_class_new_instance(2, args, cMessageBuilderContext);
   VALUE block = rb_block_proc();
   rb_funcall_with_block(ctx, rb_intern("instance_eval"), 0, NULL, block);
+  MessageBuilderContext_add_synthetic_oneofs(ctx);
   return Qnil;
 }
 

+ 3 - 3
ruby/ext/google/protobuf_c/encode_decode.c

@@ -933,7 +933,7 @@ void add_handlers_for_message(const void *closure, upb_handlers *h) {
        !upb_msg_field_done(&i);
        upb_msg_field_next(&i)) {
     const upb_fielddef *f = upb_msg_iter_field(&i);
-    const upb_oneofdef *oneof = upb_fielddef_containingoneof(f);
+    const upb_oneofdef* oneof = upb_fielddef_realcontainingoneof(f);
     size_t offset = get_field_offset(desc->layout, f);
 
     if (oneof) {
@@ -1506,7 +1506,7 @@ static void putmsg(VALUE msg_rb, const Descriptor* desc,
        !upb_msg_field_done(&i);
        upb_msg_field_next(&i)) {
     upb_fielddef *f = upb_msg_iter_field(&i);
-    const upb_oneofdef *oneof = upb_fielddef_containingoneof(f);
+    const upb_oneofdef* oneof = upb_fielddef_realcontainingoneof(f);
     bool is_matching_oneof = false;
     uint32_t offset =
         desc->layout->fields[upb_fielddef_index(f)].offset +
@@ -1714,7 +1714,7 @@ static void discard_unknown(VALUE msg_rb, const Descriptor* desc) {
        !upb_msg_field_done(&it);
        upb_msg_field_next(&it)) {
     upb_fielddef *f = upb_msg_iter_field(&it);
-    const upb_oneofdef *oneof = upb_fielddef_containingoneof(f);
+    const upb_oneofdef* oneof = upb_fielddef_realcontainingoneof(f);
     uint32_t offset =
         desc->layout->fields[upb_fielddef_index(f)].offset +
         sizeof(MessageHeader);

+ 12 - 7
ruby/ext/google/protobuf_c/message.c

@@ -242,9 +242,14 @@ static int extract_method_call(VALUE method_name, MessageHeader* self,
   // Method calls like 'has_foo?' are not allowed if field "foo" does not have
   // a hasbit (e.g. repeated fields or non-message type fields for proto3
   // syntax).
-  if (accessor_type == METHOD_PRESENCE && test_f != NULL &&
-      !upb_fielddef_haspresence(test_f)) {
-    return METHOD_UNKNOWN;
+  if (accessor_type == METHOD_PRESENCE && test_f != NULL) {
+    if (!upb_fielddef_haspresence(test_f)) return METHOD_UNKNOWN;
+
+    // TODO(haberman): remove this case, allow for proto3 oneofs.
+    if (upb_fielddef_realcontainingoneof(test_f) &&
+        upb_filedef_syntax(upb_fielddef_file(test_f)) == UPB_SYNTAX_PROTO3) {
+      return METHOD_UNKNOWN;
+    }
   }
 
   *o = test_o;
@@ -605,18 +610,18 @@ VALUE Message_inspect(VALUE _self) {
  */
 VALUE Message_to_h(VALUE _self) {
   MessageHeader* self;
-  VALUE hash;
+  VALUE hash = rb_hash_new();
   upb_msg_field_iter it;
+  bool is_proto2;
   TypedData_Get_Struct(_self, MessageHeader, &Message_type, self);
+
   // We currently have a few behaviors that are specific to proto2.
   // This is unfortunate, we should key behaviors off field attributes (like
   // whether a field has presence), not proto2 vs. proto3. We should see if we
   // can change this without breaking users.
-  bool is_proto2 =
+  is_proto2 =
       upb_msgdef_syntax(self->descriptor->msgdef) == UPB_SYNTAX_PROTO2;
 
-  hash = rb_hash_new();
-
   for (upb_msg_field_begin(&it, self->descriptor->msgdef);
        !upb_msg_field_done(&it);
        upb_msg_field_next(&it)) {

+ 1 - 0
ruby/ext/google/protobuf_c/protobuf.h

@@ -285,6 +285,7 @@ VALUE MessageBuilderContext_initialize(VALUE _self,
                                        VALUE _file_builder,
                                        VALUE name);
 VALUE MessageBuilderContext_optional(int argc, VALUE* argv, VALUE _self);
+VALUE MessageBuilderContext_proto3_optional(int argc, VALUE* argv, VALUE _self);
 VALUE MessageBuilderContext_required(int argc, VALUE* argv, VALUE _self);
 VALUE MessageBuilderContext_repeated(int argc, VALUE* argv, VALUE _self);
 VALUE MessageBuilderContext_map(int argc, VALUE* argv, VALUE _self);

+ 45 - 16
ruby/ext/google/protobuf_c/storage.c

@@ -496,11 +496,14 @@ void create_layout(Descriptor* desc) {
   const upb_msgdef *msgdef = desc->msgdef;
   MessageLayout* layout = ALLOC(MessageLayout);
   int nfields = upb_msgdef_numfields(msgdef);
-  int noneofs = upb_msgdef_numoneofs(msgdef);
+  int noneofs = upb_msgdef_numrealoneofs(msgdef);
   upb_msg_field_iter it;
   upb_msg_oneof_iter oit;
   size_t off = 0;
   size_t hasbit = 0;
+  int i;
+
+  (void)i;
 
   layout->empty_template = NULL;
   layout->desc = desc;
@@ -513,12 +516,22 @@ void create_layout(Descriptor* desc) {
     layout->oneofs = ALLOC_N(MessageOneof, noneofs);
   }
 
+#ifndef NDEBUG
+  for (i = 0; i < nfields; i++) {
+    layout->fields[i].offset = -1;
+  }
+
+  for (i = 0; i < noneofs; i++) {
+    layout->oneofs[i].offset = -1;
+  }
+#endif
+
   for (upb_msg_field_begin(&it, msgdef);
        !upb_msg_field_done(&it);
        upb_msg_field_next(&it)) {
     const upb_fielddef* field = upb_msg_iter_field(&it);
     if (upb_fielddef_haspresence(field) &&
-        !upb_fielddef_containingoneof(field)) {
+        !upb_fielddef_realcontainingoneof(field)) {
       layout->fields[upb_fielddef_index(field)].hasbit = hasbit++;
     } else {
       layout->fields[upb_fielddef_index(field)].hasbit =
@@ -541,7 +554,7 @@ void create_layout(Descriptor* desc) {
        !upb_msg_field_done(&it);
        upb_msg_field_next(&it)) {
     const upb_fielddef* field = upb_msg_iter_field(&it);
-    if (upb_fielddef_containingoneof(field) || !upb_fielddef_isseq(field) ||
+    if (upb_fielddef_realcontainingoneof(field) || !upb_fielddef_isseq(field) ||
         upb_fielddef_ismap(field)) {
       continue;
     }
@@ -556,7 +569,7 @@ void create_layout(Descriptor* desc) {
        !upb_msg_field_done(&it);
        upb_msg_field_next(&it)) {
     const upb_fielddef* field = upb_msg_iter_field(&it);
-    if (upb_fielddef_containingoneof(field) || !upb_fielddef_isseq(field) ||
+    if (upb_fielddef_realcontainingoneof(field) || !upb_fielddef_isseq(field) ||
         !upb_fielddef_ismap(field)) {
       continue;
     }
@@ -573,7 +586,7 @@ void create_layout(Descriptor* desc) {
        !upb_msg_field_done(&it);
        upb_msg_field_next(&it)) {
     const upb_fielddef* field = upb_msg_iter_field(&it);
-    if (upb_fielddef_containingoneof(field) || !is_value_field(field) ||
+    if (upb_fielddef_realcontainingoneof(field) || !is_value_field(field) ||
         upb_fielddef_isseq(field)) {
       continue;
     }
@@ -590,7 +603,7 @@ void create_layout(Descriptor* desc) {
     const upb_fielddef* field = upb_msg_iter_field(&it);
     size_t field_size;
 
-    if (upb_fielddef_containingoneof(field) || is_value_field(field)) {
+    if (upb_fielddef_realcontainingoneof(field) || is_value_field(field)) {
       continue;
     }
 
@@ -625,6 +638,10 @@ void create_layout(Descriptor* desc) {
     // Always allocate NATIVE_SLOT_MAX_SIZE bytes, but share the slot between
     // all fields.
     size_t field_size = NATIVE_SLOT_MAX_SIZE;
+
+    if (upb_oneofdef_issynthetic(oneof)) continue;
+    assert(upb_oneofdef_index(oneof) < noneofs);
+
     // Align the offset.
     off = align_up_to(off, field_size);
     // Assign all fields in the oneof this same offset.
@@ -644,6 +661,8 @@ void create_layout(Descriptor* desc) {
        upb_msg_oneof_next(&oit)) {
     const upb_oneofdef* oneof = upb_msg_iter_oneof(&oit);
     size_t field_size = sizeof(uint32_t);
+    if (upb_oneofdef_issynthetic(oneof)) continue;
+    assert(upb_oneofdef_index(oneof) < noneofs);
     // Align the offset.
     off = (off + field_size - 1) & ~(field_size - 1);
     layout->oneofs[upb_oneofdef_index(oneof)].case_offset = off;
@@ -653,6 +672,16 @@ void create_layout(Descriptor* desc) {
   layout->size = off;
   layout->msgdef = msgdef;
 
+#ifndef NDEBUG
+  for (i = 0; i < nfields; i++) {
+    assert(layout->fields[i].offset != -1);
+  }
+
+  for (i = 0; i < noneofs; i++) {
+    assert(layout->oneofs[i].offset != -1);
+  }
+#endif
+
   // Create the empty message template.
   layout->empty_template = ALLOC_N(char, layout->size);
   memset(layout->empty_template, 0, layout->size);
@@ -725,8 +754,8 @@ static void slot_clear_hasbit(MessageLayout* layout,
 static bool slot_is_hasbit_set(MessageLayout* layout,
                             const void* storage,
                             const upb_fielddef* field) {
-  assert(field_contains_hasbit(layout, field));
   size_t hasbit = layout->fields[upb_fielddef_index(field)].hasbit;
+  assert(field_contains_hasbit(layout, field));
   return DEREF_OFFSET(
       (uint8_t*)storage, hasbit / 8, char) & (1 << (hasbit % 8));
 }
@@ -734,11 +763,11 @@ static bool slot_is_hasbit_set(MessageLayout* layout,
 VALUE layout_has(MessageLayout* layout,
                  const void* storage,
                  const upb_fielddef* field) {
+  const upb_oneofdef* oneof = upb_fielddef_realcontainingoneof(field);
   assert(upb_fielddef_haspresence(field));
-  const upb_oneofdef* oneof = upb_fielddef_containingoneof(field);
   if (oneof) {
     uint32_t oneof_case = slot_read_oneof_case(layout, storage, oneof);
-    return oneof_case == upb_fielddef_number(field);
+    return oneof_case == upb_fielddef_number(field) ? Qtrue : Qfalse;
   } else {
     return slot_is_hasbit_set(layout, storage, field) ? Qtrue : Qfalse;
   }
@@ -748,7 +777,7 @@ void layout_clear(MessageLayout* layout,
                  const void* storage,
                  const upb_fielddef* field) {
   void* memory = slot_memory(layout, storage, field);
-  const upb_oneofdef* oneof = upb_fielddef_containingoneof(field);
+  const upb_oneofdef* oneof = upb_fielddef_realcontainingoneof(field);
 
   if (field_contains_hasbit(layout, field)) {
     slot_clear_hasbit(layout, storage, field);
@@ -841,7 +870,7 @@ VALUE layout_get(MessageLayout* layout,
                  const void* storage,
                  const upb_fielddef* field) {
   void* memory = slot_memory(layout, storage, field);
-  const upb_oneofdef* oneof = upb_fielddef_containingoneof(field);
+  const upb_oneofdef* oneof = upb_fielddef_realcontainingoneof(field);
   bool field_set;
   if (field_contains_hasbit(layout, field)) {
     field_set = slot_is_hasbit_set(layout, storage, field);
@@ -914,7 +943,7 @@ void layout_set(MessageLayout* layout,
                 const upb_fielddef* field,
                 VALUE val) {
   void* memory = slot_memory(layout, storage, field);
-  const upb_oneofdef* oneof = upb_fielddef_containingoneof(field);
+  const upb_oneofdef* oneof = upb_fielddef_realcontainingoneof(field);
 
   if (oneof) {
     uint32_t* oneof_case = slot_oneof_case(layout, storage, oneof);
@@ -985,7 +1014,7 @@ void layout_init(MessageLayout* layout, void* storage) {
 
 void layout_mark(MessageLayout* layout, void* storage) {
   VALUE* values = (VALUE*)CHARPTR_AT(storage, layout->value_offset);
-  int noneofs = upb_msgdef_numoneofs(layout->msgdef);
+  int noneofs = upb_msgdef_numrealoneofs(layout->msgdef);
   int i;
 
   for (i = 0; i < layout->value_count; i++) {
@@ -1007,7 +1036,7 @@ void layout_dup(MessageLayout* layout, void* to, void* from) {
        !upb_msg_field_done(&it);
        upb_msg_field_next(&it)) {
     const upb_fielddef* field = upb_msg_iter_field(&it);
-    const upb_oneofdef* oneof = upb_fielddef_containingoneof(field);
+    const upb_oneofdef* oneof = upb_fielddef_realcontainingoneof(field);
 
     void* to_memory = slot_memory(layout, to, field);
     void* from_memory = slot_memory(layout, from, field);
@@ -1041,7 +1070,7 @@ void layout_deep_copy(MessageLayout* layout, void* to, void* from) {
        !upb_msg_field_done(&it);
        upb_msg_field_next(&it)) {
     const upb_fielddef* field = upb_msg_iter_field(&it);
-    const upb_oneofdef* oneof = upb_fielddef_containingoneof(field);
+    const upb_oneofdef* oneof = upb_fielddef_realcontainingoneof(field);
 
     void* to_memory = slot_memory(layout, to, field);
     void* from_memory = slot_memory(layout, from, field);
@@ -1081,7 +1110,7 @@ VALUE layout_eq(MessageLayout* layout, void* msg1, void* msg2) {
        !upb_msg_field_done(&it);
        upb_msg_field_next(&it)) {
     const upb_fielddef* field = upb_msg_iter_field(&it);
-    const upb_oneofdef* oneof = upb_fielddef_containingoneof(field);
+    const upb_oneofdef* oneof = upb_fielddef_realcontainingoneof(field);
 
     void* msg1_memory = slot_memory(layout, msg1, field);
     void* msg2_memory = slot_memory(layout, msg2, field);

+ 57 - 14
ruby/ext/google/protobuf_c/upb.c

@@ -3032,6 +3032,7 @@ struct upb_msgdef {
   const upb_oneofdef *oneofs;
   int field_count;
   int oneof_count;
+  int real_oneof_count;
 
   /* Is this a map-entry message? */
   bool map_entry;
@@ -3203,11 +3204,14 @@ static uint32_t upb_handlers_selectorcount(const upb_fielddef *f) {
   return ret;
 }
 
+static void upb_status_setoom(upb_status *status) {
+  upb_status_seterrmsg(status, "out of memory");
+}
+
 static bool assign_msg_indices(upb_msgdef *m, upb_status *s) {
   /* Sort fields.  upb internally relies on UPB_TYPE_MESSAGE fields having the
    * lowest indexes, but we do not publicly guarantee this. */
   upb_msg_field_iter j;
-  upb_msg_oneof_iter k;
   int i;
   uint32_t selector;
   int n = upb_msgdef_numfields(m);
@@ -3248,14 +3252,38 @@ static bool assign_msg_indices(upb_msgdef *m, upb_status *s) {
   }
   m->selector_count = selector;
 
-  for(upb_msg_oneof_begin(&k, m), i = 0;
-      !upb_msg_oneof_done(&k);
-      upb_msg_oneof_next(&k), i++) {
-    upb_oneofdef *o = (upb_oneofdef*)upb_msg_iter_oneof(&k);
-    o->index = i;
+  upb_gfree(fields);
+  return true;
+}
+
+static bool check_oneofs(upb_msgdef *m, upb_status *s) {
+  int i;
+  int first_synthetic = -1;
+  upb_oneofdef *mutable_oneofs = (upb_oneofdef*)m->oneofs;
+
+  for (i = 0; i < m->oneof_count; i++) {
+    mutable_oneofs[i].index = i;
+
+    if (upb_oneofdef_issynthetic(&mutable_oneofs[i])) {
+      if (first_synthetic == -1) {
+        first_synthetic = i;
+      }
+    } else {
+      if (first_synthetic != -1) {
+        upb_status_seterrf(
+            s, "Synthetic oneofs must be after all other oneofs: %s",
+            upb_oneofdef_name(&mutable_oneofs[i]));
+        return false;
+      }
+    }
+  }
+
+  if (first_synthetic == -1) {
+    m->real_oneof_count = m->oneof_count;
+  } else {
+    m->real_oneof_count = first_synthetic;
   }
 
-  upb_gfree(fields);
   return true;
 }
 
@@ -3440,6 +3468,10 @@ uint32_t upb_fielddef_selectorbase(const upb_fielddef *f) {
   return f->selector_base;
 }
 
+const upb_filedef *upb_fielddef_file(const upb_fielddef *f) {
+  return f->file;
+}
+
 const upb_msgdef *upb_fielddef_containingtype(const upb_fielddef *f) {
   return f->msgdef;
 }
@@ -3448,6 +3480,11 @@ const upb_oneofdef *upb_fielddef_containingoneof(const upb_fielddef *f) {
   return f->oneof;
 }
 
+const upb_oneofdef *upb_fielddef_realcontainingoneof(const upb_fielddef *f) {
+  if (!f->oneof || upb_oneofdef_issynthetic(f->oneof)) return NULL;
+  return f->oneof;
+}
+
 static void chkdefaulttype(const upb_fielddef *f, int ctype) {
   UPB_UNUSED(f);
   UPB_UNUSED(ctype);
@@ -3544,9 +3581,8 @@ bool upb_fielddef_hassubdef(const upb_fielddef *f) {
 
 bool upb_fielddef_haspresence(const upb_fielddef *f) {
   if (upb_fielddef_isseq(f)) return false;
-  if (upb_fielddef_issubmsg(f)) return true;
-  if (f->proto3_optional_) return true;
-  return f->file->syntax == UPB_SYNTAX_PROTO2;
+  return upb_fielddef_issubmsg(f) || upb_fielddef_containingoneof(f) ||
+         f->file->syntax == UPB_SYNTAX_PROTO2;
 }
 
 static bool between(int32_t x, int32_t low, int32_t high) {
@@ -3651,6 +3687,10 @@ int upb_msgdef_numoneofs(const upb_msgdef *m) {
   return m->oneof_count;
 }
 
+int upb_msgdef_numrealoneofs(const upb_msgdef *m) {
+  return m->real_oneof_count;
+}
+
 const upb_msglayout *upb_msgdef_layout(const upb_msgdef *m) {
   return m->layout;
 }
@@ -3749,7 +3789,7 @@ uint32_t upb_oneofdef_index(const upb_oneofdef *o) {
   return o->index;
 }
 
-bool upb_oneofdef_synthetic(const upb_oneofdef *o) {
+bool upb_oneofdef_issynthetic(const upb_oneofdef *o) {
   upb_inttable_iter iter;
   const upb_fielddef *f;
   upb_inttable_begin(&iter, &o->itof);
@@ -3941,7 +3981,7 @@ static bool make_layout(const upb_symtab *symtab, const upb_msgdef *m) {
       submsgs[field->submsg_index] = subm->layout;
     }
 
-    if (upb_fielddef_haspresence(f) && !upb_fielddef_containingoneof(f)) {
+    if (upb_fielddef_haspresence(f) && !upb_fielddef_realcontainingoneof(f)) {
       /* We don't use hasbit 0, so that 0 can indicate "no presence" in the
        * table. This wastes one hasbit, but we don't worry about it for now. */
       field->presence = ++hasbit;
@@ -3960,7 +4000,7 @@ static bool make_layout(const upb_symtab *symtab, const upb_msgdef *m) {
     size_t field_size = upb_msg_fielddefsize(f);
     size_t index = upb_fielddef_index(f);
 
-    if (upb_fielddef_containingoneof(f)) {
+    if (upb_fielddef_realcontainingoneof(f)) {
       /* Oneofs are handled separately below. */
       continue;
     }
@@ -3975,6 +4015,8 @@ static bool make_layout(const upb_symtab *symtab, const upb_msgdef *m) {
     const upb_oneofdef* o = upb_msg_iter_oneof(&oit);
     upb_oneof_iter fit;
 
+    if (upb_oneofdef_issynthetic(o)) continue;
+
     size_t case_size = sizeof(uint32_t);  /* Could potentially optimize this. */
     size_t field_size = 0;
     uint32_t case_offset;
@@ -4587,6 +4629,7 @@ static bool create_msgdef(symtab_addctx *ctx, const char *prefix,
   }
 
   CHK(assign_msg_indices(m, ctx->status));
+  CHK(check_oneofs(m, ctx->status));
   assign_msg_wellknowntype(m);
   upb_inttable_compact2(&m->itof, ctx->alloc);
 
@@ -9649,7 +9692,7 @@ static bool multipart_text(upb_json_parser *p, const char *buf, size_t len,
 /* Note: this invalidates the accumulate buffer!  Call only after reading its
  * contents. */
 static void multipart_end(upb_json_parser *p) {
-  UPB_ASSERT(p->multipart_state != MULTIPART_INACTIVE);
+  /* UPB_ASSERT(p->multipart_state != MULTIPART_INACTIVE); */
   p->multipart_state = MULTIPART_INACTIVE;
   accumulate_clear(p);
 }

+ 21 - 718
ruby/ext/google/protobuf_c/upb.h

@@ -211,9 +211,6 @@ int msvc_vsnprintf(char* s, size_t n, const char* format, va_list arg);
 #include <string.h>
 /*
 ** This file contains shared definitions that are widely used across upb.
-**
-** This is a mixed C/C++ interface that offers a full API to both languages.
-** See the top-level README for more information.
 */
 
 #ifndef UPB_H_
@@ -226,23 +223,13 @@ int msvc_vsnprintf(char* s, size_t n, const char* format, va_list arg);
 #include <stdint.h>
 #include <string.h>
 
+
 #ifdef __cplusplus
-#include <memory>
-namespace upb {
-class Arena;
-class Status;
-template <int N> class InlinedArena;
-}
+extern "C" {
 #endif
 
-
 /* upb_status *****************************************************************/
 
-/* upb_status represents a success or failure status and error message.
- * It owns no resources and allocates no memory, so it should work
- * even in OOM situations. */
-
-/* The maximum length of an error message before it will get truncated. */
 #define UPB_STATUS_MAX_MESSAGE 127
 
 typedef struct {
@@ -250,59 +237,15 @@ typedef struct {
   char msg[UPB_STATUS_MAX_MESSAGE];  /* Error message; NULL-terminated. */
 } upb_status;
 
-#ifdef __cplusplus
-extern "C" {
-#endif
-
 const char *upb_status_errmsg(const upb_status *status);
 bool upb_ok(const upb_status *status);
 
-/* Any of the functions that write to a status object allow status to be NULL,
- * to support use cases where the function's caller does not care about the
- * status message. */
+/* These are no-op if |status| is NULL. */
 void upb_status_clear(upb_status *status);
 void upb_status_seterrmsg(upb_status *status, const char *msg);
 void upb_status_seterrf(upb_status *status, const char *fmt, ...);
 void upb_status_vseterrf(upb_status *status, const char *fmt, va_list args);
 
-UPB_INLINE void upb_status_setoom(upb_status *status) {
-  upb_status_seterrmsg(status, "out of memory");
-}
-
-#ifdef __cplusplus
-}  /* extern "C" */
-
-class upb::Status {
- public:
-  Status() { upb_status_clear(&status_); }
-
-  upb_status* ptr() { return &status_; }
-
-  /* Returns true if there is no error. */
-  bool ok() const { return upb_ok(&status_); }
-
-  /* Guaranteed to be NULL-terminated. */
-  const char *error_message() const { return upb_status_errmsg(&status_); }
-
-  /* The error message will be truncated if it is longer than
-   * UPB_STATUS_MAX_MESSAGE-4. */
-  void SetErrorMessage(const char *msg) { upb_status_seterrmsg(&status_, msg); }
-  void SetFormattedErrorMessage(const char *fmt, ...) {
-    va_list args;
-    va_start(args, fmt);
-    upb_status_vseterrf(&status_, fmt, args);
-    va_end(args);
-  }
-
-  /* Resets the status to a successful state with no message. */
-  void Clear() { upb_status_clear(&status_); }
-
- private:
-  upb_status status_;
-};
-
-#endif  /* __cplusplus */
-
 /** upb_strview ************************************************************/
 
 typedef struct {
@@ -369,16 +312,8 @@ UPB_INLINE void upb_free(upb_alloc *alloc, void *ptr) {
 
 /* The global allocator used by upb.  Uses the standard malloc()/free(). */
 
-#ifdef __cplusplus
-extern "C" {
-#endif
-
 extern upb_alloc upb_alloc_global;
 
-#ifdef __cplusplus
-}  /* extern "C" */
-#endif
-
 /* Functions that hard-code the global malloc.
  *
  * We still get benefit because we can put custom logic into our global
@@ -415,10 +350,6 @@ typedef void upb_cleanup_func(void *ud);
 struct upb_arena;
 typedef struct upb_arena upb_arena;
 
-#ifdef __cplusplus
-extern "C" {
-#endif
-
 typedef struct {
   /* We implement the allocator interface.
    * This must be the first member of upb_arena! */
@@ -468,64 +399,6 @@ UPB_INLINE upb_arena *upb_arena_new(void) {
   return upb_arena_init(NULL, 0, &upb_alloc_global);
 }
 
-#ifdef __cplusplus
-}  /* extern "C" */
-
-class upb::Arena {
- public:
-  /* A simple arena with no initial memory block and the default allocator. */
-  Arena() : ptr_(upb_arena_new(), upb_arena_free) {}
-
-  upb_arena* ptr() { return ptr_.get(); }
-
-  /* Allows this arena to be used as a generic allocator.
-   *
-   * The arena does not need free() calls so when using Arena as an allocator
-   * it is safe to skip them.  However they are no-ops so there is no harm in
-   * calling free() either. */
-  upb_alloc *allocator() { return upb_arena_alloc(ptr_.get()); }
-
-  /* Add a cleanup function to run when the arena is destroyed.
-   * Returns false on out-of-memory. */
-  bool AddCleanup(void *ud, upb_cleanup_func* func) {
-    return upb_arena_addcleanup(ptr_.get(), ud, func);
-  }
-
-  /* Total number of bytes that have been allocated.  It is undefined what
-   * Realloc() does to &arena_ counter. */
-  size_t BytesAllocated() const { return upb_arena_bytesallocated(ptr_.get()); }
-
- private:
-  std::unique_ptr<upb_arena, decltype(&upb_arena_free)> ptr_;
-};
-
-#endif
-
-/* upb::InlinedArena **********************************************************/
-
-/* upb::InlinedArena seeds the arenas with a predefined amount of memory.  No
- * heap memory will be allocated until the initial block is exceeded.
- *
- * These types only exist in C++ */
-
-#ifdef __cplusplus
-
-template <int N> class upb::InlinedArena : public upb::Arena {
- public:
-  InlinedArena() : ptr_(upb_arena_new(&initial_block_, N, &upb_alloc_global)) {}
-
-  upb_arena* ptr() { return ptr_.get(); }
-
- private:
-  InlinedArena(const InlinedArena*) = delete;
-  InlinedArena& operator=(const InlinedArena*) = delete;
-
-  std::unique_ptr<upb_arena, decltype(&upb_arena_free)> ptr_;
-  char initial_block_[N];
-};
-
-#endif  /* __cplusplus */
-
 /* Constants ******************************************************************/
 
 /* Generic function type. */
@@ -545,20 +418,15 @@ typedef enum {
  * types defined in descriptor.proto, which gives INT32 and SINT32 separate
  * types (we distinguish the two with the "integer encoding" enum below). */
 typedef enum {
-  /* Types stored in 1 byte. */
   UPB_TYPE_BOOL     = 1,
-  /* Types stored in 4 bytes. */
   UPB_TYPE_FLOAT    = 2,
   UPB_TYPE_INT32    = 3,
   UPB_TYPE_UINT32   = 4,
   UPB_TYPE_ENUM     = 5,  /* Enum values are int32. */
-  /* Types stored as void* (probably 4 or 8 bytes). */
   UPB_TYPE_MESSAGE  = 6,
-  /* Types stored as 8 bytes. */
   UPB_TYPE_DOUBLE   = 7,
   UPB_TYPE_INT64    = 8,
   UPB_TYPE_UINT64   = 9,
-  /* Types stored as upb_strview (2 * void*) (probably 8 or 16 bytes). */
   UPB_TYPE_STRING   = 10,
   UPB_TYPE_BYTES    = 11
 } upb_fieldtype_t;
@@ -615,6 +483,10 @@ typedef enum {
 #define UPB_MAP_BEGIN -1
 
 
+#ifdef __cplusplus
+}  /* extern "C" */
+#endif
+
 #endif  /* UPB_H_ */
 
 
@@ -3208,38 +3080,23 @@ UPB_INLINE void google_protobuf_GeneratedCodeInfo_Annotation_set_end(google_prot
 ** Defs are upb's internal representation of the constructs that can appear
 ** in a .proto file:
 **
-** - upb::MessageDefPtr (upb_msgdef): describes a "message" construct.
-** - upb::FieldDefPtr (upb_fielddef): describes a message field.
-** - upb::FileDefPtr (upb_filedef): describes a .proto file and its defs.
-** - upb::EnumDefPtr (upb_enumdef): describes an enum.
-** - upb::OneofDefPtr (upb_oneofdef): describes a oneof.
+** - upb_msgdef: describes a "message" construct.
+** - upb_fielddef: describes a message field.
+** - upb_filedef: describes a .proto file and its defs.
+** - upb_enumdef: describes an enum.
+** - upb_oneofdef: describes a oneof.
 **
 ** TODO: definitions of services.
-**
-** This is a mixed C/C++ interface that offers a full API to both languages.
-** See the top-level README for more information.
 */
 
 #ifndef UPB_DEF_H_
 #define UPB_DEF_H_
 
 
-#ifdef __cplusplus
-#include <cstring>
-#include <memory>
-#include <string>
-#include <vector>
-
-namespace upb {
-class EnumDefPtr;
-class FieldDefPtr;
-class FileDefPtr;
-class MessageDefPtr;
-class OneofDefPtr;
-class SymbolTable;
-}
-#endif
 
+#ifdef __cplusplus
+extern "C" {
+#endif  /* __cplusplus */
 
 struct upb_enumdef;
 typedef struct upb_enumdef upb_enumdef;
@@ -3291,10 +3148,6 @@ typedef enum {
  * protobuf wire format. */
 #define UPB_MAX_FIELDNUMBER ((1 << 29) - 1)
 
-#ifdef __cplusplus
-extern "C" {
-#endif
-
 const char *upb_fielddef_fullname(const upb_fielddef *f);
 upb_fieldtype_t upb_fielddef_type(const upb_fielddef *f);
 upb_descriptortype_t upb_fielddef_descriptortype(const upb_fielddef *f);
@@ -3305,8 +3158,10 @@ const char *upb_fielddef_jsonname(const upb_fielddef *f);
 bool upb_fielddef_isextension(const upb_fielddef *f);
 bool upb_fielddef_lazy(const upb_fielddef *f);
 bool upb_fielddef_packed(const upb_fielddef *f);
+const upb_filedef *upb_fielddef_file(const upb_fielddef *f);
 const upb_msgdef *upb_fielddef_containingtype(const upb_fielddef *f);
 const upb_oneofdef *upb_fielddef_containingoneof(const upb_fielddef *f);
+const upb_oneofdef *upb_fielddef_realcontainingoneof(const upb_fielddef *f);
 uint32_t upb_fielddef_index(const upb_fielddef *f);
 bool upb_fielddef_issubmsg(const upb_fielddef *f);
 bool upb_fielddef_isstring(const upb_fielddef *f);
@@ -3330,130 +3185,15 @@ const upb_msglayout_field *upb_fielddef_layout(const upb_fielddef *f);
 /* Internal only. */
 uint32_t upb_fielddef_selectorbase(const upb_fielddef *f);
 
-#ifdef __cplusplus
-}  /* extern "C" */
-
-/* A upb_fielddef describes a single field in a message.  It is most often
- * found as a part of a upb_msgdef, but can also stand alone to represent
- * an extension. */
-class upb::FieldDefPtr {
- public:
-  FieldDefPtr() : ptr_(nullptr) {}
-  explicit FieldDefPtr(const upb_fielddef *ptr) : ptr_(ptr) {}
-
-  const upb_fielddef* ptr() const { return ptr_; }
-  explicit operator bool() const { return ptr_ != nullptr; }
-
-  typedef upb_fieldtype_t Type;
-  typedef upb_label_t Label;
-  typedef upb_descriptortype_t DescriptorType;
-
-  const char* full_name() const { return upb_fielddef_fullname(ptr_); }
-
-  Type type() const { return upb_fielddef_type(ptr_); }
-  Label label() const { return upb_fielddef_label(ptr_); }
-  const char* name() const { return upb_fielddef_name(ptr_); }
-  const char* json_name() const { return upb_fielddef_jsonname(ptr_); }
-  uint32_t number() const { return upb_fielddef_number(ptr_); }
-  bool is_extension() const { return upb_fielddef_isextension(ptr_); }
-
-  /* For UPB_TYPE_MESSAGE fields only where is_tag_delimited() == false,
-   * indicates whether this field should have lazy parsing handlers that yield
-   * the unparsed string for the submessage.
-   *
-   * TODO(haberman): I think we want to move this into a FieldOptions container
-   * when we add support for custom options (the FieldOptions struct will
-   * contain both regular FieldOptions like "lazy" *and* custom options). */
-  bool lazy() const { return upb_fielddef_lazy(ptr_); }
-
-  /* For non-string, non-submessage fields, this indicates whether binary
-   * protobufs are encoded in packed or non-packed format.
-   *
-   * TODO(haberman): see note above about putting options like this into a
-   * FieldOptions container. */
-  bool packed() const { return upb_fielddef_packed(ptr_); }
-
-  /* An integer that can be used as an index into an array of fields for
-   * whatever message this field belongs to.  Guaranteed to be less than
-   * f->containing_type()->field_count().  May only be accessed once the def has
-   * been finalized. */
-  uint32_t index() const { return upb_fielddef_index(ptr_); }
-
-  /* The MessageDef to which this field belongs.
-   *
-   * If this field has been added to a MessageDef, that message can be retrieved
-   * directly (this is always the case for frozen FieldDefs).
-   *
-   * If the field has not yet been added to a MessageDef, you can set the name
-   * of the containing type symbolically instead.  This is mostly useful for
-   * extensions, where the extension is declared separately from the message. */
-  MessageDefPtr containing_type() const;
-
-  /* The OneofDef to which this field belongs, or NULL if this field is not part
-   * of a oneof. */
-  OneofDefPtr containing_oneof() const;
-
-  /* The field's type according to the enum in descriptor.proto.  This is not
-   * the same as UPB_TYPE_*, because it distinguishes between (for example)
-   * INT32 and SINT32, whereas our "type" enum does not.  This return of
-   * descriptor_type() is a function of type(), integer_format(), and
-   * is_tag_delimited().  */
-  DescriptorType descriptor_type() const {
-    return upb_fielddef_descriptortype(ptr_);
-  }
-
-  /* Convenient field type tests. */
-  bool IsSubMessage() const { return upb_fielddef_issubmsg(ptr_); }
-  bool IsString() const { return upb_fielddef_isstring(ptr_); }
-  bool IsSequence() const { return upb_fielddef_isseq(ptr_); }
-  bool IsPrimitive() const { return upb_fielddef_isprimitive(ptr_); }
-  bool IsMap() const { return upb_fielddef_ismap(ptr_); }
-
-  /* Returns the non-string default value for this fielddef, which may either
-   * be something the client set explicitly or the "default default" (0 for
-   * numbers, empty for strings).  The field's type indicates the type of the
-   * returned value, except for enum fields that are still mutable.
-   *
-   * Requires that the given function matches the field's current type. */
-  int64_t default_int64() const { return upb_fielddef_defaultint64(ptr_); }
-  int32_t default_int32() const { return upb_fielddef_defaultint32(ptr_); }
-  uint64_t default_uint64() const { return upb_fielddef_defaultuint64(ptr_); }
-  uint32_t default_uint32() const { return upb_fielddef_defaultuint32(ptr_); }
-  bool default_bool() const { return upb_fielddef_defaultbool(ptr_); }
-  float default_float() const { return upb_fielddef_defaultfloat(ptr_); }
-  double default_double() const { return upb_fielddef_defaultdouble(ptr_); }
-
-  /* The resulting string is always NULL-terminated.  If non-NULL, the length
-   * will be stored in *len. */
-  const char *default_string(size_t * len) const {
-    return upb_fielddef_defaultstr(ptr_, len);
-  }
-
-  /* Returns the enum or submessage def for this field, if any.  The field's
-   * type must match (ie. you may only call enum_subdef() for fields where
-   * type() == UPB_TYPE_ENUM). */
-  EnumDefPtr enum_subdef() const;
-  MessageDefPtr message_subdef() const;
-
- private:
-  const upb_fielddef *ptr_;
-};
-
-#endif  /* __cplusplus */
-
 /* upb_oneofdef ***************************************************************/
 
-#ifdef __cplusplus
-extern "C" {
-#endif
-
 typedef upb_inttable_iter upb_oneof_iter;
 
 const char *upb_oneofdef_name(const upb_oneofdef *o);
 const upb_msgdef *upb_oneofdef_containingtype(const upb_oneofdef *o);
 int upb_oneofdef_numfields(const upb_oneofdef *o);
 uint32_t upb_oneofdef_index(const upb_oneofdef *o);
-bool upb_oneofdef_synthetic(const upb_oneofdef *o);
+bool upb_oneofdef_issynthetic(const upb_oneofdef *o);
 
 /* Oneof lookups:
  * - ntof:  look up a field by name.
@@ -3480,92 +3220,6 @@ void upb_oneof_iter_setdone(upb_oneof_iter *iter);
 bool upb_oneof_iter_isequal(const upb_oneof_iter *iter1,
                             const upb_oneof_iter *iter2);
 
-#ifdef __cplusplus
-}  /* extern "C" */
-
-/* Class that represents a oneof. */
-class upb::OneofDefPtr {
- public:
-  OneofDefPtr() : ptr_(nullptr) {}
-  explicit OneofDefPtr(const upb_oneofdef *ptr) : ptr_(ptr) {}
-
-  const upb_oneofdef* ptr() const { return ptr_; }
-  explicit operator bool() { return ptr_ != nullptr; }
-
-  /* Returns the MessageDef that owns this OneofDef. */
-  MessageDefPtr containing_type() const;
-
-  /* Returns the name of this oneof. This is the name used to look up the oneof
-   * by name once added to a message def. */
-  const char* name() const { return upb_oneofdef_name(ptr_); }
-
-  /* Returns the number of fields currently defined in the oneof. */
-  int field_count() const { return upb_oneofdef_numfields(ptr_); }
-
-  /* Looks up by name. */
-  FieldDefPtr FindFieldByName(const char *name, size_t len) const {
-    return FieldDefPtr(upb_oneofdef_ntof(ptr_, name, len));
-  }
-  FieldDefPtr FindFieldByName(const char* name) const {
-    return FieldDefPtr(upb_oneofdef_ntofz(ptr_, name));
-  }
-
-  template <class T>
-  FieldDefPtr FindFieldByName(const T& str) const {
-    return FindFieldByName(str.c_str(), str.size());
-  }
-
-  /* Looks up by tag number. */
-  FieldDefPtr FindFieldByNumber(uint32_t num) const {
-    return FieldDefPtr(upb_oneofdef_itof(ptr_, num));
-  }
-
-  class const_iterator
-      : public std::iterator<std::forward_iterator_tag, FieldDefPtr> {
-   public:
-    void operator++() { upb_oneof_next(&iter_); }
-
-    FieldDefPtr operator*() const {
-      return FieldDefPtr(upb_oneof_iter_field(&iter_));
-    }
-
-    bool operator!=(const const_iterator& other) const {
-      return !upb_oneof_iter_isequal(&iter_, &other.iter_);
-    }
-
-    bool operator==(const const_iterator& other) const {
-      return upb_oneof_iter_isequal(&iter_, &other.iter_);
-    }
-
-   private:
-    friend class OneofDefPtr;
-
-    const_iterator() {}
-    explicit const_iterator(OneofDefPtr o) {
-      upb_oneof_begin(&iter_, o.ptr());
-    }
-    static const_iterator end() {
-      const_iterator iter;
-      upb_oneof_iter_setdone(&iter.iter_);
-      return iter;
-    }
-
-    upb_oneof_iter iter_;
-  };
-
-  const_iterator begin() const { return const_iterator(*this); }
-  const_iterator end() const { return const_iterator::end(); }
-
- private:
-  const upb_oneofdef *ptr_;
-};
-
-inline upb::OneofDefPtr upb::FieldDefPtr::containing_oneof() const {
-  return OneofDefPtr(upb_fielddef_containingoneof(ptr_));
-}
-
-#endif  /* __cplusplus */
-
 /* upb_msgdef *****************************************************************/
 
 typedef upb_inttable_iter upb_msg_field_iter;
@@ -3587,26 +3241,21 @@ typedef upb_strtable_iter upb_msg_oneof_iter;
 #define UPB_TIMESTAMP_SECONDS 1
 #define UPB_TIMESTAMP_NANOS 2
 
-#ifdef __cplusplus
-extern "C" {
-#endif
-
 const char *upb_msgdef_fullname(const upb_msgdef *m);
 const upb_filedef *upb_msgdef_file(const upb_msgdef *m);
 const char *upb_msgdef_name(const upb_msgdef *m);
+int upb_msgdef_numfields(const upb_msgdef *m);
 int upb_msgdef_numoneofs(const upb_msgdef *m);
+int upb_msgdef_numrealoneofs(const upb_msgdef *m);
 upb_syntax_t upb_msgdef_syntax(const upb_msgdef *m);
 bool upb_msgdef_mapentry(const upb_msgdef *m);
 upb_wellknowntype_t upb_msgdef_wellknowntype(const upb_msgdef *m);
 bool upb_msgdef_isnumberwrapper(const upb_msgdef *m);
-bool upb_msgdef_setsyntax(upb_msgdef *m, upb_syntax_t syntax);
 const upb_fielddef *upb_msgdef_itof(const upb_msgdef *m, uint32_t i);
 const upb_fielddef *upb_msgdef_ntof(const upb_msgdef *m, const char *name,
                                     size_t len);
 const upb_oneofdef *upb_msgdef_ntoo(const upb_msgdef *m, const char *name,
                                     size_t len);
-int upb_msgdef_numfields(const upb_msgdef *m);
-int upb_msgdef_numoneofs(const upb_msgdef *m);
 const upb_msglayout *upb_msgdef_layout(const upb_msgdef *m);
 const upb_fielddef *_upb_msgdef_field(const upb_msgdef *m, int i);
 
@@ -3671,194 +3320,6 @@ void upb_msg_oneof_iter_setdone(upb_msg_oneof_iter * iter);
 bool upb_msg_oneof_iter_isequal(const upb_msg_oneof_iter *iter1,
                                 const upb_msg_oneof_iter *iter2);
 
-#ifdef __cplusplus
-}  /* extern "C" */
-
-/* Structure that describes a single .proto message type. */
-class upb::MessageDefPtr {
- public:
-  MessageDefPtr() : ptr_(nullptr) {}
-  explicit MessageDefPtr(const upb_msgdef *ptr) : ptr_(ptr) {}
-
-  const upb_msgdef *ptr() const { return ptr_; }
-  explicit operator bool() const { return ptr_ != nullptr; }
-
-  const char* full_name() const { return upb_msgdef_fullname(ptr_); }
-  const char* name() const { return upb_msgdef_name(ptr_); }
-
-  /* The number of fields that belong to the MessageDef. */
-  int field_count() const { return upb_msgdef_numfields(ptr_); }
-
-  /* The number of oneofs that belong to the MessageDef. */
-  int oneof_count() const { return upb_msgdef_numoneofs(ptr_); }
-
-  upb_syntax_t syntax() const { return upb_msgdef_syntax(ptr_); }
-
-  /* These return null pointers if the field is not found. */
-  FieldDefPtr FindFieldByNumber(uint32_t number) const {
-    return FieldDefPtr(upb_msgdef_itof(ptr_, number));
-  }
-  FieldDefPtr FindFieldByName(const char* name, size_t len) const {
-    return FieldDefPtr(upb_msgdef_ntof(ptr_, name, len));
-  }
-  FieldDefPtr FindFieldByName(const char *name) const {
-    return FieldDefPtr(upb_msgdef_ntofz(ptr_, name));
-  }
-
-  template <class T>
-  FieldDefPtr FindFieldByName(const T& str) const {
-    return FindFieldByName(str.c_str(), str.size());
-  }
-
-  OneofDefPtr FindOneofByName(const char* name, size_t len) const {
-    return OneofDefPtr(upb_msgdef_ntoo(ptr_, name, len));
-  }
-
-  OneofDefPtr FindOneofByName(const char *name) const {
-    return OneofDefPtr(upb_msgdef_ntooz(ptr_, name));
-  }
-
-  template <class T>
-  OneofDefPtr FindOneofByName(const T &str) const {
-    return FindOneofByName(str.c_str(), str.size());
-  }
-
-  /* Is this message a map entry? */
-  bool mapentry() const { return upb_msgdef_mapentry(ptr_); }
-
-  /* Return the type of well known type message. UPB_WELLKNOWN_UNSPECIFIED for
-   * non-well-known message. */
-  upb_wellknowntype_t wellknowntype() const {
-    return upb_msgdef_wellknowntype(ptr_);
-  }
-
-  /* Whether is a number wrapper. */
-  bool isnumberwrapper() const { return upb_msgdef_isnumberwrapper(ptr_); }
-
-  /* Iteration over fields.  The order is undefined. */
-  class const_field_iterator
-      : public std::iterator<std::forward_iterator_tag, FieldDefPtr> {
-   public:
-    void operator++() { upb_msg_field_next(&iter_); }
-
-    FieldDefPtr operator*() const {
-      return FieldDefPtr(upb_msg_iter_field(&iter_));
-    }
-
-    bool operator!=(const const_field_iterator &other) const {
-      return !upb_msg_field_iter_isequal(&iter_, &other.iter_);
-    }
-
-    bool operator==(const const_field_iterator &other) const {
-      return upb_msg_field_iter_isequal(&iter_, &other.iter_);
-    }
-
-   private:
-    friend class MessageDefPtr;
-
-    explicit const_field_iterator() {}
-
-    explicit const_field_iterator(MessageDefPtr msg) {
-      upb_msg_field_begin(&iter_, msg.ptr());
-    }
-
-    static const_field_iterator end() {
-      const_field_iterator iter;
-      upb_msg_field_iter_setdone(&iter.iter_);
-      return iter;
-    }
-
-    upb_msg_field_iter iter_;
-  };
-
-  /* Iteration over oneofs. The order is undefined. */
-  class const_oneof_iterator
-      : public std::iterator<std::forward_iterator_tag, OneofDefPtr> {
-   public:
-
-    void operator++() { upb_msg_oneof_next(&iter_); }
-
-    OneofDefPtr operator*() const {
-      return OneofDefPtr(upb_msg_iter_oneof(&iter_));
-    }
-
-    bool operator!=(const const_oneof_iterator& other) const {
-      return !upb_msg_oneof_iter_isequal(&iter_, &other.iter_);
-    }
-
-    bool operator==(const const_oneof_iterator &other) const {
-      return upb_msg_oneof_iter_isequal(&iter_, &other.iter_);
-    }
-
-   private:
-    friend class MessageDefPtr;
-
-    const_oneof_iterator() {}
-
-    explicit const_oneof_iterator(MessageDefPtr msg) {
-      upb_msg_oneof_begin(&iter_, msg.ptr());
-    }
-
-    static const_oneof_iterator end() {
-      const_oneof_iterator iter;
-      upb_msg_oneof_iter_setdone(&iter.iter_);
-      return iter;
-    }
-
-    upb_msg_oneof_iter iter_;
-  };
-
-  class ConstFieldAccessor {
-   public:
-    explicit ConstFieldAccessor(const upb_msgdef* md) : md_(md) {}
-    const_field_iterator begin() { return MessageDefPtr(md_).field_begin(); }
-    const_field_iterator end() { return MessageDefPtr(md_).field_end(); }
-   private:
-    const upb_msgdef* md_;
-  };
-
-  class ConstOneofAccessor {
-   public:
-    explicit ConstOneofAccessor(const upb_msgdef* md) : md_(md) {}
-    const_oneof_iterator begin() { return MessageDefPtr(md_).oneof_begin(); }
-    const_oneof_iterator end() { return MessageDefPtr(md_).oneof_end(); }
-   private:
-    const upb_msgdef* md_;
-  };
-
-  const_field_iterator field_begin() const {
-    return const_field_iterator(*this);
-  }
-
-  const_field_iterator field_end() const { return const_field_iterator::end(); }
-
-  const_oneof_iterator oneof_begin() const {
-    return const_oneof_iterator(*this);
-  }
-
-  const_oneof_iterator oneof_end() const { return const_oneof_iterator::end(); }
-
-  ConstFieldAccessor fields() const { return ConstFieldAccessor(ptr()); }
-  ConstOneofAccessor oneofs() const { return ConstOneofAccessor(ptr()); }
-
- private:
-  const upb_msgdef* ptr_;
-};
-
-inline upb::MessageDefPtr upb::FieldDefPtr::message_subdef() const {
-  return MessageDefPtr(upb_fielddef_msgsubdef(ptr_));
-}
-
-inline upb::MessageDefPtr upb::FieldDefPtr::containing_type() const {
-  return MessageDefPtr(upb_fielddef_containingtype(ptr_));
-}
-
-inline upb::MessageDefPtr upb::OneofDefPtr::containing_type() const {
-  return MessageDefPtr(upb_oneofdef_containingtype(ptr_));
-}
-
-#endif  /* __cplusplus */
-
 /* upb_enumdef ****************************************************************/
 
 typedef upb_strtable_iter upb_enum_iter;
@@ -3893,75 +3354,8 @@ 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);
 
-#ifdef __cplusplus
-
-class upb::EnumDefPtr {
- public:
-  EnumDefPtr() : ptr_(nullptr) {}
-  explicit EnumDefPtr(const upb_enumdef* ptr) : ptr_(ptr) {}
-
-  const upb_enumdef* ptr() const { return ptr_; }
-  explicit operator bool() const { return ptr_ != nullptr; }
-
-  const char* full_name() const { return upb_enumdef_fullname(ptr_); }
-  const char* name() const { return upb_enumdef_name(ptr_); }
-
-  /* The value that is used as the default when no field default is specified.
-   * If not set explicitly, the first value that was added will be used.
-   * The default value must be a member of the enum.
-   * Requires that value_count() > 0. */
-  int32_t default_value() const { return upb_enumdef_default(ptr_); }
-
-  /* Returns the number of values currently defined in the enum.  Note that
-   * multiple names can refer to the same number, so this may be greater than
-   * the total number of unique numbers. */
-  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);
-  }
-
-  /* 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);
-  }
-
-  /* Iteration over name/value pairs.  The order is undefined.
-   * Adding an enum val invalidates any iterators.
-   *
-   * TODO: make compatible with range-for, with elements as pairs? */
-  class Iterator {
-   public:
-    explicit Iterator(EnumDefPtr e) { upb_enum_begin(&iter_, e.ptr()); }
-
-    int32_t number() { return upb_enum_iter_number(&iter_); }
-    const char *name() { return upb_enum_iter_name(&iter_); }
-    bool Done() { return upb_enum_done(&iter_); }
-    void Next() { return upb_enum_next(&iter_); }
-
-   private:
-    upb_enum_iter iter_;
-  };
-
- private:
-  const upb_enumdef *ptr_;
-};
-
-inline upb::EnumDefPtr upb::FieldDefPtr::enum_subdef() const {
-  return EnumDefPtr(upb_fielddef_enumsubdef(ptr_));
-}
-
-#endif  /* __cplusplus */
-
 /* upb_filedef ****************************************************************/
 
-#ifdef __cplusplus
-extern "C" {
-#endif
-
 const char *upb_filedef_name(const upb_filedef *f);
 const char *upb_filedef_package(const upb_filedef *f);
 const char *upb_filedef_phpprefix(const upb_filedef *f);
@@ -3974,57 +3368,8 @@ const upb_filedef *upb_filedef_dep(const upb_filedef *f, int i);
 const upb_msgdef *upb_filedef_msg(const upb_filedef *f, int i);
 const upb_enumdef *upb_filedef_enum(const upb_filedef *f, int i);
 
-#ifdef __cplusplus
-}  /* extern "C" */
-
-/* Class that represents a .proto file with some things defined in it.
- *
- * Many users won't care about FileDefs, but they are necessary if you want to
- * read the values of file-level options. */
-class upb::FileDefPtr {
- public:
-  explicit FileDefPtr(const upb_filedef *ptr) : ptr_(ptr) {}
-
-  const upb_filedef* ptr() const { return ptr_; }
-  explicit operator bool() const { return ptr_ != nullptr; }
-
-  /* Get/set name of the file (eg. "foo/bar.proto"). */
-  const char* name() const { return upb_filedef_name(ptr_); }
-
-  /* Package name for definitions inside the file (eg. "foo.bar"). */
-  const char* package() const { return upb_filedef_package(ptr_); }
-
-  /* Sets the php class prefix which is prepended to all php generated classes
-   * from this .proto. Default is empty. */
-  const char* phpprefix() const { return upb_filedef_phpprefix(ptr_); }
-
-  /* Use this option to change the namespace of php generated classes. Default
-   * is empty. When this option is empty, the package name will be used for
-   * determining the namespace. */
-  const char* phpnamespace() const { return upb_filedef_phpnamespace(ptr_); }
-
-  /* Syntax for the file.  Defaults to proto2. */
-  upb_syntax_t syntax() const { return upb_filedef_syntax(ptr_); }
-
-  /* Get the list of dependencies from the file.  These are returned in the
-   * order that they were added to the FileDefPtr. */
-  int dependency_count() const { return upb_filedef_depcount(ptr_); }
-  const FileDefPtr dependency(int index) const {
-    return FileDefPtr(upb_filedef_dep(ptr_, index));
-  }
-
- private:
-  const upb_filedef* ptr_;
-};
-
-#endif  /* __cplusplus */
-
 /* upb_symtab *****************************************************************/
 
-#ifdef __cplusplus
-extern "C" {
-#endif
-
 upb_symtab *upb_symtab_new(void);
 void upb_symtab_free(upb_symtab* s);
 const upb_msgdef *upb_symtab_lookupmsg(const upb_symtab *s, const char *sym);
@@ -4047,53 +3392,11 @@ typedef struct upb_def_init {
 
 bool _upb_symtab_loaddefinit(upb_symtab *s, const upb_def_init *init);
 
+
 #ifdef __cplusplus
 }  /* extern "C" */
-
-/* Non-const methods in upb::SymbolTable are NOT thread-safe. */
-class upb::SymbolTable {
- public:
-  SymbolTable() : ptr_(upb_symtab_new(), upb_symtab_free) {}
-  explicit SymbolTable(upb_symtab* s) : ptr_(s, upb_symtab_free) {}
-
-  const upb_symtab* ptr() const { return ptr_.get(); }
-  upb_symtab* ptr() { return ptr_.get(); }
-
-  /* Finds an entry in the symbol table with this exact name.  If not found,
-   * returns NULL. */
-  MessageDefPtr LookupMessage(const char *sym) const {
-    return MessageDefPtr(upb_symtab_lookupmsg(ptr_.get(), sym));
-  }
-
-  EnumDefPtr LookupEnum(const char *sym) const {
-    return EnumDefPtr(upb_symtab_lookupenum(ptr_.get(), sym));
-  }
-
-  FileDefPtr LookupFile(const char *name) const {
-    return FileDefPtr(upb_symtab_lookupfile(ptr_.get(), name));
-  }
-
-  /* TODO: iteration? */
-
-  /* Adds the given serialized FileDescriptorProto to the pool. */
-  FileDefPtr AddFile(const google_protobuf_FileDescriptorProto *file_proto,
-                     Status *status) {
-    return FileDefPtr(
-        upb_symtab_addfile(ptr_.get(), file_proto, status->ptr()));
-  }
-
- private:
-  std::unique_ptr<upb_symtab, decltype(&upb_symtab_free)> ptr_;
-};
-
-UPB_INLINE const char* upb_safecstr(const std::string& str) {
-  UPB_ASSERT(str.size() == std::strlen(str.c_str()));
-  return str.c_str();
-}
-
 #endif  /* __cplusplus */
 
-
 #endif /* UPB_DEF_H_ */
 
 #ifndef UPB_REFLECTION_H_

+ 65 - 48
ruby/tests/basic.rb

@@ -32,11 +32,11 @@ module BasicTest
     include CommonTests
 
     def test_has_field
-      m = TestMessage.new
-      assert !m.has_optional_msg?
-      m.optional_msg = TestMessage2.new
-      assert m.has_optional_msg?
-      assert TestMessage.descriptor.lookup('optional_msg').has?(m)
+      m = TestSingularFields.new
+      assert !m.has_singular_msg?
+      m.singular_msg = TestMessage2.new
+      assert m.has_singular_msg?
+      assert TestSingularFields.descriptor.lookup('singular_msg').has?(m)
 
       m = OneofMessage.new
       assert !m.has_my_oneof?
@@ -45,32 +45,31 @@ module BasicTest
       assert_raise NoMethodError do
         m.has_a?
       end
-      assert_raise ArgumentError do
-        OneofMessage.descriptor.lookup('a').has?(m)
-      end
+      assert_true OneofMessage.descriptor.lookup('a').has?(m)
 
-      m = TestMessage.new
+      m = TestSingularFields.new
       assert_raise NoMethodError do
-        m.has_optional_int32?
+        m.has_singular_int32?
       end
       assert_raise ArgumentError do
-        TestMessage.descriptor.lookup('optional_int32').has?(m)
+        TestSingularFields.descriptor.lookup('singular_int32').has?(m)
       end
 
       assert_raise NoMethodError do
-        m.has_optional_string?
+        m.has_singular_string?
       end
       assert_raise ArgumentError do
-        TestMessage.descriptor.lookup('optional_string').has?(m)
+        TestSingularFields.descriptor.lookup('singular_string').has?(m)
       end
 
       assert_raise NoMethodError do
-        m.has_optional_bool?
+        m.has_singular_bool?
       end
       assert_raise ArgumentError do
-        TestMessage.descriptor.lookup('optional_bool').has?(m)
+        TestSingularFields.descriptor.lookup('singular_bool').has?(m)
       end
 
+      m = TestMessage.new
       assert_raise NoMethodError do
         m.has_repeated_msg?
       end
@@ -79,40 +78,59 @@ module BasicTest
       end
     end
 
+    def test_no_presence
+      m = TestSingularFields.new
+
+      # Explicitly setting to zero does not cause anything to be serialized.
+      m.singular_int32 = 0
+      assert_equal "", TestSingularFields.encode(m)
+
+      # Explicitly setting to a non-zero value *does* cause serialization.
+      m.singular_int32 = 1
+      assert_not_equal "", TestSingularFields.encode(m)
+
+      m.singular_int32 = 0
+      assert_equal "", TestSingularFields.encode(m)
+    end
+
     def test_set_clear_defaults
-      m = TestMessage.new
+      m = TestSingularFields.new
+
+      m.singular_int32 = -42
+      assert_equal -42, m.singular_int32
+      m.clear_singular_int32
+      assert_equal 0, m.singular_int32
+
+      m.singular_int32 = 50
+      assert_equal 50, m.singular_int32
+      TestSingularFields.descriptor.lookup('singular_int32').clear(m)
+      assert_equal 0, m.singular_int32
+
+      m.singular_string = "foo bar"
+      assert_equal "foo bar", m.singular_string
+      m.clear_singular_string
+      assert_equal "", m.singular_string
+
+      m.singular_string = "foo"
+      assert_equal "foo", m.singular_string
+      TestSingularFields.descriptor.lookup('singular_string').clear(m)
+      assert_equal "", m.singular_string
+
+      m.singular_msg = TestMessage2.new(:foo => 42)
+      assert_equal TestMessage2.new(:foo => 42), m.singular_msg
+      assert m.has_singular_msg?
+      m.clear_singular_msg
+      assert_equal nil, m.singular_msg
+      assert !m.has_singular_msg?
+
+      m.singular_msg = TestMessage2.new(:foo => 42)
+      assert_equal TestMessage2.new(:foo => 42), m.singular_msg
+      TestSingularFields.descriptor.lookup('singular_msg').clear(m)
+      assert_equal nil, m.singular_msg
+    end
 
-      m.optional_int32 = -42
-      assert_equal -42, m.optional_int32
-      m.clear_optional_int32
-      assert_equal 0, m.optional_int32
-
-      m.optional_int32 = 50
-      assert_equal 50, m.optional_int32
-      TestMessage.descriptor.lookup('optional_int32').clear(m)
-      assert_equal 0, m.optional_int32
-
-      m.optional_string = "foo bar"
-      assert_equal "foo bar", m.optional_string
-      m.clear_optional_string
-      assert_equal "", m.optional_string
-
-      m.optional_string = "foo"
-      assert_equal "foo", m.optional_string
-      TestMessage.descriptor.lookup('optional_string').clear(m)
-      assert_equal "", m.optional_string
-
-      m.optional_msg = TestMessage2.new(:foo => 42)
-      assert_equal TestMessage2.new(:foo => 42), m.optional_msg
-      assert m.has_optional_msg?
-      m.clear_optional_msg
-      assert_equal nil, m.optional_msg
-      assert !m.has_optional_msg?
-
-      m.optional_msg = TestMessage2.new(:foo => 42)
-      assert_equal TestMessage2.new(:foo => 42), m.optional_msg
-      TestMessage.descriptor.lookup('optional_msg').clear(m)
-      assert_equal nil, m.optional_msg
+    def test_clear_repeated_fields
+      m = TestMessage.new
 
       m.repeated_int32.push(1)
       assert_equal [1], m.repeated_int32
@@ -144,7 +162,6 @@ module BasicTest
       assert !m.has_my_oneof?
     end
 
-
     def test_initialization_map_errors
       e = assert_raise ArgumentError do
         TestMessage.new(:hello => "world")

+ 25 - 11
ruby/tests/basic_test.proto

@@ -21,17 +21,17 @@ message Baz {
 }
 
 message TestMessage {
-  int32 optional_int32 = 1;
-  int64 optional_int64 = 2;
-  uint32 optional_uint32 = 3;
-  uint64 optional_uint64 = 4;
-  bool optional_bool = 5;
-  float optional_float = 6;
-  double optional_double = 7;
-  string optional_string = 8;
-  bytes optional_bytes = 9;
-  TestMessage2 optional_msg = 10;
-  TestEnum optional_enum = 11;
+  optional int32 optional_int32 = 1;
+  optional int64 optional_int64 = 2;
+  optional uint32 optional_uint32 = 3;
+  optional uint64 optional_uint64 = 4;
+  optional bool optional_bool = 5;
+  optional float optional_float = 6;
+  optional double optional_double = 7;
+  optional string optional_string = 8;
+  optional bytes optional_bytes = 9;
+  optional TestMessage2 optional_msg = 10;
+  optional TestEnum optional_enum = 11;
 
   repeated int32 repeated_int32 = 12;
   repeated int64 repeated_int64 = 13;
@@ -46,6 +46,20 @@ message TestMessage {
   repeated TestEnum repeated_enum = 22;
 }
 
+message TestSingularFields {
+  int32 singular_int32 = 1;
+  int64 singular_int64 = 2;
+  uint32 singular_uint32 = 3;
+  uint64 singular_uint64 = 4;
+  bool singular_bool = 5;
+  float singular_float = 6;
+  double singular_double = 7;
+  string singular_string = 8;
+  bytes singular_bytes = 9;
+  TestMessage2 singular_msg = 10;
+  TestEnum singular_enum = 11;
+}
+
 message TestMessage2 {
   int32 foo = 1;
 }

+ 6 - 2
src/google/protobuf/compiler/ruby/ruby_generator.cc

@@ -77,6 +77,10 @@ std::string GetOutputFilename(const std::string& proto_file) {
 }
 
 std::string LabelForField(const FieldDescriptor* field) {
+  if (field->has_optional_keyword() &&
+      field->file()->syntax() == FileDescriptor::SYNTAX_PROTO3) {
+    return "proto3_optional";
+  }
   switch (field->label()) {
     case FieldDescriptor::LABEL_OPTIONAL: return "optional";
     case FieldDescriptor::LABEL_REQUIRED: return "required";
@@ -255,12 +259,12 @@ bool GenerateMessage(const Descriptor* message, io::Printer* printer,
 
   for (int i = 0; i < message->field_count(); i++) {
     const FieldDescriptor* field = message->field(i);
-    if (!field->containing_oneof()) {
+    if (!field->real_containing_oneof()) {
       GenerateField(field, printer);
     }
   }
 
-  for (int i = 0; i < message->oneof_decl_count(); i++) {
+  for (int i = 0; i < message->real_oneof_decl_count(); i++) {
     const OneofDescriptor* oneof = message->oneof_decl(i);
     GenerateOneof(oneof, printer);
   }

+ 6 - 5
src/google/protobuf/compiler/ruby/ruby_generator.h

@@ -49,11 +49,12 @@ namespace ruby {
 // Ruby output, you can do so by registering an instance of this
 // CodeGenerator with the CommandLineInterface in your main() function.
 class PROTOC_EXPORT Generator : public CodeGenerator {
-  virtual bool Generate(
-      const FileDescriptor* file,
-      const string& parameter,
-      GeneratorContext* generator_context,
-      string* error) const;
+  bool Generate(const FileDescriptor* file, const string& parameter,
+                GeneratorContext* generator_context,
+                string* error) const override;
+  uint64 GetSupportedFeatures() const override {
+    return FEATURE_PROTO3_OPTIONAL;
+  }
 };
 
 }  // namespace ruby