Merge pull request #86134 from xedin/rdar-157249275

[CSDiagnostics] Produce a tailored diagnostic for key path mutability mismatches

Author: xedin

lib/Sema/CSDiagnostics.cpp

@@ -16,6 +16,7 @@
 
 #include "CSDiagnostics.h"
 #include "MiscDiagnostics.h"
+#include "TypeCheckAvailability.h"
 #include "TypeCheckConcurrency.h"
 #include "TypeCheckProtocol.h"
 #include "TypeCheckType.h"
@@ -47,6 +48,7 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/PointerUnion.h"
 #include "llvm/ADT/SmallString.h"
+#include "llvm/Support/ErrorHandling.h"
 #include <string>
 
 using namespace swift;
@@ -2628,6 +2630,9 @@ bool ContextualFailure::diagnoseAsError() {
     }
   }
 
+  if (diagnoseKeyPathLiteralMutabilityMismatch())
+    return true;
+
   if (diagnoseConversionToNil())
     return true;
 
@@ -2994,6 +2999,48 @@ getContextualNilDiagnostic(ContextualTypePurpose CTP) {
   llvm_unreachable("Unhandled ContextualTypePurpose in switch");
 }
 
+bool ContextualFailure::diagnoseKeyPathLiteralMutabilityMismatch() const {
+  auto fromType = getFromType();
+  auto toType = getToType();
+
+  if (!(fromType->isKeyPath() &&
+        (toType->isWritableKeyPath() || toType->isReferenceWritableKeyPath())))
+    return false;
+
+  auto keyPathLiteral = getAsExpr<KeyPathExpr>(getAnchor());
+  if (!keyPathLiteral)
+    return false;
+
+  auto &S = getSolution();
+  for (unsigned i : indices(keyPathLiteral->getComponents())) {
+    auto &component = keyPathLiteral->getComponents()[i];
+
+    auto *componentLoc = getConstraintLocator(
+        keyPathLiteral, LocatorPathElt::KeyPathComponent(i));
+
+    auto overload = S.getCalleeOverloadChoiceIfAvailable(componentLoc);
+    if (!overload)
+      continue;
+
+    auto *storageDecl =
+        dyn_cast_or_null<AbstractStorageDecl>(overload->choice.getDeclOrNull());
+    if (!storageDecl)
+      continue;
+
+    if (auto *setter = storageDecl->getOpaqueAccessor(AccessorKind::Set)) {
+      if (getUnsatisfiedAvailabilityConstraint(setter, S.getDC(),
+                                               component.getLoc())) {
+        auto where =
+            ExportContext::forFunctionBody(S.getDC(), component.getLoc());
+        return diagnoseDeclAvailability(setter, component.getLoc(),
+                                        /*call=*/nullptr, where);
+      }
+    }
+  }
+
+  return false;
+}
+
 bool ContextualFailure::diagnoseConversionToNil() const {
   auto anchor = getAnchor();
 
@@ -7462,6 +7509,9 @@ bool ArgumentMismatchFailure::diagnoseAsError() {
     return false;
   }
 
+  if (diagnoseKeyPathLiteralMutabilityMismatch())
+    return true;
+
   if (diagnoseMisplacedMissingArgument())
     return true;
 

lib/Sema/CSDiagnostics.h

@@ -690,6 +690,12 @@ class ContextualFailure : public FailureDiagnostic {
 
   bool diagnoseAsNote() override;
 
+  /// If the type of a key path literal is read-only due to setter
+  /// availability constraints but the context requires a writable
+  /// key path, let's produce a tailed availability diagnostic that
+  /// points to the offending setter.
+  bool diagnoseKeyPathLiteralMutabilityMismatch() const;
+
   /// If we're trying to convert something to `nil`.
   bool diagnoseConversionToNil() const;
 

lib/Sema/CSSimplify.cpp

@@ -14920,7 +14920,31 @@ ConstraintSystem::simplifyRestrictedConstraintImpl(
       }
 
       assert(fix);
-      return !recordFix(fix, impact);
+
+      if (recordFix(fix, impact))
+        return false;
+
+      // If we are trying to check whether "from" key path is a subclass of
+      // of a "to" key path, the problem here is either mutablity or erasure
+      // and we should still try to match their root and value types top help
+      // inference.
+      if (restriction == ConversionRestrictionKind::Superclass) {
+        if (fromType->isKnownKeyPathType() && toType->isKnownKeyPathType() &&
+            !fromType->isAnyKeyPath()) {
+          auto fromKeyPath = fromType->castTo<BoundGenericType>();
+          auto toKeyPath = toType->castTo<BoundGenericType>();
+
+          auto flags = subflags;
+          flags |= TMF_ApplyingFix;
+          flags |= TMF_MatchingGenericArguments;
+
+          (void)matchDeepTypeArguments(*this, flags,
+                                       fromKeyPath->getGenericArgs(),
+                                       toKeyPath->getGenericArgs(), locator);
+        }
+      }
+
+      return true;
     }
 
     return false;

test/Availability/availability_accessors.swift

@@ -63,14 +63,14 @@ struct BaseStruct<T> {
   var unavailableSetter: T {
     get { fatalError() }
     @available(*, unavailable)
-    set { fatalError() } // expected-note 33 {{setter for 'unavailableSetter' has been explicitly marked unavailable here}}
+    set { fatalError() } // expected-note 34 {{setter for 'unavailableSetter' has been explicitly marked unavailable here}}
   }
 
   var unavailableGetterAndSetter: T {
     @available(*, unavailable)
     get { fatalError() } // expected-note 71 {{getter for 'unavailableGetterAndSetter' has been explicitly marked unavailable here}}
     @available(*, unavailable)
-    set { fatalError() } // expected-note 33 {{setter for 'unavailableGetterAndSetter' has been explicitly marked unavailable here}}
+    set { fatalError() } // expected-note 34 {{setter for 'unavailableGetterAndSetter' has been explicitly marked unavailable here}}
   }
 
   var deprecatedGetter: T {
@@ -579,17 +579,11 @@ func testKeyPathArguments_Struct() {
   takesSendableKeyPath(x, \.deprecatedSetter)
 
   func takesWritableKeyPath<T, U>(_ t: inout T, _ keyPath: WritableKeyPath<T, U>) -> () { }
-  // expected-note@-1 2 {{in call to function 'takesWritableKeyPath'}}
 
   takesWritableKeyPath(&x, \.available)
   takesWritableKeyPath(&x, \.unavailableGetter) // expected-error {{getter for 'unavailableGetter' is unavailable}}
-  // FIXME: Ideally we would diagnose the unavailability of the setter instead
-  // of simply indicating that a conversion to WritableKeyPath is not possible
-  // (rdar://157249275)
-  takesWritableKeyPath(&x, \.unavailableSetter) // expected-error {{cannot convert value of type 'KeyPath<BaseStruct<StructValue>, StructValue>' to expected argument type 'WritableKeyPath<BaseStruct<StructValue>, U>'}}
-  // expected-error@-1 {{generic parameter 'U' could not be inferred}}
-  takesWritableKeyPath(&x, \.unavailableGetterAndSetter) // expected-error {{cannot convert value of type 'KeyPath<BaseStruct<StructValue>, StructValue>' to expected argument type 'WritableKeyPath<BaseStruct<StructValue>, U>'}}
-  // expected-error@-1 {{generic parameter 'U' could not be inferred}}
+  takesWritableKeyPath(&x, \.unavailableSetter) // expected-error {{setter for 'unavailableSetter' is unavailable}}
+  takesWritableKeyPath(&x, \.unavailableGetterAndSetter) // expected-error {{setter for 'unavailableGetterAndSetter' is unavailable}}
   takesWritableKeyPath(&x, \.deprecatedGetter) // expected-warning {{getter for 'deprecatedGetter' is deprecated: reading not recommended}}
   takesWritableKeyPath(&x, \.deprecatedSetter) // expected-warning {{setter for 'deprecatedSetter' is deprecated: writing not recommended}}
 

test/stdlib/KeyPathAppending.swift

@@ -61,7 +61,7 @@ func mismatchedAppends<T, U, V>(readOnlyLeft: KeyPath<T, U>,
   // expected-error@-1 {{no exact matches in call to instance method 'appending'}}
 
   _ = writableRight.appending(path: readOnlyLeft)
-  // expected-error@-1 {{instance method 'appending(path:)' requires that 'KeyPath<U, V>' inherit from 'KeyPath<U, T>'}}
+  // expected-error@-1 {{no exact matches in call to instance method 'appending'}}
 
   _ = writableRight.appending(path: writableLeft)
   // expected-error@-1 {{cannot convert value of type 'WritableKeyPath<T, U>' to expected argument type 'WritableKeyPath<V, U>'}}
@@ -71,7 +71,7 @@ func mismatchedAppends<T, U, V>(readOnlyLeft: KeyPath<T, U>,
   // expected-error@-1 {{no exact matches in call to instance method 'appending'}}
 
   _ = referenceRight.appending(path: readOnlyLeft)
-  // expected-error@-1 {{instance method 'appending(path:)' requires that 'KeyPath<U, V>' inherit from 'KeyPath<U, T>'}}
+  // expected-error@-1 {{no exact matches in call to instance method 'appending'}}
 
   _ = referenceRight.appending(path: writableLeft)
   // expected-error@-1 {{cannot convert value of type 'WritableKeyPath<T, U>' to expected argument type 'WritableKeyPath<V, U>'}}