os: improve the unexpectedErrno function

[perillo]

Print the error name, instead of the error number.
Error numbers are platform specific.

I have only tested this change with Linux.

[rohlem]

@tagName is checked illegal behavior for unnamed values of non-exhaustive enums, which E seems to be on most (all?) platforms.
However, I believe that the default formatting for enums (so just using {} and passing .{err} directly) already has handling for printing the name if available, and the numeric code otherwise. Perhaps we could use that?

[perillo]

Thanks for the correction. I assumed it was exhaustive without checking.

I avoided using the default formatting of enums, since it prints the full type name; e.g. on linux x86_64 I got os.linux.errno.generic.E.ACCES. I think the additional information is unnecessary.

A simple solution it to copy the code from fmt when formatting a non-exhaustive enum.

[perillo]

I fixed the incorrect handling of @TagName, but I had to add a new function.

Does it make sense to use @setCold(true); in both errorName and unexpectedErrno functions?

[jacobly0]

Isn't the new function just a copy std.enums.tagName?

[perillo]

Isn't the new function just a copy std.enums.tagName?

You are right; I was unaware of the std.enums namespace. Thanks!
The code of the errorName function has been adapted from fmt.formatType. Maybe that code should be updated?

Unrelated: does @intFromEnum cache the result value? In my code I called @intFromEnum only once.

[tauoverpi]

What's the binary size difference after this change?

[perillo]

What's the binary size difference after this change?

Using the official compiler, version 0.12.0-dev.3008+b2374c4d7 and using the source code, commit 81aa74e.

Build command: zig build --zig-lib-dir lib --prefix build/stage4 -Dno-langref -Dno-autodocs

size with this change: 91277752 bytes
size without this change: 91281224 bytes

[perillo]

The code probably needs some tests, to ensure that an error not belonging to the E enum ​works correctly. Unfortunately I can't think of a good way to implement this test.

[tauoverpi]

I should have clarified. This change should likely be opt-in/out (off by default in ReleaseSmall) given the size increase matters for embedded targets where size is a concern. Otherwise one would have to completely avoid std.os.

[perillo]

I should have clarified. This change should likely be opt-in/out (off by default in ReleaseSmall) given the size increase matters for embedded targets where size is a concern. Otherwise one would have to completely avoid std.os.

The error number or name is printed only if builtin.mode == .Debug and, due to current implementation limitations, when builtin.zig_backend == .stage2_llvm. So, unless I misunderstood what you wrote, there should be no impact on embedded targets since that code will not be compiled.

[davidgmbb]

Since you are at it, could you fix the unexpected_error_tracing and and it with the existing option @import("builtin").have_error_return_tracing. This 100% will respect the user option.

[perillo]

Since you are at it, could you fix the unexpected_error_tracing and and it with the existing option @import("builtin").have_error_return_tracing. This 100% will respect the user option.

Ok, thanks for the suggestion.

The current filter only enables error return tracing in Debug mode, but the documentation says that it is also enabled for ReleaseSafe mode.

[rohlem]

The current filter only enables error return tracing in Debug mode, but the documentation says that it is also enabled for ReleaseSafe mode.

@perillo This was changed recently in #18160 (heading "Error Return Tracing in ReleaseSafe Optimization Mode"), so I assume the documentation is out-of-date and should be changed.

[davidgmbb]

Since you are at it, could you fix the unexpected_error_tracing and and it with the existing option @import("builtin").have_error_return_tracing. This 100% will respect the user option.

Ok, thanks for the suggestion.

The current filter only enables error return tracing in Debug mode, but the documentation says that it is also enabled for ReleaseSafe mode.

Yes but the user can override it, no matter which optimization mode is chosen.

[davidgmbb]

Since the change I proposed does not seem to conflict with this branch, I opened a separate PR for it: #19140

[andrewrk]

I'm sorry, I didn't review this in time, and now it has bitrotted. Furthermore, so many pull requests have stacked up that I can't keep up and I am therefore declaring Pull Request Bankruptcy and closing old PRs that now have conflicts with master branch.

If you want to reroll, you are by all means welcome to revisit this changeset with respect to the current state of master branch, and there's a decent chance your patch will be reviewed the second time around.

Either way, I'm closing this now, otherwise the PR queue will continue to grow indefinitely.