Request for feedback: emitting warnings when weak capture list items cause strong captures

Development Compiler
,
7

a few thoughts on this.

first, during review the logic was changed from a stronger check where it would warn if the weak capture created the only reference to its referent in an outer escaping closure. since the logic was relaxed we will now warn in more scenarios where there may not actually be anything problematic happening. in such cases it can be nice to have an 'escape hatch' where writing something a little unusual signals to the compiler that you 'know what you're doing'. using the weak x = x pattern to communicate this seemed good because:

  1. i think it's fairly unusual in practice, so would 'stand out' a little bit, which could be a useful signal
  2. it is semantically identical to the original code
  3. it was easy to implement a fixit for (maybe not a compelling justification, but still true)

moving or introducing new capture list items however can change behavior more meaningfully. two examples that came up are this somewhat contrived one:

var obj = Obj()
escape { // ℹ️ suggest adding strong capture list item
  obj = Obj()
  escape { [weak obj] in // ⚠️ 
    obj?.doStuff()
  }
}

// if we add the outer strong capture, we've changed semantics:

var obj = Obj()
escape { [obj] in
  obj = Obj() // 🛑 Cannot assign to value: 'obj' is an immutable capture
  escape { [weak obj] in
    obj?.doStuff()
  }
}

and these cases which have weird potential interplay with isolation inference:

actor A {
  func f() {
    Task { // ℹ️ recommend a capture
        // if we add `[weak self]` then the Task become nonisolated
        escape { [weak self] in ... } // ⚠️
    }
  }

  static func paramIsolated(
    isolation anActor: isolated A
  ) {
    Task { // ℹ️ recommend a capture
        // both `[anActor]` & `[weak anActor]` change inference to nonisolated
        escape { [weak anActor] in ... } // ⚠️
    }
  }
}

probably in most cases the thing to do is still either hoist the capture, or add a new one in an outer scope. it's a future enhancement we could make to add fixits to offer those options.

2 Likes