Why does SE-0269 have different rules for inner closures vs inner functions?

Based on the rules defined in SE-0269, this is explicitly not allowed:

class Test {
  func test() {
    doWorkEscaping { [self] in
      doWorkEscaping {
        // Warning: call to method 'test' in closure requires explicit use of 'self' to make capture semantics explicit; this is an error in Swift 6
        test()
      }
    }
  }
}

This rule applies to inner closures, but the proposal doesn't mention whether this applies to inner functions:

class Test {
  func test() {
    doWorkEscaping { [self] in
      func innerFunction() {
        // Currently allowed
        test()
      }
    }
  }
}

This seems like a potential hole, since an inner function and an inner closure are effectively the same, right? It's easy to make the innerFunction escape, which could cause a retain cycle:

class Test {
  func test() {
    doWorkEscaping { [self] in
      func innerFunction() {
        // Currently allowed
        test()
      }

      // innerFunction with an implicit self capture has escaped
      doWorkEscaping(innerFunction)
    }
  }
}

I don't see this discussed anywhere in the proposal, review thread, or pitch thread, so it's possible this is an oversight. Does this seem like something that should be disallowed?

For reference, SE-0365 (which was modeled off of the rules in SE-0269), does not allow you to do this:

class Test {
  func test() {
    doWorkEscaping { [weak self] in
      guard let self else { return }
      func innerFunction() {
        // Error: call to method 'test' in closure requires explicit use of 'self' to make capture semantics explicit
        test()
      }
    }
  }
}
2 Likes

It's been a bit, but as far as I remember this wasn't a deliberate design choice. Seems like it was largely fallout from the fact that this has historically been a check that we applied just to closures rather than all local function contexts—the analogous code just at the level of the method behaves similarly:

class C {
  var x: Int = 0
  func f() -> () -> Void {
    func local() {
      x += 1 // ok!
    }

    return local
  }

  func g() -> () -> Void {
    let local = {
      x += 1 // error
    }
    return local
  }
}

As for whether this should be the case... I agree with your analysis that there isn't much theoretical space between a local function and a closure here. Pragmatically, I expect closures are far more common and so it may not make sense to break this in a new language mode. These errors are, of course, meant to catch the most common failures and not be bulletproof retain cycle protection.

3 Likes

I think it's pretty easy to defend this choice without resorting to source compatibility.

At a technical level, nested functions and closures are basically the same thing. But that doesn't mean they get used in the same way! In practice, I suspect that inner functions are usually called within the function body that declares them, whereas closures are usually passed out to some other API. While you certainly can pass an inner function out or call a closure within the body, that isn't the typical way these features are used.

class Test {
  func test() {
    doWorkEscaping { [self] in
      func innerFunction() {
        test()
      }

      // These uses are common:
      innerFunction()
      doWorkEscaping { test() }

      // These uses are rare:
      { test() }()
      doWorkEscaping(innerFunction)
    }
  }
}

Since the self-capture rule is a heuristic to call out likely cycles, the fact that nested functions are less likely to be used in a cycle-prone way is relevant to the decision about whether to enforce the rule inside them, even if it is technically possible to form a cycle with a nested function too.

3 Likes

I’m not sure I like allowing self-less access in local functions, but I’ll note that function syntax doesn’t have any place to put the capture list that normally allows you to leave out self in the body.

3 Likes

Capture list absence is the crucial difference between inner functions and closures, which explains the current behaviour.

1 Like

I’m not sure it really explains the pre-SE-0269 behavior, because capture lists were immaterial to the implicit self/escaping closure rules—you always had to reference members with self., which is just as applicable to local function declarations as closure expressions. I think Becca’s story is the most reasonable explanation and/or retcon. :slightly_smiling_face:

Note, that if I move the inner function from a closure to a method compiler doesn't complain:

class Foo {
    func execute(_ work: @escaping () -> Void) {}
    func foo() {}

    func test() {
        execute {
            func function() {
                foo() // 🛑 Call to method 'foo' in closure requires explicit use of 'self' to make capture semantics explicit
            }
            function()
        }
    }
}
class Foo {
    func execute(_ work: @escaping () -> Void) {}
    func foo() {}

    func test() {
        func function() {
            foo() // no complaints
        }
        execute {
            function()
        }
    }
}

Looks like another difference between closures and functions.

I think this is the same issue—the error on foo in the first snippet is a result of the implicit reference to self within the closure passed to execute.

Yes... Just from a practical perspective, instead of capturing self via "self.function()" or via "[self] in" in the closure I could just move the inner function from "execute" closure to the outer method "test" (in this example) to make compiler happy (and, perhaps shoot myself in the foot without realising).


Looks we are missing this error:

class Foo {
    func execute(_ work: @escaping () -> Void) {}
    func foo() {}

    func test() {
        func function_that_does_not_require_self() {
        }
        func function_that_requires_self() {
            foo()
        }
        execute {
            function_that_does_not_require_self() // fine
            function_that_requires_self() // "use explicit self capture" error should be here
            // should be an error, but there's no error currently
        }
    }
}

which error in this case could be only resolved via "[self]" in the capture list , obviously not with the "self. function_that_requires_self", as it is not a method.

Edit: OTOH, I can put "[weak self]" in the capture list.... should we have an ability to put a "self" prefix not just before methods, but before local functions that require self?

        // `function_that_requires_self` is not a method
        // it's a local function that depends on self
        execute { [self] in
            function_that_requires_self()
        }
        execute {
            self.function_that_requires_self()
        }
        execute { [weak self] in
            self?.function_that_requires_self()
        }

Edit2: Hmm, it's not just about self:

class Something {
    deinit { print("deinit") }
    func bar() { print("bar") }
}

class Foo {
    func execute(_ work: @escaping () -> Void) {
        DispatchQueue.main.asyncAfter(deadline: .now() + 1) {
            work()
        }
    }

    func test() {
        var something: Something = Something()
        
        func function_that_requires_something() {
            something.bar() // always captured strongly
        }
        execute { [weak something] in
            function_that_requires_something()
            // there's no "self?.function()" analogue for
            // variable called "something" like `something?.function()`
            // besides there could be several such variables, what would 
            // you do? this? (a, b, c, d)?.function()
        }
    }
}

The current behaviour of special treating local functions textually embedded into closures looks like a good compromise.

This discussion makes sense. Thanks all!

If we're ok with intentionally allowing this in [self] closures, what about [weak self] closures?

The intent behind SE-0365 was to match the inner-closure behavior of SE-0269, which makes me think this should be allowed:

func test() {
  doWorkEscaping { [weak self] in
    guard let self else { return }

    func innerFunction() {
      // Currently an error: call to method 'test' in closure requires explicit use of 'self' to make capture semantics explicit
      test()
    }
  }
}

The current situation makes it impossible to re-enable implicit self in the inner function. In an inner-closure you would repeat the [weak self] guard let self pattern, but that isn't an option here since the inner function doesn't have a capture list. So this does seem worth fixing.

Based on the intent behind SE-0365 I think this is just a bug in the implementation which we can fix without any amendments etc. Thoughts?

2 Likes

Even more dangerously, if you move the inner function out of the closure – compiler no longer complains, but your [weak self] is silently turning no-op!

    func test() {
        func innerFunction() {
            bar() // a method of self
        }
        doWorkEscaping { [weak self] in
            guard let self else { return } // self is never nil!
            innerFunction() // we always hit this line
        }
    }

It's very easy to introduce a memory leak this way. Should we address this somehow? E.g. introduce a warning when you calling such local functions-† from a closure? (unless they are local to the closure).

† - also any closure that captures self strongly, it is similar to a local function in this regard:

    func test() {
        let closure = {
            self.bar() // a method of self
        }
        doWorkEscaping { [weak self] in    // no-op!
            guard let self else { return } // no-op!
            closure()
        }
    }

I have mentioned this in the original discussion, Review capture semantics of self but then it focused on closures only.

I still stand on "explicit capture should have an explicit syntax", and prefixing with self. is not it: explicit self. does not indicate captured self.

1 Like