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

[Rui Ueyama via llvm-commits]

ruiu added inline comments.


================
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");
----------------
grimar wrote:
> ruiu wrote:
> > Do you actually need this guard?
> I think so. We want to show this warning if assigning version to symbol that is already has one,
> (and switch to error one day https://llvm.org/bugs/show_bug.cgi?id=28342) 
> also we want to show warning if symbol is both in local and global list as it is obvious error.
> Actually I even thing we want to error out in latter case, that is how gold do afaik, planned to suggest that in a separate patch to keep this bit simpler for review.
I mean, why the previous condition `Sym->VersionId != Config->DefaultSymbolVersion` wouldn't work?


================
Comment at: ELF/SymbolTable.cpp:725
   // we iterate over the definitions in the reverse order.
+  auto assignFuzzyVersion = [&](SymbolVersion &Sym, size_t Version) {
+    if (!Sym.HasWildcards)
----------------
Why symbol version number is of `size_t`?


https://reviews.llvm.org/D26395