[overlay] magic-symbols-for-install-name conflicts with @_originallyDefinedIn #71864
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ian-twilightcoder
force-pushed
the
builtin-float-overlay-magic
branch
2 times, most recently
from
80eb4a9 to
09bbcba
Compare
…efinedIn The _Builtin_float interfaces were defined in $@rpath/lib/libswiftDarwin.dylib until macOS 10.14.4 and then in usr/lib/swift/libswiftDarwin.dylib after that. We were trying to use magic-symbols-for-install-name for the first one and @_originallyDefinedIn for the second one. However, @_originallyDefinedIn uses the @available introduced version as the first bound for $ld$previous$ which conflicts with the magic-symbols-for-install-name $ld$install_name$. Eventually @_originallyDefinedIn will need to support multiple install names, but until then just use it as-is and stop using magic-symbols-for-install-name. Add the missing @_originallyDefinedIn on FLT_RADIX, and match the watchOS/tvOS order between @_originallyDefinedIn and @available. There was no tvOS 7.0, the first one was 9.0. rdar://122351557
ian-twilightcoder
force-pushed
the
builtin-float-overlay-magic
branch
from
09bbcba to
be413e9
Compare
|
@swift-ci smoke test |
|
@swift-ci test |
edymtt
approved these changes
Feb 26, 2024
edymtt
reviewed
Feb 26, 2024
Comment on lines
+16
to
+17
| @available(macOS 10.9, iOS 7.0, watchOS 2.0, tvOS 9.0, *) | ||
| @_originallyDefinedIn(module: "Darwin", macOS 9999, iOS 9999, watchOS 9999, tvOS 9999) |
There was a problem hiding this comment.
Would it make sense to use the macros defined in utils/availability-macros.def?
Suggested change
| @available(macOS 10.9, iOS 7.0, watchOS 2.0, tvOS 9.0, *) | |
| @_originallyDefinedIn(module: "Darwin", macOS 9999, iOS 9999, watchOS 9999, tvOS 9999) | |
| @available(SwiftStdlib 5.0) | |
| @_originallyDefinedIn(module: "Darwin", SwiftStdlib 6.0) |
There was a problem hiding this comment.
I tried that, but they don't appear to be used for @_originallyDefinedIn
There was a problem hiding this comment.
SwiftStdlib 5.0 isn't the correct availability, that's long after macOS 10.9, iOS 7.0, watchOS 2.0, tvOS 9.0. There aren't any earlier availability macros to use either, and it didn't seem worth adding one.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The _Builtin_float interfaces were defined in $@rpath/lib/libswiftDarwin.dylib until macOS 10.14.4 and then in usr/lib/swift/libswiftDarwin.dylib after that. We were trying to use magic-symbols-for-install-name for the first one and @_originallyDefinedIn for the second one. However,
@_originallyDefinedInuses the@availableintroduced version as the first bound for $ld$previous$ which conflicts with the magic-symbols-for-install-name $ld$install_name$. Eventually@_originallyDefinedInwill need to support multiple install names, but until then just use it as-is and stop using magic-symbols-for-install-name.Add the missing
@_originallyDefinedInon FLT_RADIX, and match the watchOS/tvOS order between@_originallyDefinedInand@available.There was no tvOS 7.0, the first one was 9.0.
rdar://122351557