[CSSimplify] Increase impact of treat r-value as l-value fix in certain situations

If location (member) isn't mutable in the current context
or there are other problems at this location, increase impact
of the fix since it compounds the problem.

Author: xedin

include/swift/Sema/CSFix.h

@@ -620,6 +620,10 @@ class TreatRValueAsLValue final : public ConstraintFix {
     return diagnose(*commonFixes.front().first);
   }
 
+  /// Assess the impact this fix is going to have at the given location.
+  static unsigned assessImpact(ConstraintSystem &cs,
+                               ConstraintLocator *atLoc);
+
   static TreatRValueAsLValue *create(ConstraintSystem &cs,
                                      ConstraintLocator *locator);
 

lib/Sema/CSFix.cpp

@@ -141,6 +141,29 @@ bool TreatRValueAsLValue::diagnose(const Solution &solution,
   return failure.diagnose(asNote);
 }
 
+unsigned TreatRValueAsLValue::assessImpact(ConstraintSystem &cs,
+                                           ConstraintLocator *atLoc) {
+    // Results of calls can never be l-value.
+    unsigned impact = isExpr<CallExpr>(atLoc->getAnchor()) ? 2 : 1;
+    // An overload choice that isn't settable is least interesting for
+    // diagnosis.
+    auto *calleeLoc = cs.getCalleeLocator(atLoc, /*lookThroughApply=*/false);
+    if (auto overload = cs.findSelectedOverloadFor(calleeLoc)) {
+      if (auto *var = dyn_cast_or_null<AbstractStorageDecl>(
+              overload->choice.getDeclOrNull())) {
+        impact += !var->isSettableInSwift(cs.DC) ? 1 : 0;
+      } else {
+        impact += 1;
+      }
+    }
+
+    // This is extra impactful if location has other issues.
+    if (cs.hasFixFor(atLoc) || cs.hasFixFor(calleeLoc))
+      impact += 2;
+
+    return impact;
+}
+
 TreatRValueAsLValue *TreatRValueAsLValue::create(ConstraintSystem &cs,
                                    ConstraintLocator *locator) {
   if (locator->isLastElement<LocatorPathElt::ApplyArgToParam>())

lib/Sema/CSSimplify.cpp

@@ -14023,26 +14023,9 @@ ConstraintSystem::simplifyLValueObjectConstraint(
     if (!shouldAttemptFixes())
       return SolutionKind::Error;
 
-    auto assessImpact = [&]() -> unsigned {
-      // If this is a projection of a member reference
-      // let's check whether the member is unconditionally
-      // settable, if so than it's a problem with its base.
-      if (locator.directlyAt<UnresolvedDotExpr>()) {
-        auto *memberLoc = getConstraintLocator(locator.getAnchor(),
-                                               ConstraintLocator::Member);
-        if (auto selected = findSelectedOverloadFor(memberLoc)) {
-          if (auto *storage = dyn_cast_or_null<AbstractStorageDecl>(
-                  selected->choice.getDeclOrNull())) {
-            return storage->isSettable(nullptr) ? 1 : 2;
-          }
-        }
-      }
-      return 2;
-    };
-
-    if (recordFix(
-            TreatRValueAsLValue::create(*this, getConstraintLocator(locator)),
-            assessImpact()))
+    auto *fixLoc = getConstraintLocator(locator);
+    if (recordFix(TreatRValueAsLValue::create(*this, fixLoc),
+                  TreatRValueAsLValue::assessImpact(*this, fixLoc)))
       return SolutionKind::Error;
 
     lvalueTy = LValueType::get(lvalueTy);
@@ -15198,21 +15181,8 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
   }
 
   case FixKind::TreatRValueAsLValue: {
-    unsigned impact = 1;
-    // If this is an attempt to use result of a function/subscript call as
-    // an l-value, it has to have an increased impact because it's either
-    // a function - which is completely incorrect, or it's a get-only
-    // subscript, which requires changes to declaration to become mutable.
-    impact += (locator.endsWith<LocatorPathElt::FunctionResult>() ||
-               locator.endsWith<LocatorPathElt::SubscriptMember>())
-                  ? 1
-                  : 0;
-    // An overload choice that isn't settable is least interesting for diagnosis.
-    if (auto overload = findSelectedOverloadFor(getCalleeLocator(fix->getLocator()))) {
-      if (auto *var = dyn_cast_or_null<VarDecl>(overload->choice.getDeclOrNull())) {
-         impact += !var->isSettableInSwift(DC) ? 1 : 0;
-      }
-    }
+    unsigned impact =
+        TreatRValueAsLValue::assessImpact(*this, fix->getLocator());
     return recordFix(fix, impact) ? SolutionKind::Error : SolutionKind::Solved;
   }
 

test/Generics/function_defs.swift

@@ -123,15 +123,15 @@ protocol Subscriptable {
   func getIndex() -> Index
   func getValue() -> Value
 
-  subscript (index : Index) -> Value { get set } // expected-note {{candidate expects value of type 'T.Index' for parameter #1 (got 'T.Value')}}
+  subscript (index : Index) -> Value { get set }
 }
 
 protocol IntSubscriptable {
   associatedtype ElementType
 
   func getElement() -> ElementType
 
-  subscript (index : Int) -> ElementType { get  } // expected-note {{candidate expects value of type 'Int' for parameter #1 (got 'T.Value')}}
+  subscript (index : Int) -> ElementType { get }
 }
 
 func subscripting<T : Subscriptable & IntSubscriptable>(_ t: T) {
@@ -144,7 +144,9 @@ func subscripting<T : Subscriptable & IntSubscriptable>(_ t: T) {
   element = t[17]
   t[42] = element // expected-error{{cannot assign through subscript: subscript is get-only}}
 
-  t[value] = 17 // expected-error{{no exact matches in call to subscript}}
+  // Note that this is not an ambiguity because only one subscript is mutating
+  t[value] = 17 // expected-error{{cannot convert value of type 'T.Value' to expected argument type 'T.Index'}}
+  // expected-error@-1 {{cannot assign value of type 'Int' to subscript of type 'T.Value'}}
 }
 
 //===----------------------------------------------------------------------===//