[Diagnostics] Improve mutability diagnostics related to dynamic member lookup

Since there is a way to mutate through use of writable keypath
diagnostics have to be adjusted to point to the members found
via keypath member lookup instead to using catch-all
"immutable base" diagnostic.

Author: xedin

lib/Sema/CSDiagnostics.cpp

@@ -776,7 +776,7 @@ bool MissingOptionalUnwrapFailure::diagnoseAsError() {
 bool RValueTreatedAsLValueFailure::diagnoseAsError() {
   Diag<StringRef> subElementDiagID;
   Diag<Type> rvalueDiagID = diag::assignment_lhs_not_lvalue;
-  Expr *diagExpr = getLocator()->getAnchor();
+  Expr *diagExpr = getRawAnchor();
   SourceLoc loc = diagExpr->getLoc();
 
   if (auto assignExpr = dyn_cast<AssignExpr>(diagExpr)) {
@@ -854,12 +854,18 @@ bool RValueTreatedAsLValueFailure::diagnoseAsError() {
       }
     }
 
-    if (auto resolvedOverload = getResolvedOverload(getLocator()))
+    if (auto resolvedOverload = getResolvedOverload(getLocator())) {
       if (resolvedOverload->Choice.getKind() ==
-              OverloadChoiceKind::DynamicMemberLookup ||
-          resolvedOverload->Choice.getKind() ==
-              OverloadChoiceKind::KeyPathDynamicMemberLookup)
+          OverloadChoiceKind::DynamicMemberLookup)
         subElementDiagID = diag::assignment_dynamic_property_has_immutable_base;
+
+      if (resolvedOverload->Choice.getKind() ==
+          OverloadChoiceKind::KeyPathDynamicMemberLookup) {
+        if (!getType(member->getBase())->hasLValueType())
+          subElementDiagID =
+              diag::assignment_dynamic_property_has_immutable_base;
+      }
+    }
   } else if (auto sub = dyn_cast<SubscriptExpr>(diagExpr)) {
       subElementDiagID = diag::assignment_subscript_has_immutable_base;
   } else {
@@ -1204,7 +1210,7 @@ AssignmentFailure::resolveImmutableBase(Expr *expr) const {
     if (!member) {
       auto loc =
           cs.getConstraintLocator(SE, ConstraintLocator::SubscriptMember);
-      member = dyn_cast_or_null<SubscriptDecl>(cs.findResolvedMemberRef(loc));
+      member = dyn_cast_or_null<SubscriptDecl>(getMemberRef(loc));
     }
 
     // If it isn't settable, return it.
@@ -1233,9 +1239,10 @@ AssignmentFailure::resolveImmutableBase(Expr *expr) const {
     // If we found a decl for the UDE, check it.
     auto loc = cs.getConstraintLocator(UDE, ConstraintLocator::Member);
 
+    auto *member = getMemberRef(loc);
     // If we can resolve a member, we can determine whether it is settable in
     // this context.
-    if (auto *member = cs.findResolvedMemberRef(loc)) {
+    if (member) {
       auto *memberVD = dyn_cast<VarDecl>(member);
 
       // If the member isn't a vardecl (e.g. its a funcdecl), or it isn't
@@ -1283,6 +1290,43 @@ AssignmentFailure::resolveImmutableBase(Expr *expr) const {
   return {expr, nullptr};
 }
 
+ValueDecl *AssignmentFailure::getMemberRef(ConstraintLocator *locator) const {
+  auto member = getOverloadChoiceIfAvailable(locator);
+  if (!member || !member->choice.isDecl())
+    return nullptr;
+
+  auto *DC = getDC();
+  auto &TC = getTypeChecker();
+
+  auto *decl = member->choice.getDecl();
+  if (isa<SubscriptDecl>(decl) &&
+      isValidDynamicMemberLookupSubscript(cast<SubscriptDecl>(decl), DC, TC)) {
+    auto *subscript = cast<SubscriptDecl>(decl);
+    // If this is a keypath dynamic member lookup, we have to
+    // adjust the locator to find member referred by it.
+    if (isValidKeyPathDynamicMemberLookup(subscript, TC)) {
+      auto &cs = getConstraintSystem();
+      // Type has a following format:
+      // `(Self) -> (dynamicMember: {Writable}KeyPath<T, U>) -> U`
+      auto *fullType = member->openedFullType->castTo<FunctionType>();
+      auto *fnType = fullType->getResult()->castTo<FunctionType>();
+
+      auto paramTy = fnType->getParams()[0].getPlainType();
+      auto keyPath = paramTy->getAnyNominal();
+      auto memberLoc = cs.getConstraintLocator(
+          locator, LocatorPathElt::getKeyPathDynamicMember(keyPath));
+
+      auto memberRef = getOverloadChoiceIfAvailable(memberLoc);
+      return memberRef ? memberRef->choice.getDecl() : nullptr;
+    }
+
+    // If this is a string based dynamic lookup, there is no member declaration.
+    return nullptr;
+  }
+
+  return decl;
+}
+
 Diag<StringRef> AssignmentFailure::findDeclDiagonstic(ASTContext &ctx,
                                                       Expr *destExpr) {
   if (isa<ApplyExpr>(destExpr) || isa<SelfApplyExpr>(destExpr))

lib/Sema/CSDiagnostics.h

@@ -647,6 +647,10 @@ class AssignmentFailure final : public FailureDiagnostic {
              isLoadedLValue(ifExpr->getElseExpr());
     return false;
   }
+
+  /// Retrive an member reference associated with given member
+  /// looking through dynamic member lookup on the way.
+  ValueDecl *getMemberRef(ConstraintLocator *locator) const;
 };
 
 /// Intended to diagnose any possible contextual failure

test/attr/attr_dynamic_member_lookup.swift

@@ -402,7 +402,7 @@ _ = \KP.testLookup
 
 struct Point {
   var x: Int
-  let y: Int
+  let y: Int // expected-note 2 {{change 'let' to 'var' to make it mutable}}
 
   private let z: Int = 0 // expected-note 7 {{declared here}}
 }
@@ -447,7 +447,7 @@ _ = lens.bottomRight.z // expected-error {{'z' is inaccessible due to 'private'
 
 lens.topLeft = Lens(Point(x: 1, y: 2)) // Ok
 lens.bottomRight.x = Lens(11)          // Ok
-lens.bottomRight.y = Lens(12)          // expected-error {{cannot assign through dynamic lookup property: 'lens' is immutable}}
+lens.bottomRight.y = Lens(12)          // expected-error {{cannot assign to property: 'y' is a 'let' constant}}
 lens.bottomRight.z = Lens(13)          // expected-error {{'z' is inaccessible due to 'private' protection level}}
 
 func acceptKeyPathDynamicLookup(_: Lens<Int>) {}
@@ -576,7 +576,7 @@ func keypath_with_subscripts(_ arr: SubscriptLens<[Int]>,
   _ = arr["hello"]  // Ok
   _ = dict["hello"] // Ok
 
-  _ = arr["hello"] = 42 // expected-error {{cannot assign through subscript: subscript is get-only}}
+  _ = arr["hello"] = 42 // expected-error {{cannot assign through subscript: 'arr' is a 'let' constant}}
   _ = dict["hello"] = 0 // Ok
 
   _ = arr[0] = 42 // expected-error {{cannot assign through subscript: 'arr' is a 'let' constant}}
@@ -620,6 +620,6 @@ func keypath_to_subscript_to_property(_ lens: inout Lens<Array<Rectangle>>) {
   _ = lens[0].topLeft.x
   _ = lens[0].topLeft.y
   _ = lens[0].topLeft.x = Lens(0)
-  _ = lens[0].topLeft.y = Lens(1) // FIXME(diagnostics): Diagnostic should point to 'y' instead of 'lens'
-  // expected-error@-1 {{cannot assign through dynamic lookup property: 'lens' is immutable}}
+  _ = lens[0].topLeft.y = Lens(1)
+  // expected-error@-1 {{cannot assign to property: 'y' is a 'let' constant}}
 }

test/expr/unary/keypath/keypath.swift

@@ -350,9 +350,9 @@ func testKeyPathSubscript(readonly: ZwithSubscript, writable: inout ZwithSubscri
   sink = writable[keyPath: wkp]
   sink = readonly[keyPath: rkp]
   sink = writable[keyPath: rkp]
-  readonly[keyPath: kp] = sink // expected-error{{cannot assign through subscript: subscript is get-only}}
-  writable[keyPath: kp] = sink // expected-error{{cannot assign through subscript: subscript is get-only}}
-  readonly[keyPath: wkp] = sink // expected-error{{cannot assign through subscript: subscript is get-only}}
+  readonly[keyPath: kp] = sink  // expected-error {{cannot assign through subscript: 'kp' is a read-only key path}}
+  writable[keyPath: kp] = sink  // expected-error {{cannot assign through subscript: 'kp' is a read-only key path}}
+  readonly[keyPath: wkp] = sink // expected-error {{cannot assign through subscript: 'readonly' is a 'let' constant}}
   // FIXME: silently falls back to keypath application, which seems inconsistent
   writable[keyPath: wkp] = sink
   // FIXME: silently falls back to keypath application, which seems inconsistent