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

[Reid Kleckner via Phabricator via llvm-commits]

rnk added a comment.

I had a naming suggestion, but otherwise it looks neat.



================
Comment at: lld/COFF/MinGW.cpp:194
+// that LTO won't eliminate them.
+std::vector<lld::coff::WrappedSymbol>
+lld::coff::addWrappedSymbols(opt::InputArgList &args) {
----------------
orgads wrote:
> You have using namespace lld::coff. Remove the qualifiers? (repeats below)
True for WrappedSymbol, but for the function definition, this applies:
https://llvm.org/docs/CodingStandards.html#use-namespace-qualifiers-to-implement-previously-declared-functions


================
Comment at: lld/COFF/Symbols.h:141
+  // doesn't know the final contents of the symbol.
+  uint8_t canInline : 1;
+
----------------
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.


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

https://reviews.llvm.org/D89004