stage2: Include catch expressions in error return trace#12990
stage2: Include catch expressions in error return trace#12990topolarity wants to merge 1 commit intoziglang:masterfrom
catch expressions in error return trace#12990Conversation
52dd669 to
c489b3a
Compare
| if (builtin.zig_backend != .stage1) | ||
| trace.index -= 1; // exclude this error-handling block |
There was a problem hiding this comment.
In the example you gave, the extra line pointing at catch is nice. However, the introduction of this code makes me think this change is not worth it.
There was a problem hiding this comment.
Fair enough
My main argument for the change is that it can be confusing to understand the "U-turn" taken by an error return trace when handling an error. It also may not be terribly obvious to users that the "parent" error trace is included intentionally in the first place
However, go ahead and close this if you think this isn't the best approach 🙂 We can always circle back later anyway
There was a problem hiding this comment.
We can also continue the discussion - maybe we can come up with a way to gain this enhancement without a perceived complexity increase in the language
There was a problem hiding this comment.
Great, let's keep brainstorming
The main complexity increase I see is for folks writing panic handlers / error trace printers who want to hide the function doing the dumping from the error trace. Is that the increase that jumps out to you?
If so, we might be able to sweep some of that complexity under the rug by adding std.debug.dumpErrorTrace or similar
c489b3a to
a6d40b3
Compare
a6d40b3 to
64e687b
Compare
This makes it easier to track errors that flow through a catch block:
```zig
fn foo() !void { return error.BigIssue; }
fn bar() !void { return error.AnotherProblem; }
fn baz() !void {
return bar();
}
test {
foo() catch {
try baz();
};
}
```
shows:
```console
./test_err9.zig:1:18: 0x2117c8 in foo (test)
fn foo() !void { return error.BigIssue; }
^
./test_err9.zig:7:11: 0x211918 in test_0 (test)
foo() catch {
^
./test_err9.zig:2:18: 0x2118e8 in bar (test)
fn bar() !void { return error.AnotherProblem; }
^
./test_err9.zig:4:5: 0x2118ce in baz (test)
return bar();
^
./test_err9.zig:8:9: 0x211934 in test_0 (test)
try baz();
^
```
It's now transparent that an error arrived in `test_0` and then was
handled unsuccessfully.
The same change was made for `else |err|` blocks.
64e687b to
ea493f8
Compare
|
Closing abandoned PR - tests were never passing and there is a discussion topic that should be moved to an appropriate issue. |
Follow-up to #12837. Wanted to separate this one out since it's a change to UX.
The idea is to make it easier to track errors that flow through a catch block:
This trace makes it clear that an error arrived in
test_0and then was handled unsuccessfully.Any
if (foo()) |non_err| { ... } else |err| { ... }expressions are also included in the return trace.Dependent on #12837 , only the last commit is specific to this PR.