SIL: Fix subclass scope calculation for constructors #22989
Conversation
The logic here was buggy; we would dyn_cast the decl to a FuncDecl and return early if the cast failed; the ConstructorDecl logic had no effect at all. Furthermore, it was wrong, for two reasons: - even non-required designated inits still appear in the vtable - in the resilient case, the constructor needs public linkage unlike other non-final public methods, since it is referenced directly by subclasses when they perform a super.init() delegation Fixes <https://bugs.swift.org/browse/SR-9939>, <rdar://problem/48403349>.
|
@swift-ci Please smoke test |
|
@swift-ci Please test source compatibility |
| if (classType->isResilient()) | ||
| if (classType->isResilient()) { | ||
| if (isa<ConstructorDecl>(decl)) | ||
| return SubclassScope::NotApplicable; |
There was a problem hiding this comment.
This doesn't look right; surely required initializers still show up, at least?
There was a problem hiding this comment.
If a base class is resilient, vtable of a derived class in another module is always built dynamically, so the base class methods don't need to have their linkage made more visible.
There was a problem hiding this comment.
But why are constructors different from other methods?
There was a problem hiding this comment.
Because they can be called directly via super.init delegation
There was a problem hiding this comment.
I still don't understand. That's an optimization, just like it is for devirtualizing method calls. (Why? Because of resilient superclass insertion.)
The logic here was buggy; we would dyn_cast the decl to a FuncDecl and
return early if the cast failed; the ConstructorDecl logic had no
effect at all.
Furthermore, it was wrong, for two reasons:
other non-final public methods, since it is referenced directly
by subclasses when they perform a super.init() delegation
Fixes https://bugs.swift.org/browse/SR-9939, rdar://problem/48403349.