[PATCH] D129206: [ADT] Use Empty Base Optimization for Allocators

[Nathan James via Phabricator via llvm-commits]

njames93 added a comment.

In D129206#3633367 <https://reviews.llvm.org/D129206#3633367>, @dblaikie wrote:

> Got some testing for this? Perhaps some static_asserts in the unit test class that demonstrates the smaller size? (specifically, I thought the EBO only applied to the first base class or something like that, so the StringMap case where the allocator is the second base I'm not sure would correctly trigger EBO?)

The standard places no such restriction and clang(on a 64 bit build) currently determines StringMap to be 32 bytes, after this patch it drops to 24 bytes.



================
Comment at: llvm/include/llvm/ADT/ScopedHashTable.h:178-181
+  AllocatorTy &getAllocator() { return static_cast<AllocTy &>(*this); }
+  const AllocatorTy &getAllocator() const {
+    return static_cast<const AllocTy &>(*this);
+  }
----------------
dblaikie wrote:
> Are these casts necessary? Looks like the conversion is implicit/would work without the casts?
These may be, but I definitely had some issues building where some of the casts I've added resulted in failed builds.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129206/new/

https://reviews.llvm.org/D129206