Devirtualize cel::Data

PiperOrigin-RevId: 733802177
google · copybara-service · Mar 5, 2025 · Mar 5, 2025 · Mar 5, 2025 · 70c216bb043ab06d435bd24cdc319cbc7cba6638
commit 70c216bb043ab06d435bd24cdc319cbc7cba6638
diff --git 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 <cstddef>
 #include <cstdint>

 #include "absl/base/nullability.h"
@@ -34,22 +35,25 @@ namespace common_internal {

 class ReferenceCount;

-void SetDataReferenceCount(
-    absl::Nonnull<const Data*> data,
-    absl::Nonnull<const ReferenceCount*> refcount) noexcept;
+void SetDataReferenceCount(absl::Nonnull<const Data*> data,
+                           absl::Nonnull<const ReferenceCount*> refcount);

 absl::Nullable<const ReferenceCount*> GetDataReferenceCount(
-    absl::Nonnull<const Data*> data) noexcept;
+    absl::Nonnull<const Data*> data);

 }  // namespace common_internal

 // `Data` is one of the base classes of objects that can be managed by
 // `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<google::protobuf::Arena*> GetArena() const noexcept {
+  absl::Nullable<google::protobuf::Arena*> GetArena() const {
    return (owner_ & kOwnerBits) == kOwnerArenaBit
               ? reinterpret_cast<google::protobuf::Arena*>(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<google::protobuf::Arena*> arena) noexcept
+  explicit Data(absl::Nullable<google::protobuf::Arena*> arena)
      : owner_(reinterpret_cast<uintptr_t>(arena) |
               (arena != nullptr ? kOwnerArenaBit : kOwnerNone)) {}

@@ -84,10 +85,9 @@ class Data {

  friend void common_internal::SetDataReferenceCount(
      absl::Nonnull<const Data*> data,
-      absl::Nonnull<const common_internal::ReferenceCount*> refcount) noexcept;
+      absl::Nonnull<const common_internal::ReferenceCount*> refcount);
  friend absl::Nullable<const common_internal::ReferenceCount*>
-  common_internal::GetDataReferenceCount(
-      absl::Nonnull<const Data*> data) noexcept;
+  common_internal::GetDataReferenceCount(absl::Nonnull<const Data*> data);
  template <typename T>
  friend struct Ownable;
  template <typename T>
@@ -100,14 +100,14 @@ namespace common_internal {

 inline void SetDataReferenceCount(
    absl::Nonnull<const Data*> data,
-    absl::Nonnull<const ReferenceCount*> refcount) noexcept {
+    absl::Nonnull<const ReferenceCount*> refcount) {
  ABSL_DCHECK_EQ(data->owner_, Data::kOwnerNone);
  data->owner_ =
      reinterpret_cast<uintptr_t>(refcount) | Data::kOwnerReferenceCountBit;
 }

 inline absl::Nullable<const ReferenceCount*> GetDataReferenceCount(
-    absl::Nonnull<const Data*> data) noexcept {
+    absl::Nonnull<const Data*> data) {
  return (data->owner_ & Data::kOwnerBits) == Data::kOwnerReferenceCountBit
             ? reinterpret_cast<const ReferenceCount*>(data->owner_ &
                                                       Data::kOwnerPointerMask)

diff --git a/common/internal/reference_count.cc b/common/internal/reference_count.cc
@@ -31,27 +31,27 @@
 namespace cel::common_internal {

 template class DeletingReferenceCount<google::protobuf::MessageLite>;
-template class DeletingReferenceCount<Data>;

 namespace {

 class ReferenceCountedStdString final : public ReferenceCounted {
 public:
+  static std::pair<absl::Nonnull<const ReferenceCount*>, 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<const std::string*>(&refcount->string_[0]));
+    return std::pair{
+        static_cast<absl::Nonnull<const ReferenceCount*>>(refcount),
+        absl::string_view(*refcount_string)};
+  }
+
  explicit ReferenceCountedStdString(std::string&& string) {
    (::new (static_cast<void*>(&string_[0])) std::string(std::move(string)))
        ->shrink_to_fit();
  }

-  const char* data() const noexcept {
-    return std::launder(reinterpret_cast<const std::string*>(&string_[0]))
-        ->data();
-  }
-
-  size_t size() const noexcept {
-    return std::launder(reinterpret_cast<const std::string*>(&string_[0]))
-        ->size();
-  }
-
 private:
  void Finalize() noexcept override {
    std::destroy_at(std::launder(reinterpret_cast<std::string*>(&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::Nonnull<const ReferenceCount*>, 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<absl::Nonnull<const ReferenceCount*>>(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::Nonnull<const ReferenceCount*>, 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::Nonnull<const ReferenceCount*>, 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
@@ -174,6 +174,7 @@ class EmplacedReferenceCount final : public ReferenceCounted {
  static_assert(!std::is_reference_v<T>, "T must not be a reference");
  static_assert(!std::is_volatile_v<T>, "T must not be volatile qualified");
  static_assert(!std::is_const_v<T>, "T must not be const qualified");
+  static_assert(!std::is_array_v<T>, "T must not be an array");

  template <typename... Args>
  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<T*>(&value_[0]))->~T();
+    std::destroy_at(std::launder(reinterpret_cast<T*>(&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 T*> const to_delete_;
 };

 extern template class DeletingReferenceCount<google::protobuf::MessageLite>;
-extern template class DeletingReferenceCount<Data>;

 template <typename T>
 absl::Nonnull<const ReferenceCount*> MakeDeletingReferenceCount(
@@ -223,12 +221,12 @@ absl::Nonnull<const ReferenceCount*> MakeDeletingReferenceCount(
  }
  if constexpr (std::is_base_of_v<google::protobuf::MessageLite, T>) {
    return new DeletingReferenceCount<google::protobuf::MessageLite>(to_delete);
-  } else if constexpr (std::is_base_of_v<Data, T>) {
-    auto* refcount = new DeletingReferenceCount<Data>(to_delete);
-    common_internal::SetDataReferenceCount(to_delete, refcount);
-    return refcount;
  } else {
-    return new DeletingReferenceCount<T>(to_delete);
+    auto* refcount = new DeletingReferenceCount<T>(to_delete);
+    if constexpr (std::is_base_of_v<Data, T>) {
+      common_internal::SetDataReferenceCount(to_delete, refcount);
+    }
+    return refcount;
  }
 }


diff --git 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<const DeletingReferenceCount<Data>*>(
-                            NotNull()));
+  EXPECT_THAT(
+      refcount,
+      WhenDynamicCastTo<const DeletingReferenceCount<DataObject>*>(NotNull()));
  EXPECT_EQ(common_internal::GetDataReferenceCount(data), refcount);
  StrongUnref(refcount);
 }