-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Proposal: Lazy error set types lazyerror{A, ?B}
#16147
Comments
Isn't const E = lazyerror{A, ?L, ?W}; //lazy-error-set type-placeholder
fn f(comptime check_l: bool, comptime check_w: bool) E!void { just equivalent to fn E(comptime check_l: bool, comptime check_w: bool) type {
return error{A} || if(check_l) error{L} else error{} || if(check_w) error{W} else error{};
}
fn f(comptime check_l: bool, comptime check_w: bool) E(check_l, check_w)!void { ? |
@N00byEdge They are not equivalent; your code snippet implements the option I listed as SQ-2 via a helper function. f(false, true) catch |e| switch(e) {
error.A, error.W => {},
error.L => {}, //triggers a compile error because L is not a part of the error set
}; A single call site using full runnable status-quo example here
// status-quo modelling via narrowest error set (SQ-2)
fn E(comptime check_l: bool, comptime check_w: bool) type {
//note: We actually need parentheses here, it resolved to a wrong set taken from your comment.
return error{A} || (if(check_l) error{L} else error{}) || (if(check_w) error{W} else error{});
}
fn f(comptime check_l: bool, comptime check_w: bool) E(check_l, check_w)!void {}
// usage code:
fn usesF(comptime check_l: bool, comptime check_w: bool) void {
f(check_l, check_w) catch |e| switch(e) {
error.A => {}, //correctly always required
error.L => {}, // providing this is a compile error if !check_l
// not providing error.W is a compile error if check_w
};
}
test usesF {
//usesF(false, false); //compile error
//usesF(false, true); //compile error
usesF(true, false); //works
//usesF(true, true); //compile error
}
// more error-prone, less readable variant, but allowed by the compiler currently:
fn genericallyUsesF(comptime check_l: bool, comptime check_w: bool) void {
f(check_l, check_w) catch |e| {
// When writing the proposal I thought these would be compile errors,
// but apparently == works with errors outside of the set
// (for now - not sure that it should).
if(e == error.A) {}
if(e == error.L) {}
if(e == error.W) {}
// Problem: The compiler cannot tell you if a branch is actually always dead code
if(e == error.Other) {}
// The simplest way to check for exhaustiveness (to make sure your errors are in sync)
// I found is to re-construct the error set.
comptime @import("std").debug.assert(@TypeOf(e) == E(check_l, check_w)); //error set of `f` out-of-sync
// Though in this example because `f` and `E` are updated at the same time we still won't know;
// we would actually need to call an "OldE" variant that is left in and only updated after all call sites
// (or one for each call site).
};
}
test genericallyUsesF { //these all work
genericallyUsesF(false, false);
genericallyUsesF(false, true);
genericallyUsesF(true, false);
genericallyUsesF(true, true);
} I believe (i.e. in my experience) this doesn't scale well:
My main issue isn't that I have to write the boilerplate once, it's that
So to recap, the main benefits of the proposal:
|
I don't see the difference to implementing "checking all errors for all targets", but I can't find the issue now. This feels to me merely like a programmer hint on what is target-defined for the documented targets. |
I do - I wanted to keep the OP general, but here are some details:
I'm currently writing a wrapper around SDL. Now if I assert (via Now here is a real section of the code.
It is not self-contained (won't build due to missing things), and I already know I'll be laughed at for my long identifiers, but anyway. // relevant helper functions
const helpers = opaque {
pub fn SingleErrorSet(comptime @"error": anytype) type { //<- turns error.ABC into error{ABC}
comptime assertIsErrorValue(@"error");
return @Type(.{ .ErrorSet = &.{@typeInfo(@TypeOf(@"error")).Error} });
}
pub fn intNarrowerThan(comptime A: type, comptime B: type) bool {
if (comptime (std.math.minInt(A) > std.math.minInt(B))) return true;
if (comptime (std.math.maxInt(A) < std.math.maxInt(B))) return true;
}
pub fn intCastCustomError(comptime Result: type, int_value: anytype, comptime @"error": anytype) IntCastCustomErrorSet(Result, @TypeOf(int_value), @"error")!Result {
comptime std.debug.assert(@typeInfo(int_value) == .Int);
return std.math.cast(Result, int_value) orelse return @"error";
}
pub fn IntCastCustomErrorSet( //<- returns either a 1- or 0-member error set, depending on the whether cast is narrowing
comptime Result: type,
comptime IntValue: type,
comptime @"error": anytype,
) type {
return if (intNarrowerThan(Result, IntValue)) return SingleErrorSet(@"error") else error{};
}
};
// internal, tagged-union type for SDL DisplayEvent subtype of SDL Event (which C union) - there are around 20 like this one
pub const DisplayEventConfig = struct {
DisplayIndex: type,
};
pub const DisplayEventSubEvent = union(enum) {
pub const OrientationChanged = struct {
new_orientation: internal.DisplayOrientation,
};
orientation_changed: OrientationChanged,
connected: void,
disconnected: void,
moved: void,
};
pub fn DisplayEvent(comptime display_event_config: DisplayEventConfig) type {
return struct {
pub const config = display_event_config;
pub const DisplayIndex = config.DisplayIndex;
pub const SubEvent = DisplayEventSubEvent;
display_index: DisplayIndex,
sub_event: SubEvent,
pub const FromRawConditionalErrorSet = //<- errors depending on config
helpers.IntCastCustomErrorSet(DisplayIndex, @TypeOf(@as(c.SDL_DisplayEvent, undefined).display), error.DisplayIndexTypeTooSmallForValue);
pub const FromRawErrorSetWithoutTopLevelSdlEventTypeValueErrors = //<- errors that the next level (PeripheryEvent) uses for its error set
error{ UnknownSdlDisplayEventType, UnknownSdlDisplayOrientationValue } || FromRawConditionalErrorSet;
pub const FromRawErrorSet = //<- the error set of the local fromRaw method
error{ UnknownSdlEventTypeValue, SdlEventTypeValueNotDisplayEventTypeValue } || FromRawErrorSetWithoutTopLevelSdlEventTypeValueErrors;
pub fn fromRaw(raw: c.SDL_DisplayEvent) FromRawErrorSet!@This() {
return switch (try internal.EventType.fromRawInt(raw.type)) {
else => return error.SdlEventTypeValueNotDisplayEventTypeValue,
.display_event => .{
.display_index = try helpers.intCastCustomError(DisplayIndex, raw.display, error.DisplayIndexTypeTooSmallForValue), //<- conditional error propagated here
.sub_event = switch (raw.event) {
else => return error.UnknownSdlDisplayEventType,
c.SDL_DISPLAYEVENT_ORIENTATION => .{ .orientation_changed = .{ .new_orientation = (try internal.DisplayOrientation.fromRawInt(raw.data1)).? } },
c.SDL_DISPLAYEVENT_CONNECTED => .connected,
c.SDL_DISPLAYEVENT_DISCONNECTED => .disconnected,
c.SDL_DISPLAYEVENT_MOVED => .moved,
},
},
};
}
};
}
// tagged-union grouping for periphery-related SDL events, including SDL DisplayEvent
pub const PeripheryEventConfig = struct {
DisplayIndex: type,
JoyDeviceIndex: type,
AudioDeviceIndex: type,
};
pub fn PeripheryEvent(comptime periphery_event_config: PeripheryEventConfig) type {
return union(enum) {
pub const config = periphery_event_config;
pub const Display = DisplayEvent(.{ .DisplayIndex = config.DisplayIndex });
pub const Keyboard = KeyboardPeripheryEvent;
pub const JoyDevice = JoyDevicePeripheryEvent(config.JoyDeviceIndex);
pub const GameController = GameControllerPeripheryEvent(config.JoyDeviceIndex);
pub const AudioDevice = AudioDevicePeripheryEvent(config.AudioDeviceIndex);
display: Display,
keyboard: Keyboard,
joy_device: JoyDevice,
game_controller: GameController,
audio_device: AudioDevice,
pub const FromRawErrorSetWithoutTopLevelSdlEventTypeValueErrors = Display.FromRawErrorSetWithoutTopLevelSdlEventTypeValueErrors || JoyDevice.FromRawErrorSetWithoutTopLevelSdlEventTypeValueErrors || GameController.FromRawErrorSetWithoutTopLevelSdlEventTypeValueErrors || AudioDevice.FromRawErrorSetWithoutTopLevelSdlEventTypeValueErrors;
pub const FromRawErrorSet = error{ UnknownSdlEventTypeValue, SdlEventTypeValueNotPeripheryEventTypeValue } || FromRawErrorSetWithoutTopLevelSdlEventTypeValueErrors;
pub fn fromRaw(raw: c.SDL_Event) FromRawErrorSet!@This() {
return switch (try internal.EventType.fromRawInt(raw.type)) {
else => return error.SdlEventTypeValueNotPeripheryEventTypeValue,
inline .display_event => |t| .{
.display = Display.fromRaw(t.extractDataAssertEventType(raw)) catch |err| switch (err) { //t.extractDataAssertEventType(raw) is safe because t comes from raw
error.UnknownSdlEventTypeValue => unreachable, //checked above
error.SdlEventTypeValueNotDisplayEventTypeValue => unreachable, //checked above
error.DisplayIndexTypeTooSmallForValue => |e| return e, //<- Here is the issue: This is required if the conditional error is part of the set, but becomes a compile error otherwise. An "else" clause also becomes a compile error when all other errors are handled. So there seems no way for me to write this as a `switch`.
},
},
inline .keyboard_keymap_changed => |t| .{
.keyboard = Keyboard.fromRaw(t.extractDataAssertEventType(raw)) catch |err| switch (err) { //t.extractDataAssertEventType(raw) is safe because t comes from raw
error.UnknownSdlEventTypeValue => unreachable, //checked above
error.SdlEventTypeValueNotKeyboardPeripheryEventTypeValue => unreachable, //checked above
},
},
.joy_device_connected,
.joy_device_disconnected,
.joy_device_remaining_energy_state_changed,
=> .{
.joy_device = JoyDevice.fromRaw(raw) catch |err| switch (err) {
error.UnknownSdlEventTypeValue => unreachable, //checked above
error.SdlEventTypeValueNotJoyDevicePeripheryEventValue => unreachable, //checked above
error.JoyDeviceIndexTypeTooSmallForValue => |e| return e, //<- same here - impossible to write as switch
},
},
inline .game_controller_connected,
.game_controller_disconnected,
.game_controller_remapped,
=> |t| .{
.game_controller = try GameController.fromRaw(t.extractDataAssertEventType(raw)) catch |err| switch (err) { //t.extractDataAssertEventType(raw) is safe because t comes from raw
error.UnknownSdlEventTypeValue => unreachable, //checked above
error.SdlEventTypeValueNotGameControllerPeripheryEventTypeValue => unreachable, //checked above
error.JoyDeviceIndexTypeTooSmallForValue => |e| return e, //<- same here - impossible to write as switch
},
},
inline .audio_device_connected,
.audio_device_disconnected,
=> |t| .{
.audio_device = try AudioDevice.fromRaw(t.extractDataAssertEventType(raw)) catch |err| switch (err) { //t.extractDataAssertEventType(raw) is safe because t comes from raw
error.UnknownSdlEventTypeValue => unreachable, //checked above
error.SdlEventTypeValueNotAudioDevicePeripheryEventTypeValue => unreachable, //checked above
error.AudioDeviceIndexTypeTooSmallForValue => |e| return e, //<- same here - impossible to write as switch
},
},
};
}
};
} Here's a much simpler example that demonstrates the same thing though (EDIT: now also placed at the beginning of the original post):
const file = openFile("abc.txt") catch |e| switch(e) {
error.WindowsOnlyError => {},
error.LinuxOnlyError => {},
}; In status-quo, either you merge all error sets (SQ-1, like Or you have a conditional error set (explicit = SQ-2, or deduced
If I understand you correctly, this is a concern that applies just the same to status-quo deduced error sets using
Example:
// status-quo
/// errors only on Linux
const LinuxErrors = error{LinuxError}
// we use old API before this version, which may lead to OldLinuxApiError
|| ( if (builtin.os.version.isAtLeast(.{.major = 3, .minor = 2, .patch = 0})) error{} else error{OldLinuxApiError} );
/// errors only on Windows
const WindowsErrors = error{WindowsError};
const ErrorSQ = error{NotFound} || switch(builtin.os.tag) {
.linux => LinuxErrors,
.windows => WindowsErrors,
else => error{}
};
// compare with lazy-error-set placeholder:
const ErrorLE = lazyerror{
NotFound,
//only on Linux
?LinuxError,
//we use old API on Linux before version 3.2.0 , which may lead to OldLinuxApiError
?OldLinuxApiError,
//only on Windows
?WindowsError,
} I don't see lazy-error-set types or placeholders having a direct negative effect on reading code without tooling - if you do, please let me know how/why.
With approach (SQ-1) of merging all error sets, we actually rob the compiler of this information: Function boundaries (in current semantics, unless you For differing error sets by logic (SQ-2 and SQ-3) there is still the main issue of the compiler currently limiting use of Generally speaking, I believe the conditional logic behind error sets generally explodes / grows exponentially:
This requires error set generation code that should be kept in-sync with the logic of actual functions that can return those errors - but this currently must be done by hand. With lazy-error-set types, the lazy-error portion of the error set will instead be deduced by the compiler. Maybe there is some exact threshold or definition of "too much boilerplate" for you, or maybe you could qualify a "common case" somehow - let me know and I'm happy to construct / provide more examples.
With "the issue", I assume you're referring to a GitHub issue about checking for each compilation target that the code handles the error sets correctly?
With "enforcing correct error sets", do you mean a compile error for when an error set is too wide? Or do you mean making sure all reachable errors are handled? |
For reference, I've now updated the OP with the maybe-main-point about |
(Another update: It just came to my mind that if #2647 got implemented first, then lazily-unreachable errors could be modeled by assigning
noreturn
payloads to errors. This would already unblock the usage patterns this proposal allows, making the rest of it strictly-speaking a quality-of-life improvement to cut down on repetition to make code more concise.)Update: The Gist
This should have been my introductory statement all along, taken from my comment below.
If I'm not being clear here, please ask for clarification (and maybe save yourself reading the rest for now).
When deciding on a function's error set in status-quo, there are mainly two approaches:
std
does this).E!T
(named SQ-2 below) or deduced!T
(named SQ-3 below)).You are instead forced to duplicate the
switch
, once with onlyerror.WindowOnlyError
, once with onlyerror.LinuxOnlyError
, and branch to the right one.That is maybe the main practical reason for this proposal. If you have a solution for this in status-quo, please let me know.
There are other benefits to this proposal, but this might be the primary motivator.
Basically, my proposed idea boils down to telling the compiler that
WindowsOnlyError
andLinuxOnlyError
arelazily-reachable
in this error set.Therefore it's okay (not a compile error) if they appear in the
switch
as clauses - but they only have to appear if the compiler determines they are actually reachable (the same way it determines the error set in!T
).If this doesn't sound too crazy yet, then here's the full proposal:
Problem statement / Scenario:
Imagine a function
f
with differing error conditions (=> a differing error set) depending oncomptime
conditions. For example:error.A
can always happen under anycomptime
configurationerror.L
can happen undercomptime
conditioncond_l
.(For example when targeting Linux, but the condition may be arbitrarily complex.)
error.W
can happen under differentcomptime
conditioncond_w
- same arbitrary complexity here.Status-quo modeling options
std
).Problems:
error.L
anderror.W
are reachable even if they aren't.(unless they use
unreachable
etc. to filter their logic based oncond_l
andcond_w
,which is complex and bloats code).
(Note: applies to callers within your module, but also users downstream.)
error{A} || (if(cond_l) error{L} else error{}) || ...
)Problems:
unreachable
etc. based on the error set(or the underlying conditions), since writing f.e.
switch
clauses for errors outside of an error value's setis a compile error.
(Note: applies to callers within your module, but also users downstream.)
!T
)Problems:
(in which case it's less maintainable, because not checked by the compiler)
(Note: applies to callers within your module, but also users downstream.)
None of these are optimal. Zig strives for optimal code, hence this proposal.
Proposal: A lazy-error-set type-placeholder syntax
lazyerror{A, ?B}
, and deduced-lazy-error-set types.The semantics are as follows:
?
areassumed-reachable
.(If no elements have a preceding
?
, the semantics are the same as for a status-quo error set.)?
arelazily-reachable
. Their actual reachability is deduced by the compiler.Short example showcase:
Details
Note that a lazy-error-set type-placeholder
lazyerror{A, ?L, ?W}!void
isn't exactly a type, just as!void
isn't either. The actual result type is deduced by the compiler, but will always contain all assumed-reachable elements, and never contain an error outside of the given set.However, unlike with
!T
, lazy-error-set type-placeholders carry more information, therefore I do think it would be helpful to promote them to actualcomptime
values:const
-s(they're technically not
type
s, so just making a new special categorylazyerrorset
would be the easiest.)||
as for status-quo error sets)Note that a deduced-lazy-error-set type must hold more information than a status-quo error set: In addition to the set of reachable errors (
assumed-reachable
+lazy
errors that the compiler determined reachable), it also needs to hold the set of remaining lazy errors for the compiler to check whether aswitch
clause or value comparison should be a compile error (e.g. to catch mistakes leading to always-dead branches in code).Deduced-lazy-error-set types would also make sense as
comptime
values, to allow merging them.Additional considerations
error
instead of introducinglazyerror
in type-placeholder syntax.This is how I typed it intially, then I changed it. Now I want to change it back.
error{(...)}
would then yield either atype
or alazyerrorset
, depending on whether there is a?
element.error{A}
andlazyerror{A}
are semantically supposed to be equivalent - if we use the same keyword they are syntactically equivalent.?
here as well would get confusing.error{A}lazilyunreachable{L,W}
is good enough.The text was updated successfully, but these errors were encountered: