Track lines and columns in error messages#191
Merged
Conversation
This extends the CharReader to return CharSpans instead of just characters so that each returned character can carry the position where it originally occurred. This will be used by the lexer to compute token positions, and in turn deliver better error messages.
This modifies the lexer to wrap Tokens into TokenSpans, which carry contextual information for the token such as its position and length. This will be used for more accurate error reporting in the parser.
This is the first pass at modifying the parser to bubble up failing positions based on the token spans. The resulting error messages are better than before, but they are still not super-helpful (because e.g. they will complain about a WEND missing and point at EOF instead of at the original WHILE that was problematic). Improvements will come separately.
For error messages that highlight unbalanced block closures (ifs, fors, and whiles), point to the opening keyword instead of the location where the problem is detected (which may be too late).
Implement Display for Tokens and use that instead of the Debug output to generate error messages. These are more user-friendly.
Implement Display for Values and use that instead of the Debug output to generate error messages. These are more user-friendly.
This replaces the tuples that represent the various Statement enum entries in the AST with span structs. Other than offering better readability due to named fields, this adds room to track position information for better error diagnostics. Note that the builtin call arguments are left untouched, but these should be spans as well. I'll do that separately because of the large implications of this change, as it will impact every single builtin command.
Instead of passing the flat list of arguments to each Command being executed, pass the span that defines the command invocation. This will allow injecting details about where the invocation is for error purposes and extend the contents of the arguments to also carry location details.
This modifies the previously-introduced BuiltinCallSpan to carry each argument in its own ArgSpan. This will permit propagating positions for the individual arguments.
This does the same as we did before for statements, but now for expressions, by adding a new span for almost all expression types. As a first pass, this leaves literal expressions and symbol references untouched.
As a continuation of the previous, this finishes the addition of spans for expressions by adding them for literals and symbol references.
Instead of passing just the list of arguments to the functions, pass the FunctionCallSpan container to them so that they can get access to more information. This is the same that was done for builtin commands.
This allows having different Error/Result types with ease for both Value computations and Expr evaluations. The former will not need to carry a location but the latter will, so we need to start making them different.
This adds extra information to the multiple Statement spans to track the positions of the elements that aren't already annotated with one. Note that these positions track details about elements within the statement, not the position of the statement itself. I think this is fine, given that the later only seems necessary during parsing... but only time will tell.
ValueErrors do not carry position information, but the caller often has such details. By removing the automatic conversions into exec::Errors, we can catch all the call sites and fix them to annotate the errors.
This prunes more instances of debug dumps into error messages in favor of more readable text.
This concludes a stream of recent changes to add tracking for lines and columns to error messages and to clean up a bunch of other details in them. Fixes #113.
3333429 to
a0de865
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.