From 0fa12dce2a1f72f737cdb40dc0be229ef938237f Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Thu, 20 Apr 2023 13:00:38 -0700 Subject: [PATCH] Fix field name conflict resolution for `has_` prefix. Note: Code looks duplicated but in C case, it is for performance. For C++, C++ and C may diverge in the future for certain methods. PiperOrigin-RevId: 525826831 --- protos_generator/gen_accessors.cc | 17 ++++++++++------ protos_generator/tests/BUILD | 14 +++++++++++++ protos_generator/tests/naming_conflict.proto | 8 ++++++++ upbc/names.cc | 21 ++++++++++++-------- 4 files changed, 46 insertions(+), 14 deletions(-) create mode 100644 protos_generator/tests/naming_conflict.proto diff --git a/protos_generator/gen_accessors.cc b/protos_generator/gen_accessors.cc index 8f498439e5..7402e3afd8 100644 --- a/protos_generator/gen_accessors.cc +++ b/protos_generator/gen_accessors.cc @@ -735,16 +735,21 @@ std::string ResolveFieldName(const protobuf::FieldDescriptor* field, "arena_", }); - static constexpr absl::string_view kClearAccessor = "clear_"; - static constexpr absl::string_view kSetAccessor = "set_"; + // C++ specific prefixes used by code generator for field access. + static constexpr absl::string_view kClearMethodPrefix = "clear_"; + static constexpr absl::string_view kSetMethodPrefix = "set_"; + static constexpr absl::string_view kHasMethodPrefix = "has_"; + static constexpr absl::string_view kDeleteMethodPrefix = "delete_"; + static constexpr absl::string_view kAddToRepeatedMethodPrefix = "add_"; + static constexpr absl::string_view kResizeArrayMethodPrefix = "resize_"; // List of generated accessor prefixes to check against. // Example: // optional repeated string phase = 236; // optional bool clear_phase = 237; static constexpr absl::string_view kAccessorPrefixes[] = { - kClearAccessor, "delete_", "add_", "resize_", kSetAccessor, - }; + kClearMethodPrefix, kDeleteMethodPrefix, kAddToRepeatedMethodPrefix, + kResizeArrayMethodPrefix, kSetMethodPrefix, kHasMethodPrefix}; absl::string_view field_name = field->name(); if (kReservedNames.count(field_name) > 0) { @@ -765,8 +770,8 @@ std::string ResolveFieldName(const protobuf::FieldDescriptor* field, if (candidate->is_repeated() || candidate->is_map() || (candidate->cpp_type() == protobuf::FieldDescriptor::CPPTYPE_STRING && - prefix == kClearAccessor) || - prefix == kSetAccessor) { + prefix == kClearMethodPrefix) || + prefix == kSetMethodPrefix || prefix == kHasMethodPrefix) { return absl::StrCat(field_name, "_"); } } diff --git a/protos_generator/tests/BUILD b/protos_generator/tests/BUILD index 1ea227ca47..a854eaca72 100644 --- a/protos_generator/tests/BUILD +++ b/protos_generator/tests/BUILD @@ -59,6 +59,13 @@ proto_library( ], ) +proto_library( + name = "naming_conflict_proto", + srcs = [ + "naming_conflict.proto", + ], +) + proto_library( name = "no_package_enum_user_proto", srcs = [ @@ -81,6 +88,12 @@ upb_cc_proto_library( deps = [":test_model_proto"], ) +upb_cc_proto_library( + name = "naming_conflict_upb_cc_proto", + visibility = ["//protos:__pkg__"], + deps = [":naming_conflict_proto"], +) + upb_cc_proto_library( name = "no_package_upb_cc_proto", deps = [ @@ -128,6 +141,7 @@ cc_test( ":no_package_upb_cc_proto", ":test_model_upb_cc_proto", ":test_model_upb_proto", + ":naming_conflict_upb_cc_proto", "@com_google_googletest//:gtest_main", "//:upb", "//protos", diff --git a/protos_generator/tests/naming_conflict.proto b/protos_generator/tests/naming_conflict.proto new file mode 100644 index 0000000000..35c57e3cb8 --- /dev/null +++ b/protos_generator/tests/naming_conflict.proto @@ -0,0 +1,8 @@ +syntax = "proto3"; + +package protos_generator.test; + +message HasChildCount { + optional HasChildCount has_child_count = 1; + optional int32 child_count = 2; +} diff --git a/upbc/names.cc b/upbc/names.cc index 6a74c05784..21a5766b31 100644 --- a/upbc/names.cc +++ b/upbc/names.cc @@ -37,16 +37,21 @@ namespace upbc { namespace protobuf = ::google::protobuf; -static constexpr absl::string_view kClearAccessor = "clear_"; -static constexpr absl::string_view kSetAccessor = "set_"; +// Prefixes used by C code generator for field access. +static constexpr absl::string_view kClearMethodPrefix = "clear_"; +static constexpr absl::string_view kSetMethodPrefix = "set_"; +static constexpr absl::string_view kHasMethodPrefix = "has_"; +static constexpr absl::string_view kDeleteMethodPrefix = "delete_"; +static constexpr absl::string_view kAddToRepeatedMethodPrefix = "add_"; +static constexpr absl::string_view kResizeArrayMethodPrefix = "resize_"; // List of generated accessor prefixes to check against. // Example: // optional repeated string phase = 236; // optional bool clear_phase = 237; static constexpr absl::string_view kAccessorPrefixes[] = { - kClearAccessor, "delete_", "add_", "resize_", kSetAccessor, -}; + kClearMethodPrefix, kDeleteMethodPrefix, kAddToRepeatedMethodPrefix, + kResizeArrayMethodPrefix, kSetMethodPrefix, kHasMethodPrefix}; std::string ResolveFieldName(const protobuf::FieldDescriptor* field, const NameToFieldDescriptorMap& field_names) { @@ -62,8 +67,8 @@ std::string ResolveFieldName(const protobuf::FieldDescriptor* field, if (candidate->is_repeated() || candidate->is_map() || (candidate->cpp_type() == protobuf::FieldDescriptor::CPPTYPE_STRING && - prefix == kClearAccessor) || - prefix == kSetAccessor) { + prefix == kClearMethodPrefix) || + prefix == kSetMethodPrefix || prefix == kHasMethodPrefix) { return absl::StrCat(field_name, "_"); } } @@ -105,8 +110,8 @@ std::string ResolveFieldName(upb::FieldDefPtr field, const auto candidate = match->second; if (candidate.IsSequence() || candidate.IsMap() || (candidate.ctype() == kUpb_CType_String && - prefix == kClearAccessor) || - prefix == kSetAccessor) { + prefix == kClearMethodPrefix) || + prefix == kSetMethodPrefix || prefix == kHasMethodPrefix) { return absl::StrCat(field_name, "_"); } }