Skip to content

Conversation

Parzival-3141
Copy link
Contributor

@Parzival-3141 Parzival-3141 commented May 2, 2023

Closes #14961

// RunStep.zig
fn make(step: *Step, prog_node: *std.Progress.Node) !void {
    try std.testing.expect(false);
    // ...

Before:

C:\zigsrc\zig\build\stage4\bin\zig.exe build run
run compiler-testing: error: TestUnexpectedResult

After:

C:\zigsrc\zig\build\stage4\bin\zig.exe build run
run compiler-testing: error: step failed with error TestUnexpectedResult:
C:\zigsrc\zig\lib\std\testing.zig:509:14: 0x7ff6bcec10b7 in expect (build.exe.obj)
    if (!ok) return error.TestUnexpectedResult;
             ^
C:\zigsrc\zig\lib\std\Build\RunStep.zig:372:5: 0x7ff6bce9799d in make (build.exe.obj)
    try std.testing.expect(false);
    ^

If the return trace is too long and fills the internal buffer, we print the partial trace and a message notifying the user.

// RunStep.zig
fn fib(n: u32) !u32 {
    if (n <= 1) return n;
    if (n == 21) return error.Fibbage;
    return try fib(n - 1) + try fib(n - 2);
}

fn make(step: *Step, prog_node: *std.Progress.Node) !void {
    _ = try fib(55);
   // ...
C:\zigsrc\zig\build\stage4\bin\zig.exe build run
run compiler-testing: error: step failed with error Fibbage:
C:\zigsrc\zig\lib\std\Build\RunStep.zig:373:18: 0x7ff6298a1110 in fib (build.exe.obj)
    if (n == 21) return error.Fibbage;
                 ^
C:\zigsrc\zig\lib\std\Build\RunStep.zig:374:12: 0x7ff6298a118b in fib (build.exe.obj)
    return try fib(n - 1) + try fib(n - 2);
           ^
C:\zigsrc\zig\lib\std\Build\RunStep.zig:374:12: 0x7ff6298a118b in fib (build.exe.obj)
    return try fib(n - 1) + try fib(n - 2);
           ^
C:\zigsrc\zig\lib\std\Build\RunStep.zig:374:12: 0x7ff6298a118b in fib (build.exe.obj)
    return try fib(n - 1) + try fib(n - 2);
           ^
// etc....
C:\zigsrc\zig\lib\std\Build\RunStep.zig:374:12: 0x7ff6298a118b in fib (build.exe.obj)
    return try fib(n - 1) + try fib(n - 2);
           ^
C:\zigsrc\zig\lib.... errorReturnTrace exceeds buffer size!

I'm assuming 4096 chars is more than enough for most people, and I call Allocator.resize() to trim unused space.

@Parzival-3141
Copy link
Contributor Author

Not really sure why the aarch64-linux-release test failed. Seems like some other commits failed in the same way.

Copy link
Member

@andrewrk andrewrk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this.

It's a nice idea, but it serializes too early. Instead, it should capture the trace and store that data, serializing it later when errors are printed to the terminal.

Unlike the previous commit, this ensures that the error message and trace are
printed together.
@Parzival-3141
Copy link
Contributor Author

I'm guessing this could be implemented through the ErrorBundle field, but I'm not sure how to achieve that and could use some guidance. The current patch works as intended though and can be merged if sufficient.

@Parzival-3141 Parzival-3141 requested a review from andrewrk August 27, 2024 04:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

collect error return traces from errors other than error.MakeFailed

2 participants