ziglang / zig

General-purpose programming language and toolchain for maintaining robust, optimal, and reusable software.
https://ziglang.org
MIT License
33.82k stars 2.47k forks source link

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

Open Arnau478 opened 10 months ago

Arnau478 commented 10 months ago

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.

Vexu commented 10 months ago

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

andrewrk commented 10 months ago

I would like to review this proposal please.

N00byEdge commented 9 months ago

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.

BratishkaErik commented 9 months ago

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.

Arnau478 commented 9 months ago

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()...

nektro commented 9 months ago

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.