Merge pull request #26577 from xedin/improve-force-downcast-diags

[Diagnostics] Make force downcast fix contextual and move it to `repa…

lib/Sema/CSDiagnostics.cpp

@@ -921,8 +921,9 @@ bool MissingExplicitConversionFailure::diagnoseAsError() {
   if (auto *paren = dyn_cast<ParenExpr>(anchor))
     anchor = paren->getSubExpr();
 
-  auto fromType = getType(anchor)->getRValueType();
-  Type toType = resolveType(ConvertingTo);
+  auto fromType = getFromType();
+  Type toType = getToType();
+
   if (!toType->hasTypeRepr())
     return false;
 

lib/Sema/CSDiagnostics.h

@@ -544,45 +544,6 @@ public:
   bool diagnoseAsError() override;
 };
 
-/// Diagnose failures related attempt to implicitly convert types which
-/// do not support such implicit converstion.
-/// "as" or "as!" has to be specified explicitly in cases like that.
-class MissingExplicitConversionFailure final : public FailureDiagnostic {
-  Type ConvertingTo;
-
-public:
-  MissingExplicitConversionFailure(Expr *expr, ConstraintSystem &cs,
-                                   ConstraintLocator *locator, Type toType)
-      : FailureDiagnostic(expr, cs, locator), ConvertingTo(toType) {}
-
-  bool diagnoseAsError() override;
-
-private:
-  bool exprNeedsParensBeforeAddingAs(Expr *expr) {
-    auto *DC = getDC();
-    auto &TC = getTypeChecker();
-
-    auto asPG = TC.lookupPrecedenceGroup(
-        DC, DC->getASTContext().Id_CastingPrecedence, SourceLoc());
-    if (!asPG)
-      return true;
-    return exprNeedsParensInsideFollowingOperator(TC, DC, expr, asPG);
-  }
-
-  bool exprNeedsParensAfterAddingAs(Expr *expr, Expr *rootExpr) {
-    auto *DC = getDC();
-    auto &TC = getTypeChecker();
-
-    auto asPG = TC.lookupPrecedenceGroup(
-        DC, DC->getASTContext().Id_CastingPrecedence, SourceLoc());
-    if (!asPG)
-      return true;
-
-    return exprNeedsParensOutsideFollowingOperator(TC, DC, expr, rootExpr,
-                                                   asPG);
-  }
-};
-
 /// Diagnose failures related to attempting member access on optional base
 /// type without optional chaining or force-unwrapping it first.
 class MemberAccessOnOptionalBaseFailure final : public FailureDiagnostic {
@@ -739,6 +700,44 @@ private:
   void tryComputedPropertyFixIts(Expr *expr) const;
 };
 
+/// Diagnose failures related attempt to implicitly convert types which
+/// do not support such implicit converstion.
+/// "as" or "as!" has to be specified explicitly in cases like that.
+class MissingExplicitConversionFailure final : public ContextualFailure {
+public:
+  MissingExplicitConversionFailure(Expr *expr, ConstraintSystem &cs,
+                                   Type fromType, Type toType,
+                                   ConstraintLocator *locator)
+      : ContextualFailure(expr, cs, fromType, toType, locator) {}
+
+  bool diagnoseAsError() override;
+
+private:
+  bool exprNeedsParensBeforeAddingAs(Expr *expr) {
+    auto *DC = getDC();
+    auto &TC = getTypeChecker();
+
+    auto asPG = TC.lookupPrecedenceGroup(
+        DC, DC->getASTContext().Id_CastingPrecedence, SourceLoc());
+    if (!asPG)
+      return true;
+    return exprNeedsParensInsideFollowingOperator(TC, DC, expr, asPG);
+  }
+
+  bool exprNeedsParensAfterAddingAs(Expr *expr, Expr *rootExpr) {
+    auto *DC = getDC();
+    auto &TC = getTypeChecker();
+
+    auto asPG = TC.lookupPrecedenceGroup(
+        DC, DC->getASTContext().Id_CastingPrecedence, SourceLoc());
+    if (!asPG)
+      return true;
+
+    return exprNeedsParensOutsideFollowingOperator(TC, DC, expr, rootExpr,
+                                                   asPG);
+  }
+};
+
 /// Diagnose failures related to passing value of some type
 /// to `inout` or pointer parameter, without explicitly specifying `&`.
 class MissingAddressOfFailure final : public ContextualFailure {

lib/Sema/CSFix.cpp

@@ -48,21 +48,24 @@ void ConstraintFix::dump() const {print(llvm::errs()); }
 
 std::string ForceDowncast::getName() const {
   llvm::SmallString<16> name;
-  name += "force downcast (as! ";
-  name += DowncastTo->getString();
+  name += "force downcast (";
+  name += getFromType()->getString();
+  name += " as! ";
+  name += getToType()->getString();
   name += ")";
   return name.c_str();
 }
 
 bool ForceDowncast::diagnose(Expr *expr, bool asNote) const {
-  MissingExplicitConversionFailure failure(expr, getConstraintSystem(),
-                                           getLocator(), DowncastTo);
+  auto &cs = getConstraintSystem();
+  MissingExplicitConversionFailure failure(expr, cs, getFromType(), getToType(),
+                                           getLocator());
   return failure.diagnose(asNote);
 }
 
-ForceDowncast *ForceDowncast::create(ConstraintSystem &cs, Type toType,
-                                     ConstraintLocator *locator) {
-  return new (cs.getAllocator()) ForceDowncast(cs, toType, locator);
+ForceDowncast *ForceDowncast::create(ConstraintSystem &cs, Type fromType,
+                                     Type toType, ConstraintLocator *locator) {
+  return new (cs.getAllocator()) ForceDowncast(cs, fromType, toType, locator);
 }
 
 bool ForceOptional::diagnose(Expr *root, bool asNote) const {

lib/Sema/CSFix.h

@@ -243,22 +243,6 @@ protected:
   ConstraintSystem &getConstraintSystem() const { return CS; }
 };
 
-/// Append 'as! T' to force a downcast to the specified type.
-class ForceDowncast final : public ConstraintFix {
-  Type DowncastTo;
-
-  ForceDowncast(ConstraintSystem &cs, Type toType, ConstraintLocator *locator)
-      : ConstraintFix(cs, FixKind::ForceDowncast, locator), DowncastTo(toType) {
-  }
-
-public:
-  std::string getName() const override;
-  bool diagnose(Expr *root, bool asNote = false) const override;
-
-  static ForceDowncast *create(ConstraintSystem &cs, Type toType,
-                               ConstraintLocator *locator);
-};
-
 /// Introduce a '!' to force an optional unwrap.
 class ForceOptional final : public ConstraintFix {
   Type BaseType;
@@ -510,6 +494,22 @@ public:
                                     ConstraintLocator *locator);
 };
 
+/// Append 'as! T' to force a downcast to the specified type.
+class ForceDowncast final : public ContextualMismatch {
+  ForceDowncast(ConstraintSystem &cs, Type fromType, Type toType,
+                ConstraintLocator *locator)
+      : ContextualMismatch(cs, FixKind::ForceDowncast, fromType, toType,
+                           locator) {}
+
+public:
+  std::string getName() const override;
+
+  bool diagnose(Expr *root, bool asNote = false) const override;
+
+  static ForceDowncast *create(ConstraintSystem &cs, Type fromType, Type toType,
+                               ConstraintLocator *locator);
+};
+
 /// Introduce a '&' to take the address of an lvalue.
 class AddAddressOf final : public ContextualMismatch {
   AddAddressOf(ConstraintSystem &cs, Type argTy, Type paramTy,

lib/Sema/CSSimplify.cpp

@@ -2186,6 +2186,58 @@ static ConstraintFix *fixPropertyWrapperFailure(
   return nullptr;
 }
 
+static bool canBridgeThroughCast(ConstraintSystem &cs, Type fromType,
+                                 Type toType) {
+  // If we have a value of type AnyObject that we're trying to convert to
+  // a class, force a downcast.
+  // FIXME: Also allow types bridged through Objective-C classes.
+  if (fromType->isAnyObject() && toType->getClassOrBoundGenericClass())
+    return true;
+
+  auto &TC = cs.getTypeChecker();
+  auto bridged = TC.getDynamicBridgedThroughObjCClass(cs.DC, fromType, toType);
+  if (!bridged)
+    return false;
+
+  // Note: don't perform this recovery for NSNumber;
+  if (auto classType = bridged->getAs<ClassType>()) {
+    SmallString<16> scratch;
+    if (classType->getDecl()->isObjC() &&
+        classType->getDecl()->getObjCRuntimeName(scratch) == "NSNumber")
+      return false;
+  }
+
+  return true;
+}
+
+static bool
+repairViaBridgingCast(ConstraintSystem &cs, Type fromType, Type toType,
+                      SmallVectorImpl<RestrictionOrFix> &conversionsOrFixes,
+                      ConstraintLocatorBuilder locator) {
+  auto objectType1 = fromType->getOptionalObjectType();
+  auto objectType2 = toType->getOptionalObjectType();
+
+  if (objectType1 && !objectType2) {
+    auto *anchor = locator.trySimplifyToExpr();
+    if (!anchor)
+      return false;
+
+    if (auto *overload = cs.findSelectedOverloadFor(anchor)) {
+      auto *decl = overload->Choice.getDeclOrNull();
+      if (decl &&
+          decl->getAttrs().hasAttribute<ImplicitlyUnwrappedOptionalAttr>())
+        fromType = objectType1;
+    }
+  }
+
+  if (!canBridgeThroughCast(cs, fromType, toType))
+    return false;
+
+  conversionsOrFixes.push_back(ForceDowncast::create(
+      cs, fromType, toType, cs.getConstraintLocator(locator)));
+  return true;
+}
+
 /// Attempt to repair typing failures and record fixes if needed.
 /// \return true if at least some of the failures has been repaired
 /// successfully, which allows type matcher to continue.
@@ -2301,6 +2353,9 @@ bool ConstraintSystem::repairFailures(
 
       if (repairByAnyToAnyObjectCast(lhs, rhs))
         return true;
+
+      if (repairViaBridgingCast(*this, lhs, rhs, conversionsOrFixes, locator))
+        return true;
     }
 
     return false;
@@ -2321,7 +2376,10 @@ bool ConstraintSystem::repairFailures(
   case ConstraintLocator::ApplyArgToParam: {
     auto loc = getConstraintLocator(locator);
     if (repairByInsertingExplicitCall(lhs, rhs))
-      return true;
+      break;
+
+    if (repairViaBridgingCast(*this, lhs, rhs, conversionsOrFixes, locator))
+      break;
 
     if (lhs->getOptionalObjectType() && !rhs->getOptionalObjectType()) {
       conversionsOrFixes.push_back(
@@ -2444,10 +2502,13 @@ bool ConstraintSystem::repairFailures(
     }
 
     if (repairByInsertingExplicitCall(lhs, rhs))
-      return true;
+      break;
 
     if (repairByAnyToAnyObjectCast(lhs, rhs))
-      return true;
+      break;
+
+    if (repairViaBridgingCast(*this, lhs, rhs, conversionsOrFixes, locator))
+      break;
 
     // If both types are key path, the only differences
     // between them are mutability and/or root, value type mismatch.
@@ -3317,32 +3378,6 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
       }
     }
 
-    // If we have a value of type AnyObject that we're trying to convert to
-    // a class, force a downcast.
-    // FIXME: Also allow types bridged through Objective-C classes.
-    if (objectType1->isAnyObject() &&
-        type2->getClassOrBoundGenericClass()) {
-      conversionsOrFixes.push_back(
-          ForceDowncast::create(*this, type2, getConstraintLocator(locator)));
-    }
-
-    // If we could perform a bridging cast, try it.
-    if (auto bridged =
-            TC.getDynamicBridgedThroughObjCClass(DC, objectType1, type2)) {
-      // Note: don't perform this recovery for NSNumber;
-      bool useFix = true;
-      if (auto classType = bridged->getAs<ClassType>()) {
-        SmallString<16> scratch;
-        if (classType->getDecl()->isObjC() &&
-            classType->getDecl()->getObjCRuntimeName(scratch) == "NSNumber")
-          useFix = false;
-      }
-
-      if (useFix)
-        conversionsOrFixes.push_back(
-            ForceDowncast::create(*this, type2, getConstraintLocator(locator)));
-    }
-
     if (!type1->is<LValueType>() && type2->is<InOutType>()) {
       // If we have a concrete type that's an rvalue, "fix" it.
       conversionsOrFixes.push_back(

test/Constraints/bridging.swift

@@ -296,7 +296,7 @@ func rdar20029786(_ ns: NSString?) {
 
   let s3: NSString? = "str" as String? // expected-error {{cannot convert value of type 'String?' to specified type 'NSString?'}}{{39-39= as NSString?}}
 
-  var s4: String = ns ?? "str" // expected-error{{cannot convert value of type 'NSString' to specified type 'String'}}{{31-31= as String}}
+  var s4: String = ns ?? "str" // expected-error{{'NSString' is not implicitly convertible to 'String'; did you mean to use 'as' to explicitly convert?}} {{20-20=(}} {{31-31=) as String}}
   var s5: String = (ns ?? "str") as String // fixed version
 }
 

test/expr/cast/array_iteration.swift

@@ -16,7 +16,7 @@ for view in rootView.subviews as! [View] { // expected-warning{{immutable value
   doFoo()
 }
 
-for view:View in rootView.subviews { // expected-error{{'AnyObject' is not convertible to 'View'}}
+for view:View in rootView.subviews { // expected-error{{cannot convert sequence element type 'AnyObject' to expected type 'View'}}
   doFoo()
 }
 

test/expr/cast/dictionary_bridge.swift

@@ -80,7 +80,9 @@ func testUpcastBridge() {
   dictOB = dictBB as [ObjC: BridgedToObjC]
 
   dictBB = dictBO // expected-error{{cannot assign value of type '[BridgedToObjC : ObjC]' to type '[BridgedToObjC : BridgedToObjC]'}}
+  // expected-note@-1 {{arguments to generic parameter 'Value' ('ObjC' and 'BridgedToObjC') are expected to be equal}}
   dictBB = dictOB // expected-error{{cannot assign value of type '[ObjC : BridgedToObjC]' to type '[BridgedToObjC : BridgedToObjC]'}}
+  // expected-note@-1 {{arguments to generic parameter 'Key' ('ObjC' and 'BridgedToObjC') are expected to be equal}}
 
   dictDO = dictBB // expected-error{{cannot assign value of type '[BridgedToObjC : BridgedToObjC]' to type '[DerivesObjC : ObjC]'}}
   //expected-note@-1 {{arguments to generic parameter 'Key' ('BridgedToObjC' and 'DerivesObjC') are expected to be equal}}

test/expr/cast/set_bridge.swift

@@ -59,6 +59,7 @@ func testUpcastBridge() {
 
   // Upcast object to bridged type
   setB = setO // expected-error{{cannot assign value of type 'Set<ObjC>' to type 'Set<BridgedToObjC>'}}
+  // expected-note@-1 {{arguments to generic parameter 'Element' ('ObjC' and 'BridgedToObjC') are expected to be equal}}
 
   // Failed upcast
   setD = setB // expected-error{{cannot assign value of type 'Set<BridgedToObjC>' to type 'Set<DerivesObjC>'}}