From df68462824476e928a79566dbc630b3a53e6a4c0 Mon Sep 17 00:00:00 2001 From: Justin King Date: Tue, 11 Mar 2025 10:59:36 -0700 Subject: [PATCH] Delete `Shared` and `SharedView` PiperOrigin-RevId: 735817036 --- common/BUILD | 7 - common/memory.h | 496 ---------- common/memory_test.cc | 854 +----------------- common/type_introspector.cc | 8 - common/type_introspector.h | 4 - common/type_manager.cc | 33 - common/type_manager.h | 5 - common/type_testing.h | 37 +- common/types/thread_compatible_type_manager.h | 45 - common/value.h | 16 - extensions/protobuf/BUILD | 1 - extensions/protobuf/type_introspector_test.cc | 42 +- 12 files changed, 27 insertions(+), 1521 deletions(-) delete mode 100644 common/type_manager.cc delete mode 100644 common/types/thread_compatible_type_manager.h diff --git a/common/BUILD b/common/BUILD index 5861e3db8..b5e9688ef 100644 --- a/common/BUILD +++ b/common/BUILD @@ -358,12 +358,6 @@ cc_library( name = "type_testing", testonly = True, hdrs = ["type_testing.h"], - deps = [ - ":memory", - ":memory_testing", - ":type", - "@com_google_absl//absl/types:optional", - ], ) cc_library( @@ -491,7 +485,6 @@ cc_library( ) + [ "type.cc", "type_introspector.cc", - "type_manager.cc", ], hdrs = glob( [ diff --git a/common/memory.h b/common/memory.h index 83e436a7f..f43439b44 100644 --- a/common/memory.h +++ b/common/memory.h @@ -33,7 +33,6 @@ #include "common/data.h" #include "common/internal/metadata.h" #include "common/internal/reference_count.h" -#include "common/native_type.h" #include "common/reference_count.h" #include "internal/exceptions.h" #include "internal/to_address.h" // IWYU pragma: keep @@ -61,10 +60,6 @@ std::ostream& operator<<(std::ostream& out, MemoryManagement memory_management); class ABSL_ATTRIBUTE_TRIVIAL_ABI [[nodiscard]] Owner; class Borrower; template -class ABSL_ATTRIBUTE_TRIVIAL_ABI Shared; -template -class ABSL_ATTRIBUTE_TRIVIAL_ABI SharedView; -template class ABSL_ATTRIBUTE_TRIVIAL_ABI [[nodiscard]] Unique; template class ABSL_ATTRIBUTE_TRIVIAL_ABI [[nodiscard]] Owned; @@ -74,8 +69,6 @@ template struct Ownable; template struct Borrowable; -template -struct EnableSharedFromThis; class MemoryManager; class ReferenceCountingMemoryManager; @@ -106,30 +99,8 @@ Owned WrapEternal(const T* value); inline constexpr uintptr_t kUniqueArenaUnownedBit = uintptr_t{1} << 0; inline constexpr uintptr_t kUniqueArenaBits = kUniqueArenaUnownedBit; inline constexpr uintptr_t kUniqueArenaPointerMask = ~kUniqueArenaBits; - -template -T* GetPointer(const Shared& shared); -template -const ReferenceCount* GetReferenceCount(const Shared& shared); -template -Shared MakeShared(AdoptRef, T* value, const ReferenceCount* refcount); -template -Shared MakeShared(T* value, const ReferenceCount* refcount); -template -T* GetPointer(SharedView shared); -template -const ReferenceCount* GetReferenceCount(SharedView shared); -template -SharedView MakeSharedView(T* value, const ReferenceCount* refcount); } // namespace common_internal -template -Shared StaticCast(const Shared& from); -template -Shared StaticCast(Shared&& from); -template -SharedView StaticCast(SharedView from); - template Owned AllocateShared(Allocator<> allocator, Args&&... args); @@ -1334,346 +1305,6 @@ struct Borrowable { } }; -// `Shared` points to an object allocated in memory which is managed by a -// `MemoryManager`. The pointed to object is valid so long as the managing -// `MemoryManager` is alive and one or more valid `Shared` exist pointing to the -// object. -// -// IMPLEMENTATION DETAILS: -// `Shared` is similar to `std::shared_ptr`, except that it works for -// region-based memory management as well. In that case the pointer to the -// reference count is `nullptr`. -template -class ABSL_ATTRIBUTE_TRIVIAL_ABI Shared final { - public: - Shared() = default; - - Shared(const Shared& other) - : value_(other.value_), refcount_(other.refcount_) { - common_internal::StrongRef(refcount_); - } - - Shared(Shared&& other) noexcept - : value_(other.value_), refcount_(other.refcount_) { - other.value_ = nullptr; - other.refcount_ = nullptr; - } - - template < - typename U, - typename = std::enable_if_t>, std::is_convertible>>> - // NOLINTNEXTLINE(google-explicit-constructor) - Shared(const Shared& other) - : value_(other.value_), refcount_(other.refcount_) { - common_internal::StrongRef(refcount_); - } - - template < - typename U, - typename = std::enable_if_t>, std::is_convertible>>> - // NOLINTNEXTLINE(google-explicit-constructor) - Shared(Shared&& other) noexcept - : value_(other.value_), refcount_(other.refcount_) { - other.value_ = nullptr; - other.refcount_ = nullptr; - } - - template >> - // NOLINTNEXTLINE(google-explicit-constructor) - explicit Shared(SharedView other); - - // An aliasing constructor. The resulting `Shared` shares ownership - // information with `alias`, but holds an unmanaged pointer to `T`. - // - // Usage: - // Shared object; - // Shared member = Shared(object, &object->member); - template - Shared(const Shared& alias, T* ptr) - : value_(ptr), refcount_(alias.refcount_) { - common_internal::StrongRef(refcount_); - } - - // An aliasing constructor. The resulting `Shared` shares ownership - // information with `alias`, but holds an unmanaged pointer to `T`. - template - Shared(Shared&& alias, T* ptr) noexcept - : value_(ptr), refcount_(alias.refcount_) { - alias.value_ = nullptr; - alias.refcount_ = nullptr; - } - - ~Shared() { common_internal::StrongUnref(refcount_); } - - Shared& operator=(const Shared& other) { - common_internal::StrongRef(other.refcount_); - common_internal::StrongUnref(refcount_); - value_ = other.value_; - refcount_ = other.refcount_; - return *this; - } - - Shared& operator=(Shared&& other) noexcept { - common_internal::StrongUnref(refcount_); - value_ = other.value_; - refcount_ = other.refcount_; - other.value_ = nullptr; - other.refcount_ = nullptr; - return *this; - } - - template < - typename U, - typename = std::enable_if_t>, std::is_convertible>>> - // NOLINTNEXTLINE(google-explicit-constructor) - Shared& operator=(const Shared& other) { - common_internal::StrongRef(other.refcount_); - common_internal::StrongUnref(refcount_); - value_ = other.value_; - refcount_ = other.refcount_; - return *this; - } - - template < - typename U, - typename = std::enable_if_t>, std::is_convertible>>> - // NOLINTNEXTLINE(google-explicit-constructor) - Shared& operator=(Shared&& other) noexcept { - common_internal::StrongUnref(refcount_); - value_ = other.value_; - refcount_ = other.refcount_; - other.value_ = nullptr; - other.refcount_ = nullptr; - return *this; - } - - template >> - U& operator*() const noexcept ABSL_ATTRIBUTE_LIFETIME_BOUND { - ABSL_DCHECK(!IsEmpty()); - return *value_; - } - - absl::Nonnull operator->() const noexcept ABSL_ATTRIBUTE_LIFETIME_BOUND { - ABSL_DCHECK(!IsEmpty()); - return value_; - } - - explicit operator bool() const { return !IsEmpty(); } - - friend constexpr void swap(Shared& lhs, Shared& rhs) noexcept { - using std::swap; - swap(lhs.value_, rhs.value_); - swap(lhs.refcount_, rhs.refcount_); - } - - private: - template - friend class Shared; - template - friend class SharedView; - template - friend Shared StaticCast(Shared&& from); - template - friend U* common_internal::GetPointer(const Shared& shared); - template - friend const common_internal::ReferenceCount* - common_internal::GetReferenceCount(const Shared& shared); - template - friend Shared common_internal::MakeShared( - common_internal::AdoptRef, U* value, - const common_internal::ReferenceCount* refcount); - - Shared(common_internal::AdoptRef, T* value, - const common_internal::ReferenceCount* refcount) noexcept - : value_(value), refcount_(refcount) {} - - Shared(T* value, const common_internal::ReferenceCount* refcount) noexcept - : value_(value), refcount_(refcount) { - common_internal::StrongRef(refcount_); - } - - bool IsEmpty() const noexcept { return value_ == nullptr; } - - T* value_ = nullptr; - const common_internal::ReferenceCount* refcount_ = nullptr; -}; - -template -inline Shared StaticCast(const Shared& from) { - return common_internal::MakeShared( - static_cast(common_internal::GetPointer(from)), - common_internal::GetReferenceCount(from)); -} - -template -inline Shared StaticCast(Shared&& from) { - To* value = static_cast(from.value_); - const auto* refcount = from.refcount_; - from.value_ = nullptr; - from.refcount_ = nullptr; - return Shared(common_internal::kAdoptRef, value, refcount); -} - -template -struct NativeTypeTraits> final { - static bool SkipDestructor(const Shared& shared) { - return common_internal::GetReferenceCount(shared) == nullptr; - } -}; - -// `SharedView` is a wrapper on top of `Shared`. It is roughly equivalent to -// `const Shared&` and can be used in places where it is not feasible to use -// `const Shared&` directly. This is also analygous to -// `std::reference_wrapper>>` and is intended to be used under -// the same cirumstances. -template -class ABSL_ATTRIBUTE_TRIVIAL_ABI SharedView final { - public: - SharedView() = default; - SharedView(const SharedView&) = default; - SharedView& operator=(const SharedView&) = default; - - template < - typename U, - typename = std::enable_if_t>, std::is_convertible>>> - // NOLINTNEXTLINE(google-explicit-constructor) - SharedView(const SharedView& other) - : value_(other.value_), refcount_(other.refcount_) {} - - template < - typename U, - typename = std::enable_if_t>, std::is_convertible>>> - // NOLINTNEXTLINE(google-explicit-constructor) - SharedView(SharedView&& other) noexcept - : value_(other.value_), refcount_(other.refcount_) {} - - template >> - // NOLINTNEXTLINE(google-explicit-constructor) - SharedView(const Shared& other ABSL_ATTRIBUTE_LIFETIME_BOUND) noexcept - : value_(other.value_), refcount_(other.refcount_) {} - - template - SharedView(SharedView alias, T* ptr) - : value_(ptr), refcount_(alias.refcount_) {} - - template < - typename U, - typename = std::enable_if_t>, std::is_convertible>>> - // NOLINTNEXTLINE(google-explicit-constructor) - SharedView& operator=(const SharedView& other) { - value_ = other.value_; - refcount_ = other.refcount_; - return *this; - } - - template < - typename U, - typename = std::enable_if_t>, std::is_convertible>>> - // NOLINTNEXTLINE(google-explicit-constructor) - SharedView& operator=(SharedView&& other) noexcept { - value_ = other.value_; - refcount_ = other.refcount_; - return *this; - } - - template >> - // NOLINTNEXTLINE(google-explicit-constructor) - SharedView& operator=( - const Shared& other ABSL_ATTRIBUTE_LIFETIME_BOUND) noexcept { - value_ = other.value_; - refcount_ = other.refcount_; - return *this; - } - - template >> - // NOLINTNEXTLINE(google-explicit-constructor) - SharedView& operator=(Shared&&) = delete; - - template >> - U& operator*() const noexcept ABSL_ATTRIBUTE_LIFETIME_BOUND { - ABSL_DCHECK(!IsEmpty()); - return *value_; - } - - absl::Nonnull operator->() const noexcept { - ABSL_DCHECK(!IsEmpty()); - return value_; - } - - explicit operator bool() const { return !IsEmpty(); } - - friend constexpr void swap(SharedView& lhs, SharedView& rhs) noexcept { - using std::swap; - swap(lhs.value_, rhs.value_); - swap(lhs.refcount_, rhs.refcount_); - } - - private: - template - friend class Shared; - template - friend class SharedView; - template - friend U* common_internal::GetPointer(SharedView shared); - template - friend const common_internal::ReferenceCount* - common_internal::GetReferenceCount(SharedView shared); - template - friend SharedView common_internal::MakeSharedView( - U* value, const common_internal::ReferenceCount* refcount); - - SharedView(T* value, const common_internal::ReferenceCount* refcount) - : value_(value), refcount_(refcount) {} - - bool IsEmpty() const noexcept { return value_ == nullptr; } - - T* value_ = nullptr; - const common_internal::ReferenceCount* refcount_ = nullptr; -}; - -template -template -Shared::Shared(SharedView other) - : value_(other.value_), refcount_(other.refcount_) { - StrongRef(refcount_); -} - -template -SharedView StaticCast(SharedView from) { - return common_internal::MakeSharedView( - static_cast(common_internal::GetPointer(from)), - common_internal::GetReferenceCount(from)); -} - -template -struct EnableSharedFromThis - : public virtual common_internal::ReferenceCountFromThis { - protected: - Shared shared_from_this() noexcept { - auto* const derived = static_cast(this); - auto* const refcount = common_internal::GetReferenceCountForThat(*this); - return common_internal::MakeShared(derived, refcount); - } - - Shared shared_from_this() const noexcept { - auto* const derived = static_cast(this); - auto* const refcount = common_internal::GetReferenceCountForThat(*this); - return common_internal::MakeShared(derived, refcount); - } -}; - // `ReferenceCountingMemoryManager` is a `MemoryManager` which employs automatic // memory management through reference counting. class ReferenceCountingMemoryManager final { @@ -1687,24 +1318,6 @@ class ReferenceCountingMemoryManager final { delete; private: - template - static ABSL_MUST_USE_RESULT Shared MakeShared(Args&&... args) { - using U = std::remove_const_t; - U* ptr; - common_internal::ReferenceCount* refcount; - std::tie(ptr, refcount) = - common_internal::MakeReferenceCount(std::forward(args)...); - return common_internal::MakeShared(common_internal::kAdoptRef, - static_cast(ptr), refcount); - } - - template - static ABSL_MUST_USE_RESULT Unique MakeUnique(Args&&... args) { - using U = std::remove_const_t; - return Unique(static_cast(new U(std::forward(args)...)), - nullptr); - } - static void* Allocate(size_t size, size_t alignment); static bool Deallocate(void* ptr, size_t size, size_t alignment) noexcept; @@ -1724,52 +1337,6 @@ class PoolingMemoryManager final { PoolingMemoryManager& operator=(PoolingMemoryManager&&) = delete; private: - template - ABSL_MUST_USE_RESULT static Shared MakeShared(google::protobuf::Arena* arena, - Args&&... args) { - using U = std::remove_const_t; - U* ptr = nullptr; - void* addr = Allocate(arena, sizeof(U), alignof(U)); - CEL_INTERNAL_TRY { - ptr = ::new (addr) U(std::forward(args)...); - if constexpr (!std::is_trivially_destructible_v) { - if (!NativeType::SkipDestructor(*ptr)) { - CEL_INTERNAL_TRY { - OwnCustomDestructor(arena, ptr, &DefaultDestructor); - } - CEL_INTERNAL_CATCH_ANY { - ptr->~U(); - CEL_INTERNAL_RETHROW; - } - } - } - if constexpr (std::is_base_of_v) { - common_internal::SetReferenceCountForThat(*ptr, nullptr); - } - } - CEL_INTERNAL_CATCH_ANY { - Deallocate(arena, addr, sizeof(U), alignof(U)); - CEL_INTERNAL_RETHROW; - } - return common_internal::MakeShared(common_internal::kAdoptRef, - static_cast(ptr), nullptr); - } - - template - ABSL_MUST_USE_RESULT static Unique MakeUnique(google::protobuf::Arena* arena, - Args&&... args) { - using U = std::remove_const_t; - U* ptr = nullptr; - void* addr = Allocate(arena, sizeof(U), alignof(U)); - CEL_INTERNAL_TRY { ptr = ::new (addr) U(std::forward(args)...); } - CEL_INTERNAL_CATCH_ANY { - Deallocate(arena, addr, sizeof(U), alignof(U)); - CEL_INTERNAL_RETHROW; - } - return Unique(static_cast(ptr), arena, /*unowned=*/true); - } - // Allocates memory directly from the allocator used by this memory manager. // If `memory_management()` returns `MemoryManagement::kReferenceCounting`, // this allocation *must* be explicitly deallocated at some point via @@ -1866,28 +1433,6 @@ class MemoryManager final { : MemoryManagement::kPooling; } - template - ABSL_MUST_USE_RESULT Shared MakeShared(Args&&... args) { - if (arena_ == nullptr) { - return ReferenceCountingMemoryManager::MakeShared( - std::forward(args)...); - } else { - return PoolingMemoryManager::MakeShared(arena_, - std::forward(args)...); - } - } - - template - ABSL_MUST_USE_RESULT Unique MakeUnique(Args&&... args) { - if (arena_ == nullptr) { - return ReferenceCountingMemoryManager::MakeUnique( - std::forward(args)...); - } else { - return PoolingMemoryManager::MakeUnique(arena_, - std::forward(args)...); - } - } - // Allocates memory directly from the allocator used by this memory manager. // If `memory_management()` returns `MemoryManagement::kReferenceCounting`, // this allocation *must* be explicitly deallocated at some point via @@ -1958,47 +1503,6 @@ class MemoryManager final { using MemoryManagerRef = MemoryManager; -namespace common_internal { - -template -inline T* GetPointer(const Shared& shared) { - return shared.value_; -} - -template -inline const ReferenceCount* GetReferenceCount(const Shared& shared) { - return shared.refcount_; -} - -template -inline Shared MakeShared(T* value, const ReferenceCount* refcount) { - StrongRef(refcount); - return MakeShared(kAdoptRef, value, refcount); -} - -template -inline Shared MakeShared(AdoptRef, T* value, - const ReferenceCount* refcount) { - return Shared(kAdoptRef, value, refcount); -} - -template -inline T* GetPointer(SharedView shared) { - return shared.value_; -} - -template -inline const ReferenceCount* GetReferenceCount(SharedView shared) { - return shared.refcount_; -} - -template -inline SharedView MakeSharedView(T* value, const ReferenceCount* refcount) { - return SharedView(value, refcount); -} - -} // namespace common_internal - } // namespace cel #endif // THIRD_PARTY_CEL_CPP_COMMON_MEMORY_H_ diff --git a/common/memory_test.cc b/common/memory_test.cc index d3d8563f5..e0b7346df 100644 --- a/common/memory_test.cc +++ b/common/memory_test.cc @@ -19,21 +19,13 @@ #include "common/memory.h" -#include -#include -#include -#include #include #include "google/protobuf/struct.pb.h" #include "absl/base/nullability.h" -#include "absl/debugging/leak_check.h" -#include "absl/log/absl_check.h" -#include "absl/types/optional.h" #include "common/allocator.h" #include "common/data.h" #include "common/internal/reference_count.h" -#include "common/native_type.h" #include "internal/testing.h" #include "google/protobuf/arena.h" @@ -44,832 +36,12 @@ namespace cel { namespace { -// NOLINTBEGIN(bugprone-use-after-move) - -using ::testing::_; using ::testing::IsFalse; using ::testing::IsNull; using ::testing::IsTrue; -using ::testing::NotNull; using ::testing::TestParamInfo; using ::testing::TestWithParam; -TEST(MemoryManagement, ostream) { - { - std::ostringstream out; - out << MemoryManagement::kPooling; - EXPECT_EQ(out.str(), "POOLING"); - } - { - std::ostringstream out; - out << MemoryManagement::kReferenceCounting; - EXPECT_EQ(out.str(), "REFERENCE_COUNTING"); - } -} - -struct TrivialSmallObject { - uintptr_t ptr; - char padding[32 - sizeof(uintptr_t)]; -}; - -TEST(RegionalMemoryManager, TrivialSmallSizes) { - google::protobuf::Arena arena; - MemoryManager memory_manager = MemoryManager::Pooling(&arena); - for (size_t i = 0; i < 1024; ++i) { - static_cast(memory_manager.MakeUnique()); - } -} - -struct TrivialMediumObject { - uintptr_t ptr; - char padding[256 - sizeof(uintptr_t)]; -}; - -TEST(RegionalMemoryManager, TrivialMediumSizes) { - google::protobuf::Arena arena; - MemoryManager memory_manager = MemoryManager::Pooling(&arena); - for (size_t i = 0; i < 1024; ++i) { - static_cast(memory_manager.MakeUnique()); - } -} - -struct TrivialLargeObject { - uintptr_t ptr; - char padding[4096 - sizeof(uintptr_t)]; -}; - -TEST(RegionalMemoryManager, TrivialLargeSizes) { - google::protobuf::Arena arena; - MemoryManager memory_manager = MemoryManager::Pooling(&arena); - for (size_t i = 0; i < 1024; ++i) { - static_cast(memory_manager.MakeUnique()); - } -} - -TEST(RegionalMemoryManager, TrivialMixedSizes) { - google::protobuf::Arena arena; - MemoryManager memory_manager = MemoryManager::Pooling(&arena); - for (size_t i = 0; i < 1024; ++i) { - switch (i % 3) { - case 0: - static_cast(memory_manager.MakeUnique()); - break; - case 1: - static_cast(memory_manager.MakeUnique()); - break; - case 2: - static_cast(memory_manager.MakeUnique()); - break; - } - } -} - -struct TrivialHugeObject { - uintptr_t ptr; - char padding[32768 - sizeof(uintptr_t)]; -}; - -TEST(RegionalMemoryManager, TrivialHugeSizes) { - google::protobuf::Arena arena; - MemoryManager memory_manager = MemoryManager::Pooling(&arena); - for (size_t i = 0; i < 1024; ++i) { - static_cast(memory_manager.MakeUnique()); - } -} - -class SkippableDestructor { - public: - explicit SkippableDestructor(bool& deleted) : deleted_(deleted) {} - - ~SkippableDestructor() { deleted_ = true; } - - private: - bool& deleted_; -}; - -} // namespace - -template <> -struct NativeTypeTraits final { - static bool SkipDestructor(const SkippableDestructor&) { return true; } -}; - -namespace { - -TEST(RegionalMemoryManager, SkippableDestructor) { - bool deleted = false; - { - google::protobuf::Arena arena; - MemoryManager memory_manager = MemoryManager::Pooling(&arena); - auto shared = memory_manager.MakeShared(deleted); - static_cast(shared); - } - EXPECT_FALSE(deleted); -} - -class MemoryManagerTest : public TestWithParam { - public: - void SetUp() override {} - - void TearDown() override { Finish(); } - - void Finish() { arena_.reset(); } - - MemoryManagerRef memory_manager() { - switch (memory_management()) { - case MemoryManagement::kReferenceCounting: - return MemoryManager::ReferenceCounting(); - case MemoryManagement::kPooling: - if (!arena_) { - arena_.emplace(); - } - return MemoryManager::Pooling(&*arena_); - } - } - - MemoryManagement memory_management() const { return GetParam(); } - - static std::string ToString(TestParamInfo param) { - std::ostringstream out; - out << param.param; - return out.str(); - } - - private: - absl::optional arena_; -}; - -TEST_P(MemoryManagerTest, AllocateAndDeallocateZeroSize) { - EXPECT_THAT(memory_manager().Allocate(0, 1), IsNull()); - EXPECT_THAT(memory_manager().Deallocate(nullptr, 0, 1), IsFalse()); -} - -TEST_P(MemoryManagerTest, AllocateAndDeallocateBadAlignment) { - EXPECT_DEBUG_DEATH(absl::IgnoreLeak(memory_manager().Allocate(1, 0)), _); - EXPECT_DEBUG_DEATH(memory_manager().Deallocate(nullptr, 0, 0), _); -} - -TEST_P(MemoryManagerTest, AllocateAndDeallocate) { - constexpr size_t kSize = 1024; - constexpr size_t kAlignment = __STDCPP_DEFAULT_NEW_ALIGNMENT__; - void* ptr = memory_manager().Allocate(kSize, kAlignment); - ASSERT_THAT(ptr, NotNull()); - if (memory_management() == MemoryManagement::kReferenceCounting) { - EXPECT_THAT(memory_manager().Deallocate(ptr, kSize, kAlignment), IsTrue()); - } -} - -TEST_P(MemoryManagerTest, AllocateAndDeallocateOveraligned) { - constexpr size_t kSize = 1024; - constexpr size_t kAlignment = __STDCPP_DEFAULT_NEW_ALIGNMENT__ * 4; - void* ptr = memory_manager().Allocate(kSize, kAlignment); - ASSERT_THAT(ptr, NotNull()); - if (memory_management() == MemoryManagement::kReferenceCounting) { - EXPECT_THAT(memory_manager().Deallocate(ptr, kSize, kAlignment), IsTrue()); - } -} - -class Object { - public: - Object() : deleted_(nullptr) {} - - explicit Object(bool& deleted) : deleted_(&deleted) {} - - ~Object() { - if (deleted_ != nullptr) { - ABSL_CHECK(!*deleted_); - *deleted_ = true; - } - } - - int member = 0; - - private: - bool* deleted_; -}; - -class Subobject : public Object { - public: - using Object::Object; -}; - -TEST_P(MemoryManagerTest, Shared) { - bool deleted = false; - { - auto object = memory_manager().MakeShared(deleted); - EXPECT_TRUE(object); - EXPECT_FALSE(deleted); - } - switch (memory_management()) { - case MemoryManagement::kPooling: - EXPECT_FALSE(deleted); - break; - case MemoryManagement::kReferenceCounting: - EXPECT_TRUE(deleted); - break; - } - Finish(); -} - -TEST_P(MemoryManagerTest, SharedAliasCopy) { - bool deleted = false; - { - auto object = memory_manager().MakeShared(deleted); - EXPECT_TRUE(object); - EXPECT_FALSE(deleted); - { - auto member = Shared(object, &object->member); - EXPECT_TRUE(object); - EXPECT_FALSE(deleted); - EXPECT_TRUE(member); - } - EXPECT_TRUE(object); - EXPECT_FALSE(deleted); - } - switch (memory_management()) { - case MemoryManagement::kPooling: - EXPECT_FALSE(deleted); - break; - case MemoryManagement::kReferenceCounting: - EXPECT_TRUE(deleted); - break; - } - Finish(); -} - -TEST_P(MemoryManagerTest, SharedAliasMove) { - bool deleted = false; - { - auto object = memory_manager().MakeShared(deleted); - EXPECT_TRUE(object); - EXPECT_FALSE(deleted); - { - auto member = Shared(std::move(object), &object->member); - EXPECT_FALSE(object); - EXPECT_FALSE(deleted); - EXPECT_TRUE(member); - } - switch (memory_management()) { - case MemoryManagement::kPooling: - EXPECT_FALSE(deleted); - break; - case MemoryManagement::kReferenceCounting: - EXPECT_TRUE(deleted); - break; - } - } - Finish(); -} - -TEST_P(MemoryManagerTest, SharedStaticCastCopy) { - bool deleted = false; - { - auto object = memory_manager().MakeShared(deleted); - EXPECT_TRUE(object); - EXPECT_FALSE(deleted); - { - auto member = StaticCast(object); - EXPECT_TRUE(object); - EXPECT_FALSE(deleted); - EXPECT_TRUE(member); - } - EXPECT_TRUE(object); - EXPECT_FALSE(deleted); - } - switch (memory_management()) { - case MemoryManagement::kPooling: - EXPECT_FALSE(deleted); - break; - case MemoryManagement::kReferenceCounting: - EXPECT_TRUE(deleted); - break; - } - Finish(); -} - -TEST_P(MemoryManagerTest, SharedStaticCastMove) { - bool deleted = false; - { - auto object = memory_manager().MakeShared(deleted); - EXPECT_TRUE(object); - EXPECT_FALSE(deleted); - { - auto member = StaticCast(std::move(object)); - EXPECT_FALSE(object); - EXPECT_FALSE(deleted); - EXPECT_TRUE(member); - } - switch (memory_management()) { - case MemoryManagement::kPooling: - EXPECT_FALSE(deleted); - break; - case MemoryManagement::kReferenceCounting: - EXPECT_TRUE(deleted); - break; - } - } - Finish(); -} - -TEST_P(MemoryManagerTest, SharedCopyConstruct) { - bool deleted = false; - { - auto object = memory_manager().MakeShared(deleted); - EXPECT_TRUE(object); - // NOLINTNEXTLINE(performance-unnecessary-copy-initialization) - Shared copied_object(object); - EXPECT_TRUE(copied_object); - EXPECT_FALSE(deleted); - } - switch (memory_management()) { - case MemoryManagement::kPooling: - EXPECT_FALSE(deleted); - break; - case MemoryManagement::kReferenceCounting: - EXPECT_TRUE(deleted); - break; - } - Finish(); -} - -TEST_P(MemoryManagerTest, SharedMoveConstruct) { - bool deleted = false; - { - auto object = memory_manager().MakeShared(deleted); - EXPECT_TRUE(object); - // NOLINTNEXTLINE(performance-unnecessary-copy-initialization) - Shared moved_object(std::move(object)); - EXPECT_FALSE(object); - EXPECT_TRUE(moved_object); - EXPECT_FALSE(deleted); - } - switch (memory_management()) { - case MemoryManagement::kPooling: - EXPECT_FALSE(deleted); - break; - case MemoryManagement::kReferenceCounting: - EXPECT_TRUE(deleted); - break; - } - Finish(); -} - -TEST_P(MemoryManagerTest, SharedCopyAssign) { - bool deleted = false; - { - auto object = memory_manager().MakeShared(deleted); - EXPECT_TRUE(object); - // NOLINTNEXTLINE(performance-unnecessary-copy-initialization) - Shared moved_object(std::move(object)); - EXPECT_FALSE(object); - EXPECT_TRUE(moved_object); - object = moved_object; - EXPECT_TRUE(object); - EXPECT_FALSE(deleted); - } - switch (memory_management()) { - case MemoryManagement::kPooling: - EXPECT_FALSE(deleted); - break; - case MemoryManagement::kReferenceCounting: - EXPECT_TRUE(deleted); - break; - } - Finish(); -} - -TEST_P(MemoryManagerTest, SharedMoveAssign) { - bool deleted = false; - { - auto object = memory_manager().MakeShared(deleted); - EXPECT_TRUE(object); - // NOLINTNEXTLINE(performance-unnecessary-copy-initialization) - Shared moved_object(std::move(object)); - EXPECT_FALSE(object); - EXPECT_TRUE(moved_object); - object = std::move(moved_object); - EXPECT_FALSE(moved_object); - EXPECT_TRUE(object); - EXPECT_FALSE(deleted); - } - switch (memory_management()) { - case MemoryManagement::kPooling: - EXPECT_FALSE(deleted); - break; - case MemoryManagement::kReferenceCounting: - EXPECT_TRUE(deleted); - break; - } - Finish(); -} - -TEST_P(MemoryManagerTest, SharedCopyConstructConvertible) { - bool deleted = false; - { - auto object = memory_manager().MakeShared(deleted); - EXPECT_TRUE(object); - // NOLINTNEXTLINE(performance-unnecessary-copy-initialization) - Shared copied_object(object); - EXPECT_TRUE(copied_object); - EXPECT_FALSE(deleted); - } - switch (memory_management()) { - case MemoryManagement::kPooling: - EXPECT_FALSE(deleted); - break; - case MemoryManagement::kReferenceCounting: - EXPECT_TRUE(deleted); - break; - } - Finish(); -} - -TEST_P(MemoryManagerTest, SharedMoveConstructConvertible) { - bool deleted = false; - { - auto object = memory_manager().MakeShared(deleted); - EXPECT_TRUE(object); - // NOLINTNEXTLINE(performance-unnecessary-copy-initialization) - Shared moved_object(std::move(object)); - EXPECT_FALSE(object); - EXPECT_TRUE(moved_object); - EXPECT_FALSE(deleted); - } - switch (memory_management()) { - case MemoryManagement::kPooling: - EXPECT_FALSE(deleted); - break; - case MemoryManagement::kReferenceCounting: - EXPECT_TRUE(deleted); - break; - } - Finish(); -} - -TEST_P(MemoryManagerTest, SharedCopyAssignConvertible) { - bool deleted = false; - { - auto subobject = memory_manager().MakeShared(deleted); - EXPECT_TRUE(subobject); - auto object = memory_manager().MakeShared(); - EXPECT_TRUE(object); - object = subobject; - EXPECT_TRUE(object); - EXPECT_TRUE(subobject); - EXPECT_FALSE(deleted); - } - switch (memory_management()) { - case MemoryManagement::kPooling: - EXPECT_FALSE(deleted); - break; - case MemoryManagement::kReferenceCounting: - EXPECT_TRUE(deleted); - break; - } - Finish(); -} - -TEST_P(MemoryManagerTest, SharedMoveAssignConvertible) { - bool deleted = false; - { - auto subobject = memory_manager().MakeShared(deleted); - EXPECT_TRUE(subobject); - auto object = memory_manager().MakeShared(); - EXPECT_TRUE(object); - object = std::move(subobject); - EXPECT_TRUE(object); - EXPECT_FALSE(subobject); - EXPECT_FALSE(deleted); - } - switch (memory_management()) { - case MemoryManagement::kPooling: - EXPECT_FALSE(deleted); - break; - case MemoryManagement::kReferenceCounting: - EXPECT_TRUE(deleted); - break; - } - Finish(); -} - -TEST_P(MemoryManagerTest, SharedSwap) { - using std::swap; - auto object1 = memory_manager().MakeShared(); - auto object2 = memory_manager().MakeShared(); - auto* const object1_ptr = object1.operator->(); - auto* const object2_ptr = object2.operator->(); - swap(object1, object2); - EXPECT_EQ(object1.operator->(), object2_ptr); - EXPECT_EQ(object2.operator->(), object1_ptr); -} - -TEST_P(MemoryManagerTest, SharedPointee) { - using std::swap; - auto object = memory_manager().MakeShared(); - EXPECT_EQ(std::addressof(*object), object.operator->()); -} - -TEST_P(MemoryManagerTest, SharedViewConstruct) { - bool deleted = false; - absl::optional> dangling_object_view; - { - auto object = memory_manager().MakeShared(deleted); - dangling_object_view.emplace(object); - EXPECT_TRUE(*dangling_object_view); - { - auto copied_object = Shared(*dangling_object_view); - EXPECT_FALSE(deleted); - } - EXPECT_FALSE(deleted); - } - switch (memory_management()) { - case MemoryManagement::kPooling: - EXPECT_FALSE(deleted); - break; - case MemoryManagement::kReferenceCounting: - EXPECT_TRUE(deleted); - break; - } - Finish(); -} - -TEST_P(MemoryManagerTest, SharedViewCopyConstruct) { - bool deleted = false; - absl::optional> dangling_object_view; - { - auto object = memory_manager().MakeShared(deleted); - auto object_view = SharedView(object); - SharedView copied_object_view(object_view); - dangling_object_view.emplace(copied_object_view); - EXPECT_FALSE(deleted); - } - switch (memory_management()) { - case MemoryManagement::kPooling: - EXPECT_FALSE(deleted); - break; - case MemoryManagement::kReferenceCounting: - EXPECT_TRUE(deleted); - break; - } - Finish(); -} - -TEST_P(MemoryManagerTest, SharedViewMoveConstruct) { - bool deleted = false; - absl::optional> dangling_object_view; - { - auto object = memory_manager().MakeShared(deleted); - auto object_view = SharedView(object); - SharedView moved_object_view(std::move(object_view)); - dangling_object_view.emplace(moved_object_view); - EXPECT_FALSE(deleted); - } - switch (memory_management()) { - case MemoryManagement::kPooling: - EXPECT_FALSE(deleted); - break; - case MemoryManagement::kReferenceCounting: - EXPECT_TRUE(deleted); - break; - } - Finish(); -} - -TEST_P(MemoryManagerTest, SharedViewCopyAssign) { - bool deleted = false; - absl::optional> dangling_object_view; - { - auto object = memory_manager().MakeShared(deleted); - auto object_view1 = SharedView(object); - SharedView object_view2(object); - object_view1 = object_view2; - dangling_object_view.emplace(object_view1); - EXPECT_FALSE(deleted); - } - switch (memory_management()) { - case MemoryManagement::kPooling: - EXPECT_FALSE(deleted); - break; - case MemoryManagement::kReferenceCounting: - EXPECT_TRUE(deleted); - break; - } - Finish(); -} - -TEST_P(MemoryManagerTest, SharedViewMoveAssign) { - bool deleted = false; - absl::optional> dangling_object_view; - { - auto object = memory_manager().MakeShared(deleted); - auto object_view1 = SharedView(object); - SharedView object_view2(object); - object_view1 = std::move(object_view2); - dangling_object_view.emplace(object_view1); - EXPECT_FALSE(deleted); - } - switch (memory_management()) { - case MemoryManagement::kPooling: - EXPECT_FALSE(deleted); - break; - case MemoryManagement::kReferenceCounting: - EXPECT_TRUE(deleted); - break; - } - Finish(); -} - -TEST_P(MemoryManagerTest, SharedViewCopyConstructConvertible) { - bool deleted = false; - absl::optional> dangling_object_view; - { - auto subobject = memory_manager().MakeShared(deleted); - auto subobject_view = SharedView(subobject); - SharedView object_view(subobject_view); - dangling_object_view.emplace(object_view); - EXPECT_FALSE(deleted); - } - switch (memory_management()) { - case MemoryManagement::kPooling: - EXPECT_FALSE(deleted); - break; - case MemoryManagement::kReferenceCounting: - EXPECT_TRUE(deleted); - break; - } - Finish(); -} - -TEST_P(MemoryManagerTest, SharedViewMoveConstructConvertible) { - bool deleted = false; - absl::optional> dangling_object_view; - { - auto subobject = memory_manager().MakeShared(deleted); - auto subobject_view = SharedView(subobject); - SharedView object_view(std::move(subobject_view)); - dangling_object_view.emplace(object_view); - EXPECT_FALSE(deleted); - } - switch (memory_management()) { - case MemoryManagement::kPooling: - EXPECT_FALSE(deleted); - break; - case MemoryManagement::kReferenceCounting: - EXPECT_TRUE(deleted); - break; - } - Finish(); -} - -TEST_P(MemoryManagerTest, SharedViewCopyAssignConvertible) { - bool deleted = false; - absl::optional> dangling_object_view; - { - auto subobject = memory_manager().MakeShared(deleted); - auto object_view1 = SharedView(subobject); - SharedView subobject_view2(subobject); - object_view1 = subobject_view2; - dangling_object_view.emplace(object_view1); - EXPECT_FALSE(deleted); - } - switch (memory_management()) { - case MemoryManagement::kPooling: - EXPECT_FALSE(deleted); - break; - case MemoryManagement::kReferenceCounting: - EXPECT_TRUE(deleted); - break; - } - Finish(); -} - -TEST_P(MemoryManagerTest, SharedViewMoveAssignConvertible) { - bool deleted = false; - absl::optional> dangling_object_view; - { - auto subobject = memory_manager().MakeShared(deleted); - auto object_view1 = SharedView(subobject); - SharedView subobject_view2(subobject); - object_view1 = std::move(subobject_view2); - dangling_object_view.emplace(object_view1); - EXPECT_FALSE(deleted); - } - switch (memory_management()) { - case MemoryManagement::kPooling: - EXPECT_FALSE(deleted); - break; - case MemoryManagement::kReferenceCounting: - EXPECT_TRUE(deleted); - break; - } - Finish(); -} - -TEST_P(MemoryManagerTest, SharedViewSwap) { - using std::swap; - auto object1 = memory_manager().MakeShared(); - auto object2 = memory_manager().MakeShared(); - auto object1_view = SharedView(object1); - auto object2_view = SharedView(object2); - swap(object1_view, object2_view); - EXPECT_EQ(object1_view.operator->(), object2.operator->()); - EXPECT_EQ(object2_view.operator->(), object1.operator->()); -} - -TEST_P(MemoryManagerTest, SharedViewPointee) { - using std::swap; - auto object = memory_manager().MakeShared(); - auto object_view = SharedView(object); - EXPECT_EQ(std::addressof(*object_view), object_view.operator->()); -} - -TEST_P(MemoryManagerTest, Unique) { - bool deleted = false; - { - auto object = memory_manager().MakeUnique(deleted); - EXPECT_TRUE(object); - EXPECT_FALSE(deleted); - } - EXPECT_TRUE(deleted); - - Finish(); -} - -TEST_P(MemoryManagerTest, UniquePointee) { - using std::swap; - auto object = memory_manager().MakeUnique(); - EXPECT_EQ(std::addressof(*object), object.operator->()); -} - -TEST_P(MemoryManagerTest, UniqueSwap) { - using std::swap; - auto object1 = memory_manager().MakeUnique(); - auto object2 = memory_manager().MakeUnique(); - auto* const object1_ptr = object1.operator->(); - auto* const object2_ptr = object2.operator->(); - swap(object1, object2); - EXPECT_EQ(object1.operator->(), object2_ptr); - EXPECT_EQ(object2.operator->(), object1_ptr); -} - -struct EnabledObject : EnableSharedFromThis { - Shared This() { return shared_from_this(); } - - Shared This() const { return shared_from_this(); } -}; - -TEST_P(MemoryManagerTest, EnableSharedFromThis) { - { - auto object = memory_manager().MakeShared(); - auto this_object = object->This(); - EXPECT_EQ(this_object.operator->(), object.operator->()); - } - { - auto object = memory_manager().MakeShared(); - auto this_object = object->This(); - EXPECT_EQ(this_object.operator->(), object.operator->()); - } - Finish(); -} - -struct ThrowingConstructorObject { - ThrowingConstructorObject() { -#ifdef ABSL_HAVE_EXCEPTIONS - throw std::invalid_argument("ThrowingConstructorObject"); -#endif - } - - char padding[64]; -}; - -TEST_P(MemoryManagerTest, SharedThrowingConstructor) { -#ifdef ABSL_HAVE_EXCEPTIONS - EXPECT_THROW(static_cast( - memory_manager().MakeShared()), - std::invalid_argument); -#else - GTEST_SKIP(); -#endif -} - -TEST_P(MemoryManagerTest, UniqueThrowingConstructor) { -#ifdef ABSL_HAVE_EXCEPTIONS - EXPECT_THROW(static_cast( - memory_manager().MakeUnique()), - std::invalid_argument); -#else - GTEST_SKIP(); -#endif -} - -INSTANTIATE_TEST_SUITE_P( - MemoryManagerTest, MemoryManagerTest, - ::testing::Values(MemoryManagement::kPooling, - MemoryManagement::kReferenceCounting), - MemoryManagerTest::ToString); - -// NOLINTEND(bugprone-use-after-move) - TEST(Owner, None) { EXPECT_THAT(Owner::None(), IsFalse()); EXPECT_THAT(Owner::None().arena(), IsNull()); @@ -994,13 +166,13 @@ TEST(Unique, ToAddress) { EXPECT_EQ(cel::to_address(unique), unique.operator->()); } -class OwnedTest : public TestWithParam { +class OwnedTest : public TestWithParam { public: Allocator<> GetAllocator() { switch (GetParam()) { - case MemoryManagement::kPooling: + case AllocatorKind::kArena: return ArenaAllocator<>{&arena_}; - case MemoryManagement::kReferenceCounting: + case AllocatorKind::kNewDelete: return NewDeleteAllocator<>{}; } } @@ -1173,18 +345,17 @@ TEST_P(OwnedTest, AssignNullPtr) { EXPECT_FALSE(owned); } -INSTANTIATE_TEST_SUITE_P( - OwnedTest, OwnedTest, - ::testing::Values(MemoryManagement::kPooling, - MemoryManagement::kReferenceCounting)); +INSTANTIATE_TEST_SUITE_P(OwnedTest, OwnedTest, + ::testing::Values(AllocatorKind::kArena, + AllocatorKind::kNewDelete)); -class BorrowedTest : public TestWithParam { +class BorrowedTest : public TestWithParam { public: Allocator<> GetAllocator() { switch (GetParam()) { - case MemoryManagement::kPooling: + case AllocatorKind::kArena: return ArenaAllocator<>{&arena_}; - case MemoryManagement::kReferenceCounting: + case AllocatorKind::kNewDelete: return NewDeleteAllocator<>{}; } } @@ -1287,10 +458,9 @@ TEST_P(BorrowedTest, AssignNullPtr) { EXPECT_FALSE(borrowed); } -INSTANTIATE_TEST_SUITE_P( - BorrowedTest, BorrowedTest, - ::testing::Values(MemoryManagement::kPooling, - MemoryManagement::kReferenceCounting)); +INSTANTIATE_TEST_SUITE_P(BorrowedTest, BorrowedTest, + ::testing::Values(AllocatorKind::kArena, + AllocatorKind::kNewDelete)); } // namespace } // namespace cel diff --git a/common/type_introspector.cc b/common/type_introspector.cc index f0737dda2..c69235b3b 100644 --- a/common/type_introspector.cc +++ b/common/type_introspector.cc @@ -23,9 +23,7 @@ #include "absl/status/statusor.h" #include "absl/strings/string_view.h" #include "absl/types/optional.h" -#include "common/memory.h" #include "common/type.h" -#include "common/types/thread_compatible_type_introspector.h" namespace cel { @@ -259,10 +257,4 @@ TypeIntrospector::FindStructTypeFieldByNameImpl(absl::string_view, return absl::nullopt; } -Shared NewThreadCompatibleTypeIntrospector( - MemoryManagerRef memory_manager) { - return memory_manager - .MakeShared(); -} - } // namespace cel diff --git a/common/type_introspector.h b/common/type_introspector.h index 00b1da758..7f4a19a31 100644 --- a/common/type_introspector.h +++ b/common/type_introspector.h @@ -20,7 +20,6 @@ #include "absl/status/statusor.h" #include "absl/strings/string_view.h" #include "absl/types/optional.h" -#include "common/memory.h" #include "common/type.h" namespace cel { @@ -77,9 +76,6 @@ class TypeIntrospector { absl::string_view name) const; }; -Shared NewThreadCompatibleTypeIntrospector( - MemoryManagerRef memory_manager); - } // namespace cel #endif // THIRD_PARTY_CEL_CPP_COMMON_TYPE_INTROSPECTOR_H_ diff --git a/common/type_manager.cc b/common/type_manager.cc deleted file mode 100644 index 0928f5346..000000000 --- a/common/type_manager.cc +++ /dev/null @@ -1,33 +0,0 @@ -// Copyright 2023 Google LLC -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// https://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include "common/type_manager.h" - -#include - -#include "common/memory.h" -#include "common/type_introspector.h" -#include "common/types/thread_compatible_type_manager.h" - -namespace cel { - -Shared NewThreadCompatibleTypeManager( - MemoryManagerRef memory_manager, - Shared type_introspector) { - return memory_manager - .MakeShared( - std::move(type_introspector)); -} - -} // namespace cel diff --git a/common/type_manager.h b/common/type_manager.h index 576fe22fa..354f4c9b8 100644 --- a/common/type_manager.h +++ b/common/type_manager.h @@ -52,11 +52,6 @@ class TypeManager : public virtual TypeFactory { virtual const TypeIntrospector& GetTypeIntrospector() const = 0; }; -// Creates a new `TypeManager` which is thread compatible. -Shared NewThreadCompatibleTypeManager( - MemoryManagerRef memory_manager, - Shared type_introspector); - } // namespace cel #endif // THIRD_PARTY_CEL_CPP_COMMON_TYPE_MANAGER_H_ diff --git a/common/type_testing.h b/common/type_testing.h index 0dc290ec7..284201101 100644 --- a/common/type_testing.h +++ b/common/type_testing.h @@ -15,44 +15,9 @@ #ifndef THIRD_PARTY_CEL_CPP_COMMON_TYPE_TESTING_H_ #define THIRD_PARTY_CEL_CPP_COMMON_TYPE_TESTING_H_ -#include "absl/types/optional.h" -#include "common/memory.h" -#include "common/memory_testing.h" -#include "common/type_factory.h" -#include "common/type_introspector.h" -#include "common/type_manager.h" - namespace cel::common_internal { -template -class ThreadCompatibleTypeTest : public ThreadCompatibleMemoryTest { - private: - using Base = ThreadCompatibleMemoryTest; - - public: - void SetUp() override { - Base::SetUp(); - type_manager_ = NewThreadCompatibleTypeManager( - this->memory_manager(), NewTypeIntrospector(this->memory_manager())); - } - - void TearDown() override { - type_manager_.reset(); - Base::TearDown(); - } - - TypeManager& type_manager() const { return **type_manager_; } - - TypeFactory& type_factory() const { return type_manager(); } - - private: - virtual Shared NewTypeIntrospector( - MemoryManagerRef memory_manager) { - return NewThreadCompatibleTypeIntrospector(memory_manager); - } - - absl::optional> type_manager_; -}; +// Empty for now. } // namespace cel::common_internal diff --git a/common/types/thread_compatible_type_manager.h b/common/types/thread_compatible_type_manager.h deleted file mode 100644 index 937d6b13f..000000000 --- a/common/types/thread_compatible_type_manager.h +++ /dev/null @@ -1,45 +0,0 @@ -// Copyright 2023 Google LLC -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// https://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -// IWYU pragma: private - -#ifndef THIRD_PARTY_CEL_CPP_COMMON_TYPES_THREAD_COMPATIBLE_TYPE_MANAGER_H_ -#define THIRD_PARTY_CEL_CPP_COMMON_TYPES_THREAD_COMPATIBLE_TYPE_MANAGER_H_ - -#include - -#include "common/memory.h" -#include "common/type_introspector.h" -#include "common/type_manager.h" - -namespace cel::common_internal { - -class ThreadCompatibleTypeManager : public virtual TypeManager { - public: - explicit ThreadCompatibleTypeManager( - Shared type_introspector) - : type_introspector_(std::move(type_introspector)) {} - - protected: - TypeIntrospector& GetTypeIntrospector() const final { - return *type_introspector_; - } - - private: - Shared type_introspector_; -}; - -} // namespace cel::common_internal - -#endif // THIRD_PARTY_CEL_CPP_COMMON_TYPES_THREAD_COMPATIBLE_TYPE_MANAGER_H_ diff --git a/common/value.h b/common/value.h index 68e6b7243..ec6cf4907 100644 --- a/common/value.h +++ b/common/value.h @@ -369,22 +369,6 @@ class Value final : private common_internal::ValueMixin { return *this; } - template >> - // NOLINTNEXTLINE(google-explicit-constructor) - Value(const Shared& interface) noexcept - : variant_( - absl::in_place_type>, - interface) {} - - template >> - // NOLINTNEXTLINE(google-explicit-constructor) - Value(Shared&& interface) noexcept - : variant_( - absl::in_place_type>, - std::move(interface)) {} - template >>> diff --git a/extensions/protobuf/BUILD b/extensions/protobuf/BUILD index fa93b8c1a..bc9f21b1b 100644 --- a/extensions/protobuf/BUILD +++ b/extensions/protobuf/BUILD @@ -147,7 +147,6 @@ cc_test( ":type", "//common:type", "//common:type_kind", - "//common:type_testing", "//internal:testing", "@com_google_absl//absl/types:optional", "@com_google_cel_spec//proto/cel/expr/conformance/proto2:test_all_types_cc_proto", diff --git a/extensions/protobuf/type_introspector_test.cc b/extensions/protobuf/type_introspector_test.cc index 592fa946f..0a7b21524 100644 --- a/extensions/protobuf/type_introspector_test.cc +++ b/extensions/protobuf/type_introspector_test.cc @@ -17,7 +17,6 @@ #include "absl/types/optional.h" #include "common/type.h" #include "common/type_kind.h" -#include "common/type_testing.h" #include "internal/testing.h" #include "cel/expr/conformance/proto2/test_all_types.pb.h" #include "google/protobuf/descriptor.h" @@ -30,40 +29,33 @@ using ::cel::expr::conformance::proto2::TestAllTypes; using ::testing::Eq; using ::testing::Optional; -class ProtoTypeIntrospectorTest - : public common_internal::ThreadCompatibleTypeTest<> { - private: - Shared NewTypeIntrospector( - MemoryManagerRef memory_manager) override { - return memory_manager.MakeShared(); - } -}; - -TEST_P(ProtoTypeIntrospectorTest, FindType) { +TEST(ProtoTypeIntrospector, FindType) { + ProtoTypeIntrospector introspector; EXPECT_THAT( - type_manager().FindType(TestAllTypes::descriptor()->full_name()), + introspector.FindType(TestAllTypes::descriptor()->full_name()), IsOkAndHolds(Optional(Eq(MessageType(TestAllTypes::GetDescriptor()))))); - EXPECT_THAT(type_manager().FindType("type.that.does.not.Exist"), + EXPECT_THAT(introspector.FindType("type.that.does.not.Exist"), IsOkAndHolds(Eq(absl::nullopt))); } -TEST_P(ProtoTypeIntrospectorTest, FindStructTypeFieldByName) { +TEST(ProtoTypeIntrospector, FindStructTypeFieldByName) { + ProtoTypeIntrospector introspector; ASSERT_OK_AND_ASSIGN( - auto field, type_manager().FindStructTypeFieldByName( + auto field, introspector.FindStructTypeFieldByName( TestAllTypes::descriptor()->full_name(), "single_int32")); ASSERT_TRUE(field.has_value()); EXPECT_THAT(field->name(), Eq("single_int32")); EXPECT_THAT(field->number(), Eq(1)); EXPECT_THAT( - type_manager().FindStructTypeFieldByName( + introspector.FindStructTypeFieldByName( TestAllTypes::descriptor()->full_name(), "field_that_does_not_exist"), IsOkAndHolds(Eq(absl::nullopt))); - EXPECT_THAT(type_manager().FindStructTypeFieldByName( - "type.that.does.not.Exist", "does_not_matter"), + EXPECT_THAT(introspector.FindStructTypeFieldByName("type.that.does.not.Exist", + "does_not_matter"), IsOkAndHolds(Eq(absl::nullopt))); } -TEST_P(ProtoTypeIntrospectorTest, FindEnumConstant) { +TEST(ProtoTypeIntrospector, FindEnumConstant) { ProtoTypeIntrospector introspector; const auto* enum_desc = TestAllTypes::NestedEnum_descriptor(); ASSERT_OK_AND_ASSIGN( @@ -77,7 +69,7 @@ TEST_P(ProtoTypeIntrospectorTest, FindEnumConstant) { EXPECT_EQ(enum_constant->number, 2); } -TEST_P(ProtoTypeIntrospectorTest, FindEnumConstantNull) { +TEST(ProtoTypeIntrospector, FindEnumConstantNull) { ProtoTypeIntrospector introspector; ASSERT_OK_AND_ASSIGN( auto enum_constant, @@ -89,7 +81,7 @@ TEST_P(ProtoTypeIntrospectorTest, FindEnumConstantNull) { EXPECT_EQ(enum_constant->number, 0); } -TEST_P(ProtoTypeIntrospectorTest, FindEnumConstantUnknownEnum) { +TEST(ProtoTypeIntrospector, FindEnumConstantUnknownEnum) { ProtoTypeIntrospector introspector; ASSERT_OK_AND_ASSIGN(auto enum_constant, @@ -97,7 +89,7 @@ TEST_P(ProtoTypeIntrospectorTest, FindEnumConstantUnknownEnum) { EXPECT_FALSE(enum_constant.has_value()); } -TEST_P(ProtoTypeIntrospectorTest, FindEnumConstantUnknownValue) { +TEST(ProtoTypeIntrospector, FindEnumConstantUnknownValue) { ProtoTypeIntrospector introspector; ASSERT_OK_AND_ASSIGN( @@ -107,11 +99,5 @@ TEST_P(ProtoTypeIntrospectorTest, FindEnumConstantUnknownValue) { ASSERT_FALSE(enum_constant.has_value()); } -INSTANTIATE_TEST_SUITE_P( - ProtoTypeIntrospectorTest, ProtoTypeIntrospectorTest, - ::testing::Values(MemoryManagement::kPooling, - MemoryManagement::kReferenceCounting), - ProtoTypeIntrospectorTest::ToString); - } // namespace } // namespace cel::extensions