Merge pull request #65806 from xedin/rdar-108977234

[CSSimplify] Detect and diagnose conformance failures related to AnyHashable conversion

Author: xedin

lib/Sema/CSDiagnostics.cpp

@@ -24,6 +24,7 @@
 #include "swift/AST/DiagnosticsClangImporter.h"
 #include "swift/AST/ExistentialLayout.h"
 #include "swift/AST/Expr.h"
+#include "swift/AST/GenericEnvironment.h"
 #include "swift/AST/GenericSignature.h"
 #include "swift/AST/ImportCache.h"
 #include "swift/AST/Initializer.h"
@@ -88,6 +89,54 @@ Type FailureDiagnostic::getRawType(ASTNode node) const {
   return S.getType(node);
 }
 
+Type FailureDiagnostic::resolveType(Type rawType, bool reconstituteSugar,
+                                    bool wantRValue) const {
+  rawType = rawType.transform([&](Type type) -> Type {
+    if (auto *typeVar = type->getAs<TypeVariableType>()) {
+      auto resolvedType = S.simplifyType(typeVar);
+
+      if (!resolvedType->hasUnresolvedType())
+        return resolvedType;
+
+      // If type variable was simplified to an unresolved pack expansion
+      // type, let's examine its original pattern type because it could
+      // contain type variables replaceable with their generic parameter
+      // types.
+      if (auto *expansion = resolvedType->getAs<PackExpansionType>()) {
+        auto *locator = typeVar->getImpl().getLocator();
+        auto *openedExpansionTy =
+            locator->castLastElementTo<LocatorPathElt::PackExpansionType>()
+                .getOpenedType();
+        auto patternType = resolveType(openedExpansionTy->getPatternType());
+        return PackExpansionType::get(patternType, expansion->getCountType());
+      }
+
+      Type GP = typeVar->getImpl().getGenericParameter();
+      return resolvedType->is<UnresolvedType>() && GP ? GP : resolvedType;
+    }
+
+    if (type->hasElementArchetype()) {
+      auto *env = getDC()->getGenericEnvironmentOfContext();
+      return env->mapElementTypeIntoPackContext(type);
+    }
+
+    if (auto *packType = type->getAs<PackType>()) {
+      if (packType->getNumElements() == 1) {
+        auto eltType = resolveType(packType->getElementType(0));
+        if (auto expansion = eltType->getAs<PackExpansionType>())
+          return expansion->getPatternType();
+      }
+    }
+
+    return type->isPlaceholder() ? Type(type->getASTContext().TheUnresolvedType)
+                                 : type;
+  });
+
+  if (reconstituteSugar)
+    rawType = rawType->reconstituteSugar(/*recursive*/ true);
+  return wantRValue ? rawType->getRValueType() : rawType;
+}
+
 template <typename... ArgTypes>
 InFlightDiagnostic
 FailureDiagnostic::emitDiagnostic(ArgTypes &&... Args) const {

lib/Sema/CSDiagnostics.h

@@ -98,48 +98,7 @@ class FailureDiagnostic {
 
   /// Resolve type variables present in the raw type, if any.
   Type resolveType(Type rawType, bool reconstituteSugar = false,
-                   bool wantRValue = true) const {
-    rawType = rawType.transform([&](Type type) -> Type {
-      if (auto *typeVar = type->getAs<TypeVariableType>()) {
-        auto resolvedType = S.simplifyType(typeVar);
-
-        if (!resolvedType->hasUnresolvedType())
-          return resolvedType;
-
-        // If type variable was simplified to an unresolved pack expansion
-        // type, let's examine its original pattern type because it could
-        // contain type variables replaceable with their generic parameter
-        // types.
-        if (auto *expansion = resolvedType->getAs<PackExpansionType>()) {
-          auto *locator = typeVar->getImpl().getLocator();
-          auto *openedExpansionTy =
-              locator->castLastElementTo<LocatorPathElt::PackExpansionType>()
-                  .getOpenedType();
-          auto patternType = resolveType(openedExpansionTy->getPatternType());
-          return PackExpansionType::get(patternType, expansion->getCountType());
-        }
-
-        Type GP = typeVar->getImpl().getGenericParameter();
-        return resolvedType->is<UnresolvedType>() && GP ? GP : resolvedType;
-      }
-
-      if (auto *packType = type->getAs<PackType>()) {
-        if (packType->getNumElements() == 1) {
-          auto eltType = resolveType(packType->getElementType(0));
-          if (auto expansion = eltType->getAs<PackExpansionType>())
-            return expansion->getPatternType();
-        }
-      }
-
-      return type->isPlaceholder()
-                 ? Type(type->getASTContext().TheUnresolvedType)
-                 : type;
-    });
-
-    if (reconstituteSugar)
-      rawType = rawType->reconstituteSugar(/*recursive*/ true);
-    return wantRValue ? rawType->getRValueType() : rawType;
-  }
+                   bool wantRValue = true) const;
 
   template <typename... ArgTypes>
   InFlightDiagnostic emitDiagnostic(ArgTypes &&... Args) const;

lib/Sema/CSSimplify.cpp

@@ -2785,6 +2785,23 @@ assessRequirementFailureImpact(ConstraintSystem &cs, Type requirementType,
         impact += choiceImpact - 1;
     }
   }
+
+  // If this requirement is associated with a call that is itself
+  // incorrect, let's increase impact to indicate that this failure
+  // has a compounding effect on viability of the overload choice it
+  // comes from.
+  if (locator.endsWith<LocatorPathElt::AnyRequirement>()) {
+    if (auto *expr = getAsExpr(anchor)) {
+      if (auto *call = getAsExpr<ApplyExpr>(cs.getParentExpr(expr))) {
+        if (call->getFn() == expr &&
+            llvm::any_of(cs.getFixes(), [&](const auto &fix) {
+              return getAsExpr(fix->getAnchor()) == call;
+            }))
+          impact += 2;
+      }
+    }
+  }
+
   return impact;
 }
 
@@ -8436,6 +8453,15 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyConformsToConstraint(
         return SolutionKind::Solved;
     }
 
+    // Conformance constraint that is introduced by an implicit conversion
+    // for example to `AnyHashable`.
+    if (kind == ConstraintKind::ConformsTo &&
+        loc->isLastElement<LocatorPathElt::ApplyArgToParam>()) {
+      auto *fix = AllowArgumentMismatch::create(*this, type, protocolTy, loc);
+      return recordFix(fix, /*impact=*/2) ? SolutionKind::Error
+                                          : SolutionKind::Solved;
+    }
+
     // If this is an implicit Hashable conformance check generated for each
     // index argument of the keypath subscript component, we could just treat
     // it as though it conforms.

lib/Sema/ConstraintSystem.cpp

@@ -4338,9 +4338,28 @@ static void diagnoseOperatorAmbiguity(ConstraintSystem &cs,
     if (overloadType->hasTypeVariable())
       continue;
 
-    if (auto *fnType = overloadType->getAs<FunctionType>())
-      parameters.insert(
-          FunctionType::getParamListAsString(fnType->getParams()));
+    auto overloadFnTy = overloadType->getAs<FunctionType>();
+    if (!overloadFnTy)
+      continue;
+
+    // If arguments to all parameters have been fixed then there is nothing
+    // to note about in this overload.
+    std::set<unsigned> fixedParams;
+    llvm::for_each(solution.Fixes, [&](const ConstraintFix *fix) {
+      auto *locator = fix->getLocator();
+      if (getAsExpr(locator->getAnchor()) != applyExpr)
+        return;
+
+      if (auto argLoc = locator->findLast<LocatorPathElt::ApplyArgToParam>()) {
+        fixedParams.insert(argLoc->getParamIdx());
+      }
+    });
+
+    if (fixedParams.size() == overloadFnTy->getNumParams())
+      continue;
+
+    parameters.insert(
+        FunctionType::getParamListAsString(overloadFnTy->getParams()));
   }
 
   // All of the overload choices had generic parameters like `Self`.

test/Constraints/bridging.swift

@@ -275,7 +275,6 @@ func rdar19831698() {
   var v71 = true + 1.0 // expected-error{{binary operator '+' cannot be applied to operands of type 'Bool' and 'Double'}}
 // expected-note@-1{{overloads for '+'}}
   var v72 = true + true // expected-error{{binary operator '+' cannot be applied to two 'Bool' operands}}
-  // expected-note@-1{{overloads for '+'}}
   var v73 = true + [] // expected-error@:13 {{cannot convert value of type 'Bool' to expected argument type 'Array<Bool>'}}
   var v75 = true + "str" // expected-error@:13 {{cannot convert value of type 'Bool' to expected argument type 'String'}}
 }

test/Constraints/diagnostics.swift

@@ -661,10 +661,7 @@ example21890157.property = "confusing"  // expected-error {{value of optional ty
 
 
 struct UnaryOp {}
-
 _ = -UnaryOp() // expected-error {{unary operator '-' cannot be applied to an operand of type 'UnaryOp'}}
-// expected-note@-1 {{overloads for '-' exist with these partially matching parameter lists: (Double), (Float)}}
-
 
 // <rdar://problem/23433271> Swift compiler segfault in failure diagnosis
 func f23433271(_ x : UnsafePointer<Int>) {}
@@ -1551,3 +1548,12 @@ func rdar86611718(list: [Int]) {
   String(list.count())
   // expected-error@-1 {{cannot call value of non-function type 'Int'}}
 }
+
+// rdar://108977234 - failed to produce diagnostic when argument to AnyHashable parameter doesn't conform to Hashable protocol
+do {
+  struct NonHashable {}
+
+  func test(result: inout [AnyHashable], value: NonHashable) {
+    result.append(value) // expected-error {{argument type 'NonHashable' does not conform to expected type 'Hashable'}}
+  }
+}

test/Constraints/fixes.swift

@@ -363,5 +363,4 @@ func testKeyPathSubscriptArgFixes(_ fn: @escaping () -> Int) {
 // https://github.com/apple/swift/issues/54865
 func f_54865(a: Any, _ str: String?) {
   a == str // expected-error {{binary operator '==' cannot be applied to operands of type 'Any' and 'String?'}}
-  // expected-note@-1 {{overloads for '==' exist with these partially matching parameter lists: (String, String)}}
 }

test/Constraints/generics.swift

@@ -1017,6 +1017,7 @@ func test_requirement_failures_in_ambiguous_context() {
   func f1<T: Equatable>(_: T, _: T) {} // expected-note {{where 'T' = 'A'}}
 
   f1(A(), B()) // expected-error {{local function 'f1' requires that 'A' conform to 'Equatable'}}
+  // expected-error@-1 {{cannot convert value of type 'B' to expected argument type 'A'}}
 
   func f2<T: P_56173, U: P_56173>(_: T, _: U) {}
   // expected-note@-1 {{candidate requires that 'B' conform to 'P_56173' (requirement specified as 'U' : 'P_56173')}}

test/Constraints/operator.swift

@@ -219,7 +219,9 @@ func rdar46459603() {
   var arr = ["key": e]
 
   _ = arr.values == [e]
-  // expected-error@-1 {{binary operator '==' cannot be applied to operands of type 'Dictionary<String, E>.Values' and '[E]'}}
+  // expected-error@-1 {{referencing operator function '==' on 'Equatable' requires that 'Dictionary<String, E>.Values' conform to 'Equatable'}}
+  // expected-error@-2 {{cannot convert value of type '[E]' to expected argument type 'Dictionary<String, E>.Values'}}
+
   _ = [arr.values] == [[e]]
   // expected-error@-1 {{referencing operator function '==' on 'Array' requires that 'E' conform to 'Equatable'}} expected-note@-1 {{binary operator '==' cannot be synthesized for enums with associated values}}
   // expected-error@-2 {{cannot convert value of type 'Dictionary<String, E>.Values' to expected element type '[E]'}}

test/Constraints/optional.swift

@@ -436,7 +436,6 @@ func test_force_unwrap_not_being_too_eager() {
 func invalidOptionalChaining(a: Any) {
   a == "="? // expected-error {{cannot use optional chaining on non-optional value of type 'String'}}
   // expected-error@-1 {{binary operator '==' cannot be applied to operands of type 'Any' and 'String?'}}
-  // expected-note@-2 {{overloads for '==' exist}}
 }
 
 /// https://github.com/apple/swift/issues/54739

test/Constraints/pack-expansion-expressions.swift

@@ -64,18 +64,16 @@ func outerArchetype<each T, U>(t: repeat each T, u: U) where repeat each T: P {
 func sameElement<each T, U>(t: repeat each T, u: U) where repeat each T: P, repeat each T == U {
 // expected-error@-1{{same-element requirements are not yet supported}}
 
-  // FIXME: Opened element archetypes in diagnostics
   let _: (repeat each T) = (repeat (each t).f(u))
-  // expected-error@-1 {{cannot convert value of type 'U' to expected argument type 'τ_1_0'}}
+  // expected-error@-1 {{cannot convert value of type 'U' to expected argument type 'each T'}}
 }
 
 func forEachEach<each C, U>(c: repeat each C, function: (U) -> Void)
     where repeat each C: Collection, repeat (each C).Element == U {
     // expected-error@-1{{same-element requirements are not yet supported}}
 
-  // FIXME: Opened element archetypes in diagnostics
   _ = (repeat (each c).forEach(function))
-  // expected-error@-1 {{cannot convert value of type '(U) -> Void' to expected argument type '(τ_1_0.Element) throws -> Void'}}
+  // expected-error@-1 {{cannot convert value of type '(U) -> Void' to expected argument type '(each C.Element) throws -> Void'}}
 }
 
 func typeReprPacks<each T: ExpressibleByIntegerLiteral>(_ t: repeat each T) {
@@ -332,6 +330,15 @@ func test_pack_expansions_with_closures() {
     takesVariadicFunction { y in fn(x, y) } // Ok
     takesVariadicFunction { y, z in fn(y, z) } // Ok
   }
+
+  // rdar://108977234 - invalid error non-pack type instead of missing `Hashable` conformance
+  func testEscapingCapture<each T>(_ t: repeat each T) -> () -> [AnyHashable] {
+    return {
+      var result = [AnyHashable]()
+      repeat result.append(each t) // expected-error {{argument type 'each T' does not conform to expected type 'Hashable'}}
+      return result
+    }
+  }
 }
 
 // rdar://107151854 - crash on invalid due to specialized pack expansion

test/StringProcessing/Parse/forward-slash-regex.swift

@@ -131,7 +131,6 @@ _ = /x/.../y/
 
 _ = /x/...
 // expected-error@-1 {{unary operator '...' cannot be applied to an operand of type 'Regex<Substring>'}}
-// expected-note@-2 {{overloads for '...' exist with these partially matching parameter lists}}
 
 do {
   _ = /x /...

test/stdlib/UnicodeScalarDiagnostics.swift

@@ -10,11 +10,8 @@ func test_UnicodeScalarDoesNotImplementArithmetic(_ us: UnicodeScalar, i: Int) {
   isString(&a1)
   // We don't check for the overload choices list on the overload note match because they may change on different platforms. 
   let a2 = "a" - "b" // expected-error {{binary operator '-' cannot be applied to two 'String' operands}}
-  // expected-note@-1 {{overloads for '-' exist with these partially matching parameter lists:}}
   let a3 = "a" * "b" // expected-error {{binary operator '*' cannot be applied to two 'String' operands}}
-  // expected-note@-1 {{overloads for '*' exist with these partially matching parameter lists:}}
   let a4 = "a" / "b" // expected-error {{binary operator '/' cannot be applied to two 'String' operands}}
-  // expected-note@-1 {{overloads for '/' exist with these partially matching parameter lists:}}
 
   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}}