[PATCH] D60274: [ELF] Implement Dependent Libraries Feature

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added inline comments.


================
Comment at: lld/ELF/Config.h:124
       CallGraphProfile;
+  bool DepLibs;
   bool AllowMultipleDefinition;
----------------
Sort alphabetically.


================
Comment at: lld/ELF/InputFiles.cpp:406
+      Driver->addFile(*S, /*WithLOption=*/true);
+    else if (Optional<std::string> S = searchLibrary(Specifier))
+      Driver->addFile(*S, /*WithLOption=*/true);
----------------
`searchLibrary` contains the support for handling flags like `-l:foo`; I guess if you don't want that behaviour here then it will need to be factored out of `searchLibrary`.


================
Comment at: llvm/docs/Extensions.rst:291
+
+This section allows for embedding a set of strings specifing libraries to be
+added to the link by the linker.
----------------
*specifying


================
Comment at: llvm/include/llvm/Object/IRSymtab.h:138
   Word Version;
   enum { kCurrentVersion = 1 };
 
----------------
I think the version number needs to be incremented.


================
Comment at: llvm/include/llvm/Object/IRSymtab.h:157
+/// Dependent Library Specifiers
+  Range<DepLib> DepLibs;
 };
----------------
Maybe just `Range<Str>`?


================
Comment at: llvm/lib/LTO/LTOModule.cpp:653
+          getModule().getNamedMetadata("llvm.deplibs")) {
+    for (unsigned i = 0, e = DepLibs->getNumOperands(); i != e; ++i) {
+      MDNode *MDOptions = DepLibs->getOperand(i);
----------------
This could use a range for loop.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60274/new/

https://reviews.llvm.org/D60274