[PATCH] D23774: [ELF] - Remove VersionScriptParser class and move the members to ScriptParser

[Rui Ueyama via llvm-commits]

ruiu added inline comments.

================
Comment at: ELF/LinkerScript.cpp:583-586
@@ -582,5 +582,6 @@
 public:
-  ScriptParser(StringRef S, bool B) : ScriptParserBase(S), IsUnderSysroot(B) {}
+  ScriptParser(StringRef S, bool B, bool V)
+      : ScriptParserBase(S), IsUnderSysroot(B), IsVersionScript(V) {}
 
   void run();
 
----------------
Instead of adding another parameter to the constructor, add another entry point function than `run`. Actually I'd rename `run` `parseLinkerScript` and add `parseVersionScript`.

================
Comment at: ELF/LinkerScript.cpp:626-629
@@ +625,6 @@
+  // For parsing version script.
+  void parseExtern(std::vector<SymbolVersion> *Globals);
+  void parseVersion(StringRef VerStr);
+  void parseGlobal(StringRef VerStr);
+  void parseLocal();
+
----------------
What's the difference between `read` prefix and `parse` prefix? If there's no reason, rename them to start with `read`.

================
Comment at: ELF/LinkerScript.cpp:1389
@@ +1388,3 @@
+//
+
+void elf::parseVersionScript(MemoryBufferRef MB) {
----------------
Remove this empty line.


https://reviews.llvm.org/D23774