Add a new test mutation strategy to flip the presence of token in the assertParse tests (fixes 5 parser bugs)
#1706
Conversation
|
@swift-ci Please test |
|
@swift-ci Please test macOS |
|
@swift-ci Please test Windows |
This uncovered two round-trip parsing failures which I’m fixing in this commit. rdar://109783066
Just make sure that we don’t crash the diagnostic generator with the mutated test cases.
ahoppen
force-pushed
the
ahoppen/flip-token-presence
branch
from
53db44a to
e9a8683
Compare
This uncovered a couple of implicit assumptions, mostly around the fact that tokens insided `UnexpectedNodesSyntax` are present, which isn’t true in general for manually generated trees. It also uncovered an issue where we weren’t able to retrieve the trivia pieces of a token after it had been modified using `with` because after the modification the token was a `parsedToken` that no longer resided in a `ParsingSyntaxArena`.
ahoppen
force-pushed
the
ahoppen/flip-token-presence
branch
from
e9a8683 to
e912ad4
Compare
|
@swift-ci Please test |
|
@swift-ci Please test Windows |
| @@ -333,7 +333,7 @@ extension Parser { | |||
| } | |||
|
|
|||
| mutating func parsePrimaryAssociatedTypes() -> RawPrimaryAssociatedTypeClauseSyntax { | |||
| let langle = self.consumeAnyToken(remapping: .leftAngle) | |||
| let langle = self.consumePrefix("<", as: .leftAngle) | |||
There was a problem hiding this comment.
Did you search for consumeAnyToken(.*leftAngle and rightAngle to make sure we don't have any more of these 😅?
There was a problem hiding this comment.
Yes, all other consumeAnyToken(remapping:) calls are fine.
| @@ -1920,7 +1933,7 @@ final class DeclarationTests: XCTestCase { | |||
| } | |||
| """, | |||
| substructure: Syntax( | |||
| MemberDeclListItemSyntax(decl: EditorPlaceholderDeclSyntax(identifier: .identifier("<#code#>"))) | |||
| MemberDeclListItemSyntax(decl: EditorPlaceholderDeclSyntax(placeholder: .identifier("<#code#>"))) | |||
There was a problem hiding this comment.
I really need to merge my refactoring PR. More conflicts :)!
| let nest = 20 | ||
| let nest = 10 |
There was a problem hiding this comment.
What was this one for? Do we hit the stack limit in debug at only 10 😓?
There was a problem hiding this comment.
I honestly don’t know what this test was meant to test. But I figured that there won’t really be any issues that we’d catch with 20 nested if statements that we wouldn’t catch if we nest 10.
And yes, this was hitting the maximum nesting level in debug builds. Not entirely sure why.
|
@swift-ci please test windows |
The idea is simple: From the tree generated during parsing, iterate over all the tokens. For each token create a new test case that has the token missing, if it was present in the original source or present if it was missing in the original source. Then run the parser to check that it round-trips and check that generating diagnostics for the mutated tree doesn’t crash.
This testing strategy found three parser bugs:
withbecause after the modification the token was aparsedTokenthat no longer resided in aParsingSyntaxArena. The fix is to search through an arena’schildRefsto find theParsingSyntaxArenathat created it. @rintaro can you review this change?ParseDiagnosticGenerator, mostly that tokens inUnexpectedNodesSyntaxare present. While this assumption is valid for trees generated bySwiftParser, it doesn’t need to hold for manually generated trees. A couple of minor adjustments to filter for only present tokens fixes this.rdar://109783066