[SR-9836] NSView subclasses can fire two designated initialisers during initialisation, so ivar initialisation happens twice for each ivar

[swift-ci]

Previous ID	SR-9836
Radar	rdar://47734208
Original Reporter	mildm8nnered (JIRA User)
Type	Bug
Status	Reopened
Resolution

Attachment: Download

Environment

10.13.6 (17G4015) 10.14.2 (18C54) Xcode 10.1 (10B61)

Additional Detail from JIRA

| | | |------------------|-----------------| |Votes | 0 | |Component/s | Compiler | |Labels | Bug | |Assignee | None | |Priority | Medium | md5: e3b0777856599ddf39ad00afef827318

Issue Description:

It looks like compilation warnings about initialisation steps that break the rules do not work in some circumstances for NSView

I can get two designated initialisers to fire, during initialisation of one object, so it's ivars are initialised twice.

Furthermore, when this happens, the first ivar created will apparently leak, due to a circular reference between it and a an anonymous block of memory

Steps to Reproduce:

Paste the following code into a Playground (Xcode 10.1)

 import Cocoa

 class ParentView: NSView {
     private var myLayer: CALayer = {
         print(">>>> creating layer")
         return CALayer()
     }()

     override init(frame frameRect: NSRect) { super.init(frame: frameRect) }
     required init?(coder decoder: NSCoder) { super.init(coder: decoder) }
     init(dummy: AnyObject?) { self.init() }
 }

 class ChildView: ParentView {
     override init(frame frameRect: NSRect) { super.init(frame: frameRect) }
     required init?(coder decoder: NSCoder) { super.init(coder: decoder) }
     override init(dummy: AnyObject?) { super.init(dummy: dummy) }
  }

  let _ = ChildView(dummy: NSObject())

Expected Results:

This should not compile.
">>>> creating layer" should get printed once

Actual Results:

This does compile
">>>> creating layer" will get printed twice

Additional Information:

The correct thing to do here would be to call super.init(frame: .zero) instead of self.init(), but the bug is that this compiles at all.

If ParentView.init(dummy🙂 is changed to be super.init(), then in a Playground this fails to compile, erroring with

 error: MyPlayground.playground:5:31: error: must call a designated initializer of the superclass 'NSView' init(dummy: AnyObject?) { super.init() }

In an Xcode project, both self.init() and super.init() will compile, and show the same issue.

The sequence of events is something like

 ChildView.init(dummy:)
     ParentView.init(dummy:)
     initialises myLayer once
     NSView.init()
         ChildView.initWithFrame()
         ParentView,initWithFrame()
         initialises myLayer again

I could see in the debugger than NSView.init() exists, and just calls NSView.init(frame), but I couldn't find it in any header file.

In the context of the double initialisation, any object based ivars (at least if they are of type CALayer or NSView, which are the ones I tried) which are initialised twice in this way will appear as leaks in the Memory Graph Debugger

To see this, open the attached project. Add a breakpoint in main.swift, line 37, at 'print("Goodbye")'. When execution pauses, go to the Memory Graph Debugger in Xcode, and you will see nearly 1,000 leaked CALayer objects.

They apparently each hold pointers to 304 byte malloc blocks, which in turns hold references to the owning CALayer. I observed similar apparent leakage with an NSView ivar, so I'm guessing something about the weird double initialisation is causing this generically.

[belkadan]

NSView's init() is supposed to be a convenience initializer, not a designated initializer. I'll pass this along to the AppKit folks, but meanwhile it's best if you don't use it at all.

[belkadan]

Ah, unless it's a compiler bug…

[swift-ci]

Comment by Martin Redington (JIRA)

Yep, @belkadan - I'm pretty sure that the compiler should be error'ing here with

 error: must call a designated initializer of the superclass 'NSView'

but for this code, it's only generating that compile error in Playgrounds if super.init() is called.

both self.init and super.init compile fine in an Xcode project, and self.init() compiles fine in playgrounds.

If I try to construct my own class non-NSView class hierarchy to get the same effect (double ivar initialisation), the compiler always prevents me, which is surely what should also be happening here - I would not have expected to ever be able to get ivar's to be initialised twice, under any circumstances.

[swift-ci]

Comment by Martin Redington (JIRA)

Looks like the same issue is present with UIView as well.

This compiles fine for me in an iOS project, and suffers from the same double initialisation issue:

class ParentView: UIView {
    private var myLayer: CALayer = {
        print(">>>> creating layer")
        return CALayer()
    }()

    override init(frame frameRect: CGRect) { super.init(frame: frameRect) }
    required init?(coder decoder: NSCoder) { super.init(coder: decoder) }
    init(dummy: AnyObject?) { self.init() }
}

class ChildView: ParentView {
    override init(frame frameRect: CGRect) { super.init(frame: frameRect) }
    required init?(coder decoder: NSCoder) { super.init(coder: decoder) }
    override init(dummy: AnyObject?) { super.init(dummy: dummy) }
} 

[belkadan]

Good analysis. Here's an AST dump I got today for the self.init() invocation:

(rebind_self_in_constructor_expr implicit type='()' location=main.swift:20:36 range=[main.swift:20:31 - line:20:41]
  (covariant_return_conversion_expr implicit type='ParentView' location=main.swift:20:36 range=[main.swift:20:31 - line:20:41]
    (call_expr type='NSView' location=main.swift:20:36 range=[main.swift:20:31 - line:20:41] nothrow arg_labels=
      (dot_syntax_call_expr type='() -> NSView' location=main.swift:20:36 range=[main.swift:20:31 - line:20:36] nothrow
        (other_constructor_ref_expr type='(NSView) -> () -> NSView' location=main.swift:20:36 range=[main.swift:20:36 - line:20:36] decl=AppKit.(file).NSView.init())
        (derived_to_base_expr implicit type='NSView' location=main.swift:20:31 range=[main.swift:20:31 - line:20:31]
          (declref_expr type='ParentView' location=main.swift:20:31 range=[main.swift:20:31 - line:20:31] decl=TestMemory.(file).ParentView.init(dummy:).self@main.swift:20:5 function_ref=unapplied)))
      (tuple_expr type='()' location=main.swift:20:40 range=[main.swift:20:40 - line:20:41]))))

We shouldn't be accepting the expression at all, but instead we just insert an upcast to the parent and don't diagnose the designated-to-convenience chaining.

This is going to be tricky to fix because it's a source compatibility issue. I'm hesitant to even put in a warning when there's not always a great workaround.

[swift-ci]

Comment by Martin Redington (JIRA)

Wouldn't the workaround be for the user to actually invoke `super.init(frame🙂` in `ParentView.init(dummy🙂`?

The other thing that's weird here is that in a Playground, invocations of `super.init()` are correctly detected as wrong (but not in an Xcode project), so there are at least some paths through the compiler that correctly detect this as a problem.

[belkadan]

That's correct in this case, but it might not be for other classes.

Also, that's definitely weird. Can you attach such a playground?

[swift-ci]

Comment by Martin Redington (JIRA)

Attached