[ELF] - Move getSymbols() methods to InputFile.

[grimar]

Summary

I suggest to do this change, together with D35987 it allows to detemplate code like:

for (elf::ObjectFile<ELFT> *File : Symtab<ELFT>::X->getObjectFiles()) {
  for (SymbolBody *Body : File->getSymbols()) {
    auto *D = dyn_cast<DefinedRegular>(Body);
    if (!D || !D->Section)
      continue;

Going to use it in D35843, which need iterating over DefinedRegular symbols
from non-ELFT-templated code.

Patch detemplates finalizeShtGroup() to show it is itself can be useful for detemplation.

[ruiu]

ELF/InputFiles.h

88

Move the definition to the .cpp file.

88

Do not rename this method unless it is needed. getSymbols should just work.

91

return {}

[grimar]

• All comments addressed.

[ruiu]

Overall, it doesn't seem like an improvement. You don't need to push too hard to remove templates. Use of templates is not necessarily bad.

That being said, if you think that this change could remove a lot of uses of templates, please do that in this patch to demonstrate it.

[grimar]

• Moved all getSymbols() methods to InputFile.

[ruiu]

ELF/OutputSections.cpp

431

This change is dubious and shouldn't be here.

[grimar]

• Addressed review comment.

ELF/OutputSections.cpp

431

Fixed.

[ruiu]

LGTM

ELF/InputFiles.h

87

on files not of ... types -> on files of other types.

as what "other types" means is obvious.

ELF/InputFiles.h

public:
 
   // Returns sections. It is a runtime error to call this function
   // on files that don't have the notion of sections.
   ArrayRef<InputSectionBase *> getSections() const {
     assert(FileKind == ObjectKind || FileKind == BinaryKind);
     return Sections;
   }
 
+  // Returns object file symbols. It is a runtime error to call this function
+  // on files not of ObjectKind type.
+  ArrayRef<SymbolBody *> getSymbols() const;
+
   // Filename of .a which contained this file. If this file was
   // not in an archive file, it is the empty string. We use this
   // string for creating error messages.
   StringRef ArchiveName;
 
   // If this is an architecture-specific file, the following members
   // have ELF type (i.e. ELF{32,64}{LE,BE}) and target machine type.
   ELFKind EKind = ELFNoneKind;
   uint16_t EMachine = llvm::ELF::EM_NONE;
   uint8_t OSABI = 0;
 
   // Cache for toString(). Only toString() should use this member.
   mutable std::string ToStringCache;
 
 protected:
   InputFile(Kind K, MemoryBufferRef M);
   std::vector<InputSectionBase *> Sections;
 
+  // List of all symbols referenced or defined by this file.
+  // Applicable for ObjectKind files only.
+  std::vector<SymbolBody *> SymbolBodies;
+
 private:
   const Kind FileKind;
 };
 
 template <typename ELFT> class ELFFileBase : public InputFile {
 public:
   typedef typename ELFT::Shdr Elf_Shdr;
   typedef typename ELFT::Sym Elf_Sym;

 
 public:
   static bool classof(const InputFile *F) {
     return F->kind() == Base::ObjectKind;
   }
 
   static std::vector<ObjFile<ELFT> *> Instances;
 
-  ArrayRef<SymbolBody *> getSymbols();
   ArrayRef<SymbolBody *> getLocalSymbols();
 
   ObjFile(MemoryBufferRef M, StringRef ArchiveName);
   void parse(llvm::DenseSet<llvm::CachedHashStringRef> &ComdatGroups);
 
   InputSectionBase *getSection(const Elf_Sym &Sym) const;
 
   SymbolBody &getSymbolBody(uint32_t SymbolIndex) const {
private:
   void initializeDwarfLine();
   InputSectionBase *getRelocTarget(const Elf_Shdr &Sec);
   InputSectionBase *createInputSection(const Elf_Shdr &Sec);
   StringRef getSectionName(const Elf_Shdr &Sec);
 
   bool shouldMerge(const Elf_Shdr &Sec);
   SymbolBody *createSymbolBody(const Elf_Sym *Sym);
 
-  // List of all symbols referenced or defined by this file.
-  std::vector<SymbolBody *> SymbolBodies;
-
   // .shstrtab contents.
   StringRef SectionStringTable;
 
   // Debugging information to retrieve source file and line for error
   // reporting. Linker may find reasonable number of errors in a
   // single object file, so we cache debugging information in order to
   // parse it only once for each object file we link.
   std::unique_ptr<llvm::DWARFDebugLine> DwarfLine;

ELF/InputFiles.cpp

 
 using namespace lld;
 using namespace lld::elf;
 
 TarWriter *elf::Tar;
 
 InputFile::InputFile(Kind K, MemoryBufferRef M) : MB(M), FileKind(K) {}
 
+ArrayRef<SymbolBody *> InputFile::getSymbols() const {
+  assert(FileKind == ObjectKind);
+  if (SymbolBodies.empty())
+    return {};
+  return llvm::makeArrayRef(SymbolBodies).slice(1);
+}
+
 Optional<MemoryBufferRef> elf::readFile(StringRef Path) {
   // The --chroot option changes our virtual root directory.
   // This is useful when you are dealing with files created by --reproduce.
   if (!Config->Chroot.empty() && Path.startswith("/"))
     Path = Saver.save(Config->Chroot + Path);
 
   log(Path);
 

 }
 
 template <class ELFT> ArrayRef<SymbolBody *> ObjFile<ELFT>::getLocalSymbols() {
   if (this->SymbolBodies.empty())
     return this->SymbolBodies;
   return makeArrayRef(this->SymbolBodies).slice(1, this->FirstNonLocal - 1);
 }
 
-template <class ELFT> ArrayRef<SymbolBody *> ObjFile<ELFT>::getSymbols() {
-  if (this->SymbolBodies.empty())
-    return this->SymbolBodies;
-  return makeArrayRef(this->SymbolBodies).slice(1);
-}
-
 template <class ELFT>
 void ObjFile<ELFT>::parse(DenseSet<CachedHashStringRef> &ComdatGroups) {
   // Read section and symbol tables.
   initializeSections(ComdatGroups);
   initializeSymbols();
 }
 
 // Sections with SHT_GROUP and comdat bits define comdat section groups.

ELF/OutputSections.cpp

static bool compareByFilePosition(InputSection *A, InputSection *B) {
   InputSection *LB = B->getLinkOrderDep();
   OutputSection *AOut = LA->getParent();
   OutputSection *BOut = LB->getParent();
   if (AOut != BOut)
     return AOut->SectionIndex < BOut->SectionIndex;
   return LA->OutSecOff < LB->OutSecOff;
 }
 
-template <class ELFT>
 static void finalizeShtGroup(OutputSection *OS,
                              ArrayRef<InputSection *> Sections) {
   assert(Config->Relocatable && Sections.size() == 1);
 
   // sh_link field for SHT_GROUP sections should contain the section index of
   // the symbol table.
   OS->Link = InX::SymTab->getParent()->SectionIndex;
 
   // sh_info then contain index of an entry in symbol table section which
   // provides signature of the section group.
-  ObjFile<ELFT> *Obj = Sections[0]->getFile<ELFT>();
-  ArrayRef<SymbolBody *> Symbols = Obj->getSymbols();
+  ArrayRef<SymbolBody *> Symbols = Sections[0]->File->getSymbols();
   OS->Info = InX::SymTab->getSymbolIndex(Symbols[Sections[0]->Info - 1]);
 }
 
 template <class ELFT> void OutputSection::finalize() {
   // Link order may be distributed across several InputSectionDescriptions
   // but sort must consider them all at once.
   std::vector<InputSection **> ScriptSections;
   std::vector<InputSection *> Sections;
if ((Flags & SHF_LINK_ORDER)) {
     // SHF_LINK_ORDER flag. The dependency is indicated by the sh_link field. We
     // need to translate the InputSection sh_link to the OutputSection sh_link,
     // all InputSections in the OutputSection have the same dependency.
     if (auto *D = Sections.front()->getLinkOrderDep())
       Link = D->getParent()->SectionIndex;
   }
 
   if (Type == SHT_GROUP) {
-    finalizeShtGroup<ELFT>(this, Sections);
+    finalizeShtGroup(this, Sections);
     return;
   }
 
   if (!Config->CopyRelocs || (Type != SHT_RELA && Type != SHT_REL))
     return;
 
   InputSection *First = Sections[0];
   if (isa<SyntheticSection>(First))