Lgtm, but define it as<div><br></div><div>Lld::<span style="font-family:Menlo,Consolas,Monaco,monospace;font-size:10px;line-height:16px;white-space:pre-wrap;background-color:rgb(208,255,208)">createSymbolVersion</span></div><div><font face="Menlo, Consolas, Monaco, monospace"><span style="font-size:10px;line-height:16px;white-space:pre-wrap"><br></span></font></div><div><font face="Menlo, Consolas, Monaco, monospace"><span style="font-size:10px;line-height:16px;white-space:pre-wrap">Cheers,</span></font></div><div><font face="Menlo, Consolas, Monaco, monospace"><span style="font-size:10px;line-height:16px;white-space:pre-wrap">Rafael <br></span></font><br>On Monday, 11 July 2016, George Rimar <<a href="mailto:grimar@accesssoftek.com">grimar@accesssoftek.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">grimar updated this revision to Diff 63484.<br>
grimar added a comment.<br>
<br>
After rebasing I had to modify that patch. I had to introduce createSymbolVersion() method,<br>
that creates new version entry in one place. So could this change be reviewed before I commit please ?<br>
<br>
<br>
<a href="http://reviews.llvm.org/D22086" target="_blank">http://reviews.llvm.org/D22086</a><br>
<br>
Files:<br>
  ELF/Config.h<br>
  ELF/SymbolListFile.cpp<br>
  ELF/SymbolTable.cpp<br>
<br>
Index: ELF/SymbolTable.cpp<br>
===================================================================<br>
--- ELF/SymbolTable.cpp<br>
+++ ELF/SymbolTable.cpp<br>
@@ -30,6 +30,8 @@<br>
 using namespace lld;<br>
 using namespace lld::elf;<br>
<br>
+size_t createSymbolVersion(StringRef Version);<br>
+<br>
 // All input object files must be for the same architecture<br>
 // (e.g. it does not make sense to link x86 object files with<br>
 // MIPS object files.) This function checks for that error.<br>
@@ -180,20 +182,18 @@<br>
   if (Default)<br>
     Version = Version.drop_front();<br>
<br>
-  size_t I = 2;<br>
-  for (elf::Version &V : Config->SymbolVersions) {<br>
+  for (elf::Version &V : Config->SymbolVersions)<br>
     if (V.Name == Version)<br>
-      return Default ? I : (I | VERSYM_HIDDEN);<br>
-    ++I;<br>
-  }<br>
+      return Default ? V.Id : (V.Id | VERSYM_HIDDEN);<br>
+<br>
<br>
   // If we are not building shared and version script<br>
   // is not specified, then it is not a error, it is<br>
   // in common not to use script for linking executables.<br>
   // In this case we just create new version.<br>
   if (!Config->Shared && !Config->HasVersionScript) {<br>
-    Config->SymbolVersions.push_back(elf::Version(Version));<br>
-    return Default ? I : (I | VERSYM_HIDDEN);<br>
+    size_t Id = createSymbolVersion(Version);<br>
+    return Default ? Id : (Id | VERSYM_HIDDEN);<br>
   }<br>
<br>
   error("symbol " + Name + " has undefined version " + Version);<br>
@@ -608,7 +608,7 @@<br>
       if (B->symbol()->VersionId != VER_NDX_GLOBAL &&<br>
           B->symbol()->VersionId != VER_NDX_LOCAL)<br>
         warning("duplicate symbol " + Name + " in version script");<br>
-      B->symbol()->VersionId = I + 2;<br>
+      B->symbol()->VersionId = V.Id;<br>
     }<br>
   }<br>
<br>
@@ -621,7 +621,7 @@<br>
       for (SymbolBody *B : findAll(Name))<br>
         if (B->symbol()->VersionId == VER_NDX_GLOBAL ||<br>
             B->symbol()->VersionId == VER_NDX_LOCAL)<br>
-          B->symbol()->VersionId = I + 2;<br>
+          B->symbol()->VersionId = V.Id;<br>
     }<br>
   }<br>
 }<br>
Index: ELF/SymbolListFile.cpp<br>
===================================================================<br>
--- ELF/SymbolListFile.cpp<br>
+++ ELF/SymbolListFile.cpp<br>
@@ -82,9 +82,17 @@<br>
   void parseVersionSymbols(StringRef Version);<br>
 };<br>
<br>
+size_t createSymbolVersion(StringRef Version) {<br>
+  // Identifiers start at 2 because 0 and 1 are reserved<br>
+  // for VER_NDX_LOCAL and VER_NDX_GLOBAL constants.<br>
+  size_t VersionId = Config->SymbolVersions.size() + 2;<br>
+  Config->SymbolVersions.push_back(elf::Version(Version, VersionId));<br>
+  return VersionId;<br>
+}<br>
+<br>
 void VersionScriptParser::parseVersion(StringRef Version) {<br>
   expect("{");<br>
-  Config->SymbolVersions.push_back(elf::Version(Version));<br>
+  createSymbolVersion(Version);<br>
   if (peek() == "global:") {<br>
     next();<br>
     parseVersionSymbols(Version);<br>
Index: ELF/Config.h<br>
===================================================================<br>
--- ELF/Config.h<br>
+++ ELF/Config.h<br>
@@ -38,8 +38,9 @@<br>
 // This struct contains symbols version definition that<br>
 // can be found in version script if it is used for link.<br>
 struct Version {<br>
-  Version(llvm::StringRef Name) : Name(Name) {}<br>
+  Version(llvm::StringRef Name, size_t Id) : Name(Name), Id(Id) {}<br>
   llvm::StringRef Name;<br>
+  size_t Id;<br>
   std::vector<llvm::StringRef> Globals;<br>
   size_t NameOff; // Offset in string table.<br>
 };<br>
<br>
<br>
</blockquote></div>