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_options a comptime struct instead of a namespace #14432

Closed
pfgithub opened this issue Jan 23, 2023 · 6 comments · Fixed by #18712
Closed

Make std_options a comptime struct instead of a namespace #14432

pfgithub opened this issue Jan 23, 2023 · 6 comments · Fixed by #18712
Labels
accepted This proposal is planned. 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

@pfgithub
Copy link
Contributor

pub const std_options = struct {
    pub fn logFn(…) void {};
};

→

fn log(…) void {}
pub const std_options: std.StdOptions = .{
    .logFn = log,
};

This would:

  • show some error messages closer to their source (for example, if a type doesn't match what's expected for that option)
  • provide a list of options and documentation in one place in the stdlib
  • allow editor extensions to suggest fields and types

Implementation in the standard library:

// zig/std.zig

pub const options = struct {
    pub const enable_segfault_handler: bool = if (@hasDecl(options_override, "enable_segfault_handler"))
        options_override.enable_segfault_handler
    else
        debug.default_enable_segfault_handler;
    …

→

// with the current language
pub const StdOptions = struct {
    enable_segfault_handler: ?bool = null,
    …
};
const options_override: StdOptions = if (@hasDecl(root, "std_options")) root.std_options else StdOptions {};
pub const options = struct {
    pub const enable_segfault_handler: bool =
        options_override.enable_segfault_handler orelse
        debug.default_enable_segfault_handler;
    …
};

// or, with a language change so defaults only evaluate if they're needed
pub const StdOptions = struct {
    enable_segfault_handler: bool = debug.default_enable_segfault_handler,
    …
};
pub const options: StdOptions = if (@hasDecl(root, "std_options")) root.std_options else StdOptions {};
@InKryption
Copy link
Contributor

I'm a fan of this, and at present it poses no immediate problems.
But I think it should at least be noted, this would likely complicate the addition of any stdlib option whose type is a function with a generic return type, since that can't be expressed as a standalone type for a struct field type. Ditto for a function for which we want the error set to be inferred.

IMO that isn't actually a strong argument against the proposal, because I can't actually imagine any use cases where any of that would be really necessary, so despite that, I'm in favor of this change.

@Vexu
Copy link
Member

Vexu commented Jan 23, 2023

The reason I went with a namespace instead of a struct instance is io_mode depends on event_loop being defined and the type of event_loop depends on io_mode but if that is cleared then I'd also be in favor of this.

@andrewrk andrewrk added 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 Jan 23, 2023
@andrewrk andrewrk added this to the 0.11.0 milestone Jan 23, 2023
@pfgithub
Copy link
Contributor Author

Ah, that's unfortunate

Assuming blocking io can't ever have an event loop, a tagged union could be used:

.io = .blocking,
.io = .{ .evented = .{} },
.io = .{ .evented = .{
    .event_loop = …,
} },

Or io_mode could be removed entirely:

event_loop: event.Loop.Instance
.event_loop = null, // blocking
.event_loop = std.event.default_instance, // default evented
.event_loop = custom, // custom evented

If blocking io is allowed to have an event loop, I'm not sure how to resolve the dependency without it being messy

@Luexa
Copy link
Contributor

Luexa commented Feb 3, 2023

The reason I went with a namespace instead of a struct instance is io_mode depends on event_loop being defined and the type of event_loop depends on io_mode but if that is cleared then I'd also be in favor of this.

I oppose this proposal because of the problems of self-referential declarations. Makes it pretty much impossible to depend on std.options in generic code that might implement some of its own std_options, whereas this is possible with declarations.

Additionally if there are options that let you declare types and implementations of those types, you'd be forced into the annoying pattern of specifying ?*anyopaque like std.builtin.Type is forced to do for default field values.

@andrewrk andrewrk added the accepted This proposal is planned. label Apr 9, 2023
@andrewrk
Copy link
Member

andrewrk commented Apr 9, 2023

Accepted as long as we can solve the issue that @Vexu mentioned.

@rohlem
Copy link
Contributor

rohlem commented Jul 7, 2023

@pfgithub I'm not sure what issue remains with a tagged union approach.

Is this what you considered being messy?:

// in @import("std") somewhere
const IoConfig = union(enum) {
  const Blocking = struct{
    EventLoopType: ?type = null,

    fn EventLoop(b: Blocking) type {
      return b.EventLoopType orelse DefaultBlockingModeEventLoop;
    }
  };
  const Evented = struct{
    EventLoopType: ?type = null,

    fn EventLoop(e: Evented) type {
      return e.EventLoopType orelse DefaultEventedModeEventLoop;
    }
  };

  blocking: Blocking,
  evented: Evented,

  const Mode = @typeInfo(@This()).Union.tag_type.?;
};

// usage examples:
const io_config_0: IoConfig = .{.blocking = .{}};
const io_config_1 = .{.evented = .{}};
const io_config_2 = .{.blocking = .{
  .EventLoopType = CustomEventLoopForBlocking,
} };
const io_config_3 = .{.evented = .{
  .EventLoopType = CustomEventLoopForEvented,
} };

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. 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