Merge pull request #61064 from slavapestov/rqm-conflicting-rules-can-remain

RequirementMachine: Leave behind conflicting requirements in the minimized signature

Author: slavapestov

lib/AST/RequirementMachine/HomotopyReduction.cpp

@@ -595,10 +595,18 @@ GenericSignatureErrors RewriteSystem::getErrors() const {
 
   GenericSignatureErrors result;
 
+  if (!ConflictingRules.empty())
+    result |= GenericSignatureErrorFlags::HasInvalidRequirements;
+
   for (const auto &rule : getLocalRules()) {
     if (rule.isPermanent())
       continue;
 
+    // The conditional requirement inference feature imports new protocol
+    // components after the basic rewrite system is already built, so that's
+    // why we end up with imported rules that appear to be in the local rules
+    // slice. Those rules are well-formed, but their isRedundant() bit isn't
+    // set, so we must ignore them here.
     if (!isInMinimizationDomain(rule.getLHS().getRootProtocol()))
       continue;
 
@@ -607,7 +615,7 @@ GenericSignatureErrors RewriteSystem::getErrors() const {
         rule.containsUnresolvedSymbols())
       result |= GenericSignatureErrorFlags::HasInvalidRequirements;
 
-    if (rule.isConflicting() || rule.isRecursive())
+    if (rule.isRecursive())
       result |= GenericSignatureErrorFlags::HasInvalidRequirements;
 
     if (!rule.isRedundant()) {

lib/AST/RequirementMachine/PropertyUnification.cpp

@@ -119,36 +119,49 @@ static void recordRelation(Term key,
 }
 
 /// Given two property rules that conflict because no concrete type
-/// can satisfy both, mark one or both rules conflicting.
-///
-/// The right hand side of one rule must be a suffix of the other
-/// (in which case the longer of the two rules is conflicting) or
-/// the right hand sides are equal (in which case both will be
-/// conflicting).
+/// can satisfy both, record the conflict. If both have the same kind,
+/// mark one or the other as conflicting, but not both.
 void RewriteSystem::recordConflict(unsigned existingRuleID,
                                    unsigned newRuleID) {
-  ConflictingRules.emplace_back(existingRuleID, newRuleID);
-
   auto &existingRule = getRule(existingRuleID);
   auto &newRule = getRule(newRuleID);
 
+  // FIXME: Property map construction shouldn't have to consider imported rules
+  // at all. We need to import the property map from each protocol component,
+  // just like we import rules.
+  if (!isInMinimizationDomain(newRule.getLHS().getRootProtocol()) &&
+      !isInMinimizationDomain(existingRule.getLHS().getRootProtocol())) {
+    return;
+  }
+
+  // Record the conflict for purposes of diagnostics.
+  ConflictingRules.emplace_back(existingRuleID, newRuleID);
+
   if (Debug.contains(DebugFlags::ConflictingRules)) {
     llvm::dbgs() << "Conflicting rules:\n";
     llvm::dbgs() << "- " << existingRule << "\n";
     llvm::dbgs() << "- " << newRule << "\n";
   }
 
-  // The identity conformance rule ([P].[P] => [P]) will conflict with
-  // a concrete type requirement in an invalid protocol declaration
-  // where 'Self' is constrained to a type that does not conform to
-  // the protocol. This rule is permanent, so don't mark it as
-  // conflicting in this case.
-  if (!existingRule.isIdentityConformanceRule() &&
-      existingRule.getRHS().size() >= newRule.getRHS().size())
-    existingRule.markConflicting();
-  if (!newRule.isIdentityConformanceRule() &&
-      newRule.getRHS().size() >= existingRule.getRHS().size())
-    newRule.markConflicting();
+  if (existingRule.getLHS().back().getKind() ==
+      newRule.getLHS().back().getKind()) {
+    assert(!existingRule.isIdentityConformanceRule() &&
+           !newRule.isIdentityConformanceRule());
+
+    // While we don't promise canonical minimization with conflicts,
+    // it's not really a big deal to spit out a generic signature with
+    // conflicts, as long as we diagnosed an error _somewhere_.
+    //
+    // However, the requirement lowering doesn't like to see two
+    // conflicting rules of the same kind, so we rule that out by
+    // marking the shorter rule as the conflict. Otherwise, we just
+    // leave both rules in place.
+    if (existingRule.getRHS().size() > newRule.getRHS().size()) {
+      existingRule.markConflicting();
+    } else {
+      newRule.markConflicting();
+    }
+  }
 }
 
 void PropertyMap::addConformanceProperty(

test/Constraints/same_types.swift

@@ -61,32 +61,32 @@ func test3<T: Fooable, U: Fooable>(_ t: T, u: U) -> (X, X)
 }
 
 // CHECK-LABEL: same_types.(file).fail1(_:u:)@
-// CHECK-NEXT: Generic signature: <T, U where T : Fooable, U : Fooable, T.[Fooable]Foo == U.[Fooable]Foo>
+// CHECK-NEXT: Generic signature: <T, U where T : Fooable, U : Fooable, T.[Fooable]Foo == Y, U.[Fooable]Foo == Y>
 func fail1< // expected-error{{no type for 'T.Foo' can satisfy both 'T.Foo == X' and 'T.Foo == Y'}}
   T: Fooable, U: Fooable
 >(_ t: T, u: U) -> (X, Y)
   where T.Foo == X, U.Foo == Y, T.Foo == U.Foo {
-  return (t.foo, u.foo) // expected-error{{cannot convert return expression of type '(T.Foo, T.Foo)' to return type '(X, Y)'}}
+  return (t.foo, u.foo) // expected-error{{cannot convert return expression of type '(Y, Y)' to return type '(X, Y)'}}
 }
 
 // CHECK-LABEL: same_types.(file).fail2(_:u:)@
-// CHECK-NEXT: Generic signature: <T, U where T : Fooable, U : Fooable, T.[Fooable]Foo == U.[Fooable]Foo>
+// CHECK-NEXT: Generic signature: <T, U where T : Fooable, U : Fooable, T.[Fooable]Foo == X, U.[Fooable]Foo == X>
 func fail2< // expected-error{{no type for 'T.Foo' can satisfy both 'T.Foo == Y' and 'T.Foo == X'}}
   T: Fooable, U: Fooable
 >(_ t: T, u: U) -> (X, Y)
   where T.Foo == U.Foo, T.Foo == X, U.Foo == Y {
-  return (t.foo, u.foo) // expected-error{{cannot convert return expression of type '(T.Foo, T.Foo)' to return type '(X, Y)'}}
+  return (t.foo, u.foo) // expected-error{{cannot convert return expression of type '(X, X)' to return type '(X, Y)'}}
 }
 
 func test4<T: Barrable>(_ t: T) -> Y where T.Bar == Y {
   return t.bar
 }
 
 // CHECK-LABEL: same_types.(file).fail3@
-// CHECK-NEXT: Generic signature: <T where T : Barrable>
+// CHECK-NEXT: Generic signature: <T where T : Barrable, T.[Barrable]Bar == X>
 func fail3<T: Barrable>(_ t: T) -> X // expected-error {{no type for 'T.Bar' can satisfy both 'T.Bar == X' and 'T.Bar : Fooable'}}
   where T.Bar == X {
-  return t.bar // expected-error{{cannot convert return expression of type 'T.Bar' }}
+  return t.bar
 }
 
 func test5<T: Barrable>(_ t: T) -> X where T.Bar.Foo == X {
@@ -122,7 +122,7 @@ func fail5<T: Barrable>(_ t: T) -> (Y, Z)
 }
 
 // CHECK-LABEL: same_types.(file).test8@
-// CHECK-NEXT: Generic signature: <T where T : Fooable>
+// CHECK-NEXT: Generic signature: <T where T : Fooable, T.[Fooable]Foo == X>
 func test8<T: Fooable>(_ t: T) // expected-error{{no type for 'T.Foo' can satisfy both 'T.Foo == Y' and 'T.Foo == X'}}
   where T.Foo == X,
   T.Foo == Y {}
@@ -300,7 +300,7 @@ protocol P4 {
 }
 
 // CHECK-LABEL: same_types.(file).test9@
-// CHECK-NEXT: Generic signature: <T where T : P4>
+// CHECK-NEXT: Generic signature: <T where T : P4, T.[P4]A : P3, T.[P4]A == X>
 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'}}
 
 // Same-type constraint conflict through protocol where clauses.
@@ -318,7 +318,7 @@ struct X5a {}
 struct X5b { }
 
 // CHECK-LABEL: same_types.(file).test9(_:u:)@
-// CHECK-NEXT: Generic signature: <T, U where T : P6, U : P6, T.[P6]Bar == U.[P6]Bar>
+// CHECK-NEXT: Generic signature: <T, U where T : P6, U : P6, T.[P6]Bar == U.[P6]Bar, T.[P6]Bar.[P5]Foo1 == X5b>
 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'}}
   where T.Bar.Foo1 == X5a,
         U.Bar.Foo2 == X5b,

test/Generics/concrete_conflict.swift

@@ -35,7 +35,7 @@ class Class<T> {}
 
 extension Class where T == Bool {
   // CHECK-LABEL: .badRequirement()@
-  // CHECK-NEXT: <T>
+  // CHECK-NEXT: <T where T == Bool>
   func badRequirement() where T == Int { }
   // expected-error@-1 {{no type for 'T' can satisfy both 'T == Int' and 'T == Bool'}}
 }

test/Generics/concrete_same_type_versus_anyobject.swift

@@ -7,7 +7,7 @@ class C {}
 struct G1<T : AnyObject> {}
 
 // CHECK: ExtensionDecl line={{.*}} base=G1
-// CHECK-NEXT: Generic signature: <T>
+// CHECK-NEXT: Generic signature: <T where T : AnyObject, T == S>
 extension G1 where T == S {}
 // expected-error@-1 {{no type for 'T' can satisfy both 'T : AnyObject' and 'T == S'}}
 
@@ -18,7 +18,7 @@ extension G1 where T == C {}
 struct G2<U> {}
 
 // CHECK: ExtensionDecl line={{.*}} base=G2
-// CHECK-NEXT: Generic signature: <U>
+// CHECK-NEXT: Generic signature: <U where U : AnyObject, U == S>
 extension G2 where U == S, U : AnyObject {}
 // expected-error@-1 {{no type for 'U' can satisfy both 'U : AnyObject' and 'U == S'}}
 

test/Generics/requirement_machine_diagnostics.swift

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

test/Generics/superclass_and_concrete_requirement.swift

@@ -12,15 +12,15 @@ protocol P1 {
 }
 
 // CHECK-LABEL: .P2@
-// CHECK-NEXT: Requirement signature: <Self>
+// CHECK-NEXT: Requirement signature: <Self where Self.[P2]T : C, Self.[P2]T == S>
 protocol P2 {
 // expected-error@-1 {{no type for 'Self.T' can satisfy both 'Self.T : C' and 'Self.T == S'}}
 // expected-error@-2 {{no type for 'Self.T' can satisfy both 'Self.T : _NativeClass' and 'Self.T == S'}}
   associatedtype T where T : C, T == S
 }
 
 // CHECK-LABEL: .P3@
-// CHECK-NEXT: Requirement signature: <Self>
+// CHECK-NEXT: Requirement signature: <Self where Self.[P3]T : C, Self.[P3]T == S>
 protocol P3 {
 // expected-error@-1 {{no type for 'Self.T' can satisfy both 'Self.T : C' and 'Self.T == S'}}
 // expected-error@-2 {{no type for 'Self.T' can satisfy both 'Self.T : _NativeClass' and 'Self.T == S'}}
@@ -32,7 +32,7 @@ protocol P4a {
 }
 
 // CHECK-LABEL: .P4@
-// CHECK-NEXT: Requirement signature: <Self where Self.[P4]T : P4>
+// CHECK-NEXT: Requirement signature: <Self where Self.[P4]T : P4, Self.[P4]T.[P4]T == S>
 protocol P4 {
 // expected-error@-1 {{no type for 'Self.T.T' can satisfy both 'Self.T.T == S' and 'Self.T.T : P4'}}
   associatedtype T : P4 where T.T == S
@@ -41,7 +41,7 @@ protocol P4 {
 class D {}
 
 // CHECK-LABEL: .P5@
-// CHECK-NEXT: Requirement signature: <Self where Self.[P5]T == D>
+// CHECK-NEXT: Requirement signature: <Self where Self.[P5]T : C, Self.[P5]T == D>
 protocol P5 {
 // expected-error@-1 {{no type for 'Self.T' can satisfy both 'Self.T : D' and 'Self.T : C'}}
   associatedtype T where T : C, T == D

test/Generics/superclass_requirement_and_objc_existential.swift

@@ -21,18 +21,18 @@ func f1<T : Q>(_: T) where T.A : C, T.A == any (C & P1) {}
 /// These are not allowed.
 
 // CHECK-LABEL: .f2@
-// CHECK-NEXT: Generic signature: <T where T : Q>
+// CHECK-NEXT: Generic signature: <T where T : Q, T.[Q]A : C, T.[Q]A == any P1>
 func f2<T : Q>(_: T) where T.A : C, T.A == any P1 {}
 // expected-error@-1 {{no type for 'T.A' can satisfy both 'T.A : C' and 'T.A == any P1'}}
 
 // CHECK-LABEL: .f3@
-// CHECK-NEXT: Generic signature: <T where T : Q>
+// CHECK-NEXT: Generic signature: <T where T : Q, T.[Q]A : C, T.[Q]A == any C & P2>
 func f3<T : Q>(_: T) where T.A : C, T.A == any (C & P2) {}
 // expected-error@-1 {{no type for 'T.A' can satisfy both 'T.A : C' and 'T.A == any C & P2'}}
 // expected-error@-2 {{no type for 'T.A' can satisfy both 'T.A : _NativeClass' and 'T.A == any C & P2'}}
 
 // CHECK-LABEL: .f4@
-// CHECK-NEXT: Generic signature: <T where T : Q>
+// CHECK-NEXT: Generic signature: <T where T : Q, T.[Q]A : C, T.[Q]A == any C & P3>
 func f4<T : Q>(_: T) where T.A : C, T.A == any (C & P3) {}
 // expected-error@-1 {{no type for 'T.A' can satisfy both 'T.A : C' and 'T.A == any C & P3'}}
 // 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

@@ -39,7 +39,7 @@
 // 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
 // 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
 // RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=EXT_ASSOC_MEMBER_1 | %FileCheck %s -check-prefix=EXT_ASSOC_MEMBER
-// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=EXT_ASSOC_MEMBER_2 | %FileCheck %s -check-prefix=EXT_ASSOC_MEMBER
+// 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
 // RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=EXT_SECONDTYPE | %FileCheck %s -check-prefix=EXT_SECONDTYPE
 // 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
 
@@ -234,8 +234,11 @@ extension WithAssoc where T.#^EXT_ASSOC_MEMBER_1^#
 // EXT_ASSOC_MEMBER-DAG: Decl[AssociatedType]/CurrNominal:   Q;
 // EXT_ASSOC_MEMBER-DAG: Keyword/None:                       Type[#Self.T.Type#];
 
+// This is kind of funny because we parse it as 'Int == T', so completing 'T'
+// shows members of 'Int'.
 extension WithAssoc where Int == T.#^EXT_ASSOC_MEMBER_2^# 
-// Same as EXT_ASSOC_MEMBER
+// EXT_ASSOC_MEMBER_2: Begin completions, {{.*}} items
+// EXT_ASSOC_MEMBER_2: Keyword/None:                           Type[#Int.Type#];
 
 extension WithAssoc where Int == #^EXT_SECONDTYPE^# 
 // EXT_SECONDTYPE-DAG: Decl[AssociatedType]/CurrNominal:   T;

test/attr/attr_specialize.swift

@@ -80,8 +80,6 @@ struct AThing : Thing {}
 // CHECK: @_specialize(exported: false, kind: full, where T == AThing)
 @_specialize(where T == AThing)
 @_specialize(where T == Int) // expected-error{{no type for 'T' can satisfy both 'T == Int' and 'T : Thing'}}
-// expected-error@-1 {{too few generic parameters are specified in '_specialize' attribute (got 0, but expected 1)}}
-// expected-note@-2 {{missing constraint for 'T' in '_specialize' attribute}}
 
 func oneRequirement<T : Thing>(_ t: T) {}
 
@@ -171,8 +169,6 @@ func funcWithForbiddenSpecializeRequirement<T>(_ t: T) {
 @_specialize(where T: _Trivial(32), T: _Trivial(64), T: _Trivial, T: _RefCountedObject)
 // expected-error@-1{{no type for 'T' can satisfy both 'T : _RefCountedObject' and 'T : _Trivial(32)'}}
 // expected-error@-2{{no type for 'T' can satisfy both 'T : _Trivial(64)' and 'T : _Trivial(32)'}}
-// expected-error@-3 {{too few generic parameters are specified in '_specialize' attribute (got 0, but expected 1)}}
-// expected-note@-4 {{missing constraint for 'T' in '_specialize' attribute}}
 @_specialize(where T: _Trivial, T: _Trivial(64))
 @_specialize(where T: _RefCountedObject, T: _NativeRefCountedObject)
 @_specialize(where Array<T> == Int) // expected-error{{generic signature requires types 'Array<T>' and 'Int' to be the same}}