[PATCH] D47565: Fix /WholeArchive bug.

[Reid Kleckner via Phabricator via llvm-commits]

rnk added inline comments.


================
Comment at: lld/COFF/Driver.cpp:1242
+    if (Optional<StringRef> Path = findFile(Arg->getValue()))
+      enqueuePath(*Path, Args.hasArg(OPT_wholearchive_flag) ||
+                             WholeArchive.count(Arg->getValue()));
----------------
ruiu wrote:
> rnk wrote:
> > I don't think `Args.hasArg(OPT_wholearchive_flag)` is the right check here, that asks if a whole archive flag appears anywhere on the command line, not if this particular flag is whole archive.
> This expression correctly checks for that condition, no?
I see, I didn't understand how OPT_wholearchive_flag vs. file worked.


================
Comment at: lld/COFF/Driver.cpp:134
+  for (StringRef Path : FilePaths) {
+    if (sys::fs::equivalent(Path, Filename)) {
+      warn(Filename + ": object specified more than once");
----------------
I think O(n**2) stats on input object files is going to be too much. Once we have the FD, you can do `fs::getUniqueId(FD)` to get the inode number, and then build a set of those. I guess if it's not a file MemoryBuffer, don't do the inode check. It probably comes from a whole archive.


================
Comment at: lld/COFF/Driver.cpp:1248-1249
+      return true;
+    for (auto *Arg : Args.filtered(OPT_wholearchive_file))
+      if (sys::fs::equivalent(Path, Arg->getValue()))
+        return true;
----------------
sys::fs::equivalent internally opens and closes a file handle for the purpose of doing a stat and comparing inodes. This will end up doing `O(inputs * wholearchiveinputs)` equivalency tests, and linkers have many input object files. Is there a way to avoid that?


https://reviews.llvm.org/D47565