[PATCH] D81109: llvm-link: Add support for archive files as inputs.

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added inline comments.


================
Comment at: llvm/tools/llvm-link/llvm-link.cpp:143
 
+static std::unique_ptr<Module> loadArFile(const char *argv0,
+                                          const std::string ArchiveName,
----------------
jsjodin wrote:
> ABataev wrote:
> > `Argv0`. Also, better to use `StringRef`, if possible
> StringRef didn't simplify anything, so I kept the type, but fixed the name.
`StringRef` is not about simplification, this is common practice in LLVM to use it instead of `const char *`.


================
Comment at: llvm/tools/llvm-link/llvm-link.cpp:202-203
+      errs() << "Linking member '" << ChildName << "' of archive library.\n";
+    bool Err = L.linkModules(*Result, std::move(M), ApplicableFlags);
+    if (Err)
+      return nullptr;
----------------
Just
```
if (L.linkModules(*Result, std::move(M), ApplicableFlags))
  return nullptr;
```


================
Comment at: llvm/tools/llvm-link/llvm-link.cpp:207
+  } // end for each child
+  ExitOnErr(std::move(Err));
+  return Result;
----------------
Does it really worth using `std::move` here and in other places?


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

https://reviews.llvm.org/D81109