Skip to content

VersionTupleSyntax should be parsed as (up to) 3 integer literals #1434

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.

Sign up for GitHub

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

Jump to bottom
Closed
bnbarham opened this issue Mar 23, 2023 · 5 comments · Fixed by #1440
Closed

VersionTupleSyntax should be parsed as (up to) 3 integer literals #1434

bnbarham opened this issue Mar 23, 2023 · 5 comments · Fixed by #1440
Labels
bug Something isn't working SwiftParser Bugs in the (new) Parser written in Swift

Comments

@bnbarham
Copy link
Contributor

Issue Kind

Other

Source Code

@available(macOS 10.4p2.1, *)

Description

The given source should produce an error, rather than a valid parse with 10.4.p2 as the majorMinor and 1 as the patch on VersionTupleSyntax. Ideally VersionTupleSyntax would contain major, minor, and patch.
@bnbarham bnbarham added bug Something isn't working SwiftParser Bugs in the (new) Parser written in Swift labels Mar 23, 2023
@rintaro
Copy link
Member

rintaro commented Mar 23, 2023 •
edited
Loading

The structure of VersionTupleSyntax should be like:

    children: [
      Child(name: "Major",
            kind: .token(choices: [.token(tokenKind: "IntegerToken")])),
      Child(name: "PeriodBeforeMinor",
            kind: .token(choices: [.token(tokenKind: "PeriodToken")]),
            isOptional: true),
      Child(name: "Minor",
            kind: .token(choices: [.token(tokenKind: "IntegerToken")]),
            isOptional: true),
      Child(name: "PeriodBeforePatch",
            kind: .token(choices: [.token(tokenKind: "PeriodToken")]),
            isOptional: true),
      Child(name: "Patch",
            kind: .token(choices: [.token(tokenKind: "IntegerToken")]),
            isOptional: true),
    ]

And the parser should reject integer literals with non-decimal characters e.g. 0x01

@grynspan
Copy link
Contributor

I imagine "1." or "1.2." should also be rejected, so it's more like (I, .I?, .I?) than (I, .?, I?, .?, I?), right?

@ahoppen
Copy link
Member

ahoppen commented Mar 23, 2023

rdar://107152155

@rintaro
Copy link
Member

rintaro commented Mar 23, 2023 •
edited
Loading

I imagine "1." or "1.2." should also be rejected, so it's more like (I, .I?, .I?) than (I, .?, I?, .?, I?), right?

Right, or more like (I, (.I, (.I)?)?). The parser will diagnose those malformed things, either by consuming it as an unexpected token, or leave it and let the "outer" parser do things.
Unfortunately, we don't have a way to declare grouped optional without introducing a nominal syntax type, the declaration will be flatten.

@grynspan
Copy link
Contributor

Gotcha. Okay, works for me! Thanks for taking a look at this.

ahoppen added a commit to ahoppen/swift-syntax that referenced this issue Mar 24, 2023
@ahoppen
Parse versions as three integer tokens
018d981 
Previously, internals from the lexer leaked through by representing the major and minor part of the version as a float literal. Also, we accepted non-decimal numbers (eg. hex) for version number, which we don’t want.

Fixes swiftlang#1434
rdar://107152155
ahoppen added a commit to ahoppen/swift-syntax that referenced this issue Mar 24, 2023
@ahoppen
Parse versions as three integer tokens
abd7eeb 
Previously, internals from the lexer leaked through by representing the major and minor part of the version as a float literal. Also, we accepted non-decimal numbers (eg. hex) for version number, which we don’t want.

Fixes swiftlang#1434
rdar://107152155
@ahoppen ahoppen mentioned this issue Mar 24, 2023
Merged
ahoppen added a commit to ahoppen/swift-syntax that referenced this issue Mar 24, 2023
@ahoppen
Parse versions as three integer tokens
cbf27d5 
Previously, internals from the lexer leaked through by representing the major and minor part of the version as a float literal. Also, we accepted non-decimal numbers (eg. hex) for version number, which we don’t want.

Fixes swiftlang#1434
rdar://107152155
ahoppen added a commit to ahoppen/swift-syntax that referenced this issue Mar 24, 2023
@ahoppen
Parse versions as three integer tokens
0a57b4e 
Previously, internals from the lexer leaked through by representing the major and minor part of the version as a float literal. Also, we accepted non-decimal numbers (eg. hex) for version number, which we don’t want.

Fixes swiftlang#1434
rdar://107152155
ahoppen added a commit to ahoppen/swift-syntax that referenced this issue Mar 24, 2023
@ahoppen
Parse versions as three integer tokens
10ef365 
Previously, internals from the lexer leaked through by representing the major and minor part of the version as a float literal. Also, we accepted non-decimal numbers (eg. hex) for version number, which we don’t want.

Fixes swiftlang#1434
rdar://107152155
ahoppen added a commit to ahoppen/swift-syntax that referenced this issue Mar 27, 2023
@ahoppen
Parse versions as three integer tokens
47a8dcd 
Previously, internals from the lexer leaked through by representing the major and minor part of the version as a float literal. Also, we accepted non-decimal numbers (eg. hex) for version number, which we don’t want.

Fixes swiftlang#1434
rdar://107152155
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working SwiftParser Bugs in the (new) Parser written in Swift
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Parse versions as three integer tokens ahoppen/swift-syntax
4 participants
@rintaro @bnbarham @ahoppen @grynspan