[PATCH] D37709: [LLD] [COFF] Add support for the -wholearchive option

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added inline comments.


================
Comment at: COFF/Driver.cpp:975
+      if (Optional<StringRef> Path = findFile(Arg->getValue()))
+        enqueuePath(*Path, WholeArchive);
+      break;
----------------
pcc wrote:
> ruiu wrote:
> > Replacing `WholeArchive` with `Args.hasArg(OPT_wholearchive_flag)` might improve readability.
> Wouldn't that create O(N^2) complexity in the size of the argument list though?
So it seems, based on reading `ArgList.h`. Will push with this changed back to a `bool WholeArchiveFlag` variable, to avoid the O(N^2) complexity.


================
Comment at: COFF/Driver.cpp:233
       if (Optional<StringRef> Path = findLib(Arg->getValue()))
-        enqueuePath(*Path);
+        enqueuePath(*Path, false);
       break;
----------------
mstorsjo wrote:
> pcc wrote:
> > Are you sure that `/wholearchive` has no effect on archives specified by linker directives?
> No, I can try to construct a test to figure out how it's supposed to behave.
I checked - link.exe doesn't apply the `-wholearchive` option to `-defaultlib` options, neither on the command line nor in directives. So I'll update the handling of the command line version of `-defaultlib` accordingly as well.


https://reviews.llvm.org/D37709