[ConstraintSystem] Adjust recording of "fixed" requirements to avoid conflicts

Currently it's possible to have a type conflict between different
requirements deduced as the same type which leads to incorrect
diagnostics. To mitigate that let's adjust how "fixed" requirements
are stored - instead of using resolved type for the left-hand side,
let's use originating generic parameter type.

Author: xedin

lib/Sema/CSSimplify.cpp

@@ -2339,7 +2339,7 @@ ConstraintSystem::matchExistentialTypes(Type type1, Type type2,
 
               auto *fix = fixRequirementFailure(*this, type1, type2, locator);
               if (fix && !recordFix(fix)) {
-                recordFixedRequirement(type1, RequirementKind::Layout, type2);
+                recordFixedRequirement(getConstraintLocator(locator), type2);
                 return getTypeMatchSuccess();
               }
             }
@@ -3875,14 +3875,13 @@ bool ConstraintSystem::repairFailures(
     if (hasConversionOrRestriction(ConversionRestrictionKind::DeepEquality))
       break;
 
-    auto reqElt = elt.castTo<LocatorPathElt::AnyRequirement>();
-    auto reqKind = reqElt.getRequirementKind();
+    auto *reqLoc = getConstraintLocator(locator);
 
-    if (hasFixedRequirement(lhs, reqKind, rhs))
+    if (isFixedRequirement(reqLoc, rhs))
       return true;
 
     if (auto *fix = fixRequirementFailure(*this, lhs, rhs, anchor, path)) {
-      recordFixedRequirement(lhs, reqKind, rhs);
+      recordFixedRequirement(reqLoc, rhs);
       conversionsOrFixes.push_back(fix);
     }
     break;
@@ -5404,7 +5403,7 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyConformsToConstraint(
   auto protocolTy = protocol->getDeclaredType();
 
   // If this conformance has been fixed already, let's just consider this done.
-  if (hasFixedRequirement(type, RequirementKind::Conformance, protocolTy))
+  if (isFixedRequirement(getConstraintLocator(locator), protocolTy))
     return SolutionKind::Solved;
 
   // If this is a generic requirement let's try to record that
@@ -5491,7 +5490,7 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyConformsToConstraint(
         auto impact = assessRequirementFailureImpact(*this, rawType, locator);
         if (!recordFix(fix, impact)) {
           // Record this conformance requirement as "fixed".
-          recordFixedRequirement(type, RequirementKind::Conformance,
+          recordFixedRequirement(getConstraintLocator(anchor, path),
                                  protocolTy);
           return SolutionKind::Solved;
         }

lib/Sema/ConstraintSystem.cpp

@@ -4810,3 +4810,103 @@ SourceLoc constraints::getLoc(ASTNode anchor) {
 SourceRange constraints::getSourceRange(ASTNode anchor) {
   return anchor.getSourceRange();
 }
+
+static Optional<Requirement> getRequirement(ConstraintSystem &cs,
+                                            ConstraintLocator *reqLocator) {
+  auto reqLoc = reqLocator->getLastElementAs<LocatorPathElt::AnyRequirement>();
+  if (!reqLoc)
+    return None;
+
+  if (reqLoc->isConditionalRequirement()) {
+    auto path = reqLocator->getPath();
+    auto *typeReqLoc =
+        cs.getConstraintLocator(reqLocator->getAnchor(), path.drop_back());
+
+    auto conformances = cs.getCheckedConformances();
+    auto result = llvm::find_if(
+        conformances,
+        [&typeReqLoc](
+            const std::pair<ConstraintLocator *, ProtocolConformanceRef>
+                &conformance) { return conformance.first == typeReqLoc; });
+    assert(result != conformances.end());
+
+    auto conformance = result->second;
+    assert(conformance.isConcrete());
+
+    return conformance.getConditionalRequirements()[reqLoc->getIndex()];
+  }
+
+  if (auto openedGeneric =
+          reqLocator->findLast<LocatorPathElt::OpenedGeneric>()) {
+    auto signature = openedGeneric->getSignature();
+    return signature->getRequirements()[reqLoc->getIndex()];
+  }
+
+  return None;
+}
+
+static Optional<std::pair<GenericTypeParamType *, RequirementKind>>
+getRequirementInfo(ConstraintSystem &cs, ConstraintLocator *reqLocator) {
+  auto requirement = getRequirement(cs, reqLocator);
+  if (!requirement)
+    return None;
+
+  auto *GP = requirement->getFirstType()->getAs<GenericTypeParamType>();
+  if (!GP)
+    return None;
+
+  auto path = reqLocator->getPath();
+  auto iter = path.rbegin();
+  auto openedGeneric =
+      reqLocator->findLast<LocatorPathElt::OpenedGeneric>(iter);
+  assert(openedGeneric);
+
+  auto newPath = path.drop_back(iter - path.rbegin() + 1);
+  auto *baseLoc = cs.getConstraintLocator(reqLocator->getAnchor(), newPath);
+
+  auto openedTypes = cs.getOpenedTypes();
+  auto substitutions = llvm::find_if(
+      openedTypes,
+      [&baseLoc](
+          const std::pair<ConstraintLocator *, ArrayRef<OpenedType>> &entry) {
+        return entry.first == baseLoc;
+      });
+
+  if (substitutions == openedTypes.end())
+    return None;
+
+  auto replacement =
+      llvm::find_if(substitutions->second, [&GP](const OpenedType &entry) {
+        auto *typeVar = entry.second;
+        return typeVar->getImpl().getGenericParameter() == GP;
+      });
+
+  if (replacement == substitutions->second.end())
+    return None;
+
+  auto *repr = cs.getRepresentative(replacement->second);
+  return std::make_pair(repr->getImpl().getGenericParameter(),
+                        requirement->getKind());
+}
+
+bool ConstraintSystem::isFixedRequirement(ConstraintLocator *reqLocator,
+                                          Type requirementTy) {
+  if (auto reqInfo = getRequirementInfo(*this, reqLocator)) {
+    auto *GP = reqInfo->first;
+    auto reqKind = static_cast<unsigned>(reqInfo->second);
+    return FixedRequirements.count(
+        std::make_tuple(GP, reqKind, requirementTy.getPointer()));
+  }
+
+  return false;
+}
+
+void ConstraintSystem::recordFixedRequirement(ConstraintLocator *reqLocator,
+                                              Type requirementTy) {
+  if (auto reqInfo = getRequirementInfo(*this, reqLocator)) {
+    auto *GP = reqInfo->first;
+    auto reqKind = static_cast<unsigned>(reqInfo->second);
+    FixedRequirements.insert(
+        std::make_tuple(GP, reqKind, requirementTy.getPointer()));
+  }
+}

lib/Sema/ConstraintSystem.h

@@ -1947,20 +1947,13 @@ class ConstraintSystem {
 
   /// The list of all generic requirements fixed along the current
   /// solver path.
-  using FixedRequirement = std::tuple<TypeBase *, RequirementKind, TypeBase *>;
-  SmallVector<FixedRequirement, 4> FixedRequirements;
+  using FixedRequirement =
+      std::tuple<GenericTypeParamType *, unsigned, TypeBase *>;
+  llvm::SmallSetVector<FixedRequirement, 4> FixedRequirements;
 
-  bool hasFixedRequirement(Type lhs, RequirementKind kind, Type rhs) {
-    auto reqInfo = std::make_tuple(lhs.getPointer(), kind, rhs.getPointer());
-    return llvm::any_of(
-        FixedRequirements,
-        [&reqInfo](const FixedRequirement &entry) { return entry == reqInfo; });
-  }
-
-  void recordFixedRequirement(Type lhs, RequirementKind kind, Type rhs) {
-    FixedRequirements.push_back(
-        std::make_tuple(lhs.getPointer(), kind, rhs.getPointer()));
-  }
+  bool isFixedRequirement(ConstraintLocator *reqLocator, Type requirementTy);
+  void recordFixedRequirement(ConstraintLocator *reqLocator,
+                              Type requirementTy);
 
   /// A mapping from constraint locators to the opened existential archetype
   /// used for the 'self' of an existential type.
@@ -3546,6 +3539,19 @@ class ConstraintSystem {
                           const DeclRefExpr *base = nullptr,
                           OpenedTypeMap *replacements = nullptr);
 
+  /// Retrieve a list of conformances established along the current solver path.
+  ArrayRef<std::pair<ConstraintLocator *, ProtocolConformanceRef>>
+  getCheckedConformances() const {
+    return CheckedConformances;
+  }
+
+  /// Retrieve a list of generic parameter types solver has "opened" (replaced
+  /// with a type variable) along the current path.
+  ArrayRef<std::pair<ConstraintLocator *, ArrayRef<OpenedType>>>
+  getOpenedTypes() const {
+    return OpenedTypes;
+  }
+
 private:
   /// Adjust the constraint system to accomodate the given selected overload, and
   /// recompute the type of the referenced declaration.

test/AutoDiff/Sema/differentiable_func_type.swift

@@ -175,10 +175,10 @@ extension Vector: Differentiable where T: Differentiable {
   mutating func move(along direction: TangentVector) { fatalError() }
 }
 
-// expected-note@+1 2 {{candidate requires that 'Int' conform to 'Differentiable' (requirement specified as 'T' == 'Differentiable')}}
+// expected-note@+1 2 {{found this candidate}}
 func inferredConformancesGeneric<T, U>(_: @differentiable (Vector<T>) -> Vector<U>) {}
 
-// expected-note  @+5 2 {{candidate requires that 'Int' conform to 'Differentiable' (requirement specified as 'T' == 'Differentiable')}}
+// expected-note  @+5 2 {{found this candidate}}
 // expected-error @+4 {{generic signature requires types 'Vector<T>' and 'Vector<T>.TangentVector' to be the same}}
 // expected-error @+3 {{generic signature requires types 'Vector<U>' and 'Vector<U>.TangentVector' to be the same}}
 // expected-error @+2 {{parameter type 'Vector<T>' does not conform to 'Differentiable' and satisfy 'Vector<T> == Vector<T>.TangentVector', but the enclosing function type is '@differentiable(linear)'}}
@@ -210,10 +210,10 @@ extension Linear: Differentiable where T: Differentiable, T == T.TangentVector {
   typealias TangentVector = Self
 }
 
-// expected-note @+1 2 {{candidate requires that 'Int' conform to 'Differentiable' (requirement specified as 'T' == 'Differentiable')}}
+// expected-note @+1 2 {{found this candidate}}
 func inferredConformancesGeneric<T, U>(_: @differentiable (Linear<T>) -> Linear<U>) {}
 
-// expected-note @+1 2 {{candidate requires that 'Int' conform to 'Differentiable' (requirement specified as 'T' == 'Differentiable')}}
+// expected-note @+1 2 {{found this candidate}}
 func inferredConformancesGenericLinear<T, U>(_: @differentiable(linear) (Linear<T>) -> Linear<U>) {}
 
 func nondiff(x: Linear<Int>) -> Linear<Int> {}

test/Constraints/diagnostics.swift

@@ -687,7 +687,7 @@ enum AssocTest {
   case one(Int)
 }
 
-if AssocTest.one(1) == AssocTest.one(1) {} // expected-error{{binary operator '==' cannot be applied to two 'AssocTest' operands}}
+if AssocTest.one(1) == AssocTest.one(1) {} // expected-error{{referencing operator function '==' on 'Equatable' requires that 'AssocTest' conform to 'Equatable'}}
 // expected-note @-1 {{binary operator '==' cannot be synthesized for enums with associated values}}
 
 
@@ -1100,9 +1100,13 @@ func rdar17170728() {
   }
 
   let _ = [i, j, k].reduce(0 as Int?) {
+    // expected-error@-1 3 {{cannot convert value of type 'Int?' to expected element type 'Int'}}
     $0 && $1 ? $0 + $1 : ($0 ? $0 : ($1 ? $1 : nil))
-    // expected-error@-1 {{binary operator '+' cannot be applied to two 'Int?' operands}}
-    // expected-error@-2 4 {{optional type 'Int?' cannot be used as a boolean; test for '!= nil' instead}}
+    // expected-error@-1 2 {{type 'Int' cannot be used as a boolean; test for '!= 0' instead}}
+    // expected-error@-2   {{value of optional type 'Int?' must be unwrapped to a value of type 'Int'}}
+    // expected-error@-3 2 {{optional type 'Int?' cannot be used as a boolean; test for '!= nil' instead}}
+    // expected-note@-4:16 {{coalesce using '??' to provide a default when the optional value contains 'nil'}}
+    // expected-note@-5:16 {{force-unwrap using '!' to abort execution if the optional value contains 'nil'}}
   }
 }
 

test/Constraints/operator.swift

@@ -288,5 +288,6 @@ func rdar_62054241() {
 
   func test(_ arr: [Foo]) -> [Foo] {
     return arr.sorted(by: <) // expected-error {{no exact matches in reference to operator function '<'}}
+    // expected-note@-1 {{found candidate with type '(Foo, Foo) -> Bool'}}
   }
 }

test/Generics/deduction.swift

@@ -247,8 +247,7 @@ protocol Addable {
   static func +(x: Self, y: Self) -> Self
 }
 func addAddables<T : Addable, U>(_ x: T, y: T, u: U) -> T {
-  // FIXME(diagnostics): This should report the "no exact matches" diagnostic.
-  u + u // expected-error{{referencing operator function '+' on 'RangeReplaceableCollection' requires that 'U' conform to 'RangeReplaceableCollection'}}
+  u + u // expected-error{{binary operator '+' cannot be applied to two 'U' operands}}
   return x+y
 }
 

test/Sema/struct_equatable_hashable.swift

@@ -114,7 +114,8 @@ struct StructWithoutExplicitConformance {
 }
 
 func structWithoutExplicitConformance() {
-  if StructWithoutExplicitConformance(a: 1, b: "b") == StructWithoutExplicitConformance(a: 2, b: "a") { } // expected-error{{binary operator '==' cannot be applied to two 'StructWithoutExplicitConformance' operands}}
+  // This diagnostic is about `Equatable` because it's considered the best possible solution among other ones for operator `==`.
+  if StructWithoutExplicitConformance(a: 1, b: "b") == StructWithoutExplicitConformance(a: 2, b: "a") { } // expected-error{{referencing operator function '==' on 'Equatable' requires that 'StructWithoutExplicitConformance' conform to 'Equatable'}}
 }
 
 // Structs with non-hashable/equatable stored properties don't derive conformance.

test/expr/unary/keypath/salvage-with-other-type-errors.swift

@@ -6,14 +6,14 @@
 struct P<T: K> { }
 
 struct S {
-    init<B>(_ a: P<B>) {
+    init<B>(_ a: P<B>) { // expected-note {{in call to initializer}}
         fatalError()
     }
 }
 
 protocol K { }
 
-func + <Object>(lhs: KeyPath<A, Object>, rhs: String) -> P<Object> { // expected-note {{where 'Object' = 'String'}}
+func + <Object>(lhs: KeyPath<A, Object>, rhs: String) -> P<Object> {
     fatalError()
 }
 
@@ -27,7 +27,9 @@ struct A {
 }
 
 extension A: K {
-    static let j = S(\A.id + "id") // expected-error {{operator function '+' requires that 'String' conform to 'K'}}
+  static let j = S(\A.id + "id") // expected-error {{generic parameter 'B' could not be inferred}}
+  // expected-error@-1 {{binary operator '+' cannot be applied to operands of type 'KeyPath<A, String>' and 'String'}}
+  // expected-note@-2 {{overloads for '+' exist with these partially matching parameter lists: (String, String)}}
 }
 
 // SR-5034

test/stdlib/UnicodeScalarDiagnostics.swift

@@ -12,7 +12,7 @@ func test_UnicodeScalarDoesNotImplementArithmetic(_ us: UnicodeScalar, i: Int) {
   let a3 = "a" * "b" // expected-error {{binary operator '*' cannot be applied to two 'String' operands}}
   let a4 = "a" / "b" // expected-error {{binary operator '/' cannot be applied to two 'String' operands}}
 
-  let b1 = us + us // expected-error {{referencing operator function '+' on 'RangeReplaceableCollection' requires that 'UnicodeScalar' (aka 'Unicode.Scalar') conform to 'RangeReplaceableCollection'}}
+  let b1 = us + us // expected-error {{binary operator '+' cannot be applied to two 'UnicodeScalar' (aka 'Unicode.Scalar') operands}}
   let b2 = us - us // expected-error {{binary operator '-' cannot be applied to two 'UnicodeScalar' (aka 'Unicode.Scalar') operands}}
   let b3 = us * us // expected-error {{binary operator '*' cannot be applied to two 'UnicodeScalar' (aka 'Unicode.Scalar') operands}}
   let b4 = us / us // expected-error {{binary operator '/' cannot be applied to two 'UnicodeScalar' (aka 'Unicode.Scalar') operands}}