TBDGen: Specify the correct macCatalyst platform ID in $ld$previous directives
#72109
Conversation
Increase type safety by consistently using the `LinkerPlatformId` enum type, instead of casting to/from `uint8_t` unnecessarily. NFC.
… directives. Resolves rdar://123491072
tshortli
force-pushed
the
maccatalyst-ldprevious
branch
from
4014795 to
f49dbb0
Compare
|
@swift-ci please test |
|
|
||
| // CHECK-ZIPPERED: $ld$previous$/System/Previous/iOS/ToasterKit.dylib$$2$10.2$13.0$_$s10ToasterKit5toastyyF$ | ||
| // CHECK-ZIPPERED: $ld$previous$/System/Previous/macOS/ToasterKit.dylib$$1$10.8$10.15$_$s10ToasterKit5toastyyF$ | ||
| // CHECK-MACCATALYST: D $ld$previous$/System/Previous/macCatalyst/ToasterKit.dylib$$6$10.2$13.0$_$s10ToasterKit5toastyyF$ |
There was a problem hiding this comment.
I'm pretty sure the versions are wrong here and they need to be the iOS versions mapped from the SDKSettings. We should also test that 10.15/13.1 is the earliest version. i.e. something available in 10.14 and moved in 12.0 should make an $ld$previous$ of 13.1$15.0. And something available in 10.14 and moved in 10.15 shouldn't make an $ld$previous$ for Mac Catalyst at all.
There was a problem hiding this comment.
10.2 and 13 come directly from the iOS availability of the declaration (see https://github.com/apple/swift/blob/main/test/TBD/Inputs/linker-directive.swift). So I don't think a remapping is needed. I can see about flooring the versions at 13.1 though.
There was a problem hiding this comment.
Actually, I'd like to separate clamping the values to minimum OS versions and potentially skipping emission of directives out into a separate radar/PR. I agree it's the right thing to do but it's also applicable to all platforms and it's unclear to me whether the current behavior is concretely causing any issues. I think it should be treated as a separate bug fix from a scheduling and risk perspective.
There was a problem hiding this comment.
Oh you're right, I saw "10.something" and got mixed up.
Resolves rdar://123491072