swiftlang / swift

The Swift Programming Language
https://swift.org
Apache License 2.0
67.58k stars 10.36k forks source link

Unexpected non-isolated deinit warning #60498

Open keith opened 2 years ago

keith commented 2 years ago

With this code:

import AppKit

class SomeClass: NSView {
    var thing1: String = ""
    lazy var thing2: String = { fatalError() }()

    deinit {
        _ = thing2
        _ = thing1
    }
}

Compiling with Swift 5.7 from Xcode 14.0 beta 5, you see these warnings:

% DEVELOPER_DIR=/Applications/Xcode-14.0.0-beta5.app swiftc foo.swift
foo.swift:9:11: warning: cannot access property 'thing1' here in deinitializer; this is an error in Swift 6
        _ = thing1
        ~~^~~~~~~~
foo.swift:8:11: note: after accessing property 'thing2', only non-isolated properties of 'self' can be accessed from a deinit
        _ = thing2
        ~~^~~~~~~~

If you flip the order of the 2 statements in the deinit the warnings disappear. If you remove the NSView conformance, and manually mark SomeClass as @MainActor, these are elevated to errors instead:

% DEVELOPER_DIR=/Applications/Xcode-14.0.0-beta5.app swiftc foo.swift
foo.swift:9:13: error: main actor-isolated property 'thing2' can not be referenced from a non-isolated context
        _ = thing2
            ^
foo.swift:6:14: note: property declared here
    lazy var thing2: String = { fatalError() }()
             ^

I would expect those to result in the same case set of warnings / errors. The same is true if you inherit from UIView instead and target iOS.

If you make thing2 non-lazy, the warning also disappears. It seems plausible to me that the fact that it's a lazy var, and the initializer of it could be called for the first time in the deinit, triggers this warning, but I think it's a surprise to see the order of statements in the deinit affect the warning behavior.

rajasekhar-pasupuleti commented 2 years ago

I have an array of structure objects in my view controller, when I call array.removeAll() in deinit it is showing following warning Cannot access property 'array' here in deinitializer; this is an error in Swift 6

ktoso commented 2 years ago

The warning is correct. Please refer to https://forums.swift.org/t/se-0371-isolated-synchronous-deinit/59754 which is addressing this in a future version of swift.

keith commented 2 years ago

@ktoso sorry do you mean my original comment and @rajasekhar-pasupuleti's? On my original my surprise was more around how the order mattered in producing the warning

mildm8nnered commented 2 years ago

I saw a lot of warnings of the form warning: cannot access property 'thing1' here in deinitializer; this is an error in Swift 6 with Xcode 14.0, some of which seemed plausible, but these all seem to have gone away with Xcode 14.1b1 - is that expected?

I couldn't find any mention of relevant changes in the release notes.

mildm8nnered commented 2 years ago

Warnings are still present in Xcode 14.0.1rc ..

jrose-signal commented 2 years ago

Another "surprising" case (reduced):

import UIKit

class TestVC: UIViewController {
  var name: String = ""
  deinit {
    NotificationCenter.default.removeObserver(self)
    print(name)
  }
}

This is reduced from a codebase that hasn't adopted Swift concurrency at all. I never claimed that name is an isolated property; that got inferred for me by UIViewController being @MainActor. Maybe I'm doing my own manual synchronization using a mutex, though.

To add insult to injury, the note refers to making a "copy", which doesn't make sense for a class. (Yes, the reference is being copied from a "references are values" perspective, but you're not allowed to escape self during a deinitializer regardless.)

<stdin>:7:9: warning: cannot access property 'name' here in deinitializer; this is an error in Swift 6
    print(name)
          ^~~~
<stdin>:6:45: note: after making a copy of 'self', only non-isolated properties of 'self' can be accessed from a deinit
    NotificationCenter.default.removeObserver(self)
                                              ^~~~