Skip to content

Existentials don't always conform to a protocol via an abstract confo… #31736

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
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
6 changes: 1 addition & 5 deletions include/swift/AST/GenericSignatureBuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -519,11 +519,7 @@ class GenericSignatureBuilder {

ProtocolConformanceRef operator()(CanType dependentType,
Type conformingReplacementType,
ProtocolDecl *conformedProtocol) const {
return builder->lookupConformance(dependentType,
conformingReplacementType,
conformedProtocol);
}
ProtocolDecl *conformedProtocol) const;
};

/// Retrieve a function that can perform conformance lookup for this
Expand Down
5 changes: 1 addition & 4 deletions include/swift/AST/Type.h
Original file line number Diff line number Diff line change
Expand Up @@ -147,10 +147,7 @@ enum class SubstFlags {
/// Map member types to their desugared witness type.
DesugarMemberTypes = 0x02,
/// Substitute types involving opaque type archetypes.
SubstituteOpaqueArchetypes = 0x04,
/// Force substitution of opened archetypes. Normally -- without these flag --
/// opened archetype conformances are not substituted.
ForceSubstituteOpenedExistentials = 0x08,
SubstituteOpaqueArchetypes = 0x04
};

/// Options for performing substitutions into a type.
Expand Down
13 changes: 13 additions & 0 deletions lib/AST/GenericSignatureBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3501,6 +3501,19 @@ GenericSignatureBuilder::getLookupConformanceFn()
return LookUpConformanceInBuilder(this);
}

ProtocolConformanceRef
GenericSignatureBuilder::LookUpConformanceInBuilder::operator()(
CanType dependentType, Type conformingReplacementType,
ProtocolDecl *conformedProtocol) const {
// Lookup conformances for opened existential.
if (conformingReplacementType->isOpenedExistential()) {
return conformedProtocol->getModuleContext()->lookupConformance(
conformingReplacementType, conformedProtocol);
}
return builder->lookupConformance(dependentType, conformingReplacementType,
conformedProtocol);
}

ProtocolConformanceRef
GenericSignatureBuilder::lookupConformance(CanType dependentType,
Type conformingReplacementType,
Expand Down
6 changes: 0 additions & 6 deletions lib/AST/ProtocolConformance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -120,12 +120,6 @@ ProtocolConformanceRef::subst(Type origType,
// Otherwise, compute the substituted type.
auto substType = origType.subst(subs, conformances, options);

// Opened existentials trivially conform and do not need to go through
// substitution map lookup.
if (substType->isOpenedExistential() &&
!options.contains(SubstFlags::ForceSubstituteOpenedExistentials))
return *this;

auto *proto = getRequirement();

// If the type is an existential, it must be self-conforming.
Expand Down
16 changes: 16 additions & 0 deletions lib/AST/Type.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3547,6 +3547,11 @@ operator()(CanType dependentType, Type conformingReplacementType,
ProtocolConformanceRef LookUpConformanceInSubstitutionMap::
operator()(CanType dependentType, Type conformingReplacementType,
ProtocolDecl *conformedProtocol) const {
// Lookup conformances for opened existential.
if (conformingReplacementType->isOpenedExistential()) {
return conformedProtocol->getModuleContext()->lookupConformance(
conformingReplacementType, conformedProtocol);
}
return Subs.lookupConformance(dependentType, conformedProtocol);
}

Expand All @@ -3558,12 +3563,23 @@ operator()(CanType dependentType, Type conformingReplacementType,
|| conformingReplacementType->is<DependentMemberType>()
|| conformingReplacementType->is<TypeVariableType>())
&& "replacement requires looking up a concrete conformance");
// Lookup conformances for opened existential.
if (conformingReplacementType->isOpenedExistential()) {
return conformedProtocol->getModuleContext()->lookupConformance(
conformingReplacementType, conformedProtocol);
}
return ProtocolConformanceRef(conformedProtocol);
}

ProtocolConformanceRef LookUpConformanceInSignature::
operator()(CanType dependentType, Type conformingReplacementType,
ProtocolDecl *conformedProtocol) const {
// Lookup conformances for opened existential.
if (conformingReplacementType->isOpenedExistential()) {
return conformedProtocol->getModuleContext()->lookupConformance(
conformingReplacementType, conformedProtocol);
}

// FIXME: Should pass dependentType instead, once
// GenericSignature::lookupConformance() does the right thing
return Sig->lookupConformance(conformingReplacementType->getCanonicalType(),
Expand Down
3 changes: 1 addition & 2 deletions lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -952,8 +952,7 @@ SILInstruction *SILCombiner::createApplyWithConcreteType(
return CEI.lookupExistentialConformance(proto);
}
return ProtocolConformanceRef(proto);
},
SubstFlags::ForceSubstituteOpenedExistentials);
});
}

// We need to make sure that we can a) update Apply to use the new args and b)
Expand Down
48 changes: 47 additions & 1 deletion test/SILOptimizer/inline_generics.sil
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// RUN: %target-sil-opt -enable-sil-verify-all %s -inline -sil-inline-generics=true | %FileCheck %s
// RUN: %target-sil-opt -enable-sil-verify-all %s -inline -sil-inline-generics=true -sil-print-generic-specialization-info | %FileCheck %s
// RUN: %target-sil-opt -enable-sil-verify-all %s -inline -sil-inline-generics=false | %FileCheck --check-prefix=DISABLED-GENERIC-INLINING-CHECK %s
// RUN: %target-swift-frontend -O -emit-ir %s

sil_stage canonical

Expand Down Expand Up @@ -178,3 +179,48 @@ bb0(%0 : $*T, %1 : $*T):
sil_default_witness_table P {
no_default
}

protocol SomeProto : class {}
protocol OtherProto {}

class MyObject {}
extension MyObject : OtherProto {}

class MyNumber : MyObject {}
extension MyNumber : SomeProto {}

sil_vtable MyObject {}
sil_vtable MyNumber {}

// Make sure we subsitute the correct conformance when inlining. When
// substituting we need to pick up the normal_conformance.

// CHECK-LABEL: sil @test_inlining : $@convention(objc_method) (@owned MyNumber) -> () {
// CHECK-NOT: Generic specialization information for call-site dootherstuff <MyNumber & SomeProto> conformances <(abstract_conformance protocol=OtherProto)>
// CHECK: Generic specialization information
// CHECK: (normal_conformance type=MyObject protocol=OtherProto)
// CHECK: end sil function 'test_inlining'

sil @test_inlining : $@convention(objc_method) (@owned MyNumber) -> () {
bb0(%0 : $MyNumber):
%12 = init_existential_ref %0 : $MyNumber : $MyNumber, $MyNumber & SomeProto
%14 = open_existential_ref %12 : $MyNumber & SomeProto to $@opened("B9711AE4-946E-11EA-A871-ACDE48001122") MyNumber & SomeProto
%15 = alloc_stack $OtherProto
%16 = init_existential_addr %15 : $*OtherProto, $@opened("B9711AE4-946E-11EA-A871-ACDE48001122") MyNumber & SomeProto
store %14 to %16 : $*@opened("B9711AE4-946E-11EA-A871-ACDE48001122") MyNumber & SomeProto
%18 = function_ref @doStuff : $@convention(thin) <τ_0_0 where τ_0_0 : OtherProto> (@in_guaranteed τ_0_0) -> ()
%20 = apply %18<@opened("B9711AE4-946E-11EA-A871-ACDE48001122") MyNumber & SomeProto>(%16) : $@convention(thin) <τ_0_0 where τ_0_0 : OtherProto> (@in_guaranteed τ_0_0) -> ()
destroy_addr %15 : $*OtherProto
dealloc_stack %15 : $*OtherProto
%27 = tuple ()
return %27 : $()
}

sil @dootherstuff : $@convention(thin) <τ_0_0 where τ_0_0 : OtherProto> (@in_guaranteed τ_0_0) -> ()

sil shared [signature_optimized_thunk] [always_inline] @doStuff : $@convention(thin) <τ_0_0 where τ_0_0 : OtherProto> (@in_guaranteed τ_0_0) -> () {
bb0(%0 : $*τ_0_0):
%2 = function_ref @dootherstuff : $@convention(thin) <τ_0_0 where τ_0_0 : OtherProto> (@in_guaranteed τ_0_0) -> ()
%3 = apply %2<τ_0_0>(%0) : $@convention(thin) <τ_0_0 where τ_0_0 : OtherProto> (@in_guaranteed τ_0_0) -> ()
return %3 : $()
}