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 closure over comptime var a compile error; comptime vars become immutable when they go out of scope #7396

Open
SpexGuy opened this issue Dec 11, 2020 · 14 comments
Labels
accepted This proposal is planned. breaking Implementing this issue could cause existing code to no longer compile or have different behavior. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Milestone

Comments

@SpexGuy
Copy link
Contributor

SpexGuy commented Dec 11, 2020

This issue is heavily inspired by #5895 and #5578.

As it stands right now, the compiler allows you to create a closure over mutable comptime state. This allows some unique abilities, like building a borrow checker and lazily creating a global list based on what functions or types get compiled.

But it also causes a lot of problems:

  • it wreaks havoc with the comptime memoization system.
  • it makes comptime execution order observable.
  • it prevents incremental compilation.

The aim of this issue is to provide a solid bedrock for how comptime var should behave, without introducing new features. There may be extensions that add new features in future proposals.

After discussing in the design meeting, this is how we believe comptime var (and comptime mutable memory in general) should work:

  • closure over comptime var is a compile error.
  • closure over const pointer to mutable memory (e.g. comptime const x: *u32 = ...) is allowed
  • when mutable pointers are written at comptime, the compiler ensures that the underlying memory is still mutable
  • the memory underlying comptime vars is marked immutable when the vars go out of scope.
  • if the compiler attempts to generate runtime code which dereferences a mutable pointer to a comptime var, that’s also a compile error.
  • for the purposes of memoization, any values which are closed over are treated like additional parameters.

This means that the example given in #5578 can be implemented like this:

const GiveCtState = struct {
    times: usize,
};
fn Give(comptime T: type, comptime state: *GiveCtState) type {
    return struct {
        val: T,
        pub fn init(_val: T) @This() {
            return Self{ .val = _val };
        }

        pub fn give(self: *@This()) T {
            comptime {
                if (state.times == 0)
                    @compileError("You can only 'give' 2 times");

                state.times -= 1;
            }
            return self.val;
        }
    };
}

test "Test 1" {
    // create comptime state in this scope
    comptime var track_a = GiveTracker{ .times = 2 };
    // create a type that closes over that state
    var a = Give(usize, &track_a).init(1);
    _ = a.give();
    _ = a.give(); // not memoized because it closes over &track_a
    _ = a.give(); // <-- Should Fail

    comptime var track_b = GiveTracker{ .times = 2 };
    var b = Give(usize, &track_b).init(1); // not memoized because it accepts &track_b
    _ = b.give();
    _ = b.give();
    _ = b.give(); // <-- Should Fail
}

The other use case, building a global index lazily based on what gets compiled, is not officially supported by Zig. The recommended alternative way to approach this problem is to use comptime code to create an index explicitly, either from a hardcoded list, or by calling functions in submodules that will return the needed parts of the index for each submodule, and aggregating those results into a single global index.

@SpexGuy SpexGuy added breaking Implementing this issue could cause existing code to no longer compile or have different behavior. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. accepted This proposal is planned. labels Dec 11, 2020
@ghost
Copy link

ghost commented Dec 12, 2020

#5895 is now appropriately divided, as per recommendation.

@andrewrk andrewrk added this to the 0.8.0 milestone Dec 13, 2020
@andrewrk andrewrk changed the title Design flaw: Closure over comptime var make closure over comptime var a compile error; comptime vars become immutable when they go out of scope Mar 28, 2021
@andrewrk andrewrk modified the milestones: 0.8.0, 0.9.0 May 19, 2021
@SpexGuy
Copy link
Contributor Author

SpexGuy commented Jun 1, 2021

Adding links to some related issues:
#1470: interactions between comptime var and runtime branches
#2710: interactions between comptime var and runtime function calls
#7948: constant deduplication for memoization

@topolarity
Copy link
Contributor

Out of curiosity, what does this change mean for examples like this?

test {
    comptime var i: usize = 0;
    const f = struct {
        fn foo(x: usize) bool {
            return x == i;
        }
    }.foo;
    try expect(f(0)); // Currently fails because of `i=1` on the next line
    i = 1;
}

Would this count as closure (by reference) over i, as it does today? Or will it capture by value?

@ghost
Copy link

ghost commented Oct 12, 2022

@topolarity That will fail to compile. No closure, no capture, error. That's the point.

@topolarity
Copy link
Contributor

@topolarity That will fail to compile. No closure, no capture, error. That's the point.

My question is whether it will be capture by value, so that it is not an error and the test passes. Or if it will be captured by reference, so that this code does not compile.

@ghost
Copy link

ghost commented Oct 12, 2022

My understanding is that your example should compile and pass:

test {
    comptime var i: usize = 0;
    const f = struct {
        fn foo(x: usize) bool {
            return x == i;
        }
    }.foo;
    try expect(f(0)); // f(0) desugars to f(i,0) as per rule 6
    i = 1; // no problems
} // i becomes const, if it is still referenced somewhere (rule 4)

@ghost
Copy link

ghost commented Oct 12, 2022

@topolarity @zzyxyzz Think of it like this: a function in function scope is a new scope, which cannot see any locals from the enclosing function. Since i is local to test, it is not visible to foo; in effect, the name is undefined inside it (though the reported error is different). If foo wants to use i, it needs to take it as an argument (by value or by reference), or test needs to assign its value to a const (comptime consts being the only locals visible to inner functions).

The point of this change is to avoid exactly this kind of question; however this works in a particular case is explicitly written. This is a problem for the compiler too, as currently it can evaluate things out of order and produce confusing results (as in your example). I was in the design meeting where this was decided and it was my proposal that was eventually shaped into this, so you can trust what I tell you about it.

@ghost
Copy link

ghost commented Oct 12, 2022

@EleanorNB
Could you comment on why closing over const pointers to mutable comptime variables is still permitted? It seems to me that this proposal could be simplified considerably if there was a blanket ban on referring to outer-scope comptime mutable state in any way within local functions.

@ghost
Copy link

ghost commented Oct 12, 2022

@zzyxyzz On the contrary: detecting that a pointer is to mutable state and banning specifically that is another layer of complication and another rule exception to remember. It's more complicated. (You might think this is detectable by the signature alone, but regular functions can be evaluated at comptime; whether a pointer is to comptime state or runtime data is only fully determinable by looking at the surrounding context.)

@ghost
Copy link

ghost commented Oct 12, 2022

Zig's comptime semantics are an incredible mess.

@ghost
Copy link

ghost commented Oct 12, 2022

Yep. That's why this exists.

@topolarity
Copy link
Contributor

Banning closure (by-reference) over a comptime var is not essential to this proposal, right?

You can still achieve the same thing by closing over a ptr to its contents:

test {
    comptime var i: usize = 0;
    const ptr: *const usize = &i;
    const f = struct {
        fn foo(x: usize) bool {
            return x == ptr.*;
        }
    }.foo;
    try expect(f(0)); // Should this pass?
    i = 1;
    try expect(f(1)); // Should this fail?
}

If I understand the proposal correctly, I believe the first expect would pass, and the second would fail.

That is, unless the first was commented out, in which case the second would suddenly pass.

This strange behavior is due to #7948, of course.

@ghost
Copy link

ghost commented Oct 13, 2022

@topolarity You sort of have it. That kind of closure by reference will be allowed, but it would play out like this:

test {
    comptime var i: usize = 0; // in scope throughout `test`
    const ptr: *const usize = &i; // can be closed over as it is constant
    const f = struct {
        fn foo(x: usize) bool { // cannot see `i`, but can see `ptr`
            return x == ptr.*; // references the current value of `i`
        }
    }.foo;
    try expect(f(0)); // will pass as `i` is 0 here
    i = 1;
    try expect(f(1)); // will also pass as `i` is now 1,
                      // and `ptr` tracks the current value just as at runtime
}

comptime vars continue to be mutable in the scope they're defined in, and any dynamically enclosed scope, regardless of depth or whether they're referenced or mutated in those inner scopes. Dereferencing a pointer, even an immutable pointer, to a comptime var returns the current value of it, regardless of where it's dereferenced, at arbitrary depth or even outside the defining scope. The only restriction is that a mutable pointer can't mutate a value that's out of scope; it can still be dereferenced, just not assigned. So here, since i is mutated between calls to f, the dereference of ptr within the function will produce different values, just as a runtime pointer dereference within a function will produce different values on different invocations.

(Btw: attempting out-of-scope mutation is a compile error, not a silent failure, just like attempting to mutate an immutable value. Or, it will be, once this is implemented. I'm not familiar with #7948, but from a glance it is a bug and will be fixed.)

@topolarity
Copy link
Contributor

topolarity commented Oct 13, 2022

@EleanorNB Great, thanks for the clear explanation.

I agree that behavior would be correct. However, I'm not sure we've achieved it yet

The gist of #7948 is that pointers are compared shallow-ly for the purposes of comptime memoization. In the original issue, that meant functions were re-analyzed too often, despite the referenced value being the same between invocations with different pointers. In this case, functions are re-analyzed too little, even when the referenced value has changed.

As your explanation demonstrates, this is unsound with respect to the runtime semantics - it means that f(0) reports the same value throughout the entire program.

I think we might need to fix memoization before this will work properly

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted This proposal is planned. breaking Implementing this issue could cause existing code to no longer compile or have different behavior. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Projects
None yet
Development

No branches or pull requests

3 participants