diff --git a/src/google/protobuf/compiler/cpp/enum.cc b/src/google/protobuf/compiler/cpp/enum.cc index 0a168f8ecca9d..421e405cf193c 100644 --- a/src/google/protobuf/compiler/cpp/enum.cc +++ b/src/google/protobuf/compiler/cpp/enum.cc @@ -46,7 +46,11 @@ absl::flat_hash_map EnumVars( return { {"DEPRECATED", enum_->options().deprecated() ? "[[deprecated]]" : ""}, {"Enum", std::string(enum_->name())}, - {"Enum_", ResolveKeyword(enum_->name())}, + {"Enum_", ResolveKnownNameCollisions(enum_->name(), + enum_->containing_type() != nullptr + ? NameContext::kMessage + : NameContext::kFile, + NameKind::kType)}, {"Msg_Enum", classname}, {"::Msg_Enum", QualifiedClassName(enum_, options)}, {"Msg_Enum_", diff --git a/src/google/protobuf/compiler/cpp/extension.cc b/src/google/protobuf/compiler/cpp/extension.cc index 40fed905f35ef..87a3a2cd3ff38 100644 --- a/src/google/protobuf/compiler/cpp/extension.cc +++ b/src/google/protobuf/compiler/cpp/extension.cc @@ -61,7 +61,11 @@ ExtensionGenerator::ExtensionGenerator(const FieldDescriptor* descriptor, variables_["extendee"] = QualifiedClassName(descriptor_->containing_type(), options_); variables_["type_traits"] = type_traits_; - variables_["name"] = ResolveKeyword(descriptor_->name()); + variables_["name"] = ResolveKnownNameCollisions( + descriptor_->name(), + descriptor_->extension_scope() != nullptr ? NameContext::kMessage + : NameContext::kFile, + NameKind::kValue); variables_["constant_name"] = FieldConstantName(descriptor_); variables_["field_type"] = absl::StrCat(static_cast(descriptor_->type())); diff --git a/src/google/protobuf/compiler/cpp/helpers.cc b/src/google/protobuf/compiler/cpp/helpers.cc index 6eaa38fb9cb0c..f6fd444b5915d 100644 --- a/src/google/protobuf/compiler/cpp/helpers.cc +++ b/src/google/protobuf/compiler/cpp/helpers.cc @@ -73,6 +73,50 @@ namespace { constexpr absl::string_view kAnyMessageName = "Any"; constexpr absl::string_view kAnyProtoFile = "google/protobuf/any.proto"; +const absl::flat_hash_set& FileScopeKnownNames() { + static constexpr const char* kValue[] = { + "swap", + }; + static const auto* const methods = new absl::flat_hash_set( + std::begin(kValue), std::end(kValue)); + return *methods; +} + +const absl::flat_hash_set& MessageKnownMethodsCamelCase() { + static constexpr const char* kMessageKnownMethods[] = { + "GetDescriptor", "GetReflection", "default_instance", + "Swap", "UnsafeArenaSwap", "New", + "CopyFrom", "MergeFrom", "IsInitialized", + "GetMetadata", "Clear", + }; + static const auto* const methods = new absl::flat_hash_set( + std::begin(kMessageKnownMethods), std::end(kMessageKnownMethods)); + return *methods; +} + +const absl::flat_hash_set& +MessageKnownNullaryMethodsSnakeCase() { + static constexpr const char* kMessageKnownMethods[] = { + "unknown_fields", + "mutable_unknown_fields", + "descriptor", + "default_instance", + }; + static const auto* const methods = new absl::flat_hash_set( + std::begin(kMessageKnownMethods), std::end(kMessageKnownMethods)); + return *methods; +} + +const absl::flat_hash_set& +MessageKnownNonNullaryMethodsSnakeCase() { + static constexpr const char* kMessageKnownMethods[] = { + "swap", + }; + static const auto* const methods = new absl::flat_hash_set( + std::begin(kMessageKnownMethods), std::end(kMessageKnownMethods)); + return *methods; +} + static const char* const kKeywordList[] = { // clang-format off "NULL", @@ -406,12 +450,14 @@ std::string ClassName(const Descriptor* descriptor) { if (parent) absl::StrAppend(&res, ClassName(parent), "_"); absl::StrAppend(&res, descriptor->name()); if (IsMapEntryMessage(descriptor)) absl::StrAppend(&res, "_DoNotUse"); - return ResolveKeyword(res); + // This is the mangled message name which always goes in file scope. + return ResolveKnownNameCollisions(res, NameContext::kFile, NameKind::kType); } std::string ClassName(const EnumDescriptor* enum_descriptor) { if (enum_descriptor->containing_type() == nullptr) { - return ResolveKeyword(enum_descriptor->name()); + return ResolveKnownNameCollisions(enum_descriptor->name(), + NameContext::kFile, NameKind::kType); } else { return absl::StrCat(ClassName(enum_descriptor->containing_type()), "_", enum_descriptor->name()); @@ -436,9 +482,14 @@ std::string QualifiedClassName(const EnumDescriptor* d) { } std::string ExtensionName(const FieldDescriptor* d) { - if (const Descriptor* scope = d->extension_scope()) - return absl::StrCat(ClassName(scope), "::", ResolveKeyword(d->name())); - return ResolveKeyword(d->name()); + if (const Descriptor* scope = d->extension_scope()) { + return absl::StrCat( + ClassName(scope), "::", + ResolveKnownNameCollisions(d->name(), NameContext::kMessage, + NameKind::kValue)); + } + return ResolveKnownNameCollisions(d->name(), NameContext::kFile, + NameKind::kValue); } std::string QualifiedExtensionName(const FieldDescriptor* d, @@ -452,23 +503,53 @@ std::string QualifiedExtensionName(const FieldDescriptor* d) { } std::string ResolveKeyword(absl::string_view name) { - if (Keywords().count(name) > 0) { + if (Keywords().contains(name)) { return absl::StrCat(name, "_"); } return std::string(name); } -std::string DotsToColons(absl::string_view name) { - std::vector scope = absl::StrSplit(name, '.', absl::SkipEmpty()); - for (auto& word : scope) { - word = ResolveKeyword(word); +std::string ResolveKnownNameCollisions(absl::string_view name, + NameContext name_context, + NameKind name_kind) { + const auto has_conflict = [&] { + if (Keywords().contains(name)) return true; + + switch (name_kind) { + // We assume the style guide: types are CamelCase, fields are snake_case. + case NameKind::kType: + // Types can't overload names of existing functions. + return MessageKnownMethodsCamelCase().contains(name); + case NameKind::kValue: + if (name_context == NameContext::kFile) { + // At file scope we don't have the normal names, except a few. + return FileScopeKnownNames().contains(name); + } + // Values can't overload names of existing functions. + return MessageKnownNullaryMethodsSnakeCase().contains(name) || + MessageKnownNonNullaryMethodsSnakeCase().contains(name); + case NameKind::kFunction: + // For functions, we can't overload existing nullary functions. + // Non-nullary functions are fine. + return MessageKnownNullaryMethodsSnakeCase().contains(name); + } + return false; + }; + if (has_conflict()) { + return absl::StrCat(name, "_"); } - return absl::StrJoin(scope, "::"); + return std::string(name); } std::string Namespace(absl::string_view package) { if (package.empty()) return ""; - return absl::StrCat("::", DotsToColons(package)); + + std::vector scope = + absl::StrSplit(package, '.', absl::SkipEmpty()); + for (auto& word : scope) { + word = ResolveKeyword(word); + } + return absl::StrCat("::", absl::StrJoin(scope, "::")); } std::string Namespace(const FileDescriptor* d) { return Namespace(d, {}); } @@ -555,12 +636,17 @@ std::string SuperClassName(const Descriptor* descriptor, } std::string FieldName(const FieldDescriptor* field) { + if (field->containing_type() != nullptr && + field->containing_type()->options().no_standard_descriptor_accessor() && + field->name() == "descriptor") { + // Special case for `optional no_standard_descriptor_accessor = true;` + return "descriptor"; + } std::string result = std::string(field->name()); absl::AsciiStrToLower(&result); - if (Keywords().count(result) > 0) { - result.append("_"); - } - return result; + ABSL_CHECK(field->containing_type() != nullptr); + return ResolveKnownNameCollisions(result, NameContext::kMessage, + NameKind::kFunction); } std::string FieldMemberName(const FieldDescriptor* field, bool split) { @@ -589,11 +675,7 @@ std::string QualifiedOneofCaseConstantName(const FieldDescriptor* field) { } std::string EnumValueName(const EnumValueDescriptor* enum_value) { - std::string result = std::string(enum_value->name()); - if (Keywords().count(result) > 0) { - result.append("_"); - } - return result; + return ResolveKeyword(enum_value->name()); } int EstimateAlignmentSize(const FieldDescriptor* field) { diff --git a/src/google/protobuf/compiler/cpp/helpers.h b/src/google/protobuf/compiler/cpp/helpers.h index c8078e15e452f..76ebd04b09277 100644 --- a/src/google/protobuf/compiler/cpp/helpers.h +++ b/src/google/protobuf/compiler/cpp/helpers.h @@ -188,6 +188,24 @@ std::string FileDllExport(const FileDescriptor* file, const Options& options); std::string SuperClassName(const Descriptor* descriptor, const Options& options); +// Add an underscore if necessary to prevent conflicting with known names and +// keywords. +// We use the context and the kind of entity to try to determine if mangling is +// necessary or not. +// For example, a message named `New` at file scope is fine, but at message +// scope it needs mangling because it collides with the `New` function. +enum class NameContext { + kFile, + kMessage, +}; +enum class NameKind { + kType, + kFunction, + kValue, +}; +std::string ResolveKnownNameCollisions(absl::string_view name, + NameContext name_context, + NameKind name_kind); // Adds an underscore if necessary to prevent conflicting with a keyword. std::string ResolveKeyword(absl::string_view name); diff --git a/src/google/protobuf/compiler/cpp/message.cc b/src/google/protobuf/compiler/cpp/message.cc index 58a2fab375d69..f5ac295c41992 100644 --- a/src/google/protobuf/compiler/cpp/message.cc +++ b/src/google/protobuf/compiler/cpp/message.cc @@ -1976,7 +1976,10 @@ void MessageGenerator::GenerateClassDefinition(io::Printer* p) { { Sub{"nested_full_name", ClassName(nested_type, false)} .AnnotatedAs(nested_type), - Sub{"nested_name", ResolveKeyword(nested_type->name())} + Sub{"nested_name", + ResolveKnownNameCollisions(nested_type->name(), + NameContext::kMessage, + NameKind::kType)} .AnnotatedAs(nested_type), }, R"cc( diff --git a/src/google/protobuf/compiler/cpp/test_bad_identifiers.proto b/src/google/protobuf/compiler/cpp/test_bad_identifiers.proto index e375362aaa60d..d06794468dbef 100644 --- a/src/google/protobuf/compiler/cpp/test_bad_identifiers.proto +++ b/src/google/protobuf/compiler/cpp/test_bad_identifiers.proto @@ -115,6 +115,41 @@ message TestConflictingSymbolNames { optional uint32 typedecl = 39; optional uint32 auto = 40; + // Methods generated in the parent type + message BadKnownNamesFields { + optional int32 unknown_fields = 1; + optional int32 mutable_unknown_fields = 2; + optional int32 descriptor = 3; + optional int32 default_instance = 4; + optional int32 swap = 5; + } + message BadKnownNamesFieldsNoStandardDescriptor { + option no_standard_descriptor_accessor = true; + optional int32 descriptor = 3; + } + message BadKnownNamesTypes { + message GetDescriptor {} + message GetReflection {} + message Swap {} + message UnsafeArenaSwap {} + message New {} + message CopyFrom {} + message MergeFrom {} + message GetMetadata {} + message Clear {} + message IsInitialized {} + } + message BadKnownNamesValues { // NO_PROTO3 + extensions 1 to 100; // NO_PROTO3 + extend BadKnownNamesValues { // NO_PROTO3 + optional int32 unknown_fields = 1; // NO_PROTO3 + optional int32 mutable_unknown_fields = 2; // NO_PROTO3 + optional int32 descriptor = 3; // NO_PROTO3 + optional int32 default_instance = 4; // NO_PROTO3 + optional int32 swap = 5; // NO_PROTO3 + } // NO_PROTO3 + } // NO_PROTO3 + // The generator used to #define a macro called "DO" inside the .cc file. message DO {} optional DO do = 32; @@ -137,6 +172,26 @@ message TestConflictingSymbolNames { extensions 1000 to max [verification = UNVERIFIED]; // NO_PROTO3 } +// Special names as above, but at file scope. +message GetDescriptor {} +message GetReflection {} +message Swap {} +message UnsafeArenaSwap {} +message New {} +message CopyFrom {} +message MergeFrom {} +message GetMetadata {} +message Clear {} +message IsInitialized {} + +extend TestConflictingSymbolNames.BadKnownNamesValues { // NO_PROTO3 + optional int32 unknown_fields = 11; // NO_PROTO3 + optional int32 mutable_unknown_fields = 12; // NO_PROTO3 + optional int32 descriptor = 13; // NO_PROTO3 + optional int32 default_instance = 14; // NO_PROTO3 + optional int32 swap = 15; // NO_PROTO3 +} // NO_PROTO3 + message TestConflictingSymbolNamesExtension { // NO_PROTO3 extend TestConflictingSymbolNames { // NO_PROTO3 repeated int32 repeated_int32_ext = 20423638 [packed = true]; // NO_PROTO3 diff --git a/src/google/protobuf/compiler/cpp/unittest.cc b/src/google/protobuf/compiler/cpp/unittest.cc index 31a34948c1de7..3f94360f5a8af 100644 --- a/src/google/protobuf/compiler/cpp/unittest.cc +++ b/src/google/protobuf/compiler/cpp/unittest.cc @@ -75,6 +75,41 @@ TEST(GENERATED_MESSAGE_TEST_NAME, TestConflictingSymbolNames) { EXPECT_EQ(123, message.GetExtension(ExtensionMessage::repeated_int32_ext, 0)); } +TEST(GENERATED_MESSAGE_TEST_NAME, TestSwapNameIsNotMangledForFields) { + // For backwards compatibility we do not mangle `swap`. It works thanks to + // overload resolution. + int v [[maybe_unused]] = + protobuf_unittest::TestConflictingSymbolNames::BadKnownNamesFields().swap(); + + // But we do mangle `swap` for extensions because there is no overloading + // there. + v = protobuf_unittest::TestConflictingSymbolNames::BadKnownNamesValues() + .GetExtension(protobuf_unittest::TestConflictingSymbolNames:: + BadKnownNamesValues::swap_); +} + +TEST(GENERATED_MESSAGE_TEST_NAME, TestNoStandardDescriptorOption) { + // When no_standard_descriptor_accessor = true, we should not mangle fields + // named `descriptor`. + int v [[maybe_unused]] = + protobuf_unittest::TestConflictingSymbolNames::BadKnownNamesFields() + .descriptor_(); + v = protobuf_unittest::TestConflictingSymbolNames:: + BadKnownNamesFieldsNoStandardDescriptor() + .descriptor(); +} + +TEST(GENERATED_MESSAGE_TEST_NAME, TestFileVsMessageScope) { + // Special names at message scope are mangled, + int v [[maybe_unused]] = + protobuf_unittest::TestConflictingSymbolNames::BadKnownNamesValues() + .GetExtension(protobuf_unittest::TestConflictingSymbolNames:: + BadKnownNamesValues::unknown_fields_); + // But not at file scope. + v = protobuf_unittest::TestConflictingSymbolNames::BadKnownNamesValues() + .GetExtension(protobuf_unittest::unknown_fields); +} + TEST(GENERATED_MESSAGE_TEST_NAME, TestConflictingEnumNames) { protobuf_unittest::TestConflictingEnumNames message; message.set_conflicting_enum(