Skip to content

Commit

Permalink
Editions: Migrate edition strings to enum in C++ code.
Browse files Browse the repository at this point in the history 
The edition specification in proto files will remain unchanged, but it will be immediately converted to an enum by the parser.  This gives us more control over the valid set of editions and simplifies ordering (just an integer comparison now).  We plan to release exactly one edition per year.

PiperOrigin-RevId: 563215992
  • Loading branch information
@mkruskal-google @copybara-github
mkruskal-google authored and copybara-github committed Sep 6, 2023
1 parent 0e484c7 commit f083ebf
Show file tree
Hide file tree
Showing 17 changed files with 447 additions and 367 deletions.
8 changes: 2 additions & 6 deletions src/google/protobuf/compiler/code_generator.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,15 +142,11 @@ class PROTOC_EXPORT CodeGenerator {

// Returns the minimum edition (inclusive) supported by this generator. Any
// proto files with an edition before this will result in an error.
virtual absl::string_view GetMinimumEdition() const {
return PROTOBUF_MINIMUM_EDITION;
}
virtual Edition GetMinimumEdition() const { return PROTOBUF_MINIMUM_EDITION; }

// Returns the maximum edition (inclusive) supported by this generator. Any
// proto files with an edition after this will result in an error.
virtual absl::string_view GetMaximumEdition() const {
return PROTOBUF_MAXIMUM_EDITION;
}
virtual Edition GetMaximumEdition() const { return PROTOBUF_MAXIMUM_EDITION; }

// Builds a default feature set mapping for this generator.
//
Expand Down
26 changes: 11 additions & 15 deletions src/google/protobuf/compiler/code_generator_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,17 +73,13 @@ class TestGenerator : public CodeGenerator {
feature_extensions_ = extensions;
}

absl::string_view GetMinimumEdition() const override {
return minimum_edition_;
}
void set_minimum_edition(absl::string_view minimum_edition) {
Edition GetMinimumEdition() const override { return minimum_edition_; }
void set_minimum_edition(Edition minimum_edition) {
minimum_edition_ = minimum_edition;
}

absl::string_view GetMaximumEdition() const override {
return maximum_edition_;
}
void set_maximum_edition(absl::string_view maximum_edition) {
Edition GetMaximumEdition() const override { return maximum_edition_; }
void set_maximum_edition(Edition maximum_edition) {
maximum_edition_ = maximum_edition;
}

Expand All @@ -92,8 +88,8 @@ class TestGenerator : public CodeGenerator {
using CodeGenerator::GetUnresolvedSourceFeatures;

private:
absl::string_view minimum_edition_ = PROTOBUF_MINIMUM_EDITION;
absl::string_view maximum_edition_ = PROTOBUF_MAXIMUM_EDITION;
Edition minimum_edition_ = PROTOBUF_MINIMUM_EDITION;
Edition maximum_edition_ = PROTOBUF_MAXIMUM_EDITION;
std::vector<const FieldDescriptor*> feature_extensions_ = {
GetExtensionReflection(pb::test)};
};
Expand Down Expand Up @@ -286,12 +282,12 @@ TEST_F(CodeGeneratorTest, BuildFeatureSetDefaultsInvalidExtension) {
TEST_F(CodeGeneratorTest, BuildFeatureSetDefaults) {
TestGenerator generator;
generator.set_feature_extensions({});
generator.set_minimum_edition("2020");
generator.set_maximum_edition("2024");
generator.set_minimum_edition(EDITION_1_TEST_ONLY);
generator.set_maximum_edition(EDITION_99999_TEST_ONLY);
EXPECT_THAT(generator.BuildFeatureSetDefaults(),
IsOkAndHolds(EqualsProto(R"pb(
defaults {
edition: "2023"
edition_enum: EDITION_2023
features {
field_presence: EXPLICIT
enum_type: OPEN
Expand All @@ -300,8 +296,8 @@ TEST_F(CodeGeneratorTest, BuildFeatureSetDefaults) {
json_format: ALLOW
}
}
minimum_edition: "2020"
maximum_edition: "2024"
minimum_edition_enum: EDITION_1_TEST_ONLY
maximum_edition_enum: EDITION_99999_TEST_ONLY
)pb")));
}

Expand Down
4 changes: 2 additions & 2 deletions src/google/protobuf/compiler/command_line_interface.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1538,8 +1538,8 @@ bool CommandLineInterface::SetupFeatureResolution(DescriptorPool& pool) {
// Calculate the feature defaults for each built-in generator. All generators
// that support editions must agree on the supported edition range.
std::vector<const FieldDescriptor*> feature_extensions;
absl::string_view minimum_edition = PROTOBUF_MINIMUM_EDITION;
absl::string_view maximum_edition = PROTOBUF_MAXIMUM_EDITION;
Edition minimum_edition = PROTOBUF_MINIMUM_EDITION;
Edition maximum_edition = PROTOBUF_MAXIMUM_EDITION;
for (const auto& output : output_directives_) {
if (output.generator == nullptr) continue;
if ((output.generator->GetSupportedFeatures() &
Expand Down
12 changes: 6 additions & 6 deletions src/google/protobuf/compiler/command_line_interface_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1494,25 +1494,25 @@ TEST_F(CommandLineInterfaceTest, FeatureExtensionError) {
TEST_F(CommandLineInterfaceTest, InvalidMinimumEditionError) {
CreateTempFile("foo.proto", R"schema(edition = "2023";)schema");

mock_generator_->set_minimum_edition("2022");
mock_generator_->set_minimum_edition(EDITION_1_TEST_ONLY);

Run("protocol_compiler --proto_path=$tmpdir --test_out=$tmpdir "
"--experimental_editions foo.proto");
ExpectErrorSubstring(
"generator --test_out specifies a minimum edition 2022 which is not the "
"protoc minimum 2023");
"generator --test_out specifies a minimum edition 1_TEST_ONLY which is "
"not the protoc minimum 2023");
}

TEST_F(CommandLineInterfaceTest, InvalidMaximumEditionError) {
CreateTempFile("foo.proto", R"schema(edition = "2023";)schema");

mock_generator_->set_maximum_edition("2123");
mock_generator_->set_maximum_edition(EDITION_99999_TEST_ONLY);

Run("protocol_compiler --proto_path=$tmpdir --test_out=$tmpdir "
"--experimental_editions foo.proto");
ExpectErrorSubstring(
"generator --test_out specifies a maximum edition 2123 which is not "
"the protoc maximum 2023");
"generator --test_out specifies a maximum edition 99999_TEST_ONLY which "
"is not the protoc maximum 2023");
}

TEST_F(CommandLineInterfaceTest, InvalidFeatureExtensionError) {
Expand Down
16 changes: 6 additions & 10 deletions src/google/protobuf/compiler/mock_code_generator.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,25 +129,21 @@ class MockCodeGenerator : public CodeGenerator {
feature_extensions_ = extensions;
}

absl::string_view GetMinimumEdition() const override {
return minimum_edition_;
}
void set_minimum_edition(absl::string_view minimum_edition) {
Edition GetMinimumEdition() const override { return minimum_edition_; }
void set_minimum_edition(Edition minimum_edition) {
minimum_edition_ = minimum_edition;
}

absl::string_view GetMaximumEdition() const override {
return maximum_edition_;
}
void set_maximum_edition(absl::string_view maximum_edition) {
Edition GetMaximumEdition() const override { return maximum_edition_; }
void set_maximum_edition(Edition maximum_edition) {
maximum_edition_ = maximum_edition;
}

private:
std::string name_;
uint64_t suppressed_features_ = 0;
absl::string_view minimum_edition_ = PROTOBUF_MINIMUM_EDITION;
absl::string_view maximum_edition_ = PROTOBUF_MAXIMUM_EDITION;
Edition minimum_edition_ = PROTOBUF_MINIMUM_EDITION;
Edition maximum_edition_ = PROTOBUF_MAXIMUM_EDITION;
std::vector<const FieldDescriptor*> feature_extensions_ = {
GetExtensionReflection(pb::test)};

Expand Down
10 changes: 5 additions & 5 deletions src/google/protobuf/compiler/parser.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -694,7 +694,7 @@ bool Parser::Parse(io::Tokenizer* input, FileDescriptorProto* file) {
if (file != nullptr) {
file->set_syntax(syntax_identifier_);
if (syntax_identifier_ == "editions") {
file->set_edition(edition_);
file->set_edition_enum(edition_);
}
}
} else if (!stop_after_syntax_identifier_) {
Expand Down Expand Up @@ -752,17 +752,17 @@ bool Parser::ParseSyntaxIdentifier(const FileDescriptorProto* file,
DO(ConsumeString(&syntax, "Expected syntax identifier."));
DO(ConsumeEndOfDeclaration(";", &syntax_location));

(has_edition ? edition_ : syntax_identifier_) = syntax;
if (has_edition) {
if (syntax.empty()) {
if (!Edition_Parse(absl::StrCat("EDITION_", syntax), &edition_) ||
edition_ == Edition::EDITION_UNKNOWN) {
RecordError(syntax_token.line, syntax_token.column,
"A file's edition must be a nonempty string.");
absl::StrCat("Unknown edition \"", syntax, "\"."));
return false;
}
edition_ = syntax;
syntax_identifier_ = "editions";
return true;
}

syntax_identifier_ = syntax;
if (syntax != "proto2" && syntax != "proto3" &&
!stop_after_syntax_identifier_) {
Expand Down
2 changes: 1 addition & 1 deletion src/google/protobuf/compiler/parser.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -553,7 +553,7 @@ class PROTOBUF_EXPORT Parser {
bool require_syntax_identifier_;
bool stop_after_syntax_identifier_;
std::string syntax_identifier_;
std::string edition_;
Edition edition_ = Edition::EDITION_UNKNOWN;

// Leading doc comments for the next declaration. These are not complete
// yet; use ConsumeEndOfDeclaration() to get the complete comments.
Expand Down
49 changes: 39 additions & 10 deletions src/google/protobuf/compiler/parser_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ TEST_F(ParserTest, RegressionNestedOpenBraceDoNotStackOverflow) {
input,
"0:10: Unexpected end of string.\n"
"0:10: Invalid control characters encountered in text.\n"
"0:12: Expected top-level statement (e.g. \"message\").\n");
"0:8: Unknown edition \"a\".\n");
}

// ===================================================================
Expand Down Expand Up @@ -891,7 +891,7 @@ TEST_F(ParseMessageTest, ReservedIdentifiers) {
"}\n",

"syntax: \"editions\" "
"edition: \"2023\" "
"edition_enum: EDITION_2023 "
"message_type {"
" name: \"TestMessage\""
" reserved_name: \"foo\""
Expand Down Expand Up @@ -1279,7 +1279,7 @@ TEST_F(ParseEnumTest, ReservedIdentifiers) {
"}\n",

"syntax: \"editions\" "
"edition: \"2023\" "
"edition_enum: EDITION_2023 "
"enum_type {"
" name: \"TestEnum\""
" value { name:\"FOO\" number:0 }"
Expand Down Expand Up @@ -4067,7 +4067,7 @@ typedef ParserTest ParseEditionsTest;
TEST_F(ParseEditionsTest, Editions) {
ExpectParsesTo(
R"schema(
edition = "super-cool";
edition = "2023";
message A {
int32 b = 1;
})schema",
Expand All @@ -4081,7 +4081,16 @@ TEST_F(ParseEditionsTest, Editions) {
" }"
"}"
"syntax: \"editions\""
"edition: \"super-cool\"\n");
"edition_enum: EDITION_2023\n");
}

TEST_F(ParseEditionsTest, TestEdition) {
ExpectParsesTo(
R"schema(
edition = "99998_TEST_ONLY";
)schema",
"syntax: \"editions\""
"edition_enum: EDITION_99998_TEST_ONLY\n");
}

TEST_F(ParseEditionsTest, ExtensionsParse) {
Expand All @@ -4107,7 +4116,7 @@ TEST_F(ParseEditionsTest, ExtensionsParse) {
" type: TYPE_STRING"
"}"
"syntax: \"editions\""
"edition: \"2023\"\n");
"edition_enum: EDITION_2023\n");
}

TEST_F(ParseEditionsTest, MapFeatures) {
Expand Down Expand Up @@ -4166,7 +4175,7 @@ TEST_F(ParseEditionsTest, MapFeatures) {
}
}
syntax: "editions"
edition: "2023")pb");
edition_enum: EDITION_2023)pb");
}

TEST_F(ParseEditionsTest, EmptyEdition) {
Expand All @@ -4176,7 +4185,27 @@ TEST_F(ParseEditionsTest, EmptyEdition) {
message A {
optional int32 b = 1;
})schema",
"1:18: A file's edition must be a nonempty string.\n");
"1:18: Unknown edition \"\".\n");
}

TEST_F(ParseEditionsTest, InvalidEdition) {
ExpectHasEarlyExitErrors(
R"schema(
edition = "2023_INVALID";
message A {
optional int32 b = 1;
})schema",
"1:18: Unknown edition \"2023_INVALID\".\n");
}

TEST_F(ParseEditionsTest, UnknownEdition) {
ExpectHasEarlyExitErrors(
R"schema(
edition = "UNKNOWN";
message A {
optional int32 b = 1;
})schema",
"1:18: Unknown edition \"UNKNOWN\".\n");
}

TEST_F(ParseEditionsTest, SyntaxEditions) {
Expand All @@ -4194,7 +4223,7 @@ TEST_F(ParseEditionsTest, MixedSyntaxAndEdition) {
ExpectHasErrors(
R"schema(
syntax = "proto2";
edition = "super-cool";
edition = "2023";
message A {
optional int32 b = 1;
})schema",
Expand All @@ -4204,7 +4233,7 @@ TEST_F(ParseEditionsTest, MixedSyntaxAndEdition) {
TEST_F(ParseEditionsTest, MixedEditionAndSyntax) {
ExpectHasErrors(
R"schema(
edition = "super-cool";
edition = "2023";
syntax = "proto2";
message A {
int32 b = 1;
Expand Down
33 changes: 15 additions & 18 deletions src/google/protobuf/descriptor.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2818,7 +2818,7 @@ void FileDescriptor::CopyHeadingTo(FileDescriptorProto* proto) const {
proto->set_syntax(FileDescriptorLegacy::SyntaxName(syntax));
}
if (syntax == FileDescriptorLegacy::Syntax::SYNTAX_EDITIONS) {
proto->set_edition(edition());
proto->set_edition_enum(edition());
}

if (&options() != &FileOptions::default_instance()) {
Expand Down Expand Up @@ -3260,8 +3260,7 @@ std::string FileDescriptor::DebugStringWithOptions(
if (FileDescriptorLegacy(this).syntax() ==
FileDescriptorLegacy::SYNTAX_EDITIONS) {
absl::SubstituteAndAppend(&contents, "edition = \"$0\";\n\n", edition());
} else // NOLINT(readability/braces)
{
} else {
absl::SubstituteAndAppend(&contents, "syntax = \"$0\";\n\n",
FileDescriptorLegacy::SyntaxName(
FileDescriptorLegacy(this).syntax()));
Expand Down Expand Up @@ -5595,10 +5594,9 @@ static void PlanAllocationSize(const FileDescriptorProto& proto,
internal::FlatAllocator& alloc) {
alloc.PlanArray<FileDescriptor>(1);
alloc.PlanArray<FileDescriptorTables>(1);
alloc.PlanArray<std::string>(
2 + (proto.has_edition() ? 1 : 0)); // name + package
alloc.PlanArray<std::string>(2); // name + package
if (proto.has_options()) alloc.PlanArray<FileOptions>(1);
if (proto.has_edition()) {
if (proto.has_edition_enum()) {
alloc.PlanArray<FeatureSet>(1);
if (HasFeatures(proto.options())) {
alloc.PlanArray<FeatureSet>(1);
Expand Down Expand Up @@ -5705,14 +5703,14 @@ FileDescriptor* DescriptorBuilder::BuildFileImpl(
FileDescriptor* result = alloc.AllocateArray<FileDescriptor>(1);
file_ = result;

if (proto.has_edition()) {
if (proto.has_edition_enum()) {
const FeatureSetDefaults& defaults =
pool_->feature_set_defaults_spec_ == nullptr
? GetCppFeatureSetDefaults()
: *pool_->feature_set_defaults_spec_;

absl::StatusOr<FeatureResolver> feature_resolver =
FeatureResolver::Create(proto.edition(), defaults);
FeatureResolver::Create(proto.edition_enum(), defaults);
if (!feature_resolver.ok()) {
AddError(
proto.name(), proto, DescriptorPool::ErrorCollector::EDITIONS,
Expand Down Expand Up @@ -5755,10 +5753,10 @@ FileDescriptor* DescriptorBuilder::BuildFileImpl(
return absl::StrCat("Unrecognized syntax: ", proto.syntax());
});
}
if (proto.has_edition()) {
file_->edition_ = alloc.AllocateStrings(proto.edition());
if (proto.has_edition_enum()) {
file_->edition_ = proto.edition_enum();
} else {
file_->edition_ = nullptr;
file_->edition_ = Edition::EDITION_UNKNOWN;
}

result->name_ = alloc.AllocateStrings(proto.name());
Expand Down Expand Up @@ -9568,15 +9566,14 @@ bool IsLazilyInitializedFile(absl::string_view filename) {
} // namespace cpp
} // namespace internal

absl::string_view FileDescriptor::edition() const {
// ASLR will help give this a random value across processes.
static const void* kAntiHyrumText = &kAntiHyrumText;
absl::string_view anti_hyrum_string(
reinterpret_cast<const char*>(kAntiHyrumText),
(reinterpret_cast<size_t>(kAntiHyrumText) >> 3) % sizeof(void*));
Edition FileDescriptor::edition() const { return edition_; }

return edition_ == nullptr ? anti_hyrum_string : *edition_;
namespace internal {
absl::string_view ShortEditionName(Edition edition) {
return absl::StripPrefix(Edition_Name(edition), "EDITION_");
}
} // namespace internal

} // namespace protobuf
} // namespace google

Expand Down
Loading

0 comments on commit f083ebf

Please sign in to comment.