Repository navigation
[SR-5255] Make this initialization pattern efficient #47830
Description
| Previous ID | SR-5255 |
| Radar | rdar://problem/32850824 |
| Original Reporter | @dabrahams |
| Type | Bug |
| Status | Closed |
| Resolution | Done |
Additional Detail from JIRA
| Votes | 0 |
| Component/s | Compiler |
| Labels | Bug, Performance |
| Assignee | @slavapestov |
| Priority | Medium |
md5: 2ca3c2e2a8c54b11f6162787e7ac565b
relates to:
- SR-9645 Convenience inits unsoundly allow calling factory inits
Issue Description:
This pattern works:
public protocol FactoryInitializable {}
extension FactoryInitializable {
public init(_ me: Self) {
self = me
}
}
class Base : FactoryInitializable {
fileprivate init(hidden: ()) {}
public convenience init(_ one: Bool) {
self.init(one ? Derived1() : Derived2())
}
}
class Derived1 : Base {
public init() { super.init(hidden: ()) }
}
class Derived2 : Base {
public init() { super.init(hidden: ()) }
}
print(Base(true), Base(false))Unfortunately, it creates calls to swift_deallocPartialClassInstance as the already-allocated Base has to be thrown out so the appropriate derived class can be allocated. A peephole optimization could be used to eliminate this extra allocation/deallocation.
Activity
dabrahams commentedon Jun 19, 2017
@swift-ci create
belkadan commentedon Jun 19, 2017
This is "just" factory initializers. I'm pretty sure we don't implement it properly, either—I have no idea what it means to assign to
selffor a class type when called from a convenience initializer.dabrahams commentedon Jun 21, 2017
According to @slavapestov we do implement it properly; you two can duke it out if you like.
belkadan commentedon Jun 21, 2017
Whether or not we implement it properly, I don't think we should optimize it. There is a correct design for this feature and this isn't it.
dabrahams commentedon Jun 21, 2017
A library publishes a protocol that happens to do self-assignment. An application happens to make a class type conform to that protocol. They don't know about one another's internals. This will come up, and IMO it should not be needlessly inefficient.
slavapestov commentedon Jun 21, 2017
I agree with Dave. Whether or not we expose some additional sugar for this, the underlying architectural change, where non-@objc convenience initializers do not get an initializing entry point, is something we should do before ABI stability. It's not an optimization as much as a sensible refactoring.
belkadan commentedon Jun 26, 2017
I forgot to point out that this isn't sound under our initializer inheritance rules.
swift-ci commentedon Oct 7, 2019
Comment by Ky (JIRA)
Whoops this wasn't resolved before ABI Stability
belkadan commentedon Oct 7, 2019
We did do the refactoring-optimization Slava talked about, and we've banned this particular incantation to avoid the unsoundness. So I think we're down to factory inits. (The code's still not 100% efficient; for some reason it's not inlining the call to
init(factory:). But it's no longer doing a spare allocation.)dabrahams commentedon Oct 7, 2019
THANKS!