cppgc: Make Trace methods const

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

include/cppgc/garbage-collected.h

@@ -40,21 +40,21 @@ class GarbageCollectedBase {
 }  // namespace internal
 
 /**
- * Base class for managed objects. Only descendent types of GarbageCollected can
- * be constructed using MakeGarbageCollected. Must be inherited from as
+ * Base class for managed objects. Only descendent types of GarbageCollected
+ * can be constructed using MakeGarbageCollected. Must be inherited from as
  * left-most base class.
  *
  * Types inheriting from GarbageCollected must provide a method of
- * signature `void Trace(cppgc::Visitor*)` that dispatchs all managed pointers
- * to the visitor and delegates to garbage-collected base classes. The method
- * must be virtual if the type is not directly a child of GarbageCollected and
- * marked as final.
+ * signature `void Trace(cppgc::Visitor*) const` that dispatchs all managed
+ * pointers to the visitor and delegates to garbage-collected base classes.
+ * The method must be virtual if the type is not directly a child of
+ * GarbageCollected and marked as final.
  *
  * \code
  * // Example using final class.
  * class FinalType final : public GarbageCollected<FinalType> {
  *  public:
- *   void Trace(cppgc::Visitor* visitor) {
+ *   void Trace(cppgc::Visitor* visitor) const {
  *     // Dispatch using visitor->Trace(...);
  *   }
  * };
@@ -62,12 +62,12 @@ class GarbageCollectedBase {
  * // Example using non-final base class.
  * class NonFinalBase : public GarbageCollected<NonFinalBase> {
  *  public:
- *   virtual void Trace(cppgc::Visitor*) {}
+ *   virtual void Trace(cppgc::Visitor*) const {}
  * };
  *
  * class FinalChild final : public NonFinalBase {
  *  public:
- *   void Trace(cppgc::Visitor* visitor) final {
+ *   void Trace(cppgc::Visitor* visitor) const final {
  *     // Dispatch using visitor->Trace(...);
  *     NonFinalBase::Trace(visitor);
  *   }
@@ -88,14 +88,14 @@ class GarbageCollected : public internal::GarbageCollectedBase {
  * directly but must be mixed into the inheritance hierarchy of a
  * GarbageCollected object.
  *
- * Types inheriting from GarbageCollectedMixin must override a virtual method of
- * signature `void Trace(cppgc::Visitor*)` that dispatchs all managed pointers
- * to the visitor and delegates to base classes.
+ * Types inheriting from GarbageCollectedMixin must override a virtual method
+ * of signature `void Trace(cppgc::Visitor*) const` that dispatchs all managed
+ * pointers to the visitor and delegates to base classes.
  *
  * \code
  * class Mixin : public GarbageCollectedMixin {
  *  public:
- *   void Trace(cppgc::Visitor* visitor) override {
+ *   void Trace(cppgc::Visitor* visitor) const override {
  *     // Dispatch using visitor->Trace(...);
  *   }
  * };
@@ -119,7 +119,7 @@ class GarbageCollectedMixin : public internal::GarbageCollectedBase {
    * This Trace method must be overriden by objects inheriting from
    * GarbageCollectedMixin.
    */
-  virtual void Trace(cppgc::Visitor*) {}
+  virtual void Trace(cppgc::Visitor*) const {}
 };
 
 /**
@@ -130,7 +130,7 @@ class GarbageCollectedMixin : public internal::GarbageCollectedBase {
  * \code
  * class Mixin : public GarbageCollectedMixin {
  *  public:
- *   void Trace(cppgc::Visitor* visitor) override {
+ *   void Trace(cppgc::Visitor* visitor) const override {
  *     // Dispatch using visitor->Trace(...);
  *   }
  * };
@@ -138,7 +138,7 @@ class GarbageCollectedMixin : public internal::GarbageCollectedBase {
  * class Foo : public GarbageCollected<Foo>, public Mixin {
  *   USING_GARBAGE_COLLECTED_MIXIN();
  *  public:
- *   void Trace(cppgc::Visitor* visitor) override {
+ *   void Trace(cppgc::Visitor* visitor) const override {
  *     // Dispatch using visitor->Trace(...);
  *     Mixin::Trace(visitor);
  *   }

include/cppgc/trace-trait.h

@@ -41,8 +41,7 @@ struct TraceTrait {
   }
 
   static void Trace(Visitor* visitor, const void* self) {
-    // TODO(chromium:1056170): Remove const_cast when Trace() methods are const.
-    static_cast<T*>(const_cast<void*>(self))->Trace(visitor);
+    static_cast<const T*>(self)->Trace(visitor);
   }
 };
 

include/cppgc/type-traits.h

@@ -37,6 +37,16 @@ struct IsSubclassOfTemplate {
       decltype(SubclassCheck(std::declval<T*>()))::value;
 };
 
+// IsTraceMethodConst is used to verify that all Trace methods are marked as
+// const. It is equivalent to IsTraceable but for a non-const object.
+template <typename T, typename = void>
+struct IsTraceMethodConst : std::false_type {};
+
+template <typename T>
+struct IsTraceMethodConst<T, void_t<decltype(std::declval<const T>().Trace(
+                                 std::declval<Visitor*>()))>> : std::true_type {
+};
+
 template <typename T, typename = void>
 struct IsTraceable : std::false_type {
   static_assert(sizeof(T), "T must be fully defined");
@@ -45,7 +55,13 @@ struct IsTraceable : std::false_type {
 template <typename T>
 struct IsTraceable<
     T, void_t<decltype(std::declval<T>().Trace(std::declval<Visitor*>()))>>
-    : std::true_type {};
+    : std::true_type {
+  // All Trace methods should be marked as const. If an object of type
+  // 'T' is traceable then any object of type 'const T' should also
+  // be traceable.
+  static_assert(IsTraceMethodConst<T>(),
+                "Trace methods should be marked as const.");
+};
 
 template <typename T>
 constexpr bool IsTraceableV = IsTraceable<T>::value;

test/unittests/heap/cppgc/garbage-collected_unittest.cc

@@ -25,7 +25,7 @@ class MergedMixins : public Mixin, public OtherMixin {
   MERGE_GARBAGE_COLLECTED_MIXINS();
 
  public:
-  void Trace(cppgc::Visitor* visitor) override {
+  void Trace(cppgc::Visitor* visitor) const override {
     Mixin::Trace(visitor);
     OtherMixin::Trace(visitor);
   }
@@ -34,7 +34,9 @@ class GCWithMergedMixins : public GCed, public MergedMixins {
   USING_GARBAGE_COLLECTED_MIXIN();
 
  public:
-  void Trace(cppgc::Visitor* visitor) override { MergedMixins::Trace(visitor); }
+  void Trace(cppgc::Visitor* visitor) const override {
+    MergedMixins::Trace(visitor);
+  }
 };
 
 class GarbageCollectedTestWithHeap

test/unittests/heap/cppgc/member_unittests.cc

@@ -20,10 +20,10 @@ namespace internal {
 namespace {
 
 struct GCed : GarbageCollected<GCed> {
-  virtual void Trace(cppgc::Visitor*) {}
+  virtual void Trace(cppgc::Visitor*) const {}
 };
 struct DerivedGCed : GCed {
-  void Trace(cppgc::Visitor* v) override { GCed::Trace(v); }
+  void Trace(cppgc::Visitor* v) const override { GCed::Trace(v); }
 };
 
 // Compile tests.

test/unittests/heap/cppgc/persistent_unittests.cc

@@ -22,12 +22,12 @@ namespace {
 
 struct GCed : GarbageCollected<GCed> {
   static size_t trace_call_count;
-  virtual void Trace(cppgc::Visitor*) { ++trace_call_count; }
+  virtual void Trace(cppgc::Visitor*) const { ++trace_call_count; }
 };
 size_t GCed::trace_call_count = 0;
 
 struct DerivedGCed : GCed {
-  void Trace(cppgc::Visitor* v) override { GCed::Trace(v); }
+  void Trace(cppgc::Visitor* v) const override { GCed::Trace(v); }
 };
 
 template <template <typename> class PersistentType>

test/unittests/heap/cppgc/prefinalizer_unittests.cc

@@ -22,7 +22,7 @@ class GCed : public GarbageCollected<GCed> {
   CPPGC_USING_PRE_FINALIZER(GCed, PreFinalizer);
 
  public:
-  void Trace(Visitor*) {}
+  void Trace(Visitor*) const {}
   void PreFinalizer() { ++prefinalizer_callcount; }
 
   static size_t prefinalizer_callcount;
@@ -175,7 +175,7 @@ class AllocatingPrefinalizer : public GarbageCollected<AllocatingPrefinalizer> {
 
  public:
   explicit AllocatingPrefinalizer(cppgc::Heap* heap) : heap_(heap) {}
-  void Trace(Visitor*) {}
+  void Trace(Visitor*) const {}
   void PreFinalizer() { MakeGarbageCollected<GCed>(heap_); }
 
  private:

test/unittests/heap/cppgc/visitor_unittest.cc

@@ -26,7 +26,7 @@ class GCed : public GarbageCollected<GCed> {
 
   GCed() { trace_callcount = 0; }
 
-  virtual void Trace(cppgc::Visitor* visitor) { trace_callcount++; }
+  virtual void Trace(cppgc::Visitor* visitor) const { trace_callcount++; }
 };
 
 size_t GCed::trace_callcount;
@@ -44,7 +44,7 @@ class GCedMixinApplication : public GCed,
   USING_GARBAGE_COLLECTED_MIXIN();
 
  public:
-  void Trace(cppgc::Visitor* visitor) override {
+  void Trace(cppgc::Visitor* visitor) const override {
     GCed::Trace(visitor);
     GCedMixin::Trace(visitor);
   }