[Diagnostics] Improve diagnostic when using == instead of = for defau… #26139
Conversation
…lt function argument.
|
cc @rintaro |
lib/Parse/ParsePattern.cpp
Outdated
| if (Tok.isBinaryOperator() && Tok.getText() == "==") { | ||
| diagnose(Tok, | ||
| diag::expected_assignment_instead_of_comparison_operator) | ||
| .fixItReplace(Tok.getLoc(), "="); |
There was a problem hiding this comment.
Is it possible for the compiler to check this earlier and recover by parsing the expression normally?
There was a problem hiding this comment.
The check could indeed be taken before l.272 for instance, but just to be sure I understand: in this case "recover" means that the token for '==' should be consumed so that the code can continue parsing the default value and so on?
There was a problem hiding this comment.
Yeah, I was thinking you'd essentially have the compiler just pretend that the user wrote =.
There was a problem hiding this comment.
Got it. Will try this.
There was a problem hiding this comment.
I think this check and recovery should be performed later, like l.385.
There was a problem hiding this comment.
but somehow I need to avoid the parsing logic to get into any of this branches, so a check would be needed before.
There was a problem hiding this comment.
It should work as expected if you consume == as if it's = and parse the default value after ==.
As well, parser recovers from usage of '==' and parses the whole parameter clause.
lib/Parse/ParsePattern.cpp
Outdated
| SyntaxParsingContext DefaultArgContext(P.SyntaxContext, | ||
| SyntaxKind::InitializerClause); | ||
| SourceLoc equalLoc = P.consumeToken(tok::equal); | ||
| SourceLoc equalLoc = P.consumeToken(assignmentTok); |
There was a problem hiding this comment.
I wonder if consumption of either '=' or '==' should be taken out of here and be performed on caller site?
There was a problem hiding this comment.
consumeToken also allows not passing a token kind at all, but I still think you might be right. It's weird to consume something when you don't know what it is.
There was a problem hiding this comment.
I'm OK with:
assert(Tok.is(tok::equal) ||
(Tok.isBinaryOperator() && Tok.getText() == "=="));
consumeToken();inside parseDefaultArgument(). Like
https://github.com/apple/swift/blob/26c4cccb4b971abae950d6a183354f70b2e3c0c7/lib/Parse/ParseDecl.cpp#L6632-L6634
There was a problem hiding this comment.
thanks for the feedback .. addressed in 2cfe8ad
|
@swift-ci Please smoke test |
|
@swift-ci Please smoke test Linux platform |
|
Thank you Victor! |
|
thank you both for the guidance. |
…lt function argument.
Resolves SR-11006.