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

argument of inline fn no longer propagates comptime-ness to result type expression #22538

Open
rohlem opened this issue Jan 18, 2025 · 7 comments · May be fixed by #22547
Open

argument of inline fn no longer propagates comptime-ness to result type expression #22538

rohlem opened this issue Jan 18, 2025 · 7 comments · May be fixed by #22547
Labels
bug Observed behavior contradicts documented or intended behavior regression It worked in a previous version of Zig, but stopped working.
Milestone

Comments

@rohlem
Copy link
Contributor

rohlem commented Jan 18, 2025

EDIT: #22537 might fix this.

Until I've verified that it actually doesn't, this might not really be worth reading tbh.

Zig Version

0.14.0-dev.2837+f38d7a92c

Steps to Reproduce and Observed Behavior

So to keep it short, there exists a hack mechanism to inspect comptime-ness of arguments of inline fn by constructing a .{v} tuple (and inspecting the comptime-ness of its field via @typeInfo).
I'm not quite confident whether this is a supported use case, but the behavior here changed since 0.14.0-dev.2628+5b5c60f43,
and the new behavior is inconsistent, so I thought I'd report it and let maintainers decide what should happen.
Reproduction for zig test .zig:

fn E(comptime flag: bool) type {
    if (flag) return enum { @"comptime" };
    return enum { runtime };
}

inline fn isComptimeKnown(comptime ValueTuple: type) bool {
    return @typeInfo(ValueTuple).@"struct".fields[0].is_comptime;
}
inline fn f(value: u1) E(isComptimeKnown(@TypeOf(.{value}))) {
    if (isComptimeKnown(@TypeOf(.{value}))) {
        return .@"comptime"; //compile error: type mismatch (=> @TypeOf(.{value}) in function body != @TypeOf(.{value}) in result type expression)
    }
    return .runtime;
}

test "inline fn" {
    const x1: u1 = 0;
    var y1: u1 = 0;
    _ = &y1;
    if (f(comptime x1) != .@"comptime") unreachable; //causes compile error in 0.14.0-dev.2837+f38d7a92c, not in 0.14.0-dev.2628+5b5c60f43
    if (f(y1) != .runtime) unreachable;
}

test "inlined by hand" { // allowed by both 0.14.0-dev.2837+f38d7a92c and 0.14.0-dev.2628+5b5c60f43
    const x1: u1 = 0;
    const comptime_x1 = comptime x1;
    const f_result = @as(E(isComptimeKnown(@TypeOf(.{comptime_x1}))), f_result: {
        if (isComptimeKnown(@TypeOf(.{comptime_x1}))) {
            break :f_result .@"comptime"; //compile error: type mismatch (=> @TypeOf(.{value}) in function body != @TypeOf(.{value}) in result type expression)
        }
        break :f_result .runtime;
    });
    if (f_result != .@"comptime") unreachable;
}

Zig version 0.14.0-dev.2628+5b5c60f43 passes both tests.
Zig version 0.14.0-dev.2837+f38d7a92c succeeds test "inlined by hand" on its own, but fails compiling test "inline fn" as outlined in the comments. Compile error output:

.zig:11:17: error: enum '.zig.E(false)' has no member named 'comptime'
        return .@"comptime"; //compile error: type mismatch (=> @TypeOf(.{value}) in function body != @TypeOf(.{value}) in result type expression)
               ~^~~~~~~~~~~
.zig:3:12: note: enum declared here
    return enum { runtime };
           ^~~~~~~~~~~~~~~~
.zig:20:10: note: called from here
    if (f(comptime x1) != .@"comptime") unreachable; //causes compile error in 0.14.0-dev.2837+f38d7a92c, not in 0.14.0-dev.2628+5b5c60f43
        ~^~~~~~~~~~~~~

Expected Behavior

The primary inconsistency here is that the type constructed by .{value} always treats value as runtime-known in the return type expression, while it doesn't in the function body.
Not propagating comptime-ness means that the inline fn behaves semantically different from manually inlining its code.
(I thought I remembered a comment somewhere, I think from mlugg, basically saying that if I find such behavior I should report it as a bug - couldn't find it anymore so maybe I'm misremembering the details.)

If those are indeed the intended semantics, I'd have to instead use inline parameters once/if they're added (#7772).
Either way I believe code in the return type expression should not construct tuples differently than in the function body.

I assume this may be related in some way to #22494, which posits that the change was introduced in #22414. While a comment there mentions a change in behavior, that seems only related to comptime-only types, not inline fn arguments of all types.

@rohlem rohlem added the bug Observed behavior contradicts documented or intended behavior label Jan 18, 2025
@rohlem
Copy link
Contributor Author

rohlem commented Jan 18, 2025

Looks like #22537 was opened while I was filing this issue - judging from the title that might be an exact fix for this as well!

@castholm
Copy link
Contributor

@rohlem I tried running your repro using a compiler built from #22537 and it still failed. #22537 fixes it so that comptime-known arguments are allowed to be evaluated at comptime, but it does not appear to propagate the comptime-knownness of the arguments when evaluating the generic expressions. Here's a different repro that more directly demonstrates what is happening:

const std = @import("std");

pub fn main() void {
    const a: u1 = 0;
    std.debug.print("expect comptime:\n", .{});
    const result1 = foo(a, @enumFromInt(0));
    std.debug.print(" result:    {}\n", .{result1});

    var b: u1 = 0;
    _ = &b;
    std.debug.print("expect runtime:\n", .{});
    const result2 = foo(b, @enumFromInt(0));
    std.debug.print(" result:    {}\n", .{result2});
}

inline fn foo(
    x: u1,
    y: E(@typeInfo(@TypeOf(.{x})).@"struct".fields[0].is_comptime),
) E(@typeInfo(@TypeOf(.{x})).@"struct".fields[0].is_comptime) {
    const x_in_body: E(@typeInfo(@TypeOf(.{x})).@"struct".fields[0].is_comptime) = @enumFromInt(x);
    std.debug.print(" x_in_body: {}\n", .{x_in_body});
    std.debug.print(" y:         {}\n", .{y});
    return @enumFromInt(x);
}

fn E(comptime flag: bool) type {
    return if (flag) EC else ER;
}

const EC = enum { @"comptime" };
const ER = enum { runtime };

On 0.14.0-dev.2837+f38d7a92c this prints:

expect comptime:
 x_in_body: repro.EC.comptime
 y:         repro.ER.runtime 
 result:    repro.ER.runtime 
expect runtime:
 x_in_body: repro.ER.runtime 
 y:         repro.ER.runtime 
 result:    repro.ER.runtime 

On 0.13.0 it prints:

expect comptime:
 x_in_body: repro.EC.comptime
 y:         repro.ER.runtime
 result:    repro.EC.comptime
expect runtime:
 x_in_body: repro.ER.runtime
 y:         repro.ER.runtime
 result:    repro.ER.runtime

@mlugg
Copy link
Member

mlugg commented Jan 19, 2025

#22537 definitely should be fixing this. I'll take a look in a moment.

@mlugg
Copy link
Member

mlugg commented Jan 19, 2025

Agh, this is stupid shit related to how the param instruction works. This might take me a couple of days to figure out.

@mlugg
Copy link
Member

mlugg commented Jan 19, 2025

okay i got the green light from Andrew to make an internally-significant-but-only-mildly-breaking language change which will restore this functionality without making the compiler code stupid. will hopefully sort it tomorrow

@rohlem
Copy link
Contributor Author

rohlem commented Jan 19, 2025

Awesome, thanks so much to both of you! ❤

@castholm
Copy link
Contributor

For posterity: The problem appears to be that the function is not considered generic under the current language rules. If we make it generic, we get the expected results with a compiler built from #22537:

diff --git a/repro_old.zig b/repro.zig
index 06fda7ab4a..ada4182ada 100644
--- a/repro_old.zig
+++ b/repro.zig
@@ -3,18 +3,19 @@ const std = @import("std");
 pub fn main() void {
     const a: u1 = 0;
     std.debug.print("expect comptime:\n", .{});
-    const result1 = foo(a, @enumFromInt(0));
+    const result1 = foo(u1, a, @enumFromInt(0));
     std.debug.print(" result:    {}\n", .{result1});
 
     var b: u1 = 0;
     _ = &b;
     std.debug.print("expect runtime:\n", .{});
-    const result2 = foo(b, @enumFromInt(0));
+    const result2 = foo(u1, b, @enumFromInt(0));
     std.debug.print(" result:    {}\n", .{result2});
 }
 
 inline fn foo(
-    x: u1,
+    comptime T: type,
+    x: T,
     y: E(@typeInfo(@TypeOf(.{x})).@"struct".fields[0].is_comptime),
 ) E(@typeInfo(@TypeOf(.{x})).@"struct".fields[0].is_comptime) {
     const x_in_body: E(@typeInfo(@TypeOf(.{x})).@"struct".fields[0].is_comptime) = @enumFromInt(x);
expect comptime:
 x_in_body: repro.EC.comptime
 y:         repro.EC.comptime
 result:    repro.EC.comptime
expect runtime:
 x_in_body: repro.ER.runtime 
 y:         repro.ER.runtime 
 result:    repro.ER.runtime 

@mlugg mlugg added this to the 0.14.0 milestone Jan 19, 2025
@mlugg mlugg added the regression It worked in a previous version of Zig, but stopped working. label Jan 19, 2025
mlugg added a commit to mlugg/zig that referenced this issue Jan 19, 2025
The original motivation here was to fix regressions caused by ziglang#22414.
However, while working on this, I ended up discussing a language
simplification with Andrew, which changes things a little from how they
worked before ziglang#22414.

The main user-facing change here is that any reference to a prior
function parameter, even if potentially comptime-known at the usage
site or even not analyzed, now makes a function generic. This applies
even if the parameter being referenced is not a `comptime` parameter,
since it could still be populated when performing an inline call. This
is a breaking language change.

The detection of this is done in AstGen; when evaluating a parameter
type or return type, we track whether it referenced any prior parameter,
and if so, we mark this type as being "generic" in ZIR. This will cause
Sema to not evaluate it until the time of instantiation or inline call.

A lovely consequence of this from an implementation perspective is that
it eliminates the need for most of the "generic poison" system. In
particular, `error.GenericPoison` is now completely unnecessary, because
we identify generic expressions earlier in the pipeline; this simplifies
the compiler and avoids redundant work. This also entirely eliminates
the concept of the "generic poison value". The only remnant of this
system is the "generic poison type" (`Type.generic_poison` and
`InternPool.Index.generic_poison_type`). This type is used in two
places:

* During semantic analysis, to represent an unknown result type.
* When storing generic function types, to represent a generic parameter/return type.

It's possible that these use cases should instead use `.none`, but I
leave that investigation to a future adventurer.

One last thing. Prior to ziglang#22414, inline calls were a little inefficient,
because they re-evaluated even non-generic parameter types whenever they
were called. Changing this behavior is what ultimately led to ziglang#22538.
Well, because the new logic will mark a type expression as generic if
there is any change its resolved type could differ in an inline call,
this redundant work is unnecessary! So, this is another way in which the
new design reduces redundant work and complexity.

Resolves: ziglang#22494
Resolves: ziglang#22532
Resolves: ziglang#22538
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 regression It worked in a previous version of Zig, but stopped working.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants