From 42c88bf5cee4e20ca8f91cf1ff9ec716bfab876d Mon Sep 17 00:00:00 2001 From: Samuel Benzaquen Date: Tue, 4 Aug 2026 06:55:36 -0700 Subject: [PATCH] Change DefPool Find methods to take absl::string_view. PiperOrigin-RevId: 958995646 --- .../protobuf/compiler/rust/extension.cc | 3 +- .../protobuf/compiler/rust/generator.cc | 3 +- src/google/protobuf/compiler/rust/message.cc | 9 ++-- upb/reflection/BUILD | 2 + upb/reflection/def.hpp | 21 +++++--- upb/reflection/reflection_test.cc | 54 +++++++++++++++++++ upb_generator/common/cpp_to_upb_def.cc | 17 +++--- upb_generator/file_layout.h | 2 +- 8 files changed, 83 insertions(+), 28 deletions(-) diff --git a/src/google/protobuf/compiler/rust/extension.cc b/src/google/protobuf/compiler/rust/extension.cc index a691b9c5f8..8003a07de7 100644 --- a/src/google/protobuf/compiler/rust/extension.cc +++ b/src/google/protobuf/compiler/rust/extension.cc @@ -114,8 +114,7 @@ void GenerateRs(Context& ctx, const FieldDescriptor& extension, } } else { std::string mini_descriptor = - pool.FindExtensionByName(std::string(extension.full_name()).c_str()) - .MiniDescriptorEncode(); + pool.FindExtensionByName(extension.full_name()).MiniDescriptorEncode(); std::string extendee = RsTypePath(ctx, *extension.containing_type()); std::string number = absl::StrCat(extension.number()); diff --git a/src/google/protobuf/compiler/rust/generator.cc b/src/google/protobuf/compiler/rust/generator.cc index 05704ed2d0..b57459e3dc 100644 --- a/src/google/protobuf/compiler/rust/generator.cc +++ b/src/google/protobuf/compiler/rust/generator.cc @@ -323,8 +323,7 @@ bool RustGenerator::Generate(const FileDescriptor* file, for (int i = 0; i < file->enum_type_count(); ++i) { auto& enum_ = *file->enum_type(i); - GenerateEnumDefinition(ctx, enum_, - pool.FindEnumByName(enum_.full_name().data())); + GenerateEnumDefinition(ctx, enum_, pool.FindEnumByName(enum_.full_name())); ctx.printer().PrintRaw("\n"); if (ctx.is_cpp()) { diff --git a/src/google/protobuf/compiler/rust/message.cc b/src/google/protobuf/compiler/rust/message.cc index e00c97388f..b9754ff5bf 100644 --- a/src/google/protobuf/compiler/rust/message.cc +++ b/src/google/protobuf/compiler/rust/message.cc @@ -224,8 +224,7 @@ void UpbGeneratedMessageTraitImpls(Context& ctx, const Descriptor& msg, if (scc.GetRepresentative() == &msg) { for (const Descriptor* d : scc.descriptors) { std::string mini_descriptor = - pool.FindMessageByName(d->full_name().data()) - .MiniDescriptorEncode(); + pool.FindMessageByName(d->full_name()).MiniDescriptorEncode(); ctx.Emit({{"name", RsTypePath(ctx, *d)}, {"minitable_symbol_name", QualifiedUpbMiniTableName(ctx, *d)}, @@ -237,8 +236,8 @@ void UpbGeneratedMessageTraitImpls(Context& ctx, const Descriptor& msg, )rs"); } for (const Descriptor* d : scc.descriptors) { - UpbMiniTableLinking( - ctx, *d, pool.FindMessageByName(d->full_name().data()), scc); + UpbMiniTableLinking(ctx, *d, + pool.FindMessageByName(d->full_name()), scc); } } else { ctx.Emit( @@ -406,7 +405,7 @@ void GenerateRs(Context& ctx, const Descriptor& msg, const upb::DefPool& pool) { return; } - upb::MessageDefPtr upb_msg = pool.FindMessageByName(msg.full_name().data()); + upb::MessageDefPtr upb_msg = pool.FindMessageByName(msg.full_name()); ctx.Emit( { // There's also ${$/$}$-style begin and end tokens, but those might diff --git a/upb/reflection/BUILD b/upb/reflection/BUILD index fc8aa3a9d6..2cfb75b700 100644 --- a/upb/reflection/BUILD +++ b/upb/reflection/BUILD @@ -94,6 +94,7 @@ bootstrap_cc_library( "//upb/mini_table", "//upb/port", "@abseil-cpp//absl/log:absl_check", + "@abseil-cpp//absl/strings:string_view", ], ) @@ -197,6 +198,7 @@ bootstrap_cc_library( "//upb/mini_table", "//upb/port", "@abseil-cpp//absl/log:absl_check", + "@abseil-cpp//absl/strings:string_view", ], ) diff --git a/upb/reflection/def.hpp b/upb/reflection/def.hpp index b3991b6ec4..ba9776e355 100644 --- a/upb/reflection/def.hpp +++ b/upb/reflection/def.hpp @@ -15,6 +15,7 @@ #include #include "absl/log/absl_check.h" +#include "absl/strings/string_view.h" #include "upb/base/descriptor_constants.h" #include "upb/base/status.hpp" #include "upb/base/string_view.h" @@ -570,20 +571,24 @@ class DefPool { // Finds an entry in the symbol table with this exact name. If not found, // returns NULL. - MessageDefPtr FindMessageByName(const char* sym) const { - return MessageDefPtr(upb_DefPool_FindMessageByName(ptr_.get(), sym)); + MessageDefPtr FindMessageByName(absl::string_view sym) const { + return MessageDefPtr(upb_DefPool_FindMessageByNameWithSize( + ptr_.get(), sym.data(), sym.size())); } - EnumDefPtr FindEnumByName(const char* sym) const { - return EnumDefPtr(upb_DefPool_FindEnumByName(ptr_.get(), sym)); + EnumDefPtr FindEnumByName(absl::string_view sym) const { + return EnumDefPtr( + upb_DefPool_FindEnumByNameWithSize(ptr_.get(), sym.data(), sym.size())); } - FileDefPtr FindFileByName(const char* name) const { - return FileDefPtr(upb_DefPool_FindFileByName(ptr_.get(), name)); + FileDefPtr FindFileByName(absl::string_view name) const { + return FileDefPtr(upb_DefPool_FindFileByNameWithSize( + ptr_.get(), name.data(), name.size())); } - FieldDefPtr FindExtensionByName(const char* name) const { - return FieldDefPtr(upb_DefPool_FindExtensionByName(ptr_.get(), name)); + FieldDefPtr FindExtensionByName(absl::string_view name) const { + return FieldDefPtr(upb_DefPool_FindExtensionByNameWithSize( + ptr_.get(), name.data(), name.size())); } void _SetPlatform(upb_MiniTablePlatform platform) { diff --git a/upb/reflection/reflection_test.cc b/upb/reflection/reflection_test.cc index 4dd55d4b4a..2efa70a190 100644 --- a/upb/reflection/reflection_test.cc +++ b/upb/reflection/reflection_test.cc @@ -461,5 +461,59 @@ TEST(ReflectionTest, EnumCustomJsonNameAliasedSameNumberSucceeds) { EXPECT_TRUE(status.ok()) << status.message(); } +TEST(ReflectionTest, DefPoolFindMethodsRespectSize) { + upb::DefPool pool = LoadDescriptorProto(R"pb( + syntax: "proto2" + name: "test.proto" + package: "pkg" + message_type { + name: "TestMessage" + extension_range { start: 1 end: 1000 } + } + enum_type { + name: "TestEnum" + value { name: "VAL" number: 1 } + } + extension { + name: "test_ext" + number: 100 + label: LABEL_OPTIONAL + type: TYPE_INT32 + extendee: ".pkg.TestMessage" + } + )pb") + .value(); + + // Test FindMessageByName + { + absl::string_view full_name = "pkg.TestMessage"; + EXPECT_TRUE(pool.FindMessageByName(full_name)); + EXPECT_FALSE( + pool.FindMessageByName(full_name.substr(0, 12))); // "pkg.TestMess" + } + + // Test FindEnumByName + { + absl::string_view full_name = "pkg.TestEnum"; + EXPECT_TRUE(pool.FindEnumByName(full_name)); + EXPECT_FALSE(pool.FindEnumByName(full_name.substr(0, 10))); // "pkg.TestEn" + } + + // Test FindFileByName + { + absl::string_view full_name = "test.proto"; + EXPECT_TRUE(pool.FindFileByName(full_name)); + EXPECT_FALSE(pool.FindFileByName(full_name.substr(0, 7))); // "test.pr" + } + + // Test FindExtensionByName + { + absl::string_view full_name = "pkg.test_ext"; + EXPECT_TRUE(pool.FindExtensionByName(full_name)); + EXPECT_FALSE( + pool.FindExtensionByName(full_name.substr(0, 11))); // "pkg.test_ex" + } +} + } // namespace } // namespace upb_test diff --git a/upb_generator/common/cpp_to_upb_def.cc b/upb_generator/common/cpp_to_upb_def.cc index 61665b5c54..faab4aba7b 100644 --- a/upb_generator/common/cpp_to_upb_def.cc +++ b/upb_generator/common/cpp_to_upb_def.cc @@ -39,8 +39,7 @@ google_protobuf_FileDescriptorProto* ToUpbProto(const FileDescriptor* file, void AddFile(const FileDescriptor* file, upb::DefPool* pool) { // Avoid adding the same file twice. - const std::string name(file->name()); - if (pool->FindFileByName(name.c_str())) return; + if (pool->FindFileByName(file->name())) return; // Like a google::protobuf::DescriptorPool, a upb::DefPool requires that all // dependencies are added first. @@ -56,17 +55,16 @@ void AddFile(const FileDescriptor* file, upb::DefPool* pool) { upb::MessageDefPtr FindMessageDef(upb::DefPool& pool, const Descriptor* descriptor) { - const std::string name(descriptor->full_name()); - upb::MessageDefPtr message_def = pool.FindMessageByName(name.c_str()); - ABSL_CHECK(message_def) << "No message named " << name; + upb::MessageDefPtr message_def = + pool.FindMessageByName(descriptor->full_name()); + ABSL_CHECK(message_def) << "No message named " << descriptor->full_name(); return message_def; } upb::EnumDefPtr FindEnumDef(upb::DefPool& pool, const EnumDescriptor* enum_descriptor) { - const std::string name(enum_descriptor->full_name()); - upb::EnumDefPtr enum_def = pool.FindEnumByName(name.c_str()); - ABSL_CHECK(enum_def) << "No enum named " << name; + upb::EnumDefPtr enum_def = pool.FindEnumByName(enum_descriptor->full_name()); + ABSL_CHECK(enum_def) << "No enum named " << enum_descriptor->full_name(); return enum_def; } @@ -84,8 +82,7 @@ upb::FieldDefPtr FindBaseFieldDef(upb::DefPool& pool, upb::FieldDefPtr FindExtensionDef(upb::DefPool& pool, const FieldDescriptor* field) { ABSL_CHECK(field->is_extension()); - const std::string name(field->full_name()); - return pool.FindExtensionByName(name.c_str()); + return pool.FindExtensionByName(field->full_name()); } const FieldDescriptor* FindFieldDescriptor( diff --git a/upb_generator/file_layout.h b/upb_generator/file_layout.h index 3054563e88..8877406a84 100644 --- a/upb_generator/file_layout.h +++ b/upb_generator/file_layout.h @@ -65,7 +65,7 @@ class DefPoolPair { } upb::FileDefPtr GetFile(absl::string_view name) const { - return pool64_.FindFileByName(name.data()); + return pool64_.FindFileByName(name); } const upb_MiniTable* GetMiniTable32(upb::MessageDefPtr m) const {