Start changing over rvalue-vs-lvalue errors to be done via constraint system fixes. For this first commit, just handling inout parameter problems.

Author: gregomni

lib/Sema/CSDiag.cpp

@@ -738,7 +738,7 @@ static void fixItChangeInoutArgType(const Expr * arg,
     .fixItReplaceChars(startLoc, endLoc, scratch);
 }
 
-static void diagnoseSubElementFailure(Expr *destExpr,
+void swift::diagnoseSubElementFailure(Expr *destExpr,
                                       SourceLoc loc,
                                       ConstraintSystem &CS,
                                       Diag<StringRef> diagID,
@@ -883,7 +883,8 @@ static void diagnoseSubElementFailure(Expr *destExpr,
     }
   }
 
-  TC.diagnose(loc, unknownDiagID, CS.getType(destExpr))
+  auto type = destExpr->getType() ?: CS.getType(destExpr);
+  TC.diagnose(loc, unknownDiagID, type)
       .highlight(immInfo.first->getSourceRange());
 }
 
@@ -6157,33 +6158,6 @@ bool FailureDiagnosis::visitApplyExpr(ApplyExpr *callExpr) {
   if (!argExpr)
     return true; // already diagnosed.
 
-  // A common error is to apply an operator that only has inout forms (e.g. +=)
-  // to non-lvalues (e.g. a local let).  Produce a nice diagnostic for this
-  // case.
-  if (calleeInfo.closeness == CC_NonLValueInOut) {
-    Diag<StringRef> subElementDiagID;
-    Diag<Type> rvalueDiagID;
-    Expr *diagExpr = nullptr;
-    
-    if (isa<PrefixUnaryExpr>(callExpr) || isa<PostfixUnaryExpr>(callExpr)) {
-      subElementDiagID = diag::cannot_apply_lvalue_unop_to_subelement;
-      rvalueDiagID = diag::cannot_apply_lvalue_unop_to_rvalue;
-      diagExpr = argExpr;
-    } else if (isa<BinaryExpr>(callExpr)) {
-      subElementDiagID = diag::cannot_apply_lvalue_binop_to_subelement;
-      rvalueDiagID = diag::cannot_apply_lvalue_binop_to_rvalue;
-      
-      if (auto argTuple = dyn_cast<TupleExpr>(argExpr))
-        diagExpr = argTuple->getElement(0);
-    }
-    
-    if (diagExpr) {
-      diagnoseSubElementFailure(diagExpr, callExpr->getFn()->getLoc(), CS,
-                                subElementDiagID, rvalueDiagID);
-      return true;
-    }
-  }
-  
   // Handle argument label mismatches when we have multiple candidates.
   if (calleeInfo.closeness == CC_ArgumentLabelMismatch) {
     auto args = decomposeArgType(CS.getType(argExpr), argLabels);

lib/Sema/CSDiag.h

@@ -26,7 +26,12 @@ namespace swift {
   /// UnresolvedType.
   Type replaceTypeParametersWithUnresolved(Type ty);
   Type replaceTypeVariablesWithUnresolved(Type ty);
-  
+
+  /// Diagnose lvalue expr error.
+  void diagnoseSubElementFailure(Expr *destExpr, SourceLoc loc,
+                                 constraints::ConstraintSystem &CS,
+                                 Diag<StringRef> diagID,
+                                 Diag<Type> unknownDiagID);
 };
 
 #endif /* SWIFT_SEMA_CSDIAG_H */

lib/Sema/CSDiagnostics.cpp

@@ -16,6 +16,7 @@
 
 #include "CSDiagnostics.h"
 #include "ConstraintSystem.h"
+#include "CSDiag.h"
 #include "MiscDiagnostics.h"
 #include "swift/AST/Expr.h"
 #include "swift/AST/GenericSignature.h"
@@ -527,3 +528,44 @@ bool MissingOptionalUnwrapFailure::diagnose() {
       .fixItReplace({tryExpr->getTryLoc(), tryExpr->getQuestionLoc()}, "try!");
   return true;
 }
+
+bool RValueTreatedAsLValueFailure::diagnose() {
+  if (auto callExpr = dyn_cast<ApplyExpr>(getAnchor())) {
+    Expr *argExpr = callExpr->getArg();
+
+    Diag<StringRef> subElementDiagID;
+    Diag<Type> rvalueDiagID;
+    Expr *diagExpr = nullptr;
+
+    if (isa<PrefixUnaryExpr>(callExpr) || isa<PostfixUnaryExpr>(callExpr)) {
+      subElementDiagID = diag::cannot_apply_lvalue_unop_to_subelement;
+      rvalueDiagID = diag::cannot_apply_lvalue_unop_to_rvalue;
+      diagExpr = argExpr;
+    } else if (isa<BinaryExpr>(callExpr)) {
+      subElementDiagID = diag::cannot_apply_lvalue_binop_to_subelement;
+      rvalueDiagID = diag::cannot_apply_lvalue_binop_to_rvalue;
+      auto argTuple = dyn_cast<TupleExpr>(argExpr);
+      diagExpr = argTuple->getElement(0);
+    } else {
+      auto lastPathElement = getLocator()->getPath().back();
+      assert(lastPathElement.getKind() == ConstraintLocator::PathElementKind::ApplyArgToParam);
+
+      subElementDiagID = diag::cannot_pass_rvalue_inout_subelement;
+      rvalueDiagID = diag::cannot_pass_rvalue_inout;
+      if (auto argTuple = dyn_cast<TupleExpr>(argExpr))
+        diagExpr = argTuple->getElement(lastPathElement.getValue());
+      else if (auto parens = dyn_cast<ParenExpr>(argExpr))
+        diagExpr = parens->getSubExpr();
+    }
+
+    // FIXME: Would like for this to be unnecessary.
+    getConstraintSystem().TC.typeCheckExpression(diagExpr, getConstraintSystem().DC);
+
+    diagnoseSubElementFailure(diagExpr, callExpr->getFn()->getLoc(),
+                              getConstraintSystem(),
+                              subElementDiagID, rvalueDiagID);
+    return true;
+  }
+  // These fixes are only being created for matching arg types right now, so this is unreachable.
+  return false;
+}

lib/Sema/CSDiagnostics.h

@@ -294,6 +294,17 @@ class MissingOptionalUnwrapFailure final : public FailureDiagnostic {
   bool diagnose() override;
 };
 
+/// Diagnose errors associated with rvalues in positions
+/// where an lvalue is required, such as inout arguments.
+class RValueTreatedAsLValueFailure final : public FailureDiagnostic {
+
+public:
+  RValueTreatedAsLValueFailure(const Solution &solution, ConstraintLocator *locator)
+    : FailureDiagnostic(nullptr, solution, locator) {}
+
+  bool diagnose() override;
+};
+
 } // end namespace constraints
 } // end namespace swift
 

lib/Sema/CSSimplify.cpp

@@ -2439,10 +2439,15 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
             ForceDowncast::create(*this, type2, getConstraintLocator(locator)));
     }
 
-    // If we're converting an lvalue to an inout type, add the missing '&'.
-    if (type2->getRValueType()->is<InOutType>() && type1->is<LValueType>()) {
-      conversionsOrFixes.push_back(
+    if (type2->getRValueType()->is<InOutType>()) {
+      if (type1->is<LValueType>()) {
+        // If we're converting an lvalue to an inout type, add the missing '&'.
+        conversionsOrFixes.push_back(
           AddAddressOf::create(*this, getConstraintLocator(locator)));
+      } else if (!isTypeVarOrMember1) {
+        // If we have a concrete type that's an rvalue, "fix" it.
+        conversionsOrFixes.push_back(FixKind::TreatRValueAsLValue);
+      }
     }
   }
 
@@ -4981,6 +4986,16 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
     return result;
   }
 
+  case FixKind::TreatRValueAsLValue: {
+    auto result = matchTypes(LValueType::get(type1), type2,
+                             matchKind, subflags, locator);
+    if (result == SolutionKind::Solved)
+      if (recordFix(fix, locator))
+        return SolutionKind::Error;
+
+    return result;
+  }
+
   case FixKind::ExplicitlyEscaping:
   case FixKind::CoerceToCheckedCast:
   case FixKind::RelabelArguments:

test/Constraints/diagnostics.swift

@@ -719,14 +719,13 @@ func overloadSetResultType(_ a : Int, b : Int) -> Int {
 }
 
 postfix operator +++
-postfix func +++ <T>(_: inout T) -> T { fatalError() } // expected-note {{in call to operator '+++'}}
+postfix func +++ <T>(_: inout T) -> T { fatalError() }
 
 // <rdar://problem/21523291> compiler error message for mutating immutable field is incorrect
 func r21523291(_ bytes : UnsafeMutablePointer<UInt8>) {
-  let i = 42
+  let i = 42 // expected-note {{change 'let' to 'var' to make it mutable}}
 
-  // FIXME: rdar://41416382
-  _ = bytes[i+++]  // expected-error {{generic parameter 'T' could not be inferred}}
+  _ = bytes[i+++]  // expected-error {{cannot pass immutable value to mutating operator: 'i' is a 'let' constant}}
 }
 
 

test/Constraints/lvalues.swift

@@ -76,14 +76,14 @@ f2(&non_settable_x) // expected-error{{cannot pass immutable value as inout argu
 f1(&non_settable_x) // expected-error{{cannot pass immutable value as inout argument: 'non_settable_x' is a get-only property}}
 // - inout assignment
 non_settable_x += x // expected-error{{left side of mutating operator isn't mutable: 'non_settable_x' is a get-only property}}
-+++non_settable_x // expected-error{{cannot pass immutable value as inout argument: 'non_settable_x' is a get-only property}}
++++non_settable_x // expected-error{{cannot pass immutable value to mutating operator: 'non_settable_x' is a get-only property}}
 
 // non-settable property is non-settable:
 z.non_settable_x = x // expected-error{{cannot assign to property: 'non_settable_x' is a get-only property}}
 f2(&z.non_settable_x) // expected-error{{cannot pass immutable value as inout argument: 'non_settable_x' is a get-only property}}
 f1(&z.non_settable_x) // expected-error{{cannot pass immutable value as inout argument: 'non_settable_x' is a get-only property}}
 z.non_settable_x += x // expected-error{{left side of mutating operator isn't mutable: 'non_settable_x' is a get-only property}}
-+++z.non_settable_x // expected-error{{cannot pass immutable value as inout argument: 'non_settable_x' is a get-only property}}
++++z.non_settable_x // expected-error{{cannot pass immutable value to mutating operator: 'non_settable_x' is a get-only property}}
 
 // non-settable subscript is non-settable:
 z[0] = 0.0 // expected-error{{cannot assign through subscript: subscript is get-only}}
@@ -97,7 +97,7 @@ fz().settable_x = x // expected-error{{cannot assign to property: 'fz' returns i
 f2(&fz().settable_x) // expected-error{{cannot pass immutable value as inout argument: 'fz' returns immutable value}}
 f1(&fz().settable_x) // expected-error{{cannot pass immutable value as inout argument: 'fz' returns immutable value}}
 fz().settable_x += x // expected-error{{left side of mutating operator isn't mutable: 'fz' returns immutable value}}
-+++fz().settable_x // expected-error{{cannot pass immutable value as inout argument: 'fz' returns immutable value}}
++++fz().settable_x // expected-error{{cannot pass immutable value to mutating operator: 'fz' returns immutable value}}
 
 // settable property of an rvalue reference type IS SETTABLE:
 fref().property = 0.0