Skip to content

Commit 3419598

Browse files
protobuf-github-botshaod2
protobuf-github-bot
authored and
shaod2
committed
Support allowing late injection of language feature set defaults from FeatureSet extensions while getting feature set extension values.
PiperOrigin-RevId: 750272602
1 parent 0fe099a commit 3419598

22 files changed

+698
-91
lines changed

‎editions/generated_files_test.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include "editions/golden/test_messages_proto2_editions.pb.h"
1313
#include "editions/golden/test_messages_proto3_editions.pb.h"
1414
#include "editions/proto/test_editions_default_features.pb.h"
15+
#include "google/protobuf/internal_feature_helper.h"
1516
#include "google/protobuf/test_textproto.h"
1617

1718
// These tests provide some basic minimal coverage that protos work as expected.

‎python/google/protobuf/pyext/descriptor.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include "absl/strings/string_view.h"
2424
#include "google/protobuf/descriptor.h"
2525
#include "google/protobuf/dynamic_message.h"
26+
#include "google/protobuf/internal_feature_helper.h"
2627
#include "google/protobuf/io/coded_stream.h"
2728
#include "google/protobuf/pyext/descriptor_containers.h"
2829
#include "google/protobuf/pyext/descriptor_pool.h"

‎src/google/protobuf/BUILD.bazel

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -718,6 +718,7 @@ PROTOBUF_HEADERS = [
718718
"descriptor_visitor.h",
719719
"dynamic_message.h",
720720
"feature_resolver.h",
721+
"internal_feature_helper.h",
721722
"field_access_listener.h",
722723
"generated_enum_reflection.h",
723724
"generated_message_bases.h",
@@ -754,6 +755,7 @@ cc_library(
754755
"generated_message_reflection.cc",
755756
"generated_message_tctable_full.cc",
756757
"generated_message_tctable_gen.cc",
758+
"internal_feature_helper.cc",
757759
"map_field.cc",
758760
"message.cc",
759761
"reflection_mode.cc",
@@ -1704,6 +1706,37 @@ cc_test(
17041706
],
17051707
)
17061708

1709+
cc_test(
1710+
name = "internal_feature_helper_test",
1711+
srcs = [
1712+
"internal_feature_helper_test.cc",
1713+
],
1714+
copts = COPTS,
1715+
deps = [
1716+
":cc_test_protos",
1717+
":port",
1718+
":protobuf",
1719+
":protobuf_lite",
1720+
":test_textproto",
1721+
":test_util",
1722+
"//src/google/protobuf/compiler:importer",
1723+
"//src/google/protobuf/io",
1724+
"//src/google/protobuf/io:tokenizer",
1725+
"//src/google/protobuf/testing",
1726+
"//src/google/protobuf/testing:file",
1727+
"@abseil-cpp//absl/log:absl_check",
1728+
"@abseil-cpp//absl/log:absl_log",
1729+
"@abseil-cpp//absl/log:die_if_null",
1730+
"@abseil-cpp//absl/memory",
1731+
"@abseil-cpp//absl/status",
1732+
"@abseil-cpp//absl/status:statusor",
1733+
"@abseil-cpp//absl/strings",
1734+
"@abseil-cpp//absl/strings:str_format",
1735+
"@googletest//:gtest",
1736+
"@googletest//:gtest_main",
1737+
],
1738+
)
1739+
17071740
cc_test(
17081741
name = "generated_message_reflection_unittest",
17091742
srcs = ["generated_message_reflection_unittest.cc"],

‎src/google/protobuf/compiler/code_generator.h

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include "google/protobuf/compiler/code_generator_lite.h" // IWYU pragma: export
2626
#include "google/protobuf/descriptor.h"
2727
#include "google/protobuf/descriptor.pb.h"
28+
#include "google/protobuf/internal_feature_helper.h"
2829

2930
// Must be included last.
3031
#include "google/protobuf/port_def.inc"
@@ -142,6 +143,20 @@ class PROTOC_EXPORT CodeGenerator {
142143
return ::google::protobuf::internal::InternalFeatureHelper::GetFeatures(desc);
143144
}
144145

146+
// Returns the resolved FeatureSet for the language extension. It is
147+
// guaranteed that the result is fully aware of the language feature set
148+
// defaults, either the defaults set to the descriptor pool, or, if not set,
149+
// the defaults embedded in the language FeatureSet extension.
150+
template <typename DescriptorT, typename TypeTraitsT, uint8_t field_type,
151+
bool is_packed>
152+
static auto GetResolvedSourceFeatureExtension(
153+
const DescriptorT& desc,
154+
const google::protobuf::internal::ExtensionIdentifier<
155+
FeatureSet, TypeTraitsT, field_type, is_packed>& extension) {
156+
return ::google::protobuf::internal::InternalFeatureHelper::
157+
GetResolvedFeatureExtension(desc, extension);
158+
}
159+
145160
// Retrieves the unresolved source features for a given descriptor. These
146161
// should be used to validate the original .proto file. These represent the
147162
// original proto files from generated code, but should be stripped of

‎src/google/protobuf/compiler/code_generator_unittest.cc

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ class TestGenerator : public CodeGenerator {
6767
}
6868

6969
// Expose the protected methods for testing.
70+
using CodeGenerator::GetResolvedSourceFeatureExtension;
7071
using CodeGenerator::GetResolvedSourceFeatures;
7172
using CodeGenerator::GetUnresolvedSourceFeatures;
7273

@@ -246,6 +247,143 @@ TEST_F(CodeGeneratorTest, GetResolvedSourceFeaturesInherited) {
246247
EXPECT_EQ(ext.source_feature2(), pb::EnumFeature::VALUE3);
247248
}
248249

250+
TEST_F(CodeGeneratorTest, GetResolvedSourceFeatureExtension) {
251+
TestGenerator generator;
252+
generator.set_feature_extensions({GetExtensionReflection(pb::test)});
253+
ASSERT_OK(pool_.SetFeatureSetDefaults(*generator.BuildFeatureSetDefaults()));
254+
255+
ASSERT_THAT(BuildFile(DescriptorProto::descriptor()->file()), NotNull());
256+
ASSERT_THAT(BuildFile(pb::TestMessage::descriptor()->file()), NotNull());
257+
auto file = BuildFile(R"schema(
258+
edition = "2023";
259+
package proto2_unittest;
260+
261+
import "google/protobuf/unittest_features.proto";
262+
263+
option features.(pb.test).file_feature = VALUE6;
264+
option features.(pb.test).source_feature = VALUE5;
265+
)schema");
266+
ASSERT_THAT(file, NotNull());
267+
const pb::TestFeatures& ext1 =
268+
TestGenerator::GetResolvedSourceFeatureExtension(*file, pb::test);
269+
const pb::TestFeatures& ext2 =
270+
TestGenerator::GetResolvedSourceFeatures(*file).GetExtension(pb::test);
271+
272+
// Since the pool provides the feature set defaults, there should be no
273+
// difference between the two results.
274+
EXPECT_EQ(ext1.enum_feature(), pb::EnumFeature::VALUE1);
275+
EXPECT_EQ(ext1.field_feature(), pb::EnumFeature::VALUE1);
276+
EXPECT_EQ(ext1.file_feature(), pb::EnumFeature::VALUE6);
277+
EXPECT_EQ(ext1.source_feature(), pb::EnumFeature::VALUE5);
278+
EXPECT_EQ(ext2.enum_feature(), ext1.enum_feature());
279+
EXPECT_EQ(ext2.field_feature(), ext1.field_feature());
280+
EXPECT_EQ(ext2.file_feature(), ext1.file_feature());
281+
EXPECT_EQ(ext2.source_feature(), ext1.source_feature());
282+
}
283+
284+
TEST_F(CodeGeneratorTest, GetResolvedSourceFeatureExtensionEditedDefaults) {
285+
FeatureSetDefaults defaults = ParseTextOrDie(R"pb(
286+
minimum_edition: EDITION_PROTO2
287+
maximum_edition: EDITION_2024
288+
defaults {
289+
edition: EDITION_LEGACY
290+
overridable_features {}
291+
fixed_features {
292+
field_presence: EXPLICIT
293+
enum_type: CLOSED
294+
repeated_field_encoding: EXPANDED
295+
utf8_validation: NONE
296+
message_encoding: LENGTH_PREFIXED
297+
json_format: LEGACY_BEST_EFFORT
298+
enforce_naming_style: STYLE_LEGACY
299+
default_symbol_visibility: EXPORT_ALL
300+
}
301+
}
302+
defaults {
303+
edition: EDITION_2023
304+
overridable_features {
305+
field_presence: EXPLICIT
306+
enum_type: OPEN
307+
repeated_field_encoding: PACKED
308+
utf8_validation: VERIFY
309+
message_encoding: LENGTH_PREFIXED
310+
json_format: ALLOW
311+
[pb.test] {
312+
file_feature: VALUE3
313+
field_feature: VALUE15
314+
enum_feature: VALUE14
315+
source_feature: VALUE1
316+
}
317+
}
318+
fixed_features {
319+
enforce_naming_style: STYLE_LEGACY
320+
default_symbol_visibility: EXPORT_ALL
321+
}
322+
}
323+
)pb");
324+
ASSERT_OK(pool_.SetFeatureSetDefaults(defaults));
325+
326+
ASSERT_THAT(BuildFile(DescriptorProto::descriptor()->file()), NotNull());
327+
ASSERT_THAT(BuildFile(pb::TestMessage::descriptor()->file()), NotNull());
328+
auto file = BuildFile(R"schema(
329+
edition = "2023";
330+
package proto2_unittest;
331+
332+
import "google/protobuf/unittest_features.proto";
333+
334+
option features.(pb.test).file_feature = VALUE6;
335+
option features.(pb.test).source_feature = VALUE5;
336+
)schema");
337+
ASSERT_THAT(file, NotNull());
338+
const pb::TestFeatures& ext =
339+
TestGenerator::GetResolvedSourceFeatureExtension(*file, pb::test);
340+
341+
// Since the pool provides the modified feature set defaults, the result
342+
// should be the one reflecting the pool's defaults.
343+
EXPECT_EQ(ext.enum_feature(), pb::EnumFeature::VALUE14);
344+
EXPECT_EQ(ext.field_feature(), pb::EnumFeature::VALUE15);
345+
EXPECT_EQ(ext.file_feature(), pb::EnumFeature::VALUE6);
346+
EXPECT_EQ(ext.source_feature(), pb::EnumFeature::VALUE5);
347+
}
348+
349+
TEST_F(CodeGeneratorTest,
350+
GetResolvedSourceFeatureExtensionDefaultsFromFeatureSetExtension) {
351+
// Make sure feature set defaults are empty in the pool.
352+
TestGenerator generator;
353+
generator.set_feature_extensions({});
354+
ASSERT_OK(pool_.SetFeatureSetDefaults(*generator.BuildFeatureSetDefaults()));
355+
356+
ASSERT_THAT(BuildFile(DescriptorProto::descriptor()->file()), NotNull());
357+
ASSERT_THAT(BuildFile(pb::TestMessage::descriptor()->file()), NotNull());
358+
auto file = BuildFile(R"schema(
359+
edition = "2023";
360+
package proto2_unittest;
361+
362+
import "google/protobuf/unittest_features.proto";
363+
364+
option features.(pb.test).file_feature = VALUE6;
365+
option features.(pb.test).source_feature = VALUE5;
366+
)schema");
367+
ASSERT_THAT(file, NotNull());
368+
369+
const pb::TestFeatures& ext1 =
370+
TestGenerator::GetResolvedSourceFeatureExtension(*file, pb::test);
371+
const pb::TestFeatures& ext2 =
372+
TestGenerator::GetResolvedSourceFeatures(*file).GetExtension(pb::test);
373+
374+
// No defaults were added to the pool, but they should be still present in the
375+
// result. On the other hand, features that are explicitly set should be also
376+
// present.
377+
EXPECT_EQ(ext1.enum_feature(), pb::EnumFeature::VALUE1);
378+
EXPECT_EQ(ext1.field_feature(), pb::EnumFeature::VALUE1);
379+
EXPECT_EQ(ext1.file_feature(), pb::EnumFeature::VALUE6);
380+
EXPECT_EQ(ext1.source_feature(), pb::EnumFeature::VALUE5);
381+
EXPECT_EQ(ext2.enum_feature(), pb::EnumFeature::TEST_ENUM_FEATURE_UNKNOWN);
382+
EXPECT_EQ(ext2.field_feature(), pb::EnumFeature::TEST_ENUM_FEATURE_UNKNOWN);
383+
EXPECT_EQ(ext2.file_feature(), pb::EnumFeature::VALUE6);
384+
EXPECT_EQ(ext2.source_feature(), pb::EnumFeature::VALUE5);
385+
}
386+
249387
// TODO: Use the gtest versions once that's available in OSS.
250388
MATCHER_P(HasError, msg_matcher, "") {
251389
return arg.status().code() == absl::StatusCode::kFailedPrecondition &&

‎src/google/protobuf/compiler/cpp/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,7 @@ cc_library(
147147
"@abseil-cpp//absl/log:die_if_null",
148148
"@abseil-cpp//absl/memory",
149149
"@abseil-cpp//absl/status",
150+
"@abseil-cpp//absl/status:statusor",
150151
"@abseil-cpp//absl/strings",
151152
"@abseil-cpp//absl/strings:str_format",
152153
"@abseil-cpp//absl/synchronization",

‎src/google/protobuf/compiler/cpp/extension.cc

Lines changed: 63 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,17 @@
1515
#include <vector>
1616

1717
#include "absl/log/absl_check.h"
18+
#include "absl/status/statusor.h"
19+
#include "absl/strings/escaping.h"
1820
#include "absl/strings/str_cat.h"
1921
#include "absl/strings/str_replace.h"
22+
#include "absl/strings/string_view.h"
23+
#include "google/protobuf/compiler/code_generator.h"
2024
#include "google/protobuf/compiler/cpp/helpers.h"
2125
#include "google/protobuf/compiler/cpp/options.h"
2226
#include "google/protobuf/descriptor.h"
2327
#include "google/protobuf/descriptor.pb.h"
28+
#include "google/protobuf/feature_resolver.h"
2429
#include "google/protobuf/io/printer.h"
2530

2631
namespace google {
@@ -96,30 +101,67 @@ bool ExtensionGenerator::IsScoped() const {
96101
return descriptor_->extension_scope() != nullptr;
97102
}
98103

104+
namespace {
105+
bool ShouldGenerateFeatureSetDefaultData(absl::string_view extension) {
106+
return extension == "pb.java" || extension == "pb.test";
107+
}
108+
} // namespace
109+
99110
void ExtensionGenerator::GenerateDeclaration(io::Printer* p) const {
100111
auto var = p->WithVars(variables_);
101112
auto annotate = p->WithAnnotations({{"name", descriptor_}});
102-
p->Emit({{"constant_qualifier",
103-
// If this is a class member, it needs to be declared
104-
// `static constexpr`.
105-
// Otherwise, it will be
106-
// `inline constexpr`.
107-
IsScoped() ? "static" : ""},
108-
{"id_qualifier",
109-
// If this is a class member, it needs to be declared "static".
110-
// Otherwise, it needs to be "extern". In the latter case, it
111-
// also needs the DLL export/import specifier.
112-
IsScoped() ? "static"
113-
: options_.dllexport_decl.empty()
114-
? "extern"
115-
: absl::StrCat(options_.dllexport_decl, " extern")}},
116-
R"cc(
117-
inline $constant_qualifier $constexpr int $constant_name$ =
118-
$number$;
119-
$id_qualifier$ $pbi$::ExtensionIdentifier<
120-
$extendee$, $pbi$::$type_traits$, $field_type$, $packed$>
121-
$name$;
122-
)cc");
113+
p->Emit(
114+
{{"constant_qualifier",
115+
// If this is a class member, it needs to be declared
116+
// `static constexpr`.
117+
// Otherwise, it will be
118+
// `inline constexpr`.
119+
IsScoped() ? "static" : ""},
120+
{"id_qualifier",
121+
// If this is a class member, it needs to be declared "static".
122+
// Otherwise, it needs to be "extern". In the latter case, it
123+
// also needs the DLL export/import specifier.
124+
IsScoped() ? "static"
125+
: options_.dllexport_decl.empty()
126+
? "extern"
127+
: absl::StrCat(options_.dllexport_decl, " extern")},
128+
{"feature_set_defaults",
129+
[&] {
130+
if (!ShouldGenerateFeatureSetDefaultData(descriptor_->full_name())) {
131+
return;
132+
}
133+
if (descriptor_->message_type() == nullptr) return;
134+
absl::string_view extendee =
135+
descriptor_->containing_type()->full_name();
136+
if (extendee != "google.protobuf.FeatureSet") return;
137+
138+
std::vector<const FieldDescriptor*> extensions = {descriptor_};
139+
absl::StatusOr<FeatureSetDefaults> defaults =
140+
FeatureResolver::CompileDefaults(
141+
descriptor_->containing_type(), extensions,
142+
ProtocMinimumEdition(), ProtocMaximumEdition());
143+
ABSL_CHECK_OK(defaults);
144+
p->Emit(
145+
{{"defaults", absl::Base64Escape(defaults->SerializeAsString())},
146+
{"extension_type", ClassName(descriptor_->message_type(), true)},
147+
{"function_name", "GetFeatureSetDefaultsData"}},
148+
R"cc(
149+
namespace internal {
150+
template <>
151+
inline ::absl::string_view $function_name$<$extension_type$>() {
152+
static constexpr char kDefaults[] = "$defaults$";
153+
return kDefaults;
154+
}
155+
} // namespace internal
156+
)cc");
157+
}}},
158+
R"cc(
159+
inline $constant_qualifier $constexpr int $constant_name$ = $number$;
160+
$id_qualifier$ $pbi$::ExtensionIdentifier<
161+
$extendee$, $pbi$::$type_traits$, $field_type$, $packed$>
162+
$name$;
163+
$feature_set_defaults$;
164+
)cc");
123165
}
124166

125167
void ExtensionGenerator::GenerateDefinition(io::Printer* p) {

‎src/google/protobuf/compiler/java/generator.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ class PROTOC_EXPORT JavaGenerator : public CodeGenerator {
6060
}
6161

6262
using CodeGenerator::GetEdition;
63+
using CodeGenerator::GetResolvedSourceFeatureExtension;
6364
using CodeGenerator::GetResolvedSourceFeatures;
6465
using CodeGenerator::GetUnresolvedSourceFeatures;
6566

‎src/google/protobuf/compiler/java/helpers.cc

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -929,15 +929,12 @@ namespace {
929929
// generated class should be nested in the generated proto file Java class.
930930
template <typename Descriptor>
931931
inline bool NestInFileClass(const Descriptor& descriptor) {
932-
auto nest_in_file_class = JavaGenerator::GetResolvedSourceFeatures(descriptor)
933-
.GetExtension(pb::java)
934-
.nest_in_file_class();
932+
auto nest_in_file_class =
933+
JavaGenerator::GetResolvedSourceFeatureExtension(descriptor, pb::java)
934+
.nest_in_file_class();
935935
ABSL_CHECK(
936936
nest_in_file_class !=
937-
pb::JavaFeatures::NestInFileClassFeature::NEST_IN_FILE_CLASS_UNKNOWN)
938-
<< "Unknown value for nest_in_file_class feature. Try populating the "
939-
"Java feature set defaults in your generator plugin or custom "
940-
"descriptor pool.";
937+
pb::JavaFeatures::NestInFileClassFeature::NEST_IN_FILE_CLASS_UNKNOWN);
941938

942939
if (nest_in_file_class == pb::JavaFeatures::NestInFileClassFeature::LEGACY) {
943940
return !descriptor.file()->options().java_multiple_files();

0 commit comments

Comments
 (0)