Enabled editions support for upb generated code.

This required enabling the feature in the code generator and fixing a few edge cases around label and type.

Also added tests to verify the special cases, and to verify that required fields work as expected.

PiperOrigin-RevId: 580263087

src/google/protobuf/compiler/allowlists/editions.cc

@@ -20,6 +20,7 @@ static constexpr auto kEarlyEditionsFile = internal::MakeAllowlist(
     {
 // Intentionally left blank.
         "google/protobuf/",
+        "upb/",
     },
     internal::AllowlistFlags::kMatchPrefix);
 

upb/reflection/field_def.c

@@ -123,11 +123,26 @@ upb_CType upb_FieldDef_CType(const upb_FieldDef* f) {
   UPB_UNREACHABLE();
 }
 
-upb_FieldType upb_FieldDef_Type(const upb_FieldDef* f) { return f->type_; }
+upb_FieldType upb_FieldDef_Type(const upb_FieldDef* f) {
+  // TODO: remove once we can deprecate kUpb_FieldType_Group.
+  if (f->type_ == kUpb_FieldType_Message &&
+      UPB_DESC(FeatureSet_message_encoding)(f->resolved_features) ==
+          UPB_DESC(FeatureSet_DELIMITED)) {
+    return kUpb_FieldType_Group;
+  }
+  return f->type_;
+}
 
 uint32_t upb_FieldDef_Index(const upb_FieldDef* f) { return f->index_; }
 
-upb_Label upb_FieldDef_Label(const upb_FieldDef* f) { return f->label_; }
+upb_Label upb_FieldDef_Label(const upb_FieldDef* f) {
+  // TODO: remove once we can deprecate kUpb_Label_Required.
+  if (UPB_DESC(FeatureSet_field_presence)(f->resolved_features) ==
+      UPB_DESC(FeatureSet_LEGACY_REQUIRED)) {
+    return kUpb_Label_Required;
+  }
+  return f->label_;
+}
 
 uint32_t upb_FieldDef_Number(const upb_FieldDef* f) { return f->number_; }
 

upb/test/BUILD

@@ -84,6 +84,24 @@ upb_c_proto_library(
     deps = [":test_proto"],
 )
 
+proto_library(
+    name = "editions_test_proto",
+    testonly = 1,
+    srcs = ["editions_test.proto"],
+)
+
+upb_c_proto_library(
+    name = "editions_test_upb_c_proto",
+    testonly = 1,
+    deps = [":editions_test_proto"],
+)
+
+upb_proto_reflection_library(
+    name = "editions_test_upb_proto_reflection",
+    testonly = 1,
+    deps = [":editions_test_proto"],
+)
+
 proto_library(
     name = "test_cpp_proto",
     srcs = ["test_cpp.proto"],
@@ -166,6 +184,20 @@ cc_test(
     ],
 )
 
+cc_test(
+    name = "editions_test",
+    srcs = ["editions_test.cc"],
+    copts = UPB_DEFAULT_CPPOPTS,
+    deps = [
+        ":editions_test_upb_c_proto",
+        ":editions_test_upb_proto_reflection",
+        "@com_google_googletest//:gtest_main",
+        "//upb:base",
+        "//upb:mem",
+        "//upb:reflection",
+    ],
+)
+
 cc_test(
     name = "test_cpp",
     srcs = ["test_cpp.cc"],

upb/test/editions_test.cc

@@ -0,0 +1,62 @@
+// Protocol Buffers - Google's data interchange format
+// Copyright 2023 Google LLC.  All rights reserved.
+//
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file or at
+// https://developers.google.com/open-source/licenses/bsd
+
+#include <gtest/gtest.h>
+#include "upb/base/descriptor_constants.h"
+#include "upb/mem/arena.hpp"
+#include "upb/reflection/def.hpp"
+#include "upb/test/editions_test.upb.h"
+#include "upb/test/editions_test.upbdefs.h"
+
+TEST(EditionsTest, PlainField) {
+  upb::DefPool defpool;
+  upb::MessageDefPtr md(upb_test_2023_EditionsMessage_getmsgdef(defpool.ptr()));
+  upb::FieldDefPtr f(md.FindFieldByName("plain_field"));
+  EXPECT_TRUE(f.has_presence());
+}
+
+TEST(EditionsTest, ImplicitPresenceField) {
+  upb::DefPool defpool;
+  upb::MessageDefPtr md(upb_test_2023_EditionsMessage_getmsgdef(defpool.ptr()));
+  upb::FieldDefPtr f(md.FindFieldByName("implicit_presence_field"));
+  EXPECT_FALSE(f.has_presence());
+}
+
+TEST(EditionsTest, DelimitedField) {
+  upb::DefPool defpool;
+  upb::MessageDefPtr md(upb_test_2023_EditionsMessage_getmsgdef(defpool.ptr()));
+  upb::FieldDefPtr f(md.FindFieldByName("delimited_field"));
+  EXPECT_EQ(kUpb_FieldType_Group, f.type());
+}
+
+TEST(EditionsTest, RequiredField) {
+  upb::DefPool defpool;
+  upb::MessageDefPtr md(upb_test_2023_EditionsMessage_getmsgdef(defpool.ptr()));
+  upb::FieldDefPtr f(md.FindFieldByName("required_field"));
+  EXPECT_EQ(kUpb_Label_Required, f.label());
+}
+
+TEST(EditionsTest, ClosedEnum) {
+  upb::DefPool defpool;
+  upb::MessageDefPtr md(upb_test_2023_EditionsMessage_getmsgdef(defpool.ptr()));
+  upb::FieldDefPtr f(md.FindFieldByName("enum_field"));
+  ASSERT_TRUE(f.enum_subdef().is_closed());
+}
+
+TEST(EditionsTest, PackedField) {
+  upb::DefPool defpool;
+  upb::MessageDefPtr md(upb_test_2023_EditionsMessage_getmsgdef(defpool.ptr()));
+  upb::FieldDefPtr f(md.FindFieldByName("unpacked_field"));
+  ASSERT_FALSE(f.packed());
+}
+
+TEST(EditionsTest, ConstructProto) {
+  // Doesn't do anything except construct the proto. This just verifies that
+  // the generated code compiles successfully.
+  upb::Arena arena;
+  upb_test_2023_EditionsMessage_new(arena.ptr());
+}

upb/test/editions_test.proto

@@ -0,0 +1,26 @@
+// Protocol Buffers - Google's data interchange format
+// Copyright 2023 Google LLC.  All rights reserved.
+//
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file or at
+// https://developers.google.com/open-source/licenses/bsd
+
+edition = "2023";
+
+package upb.test_2023;
+
+message EditionsMessage {
+  int32 plain_field = 1;
+  int32 implicit_presence_field = 2 [features.field_presence = IMPLICIT];
+  int32 required_field = 3 [features.field_presence = LEGACY_REQUIRED];
+  EditionsMessage delimited_field = 4 [features.message_encoding = DELIMITED];
+  EditionsEnum enum_field = 5;
+  repeated int32 unpacked_field = 6
+      [features.repeated_field_encoding = EXPANDED];
+}
+
+enum EditionsEnum {
+  option features.enum_type = CLOSED;
+
+  ONE = 1;
+}

upb/util/BUILD

@@ -106,7 +106,10 @@ cc_library(
 
 proto_library(
     name = "required_fields_test_proto",
-    srcs = ["required_fields_test.proto"],
+    srcs = [
+        "required_fields_editions_test.proto",
+        "required_fields_test.proto",
+    ],
 )
 
 upb_c_proto_library(
@@ -131,6 +134,7 @@ cc_test(
         "//upb:json",
         "//upb:mem",
         "//upb:reflection",
+        "//upb:reflection_internal",
         "@com_google_absl//absl/strings",
     ],
 )

upb/util/def_to_proto.c

@@ -511,6 +511,8 @@ static google_protobuf_FileDescriptorProto* filedef_toproto(upb_ToProto_Context*
 
   if (upb_FileDef_Syntax(f) == kUpb_Syntax_Proto3) {
     google_protobuf_FileDescriptorProto_set_syntax(proto, strviewdup(ctx, "proto3"));
+  } else if (upb_FileDef_Syntax(f) == kUpb_Syntax_Editions) {
+    google_protobuf_FileDescriptorProto_set_syntax(proto, strviewdup(ctx, "editions"));
   }
 
   size_t n;

upb/util/required_fields_editions_test.proto

@@ -0,0 +1,28 @@
+// Protocol Buffers - Google's data interchange format
+// Copyright 2023 Google LLC.  All rights reserved.
+//
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file or at
+// https://developers.google.com/open-source/licenses/bsd
+
+edition = "2023";
+
+package upb_util_2023_test;
+
+message EmptyMessage {}
+
+message HasRequiredField {
+  int32 required_int32 = 1 [features.field_presence = LEGACY_REQUIRED];
+}
+
+message TestRequiredFields {
+  EmptyMessage required_message = 1 [features.field_presence = LEGACY_REQUIRED];
+  TestRequiredFields optional_message = 2;
+  repeated HasRequiredField repeated_message = 3;
+  map<int32, HasRequiredField> map_int32_message = 4;
+  map<int64, HasRequiredField> map_int64_message = 5;
+  map<uint32, HasRequiredField> map_uint32_message = 6;
+  map<uint64, HasRequiredField> map_uint64_message = 7;
+  map<bool, HasRequiredField> map_bool_message = 8;
+  map<string, HasRequiredField> map_string_message = 9;
+}

upb/util/required_fields_test.cc

@@ -9,13 +9,16 @@
 
 #include <stdlib.h>
 
-#include <gmock/gmock.h>
 #include <gtest/gtest.h>
 #include "absl/strings/string_view.h"
 #include "upb/base/status.hpp"
 #include "upb/json/decode.h"
+#include "upb/mem/arena.h"
 #include "upb/mem/arena.hpp"
+#include "upb/reflection/common.h"
 #include "upb/reflection/def.hpp"
+#include "upb/util/required_fields_editions_test.upb.h"
+#include "upb/util/required_fields_editions_test.upbdefs.h"
 #include "upb/util/required_fields_test.upb.h"
 #include "upb/util/required_fields_test.upbdefs.h"
 
@@ -40,31 +43,61 @@ std::vector<std::string> PathsToText(upb_FieldPathEntry* entry) {
   return ret;
 }
 
-void CheckRequired(absl::string_view json,
-                   const std::vector<std::string>& missing) {
-  upb::Arena arena;
-  upb::DefPool defpool;
-  upb_util_test_TestRequiredFields* test_msg =
-      upb_util_test_TestRequiredFields_new(arena.ptr());
-  upb::MessageDefPtr m(
-      upb_util_test_TestRequiredFields_getmsgdef(defpool.ptr()));
-  upb::Status status;
-  EXPECT_TRUE(upb_JsonDecode(json.data(), json.size(), test_msg, m.ptr(),
-                             defpool.ptr(), 0, arena.ptr(), status.ptr()))
-      << status.error_message();
-  upb_FieldPathEntry* entries = nullptr;
-  EXPECT_EQ(!missing.empty(), upb_util_HasUnsetRequired(
-                                  test_msg, m.ptr(), defpool.ptr(), &entries));
-  if (entries) {
-    EXPECT_EQ(missing, PathsToText(entries));
-    free(entries);
+template <typename T>
+class RequiredFieldsTest : public testing::Test {
+ public:
+  void CheckRequired(absl::string_view json,
+                     const std::vector<std::string>& missing) {
+    upb::Arena arena;
+    upb::DefPool defpool;
+    auto* test_msg = T::NewMessage(arena.ptr());
+    upb::MessageDefPtr m = T::GetMessageDef(defpool.ptr());
+    upb::Status status;
+    EXPECT_TRUE(upb_JsonDecode(json.data(), json.size(), test_msg, m.ptr(),
+                               defpool.ptr(), 0, arena.ptr(), status.ptr()))
+        << status.error_message();
+    upb_FieldPathEntry* entries = nullptr;
+    EXPECT_EQ(
+        !missing.empty(),
+        upb_util_HasUnsetRequired(test_msg, m.ptr(), defpool.ptr(), &entries));
+    if (entries) {
+      EXPECT_EQ(missing, PathsToText(entries));
+      free(entries);
+    }
+
+    // Verify that we can pass a NULL pointer to entries when we don't care
+    // about them.
+    EXPECT_EQ(!missing.empty(), upb_util_HasUnsetRequired(
+                                    test_msg, m.ptr(), defpool.ptr(), nullptr));
   }
+};
 
-  // Verify that we can pass a NULL pointer to entries when we don't care about
-  // them.
-  EXPECT_EQ(!missing.empty(), upb_util_HasUnsetRequired(
-                                  test_msg, m.ptr(), defpool.ptr(), nullptr));
-}
+class Proto2Type {
+ public:
+  using MessageType = upb_util_test_TestRequiredFields;
+  static MessageType* NewMessage(upb_Arena* arena) {
+    return upb_util_test_TestRequiredFields_new(arena);
+  }
+  static upb::MessageDefPtr GetMessageDef(upb_DefPool* defpool) {
+    return upb::MessageDefPtr(
+        upb_util_test_TestRequiredFields_getmsgdef(defpool));
+  }
+};
+
+class Edition2023Type {
+ public:
+  using MessageType = upb_util_2023_test_TestRequiredFields;
+  static MessageType* NewMessage(upb_Arena* arena) {
+    return upb_util_2023_test_TestRequiredFields_new(arena);
+  }
+  static upb::MessageDefPtr GetMessageDef(upb_DefPool* defpool) {
+    return upb::MessageDefPtr(
+        upb_util_2023_test_TestRequiredFields_getmsgdef(defpool));
+  }
+};
+
+using MyTypes = ::testing::Types<Proto2Type, Edition2023Type>;
+TYPED_TEST_SUITE(RequiredFieldsTest, MyTypes);
 
 // message HasRequiredField {
 //   required int32 required_int32 = 1;
@@ -76,10 +109,10 @@ void CheckRequired(absl::string_view json,
 //   repeated HasRequiredField repeated_message = 3;
 //   map<int32, HasRequiredField> map_int32_message = 4;
 // }
-TEST(RequiredFieldsTest, TestRequired) {
-  CheckRequired(R"json({})json", {"required_message"});
-  CheckRequired(R"json({"required_message": {}}")json", {});
-  CheckRequired(
+TYPED_TEST(RequiredFieldsTest, TestRequired) {
+  TestFixture::CheckRequired(R"json({})json", {"required_message"});
+  TestFixture::CheckRequired(R"json({"required_message": {}}")json", {});
+  TestFixture::CheckRequired(
       R"json(
       {
         "optional_message": {}
@@ -88,7 +121,7 @@ TEST(RequiredFieldsTest, TestRequired) {
       {"required_message", "optional_message.required_message"});
 
   // Repeated field.
-  CheckRequired(
+  TestFixture::CheckRequired(
       R"json(
       {
         "optional_message": {
@@ -104,7 +137,7 @@ TEST(RequiredFieldsTest, TestRequired) {
        "optional_message.repeated_message[1].required_int32"});
 
   // Int32 map key.
-  CheckRequired(
+  TestFixture::CheckRequired(
       R"json(
       {
         "required_message": {},
@@ -118,7 +151,7 @@ TEST(RequiredFieldsTest, TestRequired) {
       {"map_int32_message[5].required_int32"});
 
   // Int64 map key.
-  CheckRequired(
+  TestFixture::CheckRequired(
       R"json(
       {
         "required_message": {},
@@ -132,7 +165,7 @@ TEST(RequiredFieldsTest, TestRequired) {
       {"map_int64_message[5].required_int32"});
 
   // Uint32 map key.
-  CheckRequired(
+  TestFixture::CheckRequired(
       R"json(
       {
         "required_message": {},
@@ -146,7 +179,7 @@ TEST(RequiredFieldsTest, TestRequired) {
       {"map_uint32_message[5].required_int32"});
 
   // Uint64 map key.
-  CheckRequired(
+  TestFixture::CheckRequired(
       R"json(
       {
         "required_message": {},
@@ -160,7 +193,7 @@ TEST(RequiredFieldsTest, TestRequired) {
       {"map_uint64_message[5].required_int32"});
 
   // Bool map key.
-  CheckRequired(
+  TestFixture::CheckRequired(
       R"json(
       {
         "required_message": {},
@@ -173,7 +206,7 @@ TEST(RequiredFieldsTest, TestRequired) {
       {"map_bool_message[true].required_int32"});
 
   // String map key.
-  CheckRequired(
+  TestFixture::CheckRequired(
       R"json(
       {
         "required_message": {},

upb_generator/file_layout.cc

@@ -32,8 +32,10 @@
 
 #include <string>
 #include <unordered_set>
+#include <vector>
 
 #include "upb/mini_table/internal/extension.h"
+#include "upb/reflection/def.hpp"
 #include "upb_generator/common.h"
 
 namespace upb {
@@ -43,24 +45,32 @@ const char* kEnumsInit = "enums_layout";
 const char* kExtensionsInit = "extensions_layout";
 const char* kMessagesInit = "messages_layout";
 
-void AddEnums(upb::MessageDefPtr message, std::vector<upb::EnumDefPtr>* enums) {
+void AddEnums(upb::MessageDefPtr message, std::vector<upb::EnumDefPtr>* enums,
+              WhichEnums which) {
   enums->reserve(enums->size() + message.enum_type_count());
   for (int i = 0; i < message.enum_type_count(); i++) {
-    enums->push_back(message.enum_type(i));
+    upb::EnumDefPtr enum_type = message.enum_type(i);
+    if (which == kAllEnums || enum_type.is_closed()) {
+      enums->push_back(message.enum_type(i));
+    }
   }
   for (int i = 0; i < message.nested_message_count(); i++) {
-    AddEnums(message.nested_message(i), enums);
+    AddEnums(message.nested_message(i), enums, which);
   }
 }
 
-std::vector<upb::EnumDefPtr> SortedEnums(upb::FileDefPtr file) {
+std::vector<upb::EnumDefPtr> SortedEnums(upb::FileDefPtr file,
+                                         WhichEnums which) {
   std::vector<upb::EnumDefPtr> enums;
   enums.reserve(file.toplevel_enum_count());
   for (int i = 0; i < file.toplevel_enum_count(); i++) {
-    enums.push_back(file.toplevel_enum(i));
+    upb::EnumDefPtr top_level_enum = file.toplevel_enum(i);
+    if (which == kAllEnums || top_level_enum.is_closed()) {
+      enums.push_back(file.toplevel_enum(i));
+    }
   }
   for (int i = 0; i < file.toplevel_message_count(); i++) {
-    AddEnums(file.toplevel_message(i), &enums);
+    AddEnums(file.toplevel_message(i), &enums, which);
   }
   std::sort(enums.begin(), enums.end(),
             [](upb::EnumDefPtr a, upb::EnumDefPtr b) {

upb_generator/file_layout.h

@@ -57,7 +57,13 @@
 namespace upb {
 namespace generator {
 
-std::vector<upb::EnumDefPtr> SortedEnums(upb::FileDefPtr file);
+enum WhichEnums {
+  kAllEnums = 0,
+  kClosedEnums = 1,
+};
+
+std::vector<upb::EnumDefPtr> SortedEnums(upb::FileDefPtr file,
+                                         WhichEnums which);
 
 // Ordering must match upb/def.c!
 //

upb_generator/plugin.h

@@ -187,9 +187,12 @@ class Plugin {
       ABSL_LOG(FATAL) << "Failed to parse CodeGeneratorRequest";
     }
     response_ = UPB_DESC(compiler_CodeGeneratorResponse_new)(arena_.ptr());
+
+    int features =
+        UPB_DESC(compiler_CodeGeneratorResponse_FEATURE_PROTO3_OPTIONAL) |
+        UPB_DESC(compiler_CodeGeneratorResponse_FEATURE_SUPPORTS_EDITIONS);
     UPB_DESC(compiler_CodeGeneratorResponse_set_supported_features)
-    (response_,
-     UPB_DESC(compiler_CodeGeneratorResponse_FEATURE_PROTO3_OPTIONAL));
+    (response_, features);
   }
 
   void WriteResponse() {

upb_generator/protoc-gen-upb.cc

@@ -868,7 +868,7 @@ void WriteHeader(const DefPoolPair& pools, upb::FileDefPtr file,
   const std::vector<upb::MessageDefPtr> this_file_messages =
       SortedMessages(file);
   const std::vector<upb::FieldDefPtr> this_file_exts = SortedExtensions(file);
-  std::vector<upb::EnumDefPtr> this_file_enums = SortedEnums(file);
+  std::vector<upb::EnumDefPtr> this_file_enums = SortedEnums(file, kAllEnums);
   std::vector<upb::MessageDefPtr> forward_messages =
       SortedForwardMessages(this_file_messages, this_file_exts);
 
@@ -1097,7 +1097,7 @@ void WriteMiniDescriptorSource(const DefPoolPair& pools, upb::FileDefPtr file,
     WriteMessageMiniDescriptorInitializer(msg, options, output);
   }
 
-  for (const auto msg : SortedEnums(file)) {
+  for (const auto msg : SortedEnums(file, kClosedEnums)) {
     WriteEnumMiniDescriptorInitializer(msg, options, output);
   }
 }

upb_generator/protoc-gen-upb_minitable.cc

@@ -145,12 +145,11 @@ void WriteHeader(const DefPoolPair& pools, upb::FileDefPtr file,
 
   output("\n");
 
-  std::vector<upb::EnumDefPtr> this_file_enums = SortedEnums(file);
+  std::vector<upb::EnumDefPtr> this_file_enums =
+      SortedEnums(file, kClosedEnums);
 
-  if (file.syntax() == kUpb_Syntax_Proto2) {
-    for (const auto enumdesc : this_file_enums) {
-      output("extern const upb_MiniTableEnum $0;\n", EnumInit(enumdesc));
-    }
+  for (const auto enumdesc : this_file_enums) {
+    output("extern const upb_MiniTableEnum $0;\n", EnumInit(enumdesc));
   }
 
   output("extern const upb_MiniTableFile $0;\n\n", FileLayoutName(file));
@@ -536,9 +535,8 @@ void WriteEnum(upb::EnumDefPtr e, Output& output) {
 }
 
 int WriteEnums(const DefPoolPair& pools, upb::FileDefPtr file, Output& output) {
-  if (file.syntax() != kUpb_Syntax_Proto2) return 0;
-
-  std::vector<upb::EnumDefPtr> this_file_enums = SortedEnums(file);
+  std::vector<upb::EnumDefPtr> this_file_enums =
+      SortedEnums(file, kClosedEnums);
 
   for (const auto e : this_file_enums) {
     WriteEnum(e, output);