Skip to content

Specify SwiftSyntax601 in more places. #3034

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

Merged
merged 5 commits into from
Apr 4, 2025

Conversation

mbrandonw
Copy link
Contributor

Currently the trick of using version modules to conditionally access Swift syntax is broken for version 601:

#if canImport(SwiftSyntax601)
  // Code in here is never compiled
#endif

We use these version modules heavily to support SwiftSyntax all the way back to 509, but now multiple packages in our ecosystem are broken with no way to fix them other than rolling back all SwiftSyntax support back to 600.

I believe the fix is to simply update the dependencies of SwiftSyntax to use SwiftSyntax601, but I went ahead and fixed up other inconsistencies with version modules throughout the repo. If this is the correct fix then hopefully we can get a patch release of SwiftSyntax released soon?

@bnbarham
Copy link
Contributor

bnbarham commented Apr 1, 2025

@swift-ci please test

@bnbarham
Copy link
Contributor

bnbarham commented Apr 1, 2025

Thanks @mbrandonw

If this is the correct fix then hopefully we can get a patch release of SwiftSyntax released soon?

Yes, would you be able to cherry-pick this to release/6.1 🙏?

@bnbarham
Copy link
Contributor

bnbarham commented Apr 1, 2025

Also CC @keith for the bazel changes, but they look fine to me.

@mbrandonw
Copy link
Contributor Author

Yes, would you be able to cherry-pick this to release/6.1 🙏?

Yep can do. Just so that I understand, you want me to make a new PR with these changes that targets release/6.1?

@keith
Copy link
Member

keith commented Apr 1, 2025

Yep bazel changes look good! Thanks for making them!

@bnbarham
Copy link
Contributor

bnbarham commented Apr 1, 2025

Yep can do. Just so that I understand, you want me to make a new PR with these changes that targets release/6.1?

Yep 🙇‍♂️!

@mbrandonw
Copy link
Contributor Author

@bnbarham Done here #3036.

@bnbarham
Copy link
Contributor

bnbarham commented Apr 3, 2025

@swift-ci please test

@bnbarham
Copy link
Contributor

bnbarham commented Apr 4, 2025

@swift-ci please test Windows platform

1 similar comment
@bnbarham
Copy link
Contributor

bnbarham commented Apr 4, 2025

@swift-ci please test Windows platform

@bnbarham bnbarham merged commit 820501e into swiftlang:main Apr 4, 2025
24 checks passed
@bnbarham
Copy link
Contributor

bnbarham commented Apr 4, 2025

Thanks again @mbrandonw!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants