diff --git a/common/data.h b/common/data.h index b2872c6a7..799401acc 100644 --- a/common/data.h +++ b/common/data.h @@ -15,6 +15,7 @@ #ifndef THIRD_PARTY_CEL_CPP_COMMON_DATA_H_ #define THIRD_PARTY_CEL_CPP_COMMON_DATA_H_ +#include #include #include "absl/base/nullability.h" @@ -34,12 +35,11 @@ namespace common_internal { class ReferenceCount; -void SetDataReferenceCount( - absl::Nonnull data, - absl::Nonnull refcount) noexcept; +void SetDataReferenceCount(absl::Nonnull data, + absl::Nonnull refcount); absl::Nullable GetDataReferenceCount( - absl::Nonnull data) noexcept; + absl::Nonnull data); } // namespace common_internal @@ -47,9 +47,13 @@ absl::Nullable GetDataReferenceCount( // `MemoryManager`, the other is `google::protobuf::MessageLite`. class Data { public: - virtual ~Data() = default; + Data(const Data&) = default; + Data(Data&&) = default; + ~Data() = default; + Data& operator=(const Data&) = default; + Data& operator=(Data&&) = default; - absl::Nullable GetArena() const noexcept { + absl::Nullable GetArena() const { return (owner_ & kOwnerBits) == kOwnerArenaBit ? reinterpret_cast(owner_ & kOwnerPointerMask) : nullptr; @@ -61,14 +65,11 @@ class Data { // reference count ahead of time and then update it with the data it has to // delete, but that is a bit counter intuitive. Doing it this way is also // similar to how std::enable_shared_from_this works. - Data() noexcept : Data(nullptr) {} + Data() = default; - Data(const Data&) = default; - Data(Data&&) = default; - Data& operator=(const Data&) = default; - Data& operator=(Data&&) = default; + Data(std::nullptr_t) = delete; - explicit Data(absl::Nullable arena) noexcept + explicit Data(absl::Nullable arena) : owner_(reinterpret_cast(arena) | (arena != nullptr ? kOwnerArenaBit : kOwnerNone)) {} @@ -84,10 +85,9 @@ class Data { friend void common_internal::SetDataReferenceCount( absl::Nonnull data, - absl::Nonnull refcount) noexcept; + absl::Nonnull refcount); friend absl::Nullable - common_internal::GetDataReferenceCount( - absl::Nonnull data) noexcept; + common_internal::GetDataReferenceCount(absl::Nonnull data); template friend struct Ownable; template @@ -100,14 +100,14 @@ namespace common_internal { inline void SetDataReferenceCount( absl::Nonnull data, - absl::Nonnull refcount) noexcept { + absl::Nonnull refcount) { ABSL_DCHECK_EQ(data->owner_, Data::kOwnerNone); data->owner_ = reinterpret_cast(refcount) | Data::kOwnerReferenceCountBit; } inline absl::Nullable GetDataReferenceCount( - absl::Nonnull data) noexcept { + absl::Nonnull data) { return (data->owner_ & Data::kOwnerBits) == Data::kOwnerReferenceCountBit ? reinterpret_cast(data->owner_ & Data::kOwnerPointerMask) diff --git a/common/internal/reference_count.cc b/common/internal/reference_count.cc index b383e9f6d..92021e788 100644 --- a/common/internal/reference_count.cc +++ b/common/internal/reference_count.cc @@ -31,27 +31,27 @@ namespace cel::common_internal { template class DeletingReferenceCount; -template class DeletingReferenceCount; namespace { class ReferenceCountedStdString final : public ReferenceCounted { public: + static std::pair, absl::string_view> New( + std::string&& string) { + const auto* const refcount = + new ReferenceCountedStdString(std::move(string)); + const auto* const refcount_string = std::launder( + reinterpret_cast(&refcount->string_[0])); + return std::pair{ + static_cast>(refcount), + absl::string_view(*refcount_string)}; + } + explicit ReferenceCountedStdString(std::string&& string) { (::new (static_cast(&string_[0])) std::string(std::move(string))) ->shrink_to_fit(); } - const char* data() const noexcept { - return std::launder(reinterpret_cast(&string_[0])) - ->data(); - } - - size_t size() const noexcept { - return std::launder(reinterpret_cast(&string_[0])) - ->size(); - } - private: void Finalize() noexcept override { std::destroy_at(std::launder(reinterpret_cast(&string_[0]))); @@ -60,6 +60,19 @@ class ReferenceCountedStdString final : public ReferenceCounted { alignas(std::string) char string_[sizeof(std::string)]; }; +class ReferenceCountedString final : public ReferenceCounted { + public: + static std::pair, absl::string_view> New( + absl::string_view string) { + const auto* const refcount = + ::new (internal::New(Overhead() + string.size())) + ReferenceCountedString(string); + return std::pair{ + static_cast>(refcount), + absl::string_view(refcount->data_, refcount->size_)}; + } + + private: // ReferenceCountedString is non-standard-layout due to having virtual functions // from a base class. This causes compilers to warn about the use of offsetof(), // but it still works here, so silence the warning and proceed. @@ -68,54 +81,40 @@ class ReferenceCountedStdString final : public ReferenceCounted { #pragma GCC diagnostic ignored "-Winvalid-offsetof" #endif -class ReferenceCountedString final : public ReferenceCounted { - public: - static const ReferenceCountedString* New(const char* data, size_t size) { - return ::new (internal::New(offsetof(ReferenceCountedString, data_) + size)) - ReferenceCountedString(size, data); - } - - const char* data() const noexcept { return data_; } + static size_t Overhead() { return offsetof(ReferenceCountedString, data_); } - size_t size() const noexcept { return size_; } +#if defined(__GNUC__) || defined(__clang__) +#pragma GCC diagnostic pop +#endif - private: - ReferenceCountedString(size_t size, const char* data) noexcept : size_(size) { - std::memcpy(data_, data, size); + explicit ReferenceCountedString(absl::string_view string) + : size_(string.size()) { + std::memcpy(data_, string.data(), size_); } void Delete() noexcept override { void* const that = this; const auto size = size_; std::destroy_at(this); - internal::SizedDelete(that, offsetof(ReferenceCountedString, data_) + size); + internal::SizedDelete(that, Overhead() + size); } const size_t size_; char data_[]; }; -#if defined(__GNUC__) || defined(__clang__) -#pragma GCC diagnostic pop -#endif - } // namespace std::pair, absl::string_view> MakeReferenceCountedString(absl::string_view value) { ABSL_DCHECK(!value.empty()); - const auto* refcount = - ReferenceCountedString::New(value.data(), value.size()); - return std::pair{refcount, - absl::string_view(refcount->data(), refcount->size())}; + return ReferenceCountedString::New(value); } std::pair, absl::string_view> MakeReferenceCountedString(std::string&& value) { ABSL_DCHECK(!value.empty()); - const auto* refcount = new ReferenceCountedStdString(std::move(value)); - return std::pair{refcount, - absl::string_view(refcount->data(), refcount->size())}; + return ReferenceCountedStdString::New(std::move(value)); } } // namespace cel::common_internal diff --git a/common/internal/reference_count.h b/common/internal/reference_count.h index 8bc38edb6..4bf8bc8a2 100644 --- a/common/internal/reference_count.h +++ b/common/internal/reference_count.h @@ -174,6 +174,7 @@ class EmplacedReferenceCount final : public ReferenceCounted { static_assert(!std::is_reference_v, "T must not be a reference"); static_assert(!std::is_volatile_v, "T must not be volatile qualified"); static_assert(!std::is_const_v, "T must not be const qualified"); + static_assert(!std::is_array_v, "T must not be an array"); template explicit EmplacedReferenceCount(T*& value, Args&&... args) noexcept( @@ -184,7 +185,7 @@ class EmplacedReferenceCount final : public ReferenceCounted { private: void Finalize() noexcept override { - std::launder(reinterpret_cast(&value_[0]))->~T(); + std::destroy_at(std::launder(reinterpret_cast(&value_[0]))); } // We store the instance of `T` in a char buffer and use placement new and @@ -205,15 +206,12 @@ class DeletingReferenceCount final : public ReferenceCounted { : to_delete_(to_delete) {} private: - void Finalize() noexcept override { - delete std::exchange(to_delete_, nullptr); - } + void Finalize() noexcept override { delete to_delete_; } - const T* to_delete_; + absl::Nonnull const to_delete_; }; extern template class DeletingReferenceCount; -extern template class DeletingReferenceCount; template absl::Nonnull MakeDeletingReferenceCount( @@ -223,12 +221,12 @@ absl::Nonnull MakeDeletingReferenceCount( } if constexpr (std::is_base_of_v) { return new DeletingReferenceCount(to_delete); - } else if constexpr (std::is_base_of_v) { - auto* refcount = new DeletingReferenceCount(to_delete); - common_internal::SetDataReferenceCount(to_delete, refcount); - return refcount; } else { - return new DeletingReferenceCount(to_delete); + auto* refcount = new DeletingReferenceCount(to_delete); + if constexpr (std::is_base_of_v) { + common_internal::SetDataReferenceCount(to_delete, refcount); + } + return refcount; } } diff --git a/common/internal/reference_count_test.cc b/common/internal/reference_count_test.cc index 75dcd3cd4..94da0218c 100644 --- a/common/internal/reference_count_test.cc +++ b/common/internal/reference_count_test.cc @@ -99,8 +99,9 @@ struct OtherObject final { TEST(DeletingReferenceCount, Data) { auto* data = new DataObject(); const auto* refcount = MakeDeletingReferenceCount(data); - EXPECT_THAT(refcount, WhenDynamicCastTo*>( - NotNull())); + EXPECT_THAT( + refcount, + WhenDynamicCastTo*>(NotNull())); EXPECT_EQ(common_internal::GetDataReferenceCount(data), refcount); StrongUnref(refcount); }