[QoI] Improve diagnostics for assignment expression

Currently `visitAssignExpr` always attempts to use type
derived from destination as a contextual type for assignment
source type-checking, which doesn't always lead to better
results.

Resolves: SR-5081

Author: xedin

lib/Sema/CSDiag.cpp

@@ -2134,7 +2134,8 @@ class FailureDiagnosis :public ASTVisitor<FailureDiagnosis, /*exprresult*/bool>{
 
   /// Attempt to produce a diagnostic for a mismatch between an expression's
   /// type and its assumed contextual type.
-  bool diagnoseContextualConversionError(Expr *expr, Type contextualType);
+  bool diagnoseContextualConversionError(Expr *expr, Type contextualType,
+                                         ContextualTypePurpose CTP);
 
   /// For an expression being type checked with a CTP_CalleeResult contextual
   /// type, try to diagnose a problem.
@@ -2143,7 +2144,8 @@ class FailureDiagnosis :public ASTVisitor<FailureDiagnosis, /*exprresult*/bool>{
   /// Attempt to produce a diagnostic for a mismatch between a call's
   /// type and its assumed contextual type.
   bool diagnoseCallContextualConversionErrors(ApplyExpr *callEpxr,
-                                              Type contextualType);
+                                              Type contextualType,
+                                              ContextualTypePurpose CTP);
 
 private:
   /// Validate potential contextual type for type-checking one of the
@@ -3899,9 +3901,9 @@ static bool addTypeCoerceFixit(InFlightDiagnostic &diag, ConstraintSystem &CS,
 /// of function type, giving more specific and simpler diagnostics, attaching
 /// notes on the parameter, and offering fixits to insert @escaping. Returns
 /// true if it detects and issues an error, false if it does nothing.
-static bool tryDiagnoseNonEscapingParameterToEscaping(Expr *expr, Type srcType,
-                                                      Type dstType,
-                                                      ConstraintSystem &CS) {
+static bool tryDiagnoseNonEscapingParameterToEscaping(
+    Expr *expr, Type srcType, Type dstType, ContextualTypePurpose dstPurpose,
+    ConstraintSystem &CS) {
   assert(expr);
   // Need to be referencing a parameter of function type
   auto declRef = dyn_cast<DeclRefExpr>(expr);
@@ -3921,7 +3923,7 @@ static bool tryDiagnoseNonEscapingParameterToEscaping(Expr *expr, Type srcType,
 
   // Pick a specific diagnostic for the specific use
   auto paramDecl = cast<ParamDecl>(declRef->getDecl());
-  switch (CS.getContextualTypePurpose()) {
+  switch (dstPurpose) {
   case CTP_CallArgument:
     CS.TC.diagnose(declRef->getLoc(), diag::passing_noescape_to_escaping,
                    paramDecl->getName());
@@ -3949,13 +3951,13 @@ static bool tryDiagnoseNonEscapingParameterToEscaping(Expr *expr, Type srcType,
   return true;
 }
 
-bool FailureDiagnosis::diagnoseContextualConversionError(Expr *expr,
-                                                         Type contextualType) {
+bool FailureDiagnosis::diagnoseContextualConversionError(
+    Expr *expr, Type contextualType, ContextualTypePurpose CTP) {
   // If the constraint system has a contextual type, then we can test to see if
   // this is the problem that prevents us from solving the system.
   if (!contextualType) {
     // This contextual conversion constraint doesn't install an actual type.
-    if (CS.getContextualTypePurpose() == CTP_CalleeResult)
+    if (CTP == CTP_CalleeResult)
       return diagnoseCalleeResultContextualConversionError();
 
     return false;
@@ -3995,7 +3997,7 @@ bool FailureDiagnosis::diagnoseContextualConversionError(Expr *expr,
   // If this is conversion failure due to a return statement with an argument
   // that cannot be coerced to the result type of the function, emit a
   // specific error.
-  switch (CS.getContextualTypePurpose()) {
+  switch (CTP) {
   case CTP_Unused:
   case CTP_CannotFail:
     llvm_unreachable("These contextual type purposes cannot fail with a "
@@ -4156,8 +4158,7 @@ bool FailureDiagnosis::diagnoseContextualConversionError(Expr *expr,
 
   // If this is a conversion from T to () in a call argument context, it is
   // almost certainly an extra argument being passed in.
-  if (CS.getContextualTypePurpose() == CTP_CallArgument &&
-      contextualType->isVoid()) {
+  if (CTP == CTP_CallArgument && contextualType->isVoid()) {
     diagnose(expr->getLoc(), diag::extra_argument_to_nullary_call)
       .highlight(expr->getSourceRange());
     return true;
@@ -4212,7 +4213,7 @@ bool FailureDiagnosis::diagnoseContextualConversionError(Expr *expr,
 
   // Try for better/more specific diagnostics for non-escaping to @escaping
   if (tryDiagnoseNonEscapingParameterToEscaping(expr, exprType, contextualType,
-                                                CS))
+                                                CTP, CS))
     return true;
 
   // Don't attempt fixits if we have an unsolved type variable, since
@@ -4261,7 +4262,7 @@ bool FailureDiagnosis::diagnoseContextualConversionError(Expr *expr,
   }
 
   // Attempt to add a fixit for the error.
-  switch (CS.getContextualTypePurpose()) {
+  switch (CTP) {
   case CTP_CallArgument:
   case CTP_ArrayElement:
   case CTP_DictionaryKey:
@@ -6303,7 +6304,8 @@ bool FailureDiagnosis::diagnoseTrailingClosureErrors(ApplyExpr *callExpr) {
     // related to function type, let's try to diagnose it.
     if (possibleTypes.empty() && contextualType &&
         !contextualType->hasUnresolvedType())
-      return diagnoseContextualConversionError(callExpr, contextualType);
+      return diagnoseContextualConversionError(callExpr, contextualType,
+                                               CS.getContextualTypePurpose());
   } else {
     possibleTypes.push_back(currentType);
   }
@@ -6404,7 +6406,7 @@ bool FailureDiagnosis::diagnoseTrailingClosureErrors(ApplyExpr *callExpr) {
 /// Check if there failure associated with expression is related
 /// to given contextual type.
 bool FailureDiagnosis::diagnoseCallContextualConversionErrors(
-    ApplyExpr *callExpr, Type contextualType) {
+    ApplyExpr *callExpr, Type contextualType, ContextualTypePurpose CTP) {
   if (!contextualType || contextualType->hasUnresolvedType())
     return false;
 
@@ -6436,7 +6438,7 @@ bool FailureDiagnosis::diagnoseCallContextualConversionErrors(
   // If type-checking with contextual type didn't produce any results
   // it means that we have a contextual mismatch.
   if (withContextual.empty())
-    return diagnoseContextualConversionError(callExpr, contextualType);
+    return diagnoseContextualConversionError(callExpr, contextualType, CTP);
 
   // If call produces a single type when type-checked with contextual
   // expression, it means that the problem is elsewhere, any other
@@ -6482,7 +6484,8 @@ bool FailureDiagnosis::visitApplyExpr(ApplyExpr *callExpr) {
   if (diagnoseTrailingClosureErrors(callExpr))
     return true;
 
-  if (diagnoseCallContextualConversionErrors(callExpr, CS.getContextualType()))
+  if (diagnoseCallContextualConversionErrors(callExpr, CS.getContextualType(),
+                                             CS.getContextualTypePurpose()))
     return true;
 
   auto *fnExpr = callExpr->getFn();
@@ -7004,11 +7007,31 @@ bool FailureDiagnosis::visitAssignExpr(AssignExpr *assignExpr) {
     return true;
   }
 
-  // If the source type is already an error type, we've already posted an error.
-  auto srcExpr = typeCheckChildIndependently(assignExpr->getSrc(),
-                                             destType->getRValueType(),
-                                             CTP_AssignSource);
-  if (!srcExpr) return true;
+  auto *srcExpr = assignExpr->getSrc();
+  auto contextualType = destType->getRValueType();
+
+  // Let's try to type-check assignment source expression without using
+  // destination as a contextual type, that allows us to diagnose
+  // contextual problems related to source much easier.
+  //
+  // If source expression requires contextual type to be present,
+  // let's avoid this step because it's always going to fail.
+  {
+    auto *srcExpr = assignExpr->getSrc();
+    ExprTypeSaverAndEraser eraser(srcExpr);
+
+    ConcreteDeclRef ref = nullptr;
+    auto type = CS.TC.getTypeOfExpressionWithoutApplying(srcExpr, CS.DC, ref);
+
+    if (type && !type->isEqual(contextualType))
+      return diagnoseContextualConversionError(
+          assignExpr->getSrc(), contextualType, CTP_AssignSource);
+  }
+
+  srcExpr = typeCheckChildIndependently(assignExpr->getSrc(), contextualType,
+                                        CTP_AssignSource);
+  if (!srcExpr)
+    return true;
 
   // If we are assigning to _ and have unresolved types on the RHS, then we have
   // an ambiguity problem.
@@ -8860,7 +8883,8 @@ void ConstraintSystem::diagnoseFailureForExpr(Expr *expr) {
     return;
 
   // If this is a contextual conversion problem, dig out some information.
-  if (diagnosis.diagnoseContextualConversionError(expr, getContextualType()))
+  if (diagnosis.diagnoseContextualConversionError(expr, getContextualType(),
+                                                  getContextualTypePurpose()))
     return;
 
   // If we can diagnose a problem based on the constraints left laying around in

test/ClangImporter/objc_parse.swift

@@ -175,7 +175,7 @@ func keyedSubscripting(_ b: B, idx: A, a: A) {
   dict[NSString()] = a
   let value = dict[NSString()]
 
-  dict[nil] = a // expected-error {{ambiguous reference}}
+  dict[nil] = a // expected-error {{cannot assign value of type 'A' to type 'Any?'}}
   let q = dict[nil]  // expected-error {{ambiguous subscript}}
   _ = q
 }

test/Constraints/diagnostics.swift

@@ -1085,3 +1085,9 @@ fun_31849281(a: { !$0 }, c: [nil, 42], b: { "\($0)" })
 
 func f_31849281(x: Int, y: Int, z: Int) {}
 f_31849281(42, y: 10, x: 20) // expected-error {{argument 'x' must precede unnamed argument #1}} {{12-12=x: 20, }} {{21-28=}}
+
+func sr5081() {
+  var a = ["1", "2", "3", "4", "5"]
+  var b = [String]()
+  b = a[2...4] // expected-error {{cannot assign value of type 'ArraySlice<String>' to type '[String]'}}
+}

test/Constraints/generics.swift

@@ -429,7 +429,7 @@ class GenericClass<A> {}
 func genericFunc<T>(t: T) {
   _ = [T: GenericClass] // expected-error {{generic parameter 'A' could not be inferred}}
   // expected-note@-1 {{explicitly specify the generic arguments to fix this issue}}
-  // expected-error@-2 2 {{type 'T' does not conform to protocol 'Hashable'}}
+  // expected-error@-2 3 {{type 'T' does not conform to protocol 'Hashable'}}
 }
 
 struct SR_3525<T> {}

test/Generics/inheritance.swift

@@ -25,8 +25,8 @@ func f0<T : A>(_ obji: T, _ ai: A, _ bi: B) {
   a = obj
 
   // Invalid assignments
-  obj = a // expected-error{{'A' is not convertible to 'T'}}
-  obj = b // expected-error{{'B' is not convertible to 'T'}}
+  obj = a // expected-error{{cannot assign value of type 'A' to type 'T'}}
+  obj = b // expected-error{{cannot assign value of type 'B' to type 'T'}}
 
   // Downcast that is actually a coercion
   a = (obj as? A)! // expected-warning{{conditional cast from 'T' to 'A' always succeeds}}

test/Parse/pointer_conversion.swift.gyb

@@ -60,7 +60,7 @@ func mutablePointerArguments(_ p: UnsafeMutablePointer<Int>,
 
   // We don't allow these conversions outside of function arguments.
   var x: UnsafeMutablePointer<Int> = &i // expected-error{{cannot pass immutable value as inout argument: 'i' is immutable}}
-  x = &ii // expected-error{{cannot assign value of type '[Int]' to type 'Int'}}
+  x = &ii // expected-error{{cannot assign value of type 'inout [Int]' to type 'UnsafeMutablePointer<Int>'}}
   _ = x
 }
 

test/TypeCoercion/overload_noncall.swift

@@ -52,7 +52,7 @@ func test_inout() {
   x = accept_XY(&xy);
 
   x = xy
-  x = &xy; // expected-error{{'&' used with non-inout argument of type 'X'}}
+  x = &xy; // expected-error{{cannot assign value of type 'inout X' to type 'X'}}
   accept_Z(&xy); // expected-error{{cannot convert value of type 'X' to expected argument type 'Z'}}
 }
 

test/stdlib/UnsafePointerDiagnostics.swift

@@ -56,17 +56,17 @@ func unsafePointerConversionAvailability(
   _ = UnsafeRawPointer(oumps)
   _ = UnsafeRawPointer(oups)
 
-  _ = UnsafeMutablePointer<Void>(rp) // expected-warning 3 {{UnsafeMutablePointer<Void> has been replaced by UnsafeMutableRawPointer}} expected-error {{cannot invoke initializer for type 'UnsafeMutablePointer<Void>' with an argument list of type '(UnsafeRawPointer)'}} expected-note {{overloads for 'UnsafeMutablePointer<Void>' exist with these partially matching parameter lists: (RawPointer), (OpaquePointer), (OpaquePointer?), (UnsafeMutablePointer<Pointee>), (UnsafeMutablePointer<Pointee>?)}} expected-note{{Pointer conversion restricted: use '.assumingMemoryBound(to:)' or '.bindMemory(to:capacity:)' to view memory as a type}}
-  _ = UnsafeMutablePointer<Void>(mrp) // expected-error {{cannot invoke initializer for type 'UnsafeMutablePointer<Void>' with an argument list of type '(UnsafeMutableRawPointer)'}} expected-note {{}} expected-warning 3 {{UnsafeMutablePointer<Void> has been replaced by UnsafeMutableRawPointer}} expected-note{{overloads for 'UnsafeMutablePointer<Void>' exist with these partially matching parameter lists: (RawPointer), (OpaquePointer), (OpaquePointer?), (UnsafeMutablePointer<Pointee>), (UnsafeMutablePointer<Pointee>?)}}
+  _ = UnsafeMutablePointer<Void>(rp) // expected-warning 4 {{UnsafeMutablePointer<Void> has been replaced by UnsafeMutableRawPointer}} expected-error {{cannot invoke initializer for type 'UnsafeMutablePointer<Void>' with an argument list of type '(UnsafeRawPointer)'}} expected-note {{overloads for 'UnsafeMutablePointer<Void>' exist with these partially matching parameter lists: (RawPointer), (OpaquePointer), (OpaquePointer?), (UnsafeMutablePointer<Pointee>), (UnsafeMutablePointer<Pointee>?)}} expected-note{{Pointer conversion restricted: use '.assumingMemoryBound(to:)' or '.bindMemory(to:capacity:)' to view memory as a type}}
+  _ = UnsafeMutablePointer<Void>(mrp) // expected-error {{cannot invoke initializer for type 'UnsafeMutablePointer<Void>' with an argument list of type '(UnsafeMutableRawPointer)'}} expected-note {{}} expected-warning 4 {{UnsafeMutablePointer<Void> has been replaced by UnsafeMutableRawPointer}} expected-note{{overloads for 'UnsafeMutablePointer<Void>' exist with these partially matching parameter lists: (RawPointer), (OpaquePointer), (OpaquePointer?), (UnsafeMutablePointer<Pointee>), (UnsafeMutablePointer<Pointee>?)}}
   _ = UnsafeMutablePointer<Void>(umpv) // expected-warning {{UnsafeMutablePointer<Void> has been replaced by UnsafeMutableRawPointer}}
   _ = UnsafeMutablePointer<Void>(upv)  // expected-error {{'init' has been renamed to 'init(mutating:)'}} expected-warning {{UnsafeMutablePointer<Void> has been replaced by UnsafeMutableRawPointer}}
   _ = UnsafeMutablePointer<Void>(umpi) // expected-warning {{UnsafeMutablePointer<Void> has been replaced by UnsafeMutableRawPointer}}
   _ = UnsafeMutablePointer<Void>(upi)  // expected-error {{'init' has been renamed to 'init(mutating:)'}} expected-warning {{UnsafeMutablePointer<Void> has been replaced by UnsafeMutableRawPointer}}
   _ = UnsafeMutablePointer<Void>(umps) // expected-warning {{UnsafeMutablePointer<Void> has been replaced by UnsafeMutableRawPointer}}
   _ = UnsafeMutablePointer<Void>(ups)  // expected-error {{'init' has been renamed to 'init(mutating:)'}} expected-warning {{UnsafeMutablePointer<Void> has been replaced by UnsafeMutableRawPointer}}
 
-  _ = UnsafePointer<Void>(rp)  // expected-error {{cannot invoke initializer for type 'UnsafePointer<Void>' with an argument list of type '(UnsafeRawPointer)'}} expected-note {{}} expected-warning 3 {{UnsafePointer<Void> has been replaced by UnsafeRawPointer}} expected-note{{overloads for 'UnsafePointer<Void>' exist with these partially matching parameter lists: (RawPointer), (OpaquePointer), (OpaquePointer?), (UnsafePointer<Pointee>), (UnsafePointer<Pointee>?), (UnsafeMutablePointer<Pointee>), (UnsafeMutablePointer<Pointee>?)}}
-  _ = UnsafePointer<Void>(mrp) // expected-error {{cannot invoke initializer for type 'UnsafePointer<Void>' with an argument list of type '(UnsafeMutableRawPointer)'}} expected-note {{}} expected-warning 3 {{UnsafePointer<Void> has been replaced by UnsafeRawPointer}} expected-note{{overloads for 'UnsafePointer<Void>' exist with these partially matching parameter lists: (RawPointer), (OpaquePointer), (OpaquePointer?), (UnsafePointer<Pointee>), (UnsafePointer<Pointee>?), (UnsafeMutablePointer<Pointee>), (UnsafeMutablePointer<Pointee>?)}}
+  _ = UnsafePointer<Void>(rp)  // expected-error {{cannot invoke initializer for type 'UnsafePointer<Void>' with an argument list of type '(UnsafeRawPointer)'}} expected-note {{}} expected-warning 4 {{UnsafePointer<Void> has been replaced by UnsafeRawPointer}} expected-note{{overloads for 'UnsafePointer<Void>' exist with these partially matching parameter lists: (RawPointer), (OpaquePointer), (OpaquePointer?), (UnsafePointer<Pointee>), (UnsafePointer<Pointee>?), (UnsafeMutablePointer<Pointee>), (UnsafeMutablePointer<Pointee>?)}}
+  _ = UnsafePointer<Void>(mrp) // expected-error {{cannot invoke initializer for type 'UnsafePointer<Void>' with an argument list of type '(UnsafeMutableRawPointer)'}} expected-note {{}} expected-warning 4 {{UnsafePointer<Void> has been replaced by UnsafeRawPointer}} expected-note{{overloads for 'UnsafePointer<Void>' exist with these partially matching parameter lists: (RawPointer), (OpaquePointer), (OpaquePointer?), (UnsafePointer<Pointee>), (UnsafePointer<Pointee>?), (UnsafeMutablePointer<Pointee>), (UnsafeMutablePointer<Pointee>?)}}
   _ = UnsafePointer<Void>(umpv) // expected-warning {{UnsafePointer<Void> has been replaced by UnsafeRawPointer}}
   _ = UnsafePointer<Void>(upv) // expected-warning {{UnsafePointer<Void> has been replaced by UnsafeRawPointer}}
   _ = UnsafePointer<Void>(umpi) // expected-warning {{UnsafePointer<Void> has been replaced by UnsafeRawPointer}}