[PATCH] D103821: [lld/mac] Add reexports after reexporter to inputFiles

[Nico Weber via Phabricator via llvm-commits]

thakis added a comment.

In D103821#2803575 <https://reviews.llvm.org/D103821#2803575>, @int3 wrote:

>> We already did most of that, we just added reexports before the reexporter.
>
> Hmm I thought we avoided adding re-exports to the `inputFiles` SetVector unless `isImplicitlyLinked()` returned true for them in `loadReexport()`. How does this change work with that?

I think it happens to work in practice since reexports in the SDK are also in the SDK and happen to share a prefix.

> Perhaps after D103430 <https://reviews.llvm.org/D103430> we should always insert re-exports into `inputFiles` and set the `explicitlyLinked` flag accordingly?

Maybe!

> As an aside, I would love for us to clean up the inputFile insertion / `printEachFile` logic, which AFAICT should always happen together for ObjFiles and DylibFiles. Right now some of them happen inside the ctors and some outside... I think we should be consistent about this. Can be done in a separate diff though (and I'm happy to do it myself).

The difference is that DylibFiles can trigger additional loads but ObjFiles can't (…or only via LC_LINK_COMMAND and it doesn't matter how those are ordered relative to the ObjFile, while DylibFile order matters). Making ObjFiles use the DylibFile approach sounds fine to me at first sight, but that's why DylibFiles currently are different.

> Aside 2: `isImplicitlyLinked` is a rather confusing name (though ld64 uses "implicit" in the same sense), given that this comment https://github.com/llvm/llvm-project/blob/main/lld/MachO/Writer.cpp#L688 uses implicit/explicit to mean the reverse. We should probably rename/rephrase either one...




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

https://reviews.llvm.org/D103821