[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

89 ↗

(On Diff #108447)

Move the definition to the .cpp file.

89 ↗

(On Diff #108447)

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

92 ↗

(On Diff #108447)

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

433 ↗

(On Diff #109509)

This change is dubious and shouldn't be here.

[grimar]

• Addressed review comment.

ELF/OutputSections.cpp

433 ↗

(On Diff #109509)

Fixed.

[ruiu]

LGTM

ELF/InputFiles.h

87 ↗

(On Diff #109574)

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

as what "other types" means is obvious.

lld/trunk/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 of other types.
+  ArrayRef<SymbolBody *> getSymbols() {
+    assert(FileKind == ObjectKind || FileKind == BitcodeKind ||
+           FileKind == ArchiveKind);
+    return Symbols;
+  }
+
   // 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;
+  std::vector<SymbolBody *> Symbols;
 
 private:
   const Kind FileKind;
 };
 
 template <typename ELFT> class ELFFileBase : public InputFile {
 public:
   typedef typename ELFT::Shdr Elf_Shdr;

 
 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 {
-    if (SymbolIndex >= SymbolBodies.size())
+    if (SymbolIndex >= this->Symbols.size())
       fatal(toString(this) + ": invalid symbol index");
-    return *SymbolBodies[SymbolIndex];
+    return *this->Symbols[SymbolIndex];
   }
 
   template <typename RelT>
   SymbolBody &getRelocTargetSym(const RelT &Rel) const {
     uint32_t SymIndex = Rel.getSymbol(Config->IsMips64EL);
     return getSymbolBody(SymIndex);
   }
 
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;

 };
 
 // An ArchiveFile object represents a .a file.
 class ArchiveFile : public InputFile {
 public:
   explicit ArchiveFile(std::unique_ptr<Archive> &&File);
   static bool classof(const InputFile *F) { return F->kind() == ArchiveKind; }
   template <class ELFT> void parse();
-  ArrayRef<SymbolBody *> getSymbols() { return Symbols; }
 
   // Returns a memory buffer for a given symbol and the offset in the archive
   // for the member. An empty memory buffer and an offset of zero
   // is returned if we have already returned the same memory buffer.
   // (So that we don't instantiate same members more than once.)
   std::pair<MemoryBufferRef, uint64_t> getMember(const Archive::Symbol *Sym);
 
 private:
   std::unique_ptr<Archive> File;
   llvm::DenseSet<uint64_t> Seen;
-  std::vector<SymbolBody *> Symbols;
 };
 
 class BitcodeFile : public InputFile {
 public:
   BitcodeFile(MemoryBufferRef M, StringRef ArchiveName,
               uint64_t OffsetInArchive);
   static bool classof(const InputFile *F) { return F->kind() == BitcodeKind; }
   template <class ELFT>
   void parse(llvm::DenseSet<llvm::CachedHashStringRef> &ComdatGroups);
-  ArrayRef<SymbolBody *> getSymbols() { return Symbols; }
   std::unique_ptr<llvm::lto::InputFile> Obj;
   static std::vector<BitcodeFile *> Instances;
-
-private:
-  std::vector<SymbolBody *> Symbols;
 };
 
 // .so file.
 template <class ELFT> class SharedFile : public ELFFileBase<ELFT> {
   typedef ELFFileBase<ELFT> Base;
   typedef typename ELFT::Dyn Elf_Dyn;
   typedef typename ELFT::Shdr Elf_Shdr;
   typedef typename ELFT::Sym Elf_Sym;

lld/trunk/ELF/InputFiles.cpp

 
 template <class ELFT>
 ObjFile<ELFT>::ObjFile(MemoryBufferRef M, StringRef ArchiveName)
     : ELFFileBase<ELFT>(Base::ObjectKind, M) {
   this->ArchiveName = ArchiveName;
 }
 
 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);
+  if (this->Symbols.empty())
+    return {};
+  return makeArrayRef(this->Symbols).slice(1, this->FirstNonLocal - 1);
 }
 
 template <class ELFT>
 void ObjFile<ELFT>::parse(DenseSet<CachedHashStringRef> &ComdatGroups) {
   // Read section and symbol tables.
   initializeSections(ComdatGroups);
   initializeSymbols();
 }

 
 template <class ELFT>
 StringRef ObjFile<ELFT>::getSectionName(const Elf_Shdr &Sec) {
   return check(this->getObj().getSectionName(&Sec, SectionStringTable),
                toString(this));
 }
 
 template <class ELFT> void ObjFile<ELFT>::initializeSymbols() {
-  SymbolBodies.reserve(this->ELFSyms.size());
+  this->Symbols.reserve(this->ELFSyms.size());
   for (const Elf_Sym &Sym : this->ELFSyms)
-    SymbolBodies.push_back(createSymbolBody(&Sym));
+    this->Symbols.push_back(createSymbolBody(&Sym));
 }
 
 template <class ELFT>
 InputSectionBase *ObjFile<ELFT>::getSection(const Elf_Sym &Sym) const {
   uint32_t Index = this->getSectionIndex(Sym);
   if (Index >= this->Sections.size())
     fatal(toString(this) + ": invalid section index: " + Twine(Index));
   InputSectionBase *S = this->Sections[Index];

lld/trunk/ELF/OutputSections.cpp

static void finalizeShtGroup(OutputSection *OS,
   // 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();
-  OS->Info = InX::SymTab->getSymbolIndex(Symbols[Sections[0]->Info - 1]);
+  OS->Info = InX::SymTab->getSymbolIndex(Symbols[Sections[0]->Info]);
 }
 
 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;
   for (BaseCommand *Base : Commands)