Skip to content

Using struct method as function argument causes compiler to fail #4022

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

Closed
cshenton opened this issue Jan 1, 2020 · 5 comments · Fixed by #4177
Closed

Using struct method as function argument causes compiler to fail #4022

cshenton opened this issue Jan 1, 2020 · 5 comments · Fixed by #4177
Labels
bug Observed behavior contradicts documented or intended behavior stage1 The process of building from source via WebAssembly and the C backend.
Milestone

Comments

@cshenton
Copy link
Contributor

cshenton commented Jan 1, 2020

The following simple "apply" function works for a top level declaration but not for a method. Resulting in a compiler error:

const std = @import("std");

pub fn apply(func: var, arr: []f32) void {
    for (arr) |*el| {
        el.* = func(el.*);
    }
}

pub fn double(x: f32) f32 {
    return 2*x;
}

const TimesBy = struct {
    x: f32,

    pub fn func(self: TimesBy, y: f32) f32 {
        return self.x * y;
    }
};

// This works fine
test "plain function arguments" {
    var vals = [_]f32{1, 2, 3, 4, 5};
    apply(double, vals[0..]);

    std.testing.expect(vals[0] == 2);
    std.testing.expect(vals[1] == 4);
    std.testing.expect(vals[2] == 6);
    std.testing.expect(vals[3] == 8);
    std.testing.expect(vals[4] == 10);
}

// This errors the compiler
test "functor arguments" {
    var vals = [_]f32{1, 2, 3, 4, 5};
    const functor = TimesBy{ .x = 3 };
    apply(functor.func, vals[0..]);

    std.testing.expect(vals[0] == 3);
    std.testing.expect(vals[1] == 6);
    std.testing.expect(vals[2] == 9);
    std.testing.expect(vals[3] == 12);
    std.testing.expect(vals[4] == 15);
}

The output is:

$ zig test scratch.zig
Unreachable at D:\a\1\s\src\analyze.cpp:5241 in hash_const_val. This is a bug in the Zig compiler.
Unable to dump stack trace: debug info stripped

I'm sure this is an indication I'm doing things in an unintended way, what is the correct way to implement this sort of behaviour in zig currently?

@Vexu
Copy link
Member

Vexu commented Jan 1, 2020

functor.func is a bound function the correct way to implement this behavior is to pass TimesBy.func and functor as separate arguments. Look at std.fmt as an example

zig/lib/std/fmt.zig

Lines 89 to 95 in 0fb3e66

pub fn format(
context: var,
comptime Errors: type,
output: fn (@TypeOf(context), []const u8) Errors!void,
comptime fmt: []const u8,
args: var,
) Errors!void {

Passing a bound function as a generic parameter should likely be an error.

@cshenton
Copy link
Contributor Author

cshenton commented Jan 1, 2020

There's a lot of other things going on there, so I'm having trouble parsing it. What would it look like in my example?

For the situations where the functor state is both const and comptime, would the following be considered idiomatic? (admittedly it would be a bit nicer to just return a freestanding function from this method that a type with no fields and a single method, is there a way to do that?)

pub fn TimesBy(comptime val: f32) type {
    return struct {
        const x = val;

        pub fn func(y: f32) f32 {
            return x * y;
        }
    };
}

test "functor arguments" {
    var vals = [_]f32{1, 2, 3, 4, 5};
    apply(TimesBy(3).func, vals[0..]);

    std.testing.expect(vals[0] == 3);
    std.testing.expect(vals[1] == 6);
    std.testing.expect(vals[2] == 9);
    std.testing.expect(vals[3] == 12);
    std.testing.expect(vals[4] == 15);
}

edit: this compiles and passes btw

@daurnimator
Copy link
Contributor

For the situations where the functor state is both const and comptime, would the following be considered idiomatic?

pub fn TimesBy(comptime val: f32) fn(f32) f32 {
    return struct {
        pub fn func(y: f32) f32 {
            return val * y;
        }
    }.func;
}

Once #1717 is done you won't need the struct.

@Vexu
Copy link
Member

Vexu commented Jan 1, 2020

What would it look like in my example?

const std = @import("std");

pub fn apply(func: var, ctx: var, arr: []f32) void {
    for (arr) |*el| {
        el.* = func(ctx, el.*);
    }
}

pub fn double(_: void, x: f32) f32 {
    return 2*x;
}

const TimesBy = struct {
    x: f32,

    pub fn func(self: TimesBy, y: f32) f32 {
        return self.x * y;
    }
};

test "plain function arguments" {
    var vals = [_]f32{1, 2, 3, 4, 5};
    apply(double, {}, vals[0..]);

    std.testing.expect(vals[0] == 2);
    std.testing.expect(vals[1] == 4);
    std.testing.expect(vals[2] == 6);
    std.testing.expect(vals[3] == 8);
    std.testing.expect(vals[4] == 10);
}

test "functor arguments" {
    var vals = [_]f32{1, 2, 3, 4, 5};
    const functor = TimesBy{ .x = 3 };
    apply(TimesBy.func, functor, vals[0..]);

    std.testing.expect(vals[0] == 3);
    std.testing.expect(vals[1] == 6);
    std.testing.expect(vals[2] == 9);
    std.testing.expect(vals[3] == 12);
    std.testing.expect(vals[4] == 15);
}

@cshenton
Copy link
Contributor Author

cshenton commented Jan 1, 2020

Ah, I wasn't aware of #1717, that's great. As for the context pattern, for the sake of a nicer API I'll probably just implement apply with a type switch that will just call the function or some well known method (like eval) if an instance of a struct is passed. Like:

const std = @import("std");

pub fn apply(functor: var, arr: []f32) void {
    switch (@typeInfo(@TypeOf(functor))) {
        .Fn => {
            for (arr) |*el| {
                el.* = functor(el.*);
            }
        },
        else => {
            for (arr) |*el| {
                el.* = functor.eval(el.*);
            }
        }
    }
}

const TimesBy = struct {
    x: f32,

    pub fn eval(self: TimesBy, y: f32) f32 {
        return self.x * y;
    }
};

// This errors the compiler
test "functor arguments" {
    var vals = [_]f32{1, 2, 3, 4, 5};
    const functor = TimesBy{ .x = 3 };
    apply(functor, vals[0..]);

    std.testing.expect(vals[0] == 3);
    std.testing.expect(vals[1] == 6);
    std.testing.expect(vals[2] == 9);
    std.testing.expect(vals[3] == 12);
    std.testing.expect(vals[4] == 15);
}

This works and is not too different from the user's perspective than operator(), it's just some well known method.

@andrewrk andrewrk added bug Observed behavior contradicts documented or intended behavior stage1 The process of building from source via WebAssembly and the C backend. labels Jan 2, 2020
@andrewrk andrewrk added this to the 0.7.0 milestone Jan 2, 2020
LemonBoy added a commit to LemonBoy/zig that referenced this issue Jan 14, 2020
andrewrk pushed a commit that referenced this issue Jan 14, 2020
@andrewrk andrewrk modified the milestones: 0.7.0, 0.6.0 Feb 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Observed behavior contradicts documented or intended behavior stage1 The process of building from source via WebAssembly and the C backend.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants