[ELF] - Merge SymbolListFile and LinkerScript code.

[grimar]

Summary

It was suggested in comments for D23609 to
move code from SymbolListFile to LinkerScript.

Patch do this change.

BTW, do we want to rename LinkerScript.cpp to something then ?

[ruiu]

I was trying to say is to merge the two classes. I'd remove VersionScriptParser class and move the members to ScriptParser, so that you don't need to "cross the boundary" to read a version element.

ELF/CMakeLists.txt

add_lld_library(lldELF
   LTO.cpp
   LinkerScript.cpp
   MarkLive.cpp
   Mips.cpp
   OutputSections.cpp
   Relocations.cpp
   ScriptParser.cpp
   Strings.cpp
-  SymbolListFile.cpp
   SymbolTable.cpp
   Symbols.cpp
   Target.cpp
   Thunks.cpp
   Writer.cpp
 
   LINK_COMPONENTS
   ${LLVM_TARGETS_TO_BUILD}

ELF/Driver.cpp

 #include "Driver.h"
 #include "Config.h"
 #include "Error.h"
 #include "ICF.h"
 #include "InputFiles.h"
 #include "InputSection.h"
 #include "LinkerScript.h"
 #include "Strings.h"
-#include "SymbolListFile.h"
 #include "SymbolTable.h"
 #include "Target.h"
 #include "Writer.h"
 #include "lld/Driver/Driver.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/raw_ostream.h"

ELF/LinkerScript.h

 template <class ELFT> class InputSectionBase;
 template <class ELFT> class OutputSectionBase;
 template <class ELFT> class OutputSectionFactory;
 template <class ELFT> class DefinedCommon;
 template <class ELFT> class LayoutInputSection;
 
 typedef std::function<uint64_t(uint64_t)> Expr;
 
+void parseDynamicList(MemoryBufferRef MB);
+
+void parseVersionScript(MemoryBufferRef MB);
+
 // Parses a linker script. Calling this function updates
 // Config and ScriptConfig.
 void readLinkerScript(MemoryBufferRef MB);
 
 // This enum is used to implement linker script SECTIONS command.
 // https://sourceware.org/binutils/docs/ld/SECTIONS.html#SECTIONS
 enum SectionsCommandKind {
   AssignmentKind,

ELF/LinkerScript.cpp

 using namespace llvm;
 using namespace llvm::ELF;
 using namespace llvm::object;
 using namespace lld;
 using namespace lld::elf;
 
 ScriptConfiguration *elf::ScriptConfig;
 
+// Parse the --dynamic-list argument.  A dynamic list is in the form
+//
+//  { symbol1; symbol2; [...]; symbolN };
+//
+// Multiple groups can be defined in the same file, and they are merged
+// into a single group.
+
+namespace {
+class DynamicListParser final : public ScriptParserBase {
+public:
+  DynamicListParser(StringRef S) : ScriptParserBase(S) {}
+  void run();
+};
+} // end anonymous namespace
+
+void DynamicListParser::run() {
+  while (!atEOF()) {
+    expect("{");
+    while (!Error) {
+      Config->DynamicList.push_back(next());
+      expect(";");
+      if (skip("}"))
+        break;
+    }
+    expect(";");
+  }
+}
+
+void elf::parseDynamicList(MemoryBufferRef MB) {
+  DynamicListParser(MB.getBuffer()).run();
+}
+
+// Parse the --version-script argument. We currently only accept the following
+// version script syntax:
+//
+//  { [ global: symbol1; symbol2; [...]; symbolN; ] local: *; };
+//
+
+namespace {
+class VersionScriptParser final : public ScriptParserBase {
+public:
+  VersionScriptParser(StringRef S) : ScriptParserBase(S) {}
+
+  void run();
+
+private:
+  void parseExtern(std::vector<SymbolVersion> *Globals);
+  void parseVersion(StringRef VerStr);
+  void parseGlobal(StringRef VerStr);
+  void parseLocal();
+};
+} // end anonymous namespace
+
+void VersionScriptParser::parseVersion(StringRef VerStr) {
+  // Identifiers start at 2 because 0 and 1 are reserved
+  // for VER_NDX_LOCAL and VER_NDX_GLOBAL constants.
+  size_t VersionId = Config->VersionDefinitions.size() + 2;
+  Config->VersionDefinitions.push_back({VerStr, VersionId});
+
+  if (skip("global:") || peek() != "local:")
+    parseGlobal(VerStr);
+  if (skip("local:"))
+    parseLocal();
+  expect("}");
+
+  // Each version may have a parent version. For example, "Ver2" defined as
+  // "Ver2 { global: foo; local: *; } Ver1;" has "Ver1" as a parent. This
+  // version hierarchy is, probably against your instinct, purely for human; the
+  // runtime doesn't care about them at all. In LLD, we simply skip the token.
+  if (!VerStr.empty() && peek() != ";")
+    next();
+  expect(";");
+}
+
+void VersionScriptParser::parseLocal() {
+  Config->DefaultSymbolVersion = VER_NDX_LOCAL;
+  expect("*");
+  expect(";");
+}
+
+void VersionScriptParser::parseExtern(std::vector<SymbolVersion> *Globals) {
+  expect("C++");
+  expect("{");
+
+  for (;;) {
+    if (peek() == "}" || Error)
+      break;
+    Globals->push_back({next(), true});
+    expect(";");
+  }
+
+  expect("}");
+  expect(";");
+}
+
+void VersionScriptParser::parseGlobal(StringRef VerStr) {
+  std::vector<SymbolVersion> *Globals;
+  if (VerStr.empty())
+    Globals = &Config->VersionScriptGlobals;
+  else
+    Globals = &Config->VersionDefinitions.back().Globals;
+
+  for (;;) {
+    if (skip("extern"))
+      parseExtern(Globals);
+
+    StringRef Cur = peek();
+    if (Cur == "}" || Cur == "local:" || Error)
+      return;
+    next();
+    Globals->push_back({Cur, false});
+    expect(";");
+  }
+}
+
+void VersionScriptParser::run() {
+  StringRef Msg = "anonymous version definition is used in "
+                  "combination with other version definitions";
+  if (skip("{")) {
+    parseVersion("");
+    if (!atEOF())
+      setError(Msg);
+    return;
+  }
+
+  while (!atEOF() && !Error) {
+    StringRef VerStr = next();
+    if (VerStr == "{") {
+      setError(Msg);
+      return;
+    }
+    expect("{");
+    parseVersion(VerStr);
+  }
+}
+
+void elf::parseVersionScript(MemoryBufferRef MB) {
+  VersionScriptParser(MB.getBuffer()).run();
+}
+
 template <class ELFT>
 static void addRegular(SymbolAssignment *Cmd) {
   Symbol *Sym = Symtab<ELFT>::X->addRegular(Cmd->Name, STB_GLOBAL, STV_DEFAULT);
   Sym->Visibility = Cmd->Hidden ? STV_HIDDEN : STV_DEFAULT;
   Cmd->Sym = Sym->body();
 }
 
 template <class ELFT> static void addSynthetic(SymbolAssignment *Cmd) {

ELF/SymbolListFile.h

-//===- SymbolListFile.h -----------------------------------------*- C++ -*-===//
-//
-//                             The LLVM Linker
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef LLD_ELF_SYMBOL_LIST_FILE_H
-#define LLD_ELF_SYMBOL_LIST_FILE_H
-
-#include "lld/Core/LLVM.h"
-#include "llvm/Support/MemoryBuffer.h"
-
-namespace lld {
-namespace elf {
-
-void parseDynamicList(MemoryBufferRef MB);
-void parseVersionScript(MemoryBufferRef MB);
-
-} // namespace elf
-} // namespace lld
-
-#endif

ELF/SymbolListFile.cpp

-//===- SymbolListFile.cpp -------------------------------------------------===//
-//
-//                             The LLVM Linker
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===----------------------------------------------------------------------===//
-//
-// This file contains the parser/evaluator of the linker script.
-// It does not construct an AST but consume linker script directives directly.
-// Results are written to Driver or Config object.
-//
-//===----------------------------------------------------------------------===//
-
-#include "SymbolListFile.h"
-#include "Config.h"
-#include "ScriptParser.h"
-#include "llvm/Support/MemoryBuffer.h"
-
-using namespace llvm;
-using namespace llvm::ELF;
-
-using namespace lld;
-using namespace lld::elf;
-
-// Parse the --dynamic-list argument.  A dynamic list is in the form
-//
-//  { symbol1; symbol2; [...]; symbolN };
-//
-// Multiple groups can be defined in the same file, and they are merged
-// into a single group.
-
-namespace {
-class DynamicListParser final : public ScriptParserBase {
-public:
-  DynamicListParser(StringRef S) : ScriptParserBase(S) {}
-  void run();
-};
-} // end anonymous namespace
-
-void DynamicListParser::run() {
-  while (!atEOF()) {
-    expect("{");
-    while (!Error) {
-      Config->DynamicList.push_back(next());
-      expect(";");
-      if (skip("}"))
-        break;
-    }
-    expect(";");
-  }
-}
-
-void elf::parseDynamicList(MemoryBufferRef MB) {
-  DynamicListParser(MB.getBuffer()).run();
-}
-
-// Parse the --version-script argument. We currently only accept the following
-// version script syntax:
-//
-//  { [ global: symbol1; symbol2; [...]; symbolN; ] local: *; };
-//
-// No wildcards are supported, other than for the local entry. Symbol versioning
-// is also not supported.
-
-namespace {
-class VersionScriptParser final : public ScriptParserBase {
-public:
-  VersionScriptParser(StringRef S) : ScriptParserBase(S) {}
-
-  void run();
-
-private:
-  void parseExtern(std::vector<SymbolVersion> *Globals);
-  void parseVersion(StringRef VerStr);
-  void parseGlobal(StringRef VerStr);
-  void parseLocal();
-};
-} // end anonymous namespace
-
-void VersionScriptParser::parseVersion(StringRef VerStr) {
-  // Identifiers start at 2 because 0 and 1 are reserved
-  // for VER_NDX_LOCAL and VER_NDX_GLOBAL constants.
-  size_t VersionId = Config->VersionDefinitions.size() + 2;
-  Config->VersionDefinitions.push_back({VerStr, VersionId});
-
-  if (skip("global:") || peek() != "local:")
-    parseGlobal(VerStr);
-  if (skip("local:"))
-    parseLocal();
-  expect("}");
-
-  // Each version may have a parent version. For example, "Ver2" defined as
-  // "Ver2 { global: foo; local: *; } Ver1;" has "Ver1" as a parent. This
-  // version hierarchy is, probably against your instinct, purely for human; the
-  // runtime doesn't care about them at all. In LLD, we simply skip the token.
-  if (!VerStr.empty() && peek() != ";")
-    next();
-  expect(";");
-}
-
-void VersionScriptParser::parseLocal() {
-  Config->DefaultSymbolVersion = VER_NDX_LOCAL;
-  expect("*");
-  expect(";");
-}
-
-void VersionScriptParser::parseExtern(std::vector<SymbolVersion> *Globals) {
-  expect("C++");
-  expect("{");
-
-  for (;;) {
-    if (peek() == "}" || Error)
-      break;
-    Globals->push_back({next(), true});
-    expect(";");
-  }
-
-  expect("}");
-  expect(";");
-}
-
-void VersionScriptParser::parseGlobal(StringRef VerStr) {
-  std::vector<SymbolVersion> *Globals;
-  if (VerStr.empty())
-    Globals = &Config->VersionScriptGlobals;
-  else
-    Globals = &Config->VersionDefinitions.back().Globals;
-
-  for (;;) {
-    if (skip("extern"))
-      parseExtern(Globals);
-
-    StringRef Cur = peek();
-    if (Cur == "}" || Cur == "local:" || Error)
-      return;
-    next();
-    Globals->push_back({Cur, false});
-    expect(";");
-  }
-}
-
-void VersionScriptParser::run() {
-  StringRef Msg = "anonymous version definition is used in "
-                  "combination with other version definitions";
-  if (skip("{")) {
-    parseVersion("");
-    if (!atEOF())
-      setError(Msg);
-    return;
-  }
-
-  while (!atEOF() && !Error) {
-    StringRef VerStr = next();
-    if (VerStr == "{") {
-      setError(Msg);
-      return;
-    }
-    expect("{");
-    parseVersion(VerStr);
-  }
-}
-
-void elf::parseVersionScript(MemoryBufferRef MB) {
-  VersionScriptParser(MB.getBuffer()).run();
-}

ELF/SymbolTable.cpp

 //
 //===----------------------------------------------------------------------===//
 
 #include "SymbolTable.h"
 #include "Config.h"
 #include "Error.h"
 #include "LinkerScript.h"
 #include "Strings.h"
-#include "SymbolListFile.h"
 #include "Symbols.h"
 #include "llvm/Bitcode/ReaderWriter.h"
 #include "llvm/Support/StringSaver.h"
 
 using namespace llvm;
 using namespace llvm::object;
 using namespace llvm::ELF;