[PATCH] D103499: [lld/mac] Implement -needed_framework, -needed_library, -needed-l

[Nico Weber via Phabricator via llvm-commits]

thakis added a comment.

Thanks!



================
Comment at: lld/MachO/Driver.cpp:332-333
       dylibFile->explicitlyLinked = isExplicit;
+      if (isNeeded)
+        dylibFile->forceNeeded = true;
       if (isWeak)
----------------
int3 wrote:
> why not just `dylibFile->forceNeeded = isNeeded`? (actually same question for forceWeakImport below... and I see we don't do that if check for `explicitlyLinked` above). I believe `addFile()` returns a new instance each time, so unconditionally setting these values should be fine
I did it this way for consistency with forceWeakImport, but now that I think about it I think it's needed for correctness:

> I believe addFile() returns a new instance each time, so unconditionally setting these values should be fine

That's not true: It always reads the file, but it then calls addDylib() which uses the dylib cache. That's important too: `-needed-lFoo -reexport-lFoo` marks libFoo.dylib as both needed and reexported. (…I guess I should add a test for this.)

Good point about the explicitilyLinked: I think that's wrong. If you link a dylib both explicitly (say, via an -l flag) and implicitly via LC_LINKER_COMMAND, I think the latter runs later and overwrites explicitlyLinked with false. I'll make a CL for that.


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

https://reviews.llvm.org/D103499