[ELF] - Eliminate Lazy class.

[grimar]

Summary

I am not sure why we have Lazy class, it seems to be an
excessive layer that only complicates things and does not
bring something very useful. I think we can remove it,
patch do that.

[• espindola]

LGTM. It does seem a bit simpler.

Rui, do you agree?

[ruiu]

Generally looking good.

ELF/SymbolTable.cpp

583

Please don't use auto.

ELF/SymbolTable.h

80–82

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.

ELF/Symbols.h

251–253

"represents a symbols" -> "represent a symbol"

[grimar]

• Addressed review comments.
• Updated implementation slightly.

[• espindola]

Still LGTM

[ruiu]

LGTM

ELF/Driver.cpp

1084–1094

This is a bit worse than before, but I'll fix this after you commit it.

ELF/SymbolTable.cpp

583

Remove else

ELF/Driver.cpp

template <class ELFT> void LinkerDriver::link(opt::InputArgList &Args) {
 
   // Some symbols (such as __ehdr_start) are defined lazily only when there
   // are undefined symbols for them, so we add these to trigger that logic.
   for (StringRef Sym : Script->ReferencedSymbols)
     Symtab->addUndefined<ELFT>(Sym);
 
   // Handle the `--undefined <sym>` options.
   for (StringRef S : Config->Undefined)
-    Symtab->fetchIfLazy<ELFT>(S);
+    if (Symbol *Sym = Symtab->find(S))
+      if (InputFile *F = Symtab->fetchIfLazy(Sym))
+        Symtab->addFile<ELFT>(F);
 
   // If an entry symbol is in a static archive, pull out that file now
   // to complete the symbol table. After this, no new names except a
   // few linker-synthesized ones will be added to the symbol table.
-  Symtab->fetchIfLazy<ELFT>(Config->Entry);
+  if (Symbol *Sym = Symtab->find(Config->Entry))
+    if (InputFile *F = Symtab->fetchIfLazy(Sym))
+      Symtab->addFile<ELFT>(F);
 
   // Return if there were name resolution errors.
   if (errorCount())
     return;
 
   // Now when we read all script files, we want to finalize order of linker
   // script commands, which can be not yet final because of INSERT commands.
   Script->processInsertCommands();
 

ELF/InputFiles.h

 
 // Returns "<internal>", "foo.a(bar.o)" or "baz.o".
 std::string toString(const elf::InputFile *F);
 
 namespace elf {
 
 using llvm::object::Archive;
 
-class Lazy;
 class Symbol;
 
 // If -reproduce option is given, all input files are written
 // to this tar archive.
 extern llvm::TarWriter *Tar;
 
 // Opens a given file.
 llvm::Optional<MemoryBufferRef> readFile(StringRef Path);

ELF/SymbolTable.h

Symbol *addCommon(StringRef Name, uint64_t Size, uint32_t Alignment,
                     uint8_t Binding, uint8_t StOther, uint8_t Type,
                     InputFile &File);
 
   std::pair<Symbol *, bool> insert(StringRef Name);
   std::pair<Symbol *, bool> insert(StringRef Name, uint8_t Type,
                                    uint8_t Visibility, bool CanOmitFromDynSym,
                                    InputFile *File);
 
-  template <class ELFT> void fetchIfLazy(StringRef Name);
+  InputFile *fetchIfLazy(Symbol *Sym);
+
   void scanVersionScript();
 
   Symbol *find(StringRef Name);
 
   void trace(StringRef Name);
 
   void handleDynamicList();
 
 private:

ELF/SymbolTable.cpp

Symbol *SymbolTable::addUndefined(StringRef Name, uint8_t Binding,
   }
   if (S->isShared() || S->isLazy() || (S->isUndefined() && Binding != STB_WEAK))
     S->Binding = Binding;
   if (Binding != STB_WEAK) {
     if (auto *SS = dyn_cast<SharedSymbol>(S))
       if (!Config->GcSections)
         SS->getFile<ELFT>().IsNeeded = true;
   }
-  if (auto *L = dyn_cast<Lazy>(S)) {
+  if (S->isLazy()) {
     // An undefined weak will not fetch archive members. See comment on Lazy in
     // Symbols.h for the details.
     if (Binding == STB_WEAK)
-      L->Type = Type;
-    else if (InputFile *F = L->fetch())
+      S->Type = Type;
+    else if (InputFile *F = Symtab->fetchIfLazy(S))
       addFile<ELFT>(F);
   }
   return S;
 }
 
 // Using .symver foo,foo@@VER unfortunately creates two symbols: foo and
 // foo@@VER. We want to effectively ignore foo, so give precedence to
 // foo@@VER.
if (S->isWeak()) {
     replaceSymbol<LazyObject>(S, Obj, Name, S->Type);
     S->Binding = STB_WEAK;
     return;
   }
   if (InputFile *F = Obj.fetch())
     addFile<ELFT>(F);
 }
 
-// If we already saw this symbol, force loading its file.
-template <class ELFT> void SymbolTable::fetchIfLazy(StringRef Name) {
-  if (Symbol *B = find(Name)) {
-    // Mark the symbol not to be eliminated by LTO
-    // even if it is a bitcode symbol.
-    B->IsUsedInRegularObj = true;
-    if (auto *L = dyn_cast<Lazy>(B))
-      if (InputFile *File = L->fetch())
-        addFile<ELFT>(File);
-  }
+InputFile *SymbolTable::fetchIfLazy(Symbol *Sym) {
+  Sym->IsUsedInRegularObj = true;
+  if (LazyArchive *L = dyn_cast<LazyArchive>(Sym))
+    return L->fetch();
+  if (LazyObject *L = dyn_cast<LazyObject>(Sym))
+    return cast<LazyObjFile>(L->File)->fetch();
+  return nullptr;
 }
 
 // Initialize DemangledSyms with a map from demangled symbols to symbol
 // objects. Used to handle "extern C++" directive in version scripts.
 //
 // The map will contain all demangled symbols. That can be very large,
 // and in LLD we generally want to avoid do anything for each symbol.
 // Then, why are we doing this? Here's why.
template void SymbolTable::addShared<ELF32BE>(StringRef, SharedFile<ELF32BE> &,
                                               const typename ELF32BE::Sym &,
                                               uint32_t Alignment, uint32_t);
 template void SymbolTable::addShared<ELF64LE>(StringRef, SharedFile<ELF64LE> &,
                                               const typename ELF64LE::Sym &,
                                               uint32_t Alignment, uint32_t);
 template void SymbolTable::addShared<ELF64BE>(StringRef, SharedFile<ELF64BE> &,
                                               const typename ELF64BE::Sym &,
                                               uint32_t Alignment, uint32_t);
-
-template void SymbolTable::fetchIfLazy<ELF32LE>(StringRef);
-template void SymbolTable::fetchIfLazy<ELF32BE>(StringRef);
-template void SymbolTable::fetchIfLazy<ELF64LE>(StringRef);
-template void SymbolTable::fetchIfLazy<ELF64BE>(StringRef);

ELF/Symbols.h

public:
 
   bool isLazy() const {
     return SymbolKind == LazyArchiveKind || SymbolKind == LazyObjectKind;
   }
 
   // True if this is an undefined weak symbol. This only works once
   // all input files have been added.
   bool isUndefWeak() const {
-    // See comment on Lazy for details.
+    // See comment on lazy symbols for details.
     return isWeak() && (isUndefined() || isLazy());
   }
 
   StringRef getName() const { return Name; }
   void parseSymbolVersion();
 
   bool isInGot() const { return GotIndex != -1U; }
   bool isInPlt() const { return PltIndex != -1U; }
public:
   uint64_t Size;  // st_size
 
   // This field is a index to the symbol's version definition.
   uint32_t VerdefIndex;
 
   uint32_t Alignment;
 };
 
-// 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 represent a symbols 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.
 //
 // A special complication is the handling of weak undefined symbols. They should
 // not load a file, but we have to remember we have seen both the weak undefined
 // and the lazy. We represent that with a lazy symbol with a weak binding. This
 // means that code looking for undefined symbols normally also has to take lazy
 // symbols into consideration.
-class Lazy : public Symbol {
-public:
-  static bool classof(const Symbol *S) { return S->isLazy(); }
-
-  // Returns an object file for this symbol, or a nullptr if the file
-  // was already returned.
-  InputFile *fetch();
-
-protected:
-  Lazy(Kind K, InputFile &File, StringRef Name, uint8_t Type)
-      : Symbol(K, &File, Name, llvm::ELF::STB_GLOBAL, llvm::ELF::STV_DEFAULT,
-               Type) {}
-};
 
 // This class represents a symbol defined in an archive file. It is
 // created from an archive file header, and it knows how to load an
 // object file from an archive to replace itself with a defined
 // symbol.
-class LazyArchive : public Lazy {
+class LazyArchive : public Symbol {
 public:
   LazyArchive(InputFile &File, const llvm::object::Archive::Symbol S,
               uint8_t Type)
-      : Lazy(LazyArchiveKind, File, S.getName(), Type), Sym(S) {}
+      : Symbol(LazyArchiveKind, &File, S.getName(), llvm::ELF::STB_GLOBAL,
+               llvm::ELF::STV_DEFAULT, Type),
+        Sym(S) {}
 
   static bool classof(const Symbol *S) { return S->kind() == LazyArchiveKind; }
 
   InputFile *fetch();
 
 private:
   const llvm::object::Archive::Symbol Sym;
 };
 
 // LazyObject symbols represents symbols in object files between
 // --start-lib and --end-lib options.
-class LazyObject : public Lazy {
+class LazyObject : public Symbol {
 public:
   LazyObject(InputFile &File, StringRef Name, uint8_t Type)
-      : Lazy(LazyObjectKind, File, Name, Type) {}
+      : Symbol(LazyObjectKind, &File, Name, llvm::ELF::STB_GLOBAL,
+               llvm::ELF::STV_DEFAULT, Type) {}
 
   static bool classof(const Symbol *S) { return S->kind() == LazyObjectKind; }
 };
 
 // Some linker-generated symbols need to be created as
 // Defined symbols.
 struct ElfSym {
   // __bss_start

ELF/Symbols.cpp

void Symbol::parseSymbolVersion() {
   // Usually version script is not provided when linking executable,
   // but we may still want to override a versioned symbol from DSO,
   // so we do not report error in this case.
   if (Config->Shared)
     error(toString(File) + ": symbol " + S + " has undefined version " +
           Verstr);
 }
 
-InputFile *Lazy::fetch() {
-  if (auto *Sym = dyn_cast<LazyArchive>(this))
-    return Sym->fetch();
-  return cast<LazyObjFile>(File)->fetch();
-}
-
 InputFile *LazyArchive::fetch() { return cast<ArchiveFile>(File)->fetch(Sym); }
 
 uint8_t Symbol::computeBinding() const {
   if (Config->Relocatable)
     return Binding;
   if (Visibility != STV_DEFAULT && Visibility != STV_PROTECTED)
     return STB_LOCAL;
   if (VersionId == VER_NDX_LOCAL && isDefined())