Skip to content
New issue

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

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

Already on GitHub? # to your account

use feature-specific guard for @retroactive #2581

Merged
merged 1 commit into from
Nov 1, 2023

Conversation

rnro
Copy link
Contributor

@rnro rnro commented Nov 1, 2023

Motivation:

We should use the more granular guard for uses of @retroactive rather than coarse swift versions to guard against corner-cases

Modifications:

Switch #if compiler(>=5.11) for #if hasFeature(RetroactiveAttribute)

Result:

No change in most cases, more protected against corner-cases.

@rnro rnro requested a review from Lukasa November 1, 2023 09:13
@dnadoba
Copy link
Member

dnadoba commented Nov 1, 2023

hasFeature is only a thing since Swift 5.8

Motivation:

We should use the more granular guard for uses of @retroactive rather
than coarse swift versions to guard against corner-cases

Modifications:

Switch `#if compiler(>=5.11)` for `#if hasFeature(RetroactiveAttribute)`

Result:

No change in most cases, more protected against corner-cases.
@rnro rnro force-pushed the use_hasFeature_RetroactiveAttribute branch from 78c2fba to ffc2c1d Compare November 1, 2023 10:02
@rnro
Copy link
Contributor Author

rnro commented Nov 1, 2023

hasFeature is only a thing since Swift 5.8

I spotted that, thanks! I pushed a revised commit which should work

@rnro rnro enabled auto-merge (squash) November 1, 2023 10:08
@rnro
Copy link
Contributor Author

rnro commented Nov 1, 2023

For future reference, unfortunately the short-circuiting version doesn't work (on 5.7.3 at least)

#if swift(>=5.8) && hasFeature(RetroactiveAttribute)

@rnro rnro merged commit 853522d into apple:main Nov 1, 2023
@rnro rnro deleted the use_hasFeature_RetroactiveAttribute branch November 1, 2023 11:00
@rnro rnro added the 🔨 semver/patch No public API change. label Nov 1, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
🔨 semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants