[PATCH] D45083: [ELF] - Eliminate Lazy class.

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added a comment.

Generally looking good.



================
Comment at: ELF/SymbolTable.cpp:585
+InputFile *SymbolTable::fetchIfLazy(StringRef Name) {
+  if (auto *Sym = find(Name)) {
     // Mark the symbol not to be eliminated by LTO
----------------
Please don't use auto.


================
Comment at: ELF/SymbolTable.h:80-81
 
-  template <class ELFT> void fetchIfLazy(StringRef Name);
+  InputFile *fetchLazy(Symbol *Sym);
+  InputFile *fetchIfLazy(StringRef Name);
+
----------------
I don't think they are good name. Maybe each name makes sense, but we should avoid naming two functions very similar names. I'd just inline fetchLazy.


================
Comment at: ELF/Symbols.h:251
 
-// This represents a symbol that is not yet in the link, but we know where to
-// find it if needed. If the resolver finds both Undefined and Lazy for the same
-// name, it will ask the Lazy to load a file.
+// LazyArchive and LazyObject represents a symbols that is not yet in the link,
+// but we know where to find it if needed. If the resolver finds both Undefined
----------------
"represents a symbols" -> "represent a symbol"


https://reviews.llvm.org/D45083