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

std.Build: support passing optionals to dependency args #21928

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Lzard
Copy link
Contributor

@Lzard Lzard commented Nov 6, 2024

Allows conditionally setting dependency options.

Example

a's build.zig:

const std = @import("std");

pub fn build(b: *std.Build) !void {
    const target = b.standardTargetOptions(.{});
    std.debug.print("target = {s}\n", .{ try target.result.zigTriple(b.allocator) });

    const optimize = b.standardOptimizeOption(.{});
    std.debug.print("optimize = {}\n", .{ optimize });

    const x = b.option(i32, "x", "");
    std.debug.print("x = {?}\n", .{ x });
}

b's build.zig:

const std = @import("std");

pub fn build(b: *std.Build) void {
    _ = b.standardOptimizeOption(.{});
    _ = b.dependency("a", .{
        .target = b.standardTargetOptions(.{}),
        .optimize = @as(?std.builtin.OptimizeMode, null),
        .x = b.option(i32, "x", ""),
    });
}

b zig build output:

target = x86_64-windows.win10_cu...win10_cu-gnu
optimize = builtin.OptimizeMode.Debug
x = null

b zig build -Dtarget=aarch64-linux -Doptimize=ReleaseSafe -Dx=1 output:

target = aarch64-linux.4.19...6.11.5-musl
optimize = builtin.OptimizeMode.Debug    
x = 1

Comment on lines 413 to +419
inline for (@typeInfo(@TypeOf(args)).@"struct".fields) |field| {
const v = @field(args, field.name);
const T = @TypeOf(v);
switch (T) {
const maybe_value = @field(args, field.name);
const given, const value = switch (@typeInfo(@TypeOf(maybe_value))) {
.optional => if (maybe_value) |v| .{ true, v } else .{ false, undefined },
else => .{ true, maybe_value },
};
if (given) switch (@TypeOf(value)) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like this is an odd pattern to assign a seperate boolean for the sole purpose of determining whether or not to skip this iteration of the inline loop on the next line.

Could it be replaced with this:

inline for (@typeInfo(@TypeOf(args)).@"struct".fields) |field| blk: {
    const maybe_value = @field(args, field.name);
    const value = switch (@typeInfo(@TypeOf(maybe_value))) {
        .optional => maybe_value orelse break :blk,
        else => maybe_value,
    };
    switch (@TypeOf(value)) {
        ...

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, what should the behavior be when just specifying null instead of something like @as(?std.builtin.OptimizeMode, null) as in your example? With both my suggested change and your current code, I believe this would just be a compile error (null has it's own type; it's not covered by the .optional case). I think it may be better to just treat plain null as not specified to be consistent with optionals.

Example fix:

inline for (@typeInfo(@TypeOf(args)).@"struct".fields) |field| blk: {
    const maybe_value = @field(args, field.name);
    const value = switch (@typeInfo(@TypeOf(maybe_value))) {
        .optional => maybe_value orelse break :blk,
        .null => break :blk,    // <-- Note this added case
        else => maybe_value,
    };
    switch (@TypeOf(value)) {
        ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants