Skip to content

Improve diagnostics for invalid version tuples #1809

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

TTOzzi
Copy link
Member

@TTOzzi TTOzzi commented Jun 18, 2023

Resolve #1613

During the process of rebasing the changes from the main branch, an issue arose, leading to the closure and reopening of the previous PR(#1717).
I sincerely apologize for the inconvenience caused.

Based on the feedback provided in the comment here(#1717 (comment)), I have made the necessary revisions and added several relevant test cases.

@TTOzzi TTOzzi requested a review from ahoppen as a code owner June 18, 2023 16:33
@TTOzzi TTOzzi force-pushed the remove-unexpected-node-between-argument-and-right-paren-in-attribute-syntax branch from 87f5797 to 317ead7 Compare June 18, 2023 17:14
@TTOzzi TTOzzi force-pushed the remove-unexpected-node-between-argument-and-right-paren-in-attribute-syntax branch from 317ead7 to 6cfad27 Compare June 18, 2023 17:40
@TTOzzi TTOzzi force-pushed the remove-unexpected-node-between-argument-and-right-paren-in-attribute-syntax branch from 6cfad27 to 74af3c1 Compare June 23, 2023 07:26
@TTOzzi TTOzzi force-pushed the remove-unexpected-node-between-argument-and-right-paren-in-attribute-syntax branch from 74af3c1 to ab2a4ad Compare June 23, 2023 07:47
@TTOzzi TTOzzi requested a review from ahoppen June 23, 2023 07:48
@TTOzzi TTOzzi force-pushed the remove-unexpected-node-between-argument-and-right-paren-in-attribute-syntax branch 2 times, most recently from f0cec9b to 6b8e867 Compare June 23, 2023 16:38
@TTOzzi TTOzzi requested a review from ahoppen June 23, 2023 16:39
ahoppen
ahoppen previously approved these changes Jun 26, 2023
@ahoppen ahoppen dismissed their stale review June 26, 2023 11:10

Accidentally clicked approve. I’ve had two more minor requests

@TTOzzi TTOzzi force-pushed the remove-unexpected-node-between-argument-and-right-paren-in-attribute-syntax branch from 6b8e867 to 178eccd Compare June 26, 2023 13:47
@TTOzzi TTOzzi requested a review from ahoppen June 26, 2023 13:54
Copy link
Member

@ahoppen ahoppen left a comment

Choose a reason for hiding this comment

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

One minor comment, otherwise LGTM.

Comment on lines 258 to 269
var unexpectedTokens: [RawSyntax] = []
var keepGoing: RawTokenSyntax? = nil
var loopProgress = LoopProgressCondition()
repeat {
if let keepGoing {
unexpectedTokens.append(RawSyntax(keepGoing))
}
if let unexpectedVersion = self.consume(if: .integerLiteral, .floatingLiteral, .identifier) {
unexpectedTokens.append(RawSyntax(unexpectedVersion))
}
keepGoing = self.consume(if: .period)
} while keepGoing != nil && loopProgress.evaluate(currentToken)
Copy link
Member

Choose a reason for hiding this comment

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

You could just make unexpectedTokens a [RawTokenSyntax], which simplifies the code a little

Suggested change
var unexpectedTokens: [RawSyntax] = []
var keepGoing: RawTokenSyntax? = nil
var loopProgress = LoopProgressCondition()
repeat {
if let keepGoing {
unexpectedTokens.append(RawSyntax(keepGoing))
}
if let unexpectedVersion = self.consume(if: .integerLiteral, .floatingLiteral, .identifier) {
unexpectedTokens.append(RawSyntax(unexpectedVersion))
}
keepGoing = self.consume(if: .period)
} while keepGoing != nil && loopProgress.evaluate(currentToken)
var unexpectedTokens: [RawTokenSyntax] = []
var keepGoing: RawTokenSyntax? = nil
var loopProgress = LoopProgressCondition()
repeat {
if let keepGoing {
unexpectedTokens.append(keepGoing)
}
if let unexpectedVersion = self.consume(if: .integerLiteral, .floatingLiteral, .identifier) {
unexpectedTokens.append(unexpectedVersion)
}
keepGoing = self.consume(if: .period)
} while keepGoing != nil && loopProgress.evaluate(currentToken)

Copy link
Member Author

Choose a reason for hiding this comment

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

I have committed your suggestion, thank you!

@TTOzzi TTOzzi force-pushed the remove-unexpected-node-between-argument-and-right-paren-in-attribute-syntax branch from 178eccd to 8ae11b1 Compare June 27, 2023 13:51
@TTOzzi TTOzzi requested a review from ahoppen June 27, 2023 13:52
Copy link
Member

@ahoppen ahoppen left a comment

Choose a reason for hiding this comment

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

Great, thanks. Let’s see what CI says.

@ahoppen
Copy link
Member

ahoppen commented Jun 27, 2023

@swift-ci Please test

@TTOzzi
Copy link
Member Author

TTOzzi commented Jul 12, 2023

Hello @ahoppen 👋 It seems like the CI is completed. When can it be merged?

@ahoppen
Copy link
Member

ahoppen commented Jul 13, 2023

Thanks for the ping. I forgot about this PR. I’m sorry.

@ahoppen ahoppen merged commit 0d813c3 into swiftlang:main Jul 13, 2023
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.

10e10 is not emitted as an invalid version tuple
2 participants