Skip to content

SILGen: Fix invalid SIL emitted when protocol requirement witnessed by base class convenience init #16899

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions lib/SILGen/SILGenPoly.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3345,11 +3345,20 @@ static WitnessDispatchKind getWitnessDispatchKind(SILDeclRef witness) {
// A natively ObjC method witness referenced this way will end up going
// through its native thunk, which will redispatch the method after doing
// bridging just like we want.
if (isFinal || isExtension || witness.isForeignToNativeThunk()
// Hack--We emit a static thunk for ObjC allocating constructors.
|| (decl->hasClangNode() && witness.kind == SILDeclRef::Kind::Allocator))
if (isFinal || isExtension || witness.isForeignToNativeThunk())
return WitnessDispatchKind::Static;

if (witness.kind == SILDeclRef::Kind::Allocator) {
// Non-required initializers can witness a protocol requirement if the class
// is final, so we can statically dispatch to them.
if (!cast<ConstructorDecl>(decl)->isRequired())
return WitnessDispatchKind::Static;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if it is marked required (and non-overriding) but the class is also final?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That should be caught above, where we check for final class or method.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, of course. Thanks.


// We emit a static thunk for ObjC allocating constructors.
if (decl->hasClangNode())
return WitnessDispatchKind::Static;
}

// Otherwise emit a class method.
return WitnessDispatchKind::Class;
}
Expand Down
29 changes: 29 additions & 0 deletions test/SILGen/witness-init-requirement-with-base-class-init.swift
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,32 @@ class Dog: Animal, BestFriend {}
// CHECK-LABEL: sil private [transparent] [thunk] @$S4main3DogCAA10BestFriendA2aDP6createxyFZTW
// CHECK: [[SELF:%.*]] = apply
// CHECK: unchecked_ref_cast [[SELF]] : $Animal to $Dog

class Base {
init() {}

convenience init(x: Int) {
self.init()
}
}

protocol Initable {
init(x: Int)
}

final class Derived : Base, Initable {}

// CHECK-LABEL: sil hidden @$S4main4BaseC1xACSi_tcfC : $@convention(method) (Int, @thick Base.Type) -> @owned Base
// CHECK: [[SELF:%.*]] = alloc_ref_dynamic %1 : $@thick Base.Type, $Base
// CHECK: [[METHOD:%.*]] = function_ref @$S4main4BaseC1xACSi_tcfc
// CHECK-NEXT: [[RESULT:%.*]] = apply [[METHOD]](%0, [[SELF]])
// CHECK-NEXT: return [[RESULT]]

// CHECK-LABEL: sil private [transparent] [thunk] @$S4main7DerivedCAA8InitableA2aDP1xxSi_tcfCTW : $@convention(witness_method: Initable) (Int, @thick Derived.Type) -> @out Derived
// CHECK: [[SELF:%.*]] = upcast %2 : $@thick Derived.Type to $@thick Base.Type
// CHECK: [[METHOD:%.*]] = function_ref @$S4main4BaseC1xACSi_tcfC
// CHECK-NEXT: [[RESULT:%.*]] = apply [[METHOD]](%1, [[SELF]])
// CHECK-NEXT: [[NEW_SELF:%.*]] = unchecked_ref_cast [[RESULT]] : $Base to $Derived
// CHECK-NEXT: store [[NEW_SELF]] to [init] %0 : $*Derived
// CHECK-NEXT: [[TUPLE:%.*]] = tuple ()
// CHECK-NEXT: return [[TUPLE]]