cppgc: Fix IsGarabgeCollected trait and friends

The TraceTrait<T> checks whether T is a mixin to decide whether we can
use the fast (arithmetic) or slow (bitmap) method to look up the HoH.
Before this CL, the mixin application would also be considered as a
mixin because the marker is present, resulting in all cases going
through the object start bitmap.

The initial intention was to use the arithmetic for the mixin
applications as those inherit from GCed.

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

include/cppgc/prefinalizer.h

@@ -34,7 +34,7 @@ class PrefinalizerRegistration final {
  public:                                                                       \
   static bool InvokePreFinalizer(const cppgc::LivenessBroker& liveness_broker, \
                                  void* object) {                               \
-    static_assert(cppgc::IsGarbageCollectedTypeV<Class>,                       \
+    static_assert(cppgc::IsGarbageCollectedOrMixinTypeV<Class>,                \
                   "Only garbage collected objects can have prefinalizers");    \
     Class* self = static_cast<Class*>(object);                                 \
     if (liveness_broker.IsHeapObjectAlive(self)) return false;                 \

include/cppgc/trace-trait.h

@@ -96,6 +96,8 @@ namespace internal {
 
 template <typename T>
 struct TraceTraitImpl<T, false> {
+  static_assert(IsGarbageCollectedTypeV<T>,
+                "T must be of type GarbageCollected or GarbageCollectedMixin");
   static TraceDescriptor GetTraceDescriptor(const void* self) {
     return {self, TraceTrait<T>::Trace};
   }

include/cppgc/type-traits.h

@@ -65,12 +65,12 @@ template <typename T>
 constexpr bool IsTraceableV = IsTraceable<T>::value;
 
 template <typename T, typename = void>
-struct IsGarbageCollectedMixinType : std::false_type {
+struct HasGarbageCollectedMixinTypeMarker : std::false_type {
   static_assert(sizeof(T), "T must be fully defined");
 };
 
 template <typename T>
-struct IsGarbageCollectedMixinType<
+struct HasGarbageCollectedMixinTypeMarker<
     T,
     void_t<typename std::remove_const_t<T>::IsGarbageCollectedMixinTypeMarker>>
     : std::true_type {
@@ -78,17 +78,45 @@ struct IsGarbageCollectedMixinType<
 };
 
 template <typename T, typename = void>
-struct IsGarbageCollectedType : IsGarbageCollectedMixinType<T> {
+struct HasGarbageCollectedTypeMarker : std::false_type {
   static_assert(sizeof(T), "T must be fully defined");
 };
 
 template <typename T>
-struct IsGarbageCollectedType<
+struct HasGarbageCollectedTypeMarker<
     T, void_t<typename std::remove_const_t<T>::IsGarbageCollectedTypeMarker>>
     : std::true_type {
   static_assert(sizeof(T), "T must be fully defined");
 };
 
+template <typename T, bool = HasGarbageCollectedTypeMarker<T>::value,
+          bool = HasGarbageCollectedMixinTypeMarker<T>::value>
+struct IsGarbageCollectedMixinType : std::false_type {
+  static_assert(sizeof(T), "T must be fully defined");
+};
+
+template <typename T>
+struct IsGarbageCollectedMixinType<T, false, true> : std::true_type {
+  static_assert(sizeof(T), "T must be fully defined");
+};
+
+template <typename T, bool = HasGarbageCollectedTypeMarker<T>::value>
+struct IsGarbageCollectedType : std::false_type {
+  static_assert(sizeof(T), "T must be fully defined");
+};
+
+template <typename T>
+struct IsGarbageCollectedType<T, true> : std::true_type {
+  static_assert(sizeof(T), "T must be fully defined");
+};
+
+template <typename T>
+struct IsGarbageCollectedOrMixinType
+    : std::integral_constant<bool, IsGarbageCollectedType<T>::value ||
+                                       IsGarbageCollectedMixinType<T>::value> {
+  static_assert(sizeof(T), "T must be fully defined");
+};
+
 template <typename BasicMemberCandidate, typename WeaknessTag,
           typename WriteBarrierPolicy>
 struct IsSubclassOfBasicMemberTemplate {
@@ -134,6 +162,9 @@ template <typename T>
 constexpr bool IsGarbageCollectedTypeV =
     internal::IsGarbageCollectedType<T>::value;
 template <typename T>
+constexpr bool IsGarbageCollectedOrMixinTypeV =
+    internal::IsGarbageCollectedOrMixinType<T>::value;
+template <typename T>
 constexpr bool IsMemberTypeV = internal::IsMemberType<T>::value;
 template <typename T>
 constexpr bool IsUntracedMemberTypeV = internal::IsUntracedMemberType<T>::value;

include/cppgc/visitor.h

@@ -69,7 +69,7 @@ class V8_EXPORT Visitor {
   template <typename T>
   void Trace(const T* t) {
     static_assert(sizeof(T), "Pointee type must be fully defined.");
-    static_assert(internal::IsGarbageCollectedType<T>::value,
+    static_assert(internal::IsGarbageCollectedOrMixinType<T>::value,
                   "T must be GarbageCollected or GarbageCollectedMixin type");
     if (!t) {
       return;
@@ -97,7 +97,7 @@ class V8_EXPORT Visitor {
   template <typename T>
   void Trace(const WeakMember<T>& weak_member) {
     static_assert(sizeof(T), "Pointee type must be fully defined.");
-    static_assert(internal::IsGarbageCollectedType<T>::value,
+    static_assert(internal::IsGarbageCollectedOrMixinType<T>::value,
                   "T must be GarbageCollected or GarbageCollectedMixin type");
     static_assert(!internal::IsAllocatedOnCompactableSpace<T>::value,
                   "Weak references to compactable objects are not allowed");
@@ -277,7 +277,7 @@ class V8_EXPORT Visitor {
     using PointeeType = typename Persistent::PointeeType;
     static_assert(sizeof(PointeeType),
                   "Persistent's pointee type must be fully defined");
-    static_assert(internal::IsGarbageCollectedType<PointeeType>::value,
+    static_assert(internal::IsGarbageCollectedOrMixinType<PointeeType>::value,
                   "Persistent's pointee type must be GarbageCollected or "
                   "GarbageCollectedMixin");
     if (!p.Get()) {
@@ -294,7 +294,7 @@ class V8_EXPORT Visitor {
     using PointeeType = typename WeakPersistent::PointeeType;
     static_assert(sizeof(PointeeType),
                   "Persistent's pointee type must be fully defined");
-    static_assert(internal::IsGarbageCollectedType<PointeeType>::value,
+    static_assert(internal::IsGarbageCollectedOrMixinType<PointeeType>::value,
                   "Persistent's pointee type must be GarbageCollected or "
                   "GarbageCollectedMixin");
     static_assert(!internal::IsAllocatedOnCompactableSpace<PointeeType>::value,

test/unittests/heap/cppgc/garbage-collected-unittest.cc

@@ -47,9 +47,9 @@ TEST(GarbageCollectedTest, GarbageCollectedTrait) {
   STATIC_ASSERT(!IsGarbageCollectedType<int>::value);
   STATIC_ASSERT(!IsGarbageCollectedType<NotGCed>::value);
   STATIC_ASSERT(IsGarbageCollectedType<GCed>::value);
-  STATIC_ASSERT(IsGarbageCollectedType<Mixin>::value);
+  STATIC_ASSERT(!IsGarbageCollectedType<Mixin>::value);
   STATIC_ASSERT(IsGarbageCollectedType<GCedWithMixin>::value);
-  STATIC_ASSERT(IsGarbageCollectedType<MergedMixins>::value);
+  STATIC_ASSERT(!IsGarbageCollectedType<MergedMixins>::value);
   STATIC_ASSERT(IsGarbageCollectedType<GCWithMergedMixins>::value);
 }
 
@@ -58,9 +58,19 @@ TEST(GarbageCollectedTest, GarbageCollectedMixinTrait) {
   STATIC_ASSERT(!IsGarbageCollectedMixinType<GCed>::value);
   STATIC_ASSERT(!IsGarbageCollectedMixinType<NotGCed>::value);
   STATIC_ASSERT(IsGarbageCollectedMixinType<Mixin>::value);
-  STATIC_ASSERT(IsGarbageCollectedMixinType<GCedWithMixin>::value);
+  STATIC_ASSERT(!IsGarbageCollectedMixinType<GCedWithMixin>::value);
   STATIC_ASSERT(IsGarbageCollectedMixinType<MergedMixins>::value);
-  STATIC_ASSERT(IsGarbageCollectedMixinType<GCWithMergedMixins>::value);
+  STATIC_ASSERT(!IsGarbageCollectedMixinType<GCWithMergedMixins>::value);
+}
+
+TEST(GarbageCollectedTest, GarbageCollectedOrMixinTrait) {
+  STATIC_ASSERT(!IsGarbageCollectedOrMixinType<int>::value);
+  STATIC_ASSERT(IsGarbageCollectedOrMixinType<GCed>::value);
+  STATIC_ASSERT(!IsGarbageCollectedOrMixinType<NotGCed>::value);
+  STATIC_ASSERT(IsGarbageCollectedOrMixinType<Mixin>::value);
+  STATIC_ASSERT(IsGarbageCollectedOrMixinType<GCedWithMixin>::value);
+  STATIC_ASSERT(IsGarbageCollectedOrMixinType<MergedMixins>::value);
+  STATIC_ASSERT(IsGarbageCollectedOrMixinType<GCWithMergedMixins>::value);
 }
 
 TEST_F(GarbageCollectedTestWithHeap, GetObjectStartReturnsCurrentAddress) {