Editions: Refactor feature API for C++ generators.

The new naming will use our new vocabulary, and restrict feature access:

Unresolved source features - source-retention features that generators need to validate their own features.  These should not be sent to the runtime or used for any decision-making.

Resolved source features - fully resolved source-retention features.  These should be used to make codegen decisions, but shouldn't be sent to runtimes.

PiperOrigin-RevId: 558287302
pull/13606/head
Mike Kruskal 2 years ago committed by Copybara-Service
parent bfdd31fbbc
commit da38af7191
  1. 27
      src/google/protobuf/compiler/code_generator.h
  2. 33
      src/google/protobuf/compiler/code_generator_unittest.cc
  3. 18
      src/google/protobuf/compiler/cpp/generator.cc
  4. 28
      src/google/protobuf/descriptor.h

@ -38,6 +38,7 @@
#ifndef GOOGLE_PROTOBUF_COMPILER_CODE_GENERATOR_H__
#define GOOGLE_PROTOBUF_COMPILER_CODE_GENERATOR_H__
#include <cstdint>
#include <string>
#include <utility>
#include <vector>
@ -131,19 +132,27 @@ class PROTOC_EXPORT CodeGenerator {
virtual bool HasGenerateAll() const { return true; }
protected:
// Retrieves the resolved source features for a given descriptor. These
// should be used to make any feature-based decisions during code generation.
// Retrieves the resolved source features for a given descriptor. All the
// features that are imported (from the proto file) and linked in (from the
// callers binary) will be fully resolved. These should be used to make any
// feature-based decisions during code generation.
template <typename DescriptorT>
static const FeatureSet& GetSourceFeatures(const DescriptorT& desc) {
static const FeatureSet& GetResolvedSourceFeatures(const DescriptorT& desc) {
return ::google::protobuf::internal::InternalFeatureHelper::GetFeatures(desc);
}
// Retrieves the raw source features for a given descriptor. These should be
// used to validate the original .proto file and make any decisions about it
// during code generation.
template <typename DescriptorT>
static const FeatureSet& GetSourceRawFeatures(const DescriptorT& desc) {
return ::google::protobuf::internal::InternalFeatureHelper::GetRawFeatures(desc);
// Retrieves the unresolved source features for a given descriptor. These
// should be used to validate the original .proto file. These represent the
// original proto files from generated code, but should be stripped of
// source-retention features before sending to a runtime.
template <typename DescriptorT, typename TypeTraitsT, uint8_t field_type,
bool is_packed>
static typename TypeTraitsT::ConstType GetUnresolvedSourceFeatures(
const DescriptorT& descriptor,
const google::protobuf::internal::ExtensionIdentifier<
FeatureSet, TypeTraitsT, field_type, is_packed>& extension) {
return ::google::protobuf::internal::InternalFeatureHelper::GetUnresolvedFeatures(
descriptor, extension);
}
};

@ -61,8 +61,8 @@ class TestGenerator : public CodeGenerator {
}
// Expose the protected methods for testing.
using CodeGenerator::GetSourceFeatures;
using CodeGenerator::GetSourceRawFeatures;
using CodeGenerator::GetResolvedSourceFeatures;
using CodeGenerator::GetUnresolvedSourceFeatures;
};
class SimpleErrorCollector : public io::ErrorCollector {
@ -101,7 +101,7 @@ class CodeGeneratorTest : public ::testing::Test {
DescriptorPool pool_;
};
TEST_F(CodeGeneratorTest, GetSourceRawFeaturesRoot) {
TEST_F(CodeGeneratorTest, GetUnresolvedSourceFeaturesRoot) {
auto file = BuildFile(R"schema(
edition = "2023";
package protobuf_unittest;
@ -115,15 +115,14 @@ TEST_F(CodeGeneratorTest, GetSourceRawFeaturesRoot) {
)schema");
ASSERT_THAT(file, NotNull());
EXPECT_THAT(TestGenerator::GetSourceRawFeatures(*file),
EXPECT_THAT(TestGenerator::GetUnresolvedSourceFeatures(*file, pb::test),
google::protobuf::EqualsProto(R"pb(
field_presence: EXPLICIT
enum_type: CLOSED
[pb.test] { int_file_feature: 8 string_source_feature: "file" }
int_file_feature: 8
string_source_feature: "file"
)pb"));
}
TEST_F(CodeGeneratorTest, GetSourceRawFeaturesInherited) {
TEST_F(CodeGeneratorTest, GetUnresolvedSourceFeaturesInherited) {
auto file = BuildFile(R"schema(
edition = "2023";
package protobuf_unittest;
@ -149,14 +148,14 @@ TEST_F(CodeGeneratorTest, GetSourceRawFeaturesInherited) {
file->FindMessageTypeByName("EditionsMessage")->FindFieldByName("field");
ASSERT_THAT(field, NotNull());
EXPECT_THAT(
TestGenerator::GetSourceRawFeatures(*field), google::protobuf::EqualsProto(R"pb(
field_presence: EXPLICIT
[pb.test] { int_multiple_feature: 9 string_source_feature: "field" }
)pb"));
EXPECT_THAT(TestGenerator::GetUnresolvedSourceFeatures(*field, pb::test),
google::protobuf::EqualsProto(R"pb(
int_multiple_feature: 9
string_source_feature: "field"
)pb"));
}
TEST_F(CodeGeneratorTest, GetSourceFeaturesRoot) {
TEST_F(CodeGeneratorTest, GetResolvedSourceFeaturesRoot) {
auto file = BuildFile(R"schema(
edition = "2023";
package protobuf_unittest;
@ -170,7 +169,7 @@ TEST_F(CodeGeneratorTest, GetSourceFeaturesRoot) {
)schema");
ASSERT_THAT(file, NotNull());
const FeatureSet& features = TestGenerator::GetSourceFeatures(*file);
const FeatureSet& features = TestGenerator::GetResolvedSourceFeatures(*file);
const pb::TestFeatures& ext = features.GetExtension(pb::test);
EXPECT_TRUE(features.has_repeated_field_encoding());
@ -183,7 +182,7 @@ TEST_F(CodeGeneratorTest, GetSourceFeaturesRoot) {
EXPECT_EQ(ext.string_source_feature(), "file");
}
TEST_F(CodeGeneratorTest, GetSourceFeaturesInherited) {
TEST_F(CodeGeneratorTest, GetResolvedSourceFeaturesInherited) {
auto file = BuildFile(R"schema(
edition = "2023";
package protobuf_unittest;
@ -210,7 +209,7 @@ TEST_F(CodeGeneratorTest, GetSourceFeaturesInherited) {
const FieldDescriptor* field =
file->FindMessageTypeByName("EditionsMessage")->FindFieldByName("field");
ASSERT_THAT(field, NotNull());
const FeatureSet& features = TestGenerator::GetSourceFeatures(*field);
const FeatureSet& features = TestGenerator::GetResolvedSourceFeatures(*field);
const pb::TestFeatures& ext = features.GetExtension(pb::test);
EXPECT_EQ(features.enum_type(), FeatureSet::CLOSED);

@ -365,21 +365,23 @@ bool CppGenerator::Generate(const FileDescriptor* file,
absl::Status CppGenerator::ValidateFeatures(const FileDescriptor* file) const {
absl::Status status = absl::OkStatus();
google::protobuf::internal::VisitDescriptors(*file, [&](const FieldDescriptor& field) {
const FeatureSet& source_features = GetSourceFeatures(field);
const FeatureSet& raw_features = GetSourceRawFeatures(field);
const FeatureSet& resolved_features = GetResolvedSourceFeatures(field);
const pb::CppFeatures& unresolved_features =
GetUnresolvedSourceFeatures(field, pb::cpp);
if (field.enum_type() != nullptr &&
source_features.GetExtension(::pb::cpp).legacy_closed_enum() &&
source_features.field_presence() == FeatureSet::IMPLICIT) {
resolved_features.GetExtension(::pb::cpp).legacy_closed_enum() &&
resolved_features.field_presence() == FeatureSet::IMPLICIT) {
status = absl::FailedPreconditionError(
absl::StrCat("Field ", field.full_name(),
" has a closed enum type with implicit presence."));
}
if (source_features.GetExtension(::pb::cpp).utf8_validation() ==
if (resolved_features.GetExtension(::pb::cpp).utf8_validation() ==
pb::CppFeatures::UTF8_VALIDATION_UNKNOWN) {
status = absl::FailedPreconditionError(absl::StrCat(
"Field ", field.full_name(),
" has an unknown value for the utf8_validation feature. ",
source_features.DebugString(), "\nRawFeatures: ", raw_features));
resolved_features.DebugString(),
"\nRawFeatures: ", unresolved_features));
}
if (field.containing_type() == nullptr ||
@ -388,7 +390,7 @@ absl::Status CppGenerator::ValidateFeatures(const FileDescriptor* file) const {
// will be blindly propagated from the original map field, and may violate
// a lot of these conditions. Note: we do still validate the
// user-specified map field.
if (raw_features.GetExtension(::pb::cpp).has_legacy_closed_enum() &&
if (unresolved_features.has_legacy_closed_enum() &&
field.cpp_type() != FieldDescriptor::CPPTYPE_ENUM) {
status = absl::FailedPreconditionError(
absl::StrCat("Field ", field.full_name(),
@ -397,7 +399,7 @@ absl::Status CppGenerator::ValidateFeatures(const FileDescriptor* file) const {
}
if (field.type() != FieldDescriptor::TYPE_STRING &&
!IsStringMapType(field) &&
raw_features.GetExtension(::pb::cpp).has_utf8_validation()) {
unresolved_features.has_utf8_validation()) {
status = absl::FailedPreconditionError(
absl::StrCat("Field ", field.full_name(),
" specifies the utf8_validation feature but is not of "

@ -72,6 +72,7 @@
#include "absl/strings/string_view.h"
#include "absl/synchronization/mutex.h"
#include "absl/types/optional.h"
#include "google/protobuf/extension_set.h"
#include "google/protobuf/port.h"
// Must be included last.
@ -261,9 +262,10 @@ class PROTOBUF_EXPORT SymbolBase {
template <int N>
class PROTOBUF_EXPORT SymbolBaseN : public SymbolBase {};
// This class is for internal use only and provides access to the FeatureSets
// defined on descriptors. These features are not designed to be stable, and
// depending directly on them (vs the public descriptor APIs) is not safe.
// This class is for internal use only and provides access to the resolved
// runtime FeatureSets of any descriptor. These features are not designed
// to be stable, and depending directly on them (vs the public descriptor APIs)
// is not safe.
class PROTOBUF_EXPORT InternalFeatureHelper {
public:
template <typename DescriptorT>
@ -275,14 +277,18 @@ class PROTOBUF_EXPORT InternalFeatureHelper {
friend class ::google::protobuf::compiler::CodeGenerator;
friend class ::google::protobuf::compiler::CommandLineInterface;
// Provides a restricted view exclusively to code generators. Raw features
// haven't been resolved, and are virtually meaningless to everyone else. Code
// generators will need them to validate their own features, and runtimes may
// need them internally to be able to properly represent the original proto
// files from generated code.
template <typename DescriptorT>
static const FeatureSet& GetRawFeatures(const DescriptorT& desc) {
return *desc.proto_features_;
// Provides a restricted view exclusively to code generators to query their
// own unresolved features. Unresolved features are virtually meaningless to
// everyone else. Code generators will need them to validate their own
// features, and runtimes may need them internally to be able to properly
// represent the original proto files from generated code.
template <typename DescriptorT, typename TypeTraitsT, uint8_t field_type,
bool is_packed>
static typename TypeTraitsT::ConstType GetUnresolvedFeatures(
const DescriptorT& descriptor,
const google::protobuf::internal::ExtensionIdentifier<
FeatureSet, TypeTraitsT, field_type, is_packed>& extension) {
return descriptor.proto_features_->GetExtension(extension);
}
};

Loading…
Cancel
Save