[PATCH] D124041: [ELF] Move SymbolUnion size assertion to source file

Peter Smith via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 21 03:10:39 PDT 2022


peter.smith accepted this revision.
peter.smith added a comment.
This revision is now accepted and ready to land.

This looks sensible to me. If the size of SymbolUnion is changed then Symbols.cpp will be recompiled so we'll always get the assertion if it is only changed.



================
Comment at: lld/ELF/Symbols.h:478
 // using the placement new.
 union SymbolUnion {
   alignas(Defined) char a[sizeof(Defined)];
----------------
I think it will be worth moving the majority of the comment here as this is where someone will look when trying to modify SymbolUnion.

For example:
```
// It is important to keep the size of SymbolUnion small for performance and
// memory usage reasons. 64 bytes is a soft limit based on the size of Defined
// on a 64-bit system. This is enforced by a static_assert in Symbols.cpp.
```

Not a strong opinion, just a suggestion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124041



More information about the llvm-commits mailing list