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

Side effects of function analysis should be applied at the call site #2710

Open
tiehuis opened this issue Jun 19, 2019 · 4 comments
Open

Side effects of function analysis should be applied at the call site #2710

tiehuis opened this issue Jun 19, 2019 · 4 comments
Labels
bug Observed behavior contradicts documented or intended behavior frontend Tokenization, parsing, AstGen, Sema, and Liveness.
Milestone

Comments

@tiehuis
Copy link
Member

tiehuis commented Jun 19, 2019

Edit: Original title was "Add compiler error for comptime pointer parameter with side-effects". The correct behavior is described in this comment.


const warn = @import("std").debug.warn;

fn inc(comptime foo: *usize) void {
    foo.* += 1;
}

pub fn main() void {
    comptime var bar: usize = 0;
    inc(&bar);
    warn("{}\n", bar);
}

This outputs 0 which is unexpected behavior to me. I would have expected the inc call would implicitly be called at compile-time.

Explicitly calling the function at comptime resolves this. This isn't a suitable workaround for me unfortunately since I'm actually passing varargs as well which do not work at comptime.

const warn = @import("std").debug.warn;

fn inc(comptime foo: *usize) void {
    foo.* += 1;
}

pub fn main() void {
    comptime var bar: usize = 0;
    comptime inc(&bar);
    warn("{}\n", bar);
}

I presume the compiler doesn't see that inc has side-effects for its input, and the runtime call simply becomes an empty function after it is resolved. Thinking a bit more, I think the existing behavior is correct, and it is more the lack of a compiler error which is the issue.

A workaround is to avoid comptime pointer parameters and return the values as needed. This correctly catches the issue with a compiler error.

const warn = @import("std").debug.warn;

fn inc(comptime foo: usize) usize {
    return foo + 1;
}

pub fn main() void {
    comptime var bar: usize = 0;
    bar = inc(bar);
    warn("{}\n", bar);
}
$ zig run t.zig 
/tmp/t.zig:9:9: error: cannot store runtime value in compile time variable
    bar = inc(bar);
        ^

May be fundamentally the same as #906.

@tiehuis
Copy link
Member Author

tiehuis commented Jun 19, 2019

I note above that this wasn't a workaround for me, but I can restructure to avoid comptime varargs and thus work around this so this shouldn't block me on anything.

@andrewrk andrewrk added this to the 0.5.0 milestone Jun 27, 2019
@andrewrk andrewrk added the bug Observed behavior contradicts documented or intended behavior label Jun 27, 2019
@andrewrk andrewrk modified the milestones: 0.5.0, 0.6.0 Sep 11, 2019
@andrewrk andrewrk modified the milestones: 0.6.0, 0.7.0 Jan 8, 2020
@ghost
Copy link

ghost commented Jul 15, 2020

Hang on now. #5675 presents a use case for comptime mutable pointer arguments. I don't think we should be so heavy-handed.

@andrewrk andrewrk added proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. and removed bug Observed behavior contradicts documented or intended behavior labels Oct 30, 2020
@andrewrk andrewrk modified the milestones: 0.7.0, 0.8.0 Oct 30, 2020
@SpexGuy SpexGuy added bug Observed behavior contradicts documented or intended behavior and removed proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. labels May 13, 2021
@SpexGuy
Copy link
Contributor

SpexGuy commented May 13, 2021

We discussed this in a design meeting and decided that, anywhere observable, functions which mutate comptime state through pointers should behave as if they were analyzed at the call site. So in the following example:

const warn = @import("std").debug.warn;

fn inc(comptime foo: *usize) void {
    warn("{}\n", foo.*);
    foo.* += 1;
}

pub fn main() void {
    comptime var bar: usize = 0;
    inc(&bar);
    inc(&bar);
    bar += 1;
    warn("{}\n", bar);
}

The first call to inc is analyzed immediately, generates code to print 0, and increments bar to 1. It isn't cached because it modifies a comptime var.
The second call to inc is also analyzed immediately, generates code to print 1, and increments bar to 2. Again, it's not cached.
Finally, bar is incremented within main and becomes 3. Then code for the last warn is generated to print 3.

So in short, it behaves the way you wanted it to when you first wrote the code.

@andrewrk
Copy link
Member

functions which mutate comptime state through pointers

I want to point out that the language spec already requires this concept, in order to detect when to not memoize comptime function calls. Memoizing comptime function calls is necessary for functions that return types to work properly, e.g. ArrayList(i32). Not memoizing comptime function calls that mutate comptime state is then required to uphold other properties of the language. Anyway long story short, this solution is not inventing a special concept just to tackle the problem; it is re-using an existing concept to also address this.

@SpexGuy SpexGuy changed the title Add compiler error for comptime pointer parameter with side-effects Side effects of function analysis should be applied at the call site May 13, 2021
@andrewrk andrewrk added the frontend Tokenization, parsing, AstGen, Sema, and Liveness. label May 13, 2021
@andrewrk andrewrk modified the milestones: 0.8.0, 0.8.1 Jun 4, 2021
@andrewrk andrewrk modified the milestones: 0.8.1, 0.9.1 Sep 1, 2021
@andrewrk andrewrk modified the milestones: 0.9.1, 0.9.0, 0.10.0 Nov 20, 2021
@andrewrk andrewrk removed this from the 0.10.0 milestone Aug 23, 2022
@andrewrk andrewrk added this to the 0.11.0 milestone Aug 23, 2022
@Vexu Vexu modified the milestones: 0.11.0, 0.12.0 Apr 23, 2023
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 frontend Tokenization, parsing, AstGen, Sema, and Liveness.
Projects
None yet
Development

No branches or pull requests

4 participants