Skip to content

Commit 02c7eba

Browse files
slavapestovslavapestov
authored
Merge pull request #40795 from slavapestov/rqm-property-unification-rewrite-path-fixes
RequirementMachine: Two more fixes for -requirement-machine-protocol-signatures=verify
2 parents c78674f + f674ca9 commit 02c7eba

11 files changed

+73
-70
lines changed

‎lib/AST/RequirementMachine/PropertyMap.cpp

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,6 @@ void PropertyMap::clear() {
304304

305305
Trie.clear();
306306
Entries.clear();
307-
ConcreteTypeInDomainMap.clear();
308307
}
309308

310309
/// Build the property map from all rules of the form T.[p] => T, where
@@ -340,7 +339,11 @@ PropertyMap::buildPropertyMap(unsigned maxIterations,
340339
if (rule.isSimplified())
341340
continue;
342341

343-
if (rule.isPermanent())
342+
// Identity conformances ([P].[P] => [P]) are permanent rules, but we
343+
// keep them around to ensure that concrete conformance introduction
344+
// works in the case where the protocol's Self type is itself subject
345+
// to a superclass or concrete type requirement.
346+
if (rule.isPermanent() && !rule.isIdentityConformanceRule())
344347
continue;
345348

346349
// Collect all rules of the form T.[p] => T where T is canonical.
@@ -371,10 +374,6 @@ PropertyMap::buildPropertyMap(unsigned maxIterations,
371374
// Now, check for conflicts between superclass and concrete type rules.
372375
checkConcreteTypeRequirements(inducedRules);
373376

374-
// We collect terms with fully concrete types so that we can re-use them
375-
// to tie off recursion in the next step.
376-
computeConcreteTypeInDomainMap();
377-
378377
// Now, we merge concrete type rules with conformance rules, by adding
379378
// relations between associated type members of type parameters with
380379
// the concrete type witnesses in the concrete type's conformance.

‎lib/AST/RequirementMachine/PropertyMap.h

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ class PropertyBag {
117117
return Superclass.hasValue();
118118
}
119119

120-
Type getSuperclassBound() const {
120+
CanType getSuperclassBound() const {
121121
return Superclass->getSuperclass();
122122
}
123123

@@ -130,7 +130,7 @@ class PropertyBag {
130130
return ConcreteType.hasValue();
131131
}
132132

133-
Type getConcreteType() const {
133+
CanType getConcreteType() const {
134134
return ConcreteType->getConcreteType();
135135
}
136136

@@ -165,9 +165,6 @@ class PropertyMap {
165165
std::vector<PropertyBag *> Entries;
166166
Trie<PropertyBag *, MatchKind::Longest> Trie;
167167

168-
using ConcreteTypeInDomain = std::pair<CanType, ArrayRef<const ProtocolDecl *>>;
169-
llvm::DenseMap<ConcreteTypeInDomain, Term> ConcreteTypeInDomainMap;
170-
171168
// Building the property map introduces new induced rules, which
172169
// runs another round of Knuth-Bendix completion, which rebuilds the
173170
// property map again.
@@ -235,7 +232,6 @@ class PropertyMap {
235232
void checkConcreteTypeRequirements(
236233
SmallVectorImpl<InducedRule> &inducedRules);
237234

238-
void computeConcreteTypeInDomainMap();
239235
void concretizeNestedTypesFromConcreteParents(
240236
SmallVectorImpl<InducedRule> &inducedRules);
241237

‎lib/AST/RequirementMachine/PropertyUnification.cpp

Lines changed: 33 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -578,33 +578,6 @@ void PropertyMap::checkConcreteTypeRequirements(
578578
}
579579
}
580580

581-
/// For each fully-concrete type, find the shortest term having that concrete type.
582-
/// This is later used by computeConstraintTermForTypeWitness().
583-
void PropertyMap::computeConcreteTypeInDomainMap() {
584-
for (auto *props : Entries) {
585-
if (!props->isConcreteType())
586-
continue;
587-
588-
auto concreteType = props->ConcreteType->getConcreteType();
589-
if (concreteType->hasTypeParameter())
590-
continue;
591-
592-
assert(props->ConcreteType->getSubstitutions().empty());
593-
594-
auto domain = props->Key.getRootProtocols();
595-
auto concreteTypeKey = std::make_pair(concreteType, domain);
596-
597-
auto found = ConcreteTypeInDomainMap.find(concreteTypeKey);
598-
if (found != ConcreteTypeInDomainMap.end())
599-
continue;
600-
601-
auto inserted = ConcreteTypeInDomainMap.insert(
602-
std::make_pair(concreteTypeKey, props->Key));
603-
assert(inserted.second);
604-
(void) inserted;
605-
}
606-
}
607-
608581
void PropertyMap::concretizeNestedTypesFromConcreteParents(
609582
SmallVectorImpl<InducedRule> &inducedRules) {
610583
for (auto *props : Entries) {
@@ -945,6 +918,39 @@ MutableTerm PropertyMap::computeConstraintTermForTypeWitness(
945918
return result;
946919
}
947920

921+
// If the type witness is completely concrete, check if one of our prefix
922+
// types has the same concrete type, and if so, introduce a same-type
923+
// requirement between the subject type and the prefix.
924+
if (!typeWitness->hasTypeParameter()) {
925+
auto begin = key.begin();
926+
auto end = key.end();
927+
928+
while (begin != end) {
929+
MutableTerm prefix(begin, end);
930+
if (auto *props = lookUpProperties(prefix)) {
931+
if (props->isConcreteType() &&
932+
props->getConcreteType() == typeWitness) {
933+
auto result = props->getKey();
934+
935+
RewriteSystem::TypeWitness witness(Term::get(subjectType, Context),
936+
result);
937+
unsigned witnessID = System.recordTypeWitness(witness);
938+
path.add(RewriteStep::forAbstractTypeWitness(
939+
witnessID, /*inverse=*/false));
940+
941+
if (Debug.contains(DebugFlags::ConcretizeNestedTypes)) {
942+
llvm::dbgs() << "^^ Type witness can re-use property bag of "
943+
<< result << "\n";
944+
}
945+
946+
return MutableTerm(result);
947+
}
948+
}
949+
950+
--end;
951+
}
952+
}
953+
948954
// Otherwise the type witness is concrete, but may contain type
949955
// parameters in structural position.
950956

@@ -970,9 +976,6 @@ MutableTerm PropertyMap::computeConstraintTermForTypeWitness(
970976
if (requirementKind == RequirementKind::SameType &&
971977
typeWitnessSymbol.getConcreteType() == concreteType &&
972978
typeWitnessSymbol.getSubstitutions() == substitutions) {
973-
// FIXME: ConcreteTypeInDomainMap should support substitutions so
974-
// that we can remove this.
975-
976979
if (Debug.contains(DebugFlags::ConcretizeNestedTypes)) {
977980
llvm::dbgs() << "^^ Type witness is the same as the concrete type\n";
978981
}
@@ -995,29 +998,6 @@ MutableTerm PropertyMap::computeConstraintTermForTypeWitness(
995998
return result;
996999
}
9971000

998-
// If the type witness is completely concrete, try to introduce a
999-
// same-type requirement with another representative type parameter,
1000-
// if we have one.
1001-
if (!typeWitness->hasTypeParameter()) {
1002-
// Check if we have a shorter representative we can use.
1003-
auto domain = key.getRootProtocols();
1004-
auto concreteTypeKey = std::make_pair(typeWitness, domain);
1005-
1006-
auto found = ConcreteTypeInDomainMap.find(concreteTypeKey);
1007-
if (found != ConcreteTypeInDomainMap.end()) {
1008-
MutableTerm result(found->second);
1009-
if (result != subjectType) {
1010-
if (Debug.contains(DebugFlags::ConcretizeNestedTypes)) {
1011-
llvm::dbgs() << "^^ Type witness can re-use property bag of "
1012-
<< found->second << "\n";
1013-
}
1014-
1015-
// FIXME: Record a rewrite path.
1016-
return result;
1017-
}
1018-
}
1019-
}
1020-
10211001
// Otherwise, add a concrete type requirement for the type witness.
10221002
//
10231003
// Add a rule:

‎lib/AST/RequirementMachine/RequirementLowering.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1003,6 +1003,7 @@ void RuleBuilder::collectRulesFromReferencedProtocols() {
10031003
llvm::dbgs() << "protocol " << proto->getName() << " {\n";
10041004
}
10051005

1006+
// Add the identity conformance rule [P].[P] => [P].
10061007
MutableTerm lhs;
10071008
lhs.add(Symbol::forProtocol(proto, Context));
10081009
lhs.add(Symbol::forProtocol(proto, Context));

‎test/Generics/circular_protocol_superclass.swift

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// RUN: not %target-swift-frontend -typecheck %s -debug-generic-signatures -requirement-machine-protocol-signatures=on 2>&1 | %FileCheck %s
2+
3+
// CHECK-LABEL: circular_protocol_superclass.(file).A@
4+
// CHECK-NEXT: Requirement signature: <Self where Self : a>
5+
protocol A : a {
6+
associatedtype e : a
7+
}
8+
9+
class a : A {
10+
typealias e = a
11+
}

‎test/Generics/rdar79564324.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ public func test<T : P>(_ t: T) where T == T.A {
3838
// CHECK-NEXT: Rewrite loops: {
3939
// CHECK: }
4040
// CHECK-NEXT: Property map: {
41+
// CHECK-NEXT: [P] => { conforms_to: [P] }
4142
// CHECK-NEXT: [P:A] => { conforms_to: [P] }
4243
// CHECK-NEXT: τ_0_1 => { conforms_to: [P] }
4344
// CHECK-NEXT: τ_0_0 => { conforms_to: [P] }

‎test/Generics/unify_nested_types_4.swift

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,20 +37,28 @@ struct G<T : P1 & P2> {}
3737
// we solve this by merging the repeated types with T.A or T.B:
3838
//
3939
// T.A.A => T.A
40-
// T.A.B => T.B
4140
// T.B.A => T.B
42-
// T.B.B => T.A
4341
// ...
4442

4543
// CHECK-LABEL: Requirement machine for <τ_0_0 where τ_0_0 : P1, τ_0_0 : P2>
4644
// CHECK-LABEL: Rewrite system: {
45+
// CHECK: - τ_0_0.[P1&P2:A].[concrete: S1 : P1] => τ_0_0.[P1&P2:A]
4746
// CHECK: - τ_0_0.[P1&P2:A].[P1:A] => τ_0_0.[P1&P2:A]
48-
// CHECK: - τ_0_0.[P1&P2:A].[P1:B] => τ_0_0.[P1&P2:B]
47+
// CHECK: - τ_0_0.[P1&P2:A].[P1:B].[concrete: S2] => τ_0_0.[P1&P2:A].[P1:B]
48+
// CHECK: - τ_0_0.[P1&P2:B].[concrete: S2 : P1] => τ_0_0.[P1&P2:B]
4949
// CHECK: - τ_0_0.[P1&P2:B].[P1:A] => τ_0_0.[P1&P2:B]
50-
// CHECK: - τ_0_0.[P1&P2:B].[P1:B] => τ_0_0.[P1&P2:A]
50+
// CHECK: - τ_0_0.[P1&P2:B].[P1:B].[concrete: S1] => τ_0_0.[P1&P2:B].[P1:B]
51+
// CHECK: - τ_0_0.[P1&P2:A].[P1:B].[concrete: S2 : P1] => τ_0_0.[P1&P2:A].[P1:B]
52+
// CHECK: - τ_0_0.[P1&P2:A].[P1:B].[P1:A] => τ_0_0.[P1&P2:A].[P1:B]
53+
// CHECK: - τ_0_0.[P1&P2:A].[P1:B].[P1:B] => τ_0_0.[P1&P2:A]
54+
// CHECK: - τ_0_0.[P1&P2:B].[P1:B].[concrete: S1 : P1] => τ_0_0.[P1&P2:B].[P1:B]
55+
// CHECK: - τ_0_0.[P1&P2:B].[P1:B].[P1:A] => τ_0_0.[P1&P2:B].[P1:B]
56+
// CHECK: - τ_0_0.[P1&P2:B].[P1:B].[P1:B] => τ_0_0.[P1&P2:B]
5157
// CHECK: }
5258
// CHECK-LABEL: Property map: {
5359
// CHECK: τ_0_0.[P1&P2:A] => { conforms_to: [P1] concrete_type: [concrete: S1] }
5460
// CHECK: τ_0_0.[P1&P2:B] => { conforms_to: [P1] concrete_type: [concrete: S2] }
61+
// CHECK: τ_0_0.[P1&P2:A].[P1:B] => { conforms_to: [P1] concrete_type: [concrete: S2] }
62+
// CHECK: τ_0_0.[P1&P2:B].[P1:B] => { conforms_to: [P1] concrete_type: [concrete: S1] }
5563
// CHECK: }
5664
// CHECK: }

‎test/Generics/unify_superclass_types_1.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,6 @@ extension P where Self : Derived {
2626
// CHECK-NEXT: Rewrite loops: {
2727
// CHECK: }
2828
// CHECK-NEXT: Property map: {
29-
// CHECK-NEXT: [P] => { layout: _NativeClass superclass: [superclass: Base] }
29+
// CHECK-NEXT: [P] => { conforms_to: [P] layout: _NativeClass superclass: [superclass: Base] }
3030
// CHECK-NEXT: τ_0_0 => { conforms_to: [P] layout: _NativeClass superclass: [superclass: Derived] }
3131
// CHECK-NEXT: }

‎test/Generics/unify_superclass_types_2.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ func unifySuperclassTest<T : P1 & P2>(_: T) {
4444
// CHECK-NEXT: Rewrite loops: {
4545
// CHECK: }
4646
// CHECK-NEXT: Property map: {
47+
// CHECK-NEXT: [P1] => { conforms_to: [P1] }
48+
// CHECK-NEXT: [P2] => { conforms_to: [P2] }
4749
// CHECK-NEXT: [P1:X] => { layout: _NativeClass superclass: [superclass: Generic<Int, τ_0_0, τ_0_1> with <[P1:A1], [P1:B1]>] }
4850
// CHECK-NEXT: [P2:X] => { layout: _NativeClass superclass: [superclass: Generic<τ_0_0, String, τ_0_1> with <[P2:A2], [P2:B2]>] }
4951
// CHECK-NEXT: τ_0_0 => { conforms_to: [P1 P2] }

‎test/Generics/unify_superclass_types_3.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,8 @@ func unifySuperclassTest<T : P1 & P2>(_: T) {
4848
// CHECK-NEXT: Rewrite loops: {
4949
// CHECK: }
5050
// CHECK-NEXT: Property map: {
51+
// CHECK-NEXT: [P1] => { conforms_to: [P1] }
52+
// CHECK-NEXT: [P2] => { conforms_to: [P2] }
5153
// CHECK-NEXT: [P1:X] => { layout: _NativeClass superclass: [superclass: Derived<τ_0_0, τ_0_1> with <[P1:A1], [P1:B1]>] }
5254
// CHECK-NEXT: [P2:X] => { layout: _NativeClass superclass: [superclass: Generic<τ_0_0, String, τ_0_1> with <[P2:A2], [P2:B2]>] }
5355
// CHECK-NEXT: τ_0_0 => { conforms_to: [P1 P2] }

‎test/Generics/unify_superclass_types_4.swift

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,9 @@ func unifySuperclassTest<T : P1 & P2>(_: T) {
4747
// CHECK-NEXT: Rewrite loops: {
4848
// CHECK: }
4949
// CHECK-NEXT: Property map: {
50+
// CHECK-NEXT: [P1] => { conforms_to: [P1] }
51+
// CHECK-NEXT: [P2] => { conforms_to: [P2] }
52+
// CHECK-NEXT: [Q] => { conforms_to: [Q] }
5053
// CHECK-NEXT: [P1:X] => { layout: _NativeClass superclass: [superclass: Base<τ_0_0> with <[P1:A1]>] }
5154
// CHECK-NEXT: [P2:A2] => { conforms_to: [Q] }
5255
// CHECK-NEXT: [P2:X] => { layout: _NativeClass superclass: [superclass: Derived<τ_0_0> with <[P2:A2]>] }

0 commit comments

Comments
 (0)