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

[Rafael Avila de Espindola via llvm-commits]

This LGTM, but wait for Rui's opinion too.

Thanks,
Rafael

George Rimar via Phabricator <reviews at reviews.llvm.org> writes:

> grimar updated this revision to Diff 109574.
> grimar added a comment.
>
> - Addressed review comment.
>
>
> https://reviews.llvm.org/D35936
>
> Files:
>   ELF/InputFiles.cpp
>   ELF/InputFiles.h
>   ELF/OutputSections.cpp
>
> Index: ELF/OutputSections.cpp
> ===================================================================
> --- ELF/OutputSections.cpp
> +++ ELF/OutputSections.cpp
> @@ -431,7 +431,7 @@
>    // 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() {
> Index: ELF/InputFiles.h
> ===================================================================
> --- ELF/InputFiles.h
> +++ ELF/InputFiles.h
> @@ -83,6 +83,14 @@
>      return Sections;
>    }
>  
> +  // Returns object file symbols. It is a runtime error to call this function
> +  // on files not of ArchiveKind/BitcodeKind/ObjectKind 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.
> @@ -100,6 +108,7 @@
>  protected:
>    InputFile(Kind K, MemoryBufferRef M);
>    std::vector<InputSectionBase *> Sections;
> +  std::vector<SymbolBody *> Symbols;
>  
>  private:
>    const Kind FileKind;
> @@ -157,18 +166,17 @@
>  
>    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 >= Symbols.size())
>        fatal(toString(this) + ": invalid symbol index");
> -    return *SymbolBodies[SymbolIndex];
> +    return *Symbols[SymbolIndex];
>    }
>  
>    template <typename RelT>
> @@ -204,9 +212,6 @@
>    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;
>  
> @@ -258,7 +263,6 @@
>    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
> @@ -269,7 +273,6 @@
>  private:
>    std::unique_ptr<Archive> File;
>    llvm::DenseSet<uint64_t> Seen;
> -  std::vector<SymbolBody *> Symbols;
>  };
>  
>  class BitcodeFile : public InputFile {
> @@ -279,12 +282,8 @@
>    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.
> Index: ELF/InputFiles.cpp
> ===================================================================
> --- ELF/InputFiles.cpp
> +++ ELF/InputFiles.cpp
> @@ -162,15 +162,9 @@
>  }
>  
>  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>
> @@ -523,9 +517,9 @@
>  }
>  
>  template <class ELFT> void ObjFile<ELFT>::initializeSymbols() {
> -  SymbolBodies.reserve(this->ELFSyms.size());
> +  Symbols.reserve(this->ELFSyms.size());
>    for (const Elf_Sym &Sym : this->ELFSyms)
> -    SymbolBodies.push_back(createSymbolBody(&Sym));
> +    Symbols.push_back(createSymbolBody(&Sym));
>  }
>  
>  template <class ELFT>