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

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

Open
Arnau478 opened this issue Oct 29, 2023 · 6 comments
Open

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

Arnau478 opened this issue Oct 29, 2023 · 6 comments
Labels
breaking Implementing this issue could cause existing code to no longer compile or have different behavior. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. standard library This issue involves writing Zig code for the standard library.
Milestone

Comments

@Arnau478
Copy link
Contributor

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 Vexu added breaking Implementing this issue could cause existing code to no longer compile or have different behavior. standard library This issue involves writing Zig code for the standard library. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. labels Oct 30, 2023
@Vexu Vexu added this to the 0.13.0 milestone Oct 30, 2023
@Vexu
Copy link
Member

Vexu commented Oct 30, 2023

Marking as accepted based on #3257 (comment)

@Vexu Vexu added the accepted This proposal is planned. label Oct 30, 2023
@andrewrk andrewrk removed the accepted This proposal is planned. label Oct 31, 2023
@andrewrk
Copy link
Member

I would like to review this proposal please.

@N00byEdge
Copy link
Contributor

N00byEdge commented Dec 24, 2023

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
Copy link
Contributor

BratishkaErik commented Dec 24, 2023

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
Copy link
Contributor Author

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
Copy link
Contributor

nektro commented Dec 24, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Implementing this issue could cause existing code to no longer compile or have different behavior. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. standard library This issue involves writing Zig code for the standard library.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants