[lld] r287319 - Split ScriptParser::readVersionDeclaration.

[Rui Ueyama via llvm-commits]

Author: ruiu
Date: Fri Nov 18 00:30:09 2016
New Revision: 287319

URL: http://llvm.org/viewvc/llvm-project?rev=287319&view=rev
Log:
Split ScriptParser::readVersionDeclaration.

readVersionDeclaration was to read anonymous version definition and
named version definition. Splitting it into two functions should
improve readability as the two cases are different enough.

I also changed a few helper functions to return values instead of
mutating given references.

Modified:
    lld/trunk/ELF/LinkerScript.cpp

Modified: lld/trunk/ELF/LinkerScript.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/LinkerScript.cpp?rev=287319&r1=287318&r2=287319&view=diff
==============================================================================
--- lld/trunk/ELF/LinkerScript.cpp (original)
+++ lld/trunk/ELF/LinkerScript.cpp Fri Nov 18 00:30:09 2016
@@ -1004,11 +1004,10 @@ private:
   Expr readParenExpr();
 
   // For parsing version script.
-  void readVersionExtern(std::vector<SymbolVersion> *Globals);
+  std::vector<SymbolVersion> readVersionExtern();
+  void readAnonymousDeclaration();
   void readVersionDeclaration(StringRef VerStr);
-  void readGlobal(StringRef VerStr);
-  void readLocal(StringRef VerStr);
-  void readSymbols(std::vector<SymbolVersion> &V);
+  std::vector<SymbolVersion> readSymbols();
 
   ScriptConfiguration &Opt = *ScriptConfig;
   bool IsUnderSysroot;
@@ -1022,7 +1021,7 @@ void ScriptParser::readVersionScript() {
 
 void ScriptParser::readVersionScriptCommand() {
   if (consume("{")) {
-    readVersionDeclaration("");
+    readAnonymousDeclaration();
     return;
   }
 
@@ -1812,73 +1811,93 @@ unsigned ScriptParser::readPhdrType() {
   return Ret;
 }
 
+// Reads a list of symbols, e.g. "{ global: foo; bar; local: *; };".
+void ScriptParser::readAnonymousDeclaration() {
+  // Read global symbols first. "global:" is default, so if there's
+  // no label, we assume global symbols.
+  if (consume("global:") || peek() != "local:")
+    Config->VersionScriptGlobals = readSymbols();
+
+  // Next, read local symbols.
+  if (consume("local:")) {
+    if (consume("*")) {
+      Config->DefaultSymbolVersion = VER_NDX_LOCAL;
+      expect(";");
+    } else {
+      setError("local symbol list for anonymous version is not supported");
+    }
+  }
+  expect("}");
+  expect(";");
+}
+
+// Reads a list of symbols, e.g. "VerStr { global: foo; bar; local: *; };".
 void ScriptParser::readVersionDeclaration(StringRef VerStr) {
   // Identifiers start at 2 because 0 and 1 are reserved
   // for VER_NDX_LOCAL and VER_NDX_GLOBAL constants.
   uint16_t VersionId = Config->VersionDefinitions.size() + 2;
   Config->VersionDefinitions.push_back({VerStr, VersionId});
 
+  // Read global symbols.
   if (consume("global:") || peek() != "local:")
-    readGlobal(VerStr);
-  if (consume("local:"))
-    readLocal(VerStr);
+    Config->VersionDefinitions.back().Globals = readSymbols();
+
+  // Read local symbols.
+  if (consume("local:")) {
+    if (consume("*")) {
+      Config->DefaultSymbolVersion = VER_NDX_LOCAL;
+      expect(";");
+    } else {
+      for (SymbolVersion V : readSymbols())
+        Config->VersionScriptLocals.push_back(V);
+    }
+  }
   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() != ";")
+  // 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 hint; the runtime doesn't care about it
+  // at all. In LLD, we simply ignore it.
+  if (peek() != ";")
     skip();
   expect(";");
 }
 
-void ScriptParser::readSymbols(std::vector<SymbolVersion> &V) {
+// Reads a list of symbols for a versions cript.
+std::vector<SymbolVersion> ScriptParser::readSymbols() {
+  std::vector<SymbolVersion> Ret;
   for (;;) {
     if (consume("extern"))
-      readVersionExtern(&V);
+      for (SymbolVersion V : readVersionExtern())
+        Ret.push_back(V);
 
     if (peek() == "}" || peek() == "local:" || Error)
-      return;
+      break;
     StringRef Tok = next();
-    V.push_back({unquote(Tok), false, hasWildcard(Tok)});
+    Ret.push_back({unquote(Tok), false, hasWildcard(Tok)});
     expect(";");
   }
+  return Ret;
 }
 
-void ScriptParser::readLocal(StringRef VerStr) {
-  if (consume("*")) {
-    Config->DefaultSymbolVersion = VER_NDX_LOCAL;
-    expect(";");
-    return;
-  }
-
-  if (VerStr.empty())
-    setError("locals list for anonymous version is not supported");
-
-  readSymbols(Config->VersionScriptLocals);
-}
-
-void ScriptParser::readVersionExtern(std::vector<SymbolVersion> *V) {
+// Reads an "extern C++" directive, e.g.,
+// "extern "C++" { ns::*; "f(int, double)"; };"
+std::vector<SymbolVersion> ScriptParser::readVersionExtern() {
   expect("\"C++\"");
   expect("{");
 
+  std::vector<SymbolVersion> Ret;
   while (!Error && peek() != "}") {
     StringRef Tok = next();
     bool HasWildcard = !Tok.startswith("\"") && hasWildcard(Tok);
-    V->push_back({unquote(Tok), true, HasWildcard});
+    Ret.push_back({unquote(Tok), true, HasWildcard});
     expect(";");
   }
 
   expect("}");
   expect(";");
-}
-
-void ScriptParser::readGlobal(StringRef VerStr) {
-  if (VerStr.empty())
-    readSymbols(Config->VersionScriptGlobals);
-  else
-    readSymbols(Config->VersionDefinitions.back().Globals);
+  return Ret;
 }
 
 static bool isUnderSysroot(StringRef Path) {