1

I've been working on landing the implementation of SE-0365. I previously thought implicit self calls were never allowed for weak self captures, but have found that this is actually allowed in non-escaping closures (even when self is optional):

For example, this compiles in Swift 5.7:

class Object {
  func getSelf() -> Object {
    self
  }

  func test() {
    doVoidStuff { [weak self] in
      let getSelf: () -> Object = getSelf // refers to self.getSelf
      let nonoptionalSelf: Object = getSelf()
      let optionalSelf: Object? = self
    }
  }
}

func doVoidStuff(_ closure: () -> Void) { closure() }

This is surprising to me, considering self is optional here so, at a minimum, a reference to a function or property on self should have an optional type, right?

I haven't been able to construct an example where this is unsafe, since I think self is practically speaking always guaranteed to exist for the lifetime of a non-escaping closure (right?). However it seems incorrect to me based on my understanding of the user-facing model of how weak self works.

Does anyone know why this behavior exists? Is this intentional? There aren't any tests for this in the compiler's main test suite (and just a single example in the source compatibility suite) which makes me think this could potentially be a bug.

Even if this is a bug, I don't think we can remove support for it in Swift 5.8 (right?). Would it make sense to remove support for this in Swift 6?

4 Likes
2

For reference, this did not compile in Swift 3.x but does compile in Swift 4.0 and later. I don't see a relevant evolution proposal from this time period, so I'm not sure why this change was made or if it was intentional.

If this is a bug, can we fix it in Swift 6 without running an evolution proposal?

3

Hah, wow, good find. What does the usage of this behavior in the source compatibility suite look like? This behavior seems like a straightforward bug to me, so I'd probably be in favor of deprecating it, either in the next Swift version or the next language mode if it seems like the potential for breakage is too high.

Here's a contrived example that exploits the fact that you can have mutating methods on classes via protocol extensions:

protocol P {
  init()
}

extension P {
  mutating func changeSelf() {
    self = Self()
  }
  mutating func runTest() where Self: AnyObject {
    doVoidStuff { [weak self] in
      print(ObjectIdentifier(self!))
      changeSelf()
      print(self as Any)
    }
  }
}

final class Object: P {
  deinit {
    print("deinit \(ObjectIdentifier(self))")
  }
}

func doVoidStuff(_ closure: () -> Void) { closure() }

var obj = Object()
obj.runTest()

This prints:

ObjectIdentifier(0x0000600001060590)
deinit ObjectIdentifier(0x0000600001060590)
nil

However, I still don't think there's a safety hole here—I expect that when referencing non-weak self implicitly via getSelf the compiler will properly reference count this invisible capture. In fact, we can see this happen with the following modification (using your getSelf method):

mutating func runTest() where Self: AnyObject {
  doVoidStuff { [weak self] in
    let method = getSelf
    print(ObjectIdentifier(self!))
    changeSelf()
    print(self as Any)
    print(method())
  }
}

This prints:

ObjectIdentifier(0x0000600003ea4c20)
Optional(__lldb_expr_20.Object)
__lldb_expr_20.Object
deinit ObjectIdentifier(0x0000600003ea4c20)
2 Likes
4

Here's an example from CareKit (link), which is the only project in the source compatibility suite that does this:

open func didSelectContactView(_ contactView: UIView & OCKContactDisplayable) {
    handleThrowable(method: controller.initiateSystemContactLookup) { [weak self] contactViewController in
        contactViewController.navigationItem.rightBarButtonItem = UIBarButtonItem(barButtonSystemItem: .done, target: self,
                                                                                  action: #selector(dismissViewController))
        let navigationController = UINavigationController(rootViewController: contactViewController)
        present(navigationController, animated: true, completion: nil)
    }
}

The #selector(dismissViewController) and present(navigationController, animated: true, completion: nil) calls both refer to instance methods on self.

According to the behavior described in SE-0365, using implicit self like this on a weak self capture is valid but only after self has been unwrapped and is no longer an optional type.

The existing implementation of SE-0365 actually correctly (imo) rejects this code as invalid. I think the best thing to do here is probably to continue accept this code in the Swift 5 language mode (with a warning), and then reject it as an error in Swift 6. I can try to make this change as a part of landing SE-0365.

On the implementation side of things, I'm not entirely sure it's possible to continue supporting this for non-escaping closures while also supporting the behavior described in SE-0365 for escaping closures. Supporting the new behavior requires changes to AST lookup, which I'm not sure we can make conditional on whether or not the closure is non-escaping (since the closure itself isn't type-checked yet at this point, afaict). Continuing to investigate options here, though.

@Jumhyn -- if this bug blocks landing the implementation of SE-0365, do you think it would make sense to fix this bug in Swift 5.8 instead of Swift 6 (so we can land SE-0365 in Swift 5.8), or do you think we'd need to delay SE-0365 until Swift 6?

edit: good news, I was able to update the implementation of SE-0365 to handle this in Swift 5.8 without breaking existing code (but not allow this in Swift 6). Hopefully we're able to land that soon!

5

Great, glad to hear it. I think warning in Swift 5.8 and erroring in Swift 6 seems like a reasonable approach.

Tangentially, I actually kind of feel like the #selector part here should work? We're not capturing self in that case (I hope? :grimacing:) so the logic around diagnosing implicit self doesn't really make sense to me. But as you note it looks like we reject this for escaping closures and it's easy enough I suppose to write self?.method or Self.method so meh.

2 Likes
6

I wonder if the "doVoidStuff { [weak self] in" part itself should be allowed in case of non escaping closures. It's kind of a lie to begin with.

7

I agree in principle that it could be reasonable for the #selector pattern to work even when self is not unwrapped yet (since the instance member lookup is really just a side-effect of the spelling of this syntax). However, this isn't permitted in Swift 5.7 if the closure is @escaping, and I think it's probably best to be consistent with that.

Agreed -- is there any real reason to use a weak self capture in a non-escaping closure? I wonder if it would make sense to have a warning in this case (or disallow it completely in Swift 6).

8

Yup, agreed that this is totally orthogonal.

1 Like
9

It's not really a lie since as noted above you can contrive a setup where the weak-ness of the capture actually does matter, but from within a non-mutating method that's actually defined on the class in question I can't think of any situation where this would realistically matter... maybe if we got more aggressive about optimizing lifetimes in a way that could cause self to go away before the end of a method?

1 Like