Skip to content

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