[PATCH] D44860: Refactor SharedFile::parseRest. NFC.

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Mar 24 06:06:59 PDT 2018


grimar accepted this revision.
grimar added a comment.
This revision is now accepted and ready to land.

This LGTM. Minor nits below.



================
Comment at: lld/ELF/InputFiles.cpp:847
+    uint64_t SecAlign = Sections[Sym.st_shndx].sh_addralign;
+    Ret = std::min(Ret, SecAlign);
+  }
----------------
I would probably inline `SecAlign`




================
Comment at: lld/ELF/InputFiles.cpp:857
+//
+// This function parse symbol versions. If an DSO has version information,
+// the file has a ".gnu.version_d" section which contains symbol version
----------------
I think it should be "**a** DSO"


================
Comment at: lld/ELF/InputFiles.cpp:913
+    // with explicit versions.
+    if (Idx != VER_NDX_GLOBAL) {
+      if (Idx >= Verdefs.size() || Idx == VER_NDX_LOCAL) {
----------------
Lets early continue?

```
 if (Idx == VER_NDX_GLOBAL)
   continue;
```


================
Comment at: lld/ELF/InputFiles.cpp:921
 
-    if (!Hidden)
-      Symtab->addShared(Name, *this, Sym, Alignment, VersymIndex);
-
-    // Also add the symbol with the versioned name to handle undefined symbols
-    // with explicit versions.
-    if (Ver) {
+      const Elf_Verdef *Ver = Verdefs[Idx];
       StringRef VerName = this->StringTable.data() + Ver->getAux()->vda_name;
----------------
I would inline `Ver`.


https://reviews.llvm.org/D44860





More information about the llvm-commits mailing list