zig icon indicating copy to clipboard operation
zig copied to clipboard

Make `std.process.execv` return type be `ExecvError!noreturn`

Open Arnau478 opened this issue 1 year ago • 6 comments

Make std.process.execv (and similar) return type be ExecvError!noreturn, instead of ExecvError. This plays better with the type system, and helps the user understand that the function would not return under normal circumstances.

Side-effects

One way to do things

There are now two ways to do the same thing:

const e = std.process.execv(...);
reportError(e);

and

std.process.execv(...) catch |e| reportError(e);

This would eliminate the first one.

Ability to use it with try

Making this an error union rather than an error set would allow it to be used with try

More readable code

Having the error-handling code always inside a catch and not dangling around after the execv would make it more readable and require less context to understand.


As a reminder:

* Communicate intent precisely. * Favor reading code over writing code. * Only one obvious way to do things.

Arnau478 avatar Oct 29 '23 21:10 Arnau478

Marking as accepted based on https://github.com/ziglang/zig/issues/3257#issuecomment-542846671

Vexu avatar Oct 30 '23 09:10 Vexu

I would like to review this proposal please.

andrewrk avatar Oct 31 '23 08:10 andrewrk

Ability to use it with try

Making this an error union rather than an error set would allow it to be used with try

is it just me or is return execve(...); much more clear than try execve(...);? It actually shows what's happening here since there's no success path. This actually seems like an argument against the change and also violates all three rules you listed in the OP.

The only case where I could see this being useful would be in generic code.

N00byEdge avatar Dec 24 '23 02:12 N00byEdge

Ability to use it with try

Making this an error union rather than an error set would allow it to be used with try

is it just me or is return execve(...); much more clear than try execve(...);? It actually shows what's happening here since there's no success path. This actually seems like an argument against the change and also violates all three rules you listed in the OP.

With this proposal you can write it as return try execve(...); which is IMHO more readable and has benefits of both variants.

UPD: errdefer |err| ... works with both.

BratishkaErik avatar Dec 24 '23 08:12 BratishkaErik

With this proposal you can write it as return try execve(...); which is IMHO more readable and has benefits of both variants.

Hmm, that doesn't look right to me, as, if execve() returned a !noreturn, try execve() would have a type of noreturn. So you're returning a noreturn. It's kind of like doing return @panic()...

Arnau478 avatar Dec 24 '23 09:12 Arnau478

remember that try is sugar. the point here is not that you can return, but that you can catch failure. return is just one of many strategies to handle an error.

nektro avatar Dec 24 '23 20:12 nektro