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

when handling a returned error, if the expression/block does not try/return error, reset error return trace index to 0 #1923

Closed
Sahnvour opened this issue Feb 6, 2019 · 17 comments · Fixed by #12837
Labels
accepted This proposal is planned. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Milestone

Comments

@Sahnvour
Copy link
Contributor

Sahnvour commented Feb 6, 2019

When trying to run the first example code from #1813 with an executable that can't be found, the error trace is weird. It looks like the return address array is filled with addresses from windowsCreateProcess.
I believe this is due to the loop in spawnWindows that tries every directory from PATH and returns a lot of errors because the file doesn't exist.

As for why it is sometimes PATH_NOT_FOUND and others FILE_NOT_FOUND, I assume my PATH contains some old directories that don't exist anymore.

Example output:

$ zig run childprocess.zig
error: Unexpected
D:\DEV\Install\zig\lib\zig\std\os\child_process.zig:693:45: 0x7ff663ac4a97 in windowsCreateProcess (run.obj)
            windows.ERROR.PATH_NOT_FOUND => return error.FileNotFound,
                                            ^
D:\DEV\Install\zig\lib\zig\std\os\child_process.zig:692:45: 0x7ff663ac4a7a in windowsCreateProcess (run.obj)
            windows.ERROR.FILE_NOT_FOUND => return error.FileNotFound,
                                            ^
D:\DEV\Install\zig\lib\zig\std\os\child_process.zig:693:45: 0x7ff663ac4a97 in windowsCreateProcess (run.obj)
            windows.ERROR.PATH_NOT_FOUND => return error.FileNotFound,
                                            ^
D:\DEV\Install\zig\lib\zig\std\os\child_process.zig:693:45: 0x7ff663ac4a97 in windowsCreateProcess (run.obj)
            windows.ERROR.PATH_NOT_FOUND => return error.FileNotFound,
                                            ^
D:\DEV\Install\zig\lib\zig\std\os\child_process.zig:692:45: 0x7ff663ac4a7a in windowsCreateProcess (run.obj)
            windows.ERROR.FILE_NOT_FOUND => return error.FileNotFound,
                                            ^
D:\DEV\Install\zig\lib\zig\std\os\child_process.zig:692:45: 0x7ff663ac4a7a in windowsCreateProcess (run.obj)
            windows.ERROR.FILE_NOT_FOUND => return error.FileNotFound,
                                            ^
D:\DEV\Install\zig\lib\zig\std\os\child_process.zig:692:45: 0x7ff663ac4a7a in windowsCreateProcess (run.obj)
            windows.ERROR.FILE_NOT_FOUND => return error.FileNotFound,
                                            ^
D:\DEV\Install\zig\lib\zig\std\os\child_process.zig:692:45: 0x7ff663ac4a7a in windowsCreateProcess (run.obj)
            windows.ERROR.FILE_NOT_FOUND => return error.FileNotFound,
                                            ^
D:\DEV\Install\zig\lib\zig\std\os\child_process.zig:693:45: 0x7ff663ac4a97 in windowsCreateProcess (run.obj)
            windows.ERROR.PATH_NOT_FOUND => return error.FileNotFound,
                                            ^
D:\DEV\Install\zig\lib\zig\std\os\child_process.zig:693:45: 0x7ff663ac4a97 in windowsCreateProcess (run.obj)
            windows.ERROR.PATH_NOT_FOUND => return error.FileNotFound,
                                            ^
D:\DEV\Install\zig\lib\zig\std\os\child_process.zig:692:45: 0x7ff663ac4a7a in windowsCreateProcess (run.obj)
            windows.ERROR.FILE_NOT_FOUND => return error.FileNotFound,
                                            ^
D:\DEV\Install\zig\lib\zig\std\os\child_process.zig:693:45: 0x7ff663ac4a97 in windowsCreateProcess (run.obj)
            windows.ERROR.PATH_NOT_FOUND => return error.FileNotFound,
                                            ^
D:\DEV\Install\zig\lib\zig\std\os\child_process.zig:693:45: 0x7ff663ac4a97 in windowsCreateProcess (run.obj)
            windows.ERROR.PATH_NOT_FOUND => return error.FileNotFound,
                                            ^
D:\DEV\Install\zig\lib\zig\std\os\child_process.zig:693:45: 0x7ff663ac4a97 in windowsCreateProcess (run.obj)
            windows.ERROR.PATH_NOT_FOUND => return error.FileNotFound,
                                            ^
D:\DEV\Install\zig\lib\zig\std\os\child_process.zig:692:45: 0x7ff663ac4a7a in windowsCreateProcess (run.obj)
            windows.ERROR.FILE_NOT_FOUND => return error.FileNotFound,
                                            ^
D:\DEV\Install\zig\lib\zig\std\os\child_process.zig:692:45: 0x7ff663ac4a7a in windowsCreateProcess (run.obj)
            windows.ERROR.FILE_NOT_FOUND => return error.FileNotFound,
                                            ^
D:\DEV\Install\zig\lib\zig\std\os\child_process.zig:692:45: 0x7ff663ac4a7a in windowsCreateProcess (run.obj)
            windows.ERROR.FILE_NOT_FOUND => return error.FileNotFound,
                                            ^
D:\DEV\Install\zig\lib\zig\std\os\child_process.zig:693:45: 0x7ff663ac4a97 in windowsCreateProcess (run.obj)
            windows.ERROR.PATH_NOT_FOUND => return error.FileNotFound,
                                            ^
D:\DEV\Install\zig\lib\zig\std\os\child_process.zig:692:45: 0x7ff663ac4a7a in windowsCreateProcess (run.obj)
            windows.ERROR.FILE_NOT_FOUND => return error.FileNotFound,
                                            ^
D:\DEV\Install\zig\lib\zig\std\os\child_process.zig:693:45: 0x7ff663ac4a97 in windowsCreateProcess (run.obj)
            windows.ERROR.PATH_NOT_FOUND => return error.FileNotFound,
                                            ^
D:\DEV\Install\zig\lib\zig\std\os\child_process.zig:692:45: 0x7ff663ac4a7a in windowsCreateProcess (run.obj)
            windows.ERROR.FILE_NOT_FOUND => return error.FileNotFound,
                                            ^
D:\DEV\Install\zig\lib\zig\std\os\child_process.zig:692:45: 0x7ff663ac4a7a in windowsCreateProcess (run.obj)
            windows.ERROR.FILE_NOT_FOUND => return error.FileNotFound,
                                            ^
D:\DEV\Install\zig\lib\zig\std\os\child_process.zig:692:45: 0x7ff663ac4a7a in windowsCreateProcess (run.obj)
            windows.ERROR.FILE_NOT_FOUND => return error.FileNotFound,
                                            ^
D:\DEV\Install\zig\lib\zig\std\os\index.zig:2253:5: 0x7ff663aa3442 in unexpectedErrorWindows (run.obj)
    return error.Unexpected;
    ^
D:\DEV\Install\zig\lib\zig\std\os\child_process.zig:696:21: 0x7ff663ac4a5d in windowsCreateProcess (run.obj)
            else => return os.unexpectedErrorWindows(err),
                    ^
D:\DEV\Install\zig\lib\zig\std\os\child_process.zig:611:21: 0x7ff663ac34d6 in ChildProcess::ChildProcess_spawnWindows (run.obj)
                    return err;
                    ^
D:\DEV\Install\zig\lib\zig\std\os\child_process.zig:127:13: 0x7ff663ac09ce in ChildProcess::ChildProcess_spawn (run.obj)
            return self.spawnWindows();
            ^
D:\DEV\Install\zig\lib\zig\std\os\child_process.zig:209:9: 0x7ff663ac01bb in ChildProcess::ChildProcess_exec (run.obj)
        try child.spawn();
        ^
D:\DEV\Zig\error-traces\childprocess.zig:5:18: 0x7ff663abff2a in main (run.obj)
  const result = try std.os.ChildProcess.exec(
                 ^
D:\DEV\Install\zig\lib\zig\std\os\child_process.zig:692:45: 0x7ff663ac4a7a in windowsCreateProcess (run.obj)
            windows.ERROR.FILE_NOT_FOUND => return error.FileNotFound,
@andrewrk andrewrk added this to the 0.4.0 milestone Feb 6, 2019
@andrewrk andrewrk added the bug Observed behavior contradicts documented or intended behavior label Feb 6, 2019
@andrewrk
Copy link
Member

andrewrk commented Feb 6, 2019

I think there are 2 components to this issue:

  • bug: the stack trace printer is printing the error return trace in backwards (or otherwise incorrect) order
  • enhancement: "caught" errors still show up in errror return traces. This is a little tricky because we can definitely change the behavior but it has performance implications to consider.

@Sahnvour
Copy link
Contributor Author

Sahnvour commented Feb 7, 2019

catch and if else don't decrement stack_trace.index at the moment ?

@andrewrk
Copy link
Member

andrewrk commented Feb 7, 2019

That's correct - and intentional. The example in the docs here shows how this is useful: https://ziglang.org/documentation/master/#Error-Return-Traces

If we wanted to avoid giving this up, what we could do is "reset" the error return trace when a function returns a non-error, by setting the index to 0. This would solve the problem, but require an extra memory store for every return in functions which return error unions. Maybe that is worth it.

@Sahnvour
Copy link
Contributor Author

Sahnvour commented Feb 7, 2019

I think that would be insufficient, in my example as the executable is never found, it would still keep all of windowsCreateProcess's errors since it always returned an error, but the first line we want to see in the error trace is

} else {
// Every other error would have been returned earlier.
return error.FileNotFound;
}

How about this : in a catch or else (handling a returned error), if the block does not try or return error.xyz, set the index to 0 ?

@andrewrk
Copy link
Member

andrewrk commented Feb 8, 2019

How about this : in a catch or else (handling a returned error), if the block does not try or return error.xyz, set the index to 0 ?

I like this idea a lot! And I think it will be straightforward to implement too. 💯 from me

@andrewrk
Copy link
Member

andrewrk commented Feb 8, 2019

BTW I believe this is a duplicate of #1810

@andrewrk
Copy link
Member

Closing as a duplicate of #1810 but with the note that @Sahnvour's proposed fix in #1923 (comment) is planned.

@andrewrk
Copy link
Member

I don't know why I thought this was a duplicate of #1810. The issues are entirely unrelated.

@andrewrk andrewrk reopened this Feb 27, 2019
@andrewrk andrewrk changed the title Unusually long error stack trace when handling a returned error, if the expression/block does not try/return error, reset error return trace index to 0 Feb 27, 2019
@andrewrk andrewrk added proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. accepted This proposal is planned. and removed bug Observed behavior contradicts documented or intended behavior labels Feb 27, 2019
@andrewrk andrewrk modified the milestones: 0.4.0, 0.5.0 Apr 4, 2019
@andrewrk andrewrk modified the milestones: 0.5.0, 0.6.0 Sep 20, 2019
@andrewrk andrewrk modified the milestones: 0.6.0, 0.7.0 Feb 10, 2020
@FireFox317
Copy link
Contributor

FireFox317 commented Oct 2, 2020

I'm implementing this (have working code that resets the index), but it's not clear to me where to put that code in the catch/else block. There are a few options:

  1. If no try/error then place the reset index code at the beginning of a block
    However this breaks the error return trace printing in start.zig:
           loop.init() catch |err| {
                std.log.err("{}", .{@errorName(err)});
                if (@errorReturnTrace()) |trace| {
                    std.debug.dumpStackTrace(trace.*);
                }
                return 1;
            };

To solve that we might want to check for a @errorReturnTrace in the block too?

  1. If no try/error then place the code at the end of a block
    In that case a catch could return early (still success) but then the index is not reset to zero.

  2. Put it before any return in a catch/else block that does not return an error.

I'm not sure what would be the best solution.

@Sahnvour
Copy link
Contributor Author

Sahnvour commented Oct 3, 2020

I think we could emit a single control flow block to do the reset to 0 (it's only a constant write to a static variable right ?), and have all returns and such jump to it. It shouldn't need any stack or registers so that's fine ? However I don't know if that is possible as is with LLVM.

If not, I would go for 3. and put a redundant block before any return/break/end of zig block. The optimizer should be able to get away with it, and at least that is a simple implementation we can still reevaluate in the future if need be.

@andrewrk andrewrk removed this from the 0.7.0 milestone Oct 13, 2020
kristoff-it pushed a commit that referenced this issue Aug 23, 2022
This reverts commit 1a32f2a.

Sorry, this workaround is not welcome. Instead, please solve the actual
issue by doing the accepted behavior in the compiler itself:

> in a catch or else (handling a returned error), if the block does not
> try or return error.xyz, set the index to 0

This also applies to if statements, such as the one that test runner is
doing just above this hack.
topolarity added a commit to topolarity/zig that referenced this issue Sep 14, 2022
This implement trace "popping" for correctly handled errors within
`catch { ... }` and `else { ... }` blocks.

When breaking from these blocks with any non-error, we pop the error
trace frames corresponding to the operand. When breaking with an error,
we preserve the frames so that error traces "chain" together as usual.

```zig
fn foo(cond1: bool, cond2: bool) !void {
    bar() catch {
    	if (cond1) {
	    // If baz() result is a non-error, pop the error trace frames from bar()
	    // If baz() result is an error, leave the bar() frames on the error trace
            return baz();
	} else if (cond2) {
	    // If we break/return an error, then leave the error frames from bar() on the error trace
	    return error.Foo;
	}
    };

    // An error returned from here does not include bar()'s error frames in the trace
    return error.Bar;
}
```

Notice that if foo() does not return an error it, it leaves no extra
frames on the error trace.

This is piece (1/3) of ziglang#1923 (comment)
topolarity added a commit to topolarity/zig that referenced this issue Sep 14, 2022
This allows for errors to be "re-thrown" by yielding any error as the
result of a catch block. For example:

```zig
fn errorable() !void {
    return error.FallingOutOfPlane;
}

fn foo(have_parachute: bool) !void {
    return errorable() catch |err| b: {
        if (have_parachute) {
            // error trace will include the call to errorable()
            break :b error.NoParachute;
        } else {
            return;
        }
    };
}

pub fn main() !void {
    // Anything that returns a non-error does not pollute the error trace.
    try foo(true);

    // This error trace will still include errorable(), whose error was "re-thrown" by foo()
    try foo(false);
}
```

This is piece (2/3) of ziglang#1923 (comment)
topolarity added a commit to topolarity/zig that referenced this issue Sep 14, 2022
In order to enforce a strict stack discipline for error return traces,
we cannot track error return traces that are stored in variables:

  ```zig
  const x = errorable(); // errorable()'s error return trace is killed here

  // v-- error trace starts here instead
  return x catch error.UnknownError;
  ```

In order to propagate error return traces, function calls need to be passed
directly to an error-handling expression (`if`, `catch`, `try` or `return`):

  ```zig
  // When passed directly to `catch`, the return trace is propagated
  return errorable() catch error.UnknownError;

  // Using a break also works
  return blk: {
      // code here
      break :blk errorable();
  } catch error.UnknownError;
  ```

Why do we need this restriction? Without it, multiple errors can co-exist
with their own error traces. Handling that situation correctly means either:
  a. Dynamically allocating trace memory and tracking lifetimes, OR
  b. Allowing the production of one error to interfere with the trace of another
     (which is the current status quo)

This is piece (3/3) of ziglang#1923 (comment)
topolarity added a commit to topolarity/zig that referenced this issue Sep 14, 2022
This allows for errors to be "re-thrown" by yielding any error as the
result of a catch block. For example:

```zig
fn errorable() !void {
    return error.FallingOutOfPlane;
}

fn foo(have_parachute: bool) !void {
    return errorable() catch |err| b: {
        if (have_parachute) {
            // error trace will include the call to errorable()
            break :b error.NoParachute;
        } else {
            return;
        }
    };
}

pub fn main() !void {
    // Anything that returns a non-error does not pollute the error trace.
    try foo(true);

    // This error trace will still include errorable(), whose error was "re-thrown" by foo()
    try foo(false);
}
```

This is piece (2/3) of ziglang#1923 (comment)
topolarity added a commit to topolarity/zig that referenced this issue Sep 14, 2022
In order to enforce a strict stack discipline for error return traces,
we cannot track error return traces that are stored in variables:

  ```zig
  const x = errorable(); // errorable()'s error return trace is killed here

  // v-- error trace starts here instead
  return x catch error.UnknownError;
  ```

In order to propagate error return traces, function calls need to be passed
directly to an error-handling expression (`if`, `catch`, `try` or `return`):

  ```zig
  // When passed directly to `catch`, the return trace is propagated
  return errorable() catch error.UnknownError;

  // Using a break also works
  return blk: {
      // code here
      break :blk errorable();
  } catch error.UnknownError;
  ```

Why do we need this restriction? Without it, multiple errors can co-exist
with their own error traces. Handling that situation correctly means either:
  a. Dynamically allocating trace memory and tracking lifetimes, OR
  b. Allowing the production of one error to interfere with the trace of another
     (which is the current status quo)

This is piece (3/3) of ziglang#1923 (comment)
topolarity added a commit to topolarity/zig that referenced this issue Sep 14, 2022
This allows for errors to be "re-thrown" by yielding any error as the
result of a catch block. For example:

```zig
fn errorable() !void {
    return error.FallingOutOfPlane;
}

fn foo(have_parachute: bool) !void {
    return errorable() catch |err| b: {
        if (have_parachute) {
            // error trace will include the call to errorable()
            break :b error.NoParachute;
        } else {
            return;
        }
    };
}

pub fn main() !void {
    // Anything that returns a non-error does not pollute the error trace.
    try foo(true);

    // This error trace will still include errorable(), whose error was "re-thrown" by foo()
    try foo(false);
}
```

This is piece (2/3) of ziglang#1923 (comment)
topolarity added a commit to topolarity/zig that referenced this issue Sep 14, 2022
In order to enforce a strict stack discipline for error return traces,
we cannot track error return traces that are stored in variables:

  ```zig
  const x = errorable(); // errorable()'s error return trace is killed here

  // v-- error trace starts here instead
  return x catch error.UnknownError;
  ```

In order to propagate error return traces, function calls need to be passed
directly to an error-handling expression (`if`, `catch`, `try` or `return`):

  ```zig
  // When passed directly to `catch`, the return trace is propagated
  return errorable() catch error.UnknownError;

  // Using a break also works
  return blk: {
      // code here
      break :blk errorable();
  } catch error.UnknownError;
  ```

Why do we need this restriction? Without it, multiple errors can co-exist
with their own error traces. Handling that situation correctly means either:
  a. Dynamically allocating trace memory and tracking lifetimes, OR
  b. Allowing the production of one error to interfere with the trace of another
     (which is the current status quo)

This is piece (3/3) of ziglang#1923 (comment)
topolarity added a commit to topolarity/zig that referenced this issue Sep 23, 2022
This implement trace "popping" for correctly handled errors within
`catch { ... }` and `else { ... }` blocks.

When breaking from these blocks with any non-error, we pop the error
trace frames corresponding to the operand. When breaking with an error,
we preserve the frames so that error traces "chain" together as usual.

```zig
fn foo(cond1: bool, cond2: bool) !void {
    bar() catch {
    	if (cond1) {
	    // If baz() result is a non-error, pop the error trace frames from bar()
	    // If baz() result is an error, leave the bar() frames on the error trace
            return baz();
	} else if (cond2) {
	    // If we break/return an error, then leave the error frames from bar() on the error trace
	    return error.Foo;
	}
    };

    // An error returned from here does not include bar()'s error frames in the trace
    return error.Bar;
}
```

Notice that if foo() does not return an error it, it leaves no extra
frames on the error trace.

This is piece (1/3) of ziglang#1923 (comment)
topolarity added a commit to topolarity/zig that referenced this issue Sep 23, 2022
This allows for errors to be "re-thrown" by yielding any error as the
result of a catch block. For example:

```zig
fn errorable() !void {
    return error.FallingOutOfPlane;
}

fn foo(have_parachute: bool) !void {
    return errorable() catch |err| b: {
        if (have_parachute) {
            // error trace will include the call to errorable()
            break :b error.NoParachute;
        } else {
            return;
        }
    };
}

pub fn main() !void {
    // Anything that returns a non-error does not pollute the error trace.
    try foo(true);

    // This error trace will still include errorable(), whose error was "re-thrown" by foo()
    try foo(false);
}
```

This is piece (2/3) of ziglang#1923 (comment)
topolarity added a commit to topolarity/zig that referenced this issue Sep 23, 2022
In order to enforce a strict stack discipline for error return traces,
we cannot track error return traces that are stored in variables:

  ```zig
  const x = errorable(); // errorable()'s error return trace is killed here

  // v-- error trace starts here instead
  return x catch error.UnknownError;
  ```

In order to propagate error return traces, function calls need to be passed
directly to an error-handling expression (`if`, `catch`, `try` or `return`):

  ```zig
  // When passed directly to `catch`, the return trace is propagated
  return errorable() catch error.UnknownError;

  // Using a break also works
  return blk: {
      // code here
      break :blk errorable();
  } catch error.UnknownError;
  ```

Why do we need this restriction? Without it, multiple errors can co-exist
with their own error traces. Handling that situation correctly means either:
  a. Dynamically allocating trace memory and tracking lifetimes, OR
  b. Allowing the production of one error to interfere with the trace of another
     (which is the current status quo)

This is piece (3/3) of ziglang#1923 (comment)
topolarity added a commit to topolarity/zig that referenced this issue Sep 26, 2022
This implement trace "popping" for correctly handled errors within
`catch { ... }` and `else { ... }` blocks.

When breaking from these blocks with any non-error, we pop the error
trace frames corresponding to the operand. When breaking with an error,
we preserve the frames so that error traces "chain" together as usual.

```zig
fn foo(cond1: bool, cond2: bool) !void {
    bar() catch {
    	if (cond1) {
	    // If baz() result is a non-error, pop the error trace frames from bar()
	    // If baz() result is an error, leave the bar() frames on the error trace
            return baz();
	} else if (cond2) {
	    // If we break/return an error, then leave the error frames from bar() on the error trace
	    return error.Foo;
	}
    };

    // An error returned from here does not include bar()'s error frames in the trace
    return error.Bar;
}
```

Notice that if foo() does not return an error it, it leaves no extra
frames on the error trace.

This is piece (1/3) of ziglang#1923 (comment)
topolarity added a commit to topolarity/zig that referenced this issue Sep 26, 2022
This allows for errors to be "re-thrown" by yielding any error as the
result of a catch block. For example:

```zig
fn errorable() !void {
    return error.FallingOutOfPlane;
}

fn foo(have_parachute: bool) !void {
    return errorable() catch |err| b: {
        if (have_parachute) {
            // error trace will include the call to errorable()
            break :b error.NoParachute;
        } else {
            return;
        }
    };
}

pub fn main() !void {
    // Anything that returns a non-error does not pollute the error trace.
    try foo(true);

    // This error trace will still include errorable(), whose error was "re-thrown" by foo()
    try foo(false);
}
```

This is piece (2/3) of ziglang#1923 (comment)
topolarity added a commit to topolarity/zig that referenced this issue Sep 26, 2022
In order to enforce a strict stack discipline for error return traces,
we cannot track error return traces that are stored in variables:

  ```zig
  const x = errorable(); // errorable()'s error return trace is killed here

  // v-- error trace starts here instead
  return x catch error.UnknownError;
  ```

In order to propagate error return traces, function calls need to be passed
directly to an error-handling expression (`if`, `catch`, `try` or `return`):

  ```zig
  // When passed directly to `catch`, the return trace is propagated
  return errorable() catch error.UnknownError;

  // Using a break also works
  return blk: {
      // code here
      break :blk errorable();
  } catch error.UnknownError;
  ```

Why do we need this restriction? Without it, multiple errors can co-exist
with their own error traces. Handling that situation correctly means either:
  a. Dynamically allocating trace memory and tracking lifetimes, OR
  b. Allowing the production of one error to interfere with the trace of another
     (which is the current status quo)

This is piece (3/3) of ziglang#1923 (comment)
topolarity added a commit to topolarity/zig that referenced this issue Oct 15, 2022
This implement trace "popping" for correctly handled errors within
`catch { ... }` and `else { ... }` blocks.

When breaking from these blocks with any non-error, we pop the error
trace frames corresponding to the operand. When breaking with an error,
we preserve the frames so that error traces "chain" together as usual.

```zig
fn foo(cond1: bool, cond2: bool) !void {
    bar() catch {
    	if (cond1) {
	    // If baz() result is a non-error, pop the error trace frames from bar()
	    // If baz() result is an error, leave the bar() frames on the error trace
            return baz();
	} else if (cond2) {
	    // If we break/return an error, then leave the error frames from bar() on the error trace
	    return error.Foo;
	}
    };

    // An error returned from here does not include bar()'s error frames in the trace
    return error.Bar;
}
```

Notice that if foo() does not return an error it, it leaves no extra
frames on the error trace.

This is piece (1/3) of ziglang#1923 (comment)
topolarity added a commit to topolarity/zig that referenced this issue Oct 15, 2022
This allows for errors to be "re-thrown" by yielding any error as the
result of a catch block. For example:

```zig
fn errorable() !void {
    return error.FallingOutOfPlane;
}

fn foo(have_parachute: bool) !void {
    return errorable() catch |err| b: {
        if (have_parachute) {
            // error trace will include the call to errorable()
            break :b error.NoParachute;
        } else {
            return;
        }
    };
}

pub fn main() !void {
    // Anything that returns a non-error does not pollute the error trace.
    try foo(true);

    // This error trace will still include errorable(), whose error was "re-thrown" by foo()
    try foo(false);
}
```

This is piece (2/3) of ziglang#1923 (comment)
topolarity added a commit to topolarity/zig that referenced this issue Oct 15, 2022
In order to enforce a strict stack discipline for error return traces,
we cannot track error return traces that are stored in variables:

  ```zig
  const x = errorable(); // errorable()'s error return trace is killed here

  // v-- error trace starts here instead
  return x catch error.UnknownError;
  ```

In order to propagate error return traces, function calls need to be passed
directly to an error-handling expression (`if`, `catch`, `try` or `return`):

  ```zig
  // When passed directly to `catch`, the return trace is propagated
  return errorable() catch error.UnknownError;

  // Using a break also works
  return blk: {
      // code here
      break :blk errorable();
  } catch error.UnknownError;
  ```

Why do we need this restriction? Without it, multiple errors can co-exist
with their own error traces. Handling that situation correctly means either:
  a. Dynamically allocating trace memory and tracking lifetimes, OR
  b. Allowing the production of one error to interfere with the trace of another
     (which is the current status quo)

This is piece (3/3) of ziglang#1923 (comment)
topolarity added a commit to topolarity/zig that referenced this issue Oct 20, 2022
This implement trace "popping" for correctly handled errors within
`catch { ... }` and `else { ... }` blocks.

When breaking from these blocks with any non-error, we pop the error
trace frames corresponding to the operand. When breaking with an error,
we preserve the frames so that error traces "chain" together as usual.

```zig
fn foo(cond1: bool, cond2: bool) !void {
    bar() catch {
    	if (cond1) {
	    // If baz() result is a non-error, pop the error trace frames from bar()
	    // If baz() result is an error, leave the bar() frames on the error trace
            return baz();
	} else if (cond2) {
	    // If we break/return an error, then leave the error frames from bar() on the error trace
	    return error.Foo;
	}
    };

    // An error returned from here does not include bar()'s error frames in the trace
    return error.Bar;
}
```

Notice that if foo() does not return an error it, it leaves no extra
frames on the error trace.

This is piece (1/3) of ziglang#1923 (comment)
topolarity added a commit to topolarity/zig that referenced this issue Oct 20, 2022
This allows for errors to be "re-thrown" by yielding any error as the
result of a catch block. For example:

```zig
fn errorable() !void {
    return error.FallingOutOfPlane;
}

fn foo(have_parachute: bool) !void {
    return errorable() catch |err| b: {
        if (have_parachute) {
            // error trace will include the call to errorable()
            break :b error.NoParachute;
        } else {
            return;
        }
    };
}

pub fn main() !void {
    // Anything that returns a non-error does not pollute the error trace.
    try foo(true);

    // This error trace will still include errorable(), whose error was "re-thrown" by foo()
    try foo(false);
}
```

This is piece (2/3) of ziglang#1923 (comment)
topolarity added a commit to topolarity/zig that referenced this issue Oct 20, 2022
In order to enforce a strict stack discipline for error return traces,
we cannot track error return traces that are stored in variables:

  ```zig
  const x = errorable(); // errorable()'s error return trace is killed here

  // v-- error trace starts here instead
  return x catch error.UnknownError;
  ```

In order to propagate error return traces, function calls need to be passed
directly to an error-handling expression (`if`, `catch`, `try` or `return`):

  ```zig
  // When passed directly to `catch`, the return trace is propagated
  return errorable() catch error.UnknownError;

  // Using a break also works
  return blk: {
      // code here
      break :blk errorable();
  } catch error.UnknownError;
  ```

Why do we need this restriction? Without it, multiple errors can co-exist
with their own error traces. Handling that situation correctly means either:
  a. Dynamically allocating trace memory and tracking lifetimes, OR
  b. Allowing the production of one error to interfere with the trace of another
     (which is the current status quo)

This is piece (3/3) of ziglang#1923 (comment)
topolarity added a commit to topolarity/zig that referenced this issue Oct 21, 2022
This implement trace "popping" for correctly handled errors within
`catch { ... }` and `else { ... }` blocks.

When breaking from these blocks with any non-error, we pop the error
trace frames corresponding to the operand. When breaking with an error,
we preserve the frames so that error traces "chain" together as usual.

```zig
fn foo(cond1: bool, cond2: bool) !void {
    bar() catch {
    	if (cond1) {
	    // If baz() result is a non-error, pop the error trace frames from bar()
	    // If baz() result is an error, leave the bar() frames on the error trace
            return baz();
	} else if (cond2) {
	    // If we break/return an error, then leave the error frames from bar() on the error trace
	    return error.Foo;
	}
    };

    // An error returned from here does not include bar()'s error frames in the trace
    return error.Bar;
}
```

Notice that if foo() does not return an error it, it leaves no extra
frames on the error trace.

This is piece (1/3) of ziglang#1923 (comment)
topolarity added a commit to topolarity/zig that referenced this issue Oct 21, 2022
This allows for errors to be "re-thrown" by yielding any error as the
result of a catch block. For example:

```zig
fn errorable() !void {
    return error.FallingOutOfPlane;
}

fn foo(have_parachute: bool) !void {
    return errorable() catch |err| b: {
        if (have_parachute) {
            // error trace will include the call to errorable()
            break :b error.NoParachute;
        } else {
            return;
        }
    };
}

pub fn main() !void {
    // Anything that returns a non-error does not pollute the error trace.
    try foo(true);

    // This error trace will still include errorable(), whose error was "re-thrown" by foo()
    try foo(false);
}
```

This is piece (2/3) of ziglang#1923 (comment)
topolarity added a commit to topolarity/zig that referenced this issue Oct 21, 2022
In order to enforce a strict stack discipline for error return traces,
we cannot track error return traces that are stored in variables:

  ```zig
  const x = errorable(); // errorable()'s error return trace is killed here

  // v-- error trace starts here instead
  return x catch error.UnknownError;
  ```

In order to propagate error return traces, function calls need to be passed
directly to an error-handling expression (`if`, `catch`, `try` or `return`):

  ```zig
  // When passed directly to `catch`, the return trace is propagated
  return errorable() catch error.UnknownError;

  // Using a break also works
  return blk: {
      // code here
      break :blk errorable();
  } catch error.UnknownError;
  ```

Why do we need this restriction? Without it, multiple errors can co-exist
with their own error traces. Handling that situation correctly means either:
  a. Dynamically allocating trace memory and tracking lifetimes, OR
  b. Allowing the production of one error to interfere with the trace of another
     (which is the current status quo)

This is piece (3/3) of ziglang#1923 (comment)
andrewrk added a commit that referenced this issue Oct 22, 2022
…1923

stage2: Pop error trace frames for handled errors (#1923)
@andrewrk andrewrk modified the milestones: 0.11.0, 0.10.0 Oct 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted This proposal is planned. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Projects
None yet
8 participants