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.
2025-12-14 20:36:38 +01:00 · 2024-02-09 00:15:06 -05:00
parent ffd651c7b4
commit 70c9f8a47e
15 changed files with 99 additions and 51 deletions
--- a/lib/AST/RequirementMachine/HomotopyReduction.cpp
+++ b/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()) {
--- a/lib/AST/RequirementMachine/PropertyUnification.cpp
+++ b/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(
--- a/test/Constraints/same_types.swift
+++ b/test/Constraints/same_types.swift
@@ -61,21 +61,21 @@ 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 {
@@ -83,10 +83,10 @@ func test4<T: Barrable>(_ t: T) -> Y where T.Bar == Y {
 }

 // 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,
--- a/test/Generics/concrete_conflict.swift
+++ b/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'}}
 }
--- a/test/Generics/concrete_same_type_versus_anyobject.swift
+++ b/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'}}

--- a/test/Generics/requirement_machine_diagnostics.swift
+++ b/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}}
 }
--- a/test/Generics/superclass_and_concrete_requirement.swift
+++ b/test/Generics/superclass_and_concrete_requirement.swift
@@ -12,7 +12,7 @@ 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'}}
@@ -20,7 +20,7 @@ protocol P2 {
 }

 // 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
--- a/test/Generics/superclass_requirement_and_objc_existential.swift
+++ b/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'}}
--- a/test/IDE/complete_where_clause.swift
+++ b/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;
--- a/test/attr/attr_specialize.swift
+++ b/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}}
--- a/test/decl/protocol/req/associated_type_inference_fixed_type.swift
+++ b/test/decl/protocol/req/associated_type_inference_fixed_type.swift
@@ -48,11 +48,12 @@ protocol P6 where A == Never { // expected-error {{no type for 'Self.A' can sati
 struct S6: P6 {} // expected-error {{type 'S6' does not conform to protocol 'P6'}}

 protocol P7a where A == Never {
-  associatedtype A
+  associatedtype A // expected-note {{protocol requires nested type 'A'; add nested type 'A' for conformance}}
 }
 // expected-error@+1 {{no type for 'Self.A' can satisfy both 'Self.A == Never' and 'Self.A == Bool'}}
 protocol P7b: P7a where A == Bool {}
 struct S7: P7b {}
+// expected-error@-1 {{type 'S7' does not conform to protocol 'P7a'}}

 protocol P8a where A == Never {
  associatedtype A
--- a/test/decl/protocol/req/associated_type_inference_fixed_type_experimental_inference.swift
+++ b/test/decl/protocol/req/associated_type_inference_fixed_type_experimental_inference.swift
@@ -57,17 +57,15 @@ protocol P6 where A == Never { // expected-error {{no type for 'Self.A' can sati
  // expected-note@+1 {{protocol requires nested type 'A}}
  associatedtype A: P6
 }
-// CHECK-LABEL: Abstract type witness system for conformance of S6 to P6: {
-// CHECK-NEXT: A => (unresolved){{$}}
-// CHECK-NEXT: }
 struct S6: P6 {} // expected-error {{type 'S6' does not conform to protocol 'P6'}}

 protocol P7a where A == Never {
-  associatedtype A
+  associatedtype A // expected-note {{protocol requires nested type 'A'; add nested type 'A' for conformance}}
 }
 // expected-error@+1 {{no type for 'Self.A' can satisfy both 'Self.A == Never' and 'Self.A == Bool'}}
 protocol P7b: P7a where A == Bool {}
 struct S7: P7b {}
+// expected-error@-1 {{type 'S7' does not conform to protocol 'P7a'}}

 protocol P8a where A == Never {
  associatedtype A // expected-note {{protocol requires nested type 'A'; add nested type 'A' for conformance}}
--- a/validation-test/compiler_crashers_2_fixed/0159-rdar40009245.swift
+++ b/validation-test/compiler_crashers_2_fixed/0159-rdar40009245.swift
@@ -2,7 +2,6 @@

 protocol P {
    associatedtype A : P where A.X == Self
-    // expected-error@-1{{'X' is not a member type of type 'Self.A'}}
    associatedtype X : P where P.A == Self
    // expected-error@-1{{cannot access associated type 'A' from 'P'; use a concrete type or generic parameter base instead}}
 }
--- a/validation-test/compiler_crashers_2_fixed/issue-61031.swift
+++ b/validation-test/compiler_crashers_2_fixed/issue-61031.swift
@@ -0,0 +1,16 @@
+// RUN: %target-typecheck-verify-swift
+
+public struct Wrapped<Values>: Sequence where Values: Sequence {
+    public var values: Values
+
+    public func makeIterator() -> Values.Iterator {
+        values.makeIterator()
+    }
+}
+
+// expected-error@+1 {{reference to generic type 'Array' requires arguments in <...>}}
+extension Wrapped where Values == Array {
+    public init(repeating value: Element, count: Int) {
+        values = Array(repeating: value, count: count)
+    }
+}
--- a/validation-test/compiler_crashers_2_fixed/issue-63997.swift
+++ b/validation-test/compiler_crashers_2_fixed/issue-63997.swift
@@ -0,0 +1,13 @@
+// RUN: %target-typecheck-verify-swift
+
+protocol P {
+  associatedtype A
+}
+
+struct J<A> { }
+
+// expected-error@+2 {{same-type constraint 'Self' == 'J<Self.A>' is recursive}}
+// expected-error@+1 {{no type for 'Self' can satisfy both 'Self == J<Self.A>' and 'Self : P'}}
+extension P where Self == J<A> {
+  static func just(_: A) { }
+}