Merge pull request #316 An error occurred from haberman/submsg-array

Fixed bug where submsg array could have excess elements.

generated_for_cmake/google/protobuf/descriptor.upb.c

@@ -56,7 +56,7 @@ const upb_msglayout google_protobuf_FileDescriptorProto_msginit = {
   UPB_SIZE(64, 128), 12, false,
 };
 
-static const upb_msglayout *const google_protobuf_DescriptorProto_submsgs[8] = {
+static const upb_msglayout *const google_protobuf_DescriptorProto_submsgs[7] = {
   &google_protobuf_DescriptorProto_msginit,
   &google_protobuf_DescriptorProto_ExtensionRange_msginit,
   &google_protobuf_DescriptorProto_ReservedRange_msginit,

upbc/generator.cc

@@ -703,6 +703,40 @@ int TableDescriptorType(const protobuf::FieldDescriptor* field) {
   }
 }
 
+struct SubmsgArray {
+ public:
+  SubmsgArray(const protobuf::Descriptor* message) : message_(message) {
+    MessageLayout layout(message);
+    std::vector<const protobuf::FieldDescriptor*> sorted_submsgs =
+        SortedSubmessages(message);
+    int i = 0;
+    for (auto submsg : sorted_submsgs) {
+      if (indexes_.find(submsg->message_type()) != indexes_.end()) {
+        continue;
+      }
+      submsgs_.push_back(submsg->message_type());
+      indexes_[submsg->message_type()] = i++;
+    }
+  }
+
+  const std::vector<const protobuf::Descriptor*>& submsgs() const {
+    return submsgs_;
+  }
+
+  int GetIndex(const protobuf::FieldDescriptor* field) {
+    (void)message_;
+    assert(field->containing_type() == message_);
+    auto it = indexes_.find(field->message_type());
+    assert(it != indexes_.end());
+    return it->second;
+  }
+
+ private:
+  const protobuf::Descriptor* message_;
+  std::vector<const protobuf::Descriptor*> submsgs_;
+  absl::flat_hash_map<const protobuf::Descriptor*, int> indexes_;
+};
+
 void WriteSource(const protobuf::FileDescriptor* file, Output& output) {
   EmitFileWarning(file, output);
 
@@ -726,27 +760,19 @@ void WriteSource(const protobuf::FileDescriptor* file, Output& output) {
     std::string msgname = ToCIdent(message->full_name());
     std::string fields_array_ref = "NULL";
     std::string submsgs_array_ref = "NULL";
-    absl::flat_hash_map<const protobuf::Descriptor*, int> submsg_indexes;
     MessageLayout layout(message);
-    std::vector<const protobuf::FieldDescriptor*> sorted_submsgs =
+    SubmsgArray submsg_array(message);
-        SortedSubmessages(message);
 
-    if (!sorted_submsgs.empty()) {
+    if (!submsg_array.submsgs().empty()) {
       // TODO(haberman): could save a little bit of space by only generating a
       // "submsgs" array for every strongly-connected component.
       std::string submsgs_array_name = msgname + "_submsgs";
       submsgs_array_ref = "&" + submsgs_array_name + "[0]";
       output("static const upb_msglayout *const $0[$1] = {\n",
-             submsgs_array_name, sorted_submsgs.size());
+             submsgs_array_name, submsg_array.submsgs().size());
 
-      int i = 0;
+      for (auto submsg : submsg_array.submsgs()) {
-      for (auto submsg : sorted_submsgs) {
+        output("  &$0,\n", MessageInit(submsg));
-        if (submsg_indexes.find(submsg->message_type()) !=
-            submsg_indexes.end()) {
-          continue;
-        }
-        output("  &$0,\n", MessageInit(submsg->message_type()));
-        submsg_indexes[submsg->message_type()] = i++;
       }
 
       output("};\n\n");
@@ -764,7 +790,7 @@ void WriteSource(const protobuf::FileDescriptor* file, Output& output) {
         std::string presence = "0";
 
         if (field->cpp_type() == protobuf::FieldDescriptor::CPPTYPE_MESSAGE) {
-          submsg_index = submsg_indexes[field->message_type()];
+          submsg_index = submsg_array.GetIndex(field);
         }
 
         if (MessageLayout::HasHasbit(field)) {