-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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.os.exit cross-platform and related start.zig changes #16094
Conversation
This deduplicates logic that was previously duplicated for Windows specifically for the case of the user having a public wWinMain function declaration instead of the normal main function declaration. This is done by taking logic from the top-level comptime block and reusing in a different place to decide whether to call call_wWinMain or continue with the logic for the normal main. Additionally, CallMainReturnType is introduced which changes based on whether wWinMain is to be called instead of the normal main. This is how I tested the changes for Windows: ``` $ vi x.zig $ cat x.zig const std = @import("std"); pub fn main() u8 { std.debug.print("hello from normal main\n", .{}); return 10; } $ zig build-exe x.zig -target x86_64-windows --zig-lib-dir lib $ wine x.exe hello from normal main $ $? 10: command not found $ vi x.zig $ cat x.zig const std = @import("std"); pub fn wWinMain(hInstance: std.os.windows.HINSTANCE, hPrevInstance: ?std.os.windows.HINSTANCE, pCmdLine: std.os.windows.PWSTR, nCmdShow: c_int) c_int { _ = hInstance; _ = hPrevInstance; _ = pCmdLine; _ = nCmdShow; std.debug.print("hello from wWinMain\n", .{}); return 5; } $ zig build-exe x.zig -target x86_64-windows --zig-lib-dir lib $ wine x.exe hello from wWinMain $ $? 5: command not found $ ```
This removes std.start from the public API surface because it doesn't need to be public. The only remaining place where something inside start.zig is referenced is in os.zig in `fn getenv` and that still works. This last reference will eventually be removed, too. The only possibly useful thing std.start had was callMain and call_wWinMain but if you want to call main, just call `main`. I can't think of an actual use case where you would have to call these internal functions instead. Nothing is made impossible by making these private. std.os.getenv still works after this: ``` $ cat x.zig const std = @import("std"); pub fn main() void { std.debug.print("{?s}\n", .{std.os.getenv("LC_ALL")}); } $ zig run x.zig --zig-lib-dir lib C ```
I know this is kind of abrupt and there's no deprecation either but I don't think many people used this anyway. The reason I removed it is because std.os.uefi.Status is a usize too and is a non-exhaustive enum which means you can @intToEnum any value to std.os.uefi.Status if you want to return something outside of the standard status values. I suppose returning non-specified status values is discouraged so this would add to that. This change will make more sense in the following commits. The purpose is mainly unification.
This makes the printing code use std.os.uefi.system_table.con_out (which is like stdout) because I simply can't see the panic message actually being printed when it's using std.os.uefi.system_table.std_err. Additionally, this changes the color for after the error message from white to light gray which seems to match the default foreground text color. Finally, it prints "error" instead of "err" which matches the std.log.err output more which is used in callMain on Linux for example. Screenshots are included: ``` $ cat uefi.zig pub fn main() void { @Panic("something went wrong"); } $ # OVMF = TianoCore UEFI firmware. your package manager might have a package for it $ # BEFORE: $ zig build-exe uefi.zig --name bootx64 -target x86_64-uefi-msvc; mkdir -p Boot/EFI/BOOT; mv bootx64.efi bootx64.pdb Boot/EFI/BOOT/ $ qemu-system-x86_64 -bios /usr/share/OVMF/OVMF_CODE.fd -drive format=raw,file=fat:rw:Boot $ # screenshot: https://i.imgur.com/2AbVgpL.png $ # AFTER: $ zig build-exe uefi.zig --name bootx64 -target x86_64-uefi-msvc --zig-lib-dir lib; mkdir -p Boot/EFI/BOOT; mv bootx64.efi bootx64.pdb Boot/EFI/BOOT/ $ qemu-system-x86_64 -bios /usr/share/OVMF/OVMF_CODE.fd -drive format=raw,file=fat:rw:Boot $ # screenshot: https://i.imgur.com/BnRzQEV.png ```
The motivation for this change is that `std.os.exit(int)` **is not cross-platform**. The usual assumption is that the OS allows returning an integer upon exiting which actually is in the range 0 to 255. This is not the case on all operating systems. We cannot assume u8 as the exit status type on all systems. Therefore we make the API cross-platform with this change. One example is Plan 9. On Plan 9 you can't return exit codes like on Linux. Instead, you return strings. This is now supported. See the TODO for this that is now resolved. I have confirmed this to work on the UEFI with this code: ``` pub fn main() !@import("std").os.uefi.Status { @import("std").process.exit(.Aborted); return error.SomethingWentWrong; } ``` I've also tested this on Linux and Windows. I was not able to test this on Plan 9 unfortunately because I don't understand how to pass a compiled binary through QEMU to run it on Plan 9. I'm quite confident any regressions will be easy to fix though. Indeed, this means that if you write cross-platform code, you should now use `std.os.exit(std.os.exit_status_success)` instead of `std.os.exit(0)`. And similarly `std.os.exit(exit_status_failure)` instead of `std.os.exit(1)`. I made some but not all invocations of `std.os.exit` to be cross-platform but it's not too important right now because currently no platform that doesn't use integers as the exit status runs on the CI or is one of the commonly used platforms. Replacing the rest can be done with time. --- In start.zig, we now also use the same "bad main return type" error message everywhere. This is possible thanks to a previous commit that removes support for usize as the return type for main on the UEFI. Regression: one thing that's slightly worse after this is the bad_main_ret error message on the UEFI. For UEFI programs it now prints "expected return type of main to be 'void', '!void, 'os.uefi.status.Status', '!os.uefi.status.Status', or 'noreturn'" Previously it was "std.os.uefi.Status". Now it's "os.uefi.status.Status". This is a problem caused by using @typename. One way to solve this would be to add a function to std.meta or similar that creates a type string using its own logic. It could skip private namespaces (like os.uefi.status). This is a rather minor problem though and I think it's okay for now. On other platforms like Linux the error message looks pretty good: ``` $ cat x.zig const std = @import("std"); pub fn main() !u123 { return 126; } $ zig run x.zig --zig-lib-dir lib lib/std/start.zig:582:29: error: expected return type of main to be 'void', '!void, 'u8', '!u8', or 'noreturn' else => @CompileError(bad_main_ret), ^~~~~~~~~~~~~~~~~~~~~~~~~~~ ``` As for `if (std.os.ExitStatus == void)` in the bad_main_ret definition, std.os.ExitStatus can currently not be `void` but in the future that will almsot certainly change. I'm very confident there will be an operating system that you cannot pass a value to when exiting.
Here's what it looks like: ``` $ cat uefi.zig pub fn main() !@import("std").os.uefi.Status { return error.SomethingWentWrong; } $ # OVMF = TianoCore UEFI firmware. your package manager might have a package for it $ zig build-exe uefi.zig --name bootx64 -target x86_64-uefi-msvc --zig-lib-dir lib; mkdir -p Boot/EFI/BOOT; mv bootx64.efi bootx64.pdb Boot/EFI/BOOT/ $ qemu-system-x86_64 -bios /usr/share/OVMF/OVMF_CODE.fd -drive format=raw,file=fat:rw:Boot $ # screenshot: https://i.imgur.com/OmkauY9.png ```
This makes it easier to understand where and for which platform these functions are used.
Another API I just thought of would be Again, this PR as-is isn't supposed to be a breaking change for any of the major platforms. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
child_process needs to be also adjusted, since the exit status is taken from the return value.
As far as I understand it as of now, plan9 is not ci-tested.
You can test wasm with wasmtime: TODO:
.wasi => u7,``
I just tested it on Plan9 and it seems like it worked:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah. On POSIX AFAIU the exit code is pretty much defined as
|
This is blocked on #16135 for now. |
It is highly recommended you read the commit descriptions. A lot of information in these commit descriptions.
The main focus here was fixing the fact that
std.os.exit
is not appropriately abstracted so that it's cross-platform.std.os.exit(0)
is not cross-platform. There are operating systems where you can't return an integer (Plan 9), or at least notu8
when exiting (UEFI usesusize
), or anything at all (many older systems). On Plan 9 for example, you return a string as the exit status instead of an integer.Or maybe you wrote an operating system where you return a data structure as the exit status.
TL;DR we cannot assume
u8
as the exit status for all platforms.The current approach of taking
u8
but then fixing it up to make it kind of compatible with the underlying system is not ideal either.This means there is now
std.os.ExitStatus
which is the type of the value that can be returned to the operating system when exiting.However to make it easier to write cross-platform code without having to deal with
std.os.ExitStatus
directly (because the type depends on the OS), the declarationsstd.os.exit_status_success
andstd.os.exit_status_failure
are provided for these two most common situations: 1. exiting and communicating success and 2. exiting and communicating failure.This abstracts the API so that
std.os.exit(0)
still works but, assuming 0 means success here, it's now considered unidiomatic and the recommended pattern isstd.os.exit(std.os.exit_status_success)
.Similarly, instead of
std.os.exit(1)
you usestd.os.exit(std.os.exit_status_failure)
.For more specific exit statuses you either ignore cross-platform compatibility or come up with a different exit status for that specific case for each platform that you support.
Exiting a program goes in hand with the return type of the main function, because that is what's returned, so there are also many changes to
lib/std/start.zig
, namely:std.start
no longer exists in the public std API surface.void
,!void
,noreturn
+ whateverstd.os.ExitStatus
is are now supported as the return types formain
on all platforms. Previously, on the UEFI, error unions weren't supported.Finally, now if your program returns from
main
a value of a type other thanvoid
,!void
, ornoreturn
, you might want to usestd.os.ExitStatus
instead ofu8
.Basically,
still works as before but the cross-platform way of doing it would be
(assuming that 0 means success).
Uses of
std.os
always look kind of low-level though so maybe we can create aliases for these definitions instd.process
(std.process.exit
is also an alias ofstd.os.exit
).Or maybe aliases in
std.start
?It is highly recommended you read the commit descriptions. A lot of information in these commit descriptions.