Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clean up @retroactive conformances #2719

Merged
merged 1 commit into from
May 9, 2024
Merged

Conversation

weissi
Copy link
Member

@weissi weissi commented May 9, 2024

Motivation:

  • #if swift is mostly wrong
  • too many #ifs that were misleading and unnecessary

Modifications:

  • Clean up with a single #if compiler(>=6.0)

Result:

  • Nicer code

proof 1: Only Swift 6 has @retroactive via check for #if $RetroactiveAttribute

$ for image in swift:5.8 swift:5.9 swift:5.10 swiftlang/swift:nightly-6.0-jammy; do echo -n "does $image have @retroactive? "; echo -e '#if $RetroactiveAttribute\nprint("yes")\n#else\nprint("no")\n#endif\n' | docker run -i --rm "$image" swift -; done
does swift:5.8 have @retroactive? no
does swift:5.9 have @retroactive? no
does swift:5.10 have @retroactive? no
does swiftlang/swift:nightly-6.0-jammy have @retroactive? yes

proof 2: Only Swift 6 has @retroactive via compiling something

$ for image in swift:5.8 swift:5.9 swift:5.10 swiftlang/swift:nightly-6.0-jammy; do echo -n "does $image have @retroactive? "; echo -e 'protocol P {}\nstruct S {}\n extension S: @retroactive P {}\n' | docker run -i --rm "$image" swift - 2>/dev/null && echo yes || echo no; done
does swift:5.8 have @retroactive? no
does swift:5.9 have @retroactive? no
does swift:5.10 have @retroactive? no
does swiftlang/swift:nightly-6.0-jammy have @retroactive? yes

See also #2718

Copy link
Member

@dnadoba dnadoba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Matches with the proposal.
IIRC, this was only necessary during the transition period.

@Lukasa Lukasa merged commit cd51717 into apple:main May 9, 2024
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants