[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