Merge pull request #18950 from gregomni/rvalue-as-lvalue

[Diagnostics] Adding assignments directly to CS and diagnosing from there.

Author: gregomni

lib/Sema/CSApply.cpp

@@ -3832,18 +3832,11 @@ namespace {
     }
 
     Expr *visitAssignExpr(AssignExpr *expr) {
-      // Compute the type to which the source must be converted to allow
-      // assignment to the destination.
-      //
-      // FIXME: This is also computed when the constraint system is set up.
-      auto destTy = cs.computeAssignDestType(expr->getDest(), expr->getLoc());
-      if (!destTy)
-        return nullptr;
-
       // Convert the source to the simplified destination type.
+      auto destTy = simplifyType(cs.getType(expr->getDest()));
       auto locator =
         ConstraintLocatorBuilder(cs.getConstraintLocator(expr->getSrc()));
-      Expr *src = coerceToType(expr->getSrc(), destTy, locator);
+      Expr *src = coerceToType(expr->getSrc(), destTy->getRValueType(), locator);
       if (!src)
         return nullptr;
 

lib/Sema/CSDiag.cpp

@@ -375,6 +375,17 @@ resolveImmutableBase(Expr *expr, ConstraintSystem &CS) {
         return { expr, member };
     }
 
+    if (auto tupleExpr = dyn_cast<TupleExpr>(SE->getIndex())) {
+      if (tupleExpr->getNumElements() == 1 && tupleExpr->getElementName(0).str() == "keyPath") {
+        auto indexType = CS.simplifyType(CS.getType(tupleExpr->getElement(0)));
+        if (auto bgt = indexType->getAs<BoundGenericType>()) {
+          auto decl = bgt->getDecl();
+          if (decl == CS.getASTContext().getKeyPathDecl())
+            return resolveImmutableBase(tupleExpr->getElement(0), CS);
+        }
+      }
+    }
+
     // If it is settable, then the base must be the problem, recurse.
     return resolveImmutableBase(SE->getBase(), CS);
   }
@@ -430,6 +441,9 @@ resolveImmutableBase(Expr *expr, ConstraintSystem &CS) {
     if (!isa<LoadExpr>(ICE->getSubExpr()))
       return resolveImmutableBase(ICE->getSubExpr(), CS);
 
+  if (auto *SAE = dyn_cast<SelfApplyExpr>(expr))
+    return resolveImmutableBase(SAE->getFn(), CS);
+
   return { expr, nullptr };
 }
 
@@ -526,7 +540,11 @@ void swift::diagnoseSubElementFailure(Expr *destExpr,
     message += VD->getName().str().str();
     message += "'";
 
-    if (VD->isCaptureList())
+    auto type = CS.simplifyType(CS.getType(immInfo.first));
+    auto bgt = type ? type->getAs<BoundGenericType>() : nullptr;
+    if (bgt && bgt->getDecl() == CS.getASTContext().getKeyPathDecl())
+      message += " is a read-only key path";
+    else if (VD->isCaptureList())
       message += " is an immutable capture";
     else if (VD->isImplicit())
       message += " is immutable";
@@ -584,6 +602,14 @@ void swift::diagnoseSubElementFailure(Expr *destExpr,
     return;
   }
 
+  // If a keypath was the problem but wasn't resolved into a vardecl
+  // it is ambiguous or unable to be used for setting.
+  if (auto *KPE = dyn_cast_or_null<KeyPathExpr>(immInfo.first)) {
+    TC.diagnose(loc, diagID, "immutable key path")
+      .highlight(KPE->getSourceRange());
+    return;
+  }
+
   // If the expression is the result of a call, it is an rvalue, not a mutable
   // lvalue.
   if (auto *AE = dyn_cast<ApplyExpr>(immInfo.first)) {
@@ -3087,44 +3113,30 @@ bool FailureDiagnosis::diagnoseContextualConversionError(
 
 /// When an assignment to an expression is detected and the destination is
 /// invalid, emit a detailed error about the condition.
-void ConstraintSystem::diagnoseAssignmentFailure(Expr *dest, Type destTy,
+bool ConstraintSystem::diagnoseAssignmentFailure(Expr *dest, Type destTy,
                                                  SourceLoc equalLoc) {
   auto &TC = getTypeChecker();
 
-  // Diagnose obvious assignments to literals.
-  if (isa<LiteralExpr>(dest->getValueProvidingExpr())) {
-    TC.diagnose(equalLoc, diag::cannot_assign_to_literal);
-    return;
-  }
-
-  // Diagnose assignments to let-properties in delegating initializers.
-  if (auto *member = dyn_cast<UnresolvedDotExpr>(dest)) {
-    if (auto *ctor = dyn_cast<ConstructorDecl>(DC)) {
-      if (auto *baseRef = dyn_cast<DeclRefExpr>(member->getBase())) {
-        if (baseRef->getDecl() == ctor->getImplicitSelfDecl() &&
-            ctor->getDelegatingOrChainedInitKind(nullptr) ==
-              ConstructorDecl::BodyInitKind::Delegating) {
-          auto resolved = resolveImmutableBase(member, *this);
-          assert(resolved.first == member);
-          TC.diagnose(equalLoc, diag::assignment_let_property_delegating_init,
-                      member->getName());
-          if (resolved.second) {
-            TC.diagnose(resolved.second, diag::decl_declared_here,
-                        member->getName());
-          }
-          return;
-        }
-      }
+  // Assignments to let-properties in delegating initializers will be caught
+  // elsewhere now, so if we see them here, it isn't an assignment problem.
+  if (auto *ctor = dyn_cast<ConstructorDecl>(DC)) {
+    DeclRefExpr *baseRef = nullptr;
+    if (auto *member = dyn_cast<UnresolvedDotExpr>(dest)) {
+      baseRef = dyn_cast<DeclRefExpr>(member->getBase());
+    } else if (auto *member = dyn_cast<MemberRefExpr>(dest)) {
+      if (auto *load = dyn_cast<LoadExpr>(member->getBase()))
+        baseRef = dyn_cast<DeclRefExpr>(load->getSubExpr());
+    }
+    if (baseRef && baseRef->getDecl() == ctor->getImplicitSelfDecl() &&
+        ctor->getDelegatingOrChainedInitKind(nullptr) ==
+          ConstructorDecl::BodyInitKind::Delegating) {
+      return false;
     }
   }
 
   Diag<StringRef> diagID;
-  if (isa<ApplyExpr>(dest))
+  if (isa<ApplyExpr>(dest) || isa<SelfApplyExpr>(dest))
     diagID = diag::assignment_lhs_is_apply_expression;
-  else if (isa<DeclRefExpr>(dest))
-    diagID = diag::assignment_lhs_is_immutable_variable;
-  else if (isa<ForceValueExpr>(dest))
-    diagID = diag::assignment_bang_has_immutable_subcomponent;
   else if (isa<UnresolvedDotExpr>(dest) || isa<MemberRefExpr>(dest))
     diagID = diag::assignment_lhs_is_immutable_property;
   else if (auto sub = dyn_cast<SubscriptExpr>(dest)) {
@@ -3137,12 +3149,12 @@ void ConstraintSystem::diagnoseAssignmentFailure(Expr *dest, Type destTy,
         sub->getArgumentLabels().size() == 1 &&
         sub->getArgumentLabels().front() == TC.Context.Id_dynamicMember)
       diagID = diag::assignment_dynamic_property_has_immutable_base;
-  } else {
+  } else
     diagID = diag::assignment_lhs_is_immutable_variable;
-  }
 
   diagnoseSubElementFailure(dest, equalLoc, *this, diagID,
                             diag::assignment_lhs_not_lvalue);
+  return true;
 }
 
 //===----------------------------------------------------------------------===//
@@ -6231,8 +6243,8 @@ bool FailureDiagnosis::visitAssignExpr(AssignExpr *assignExpr) {
       if (diagnoseSubscriptErrors(subscriptExpr, /* inAssignmentDestination = */ true))
         return true;
     }
-    CS.diagnoseAssignmentFailure(destExpr, destType, assignExpr->getLoc());
-    return true;
+    if (CS.diagnoseAssignmentFailure(destExpr, destType, assignExpr->getLoc()))
+      return true;
   }
 
   auto *srcExpr = assignExpr->getSrc();

lib/Sema/CSDiagnostics.cpp

@@ -279,7 +279,10 @@ bool MissingForcedDowncastFailure::diagnoseAsError() {
 
   auto &TC = getTypeChecker();
 
-  auto *coerceExpr = dyn_cast<CoerceExpr>(getAnchor());
+  auto *expr = getAnchor();
+  if (auto *assignExpr = dyn_cast<AssignExpr>(expr))
+    expr = assignExpr->getSrc();
+  auto *coerceExpr = dyn_cast<CoerceExpr>(expr);
   if (!coerceExpr)
     return false;
 
@@ -333,6 +336,8 @@ bool MissingExplicitConversionFailure::diagnoseAsError() {
   auto &TC = getTypeChecker();
 
   auto *anchor = getAnchor();
+  if (auto *assign = dyn_cast<AssignExpr>(anchor))
+    anchor = assign->getSrc();
   if (auto *paren = dyn_cast<ParenExpr>(anchor))
     anchor = paren->getSubExpr();
 
@@ -540,6 +545,10 @@ bool MissingOptionalUnwrapFailure::diagnoseAsError() {
     return false;
 
   auto *anchor = getAnchor();
+
+  if (auto assignExpr = dyn_cast<AssignExpr>(anchor))
+    anchor = assignExpr->getSrc();
+  
   auto *unwrapped = anchor->getValueProvidingExpr();
   auto type = getType(anchor)->getRValueType();
 
@@ -554,9 +563,13 @@ bool MissingOptionalUnwrapFailure::diagnoseAsError() {
 
 bool RValueTreatedAsLValueFailure::diagnoseAsError() {
   Diag<StringRef> subElementDiagID;
-  Diag<Type> rvalueDiagID;
+  Diag<Type> rvalueDiagID = diag::assignment_lhs_not_lvalue;
   Expr *diagExpr = getLocator()->getAnchor();
-  SourceLoc loc;
+  SourceLoc loc = diagExpr->getLoc();
+
+  if (auto assignExpr = dyn_cast<AssignExpr>(diagExpr)) {
+    diagExpr = assignExpr->getDest();
+  }
 
   if (auto callExpr = dyn_cast<ApplyExpr>(diagExpr)) {
     Expr *argExpr = callExpr->getArg();
@@ -571,7 +584,7 @@ bool RValueTreatedAsLValueFailure::diagnoseAsError() {
       rvalueDiagID = diag::cannot_apply_lvalue_binop_to_rvalue;
       auto argTuple = dyn_cast<TupleExpr>(argExpr);
       diagExpr = argTuple->getElement(0);
-    } else {
+    } else if (getLocator()->getPath().size() > 0) {
       auto lastPathElement = getLocator()->getPath().back();
       assert(lastPathElement.getKind() ==
              ConstraintLocator::PathElementKind::ApplyArgToParam);
@@ -582,10 +595,11 @@ bool RValueTreatedAsLValueFailure::diagnoseAsError() {
         diagExpr = argTuple->getElement(lastPathElement.getValue());
       else if (auto parens = dyn_cast<ParenExpr>(argExpr))
         diagExpr = parens->getSubExpr();
+    } else {
+      subElementDiagID = diag::assignment_lhs_is_apply_expression;
     }
   } else if (auto inoutExpr = dyn_cast<InOutExpr>(diagExpr)) {
-    Type type = getConstraintSystem().getType(inoutExpr);
-    if (auto restriction = restrictionForType(type)) {
+    if (auto restriction = getRestrictionForType(getType(inoutExpr))) {
       PointerTypeKind pointerKind;
       if (restriction->second == ConversionRestrictionKind::ArrayToPointer &&
           restriction->first->getAnyPointerElementType(pointerKind) &&
@@ -603,10 +617,42 @@ bool RValueTreatedAsLValueFailure::diagnoseAsError() {
 
     subElementDiagID = diag::cannot_pass_rvalue_inout_subelement;
     rvalueDiagID = diag::cannot_pass_rvalue_inout;
-    loc = diagExpr->getLoc();
     diagExpr = inoutExpr->getSubExpr();
+  } else if (isa<DeclRefExpr>(diagExpr)) {
+    subElementDiagID = diag::assignment_lhs_is_immutable_variable;
+  } else if (isa<ForceValueExpr>(diagExpr)) {
+    subElementDiagID = diag::assignment_bang_has_immutable_subcomponent;
+  } else if (isa<MemberRefExpr>(diagExpr)) {
+    subElementDiagID = diag::assignment_lhs_is_immutable_property;
+  } else if (auto member = dyn_cast<UnresolvedDotExpr>(diagExpr)) {
+    subElementDiagID = diag::assignment_lhs_is_immutable_property;
+
+    if (auto *ctor = dyn_cast<ConstructorDecl>(getDC())) {
+      if (auto *baseRef = dyn_cast<DeclRefExpr>(member->getBase())) {
+        if (baseRef->getDecl() == ctor->getImplicitSelfDecl() &&
+            ctor->getDelegatingOrChainedInitKind(nullptr) ==
+            ConstructorDecl::BodyInitKind::Delegating) {
+          emitDiagnostic(loc, diag::assignment_let_property_delegating_init,
+                      member->getName());
+          if (auto *ref = getResolvedMemberRef(member)) {
+            emitDiagnostic(ref, diag::decl_declared_here, member->getName());
+          }
+          return true;
+        }
+      }
+    }
+  } else if (auto sub = dyn_cast<SubscriptExpr>(diagExpr)) {
+      subElementDiagID = diag::assignment_subscript_has_immutable_base;
+
+      // If the destination is a subscript with a 'dynamicLookup:' label and if
+      // the tuple is implicit, then this was actually a @dynamicMemberLookup
+      // access. Emit a more specific diagnostic.
+      if (sub->getIndex()->isImplicit() &&
+          sub->getArgumentLabels().size() == 1 &&
+          sub->getArgumentLabels().front() == getTypeChecker().Context.Id_dynamicMember)
+        subElementDiagID = diag::assignment_dynamic_property_has_immutable_base;
   } else {
-    return false;
+    subElementDiagID = diag::assignment_lhs_is_immutable_variable;
   }
 
   diagnoseSubElementFailure(diagExpr, loc, getConstraintSystem(),

lib/Sema/CSDiagnostics.h

@@ -95,7 +95,7 @@ class FailureDiagnostic {
   DeclContext *getDC() const { return CS.DC; }
 
   Optional<std::pair<Type, ConversionRestrictionKind>>
-  restrictionForType(Type type) const {
+  getRestrictionForType(Type type) const {
     for (auto &restriction : CS.ConstraintRestrictions) {
       if (std::get<0>(restriction)->isEqual(type))
         return std::pair<Type, ConversionRestrictionKind>(
@@ -104,6 +104,11 @@ class FailureDiagnostic {
     return None;
   }
 
+  ValueDecl *getResolvedMemberRef(UnresolvedDotExpr *member) {
+    auto locator = CS.getConstraintLocator(member, ConstraintLocator::Member);
+    return CS.findResolvedMemberRef(locator);
+  }
+
   Optional<SelectedOverload>
   getOverloadChoiceIfAvailable(ConstraintLocator *locator) const {
     if (auto *overload = getResolvedOverload(locator))

lib/Sema/CSGen.cpp

@@ -2745,26 +2745,30 @@ namespace {
       auto typeVar = CS.createTypeVariable(locator);
       return LValueType::get(typeVar);
     }
-    
+
+    static Type genAssignDestType(Expr *expr, ConstraintSystem &CS) {
+      if (auto *TE = dyn_cast<TupleExpr>(expr)) {
+        SmallVector<TupleTypeElt, 4> destTupleTypes;
+        for (unsigned i = 0; i !=  TE->getNumElements(); ++i) {
+          Type subType = genAssignDestType(TE->getElement(i), CS);
+          destTupleTypes.push_back(TupleTypeElt(subType, TE->getElementName(i)));
+        }
+        return TupleType::get(destTupleTypes, CS.getASTContext());
+      } else {
+        Type destTy = CS.createTypeVariable(CS.getConstraintLocator(expr));
+        CS.addConstraint(ConstraintKind::Bind, CS.getType(expr), LValueType::get(destTy),
+                         CS.getConstraintLocator(expr));
+        return destTy;
+      }
+    }
+
     Type visitAssignExpr(AssignExpr *expr) {
       // Handle invalid code.
       if (!expr->getDest() || !expr->getSrc())
         return Type();
-      
-      // Compute the type to which the source must be converted to allow
-      // assignment to the destination.
-      auto destTy = CS.computeAssignDestType(expr->getDest(), expr->getLoc());
-      if (!destTy)
-        return Type();
-      if (destTy->is<UnresolvedType>()) {
-        return CS.createTypeVariable(CS.getConstraintLocator(expr));
-      }
-      
-      // The source must be convertible to the destination.
-      CS.addConstraint(ConstraintKind::Conversion,
-                       CS.getType(expr->getSrc()), destTy,
-                       CS.getConstraintLocator(expr->getSrc()));
-
+      Type destTy = genAssignDestType(expr->getDest(), CS);
+      CS.addConstraint(ConstraintKind::Conversion, CS.getType(expr->getSrc()), destTy,
+                       CS.getConstraintLocator(expr));
       return TupleType::getEmpty(CS.getASTContext());
     }