cppgc: Force EBO to always work with GCed

Currently, in the following struct

struct LayoutObject : GarbageCollected<>, MixinA, MixinB {};

the subobject that corresponds to the first base GarbageCollected<>
always takes up some space (one word). The empty-base-optimization
doesn't happen because the second base (MixinA) has the same subobject
as the first base (GarbageCollected), which is the most parent class
GarbageCollectedBase. The compiler can't "merge" them because it must
guarantee that distinct objects of the same type have distinct
addresses.

The attribute [[no_unique_address]] doesn't work for base classes,
unfortunately (but is a good idea for a Standard proposal). As a
solution, the CL simply removes GarbageCollectedBase.

Bug: chromium:1260797
Change-Id: I415b10a5fbcebce3d6ee97b8870ea9ae90f383a8
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3259654
Commit-Queue: Anton Bikineev <bikineev@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#77693}

include/cppgc/garbage-collected.h

@@ -5,8 +5,6 @@
 #ifndef INCLUDE_CPPGC_GARBAGE_COLLECTED_H_
 #define INCLUDE_CPPGC_GARBAGE_COLLECTED_H_
 
-#include <type_traits>
-
 #include "cppgc/internal/api-constants.h"
 #include "cppgc/platform.h"
 #include "cppgc/trace-trait.h"
@@ -16,28 +14,6 @@ namespace cppgc {
 
 class Visitor;
 
-namespace internal {
-
-class GarbageCollectedBase {
- public:
-  // Must use MakeGarbageCollected.
-  void* operator new(size_t) = delete;
-  void* operator new[](size_t) = delete;
-  // The garbage collector is taking care of reclaiming the object. Also,
-  // virtual destructor requires an unambiguous, accessible 'operator delete'.
-  void operator delete(void*) {
-#ifdef V8_ENABLE_CHECKS
-    internal::Abort();
-#endif  // V8_ENABLE_CHECKS
-  }
-  void operator delete[](void*) = delete;
-
- protected:
-  GarbageCollectedBase() = default;
-};
-
-}  // namespace internal
-
 /**
  * Base class for managed objects. Only descendent types of `GarbageCollected`
  * can be constructed using `MakeGarbageCollected()`. Must be inherited from as
@@ -74,11 +50,23 @@ class GarbageCollectedBase {
  * \endcode
  */
 template <typename T>
-class GarbageCollected : public internal::GarbageCollectedBase {
+class GarbageCollected {
  public:
   using IsGarbageCollectedTypeMarker = void;
   using ParentMostGarbageCollectedType = T;
 
+  // Must use MakeGarbageCollected.
+  void* operator new(size_t) = delete;
+  void* operator new[](size_t) = delete;
+  // The garbage collector is taking care of reclaiming the object. Also,
+  // virtual destructor requires an unambiguous, accessible 'operator delete'.
+  void operator delete(void*) {
+#ifdef V8_ENABLE_CHECKS
+    internal::Abort();
+#endif  // V8_ENABLE_CHECKS
+  }
+  void operator delete[](void*) = delete;
+
  protected:
   GarbageCollected() = default;
 };
@@ -101,7 +89,7 @@ class GarbageCollected : public internal::GarbageCollectedBase {
  * };
  * \endcode
  */
-class GarbageCollectedMixin : public internal::GarbageCollectedBase {
+class GarbageCollectedMixin {
  public:
   using IsGarbageCollectedMixinTypeMarker = void;
 

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

@@ -248,5 +248,17 @@ TEST_F(GarbageCollectedTestWithHeap,
       GetAllocationHandle());
 }
 
+namespace {
+
+struct MixinA : GarbageCollectedMixin {};
+struct MixinB : GarbageCollectedMixin {};
+struct GCed1 : GarbageCollected<GCed>, MixinA, MixinB {};
+struct GCed2 : MixinA, MixinB {};
+
+static_assert(
+    sizeof(GCed1) == sizeof(GCed2),
+    "Check that empty base optimization always works for GarbageCollected");
+}  // namespace
+
 }  // namespace internal
 }  // namespace cppgc