Merge pull request #81779 from xedin/diagnostic-fixes

[Diagnostics] A collection of diagnostic fixes/improvements

Author: xedin

include/swift/Sema/ConstraintSystem.h

@@ -3484,6 +3484,15 @@ class ConstraintSystem {
     return nullptr;
   }
 
+  Expr *getSemanticsProvidingParentExpr(Expr *expr) {
+    while (auto *parent = getParentExpr(expr)) {
+      if (parent->getSemanticsProvidingExpr() == parent)
+        return parent;
+      expr = parent;
+    }
+    return nullptr;
+  }
+
   /// Retrieve the depth of the given expression.
   std::optional<unsigned> getExprDepth(Expr *expr) {
     if (auto result = getExprDepthAndParent(expr))

lib/Sema/CSSimplify.cpp

@@ -4282,7 +4282,8 @@ ConstraintSystem::matchExistentialTypes(Type type1, Type type2,
         if (!path.empty()) {
           auto last = path.back();
 
-          if (last.is<LocatorPathElt::ApplyArgToParam>()) {
+          if (last.is<LocatorPathElt::ApplyArgToParam>() ||
+              last.is<LocatorPathElt::AutoclosureResult>()) {
             auto proto = protoDecl->getDeclaredInterfaceType();
             // Impact is 2 here because there are two failures
             // 1 - missing conformance and 2 - incorrect argument type.
@@ -4310,6 +4311,15 @@ ConstraintSystem::matchExistentialTypes(Type type1, Type type2,
             break;
           }
 
+          if ((isExpr<ArrayExpr>(anchor) || isExpr<DictionaryExpr>(anchor)) &&
+              last.is<LocatorPathElt::TupleElement>()) {
+            auto *fix = CollectionElementContextualMismatch::create(
+                *this, type1, type2, getConstraintLocator(anchor, path));
+            if (recordFix(fix, /*impact=*/2))
+              return getTypeMatchFailure(locator);
+            break;
+          }
+
           // TODO(diagnostics): If there are any requirement failures associated
           // with result types which are part of a function type conversion,
           // let's record general conversion mismatch in order for it to capture
@@ -4979,8 +4989,18 @@ repairViaOptionalUnwrap(ConstraintSystem &cs, Type fromType, Type toType,
 
     // First, let's check whether it has been determined that
     // it was incorrect to use `?` in this position.
-    if (cs.hasFixFor(cs.getConstraintLocator(subExpr), FixKind::RemoveUnwrap))
+    if (cs.hasFixFor(cs.getConstraintLocator(subExpr), FixKind::RemoveUnwrap)) {
+      if (auto *typeVar =
+              fromType->getOptionalObjectType()->getAs<TypeVariableType>()) {
+        // If the optional chain is invalid let's unwrap optional and
+        // re-introduce the constraint to be solved later once both sides
+        // are sufficiently resolved, this would allow to diagnose not only
+        // the invalid unwrap but an invalid conversion (if any) as well.
+        cs.addConstraint(matchKind, typeVar, toType,
+                         cs.getConstraintLocator(locator));
+      }
       return true;
+    }
 
     auto type = cs.getType(subExpr);
     // If the type of sub-expression is optional, type of the
@@ -5775,6 +5795,41 @@ bool ConstraintSystem::repairFailures(
         break;
       }
 
+      // There is no subtyping between object types of inout argument/parameter.
+      if (auto argConv = path.back().getAs<LocatorPathElt::ApplyArgToParam>()) {
+        // Attempt conversions first.
+        if (hasAnyRestriction())
+          break;
+
+        // Unwraps are allowed to preserve l-valueness so we can suggest
+        // them here.
+        if (repairViaOptionalUnwrap(*this, lhs, rhs, matchKind,
+                                    conversionsOrFixes, locator))
+          return true;
+
+        auto *loc = getConstraintLocator(locator);
+
+        auto result = matchTypes(lhs, rhs, ConstraintKind::Conversion,
+                                 TMF_ApplyingFix, locator);
+
+        ConstraintFix *fix = nullptr;
+        if (result.isFailure()) {
+          // If this is a "destination" argument to a mutating operator
+          // like `+=`, let's consider it contextual and only attempt
+          // to fix type mismatch on the "source" right-hand side of
+          // such operators.
+          if (isOperatorArgument(loc) && argConv->getArgIdx() == 0)
+            break;
+
+          fix = AllowArgumentMismatch::create(*this, lhs, rhs, loc);
+        } else {
+          fix = AllowInOutConversion::create(*this, lhs, rhs, loc);
+        }
+
+        conversionsOrFixes.push_back(fix);
+        break;
+      }
+
       // If this is a problem with result type of a subscript setter,
       // let's re-attempt to repair without l-value conversion in the
       // locator to fix underlying type mismatch.
@@ -5794,7 +5849,7 @@ bool ConstraintSystem::repairFailures(
         break;
     }
 
-    LLVM_FALLTHROUGH;
+    break;
   }
 
   case ConstraintLocator::ApplyArgToParam: {
@@ -5874,52 +5929,6 @@ bool ConstraintSystem::repairFailures(
     if (repairByTreatingRValueAsLValue(lhs, rhs))
       break;
 
-    // If the problem is related to missing unwrap, there is a special
-    // fix for that.
-    if (lhs->getOptionalObjectType() && !rhs->getOptionalObjectType()) {
-      // If this is an attempt to check whether optional conforms to a
-      // particular protocol, let's do that before attempting to force
-      // unwrap the optional.
-      if (hasConversionOrRestriction(ConversionRestrictionKind::Existential))
-        break;
-
-      auto result = matchTypes(lhs->getOptionalObjectType(), rhs, matchKind,
-                               TMF_ApplyingFix, locator);
-
-      if (result.isSuccess()) {
-        conversionsOrFixes.push_back(
-            ForceOptional::create(*this, lhs, rhs, loc));
-        break;
-      }
-    }
-
-    // There is no subtyping between object types of inout argument/parameter.
-    if (elt.getKind() == ConstraintLocator::LValueConversion) {
-      auto result = matchTypes(lhs, rhs, ConstraintKind::Conversion,
-                               TMF_ApplyingFix, locator);
-
-      ConstraintFix *fix = nullptr;
-      if (result.isFailure()) {
-        // If this is a "destination" argument to a mutating operator
-        // like `+=`, let's consider it contextual and only attempt
-        // to fix type mismatch on the "source" right-hand side of
-        // such operators.
-        if (isOperatorArgument(loc) &&
-            loc->findLast<LocatorPathElt::ApplyArgToParam>()->getArgIdx() == 0)
-          break;
-
-        fix = AllowArgumentMismatch::create(*this, lhs, rhs, loc);
-      } else {
-        fix = AllowInOutConversion::create(*this, lhs, rhs, loc);
-      }
-
-      conversionsOrFixes.push_back(fix);
-      break;
-    }
-
-    if (elt.getKind() != ConstraintLocator::ApplyArgToParam)
-      break;
-
     // If argument in l-value type and parameter is `inout` or a pointer,
     // let's see if it's generic parameter matches and suggest adding explicit
     // `&`.
@@ -6046,7 +6055,7 @@ bool ConstraintSystem::repairFailures(
 
     if (repairViaOptionalUnwrap(*this, lhs, rhs, matchKind, conversionsOrFixes,
                                 locator))
-      break;
+      return true;
 
     {
       auto *calleeLocator = getCalleeLocator(loc);
@@ -6856,9 +6865,28 @@ bool ConstraintSystem::repairFailures(
     if (!path.empty() && path.back().is<LocatorPathElt::PackElement>())
       path.pop_back();
 
-    if (!path.empty() && path.back().is<LocatorPathElt::AnyRequirement>()) {
-      return repairFailures(lhs, rhs, matchKind, flags, conversionsOrFixes,
-                            getConstraintLocator(anchor, path));
+    if (!path.empty()) {
+      if (path.back().is<LocatorPathElt::AnyRequirement>()) {
+        return repairFailures(lhs, rhs, matchKind, flags, conversionsOrFixes,
+                              getConstraintLocator(anchor, path));
+      }
+
+      if (auto argConv = path.back().getAs<LocatorPathElt::ApplyArgToParam>()) {
+        auto argIdx = argConv->getArgIdx();
+        auto paramIdx = argConv->getParamIdx();
+
+        auto *argLoc = getConstraintLocator(anchor, path);
+        if (auto overload = findSelectedOverloadFor(getCalleeLocator(argLoc))) {
+          auto *overloadTy =
+              simplifyType(overload->boundType)->castTo<FunctionType>();
+          auto *argList = getArgumentList(argLoc);
+          ASSERT(argList);
+          conversionsOrFixes.push_back(AllowArgumentMismatch::create(
+              *this, getType(argList->getExpr(argIdx)),
+              overloadTy->getParams()[paramIdx].getPlainType(), argLoc));
+          return true;
+        }
+      }
     }
 
     // When the solver sets `TMF_MatchingGenericArguments` it means
@@ -11410,6 +11438,14 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyMemberConstraint(
             // `key path` constraint can't be retired until all components
             // are simplified.
             addTypeVariableConstraintsToWorkList(memberTypeVar);
+          } else if (locator->getAnchor().is<Expr *>() &&
+                     !getSemanticsProvidingParentExpr(
+                         getAsExpr(locator->getAnchor()))) {
+            // If there are no contextual expressions that could provide
+            // a type for the member type variable, let's default it to
+            // a placeholder eagerly so it could be propagated to the
+            // pattern if necessary.
+            recordTypeVariablesAsHoles(memberTypeVar);
           } else if (locator->isLastElement<LocatorPathElt::PatternMatch>()) {
             // Let's handle member patterns specifically because they use
             // equality instead of argument application constraint, so allowing

test/Constraints/bridging.swift

@@ -296,9 +296,9 @@ func rdar19836341(_ ns: NSString?, vns: NSString?) {
 
 // <rdar://problem/20029786> Swift compiler sometimes suggests changing "as!" to "as?!"
 func rdar20029786(_ ns: NSString?) {
-  var s: String = ns ?? "str" as String as String // expected-error{{'NSString' is not implicitly convertible to 'String'; did you mean to use 'as' to explicitly convert?}} {{19-19=(}} {{50-50=) as String}}
-  // expected-error@-1 {{cannot convert value of type 'String' to expected argument type 'NSString'}} {{50-50= as NSString}}
-  var s2 = ns ?? "str" as String as String // expected-error {{cannot convert value of type 'String' to expected argument type 'NSString'}}{{43-43= as NSString}}
+  var s: String = ns ?? "str" as String as String
+  // expected-error@-1 {{cannot convert value of type 'NSString?' to expected argument type 'String?'}} {{21-21= as String?}}
+  var s2 = ns ?? "str" as String as String // expected-error {{binary operator '??' cannot be applied to operands of type 'NSString?' and 'String'}}
 
   let s3: NSString? = "str" as String? // expected-error {{cannot convert value of type 'String?' to specified type 'NSString?'}}{{39-39= as NSString?}}
 

test/Constraints/closures.swift

@@ -803,7 +803,6 @@ overloaded { print("hi"); print("bye") } // multiple expression closure without
 // expected-error@-1 {{ambiguous use of 'overloaded'}}
 
 func not_overloaded(_ handler: () -> Int) {}
-// expected-note@-1 {{'not_overloaded' declared here}}
 
 not_overloaded { } // empty body
 // expected-error@-1 {{cannot convert value of type '()' to closure result type 'Int'}}
@@ -1156,7 +1155,7 @@ struct R_76250381<Result, Failure: Error> {
 func rdar77022842(argA: Bool? = nil, argB: Bool? = nil) {
   if let a = argA ?? false, if let b = argB ?? {
     // expected-error@-1 {{initializer for conditional binding must have Optional type, not 'Bool'}}
-    // expected-error@-2 {{closure passed to parameter of type 'Bool?' that does not accept a closure}}
+    // expected-error@-2 {{cannot convert value of type 'Bool?' to expected argument type '(() -> ())?'}}
     // expected-error@-3 {{cannot convert value of type 'Void' to expected condition type 'Bool'}}
     // expected-error@-4 {{'if' may only be used as expression in return, throw, or as the source of an assignment}}
     // expected-error@-5 {{'if' must have an unconditional 'else' to be used as expression}}
@@ -1337,3 +1336,33 @@ func rdar143338891() {
     }
   }
 }
+
+do {
+  struct V {
+    init(value: @autoclosure @escaping () -> any Hashable) { }
+    init(other: @autoclosure @escaping () -> String) { }
+  }
+
+  let _ = V(value: { [Int]() }) // expected-error {{add () to forward '@autoclosure' parameter}} {{31-31=()}}
+  let _ = V(other: { [Int]() }) // expected-error {{cannot convert value of type '[Int]' to closure result type 'String'}}
+}
+
+// https://github.com/swiftlang/swift/issues/81770
+do {
+  func test(_: Int) {}
+  func test(_: Int = 42, _: (Int) -> Void) {}
+
+  test {
+    if let _ = $0.missing { // expected-error {{value of type 'Int' has no member 'missing'}}
+    }
+  }
+
+  test {
+    if let _ = (($0.missing)) { // expected-error {{value of type 'Int' has no member 'missing'}}
+    }
+  }
+
+  test { // expected-error {{invalid conversion from throwing function of type '(Int) throws -> Void' to non-throwing function type '(Int) -> Void'}}
+    try $0.missing // expected-error {{value of type 'Int' has no member 'missing'}}
+  }
+}

test/Constraints/if_expr.swift

@@ -705,6 +705,7 @@ func builderInClosure() {
 func testInvalidOptionalChainingInIfContext() {
   let v63796 = 1
   if v63796? {} // expected-error{{cannot use optional chaining on non-optional value of type 'Int'}}
+  // expected-error@-1 {{type 'Int' cannot be used as a boolean; test for '!= 0' instead}}
 }
 
 // https://github.com/swiftlang/swift/issues/79395

test/Constraints/optional.swift

@@ -435,7 +435,7 @@ func test_force_unwrap_not_being_too_eager() {
 // rdar://problem/57097401
 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-error@-1 {{cannot convert value of type 'Any' to expected argument type 'String'}}
 }
 
 /// https://github.com/apple/swift/issues/54739
@@ -595,3 +595,26 @@ do {
   test(x!) // expected-error {{no exact matches in call to local function 'test'}}
   // expected-error@-1 {{cannot force unwrap value of non-optional type 'Double'}}
 }
+
+func testExtraQuestionMark(action: () -> Void, v: Int) {
+  struct Test {
+    init(action: () -> Void) {}
+  }
+
+  Test(action: action?)
+  // expected-error@-1 {{cannot use optional chaining on non-optional value of type '() -> Void'}}
+  Test(action: v?)
+  // expected-error@-1 {{cannot convert value of type 'Int' to expected argument type '() -> Void'}}
+  // expected-error@-2 {{cannot use optional chaining on non-optional value of type 'Int'}}
+}
+
+func testPassingOptionalChainAsWrongArgument() {
+  class Test {
+    func fn(_ asdType: String?) {
+    }
+  }
+
+  func test(test: Test, arr: [Int]?) {
+    test.fn(arr?.first) // expected-error {{cannot convert value of type 'Int?' to expected argument type 'String?'}}
+  }
+}

test/Constraints/protocols.swift

@@ -577,3 +577,11 @@ do {
 
   isFooableError(overloaded()) // Ok
 }
+
+do {
+  func takesFooables(_: [any Fooable]) {}
+
+  func test(v: String) {
+    takesFooables([v]) // expected-error {{cannot convert value of type 'String' to expected element type 'any Fooable'}}
+  }
+}

test/Constraints/rdar44770297.swift

@@ -10,6 +10,3 @@ func foo<T: P>(_: () throws -> T) -> T.A? { // expected-note {{where 'T' = 'Neve
 
 let _ = foo() {fatalError()} & nil
 // expected-error@-1 {{global function 'foo' requires that 'Never' conform to 'P'}}
-// expected-error@-2 {{value of optional type 'Never.A?' must be unwrapped to a value of type 'Never.A'}}
-// expected-note@-3 {{coalesce using '??' to provide a default when the optional value contains 'nil'}}
-// expected-note@-4 {{force-unwrap using '!' to abort execution if the optional value contains 'nil'}}

test/decl/func/typed_throws.swift

@@ -192,7 +192,7 @@ enum G_E<T> {
 
 func testArrMap(arr: [String]) {
   _ = mapArray(arr, body: G_E<Int>.tuple)
-  // expected-error@-1{{cannot convert value of type '((x: Int, y: Int)) -> G_E<Int>' to expected argument type '(String) -> G_E<Int>'}}
+  // expected-error@-1{{conflicting arguments to generic parameter 'T' ('String' vs. '(x: Int, y: Int)')}}
 }
 
 // Shadowing of typed-throws Result.get() addresses a source compatibility

test/expr/expressions.swift

@@ -794,7 +794,7 @@ func testNilCoalescePrecedence(cond: Bool, a: Int?, r: ClosedRange<Int>?) {
 
   // ?? should have lower precedence than range and arithmetic operators.
   let r1 = r ?? (0...42) // ok
-  let r2 = (r ?? 0)...42 // not ok: expected-error {{binary operator '??' cannot be applied to operands of type 'ClosedRange<Int>?' and 'Int'}}
+  let r2 = (r ?? 0)...42 // not ok: expected-error {{cannot convert value of type 'ClosedRange<Int>?' to expected argument type 'Int?'}}
   let r3 = r ?? 0...42 // parses as the first one, not the second.
 
 

test/stdlib/UnsafePointerDiagnostics.swift

@@ -78,17 +78,14 @@ func unsafePointerConversionAvailability(
 
   _ = UnsafeMutablePointer<Int>(rp) // expected-error {{cannot convert value of type 'UnsafeRawPointer' to expected argument type 'UnsafeMutablePointer<Int>'}}
   _ = UnsafeMutablePointer<Int>(mrp) // expected-error {{cannot convert value of type 'UnsafeMutableRawPointer' to expected argument type 'UnsafeMutablePointer<Int>'}}
-  // Two candidates here: OpaquePointer? and UnsafeMutablePointer<Int>?
+  // This is ambiguous because we have failable and non-failable initializers that accept the same argument type.
   _ = UnsafeMutablePointer<Int>(orp) // expected-error {{no exact matches in call to initializer}}
-  // Two candidates here: OpaquePointer? and UnsafeMutablePointer<Int>?
-  _ = UnsafeMutablePointer<Int>(omrp) // expected-error {{no exact matches in call to initializer}}
+  _ = UnsafeMutablePointer<Int>(omrp) // expected-error {{cannot convert value of type 'UnsafeMutableRawPointer' to expected argument type 'UnsafeMutablePointer<Int>'}}
 
   _ = UnsafePointer<Int>(rp)  // expected-error {{cannot convert value of type 'UnsafeRawPointer' to expected argument type 'UnsafePointer<Int>'}}
   _ = UnsafePointer<Int>(mrp) // expected-error {{cannot convert value of type 'UnsafeMutableRawPointer' to expected argument type 'UnsafePointer<Int>'}}
-  // Two candidates here: OpaquePointer? and UnsafeMutablePointer<Int>?
-  _ = UnsafePointer<Int>(orp)  // expected-error {{no exact matches in call to initializer}}
-  // Two candidates here: OpaquePointer? and UnsafeMutablePointer<Int>?
-  _ = UnsafePointer<Int>(omrp) // expected-error {{no exact matches in call to initializer}}
+  _ = UnsafePointer<Int>(orp)  // expected-error {{cannot convert value of type 'UnsafeRawPointer' to expected argument type 'UnsafePointer<Int>'}}
+  _ = UnsafePointer<Int>(omrp) // expected-error {{cannot convert value of type 'UnsafeMutableRawPointer' to expected argument type 'UnsafePointer<Int>'}}
 
   _ = UnsafePointer<Int>(ups) // expected-error {{cannot convert value of type 'UnsafePointer<String>' to expected argument type 'UnsafePointer<Int>'}}
   // expected-note@-1 {{arguments to generic parameter 'Pointee' ('String' and 'Int') are expected to be equal}}