Skip to content

Commit 70c9f8a

Browse files
slavapestovslavapestov
committed
RequirementMachine: Leave behind conflicting requirements in the minimized signature
Requirement lowering only expects that it won't see two requirements of the same kind (except for conformance requirements). So only mark those as conflicting. This addresses a crash-on-invalid and improves diagnostics for move-only generics, because a conflict won't drop the copyability of a generic parameter and expose a move-only-naive user to confusing error messages. Fixes #61031. Fixes #63997. Fixes rdar://problem/111991454.
1 parent ffd651c commit 70c9f8a

15 files changed

+99
-51
lines changed

‎lib/AST/RequirementMachine/HomotopyReduction.cpp

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -595,10 +595,18 @@ GenericSignatureErrors RewriteSystem::getErrors() const {
595595

596596
GenericSignatureErrors result;
597597

598+
if (!ConflictingRules.empty())
599+
result |= GenericSignatureErrorFlags::HasInvalidRequirements;
600+
598601
for (const auto &rule : getLocalRules()) {
599602
if (rule.isPermanent())
600603
continue;
601604

605+
// The conditional requirement inference feature imports new protocol
606+
// components after the basic rewrite system is already built, so that's
607+
// why we end up with imported rules that appear to be in the local rules
608+
// slice. Those rules are well-formed, but their isRedundant() bit isn't
609+
// set, so we must ignore them here.
602610
if (!isInMinimizationDomain(rule.getLHS().getRootProtocol()))
603611
continue;
604612

@@ -607,7 +615,7 @@ GenericSignatureErrors RewriteSystem::getErrors() const {
607615
rule.containsUnresolvedSymbols())
608616
result |= GenericSignatureErrorFlags::HasInvalidRequirements;
609617

610-
if (rule.isConflicting() || rule.isRecursive())
618+
if (rule.isRecursive())
611619
result |= GenericSignatureErrorFlags::HasInvalidRequirements;
612620

613621
if (!rule.isRedundant()) {

‎lib/AST/RequirementMachine/PropertyUnification.cpp

Lines changed: 32 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -119,36 +119,49 @@ static void recordRelation(Term key,
119119
}
120120

121121
/// Given two property rules that conflict because no concrete type
122-
/// can satisfy both, mark one or both rules conflicting.
123-
///
124-
/// The right hand side of one rule must be a suffix of the other
125-
/// (in which case the longer of the two rules is conflicting) or
126-
/// the right hand sides are equal (in which case both will be
127-
/// conflicting).
122+
/// can satisfy both, record the conflict. If both have the same kind,
123+
/// mark one or the other as conflicting, but not both.
128124
void RewriteSystem::recordConflict(unsigned existingRuleID,
129125
unsigned newRuleID) {
130-
ConflictingRules.emplace_back(existingRuleID, newRuleID);
131-
132126
auto &existingRule = getRule(existingRuleID);
133127
auto &newRule = getRule(newRuleID);
134128

129+
// FIXME: Property map construction shouldn't have to consider imported rules
130+
// at all. We need to import the property map from each protocol component,
131+
// just like we import rules.
132+
if (!isInMinimizationDomain(newRule.getLHS().getRootProtocol()) &&
133+
!isInMinimizationDomain(existingRule.getLHS().getRootProtocol())) {
134+
return;
135+
}
136+
137+
// Record the conflict for purposes of diagnostics.
138+
ConflictingRules.emplace_back(existingRuleID, newRuleID);
139+
135140
if (Debug.contains(DebugFlags::ConflictingRules)) {
136141
llvm::dbgs() << "Conflicting rules:\n";
137142
llvm::dbgs() << "- " << existingRule << "\n";
138143
llvm::dbgs() << "- " << newRule << "\n";
139144
}
140145

141-
// The identity conformance rule ([P].[P] => [P]) will conflict with
142-
// a concrete type requirement in an invalid protocol declaration
143-
// where 'Self' is constrained to a type that does not conform to
144-
// the protocol. This rule is permanent, so don't mark it as
145-
// conflicting in this case.
146-
if (!existingRule.isIdentityConformanceRule() &&
147-
existingRule.getRHS().size() >= newRule.getRHS().size())
148-
existingRule.markConflicting();
149-
if (!newRule.isIdentityConformanceRule() &&
150-
newRule.getRHS().size() >= existingRule.getRHS().size())
151-
newRule.markConflicting();
146+
if (existingRule.getLHS().back().getKind() ==
147+
newRule.getLHS().back().getKind()) {
148+
assert(!existingRule.isIdentityConformanceRule() &&
149+
!newRule.isIdentityConformanceRule());
150+
151+
// While we don't promise canonical minimization with conflicts,
152+
// it's not really a big deal to spit out a generic signature with
153+
// conflicts, as long as we diagnosed an error _somewhere_.
154+
//
155+
// However, the requirement lowering doesn't like to see two
156+
// conflicting rules of the same kind, so we rule that out by
157+
// marking the shorter rule as the conflict. Otherwise, we just
158+
// leave both rules in place.
159+
if (existingRule.getRHS().size() > newRule.getRHS().size()) {
160+
existingRule.markConflicting();
161+
} else {
162+
newRule.markConflicting();
163+
}
164+
}
152165
}
153166

154167
void PropertyMap::addConformanceProperty(

‎test/Constraints/same_types.swift

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -61,32 +61,32 @@ func test3<T: Fooable, U: Fooable>(_ t: T, u: U) -> (X, X)
6161
}
6262

6363
// CHECK-LABEL: same_types.(file).fail1(_:u:)@
64-
// CHECK-NEXT: Generic signature: <T, U where T : Fooable, U : Fooable, T.[Fooable]Foo == U.[Fooable]Foo>
64+
// CHECK-NEXT: Generic signature: <T, U where T : Fooable, U : Fooable, T.[Fooable]Foo == Y, U.[Fooable]Foo == Y>
6565
func fail1< // expected-error{{no type for 'T.Foo' can satisfy both 'T.Foo == X' and 'T.Foo == Y'}}
6666
T: Fooable, U: Fooable
6767
>(_ t: T, u: U) -> (X, Y)
6868
where T.Foo == X, U.Foo == Y, T.Foo == U.Foo {
69-
return (t.foo, u.foo) // expected-error{{cannot convert return expression of type '(T.Foo, T.Foo)' to return type '(X, Y)'}}
69+
return (t.foo, u.foo) // expected-error{{cannot convert return expression of type '(Y, Y)' to return type '(X, Y)'}}
7070
}
7171

7272
// CHECK-LABEL: same_types.(file).fail2(_:u:)@
73-
// CHECK-NEXT: Generic signature: <T, U where T : Fooable, U : Fooable, T.[Fooable]Foo == U.[Fooable]Foo>
73+
// CHECK-NEXT: Generic signature: <T, U where T : Fooable, U : Fooable, T.[Fooable]Foo == X, U.[Fooable]Foo == X>
7474
func fail2< // expected-error{{no type for 'T.Foo' can satisfy both 'T.Foo == Y' and 'T.Foo == X'}}
7575
T: Fooable, U: Fooable
7676
>(_ t: T, u: U) -> (X, Y)
7777
where T.Foo == U.Foo, T.Foo == X, U.Foo == Y {
78-
return (t.foo, u.foo) // expected-error{{cannot convert return expression of type '(T.Foo, T.Foo)' to return type '(X, Y)'}}
78+
return (t.foo, u.foo) // expected-error{{cannot convert return expression of type '(X, X)' to return type '(X, Y)'}}
7979
}
8080

8181
func test4<T: Barrable>(_ t: T) -> Y where T.Bar == Y {
8282
return t.bar
8383
}
8484

8585
// CHECK-LABEL: same_types.(file).fail3@
86-
// CHECK-NEXT: Generic signature: <T where T : Barrable>
86+
// CHECK-NEXT: Generic signature: <T where T : Barrable, T.[Barrable]Bar == X>
8787
func fail3<T: Barrable>(_ t: T) -> X // expected-error {{no type for 'T.Bar' can satisfy both 'T.Bar == X' and 'T.Bar : Fooable'}}
8888
where T.Bar == X {
89-
return t.bar // expected-error{{cannot convert return expression of type 'T.Bar' }}
89+
return t.bar
9090
}
9191

9292
func test5<T: Barrable>(_ t: T) -> X where T.Bar.Foo == X {
@@ -122,7 +122,7 @@ func fail5<T: Barrable>(_ t: T) -> (Y, Z)
122122
}
123123

124124
// CHECK-LABEL: same_types.(file).test8@
125-
// CHECK-NEXT: Generic signature: <T where T : Fooable>
125+
// CHECK-NEXT: Generic signature: <T where T : Fooable, T.[Fooable]Foo == X>
126126
func test8<T: Fooable>(_ t: T) // expected-error{{no type for 'T.Foo' can satisfy both 'T.Foo == Y' and 'T.Foo == X'}}
127127
where T.Foo == X,
128128
T.Foo == Y {}
@@ -300,7 +300,7 @@ protocol P4 {
300300
}
301301

302302
// CHECK-LABEL: same_types.(file).test9@
303-
// CHECK-NEXT: Generic signature: <T where T : P4>
303+
// CHECK-NEXT: Generic signature: <T where T : P4, T.[P4]A : P3, T.[P4]A == X>
304304
func test9<T>(_: T) where T.A == X, T: P4, T.A: P3 { } // expected-error{{no type for 'T.A' can satisfy both 'T.A == X' and 'T.A : P3'}}
305305

306306
// Same-type constraint conflict through protocol where clauses.
@@ -318,7 +318,7 @@ struct X5a {}
318318
struct X5b { }
319319

320320
// CHECK-LABEL: same_types.(file).test9(_:u:)@
321-
// CHECK-NEXT: Generic signature: <T, U where T : P6, U : P6, T.[P6]Bar == U.[P6]Bar>
321+
// CHECK-NEXT: Generic signature: <T, U where T : P6, U : P6, T.[P6]Bar == U.[P6]Bar, T.[P6]Bar.[P5]Foo1 == X5b>
322322
func test9<T: P6, U: P6>(_ t: T, u: U) // expected-error{{no type for 'T.Bar.Foo1' can satisfy both 'T.Bar.Foo1 == X5a' and 'T.Bar.Foo1 == X5b'}}
323323
where T.Bar.Foo1 == X5a,
324324
U.Bar.Foo2 == X5b,

‎test/Generics/concrete_conflict.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ class Class<T> {}
3535

3636
extension Class where T == Bool {
3737
// CHECK-LABEL: .badRequirement()@
38-
// CHECK-NEXT: <T>
38+
// CHECK-NEXT: <T where T == Bool>
3939
func badRequirement() where T == Int { }
4040
// expected-error@-1 {{no type for 'T' can satisfy both 'T == Int' and 'T == Bool'}}
4141
}

‎test/Generics/concrete_same_type_versus_anyobject.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ class C {}
77
struct G1<T : AnyObject> {}
88

99
// CHECK: ExtensionDecl line={{.*}} base=G1
10-
// CHECK-NEXT: Generic signature: <T>
10+
// CHECK-NEXT: Generic signature: <T where T : AnyObject, T == S>
1111
extension G1 where T == S {}
1212
// expected-error@-1 {{no type for 'T' can satisfy both 'T : AnyObject' and 'T == S'}}
1313

@@ -18,7 +18,7 @@ extension G1 where T == C {}
1818
struct G2<U> {}
1919

2020
// CHECK: ExtensionDecl line={{.*}} base=G2
21-
// CHECK-NEXT: Generic signature: <U>
21+
// CHECK-NEXT: Generic signature: <U where U : AnyObject, U == S>
2222
extension G2 where U == S, U : AnyObject {}
2323
// expected-error@-1 {{no type for 'U' can satisfy both 'U : AnyObject' and 'U == S'}}
2424

‎test/Generics/requirement_machine_diagnostics.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -327,4 +327,5 @@ func sameTypeConflicts() {
327327

328328
// expected-error@+1{{no type for 'T' can satisfy both 'T == G<U.Foo>' and 'T == Int'}}
329329
func fail9<T, U: Fooable>(_: U, _: T) where T == Int, T == G<U.Foo> {}
330+
// expected-warning@-1{{same-type requirement makes generic parameter 'T' non-generic; this is an error in Swift 6}}
330331
}

‎test/Generics/superclass_and_concrete_requirement.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,15 @@ protocol P1 {
1212
}
1313

1414
// CHECK-LABEL: .P2@
15-
// CHECK-NEXT: Requirement signature: <Self>
15+
// CHECK-NEXT: Requirement signature: <Self where Self.[P2]T : C, Self.[P2]T == S>
1616
protocol P2 {
1717
// expected-error@-1 {{no type for 'Self.T' can satisfy both 'Self.T : C' and 'Self.T == S'}}
1818
// expected-error@-2 {{no type for 'Self.T' can satisfy both 'Self.T : _NativeClass' and 'Self.T == S'}}
1919
associatedtype T where T : C, T == S
2020
}
2121

2222
// CHECK-LABEL: .P3@
23-
// CHECK-NEXT: Requirement signature: <Self>
23+
// CHECK-NEXT: Requirement signature: <Self where Self.[P3]T : C, Self.[P3]T == S>
2424
protocol P3 {
2525
// expected-error@-1 {{no type for 'Self.T' can satisfy both 'Self.T : C' and 'Self.T == S'}}
2626
// expected-error@-2 {{no type for 'Self.T' can satisfy both 'Self.T : _NativeClass' and 'Self.T == S'}}
@@ -32,7 +32,7 @@ protocol P4a {
3232
}
3333

3434
// CHECK-LABEL: .P4@
35-
// CHECK-NEXT: Requirement signature: <Self where Self.[P4]T : P4>
35+
// CHECK-NEXT: Requirement signature: <Self where Self.[P4]T : P4, Self.[P4]T.[P4]T == S>
3636
protocol P4 {
3737
// expected-error@-1 {{no type for 'Self.T.T' can satisfy both 'Self.T.T == S' and 'Self.T.T : P4'}}
3838
associatedtype T : P4 where T.T == S
@@ -41,7 +41,7 @@ protocol P4 {
4141
class D {}
4242

4343
// CHECK-LABEL: .P5@
44-
// CHECK-NEXT: Requirement signature: <Self where Self.[P5]T == D>
44+
// CHECK-NEXT: Requirement signature: <Self where Self.[P5]T : C, Self.[P5]T == D>
4545
protocol P5 {
4646
// expected-error@-1 {{no type for 'Self.T' can satisfy both 'Self.T : D' and 'Self.T : C'}}
4747
associatedtype T where T : C, T == D

‎test/Generics/superclass_requirement_and_objc_existential.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,18 +21,18 @@ func f1<T : Q>(_: T) where T.A : C, T.A == any (C & P1) {}
2121
/// These are not allowed.
2222

2323
// CHECK-LABEL: .f2@
24-
// CHECK-NEXT: Generic signature: <T where T : Q>
24+
// CHECK-NEXT: Generic signature: <T where T : Q, T.[Q]A : C, T.[Q]A == any P1>
2525
func f2<T : Q>(_: T) where T.A : C, T.A == any P1 {}
2626
// expected-error@-1 {{no type for 'T.A' can satisfy both 'T.A : C' and 'T.A == any P1'}}
2727

2828
// CHECK-LABEL: .f3@
29-
// CHECK-NEXT: Generic signature: <T where T : Q>
29+
// CHECK-NEXT: Generic signature: <T where T : Q, T.[Q]A : C, T.[Q]A == any C & P2>
3030
func f3<T : Q>(_: T) where T.A : C, T.A == any (C & P2) {}
3131
// expected-error@-1 {{no type for 'T.A' can satisfy both 'T.A : C' and 'T.A == any C & P2'}}
3232
// expected-error@-2 {{no type for 'T.A' can satisfy both 'T.A : _NativeClass' and 'T.A == any C & P2'}}
3333

3434
// CHECK-LABEL: .f4@
35-
// CHECK-NEXT: Generic signature: <T where T : Q>
35+
// CHECK-NEXT: Generic signature: <T where T : Q, T.[Q]A : C, T.[Q]A == any C & P3>
3636
func f4<T : Q>(_: T) where T.A : C, T.A == any (C & P3) {}
3737
// expected-error@-1 {{no type for 'T.A' can satisfy both 'T.A : C' and 'T.A == any C & P3'}}
3838
// expected-error@-2 {{no type for 'T.A' can satisfy both 'T.A : _NativeClass' and 'T.A == any C & P3'}}

‎test/IDE/complete_where_clause.swift

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@
3939
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=NOMINAL_TYPEALIAS_NESTED1_EXT | %FileCheck %s -check-prefix=NOMINAL_TYPEALIAS_NESTED1_EXT
4040
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=NOMINAL_TYPEALIAS_NESTED2_EXT | %FileCheck %s -check-prefix=NOMINAL_TYPEALIAS_NESTED2_EXT
4141
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=EXT_ASSOC_MEMBER_1 | %FileCheck %s -check-prefix=EXT_ASSOC_MEMBER
42-
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=EXT_ASSOC_MEMBER_2 | %FileCheck %s -check-prefix=EXT_ASSOC_MEMBER
42+
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=EXT_ASSOC_MEMBER_2 | %FileCheck %s -check-prefix=EXT_ASSOC_MEMBER_2
4343
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=EXT_SECONDTYPE | %FileCheck %s -check-prefix=EXT_SECONDTYPE
4444
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=WHERE_CLAUSE_WITH_EQUAL | %FileCheck %s -check-prefix=WHERE_CLAUSE_WITH_EQUAL
4545

@@ -234,8 +234,11 @@ extension WithAssoc where T.#^EXT_ASSOC_MEMBER_1^#
234234
// EXT_ASSOC_MEMBER-DAG: Decl[AssociatedType]/CurrNominal: Q;
235235
// EXT_ASSOC_MEMBER-DAG: Keyword/None: Type[#Self.T.Type#];
236236

237+
// This is kind of funny because we parse it as 'Int == T', so completing 'T'
238+
// shows members of 'Int'.
237239
extension WithAssoc where Int == T.#^EXT_ASSOC_MEMBER_2^#
238-
// Same as EXT_ASSOC_MEMBER
240+
// EXT_ASSOC_MEMBER_2: Begin completions, {{.*}} items
241+
// EXT_ASSOC_MEMBER_2: Keyword/None: Type[#Int.Type#];
239242

240243
extension WithAssoc where Int == #^EXT_SECONDTYPE^#
241244
// EXT_SECONDTYPE-DAG: Decl[AssociatedType]/CurrNominal: T;

‎test/attr/attr_specialize.swift

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,6 @@ struct AThing : Thing {}
8080
// CHECK: @_specialize(exported: false, kind: full, where T == AThing)
8181
@_specialize(where T == AThing)
8282
@_specialize(where T == Int) // expected-error{{no type for 'T' can satisfy both 'T == Int' and 'T : Thing'}}
83-
// expected-error@-1 {{too few generic parameters are specified in '_specialize' attribute (got 0, but expected 1)}}
84-
// expected-note@-2 {{missing constraint for 'T' in '_specialize' attribute}}
8583

8684
func oneRequirement<T : Thing>(_ t: T) {}
8785

@@ -171,8 +169,6 @@ func funcWithForbiddenSpecializeRequirement<T>(_ t: T) {
171169
@_specialize(where T: _Trivial(32), T: _Trivial(64), T: _Trivial, T: _RefCountedObject)
172170
// expected-error@-1{{no type for 'T' can satisfy both 'T : _RefCountedObject' and 'T : _Trivial(32)'}}
173171
// expected-error@-2{{no type for 'T' can satisfy both 'T : _Trivial(64)' and 'T : _Trivial(32)'}}
174-
// expected-error@-3 {{too few generic parameters are specified in '_specialize' attribute (got 0, but expected 1)}}
175-
// expected-note@-4 {{missing constraint for 'T' in '_specialize' attribute}}
176172
@_specialize(where T: _Trivial, T: _Trivial(64))
177173
@_specialize(where T: _RefCountedObject, T: _NativeRefCountedObject)
178174
@_specialize(where Array<T> == Int) // expected-error{{generic signature requires types 'Array<T>' and 'Int' to be the same}}

0 commit comments

Comments
 (0)