[PATCH] D26395: [ELF] - Add support for locals list in version script.

[Rui Ueyama via llvm-commits]

ruiu added inline comments.


================
Comment at: ELF/LinkerScript.cpp:1807
+  if (VerStr.empty())
+    error("locals list for anonymous version is not supported");
+
----------------
Use setError.


================
Comment at: ELF/LinkerScript.cpp:1810
+  std::vector<SymbolVersion> &Locals = Config->VersionDefinitions.back().Locals;
+  for (;;) {
+    StringRef Cur = peek();
----------------
Please take a look at other places. We usually write a loop this way to consume tokens until "}".

  while (!Error && !consume("}")) {
    ...
  }


================
Comment at: ELF/SymbolTable.cpp:580-581
   Symbol *Sym = Body->symbol();
-  if (Sym->VersionId != Config->DefaultSymbolVersion)
+  auto It = Proccessed.insert(Body);
+  if (!It.second)
     warn("duplicate symbol " + Name + " in version script");
----------------
Do you actually need this guard?


================
Comment at: ELF/SymbolTable.cpp:697
+        continue;
+      setVersionId(find(Sym.Name), V.Name, Sym.Name, VER_NDX_LOCAL, Proccessed);
     }
----------------
You want to care about the case in which IsExternCpp is true.


https://reviews.llvm.org/D26395