Skip to content

Remove CanImportExprSyntax and provide API to interpret an ExprSyntax as a VersionTupleSyntax #2025

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

Conversation

StevenWong12
Copy link
Contributor

Resolves #1972

@StevenWong12 StevenWong12 requested a review from ahoppen as a code owner August 8, 2023 01:28
extension ExprSyntax {
public func asVersionTuple(with maxComponentCount: Int) -> VersionTupleSyntax? {
var parser = Parser(self.description)
let raw = parser.parseVersionTuple(maxComponentCount: maxComponentCount)
Copy link
Member

Choose a reason for hiding this comment

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

You should make sure that all tokens in the ExprSyntax are represented idk the VersionTupleSyntax, similar to what we do when creating syntax nodes from string literals

https://github.com/apple/swift-syntax/blob/e42bece52df2496d60b1b2a762fee9ffde7fc205/Sources/SwiftParser/generated/LayoutNodes%2BParsable.swift#L34

And now that I think about it, I think the best thing to do, would be add a parser entry function for VersionTupleSyntax so that all that code gets auto-generated, including the following, which is necessary for memory safety

https://github.com/apple/swift-syntax/blob/e42bece52df2496d60b1b2a762fee9ffde7fc205/Sources/SwiftParser/generated/LayoutNodes%2BParsable.swift#L23-L32


Could you add a test case for 2.0 + 1. + 1 should be unexpected nodes in that case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think making this a parser entry function is a good idea! And we have to pass the maxComponentCount parameter to that function so I add a helper function below.

@StevenWong12 StevenWong12 force-pushed the interpret_expr_syntax_as_version_tuple_syntax branch from ea1c73b to 53101dd Compare August 14, 2023 15:20
@StevenWong12 StevenWong12 force-pushed the interpret_expr_syntax_as_version_tuple_syntax branch from 53101dd to 2244549 Compare August 20, 2023 14:52
@StevenWong12 StevenWong12 force-pushed the interpret_expr_syntax_as_version_tuple_syntax branch 2 times, most recently from 3172161 to ce2f2f2 Compare August 30, 2023 15:38
@StevenWong12 StevenWong12 force-pushed the interpret_expr_syntax_as_version_tuple_syntax branch 2 times, most recently from 731504b to b0e01fd Compare August 31, 2023 01:03
@ahoppen
Copy link
Member

ahoppen commented Aug 31, 2023

@swift-ci Please test

@ahoppen ahoppen force-pushed the interpret_expr_syntax_as_version_tuple_syntax branch from b0e01fd to 490fe3d Compare March 15, 2024 17:41
@StevenWong12 StevenWong12 requested a review from bnbarham as a code owner March 15, 2024 17:41
…ression node

To make it easier to interpret the version passed to `canImport(MyModule, _version: <#version#>)`, add `ExprSyntax.interpretedAsVersionTuple` that takes eg. `1.2.3`, which got parsed as a member access `3` to the `1.2` float literal and re-parses it as a `VersionTupleSyntax`.

This relaxation also allows string literals as version parameters to `#if canImport`.

Fixes swiftlang#1972
rdar://121070235

Co-Authored-By: Ziyang Huang <[email protected]>
@ahoppen ahoppen force-pushed the interpret_expr_syntax_as_version_tuple_syntax branch from 490fe3d to 1382ffb Compare March 15, 2024 17:42
@ahoppen
Copy link
Member

ahoppen commented Mar 15, 2024

Picking this PR up.

@ahoppen
Copy link
Member

ahoppen commented Mar 15, 2024

@swift-ci Please test

bnbarham
bnbarham approved these changes Mar 15, 2024
@ahoppen ahoppen merged commit 45451a8 into swiftlang:main Mar 19, 2024
3 checks passed
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.

Remove CanImportExprSyntax and provide API to interpret an ExprSyntax as a VersionTupleSyntax
4 participants