Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add lines-of-code and show-trace features to new error format. #3773

merged 38 commits into from Jul 3, 2020


Copy link

bburdette commented Jul 1, 2020

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 line 1293.

Copy link

edolstra left a comment

show-trace is currently a Setting in settings, which is in libstore.

We could move show-trace to libutil. See ArchiveSettings in libutil/ as an example.

src/libexpr/parser.y Outdated Show resolved Hide resolved
bburdette added 2 commits Jul 2, 2020
Copy link
Contributor Author

bburdette commented Jul 2, 2020

We could move show-trace to libutil. See ArchiveSettings in libutil/ as an example.

Great - I moved show-trace to loggerSettings and ripped out the getter and setter code. Much cleaner.

bburdette added 2 commits Jul 2, 2020
@edolstra edolstra merged commit 14227ae into NixOS:master Jul 3, 2020
2 checks passed
2 checks passed
tests (ubuntu-latest)
tests (macos-latest)
@Infinisil Infinisil mentioned this pull request Jul 25, 2020
1 of 2 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.