[PATCH] D89004: [LLD] [COFF] Implement a GNU/ELF like -wrap option

Tue Oct 13 21:29:49 PDT 2020

MaskRay added inline comments.

================
Comment at: lld/COFF/Symbols.h:141
+  // doesn't know the final contents of the symbol.
+  uint8_t canInline : 1;
+
----------------
mstorsjo wrote:
> rnk wrote:
> > There are more kinds of IPO than just inlining, so the name isn't quite right. I also would prefer to reverse the sense so that a default value of false is the more prevalent value, and only special wrapped symbols set this to true. I suggest using the name `isHiddenFromLTO` for that purpose.
> Sounds like a good suggestion, but this is (including the comment) an exact copy of the corresponding field from the ELF side - and consistency also is nice.
> 
> Or should I go with this and then rename accordingly on the ELF side afterwards - WDYT @MaskRay?
I agree that canInline is a misnomer. `isHiddenFromLTO` may be a bit confusing as well: there is an LTO property called VisibleToRegularObj. "hidden" and "Visible" are confusing when used together. FWIW in lib/LTO, this property is called `LinkerRedefined`.

However, `LinkerRedefined` is not great for the ELF case because a linker script symbol assignment (e.g. `a = 1013;`) does not set `LinkerRedefined`. The ELF port just overrides the LTO definition in a later pass.

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

https://reviews.llvm.org/D89004