[PATCH] D21018: [ELF] - Basic versioned symbols support implemented.

[Rui Ueyama via llvm-commits]

ruiu added a comment.

Here is my first round of review.


================
Comment at: ELF/InputFiles.cpp:343
@@ -341,1 +342,3 @@
 
+static StringRef filterVersion(StringRef Name) {
+  StringRef Version;
----------------
Why do you need this?

================
Comment at: ELF/SymbolListFile.cpp:84
@@ -79,2 +83,3 @@
 
 void VersionScriptParser::run() {
+  bool HasAnonymousTag = false;
----------------
A version definition without a version name is not actually a version definition in the sense that they controls only visibility and have nothing to do with symbol versioning. I think it is better to handle that case completely differently from symbol versioning.

================
Comment at: ELF/SymbolListFile.cpp:121
@@ +120,3 @@
+    if (peek() != ";")
+      V.Parent = next();
+    expect(";");
----------------
Since you are not using this member at all, please remove Parent member from the struct and just discard the next token here.

================
Comment at: ELF/SymbolListFile.h:23-27
@@ -21,1 +22,7 @@
 
+struct Version {
+  StringRef Name;
+  StringRef Parent;
+  std::vector<llvm::StringRef> Globals;
+};
+
----------------
Move this definition to Config.h and make version definitions a member of Config class.


http://reviews.llvm.org/D21018