cppgc: Add support for const T in Member and friends

Adds support for Member<const T> by keeping the untyped storage in
MemberBase const, which is stronger than the required constness. All
accesses go through BasicMember which can re-add the appropriate
constness specified by the user.

The same concept is applied to all Member and Persistent handles.

Bug: chromium:1056170
Change-Id: I5a620258be3acb6a1b4b1437e69b8d7d1ec5ce6f
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2625871Reviewed-by: Omer Katz <omerkatz@chromium.org>
Reviewed-by: Anton Bikineev <bikineev@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#72090}

include/cppgc/cross-thread-persistent.h

@@ -162,7 +162,7 @@ class BasicCrossThreadPersistent final : public PersistentBase,
   // heterogeneous assignments between different Member and Persistent handles
   // based on their actual types.
   V8_CLANG_NO_SANITIZE("cfi-unrelated-cast") T* Get() const {
-    return static_cast<T*>(GetValue());
+    return static_cast<T*>(const_cast<void*>(GetValue()));
   }
 
   /**
@@ -210,7 +210,9 @@ class BasicCrossThreadPersistent final : public PersistentBase,
   T& operator*() const { return *Get(); }
 
  private:
-  static bool IsValid(void* ptr) { return ptr && ptr != kSentinelPointer; }
+  static bool IsValid(const void* ptr) {
+    return ptr && ptr != kSentinelPointer;
+  }
 
   static void Trace(Visitor* v, const void* ptr) {
     const auto* handle = static_cast<const BasicCrossThreadPersistent*>(ptr);
@@ -218,7 +220,7 @@ class BasicCrossThreadPersistent final : public PersistentBase,
   }
 
   void Assign(T* ptr) {
-    void* old_value = GetValue();
+    const void* old_value = GetValue();
     if (IsValid(old_value)) {
       PersistentRegionLock guard;
       PersistentRegion& region = this->GetPersistentRegion(old_value);
@@ -238,7 +240,7 @@ class BasicCrossThreadPersistent final : public PersistentBase,
   }
 
   void AssignUnsafe(T* ptr) {
-    void* old_value = GetValue();
+    const void* old_value = GetValue();
     if (IsValid(old_value)) {
       PersistentRegion& region = this->GetPersistentRegion(old_value);
       if (IsValid(ptr) && (&region == &this->GetPersistentRegion(ptr))) {

include/cppgc/internal/pointer-policies.h

@@ -105,22 +105,22 @@ using DefaultLocationPolicy = IgnoreLocationPolicy;
 
 struct StrongPersistentPolicy {
   using IsStrongPersistent = std::true_type;
-  static V8_EXPORT PersistentRegion& GetPersistentRegion(void* object);
+  static V8_EXPORT PersistentRegion& GetPersistentRegion(const void* object);
 };
 
 struct WeakPersistentPolicy {
   using IsStrongPersistent = std::false_type;
-  static V8_EXPORT PersistentRegion& GetPersistentRegion(void* object);
+  static V8_EXPORT PersistentRegion& GetPersistentRegion(const void* object);
 };
 
 struct StrongCrossThreadPersistentPolicy {
   using IsStrongPersistent = std::true_type;
-  static V8_EXPORT PersistentRegion& GetPersistentRegion(void* object);
+  static V8_EXPORT PersistentRegion& GetPersistentRegion(const void* object);
 };
 
 struct WeakCrossThreadPersistentPolicy {
   using IsStrongPersistent = std::false_type;
-  static V8_EXPORT PersistentRegion& GetPersistentRegion(void* object);
+  static V8_EXPORT PersistentRegion& GetPersistentRegion(const void* object);
 };
 
 // Forward declarations setting up the default policies.

include/cppgc/member.h

@@ -19,28 +19,30 @@ class Visitor;
 
 namespace internal {
 
+// MemberBase always refers to the object as const object and defers to
+// BasicMember on casting to the right type as needed.
 class MemberBase {
  protected:
   MemberBase() = default;
-  explicit MemberBase(void* value) : raw_(value) {}
+  explicit MemberBase(const void* value) : raw_(value) {}
 
-  void** GetRawSlot() const { return &raw_; }
-  void* GetRaw() const { return raw_; }
+  const void** GetRawSlot() const { return &raw_; }
+  const void* GetRaw() const { return raw_; }
   void SetRaw(void* value) { raw_ = value; }
 
-  void* GetRawAtomic() const {
-    return reinterpret_cast<const std::atomic<void*>*>(&raw_)->load(
+  const void* GetRawAtomic() const {
+    return reinterpret_cast<const std::atomic<const void*>*>(&raw_)->load(
         std::memory_order_relaxed);
   }
-  void SetRawAtomic(void* value) {
-    reinterpret_cast<std::atomic<void*>*>(&raw_)->store(
+  void SetRawAtomic(const void* value) {
+    reinterpret_cast<std::atomic<const void*>*>(&raw_)->store(
         value, std::memory_order_relaxed);
   }
 
   void ClearFromGC() const { raw_ = nullptr; }
 
  private:
-  mutable void* raw_ = nullptr;
+  mutable const void* raw_ = nullptr;
 };
 
 // The basic class from which all Member classes are 'generated'.
@@ -167,7 +169,11 @@ class BasicMember final : private MemberBase, private CheckingPolicy {
   // based on their actual types.
   V8_CLANG_NO_SANITIZE("cfi-unrelated-cast") T* Get() const {
     // Executed by the mutator, hence non atomic load.
-    return static_cast<T*>(MemberBase::GetRaw());
+    //
+    // The const_cast below removes the constness from MemberBase storage. The
+    // following static_cast re-adds any constness if specified through the
+    // user-visible template parameter T.
+    return static_cast<T*>(const_cast<void*>(MemberBase::GetRaw()));
   }
 
   void Clear() { SetRawAtomic(nullptr); }
@@ -179,12 +185,12 @@ class BasicMember final : private MemberBase, private CheckingPolicy {
   }
 
   const T** GetSlotForTesting() const {
-    return reinterpret_cast<const T**>(const_cast<const void**>(GetRawSlot()));
+    return reinterpret_cast<const T**>(GetRawSlot());
   }
 
  private:
-  T* GetRawAtomic() const {
-    return static_cast<T*>(MemberBase::GetRawAtomic());
+  const T* GetRawAtomic() const {
+    return static_cast<const T*>(MemberBase::GetRawAtomic());
   }
 
   void InitializingWriteBarrier() const {

include/cppgc/persistent.h

@@ -20,13 +20,15 @@ class Visitor;
 
 namespace internal {
 
+// PersistentBase always refers to the object as const object and defers to
+// BasicPersistent on casting to the right type as needed.
 class PersistentBase {
  protected:
   PersistentBase() = default;
-  explicit PersistentBase(void* raw) : raw_(raw) {}
+  explicit PersistentBase(const void* raw) : raw_(raw) {}
 
-  void* GetValue() const { return raw_; }
-  void SetValue(void* value) { raw_ = value; }
+  const void* GetValue() const { return raw_; }
+  void SetValue(const void* value) { raw_ = value; }
 
   PersistentNode* GetNode() const { return node_; }
   void SetNode(PersistentNode* node) { node_ = node; }
@@ -39,7 +41,7 @@ class PersistentBase {
   }
 
  private:
-  mutable void* raw_ = nullptr;
+  mutable const void* raw_ = nullptr;
   mutable PersistentNode* node_ = nullptr;
 
   friend class PersistentRegion;
@@ -178,7 +180,7 @@ class BasicPersistent final : public PersistentBase,
   }
 
   explicit operator bool() const { return Get(); }
-  operator T*() const { return Get(); }
+  operator T*() const { return Get(); }  // NOLINT
   T* operator->() const { return Get(); }
   T& operator*() const { return *Get(); }
 
@@ -186,7 +188,10 @@ class BasicPersistent final : public PersistentBase,
   // heterogeneous assignments between different Member and Persistent handles
   // based on their actual types.
   V8_CLANG_NO_SANITIZE("cfi-unrelated-cast") T* Get() const {
-    return static_cast<T*>(GetValue());
+    // The const_cast below removes the constness from PersistentBase storage.
+    // The following static_cast re-adds any constness if specified through the
+    // user-visible template parameter T.
+    return static_cast<T*>(const_cast<void*>(GetValue()));
   }
 
   void Clear() { Assign(nullptr); }

src/heap/cppgc/heap-page.h

@@ -33,8 +33,7 @@ class V8_EXPORT_PRIVATE BasePage {
   BasePage(const BasePage&) = delete;
   BasePage& operator=(const BasePage&) = delete;
 
-  HeapBase* heap() { return heap_; }
-  const HeapBase* heap() const { return heap_; }
+  HeapBase* heap() const { return heap_; }
 
   BaseSpace* space() { return space_; }
   const BaseSpace* space() const { return space_; }

src/heap/cppgc/pointer-policies.cc

@@ -21,24 +21,26 @@ void EnabledCheckingPolicy::CheckPointer(const void* ptr) {
   // TODO(chromium:1056170): Provide implementation.
 }
 
-PersistentRegion& StrongPersistentPolicy::GetPersistentRegion(void* object) {
+PersistentRegion& StrongPersistentPolicy::GetPersistentRegion(
+    const void* object) {
   auto* heap = BasePage::FromPayload(object)->heap();
   return heap->GetStrongPersistentRegion();
 }
 
-PersistentRegion& WeakPersistentPolicy::GetPersistentRegion(void* object) {
+PersistentRegion& WeakPersistentPolicy::GetPersistentRegion(
+    const void* object) {
   auto* heap = BasePage::FromPayload(object)->heap();
   return heap->GetWeakPersistentRegion();
 }
 
 PersistentRegion& StrongCrossThreadPersistentPolicy::GetPersistentRegion(
-    void* object) {
+    const void* object) {
   auto* heap = BasePage::FromPayload(object)->heap();
   return heap->GetStrongCrossThreadPersistentRegion();
 }
 
 PersistentRegion& WeakCrossThreadPersistentPolicy::GetPersistentRegion(
-    void* object) {
+    const void* object) {
   auto* heap = BasePage::FromPayload(object)->heap();
   return heap->GetWeakCrossThreadPersistentRegion();
 }

test/unittests/heap/cppgc/member-unittest.cc

@@ -98,6 +98,12 @@ void EmptyTest() {
     EXPECT_EQ(nullptr, empty.Get());
     EXPECT_EQ(nullptr, empty.Release());
   }
+  {
+    // Move-constructs empty from another Member that is created from nullptr.
+    MemberType<const GCed> empty = nullptr;
+    EXPECT_EQ(nullptr, empty.Get());
+    EXPECT_EQ(nullptr, empty.Release());
+  }
 }
 
 TEST_F(MemberTest, Empty) {

test/unittests/heap/cppgc/persistent-family-unittest.cc

@@ -135,7 +135,7 @@ void NullStateCtor(cppgc::Heap* heap) {
   }
   {
     // Runtime null must not allocated associated node.
-    PersistentType<GCed> empty = static_cast<GCed*>(0);
+    PersistentType<GCed> empty = static_cast<GCed*>(nullptr);
     EXPECT_EQ(nullptr, empty.Get());
     EXPECT_EQ(nullptr, empty.Release());
     EXPECT_EQ(0u, GetRegion<Persistent>(heap).NodesInUse());
@@ -167,6 +167,12 @@ void RawCtor(cppgc::Heap* heap) {
     EXPECT_EQ(1u, GetRegion<PersistentType>(heap).NodesInUse());
   }
   EXPECT_EQ(0u, GetRegion<PersistentType>(heap).NodesInUse());
+  {
+    PersistentType<const GCed> p = gced;
+    EXPECT_EQ(gced, p.Get());
+    EXPECT_EQ(1u, GetRegion<PersistentType>(heap).NodesInUse());
+  }
+  EXPECT_EQ(0u, GetRegion<PersistentType>(heap).NodesInUse());
 }
 
 TEST_F(PersistentTest, RawCtor) {