From 6250d09c426f5b42b7d9f954fc73d24c644a221e Mon Sep 17 00:00:00 2001 From: zhangskz Date: Wed, 5 Feb 2025 15:26:43 -0500 Subject: [PATCH] Replace `std::any` with a custom solution. (#20251) `std::any` leads to compiler errors in some versions of gcc during constructibility trait checks. Now that we can guarantee it, return by reference to avoid extra costs in copies. PiperOrigin-RevId: 723478744 Co-authored-by: Protobuf Team Bot --- src/google/protobuf/descriptor.h | 33 ++++++++++++++++------ src/google/protobuf/descriptor_unittest.cc | 14 ++++++--- 2 files changed, 35 insertions(+), 12 deletions(-) diff --git a/src/google/protobuf/descriptor.h b/src/google/protobuf/descriptor.h index 806337e328..f52182de6f 100644 --- a/src/google/protobuf/descriptor.h +++ b/src/google/protobuf/descriptor.h @@ -31,7 +31,6 @@ #ifndef GOOGLE_PROTOBUF_DESCRIPTOR_H__ #define GOOGLE_PROTOBUF_DESCRIPTOR_H__ -#include #include #include #include @@ -2455,11 +2454,20 @@ class PROTOBUF_EXPORT DescriptorPool { friend class google::protobuf::descriptor_unittest::ValidationErrorTest; friend class ::google::protobuf::compiler::CommandLineInterface; friend class TextFormat; + + struct MemoBase { + virtual ~MemoBase() = default; + }; + template + struct MemoData : MemoBase { + T value; + }; + // Memoize a projection of a field. This is used to cache the results of // calling a function on a field, used for expensive descriptor calculations. template - auto MemoizeProjection(const FieldDescriptor* field, Func func) const { - using ResultT = decltype(func(field)); + const auto& MemoizeProjection(const FieldDescriptor* field, Func func) const { + using ResultT = std::decay_t; ABSL_DCHECK(field->file()->pool() == this); static_assert(std::is_empty_v); // This static bool is unique per-Func, so its address can be used as a key. @@ -2469,15 +2477,21 @@ class PROTOBUF_EXPORT DescriptorPool { absl::ReaderMutexLock lock(&field_memo_table_mutex_); auto it = field_memo_table_.find(key); if (it != field_memo_table_.end()) { - return std::any_cast(it->second); + return internal::DownCast&>(*it->second).value; } } - ResultT result = func(field); + auto result = std::make_unique>(); + result->value = func(field); { absl::MutexLock lock(&field_memo_table_mutex_); - field_memo_table_[key] = result; + auto& res = field_memo_table_[key]; + // Only initialize the first time. We don't want to invalidate old + // references. + if (res == nullptr) { + res = std::move(result); + } + return internal::DownCast&>(*res).value; } - return result; } // Return true if the given name is a sub-symbol of any non-package // descriptor that already exists in the descriptor pool. (The full @@ -2537,9 +2551,12 @@ class PROTOBUF_EXPORT DescriptorPool { Symbol NewPlaceholderWithMutexHeld(absl::string_view name, PlaceholderType placeholder_type) const; +#ifndef SWIG mutable absl::Mutex field_memo_table_mutex_; - mutable absl::flat_hash_map, std::any> + mutable absl::flat_hash_map, + std::unique_ptr> field_memo_table_ ABSL_GUARDED_BY(field_memo_table_mutex_); +#endif // SWIG // If fallback_database_ is nullptr, this is nullptr. Otherwise, this is a // mutex which must be locked while accessing tables_. diff --git a/src/google/protobuf/descriptor_unittest.cc b/src/google/protobuf/descriptor_unittest.cc index 8650815acd..240fe223bd 100644 --- a/src/google/protobuf/descriptor_unittest.cc +++ b/src/google/protobuf/descriptor_unittest.cc @@ -27,6 +27,7 @@ #include #include // NOLINT #include +#include #include #include @@ -11819,8 +11820,8 @@ TEST_F(DescriptorPoolFeaturesTest, ResolvesFeaturesFor) { class DescriptorPoolMemoizationTest : public ::testing::Test { protected: template - auto MemoizeProjection(const DescriptorPool* pool, - const FieldDescriptor* field, Func func) { + const auto& MemoizeProjection(const DescriptorPool* pool, + const FieldDescriptor* field, Func func) { return pool->MemoizeProjection(field, func); }; }; @@ -11834,15 +11835,20 @@ TEST_F(DescriptorPoolMemoizationTest, MemoizeProjectionBasic) { proto2_unittest::TestAllTypes message; const Descriptor* descriptor = message.GetDescriptor(); - auto name = DescriptorPoolMemoizationTest::MemoizeProjection( + const auto& name = DescriptorPoolMemoizationTest::MemoizeProjection( descriptor->file()->pool(), descriptor->field(0), name_lambda); - auto dupe_name = DescriptorPoolMemoizationTest::MemoizeProjection( + const auto& dupe_name = DescriptorPoolMemoizationTest::MemoizeProjection( descriptor->file()->pool(), descriptor->field(0), name_lambda); ASSERT_EQ(counter, 1); ASSERT_EQ(name, "proto2_unittest.TestAllTypes.optional_int32"); ASSERT_EQ(dupe_name, "proto2_unittest.TestAllTypes.optional_int32"); + // Check that they are references aliasing the same object. + EXPECT_TRUE( + (std::is_same_vname()) &>)); + EXPECT_EQ(&name, &dupe_name); + auto other_name = DescriptorPoolMemoizationTest::MemoizeProjection( descriptor->file()->pool(), descriptor->field(1), name_lambda);