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

[Jez Ng via Phabricator via llvm-commits]

int3 accepted this revision.
int3 added inline comments.
This revision is now accepted and ready to land.


================
Comment at: lld/MachO/Driver.cpp:332-333
       dylibFile->explicitlyLinked = isExplicit;
+      if (isNeeded)
+        dylibFile->forceNeeded = true;
       if (isWeak)
----------------
thakis wrote:
> 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.
ah I see. Makes sense now!


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

https://reviews.llvm.org/D103499