[swift-dev] Question about implementation of closure capture lists

[Chris Lattner]

> On Dec 9, 2015, at 8:57 AM, Greg Titus via swift-dev <swift-dev at swift.org> wrote:
> 
> Hi all,
> 
> I thought I’d take a look at SR-153 "Bad fix suggestion for changing value of capture list constants" <https://bugs.swift.org/browse/SR-153>, and I’d really appreciate it if someone more familiar with the code could check my thoughts, rather than me jumping straight to submitting a pull request that might be going in the wrong direction.
> 
> First, an entry in a capture list should always be semantically a constant, correct? Right now, the VarDecl ‘isLet’ flag is being set to ownershipKind != Ownership::Weak, which means that a capture of [a] is treated as constant by the type checker, but a capture of [weak a] is not. Thus, this code compiles without error (and prints “nil” and “A" when run), which is certainly unexpected behavior to me:
> 
> class A {}
> var a = A()
> let f = {
>    [weak a] in
>    a = A()
>    print(a)
> }
> f()
> print(a)

Yes, this is not the right behavior.  We have to do this, because weak values can actually be mutated at runtime (by the pointee being deallocated, and thus dropping to nil).

The issue here is that (at a pretty deep level) the compiler has conflated two different concepts into the single “isLet” bit: 1) can be mutated at runtime, and 2) can be assigned to.  Two examples that violate this: "lazy lets” and “weak lets”.  Both of these things can be mutated at the “bits” level, but neither can be assigned to within their current scope.

> I can imagine there being a good reason why a weak var shouldn’t be marked as isLet for optimization purposes, etc, since you can’t assume that it won’t change, but surely the programmer shouldn’t be explicitly using it as an lvalue, right?

Right.

> I think the solution here is to add a VarDecl flag bit for InClosureCaptureList, set that when setting up the VarDecl in ParseExpr, and then return false from isSettable() (to fix the issue shown here), and early return from emitLetToVarNoteIfSimple() (to fix the original issue 153). These would be similar and in the same places as the existing isa<ParamDecl> checks.

This isn’t specific to capture lists.  I’d rather see the isLet bit generalized in one of two ways.  Either:

1) Turn it into a three state enum, capturing the ideas of “var” and “let, but can change at runtime” and “let”.
2) Separate these two concepts into two bools.

-Chris