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

allow configuration of error set tag type via a public const in the root source file #786

Closed
andrewrk opened this issue Feb 24, 2018 · 13 comments · Fixed by #17532 or #17651
Closed
Labels
accepted This proposal is planned. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Milestone

Comments

@andrewrk
Copy link
Member

Right now error sets are hardcoded to codegen to a u16.

Ideally, zig would choose an appropriate sized integer based on the total number of error values in the compilation, but the user may do @sizeOf(SomeStruct) which has an error set type as a field before the entire compilation finishes analysis. This would create a circular dependency.

One idea is to have the error set integer type have a default of u32, but in the same way that you can override the default panic handler in the root source file, you can override the error set integer type in the root source file by doing:

pub const ErrorIntegerType = u8;

Then you'd get a compile error if zig ran out of bits when choosing error values.

@andrewrk andrewrk added the proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. label Feb 24, 2018
@andrewrk andrewrk modified the milestones: 0.3.0, 0.4.0 Feb 24, 2018
@andrewrk andrewrk added the accepted This proposal is planned. label Nov 21, 2018
@andrewrk andrewrk modified the milestones: 0.4.0, 0.5.0 Mar 22, 2019
@andrewrk andrewrk modified the milestones: 0.5.0, 0.6.0 Sep 2, 2019
@andrewrk andrewrk modified the milestones: 0.6.0, 0.7.0 Feb 10, 2020
@andrewrk andrewrk modified the milestones: 0.7.0, 0.8.0 Oct 9, 2020
@daurnimator
Copy link
Collaborator

daurnimator commented Feb 23, 2021

Further proposal:

For performance reasons it would be nice if errno values mapped to their same number in the error set, e.g. EPERM -> 1, that way most errno processing code doesn't need a lookup table; this saves in both runtime cost and binary size (smaller error lookup tables).

One way we could do this is by allowing the ErrorSet type to be declared via an enum in the root file (rather than the original proposal of an integer type). Lets make the restriction that it must be a non-exhaustive enum.

A straightforward setup, just declaring the error set size:

pub const ErrorSetType = enum(u8) { _ };

Mapping errno:

pub const ErrorSetType = enum(u32) {
  PermissionDenied = std.os.EPERM,
  FileNotFound = std.os.ENOENT,
  ProcessNotFound = std.os.ESRCH,
  // etc.
  _,
};

@g-w1
Copy link
Contributor

g-w1 commented Feb 23, 2021

What should be the max size of an error specified by the user? as @MasterQ32 pointed out, even u32 requires a LOT (34 gb) of source code to use up fully. I think u32 is a good limit. https://freenode.irclog.whitequark.org/zig/2021-02-23#1614089709-1614090346;

@andrewrk andrewrk changed the title better handling of error set integer type allow configuration of error set tag type via a public const in the root source file Mar 28, 2021
@andrewrk andrewrk modified the milestones: 0.8.0, 0.9.0 May 19, 2021
@lemaitre
Copy link

I don't know if the language can take advantage of this, but I think u31 could be better as it would make it possible for anyerror!u31 to be 4 byte large.

@andrewrk andrewrk modified the milestones: 0.9.0, 0.10.0 Nov 20, 2021
@PaperPrototype
Copy link

I think we kinda create a problem by allowing multiple errors with the same name.

We could instead force user's to set the uint size of the error, and then let them handle the casting between different error types. It would be more verbose (also more C like), and I feel it would help reduce ambiguity.

@PaperPrototype
Copy link

PaperPrototype commented Feb 14, 2022

Since errors are just enums you could force users to do this

const Values = error(u32) {
    zero,
    one,
    two,
};

And if they don't it can default to u16. It's simple and possible since you don't have to combine errors with the same name and assign them the same number (doing that was cool, but I feel unecessary)

Anyway, hope this small thinking of mine prevents zig from having those "special cases" that often trip programmers up

@InKryption
Copy link
Contributor

@PaperPrototype

const Values = error(u32) {
    zero,
    one,
    two,
};

How would this interact with the global error set? Afaict, that would require that the backing integer for errors would have to be the minimum integer type specified by any error set referenced/used in the program, which just sounds like something bad waiting to happen.

A way to mitigate this would be to force the user to specify the maximum backing integer type for the error set, which brings us right back to this original proposal, making your suggestion somewhat superfluous.

@PaperPrototype
Copy link

PaperPrototype commented Feb 16, 2022

How would this interact with the global error set?

I never thought of it... but now you you point it out, we would be forced to work with the global error set to make sure each error is unambiguous since 2 separate error types could get the same number and would be considered equal even though they were declared as separate errors 😂 and that would be very confusing!

Thank you for helping me see this 😂 @InKryption

@PaperPrototype
Copy link

So does zig literally have to resolve all errors globally? I can think of no other way to do it that would make it possible to resolve the errors on a per package basis since they are all unsigned integers

@PaperPrototype
Copy link

Random idea
What if the package hash + the error number were stored in the error? That would make errors more complex though...

@PaperPrototype
Copy link

I'm just realizing that zig is going to have trouble with DLL's, and that could be a problem

I am aware of Fast Binary Patching (which is totally epic), but I question it's viability in a browser environment.

@andrewrk andrewrk modified the milestones: 0.10.0, 0.11.0 Apr 16, 2022
Vexu added a commit to Vexu/zig that referenced this issue Jan 24, 2023
@andrewrk andrewrk modified the milestones: 0.11.0, 0.12.0 Apr 9, 2023
@kfird214
Copy link

kfird214 commented Jun 8, 2023

I see three options, all feel not so good to me, but just putting it out there for discussion.

  1. Before the interpreter starts to interpret comptime stuff, count all error. symbols. and this count will choose the backing integer.
    • Keep in mind that there should not be a possibility to add errors at the comptime interpretation time. and also it would count unused errors.
  2. Extends idea 1 but instead of not being able to add errors at comptime and eliminate unused errors count.
    if after the interpretation step the back integer size changes, force hardcode it.
  3. Just set the minimum to u8 and if the compiler error set exceeds this, throw comptime error and force the use of set it on the root file.

@andrewrk andrewrk modified the milestones: 0.13.0, 0.12.0 Jun 29, 2023
@ethindp
Copy link

ethindp commented Jul 8, 2023

Here's an idea that I feel is being overlooked:

  1. When compiling, before comptime, merge all error sets into a temporary global error set, obviously eliminating any duplicates.
  2. Determine the number of elements in the global error set
  3. The number of elements determines the size of the error type for all compilation units being compiled.

Given that you have 65536 possibilities, this should be sufficient for any possible task, even having millions and millions of error sets. This seems like a sufficient method and gets rid of all the over-complexity that I've seen in this thread.

Vexu added a commit to Vexu/zig that referenced this issue Oct 15, 2023
andrewrk pushed a commit that referenced this issue Oct 16, 2023
@andrewrk andrewrk modified the milestones: 0.13.0, 0.12.0 Oct 16, 2023
scheibo added a commit to scheibo/zig that referenced this issue Oct 19, 2023
This flag was added in ziglang#17532 to close ziglang#786, but the language
reference was never updated to reflect the change.
scheibo added a commit to scheibo/zig that referenced this issue Oct 20, 2023
This flag was added in ziglang#17532 to close ziglang#786, but the language
reference was never updated to reflect the change.
andrewrk added a commit that referenced this issue Oct 20, 2023
This reverts commit 78855bd.

This commit did not replace uses of `Type.err_int` of which there are
currently 60 uses.

Re-opens #786
@andrewrk
Copy link
Member Author

Implementation reverted in 6dc45e7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted This proposal is planned. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Projects
None yet
8 participants