Reduce syntax error noise by swallowing dedents like indents - #27170
Conversation
AlexWaygood
added
diagnostics
| { | ||
| // For this invalid `Indent`, ensure the matching `Dedent` gets consumed as well | ||
| unexpected_indents += 1; | ||
| } |
There was a problem hiding this comment.
It's a bit late here, so I don't have a great suggestion. But it feels a bit off to put this specific statement recovery logic into the generic recovery flow
There was a problem hiding this comment.
If nothing else, this is at least giving some ideas of what to dig into to further understand this code.
The best I've come up with so far for keeping the statement-specific logic in the statement code is we treat an indent as a list element for block statements and then have the parse statement deal with the indent and recover on the matching dedent. I've not yet checked to see what error cases this seemingly-innocent lie might cause.
Memory usage reportMemory usage unchanged ✅ |
|
|
|
The syntax errors are a huge improvement. Thank you |
|
While I know this isn't the prettiest, I'm going to go ahead and merge. Sometimes good error recovery is messy and sometimes making things more general / cleaner can increase the complexity. It might be best to be surgical for this change for now and see how it evolves and refactor in the future. However, if people have ideas before that future date, I'd be up for trying them out. |
3 participants
Summary
The error recover code swallows an indent but then acts on the dedent, making things imbalanced, causing a cascade of more failures.
Some of these failures are confusing with
^ Syntax Error: Expected a statementin a location that doesn't make sense. #27033 shifts the rendering of that error location, making it make even less sense. Instead, we can just not create these spurious extra errors which this PR aims to handle, at least for some cases.Test Plan
Added a few more indentation cases