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

Shoaib Meenai via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 21 18:05:00 PDT 2022


smeenai marked an inline comment as done.
smeenai added inline comments.


================
Comment at: lld/ELF/Symbols.h:478
 // using the placement new.
 union SymbolUnion {
   alignas(Defined) char a[sizeof(Defined)];
----------------
peter.smith wrote:
> 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.
Good idea, thanks. I also belatedly noticed that the other assertions would have the same issue, so I moved them as well.


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