Add lines-of-code and show-trace features to new error format. #3773
Conversation
This file contains 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
edolstra
approved these changes
Jul 2, 2020
Great - I moved show-trace to loggerSettings and ripped out the getter and setter code. Much cleaner. |
2 tasks
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.
This PR brings in show-trace to the new error format, and adds lines of code too.
I renamed addPrefix in BaseError to addTrace, since it was always being used for trace anyway. addTrace takes an optional ErrPos and stores the traces in a list now.
The Pos struct gets a new 'origin' enum, which can be string, stdin, or file. if Pos.origin is stdin or string, then Pos.file points to the actual code with the error (stored in the symbol table). If Pos.origin is file, then Pos.file is the filename of the code with the error, as before.
At error print time, I get the code either from the symbol table for stdin or string, or by reading in the file. This is a little redundant in the case of files, since we just read those in for parsing - now we have to read them again for error reporting. But in the 'happy path' case its a good thing - we're not caching the nix file text for error reporting that may never happen.
Since the nix code is being read from the file or string, I removed the lines of code part from ErrorInfo. So now ErrorInfo only has errPos, and not nixCode. The downside here is if we wanted to 'hydrate' errors with the LOC and store them for later reference, that no longer is possible. But that's only a theoretical use case at this point, which we could restore when needed. For now it makes things simpler at the call site to have just errPos.
show-trace is currently a Setting in settings, which is in libstore. Since ErrorInfo and the loggers are in libutil they don't have access to this flag. So, I put a showTrace flag into the loggers, and I set that flag in handleExceptions, which is where traces were being printed before.
Some examples of lines-of-code and show-trace:
Simple LOC with highlighted error location:
Hash error followed by trace. Error is first, with call sites in reverse order, which is different from before.
addErrorContext doesn't include an errPos, just the context message.
'double' trace with some lambdas. See eval.cc line 1293.
