RequirementMachine: Fix minimal conformances algorithm to cope with unordered concrete conformances

Author: slavapestov

lib/AST/RequirementMachine/MinimalConformances.cpp

@@ -418,16 +418,25 @@ void MinimalConformances::collectConformanceRules() {
 
   // Sort the list of conformance rules in reverse order; we're going to try
   // to minimize away less canonical rules first.
-  std::sort(ConformanceRules.begin(), ConformanceRules.end(),
-            [&](unsigned lhs, unsigned rhs) -> bool {
-              const auto &lhsRule = System.getRule(lhs);
-              const auto &rhsRule = System.getRule(rhs);
-
-              if (lhsRule.isExplicit() != rhsRule.isExplicit())
-                return !lhsRule.isExplicit();
-
-              return *lhsRule.getLHS().compare(rhsRule.getLHS(), Context) > 0;
-            });
+  std::stable_sort(ConformanceRules.begin(), ConformanceRules.end(),
+                   [&](unsigned lhs, unsigned rhs) -> bool {
+                     const auto &lhsRule = System.getRule(lhs);
+                     const auto &rhsRule = System.getRule(rhs);
+
+                     if (lhsRule.isExplicit() != rhsRule.isExplicit())
+                       return !lhsRule.isExplicit();
+
+                     auto result = lhsRule.getLHS().compare(rhsRule.getLHS(), Context);
+
+                     // Concrete conformance rules are unordered if they name the
+                     // same protocol but have different types. This can come up
+                     // if we have a class inheritance relationship 'Derived : Base',
+                     // and Base conforms to a protocol:
+                     //
+                     //     T.[Base : P] => T
+                     //     T.[Derived : P] => T
+                     return (result ? *result > 0 : 0);
+                   });
 
   Context.ConformanceRulesHistogram.add(ConformanceRules.size());
 }

test/Generics/minimal_conformances_compare_concrete.swift

@@ -0,0 +1,39 @@
+// RUN: %target-swift-frontend -typecheck %s -debug-generic-signatures -requirement-machine-inferred-signatures=on 2>&1 | %FileCheck %s
+// RUN: %target-swift-frontend -typecheck %s -debug-generic-signatures -requirement-machine-inferred-signatures=on -disable-requirement-machine-concrete-contraction 2>&1 | %FileCheck %s
+// RUN: %target-swift-frontend -typecheck %s -debug-generic-signatures -requirement-machine-inferred-signatures=on -disable-requirement-machine-concrete-contraction -dump-requirement-machine 2>&1 | %FileCheck %s --check-prefix=RULE
+
+protocol P {}
+
+class Base : P {}
+
+class Derived : Base {}
+
+struct G<X> {}
+
+// CHECK-LABEL: ExtensionDecl line={{.*}} base=G
+// CHECK-NEXT: Generic signature: <X where X == Derived>
+extension G where X : Base, X : P, X == Derived {}
+
+// RULE: + superclass: τ_0_0 Base
+// RULE: + conforms_to: τ_0_0 P
+// RULE: + same_type: τ_0_0 Derived
+
+// RULE: Rewrite system: {
+// RULE-NEXT: - [P].[P] => [P] [permanent]
+// RULE-NEXT: - τ_0_0.[superclass: Base] => τ_0_0 [explicit]
+// RULE-NEXT: - τ_0_0.[P] => τ_0_0 [explicit]
+// RULE-NEXT: - τ_0_0.[concrete: Derived] => τ_0_0 [explicit]
+// RULE-NEXT: - τ_0_0.[layout: _NativeClass] => τ_0_0
+// RULE-NEXT: - τ_0_0.[superclass: Derived] => τ_0_0
+// RULE-NEXT: - τ_0_0.[concrete: Derived : P] => τ_0_0
+// RULE-NEXT: - τ_0_0.[concrete: Base : P] => τ_0_0
+// RULE-NEXT: }
+
+// Notes:
+//
+// - concrete contraction kills the 'X : P' requirement before building the
+//   rewrite system so this test passes trivially.
+//
+// - without concrete contraction, we used to hit a problem where the sort
+//   in the minimal conformances algorithm would hit the unordered concrete
+//   conformances [concrete: Derived : P] vs [concrete: Base : P].