Want to directly use the locator anchor for RValueTreatedAsLValueFailure::diagnose() rather than the computed anchor. Assorted other cleanup, comment improvement.

Author: gregomni

lib/Sema/CSDiagnostics.cpp

@@ -530,42 +530,50 @@ bool MissingOptionalUnwrapFailure::diagnose() {
 }
 
 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();
-    }
+  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();
+  }
 
-    // FIXME: Would like for this to be unnecessary.
-    getConstraintSystem().TC.typeCheckExpression(diagExpr, getConstraintSystem().DC);
+  // FIXME: Needed right now to apply correct overload choices to diagExpr for
+  // use by diagnoseSubElementFailure(). Once all callers are routed through
+  // here, that function can be rewritten to take the current Solution and use
+  // it for determining chosen decl bindings instead, making this extra
+  // typecheck 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;
+  diagnoseSubElementFailure(diagExpr, loc, getConstraintSystem(),
+                            subElementDiagID, rvalueDiagID);
+  return true;
 }

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);