Merge pull request swiftlang#18608 from gregomni/rvalue-as-lvalue

[ConstraintSystem] New FailureDiagnostic for rvalues that should be lvalues

Author: gregomni

lib/Sema/CSApply.cpp

@@ -7764,6 +7764,8 @@ bool ConstraintSystem::applySolutionFixes(Expr *E, const Solution &solution) {
   llvm::SmallDenseMap<Expr *, SmallVector<const ConstraintFix *, 4>>
       fixesPerExpr;
 
+  applySolution(solution);
+
   for (auto *fix : solution.Fixes)
     fixesPerExpr[fix->getAnchor()].push_back(fix);
 

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.simplifyType(CS.getType(destExpr));
+  TC.diagnose(loc, unknownDiagID, type)
       .highlight(immInfo.first->getSourceRange());
 }
 
@@ -6153,33 +6154,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"
@@ -532,3 +533,44 @@ bool MissingOptionalUnwrapFailure::diagnose() {
       .fixItReplace({tryExpr->getTryLoc(), tryExpr->getQuestionLoc()}, "try!");
   return true;
 }
+
+bool RValueTreatedAsLValueFailure::diagnose() {
+  Diag<StringRef> subElementDiagID;
+  Diag<Type> rvalueDiagID;
+  Expr *diagExpr = getLocator()->getAnchor();
+  SourceLoc loc;
+
+  auto callExpr = dyn_cast<ApplyExpr>(diagExpr);
+  if (!callExpr)
+    return false; // currently only creating these for args, so should be
+                  // unreachable
+
+  Expr *argExpr = callExpr->getArg();
+  loc = callExpr->getFn()->getLoc();
+
+  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();
+  }
+
+  diagnoseSubElementFailure(diagExpr, loc, getConstraintSystem(),
+                            subElementDiagID, rvalueDiagID);
+  return true;
+}

lib/Sema/CSDiagnostics.h

@@ -345,6 +345,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/CSFix.cpp

@@ -104,6 +104,17 @@ AddAddressOf *AddAddressOf::create(ConstraintSystem &cs,
   return new (cs.getAllocator()) AddAddressOf(locator);
 }
 
+bool TreatRValueAsLValue::diagnose(Expr *root, const Solution &solution) const {
+  RValueTreatedAsLValueFailure failure(solution, getLocator());
+  return failure.diagnose();
+}
+
+TreatRValueAsLValue *TreatRValueAsLValue::create(ConstraintSystem &cs,
+                                   ConstraintLocator *locator) {
+  return new (cs.getAllocator()) TreatRValueAsLValue(locator);
+}
+
+
 bool CoerceToCheckedCast::diagnose(Expr *root, const Solution &solution) const {
   MissingForcedDowncastFailure failure(root, solution, getLocator());
   return failure.diagnose();

lib/Sema/CSFix.h

@@ -68,6 +68,9 @@ enum class FixKind : uint8_t {
 
   /// Add a new conformance to the type to satisfy a requirement.
   AddConformance,
+
+  /// Treat rvalue as lvalue
+  TreatRValueAsLValue,
 };
 
 class ConstraintFix {
@@ -168,6 +171,20 @@ class AddAddressOf final : public ConstraintFix {
   static AddAddressOf *create(ConstraintSystem &cs, ConstraintLocator *locator);
 };
 
+// Treat rvalue as if it was an lvalue
+class TreatRValueAsLValue final : public ConstraintFix {
+  TreatRValueAsLValue(ConstraintLocator *locator)
+    : ConstraintFix(FixKind::TreatRValueAsLValue, locator) {}
+
+public:
+  std::string getName() const override { return "treat rvalue as lvalue"; }
+
+  bool diagnose(Expr *root, const Solution &solution) const override;
+
+  static TreatRValueAsLValue *create(ConstraintSystem &cs, ConstraintLocator *locator);
+};
+
+
 /// Replace a coercion ('as') with a forced checked cast ('as!').
 class CoerceToCheckedCast final : public ConstraintFix {
   CoerceToCheckedCast(ConstraintLocator *locator)

lib/Sema/CSSimplify.cpp

@@ -2439,10 +2439,16 @@ 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(
+          TreatRValueAsLValue::create(*this, getConstraintLocator(locator)));
+      }
     }
   }
 
@@ -4980,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))
+        return SolutionKind::Error;
+
+    return result;
+  }
+
   case FixKind::ExplicitlyEscaping:
   case FixKind::CoerceToCheckedCast:
   case FixKind::RelabelArguments:

lib/Sema/ConstraintLocator.h

@@ -343,7 +343,7 @@ class ConstraintLocator : public llvm::FoldingSetNode {
       return PathElement(NamedTupleElement, position);
     }
 
-    /// Retrieve a patch element for an argument/parameter comparison in a
+    /// Retrieve a path element for an argument/parameter comparison in a
     /// function application.
     static PathElement getApplyArgToParam(unsigned argIdx, unsigned paramIdx) {
       return PathElement(ApplyArgToParam, argIdx, paramIdx);

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}}
 }