"Instance will be immediately deallocated" warning in Xcode when using weak property

Using Swift
1

following code worked fine with Xcode 9 and swift 4.2:

class MyViewController : UIViewController {

	weak var vc: UIViewController?

    override func loadView() {
		vc = UIViewController()
		navigationController?.pushViewController(vc!, animated: true)
    }
}

Instance will be immediately deallocated because property 'vc' is 'weak'

Is the warning valid and can instance of UIViewController be indeed deallocated between those two line or is it just a case of xcode/swift being confused?

2

Imho the warning is valid (actually, I'm pretty sure here ;-)
Just introduce a local variable, which will not only silence the warning, but also make force unwrapping superfluous.

3

That's exactly how I fixed it, but I am still curious if it is valid. It seems strange to me that compiler wouldn't see that it is being used right away and wouldn't retain it.

Also, did the behaviour changed between swift 4.1 and 4.2 or is just the warning new and it could in fact happen in previous swift version and we just weren't warned?

Code just like this was used in production for almost 2 years and I don't remember having any issues (but maybe it is very rare and we just got lucky)

4

The warning is valid. While it's unlikely the compiler would insert a release before the end of the function, it has always been allowed to.

2 Likes
5

Well, where would you draw the line?
Is it ok to use a weak variable in the next line, ten line below, two screens below, as long as it's the same method?
It's much more simple to say that deallocation happens when there are no strong references left, even if that's not always true.

In your case, UIKit might have some influence on what is happening - you aren't dealing with true Swift objects here, so vc might simply be autoreleased.
I just created a simple example in pure Swift where the force unwrapping actually fails.

6

I would personally draw line at the end of scope, but I am far from being able to even contribute to swift development, so there is a lot I don't know :).

If doing experiment in pure swift indeed crashes force unwrap, then there is nothing else to add. I should have tried that myself. Thanks for the confirmation!

7

To bring a little more context, there's no way* the compiler can "just do what you mean" here. After all, right after you assign to vc, you access navigationController. That's a property, which means you could override it to do something like this:

override var navigationController: UINavigationController? {
  self.vc = SomeOtherViewController() // Muahaha!
  return super.navigationController
}

…which means that the second time you access vc, it needs to be read out of self again, because it might have changed. In this case, that's not very useful behavior, but for some other property it could be important.

* Okay, yes, the compiler could in this case verify that you haven't done this unusual thing, but language rules are easier to follow when they don't change based on what you might have defined elsewhere.

4 Likes
8

The more I think about it, the more I think the error message is incorrect.

It is correct to reason that there's nothing in this scope:

  override func loadView() {
      vc = UIViewController() 
      // ❶
      navigationController?.pushViewController(vc!, animated: true)
  }

to keep the UIViewController object alive past the point marked "❶". The assumption that deallocation somehow can't happen there is based on knowledge of implementation details.

However, the fact that vc doesn't (or wouldn't) contain an owning reference doesn't mean that an owning reference doesn't exist elsewhere.

I have often, for example, used a singleton view controller pattern, where the initializer puts an owning reference to itself in a static property. If that behavior were part of a public API contract, storing the result of the initializer in a weak property might be a meaningful thing to do.

9

But how should the warning look like?
I'm not even sure if it's feasible for the compiler to detect that the rhs of the assignment is a singleton...

10

The ability of an initialiser to keep the newly constructed instance retained by e.g assigning it to a global or static variable was raised during the implementation of the immediate deallocation diagnostic, but it wasn't considered a common enough pattern to limit the diagnostic to initialisers where we can prove that the instance doesn't escape. As stated in the linked PR, you can use an immediately evaluated closure or a temporary strong variable in order to silence the warning if you so wish.

Though IMO the use of an initialiser to escape a newly constructed instance is an anti-pattern, you should use a static method instead if you want such side effects.

2 Likes
11

Yes, I wasn't suggesting that the warning should be eliminated. I was answering the OP's question, whether the message is valid.

I'm not sure it's worth agonizing over, but perhaps the message could say "may" instead of "will"?

12

I think it is better to use "will", adding any uncertainty would just confuse people (including me) more.

1 Like