Switch to using clang as linker on darwin #1111
Conversation
|
@swift-ci please test |
|
This isn't done, but I just want to see how CI feels about it vs my local test |
keith
force-pushed
the
ks/switch-to-using-clang-as-linker-on-darwin
branch
from
7a877c0 to
7c5160d
Compare
|
@swift-ci please test |
|
@swift-ci Please test source compatibility |
|
There's no source-compat testing on this repo, it needs to be requested from |
|
thanks, doing on swiftlang/swift#59414 |
keith
force-pushed
the
ks/switch-to-using-clang-as-linker-on-darwin
branch
from
7c5160d to
b7bf20c
Compare
|
Thanks for doing this! I had mentioned to @artemcm that I would like to see this done but wanted to ensure that Apple was okay with the change. Unifying the behaviour and relying on clang to drive the linker is definitely going to help simplify the code base, but also help the C++ interop functionality as well IMO. |
I haven't consulted with anyone on this so I would definitely be interested to hear what folks think about this |
|
@swift-ci please test |
|
source compatibility on swiftlang/swift#59414 looks good, realistically im not entirely sure how we'll be 100% confident here though |
|
@artemcm can you take a look? 🙏🏻 |
|
@swift-ci please test |
This allows us to remove lots of duplicated logic from the clang driver.
keith
force-pushed
the
ks/switch-to-using-clang-as-linker-on-darwin
branch
from
b7bf20c to
7d910ed
Compare
|
awesome thanks! sorry i just rebased, will kick off ci again |
|
@swift-ci please test |
|
if you hear about any definitely cc me, i wouldn't be surprised if there is something i missed but i think fixes should be straight forward if so! |
| commandLine.appendPath(VirtualPath.lookup(sdkPath)) | ||
| } | ||
|
|
||
| commandLine.appendFlags( | ||
| "-fobjc-link-runtime", |
There was a problem hiding this comment.
@keith, what was the reason for always adding this flag?
It appears it does not match previous behavior and causes clang to always import Foundation, whether it is needed or not.
There was a problem hiding this comment.
Hrm, I don't recall since it has been a while, but I agree I don't see where it clearly came from. I checked clang as well in case I was trying to mirror that and don't see it there either. I suppose I could have pulled it from a -v invocation of what this was doing before, but if that flag is present there I don't see where it's coming from.
Note that it does look like https://github.com/apple/llvm-project/blob/c10fcd3e640b3736a7b2442f00db8feca674648c/clang/lib/Driver/ToolChains/Darwin.cpp#L745-L746 relies on this being passed, so maybe it was required for that case?
There was a problem hiding this comment.
This really should be gated on -enable-objc-interop I think. -lSystem is also rather odd. This should no longer be needed as we now rely on clang as the driver which will add -lSystem as appropriate.
This allows us to remove lots of duplicated logic from the clang driver.