[ELF] - Minor refactor of LinkerScript file.

[grimar]

Summary

Removes lots of else-ifs from ScriptParser::run();

[grimar]

Removed all changes except one because:

• Moving allocator inside ScriptParser was not good idea. Its much more clear to know it's lifetime is attached to LinkerScript class and not to parser. It did not work correctly I think.
• Restored checks of Error flag where removed. That also was incorrect change, thanks to Rui Ueyama for noticing that during review of another patch.

[• rafael]

This is similar to how MC parser is done, so it is fine with me.

Rui, what do you think?

ELF/LinkerScript.cpp

122

Any particular reason for this order?

143

C is only used inside the if. Move the find call there?

[• rafael]

ELF/LinkerScript.cpp

143

Actually. The if is on a repeated call to find. So just use

auto C = Cmd.find(Tok);
if (C != Cmd.end())

C->second(*this);

[grimar]

ELF/LinkerScript.cpp

122

I think we do that in other places. Flags with flags etc.

143

Yep, that what I wanted initially.

[ruiu]

LGTM with a few nits.

ELF/LinkerScript.cpp

92

Remove this function and move the code inside the constructor.

143

Please rename C -> It (because it is not a command but an iterator.)

144–145

Please assign to a local variable to make the actual type explicit.

if (It != Cmd.end()) {
  std::function<void(ScriptParser &)> &Handler = It->second;
  Handler();
} else {
  setError(...);
}

ELF/LinkerScript.cpp

 
 class elf2::ScriptParser {
 public:
   ScriptParser(BumpPtrAllocator *A, StringRef S, bool B)
       : Saver(*A), Tokens(tokenize(S)), IsUnderSysroot(B) {}
   void run();
 
 private:
+  void init();
+
   void setError(const Twine &Msg);
   static std::vector<StringRef> tokenize(StringRef S);
   static StringRef skipSpace(StringRef S);
   bool atEOF();
   StringRef next();
   bool skip(StringRef Tok);
   void expect(StringRef Expect);
 
private:
   void readOutputFormat();
   void readSearchDir();
   void readSections();
 
   void readOutputSectionDescription();
 
   StringSaver Saver;
   std::vector<StringRef> Tokens;
-  bool Error = false;
+  llvm::StringMap<std::function<void(ScriptParser &)>> Cmd;
   size_t Pos = 0;
   bool IsUnderSysroot;
+  bool Error = false;
 };
 
+void ScriptParser::init() {
+  Cmd["ENTRY"] = &ScriptParser::readEntry;
+  Cmd["EXTERN"] = &ScriptParser::readExtern;
+  Cmd["GROUP"] = &ScriptParser::readGroup;
+  Cmd["INCLUDE"] = &ScriptParser::readInclude;
+  Cmd["INPUT"] = &ScriptParser::readGroup;
+  Cmd["OUTPUT"] = &ScriptParser::readOutput;
+  Cmd["OUTPUT_ARCH"] = &ScriptParser::readOutputArch;
+  Cmd["OUTPUT_FORMAT"] = &ScriptParser::readOutputFormat;
+  Cmd["SEARCH_DIR"] = &ScriptParser::readSearchDir;
+  Cmd["SECTIONS"] = &ScriptParser::readSections;
+  Cmd[";"] = [](ScriptParser &) {};
+}
+
 void ScriptParser::run() {
+  init();
   while (!atEOF()) {
     StringRef Tok = next();
-    if (Tok == ";")
-      continue;
-    if (Tok == "ENTRY") {
-      readEntry();
-    } else if (Tok == "EXTERN") {
-      readExtern();
-    } else if (Tok == "GROUP" || Tok == "INPUT") {
-      readGroup();
-    } else if (Tok == "INCLUDE") {
-      readInclude();
-    } else if (Tok == "OUTPUT") {
-      readOutput();
-    } else if (Tok == "OUTPUT_ARCH") {
-      readOutputArch();
-    } else if (Tok == "OUTPUT_FORMAT") {
-      readOutputFormat();
-    } else if (Tok == "SEARCH_DIR") {
-      readSearchDir();
-    } else if (Tok == "SECTIONS") {
-      readSections();
-    } else {
+    auto C = Cmd.find(Tok);
+    if (Cmd.find(Tok) != Cmd.end())
+      C->second(*this);
+    else
       setError("unknown directive: " + Tok);
-      return;
-    }
   }
 }
 
 // We don't want to record cascading errors. Keep only the first one.
 void ScriptParser::setError(const Twine &Msg) {
   if (Error)
     return;
   error(Msg);