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

[Dmitry Mikulin via Phabricator via llvm-commits]

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


================
Comment at: lld/ELF/Config.h:102
+
+  // Mapping of renamed symbols to their original pre-LTO bindings.
+  llvm::DenseMap<Symbol*, uint8_t> RenamedSymbols;
----------------
davide wrote:
> This is the only field in `Config` that has a comment. Not sure how I feel about it, I'll defer it to Rui.
Removed the comment. Now it's next to the RemappedSymbol structure.


================
Comment at: lld/ELF/SymbolTable.cpp:191-194
+  if (!SB) {
+    error("-defsym: undefined symbol: " + Name);
+    return;
+  }
----------------
davide wrote:
> Is this bit needed? I don't see a test for it.
It was moved here from SymbolTable::alias, there is a test case for it already.


================
Comment at: lld/ELF/SymbolTable.cpp:195
+  }
+  Symbol *Sym = SB->symbol();
+  Symbol *AliasSym = addUndefined(Alias);
----------------
davide wrote:
> Can this ever be null? If so, you should check for it.
> If not, you may as well inline this in the single use.
It can't be null, I just find it easier to read, that's all.


https://reviews.llvm.org/D33621