Skip to content

Commit 9cba638

Browse files
slavapestovslavapestov
committed
AST: Fix derivation of conformances in Substitution::subst() in the presence of same-type constraints
Applying a substitution list to a Substitution is done in two steps: 1) First, apply the substitution list to the original replacement type R to get the substituted replacement type R'. 2) Second, for each abstract conformance of R from the original substitution, look up a concrete conformance of R' from the substitution list. With minimized generic signatures, we would omit conformances of a nested type T.A if they could be derived from some other conformance already in the substitution list. However, the derivation process was incorrect, because it would only look at the canonical parent type T, and not any other parent type that had a child A which was same-type equivalent to T.A. For example, consider the following code: protocol P1 { associatedtype A : P3 } protocol P2 { associatedtype A : P4 } protocol P3 {} protocol P4 {} func doSomething<T : P4>(...) {} func doSomethingElse<T1 : P1, T2 : P2>(...) where T1.A == T2.A { doSomething(...) } If we specialize doSomethingElse() with a pair of concrete types to replace T1 and T2, we would need to find a concrete conformance to replace the abstract conformance T2.A : P4 in the call to doSomething(). Since the conformance of T2.A : P4 is a redundant requirement, it does not appear in the conformance map; furthermore, T1.A and T2.A are same-type equivalent, so they map to the same archetype. We would therefore look at the canonical parent of T2.A, which is T1, and look up the conformance of T1 : P1 in the substitution list. However, the only requirement P1 imposes on A is the conformance A : P3. There's no conformance A : P4 inside T1 : P1, so we would hit an assertion. Indeed, the conformance T1.A : P4 must be recovered from the conformance of T2 : P2, because P2 requires that A : P4. This patch ensures that the conformance can be found by changing the ArchetypeConformanceMap from a simple mapping of archetypes to conformances into a structure that records same-type constraints as well. So instead of just looking at the canonical parent of the archetype T1.A, we consult the structure to check all archetypes that have T1.A as a child, in this case both T1 and T2. T2 : P2 contains the conformance we need, allowing the above code to be specialized correctly.
1 parent 816b316 commit 9cba638

File tree

4 files changed

+182
-54
lines changed

4 files changed

+182
-54
lines changed

‎include/swift/AST/Substitution.h

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
#include "swift/AST/Type.h"
2121
#include "llvm/ADT/ArrayRef.h"
22+
#include "llvm/ADT/Optional.h"
2223

2324
namespace llvm {
2425
class raw_ostream;
@@ -28,11 +29,30 @@ namespace swift {
2829
class ArchetypeType;
2930
class GenericEnvironment;
3031
class ProtocolConformanceRef;
31-
32-
/// DenseMap type used internally by Substitution::subst to track conformances
33-
/// applied to archetypes.
34-
using ArchetypeConformanceMap
35-
= llvm::DenseMap<ArchetypeType*, ArrayRef<ProtocolConformanceRef>>;
32+
33+
/// Data structure type used internally by Substitution::subst to track
34+
/// conformances applied to archetypes.
35+
class ArchetypeConformanceMap {
36+
using ParentType = std::pair<ArchetypeType *, AssociatedTypeDecl *>;
37+
38+
llvm::DenseMap<ArchetypeType *, ArrayRef<ProtocolConformanceRef>> map;
39+
llvm::DenseMap<ArchetypeType *, SmallVector<ParentType, 1>> parents;
40+
41+
Optional<ProtocolConformanceRef>
42+
lookupArchetypeConformance(ProtocolDecl *proto,
43+
ArrayRef<ProtocolConformanceRef> conformances) const;
44+
45+
public:
46+
Optional<ProtocolConformanceRef>
47+
lookupArchetypeConformance(ArchetypeType *replacement,
48+
ProtocolDecl *proto) const;
49+
50+
void addArchetypeConformances(ArchetypeType *replacement,
51+
ArrayRef<ProtocolConformanceRef> conformances);
52+
53+
void addArchetypeParent(ArchetypeType *replacement, ArchetypeType *parent,
54+
AssociatedTypeDecl *assocType);
55+
};
3656

3757
/// Substitution - A substitution into a generic specialization.
3858
class Substitution {

‎lib/AST/GenericEnvironment.cpp

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,8 +98,39 @@ getSubstitutionMap(ModuleDecl *mod,
9898

9999
// Record the replacement type and its conformances.
100100
subsMap[archetype] = sub.getReplacement();
101-
conformanceMap[archetype] = sub.getConformances();
101+
conformanceMap.addArchetypeConformances(archetype, sub.getConformances());
102102
}
103-
103+
104+
for (auto reqt : sig->getRequirements()) {
105+
if (reqt.getKind() != RequirementKind::SameType)
106+
continue;
107+
108+
auto first = reqt.getFirstType()->getAs<DependentMemberType>();
109+
auto second = reqt.getSecondType()->getAs<DependentMemberType>();
110+
111+
if (!first || !second)
112+
continue;
113+
114+
auto archetype = mapTypeIntoContext(mod, first)->getAs<ArchetypeType>();
115+
if (!archetype)
116+
continue;
117+
118+
auto firstBase = first->getBase();
119+
auto secondBase = second->getBase();
120+
121+
auto firstBaseArchetype = mapTypeIntoContext(mod, firstBase)->getAs<ArchetypeType>();
122+
auto secondBaseArchetype = mapTypeIntoContext(mod, secondBase)->getAs<ArchetypeType>();
123+
124+
if (!firstBaseArchetype || !secondBaseArchetype)
125+
continue;
126+
127+
if (archetype->getParent() != firstBaseArchetype)
128+
conformanceMap.addArchetypeParent(archetype, firstBaseArchetype,
129+
first->getAssocType());
130+
if (archetype->getParent() != secondBaseArchetype)
131+
conformanceMap.addArchetypeParent(archetype, secondBaseArchetype,
132+
second->getAssocType());
133+
}
134+
104135
assert(subs.empty() && "did not use all substitutions?!");
105136
}

‎lib/AST/Substitution.cpp

Lines changed: 71 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -24,38 +24,9 @@
2424

2525
using namespace swift;
2626

27-
bool Substitution::operator==(const Substitution &other) const {
28-
// The archetypes may be missing, but we can compare them directly
29-
// because archetypes are always canonical.
30-
return
31-
Replacement->getCanonicalType() == other.Replacement->getCanonicalType() &&
32-
Conformance.equals(other.Conformance);
33-
}
34-
35-
Substitution::Substitution(Type Replacement,
36-
ArrayRef<ProtocolConformanceRef> Conformance)
37-
: Replacement(Replacement), Conformance(Conformance)
38-
{
39-
// The replacement type must be materializable.
40-
assert(Replacement->isMaterializable()
41-
&& "cannot substitute with a non-materializable type");
42-
}
43-
44-
Substitution Substitution::subst(Module *module,
45-
GenericSignature *sig,
46-
GenericEnvironment *env,
47-
ArrayRef<Substitution> subs) const {
48-
TypeSubstitutionMap subMap;
49-
ArchetypeConformanceMap conformanceMap;
50-
51-
assert(sig && env);
52-
env->getSubstitutionMap(module, sig, subs, subMap, conformanceMap);
53-
return subst(module, subMap, conformanceMap);
54-
}
55-
56-
static Optional<ProtocolConformanceRef>
27+
Optional<ProtocolConformanceRef> ArchetypeConformanceMap::
5728
lookupArchetypeConformance(ProtocolDecl *proto,
58-
ArrayRef<ProtocolConformanceRef> conformances) {
29+
ArrayRef<ProtocolConformanceRef> conformances) const {
5930
for (ProtocolConformanceRef found : conformances) {
6031
auto foundProto = found.getRequirement();
6132
if (foundProto == proto) {
@@ -73,40 +44,93 @@ lookupArchetypeConformance(ProtocolDecl *proto,
7344
return None;
7445
}
7546

76-
static Optional<ProtocolConformanceRef>
47+
Optional<ProtocolConformanceRef> ArchetypeConformanceMap::
7748
lookupArchetypeConformance(ArchetypeType *replacement,
78-
ProtocolDecl *proto,
79-
ArchetypeConformanceMap &conformanceMap) {
49+
ProtocolDecl *proto) const {
8050
// Check for conformances for the type that apply to the original
8151
// substituted archetype.
82-
auto it = conformanceMap.find(replacement);
83-
if (it != conformanceMap.end()) {
84-
if (auto conformance = lookupArchetypeConformance(proto, it->second)) {
52+
auto foundReplacement = map.find(replacement);
53+
if (foundReplacement != map.end()) {
54+
auto substReplacement = foundReplacement->second;
55+
if (auto conformance = lookupArchetypeConformance(proto, substReplacement))
8556
return conformance;
86-
}
8757
}
8858

89-
// Check if we have substitutions for the parent.
90-
if (auto *parent = replacement->getParent()) {
91-
auto *assocType = replacement->getAssocType();
92-
auto *parentProto = assocType->getProtocol();
59+
// Check if we have substitutions from one of our parent archetypes.
60+
auto foundParents = parents.find(replacement);
61+
if (foundParents == parents.end())
62+
return None;
63+
64+
for (auto parent : foundParents->second) {
65+
auto *parentProto = parent.second->getProtocol();
9366
auto conformance =
94-
lookupArchetypeConformance(parent, parentProto, conformanceMap);
67+
lookupArchetypeConformance(parent.first, parentProto);
9568

9669
if (conformance) {
9770
if (!conformance->isConcrete())
9871
return ProtocolConformanceRef(proto);
9972

10073
auto sub = conformance->getConcrete()->getTypeWitnessSubstAndDecl(
101-
assocType, nullptr).first;
74+
parent.second, nullptr).first;
10275

103-
return lookupArchetypeConformance(proto, sub.getConformances());
76+
if (auto result = lookupArchetypeConformance(proto, sub.getConformances()))
77+
return result;
10478
}
10579
}
10680

10781
return None;
10882
}
10983

84+
void ArchetypeConformanceMap::
85+
addArchetypeConformances(ArchetypeType *replacement,
86+
ArrayRef<ProtocolConformanceRef> conformances) {
87+
assert(replacement);
88+
89+
auto result = map.insert(std::make_pair(replacement, conformances));
90+
assert(result.second);
91+
(void) result;
92+
93+
if (auto *parent = replacement->getParent())
94+
addArchetypeParent(replacement, parent, replacement->getAssocType());
95+
}
96+
97+
void ArchetypeConformanceMap::
98+
addArchetypeParent(ArchetypeType *replacement,
99+
ArchetypeType *parent,
100+
AssociatedTypeDecl *assocType) {
101+
assert(replacement && parent && assocType);
102+
parents[replacement].push_back(std::make_pair(parent, assocType));
103+
}
104+
105+
bool Substitution::operator==(const Substitution &other) const {
106+
// The archetypes may be missing, but we can compare them directly
107+
// because archetypes are always canonical.
108+
return
109+
Replacement->getCanonicalType() == other.Replacement->getCanonicalType() &&
110+
Conformance.equals(other.Conformance);
111+
}
112+
113+
Substitution::Substitution(Type Replacement,
114+
ArrayRef<ProtocolConformanceRef> Conformance)
115+
: Replacement(Replacement), Conformance(Conformance)
116+
{
117+
// The replacement type must be materializable.
118+
assert(Replacement->isMaterializable()
119+
&& "cannot substitute with a non-materializable type");
120+
}
121+
122+
Substitution Substitution::subst(Module *module,
123+
GenericSignature *sig,
124+
GenericEnvironment *env,
125+
ArrayRef<Substitution> subs) const {
126+
TypeSubstitutionMap subMap;
127+
ArchetypeConformanceMap conformanceMap;
128+
129+
assert(sig && env);
130+
env->getSubstitutionMap(module, sig, subs, subMap, conformanceMap);
131+
return subst(module, subMap, conformanceMap);
132+
}
133+
110134
Substitution Substitution::subst(Module *module,
111135
TypeSubstitutionMap &subMap,
112136
ArchetypeConformanceMap &conformanceMap) const {
@@ -143,8 +167,8 @@ Substitution Substitution::subst(Module *module,
143167

144168
// If the original type was an archetype, check the conformance map.
145169
if (auto replacementArch = Replacement->getAs<ArchetypeType>()) {
146-
conformance = lookupArchetypeConformance(replacementArch, proto,
147-
conformanceMap);
170+
conformance = conformanceMap.lookupArchetypeConformance(
171+
replacementArch, proto);
148172
}
149173

150174
// If that didn't find anything, we can still synthesize AnyObject

‎test/SILOptimizer/specialize_same_type_constraint.swift

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
// RUN: %target-swift-frontend -O -emit-sil -primary-file %s | %FileCheck %s
2+
3+
protocol FirstChild {}
4+
5+
protocol FirstParent {
6+
associatedtype Child : FirstChild
7+
8+
var firstChild: Child { get }
9+
}
10+
11+
protocol SecondChild {}
12+
13+
protocol SecondParent {
14+
associatedtype Child : SecondChild
15+
16+
var secondChild: Child { get }
17+
}
18+
19+
@_semantics("optimize.sil.never")
20+
func takesFirstChild<T : FirstChild>(t: T) {}
21+
22+
@_semantics("optimize.sil.never")
23+
func takesSecondChild<T : SecondChild>(t: T) {}
24+
25+
@inline(never)
26+
func doStuff<First : FirstParent, Second : SecondParent>(f: First, s: Second)
27+
where First.Child == Second.Child {
28+
takesFirstChild(t: f.firstChild)
29+
takesSecondChild(t: f.firstChild)
30+
31+
takesFirstChild(t: s.secondChild)
32+
takesSecondChild(t: s.secondChild)
33+
}
34+
35+
struct ConcreteChild : FirstChild, SecondChild {}
36+
37+
struct ConcreteFirstParent<T> : FirstParent {
38+
var firstChild: ConcreteChild { return ConcreteChild() }
39+
}
40+
41+
struct ConcreteSecondParent<T> : SecondParent {
42+
var secondChild: ConcreteChild { return ConcreteChild() }
43+
}
44+
45+
doStuff(f: ConcreteFirstParent<ConcreteChild>(),
46+
s: ConcreteSecondParent<ConcreteChild>())
47+
48+
// CHECK-LABEL: sil shared [noinline] @_TTSf4d_d___TTSg5GV31specialize_same_type_constraint19ConcreteFirstParentVS_13ConcreteChild_GS0_S1__S_11FirstParentS__GVS_20ConcreteSecondParentS1__GS3_S1__S_12SecondParentS____TF31specialize_same_type_constraint7doStuffu0_RxS_11FirstParent_S_12SecondParentwx5Childzw_5ChildrFT1fx1sq__T_
49+
// CHECK: [[FIRST:%.*]] = function_ref @_TF31specialize_same_type_constraint15takesFirstChilduRxS_10FirstChildrFT1tx_T_
50+
// CHECK: apply [[FIRST]]<ConcreteChild>({{.*}}) : $@convention(thin) <τ_0_0 where τ_0_0 : FirstChild> (@in τ_0_0) -> ()
51+
// CHECK: [[SECOND:%.*]] = function_ref @_TF31specialize_same_type_constraint16takesSecondChilduRxS_11SecondChildrFT1tx_T_
52+
// CHECK: apply [[SECOND]]<ConcreteChild>({{.*}}) : $@convention(thin) <τ_0_0 where τ_0_0 : SecondChild> (@in τ_0_0) -> ()
53+
// CHECK: return

0 commit comments

Comments
 (0)