[PATCH] D21681: [ELF] - Implemented support of default/non-default symbols versions

[George Rimar via llvm-commits]

grimar added inline comments.

================
Comment at: ELF/SymbolTable.cpp:166
@@ -165,1 +165,3 @@
 
+static void setupVersionAttributes(Symbol *Sym, StringRef Name) {
+  size_t VersionBegin = Name.find('@');
----------------
ruiu wrote:
> grimar wrote:
> > ruiu wrote:
> > > You can make this function side-effect-free.
> > > 
> > >   static uint16_t getVersionId(Symbol *Sym, StringRef Name)
> > > 
> > > which returns a version ID. Then use it in insert() like this.
> > > 
> > >   Sym->VersionId = getVersionId(Sym, Name);
> > I can, but what about VersionedName flag ?
> > I need to set it too, I think the only way to do that without second search of '@' would be:
> > 
> > ```
> > Sym->VersionId = getVersionId(Sym, Name); //search is inside
> > Sym->VersionedName = Sym->VersionId != VER_NDX_LOCAL && 
> >                                             Sym->VersionId != VER_NDX_GLOBAL;
> > ```
> > 
> > Looks ok ?
> Well, if you can compute `VersionedName` from `VersionId`,  you don't need to store it to a symbol. Instead, you can define `bool isVersioned()`.
No, generally I can't compute it. 
Only in this place it is possible to set this flag, because getVersionId() can return version > than VER_NDX_GLOBAL only if
version was extracted from symbol name, so it is versioned name. Later, scanVersionScript() will assign
versions to regular symbols and it will not be possible to compute if name is versioned or not. 
So I need this flag I think.


http://reviews.llvm.org/D21681