[PATCH] D33621: Fix for -wrap linker option and LTO, PR 33145

[Dmitry Mikulin via Phabricator via llvm-commits]

dmikulin marked 5 inline comments as done.
dmikulin added inline comments.


================
Comment at: lld/ELF/SymbolTable.cpp:131
+  // Restore bindings of renamed symbols.
+  for (auto &i : Config->RenamedSymbols) {
+    SymbolBody *Body = find(i.first());
----------------
davide wrote:
> Convention says all Variable names should start with capital letter. Also, `i` is a little vague.
Changed it to RSI (Renamed Symbol Iterator)


================
Comment at: llvm/lib/LTO/LTO.cpp:421
   GlobalRes.VisibleOutsideThinLTO |=
-      (Res.VisibleToRegularObj || Sym.isUsed() ||
+      (Res.LinkerRedefined || Res.VisibleToRegularObj || Sym.isUsed() ||
        Partition == GlobalResolution::RegularLTO);
----------------
davide wrote:
> If I remove this line, all the tests still pass.
This was meant for ThinLTO support, which hasn't been implemented yet. I can take it out from this patch.


================
Comment at: llvm/lib/LTO/LTO.cpp:441-442
       OS << 'x';
+    if (Res.LinkerRedefined)
+      OS << 'r';
     OS << '\n';
----------------
davide wrote:
> If I comment out this line, all the tests still pass.
Added a test point for this


https://reviews.llvm.org/D33621