[PATCH] D21894: [ELF] - Fixed incorrect logic of version assignments when mixing wildcards with values matching.

[Rui Ueyama via llvm-commits]

ruiu added inline comments.

================
Comment at: ELF/Config.h:40
@@ +39,3 @@
+  llvm::StringRef Name;
+  bool HasWildcard;
+};
----------------
I think it's not worth to store this property as a member of this variable. You can always compute it from `Name` when needed.

================
Comment at: ELF/SymbolTable.cpp:565-566
@@ -563,1 +564,4 @@
 
+template <class ELFT>
+void SymbolTable<ELFT>::applySymbolVersion(VersionSymbol &Sym, size_t Version) {
+  std::vector<SymbolBody *> Syms = findAll(Sym.Name, !Sym.HasWildcard);
----------------
This function should probably be inlined to the place where you call it because non-wildcard patterns and wildcard patterns shares less code. For example, you don't need to call `findAll` for non-wildcards (but instead you want to call `find`). You don't want to issue a warning if non-wildcards. So this function is not actually common code for both cases.

================
Comment at: ELF/SymbolTable.cpp:628
@@ +627,3 @@
+
+  for (Version &V : Reverse(Config->SymbolVersions))
+    for (VersionSymbol &Sym : V.Globals)
----------------
`Reverse` is overkill. You can just iterate in the reverse order by this.

  for (size_t I = Config->SymbolVersions.size() - 1; I != 0; --I) {
    VersionSymbol &Sym = Config->SymbolVersions[I];
    ...
  }


http://reviews.llvm.org/D21894