Merge pull request #42254 from LucianoPAlmeida/refactor-diag-checked-cast

[Sema] Improvements on typeCheckCheckedCast and checked cast diagnostics

Author: LucianoPAlmeida

include/swift/Sema/CSFix.h

@@ -316,11 +316,19 @@ enum class FixKind : uint8_t {
   /// succeed.
   AllowNoopCheckedCast,
 
+  /// Warn about special runtime case where statically known
+  /// checked cast from existentials to CFType always succeed.
+  AllowNoopExistentialToCFTypeCheckedCast,
+
   /// Allow a runtime checked cast where at compile time the from is
   /// convertible, but runtime does not support such convertions. e.g.
   /// function type casts.
   AllowUnsupportedRuntimeCheckedCast,
 
+  /// Allow a runtime checked cast where it is known at compile time
+  /// always fails.
+  AllowCheckedCastToUnrelated,
+
   /// Allow reference to a static member on a protocol metatype
   /// even though result type of the reference doesn't conform
   /// to an expected protocol.
@@ -2789,6 +2797,31 @@ class AllowNoopCheckedCast final : public CheckedCastContextualMismatchWarning {
   }
 };
 
+class AllowNoopExistentialToCFTypeCheckedCast final
+    : public CheckedCastContextualMismatchWarning {
+  AllowNoopExistentialToCFTypeCheckedCast(ConstraintSystem &cs, Type fromType,
+                                          Type toType, CheckedCastKind kind,
+                                          ConstraintLocator *locator)
+      : CheckedCastContextualMismatchWarning(
+            cs, FixKind::AllowNoopExistentialToCFTypeCheckedCast, fromType,
+            toType, kind, locator) {}
+
+public:
+  std::string getName() const override {
+    return "checked cast from existential to CFType always succeeds";
+  }
+
+  bool diagnose(const Solution &solution, bool asNote = false) const override;
+
+  static AllowNoopExistentialToCFTypeCheckedCast *
+  attempt(ConstraintSystem &cs, Type fromType, Type toType,
+          CheckedCastKind kind, ConstraintLocator *locator);
+
+  static bool classof(ConstraintFix *fix) {
+    return fix->getKind() == FixKind::AllowNoopExistentialToCFTypeCheckedCast;
+  }
+};
+
 class AllowUnsupportedRuntimeCheckedCast final
     : public CheckedCastContextualMismatchWarning {
   AllowUnsupportedRuntimeCheckedCast(ConstraintSystem &cs, Type fromType,
@@ -2816,6 +2849,29 @@ class AllowUnsupportedRuntimeCheckedCast final
   }
 };
 
+class AllowCheckedCastToUnrelated final
+    : public CheckedCastContextualMismatchWarning {
+  AllowCheckedCastToUnrelated(ConstraintSystem &cs, Type fromType, Type toType,
+                              CheckedCastKind kind, ConstraintLocator *locator)
+      : CheckedCastContextualMismatchWarning(
+            cs, FixKind::AllowCheckedCastToUnrelated, fromType, toType, kind,
+            locator) {}
+
+public:
+  std::string getName() const override { return "checked cast always fails"; }
+
+  bool diagnose(const Solution &solution, bool asNote = false) const override;
+
+  static AllowCheckedCastToUnrelated *attempt(ConstraintSystem &cs,
+                                              Type fromType, Type toType,
+                                              CheckedCastKind kind,
+                                              ConstraintLocator *locator);
+
+  static bool classof(ConstraintFix *fix) {
+    return fix->getKind() == FixKind::AllowCheckedCastToUnrelated;
+  }
+};
+
 class AllowInvalidStaticMemberRefOnProtocolMetatype final
     : public ConstraintFix {
   AllowInvalidStaticMemberRefOnProtocolMetatype(ConstraintSystem &cs,

lib/Sema/CSApply.cpp

@@ -3726,12 +3726,8 @@ namespace {
       expr->setCastType(toType);
       cs.setType(castTypeRepr, toType);
 
-      auto castContextKind =
-          SuppressDiagnostics ? CheckedCastContextKind::None
-                              : CheckedCastContextKind::IsExpr;
       auto castKind = TypeChecker::typeCheckCheckedCast(
-          fromType, toType, castContextKind, dc, expr->getLoc(), sub,
-          castTypeRepr->getSourceRange());
+          fromType, toType, CheckedCastContextKind::IsExpr, dc);
 
       switch (castKind) {
       case CheckedCastKind::Unresolved:
@@ -3740,19 +3736,7 @@ namespace {
 
       case CheckedCastKind::Coercion:
       case CheckedCastKind::BridgingCoercion:
-        expr->setCastKind(castKind);
-        break;
       case CheckedCastKind::ValueCast:
-        // Check the cast target is a non-foreign type
-        if (auto cls = toType->getAs<ClassType>()) {
-          if (cls->getDecl()->getForeignClassKind() ==
-                ClassDecl::ForeignKind::CFType) {
-            ctx.Diags.diagnose(expr->getLoc(), diag::isa_is_foreign_check,
-                               toType);
-          }
-        }
-        expr->setCastKind(castKind);
-        break;
       case CheckedCastKind::ArrayDowncast:
       case CheckedCastKind::DictionaryDowncast:
       case CheckedCastKind::SetDowncast:
@@ -4183,17 +4167,16 @@ namespace {
       if (hasForcedOptionalResult(expr))
         toType = toType->getOptionalObjectType();
 
-      auto castContextKind = SuppressDiagnostics || expr->isImplicit()
-                                 ? CheckedCastContextKind::None
-                                 : CheckedCastContextKind::ForcedCast;
-
       const auto castKind = TypeChecker::typeCheckCheckedCast(
-          fromType, toType, castContextKind, dc, expr->getLoc(), sub,
-          castTypeRange);
+          fromType, toType, CheckedCastContextKind::ForcedCast, dc);
       switch (castKind) {
         /// Invalid cast.
       case CheckedCastKind::Unresolved:
-        return nullptr;
+        if (expr->isImplicit())
+          return nullptr;
+
+        expr->setCastKind(CheckedCastKind::ValueCast);
+        break;
       case CheckedCastKind::Coercion:
       case CheckedCastKind::BridgingCoercion: {
         expr->setCastKind(castKind);
@@ -4254,47 +4237,18 @@ namespace {
              "cast requires TypeRepr; implicit casts are superfluous");
 
       // The subexpression is always an rvalue.
-      auto &ctx = cs.getASTContext();
       auto sub = cs.coerceToRValue(expr->getSubExpr());
       expr->setSubExpr(sub);
 
       // Simplify and update the type we're casting to.
       const auto fromType = cs.getType(sub);
       const auto toType = expr->getCastType();
 
-      bool isSubExprLiteral = isa<LiteralExpr>(sub);
-      auto castContextKind =
-          (SuppressDiagnostics || expr->isImplicit() || isSubExprLiteral)
-              ? CheckedCastContextKind::None
-              : CheckedCastContextKind::ConditionalCast;
-
       auto castKind = TypeChecker::typeCheckCheckedCast(
-          fromType, toType, castContextKind, dc, expr->getLoc(), sub,
-          castTypeRepr->getSourceRange());
+          fromType, toType, CheckedCastContextKind::ConditionalCast, dc);
       switch (castKind) {
       // Invalid cast.
       case CheckedCastKind::Unresolved:
-        // FIXME: This literal diagnostics needs to be revisited by a proposal
-        // to unify casting semantics for literals.
-        // https://bugs.swift.org/browse/SR-12093
-        if (isSubExprLiteral) {
-          auto protocol = TypeChecker::getLiteralProtocol(ctx, sub);
-          // Special handle for literals conditional checked cast when they can
-          // be statically coerced to the cast type.
-          if (protocol && TypeChecker::conformsToProtocol(
-                              toType, protocol, dc->getParentModule())) {
-            ctx.Diags
-                .diagnose(expr->getLoc(),
-                          diag::literal_conditional_downcast_to_coercion,
-                          toType);
-          } else {
-            ctx.Diags
-                .diagnose(expr->getLoc(), diag::downcast_to_unrelated, fromType,
-                          toType)
-                .highlight(sub->getSourceRange())
-                .highlight(castTypeRepr->getSourceRange());
-          }
-        }
         expr->setCastKind(CheckedCastKind::ValueCast);
         break;
 
@@ -6162,12 +6116,9 @@ static Expr *buildElementConversion(ExprRewriter &rewriter,
                                     Type destType, bool bridged,
                                     ConstraintLocatorBuilder locator,
                                     Expr *element) {
-  if (bridged &&
-      TypeChecker::typeCheckCheckedCast(srcType, destType,
-                                        CheckedCastContextKind::None,
-                                        rewriter.dc,
-                                        SourceLoc(), nullptr, SourceRange())
-        != CheckedCastKind::Coercion) {
+  if (bridged && TypeChecker::typeCheckCheckedCast(
+                     srcType, destType, CheckedCastContextKind::None,
+                     rewriter.dc) != CheckedCastKind::Coercion) {
     if (auto conversion =
           rewriter.buildObjCBridgeExpr(element, destType, locator))
         return conversion;

lib/Sema/CSDiagnostics.cpp

@@ -3035,10 +3035,8 @@ bool ContextualFailure::tryTypeCoercionFixIt(
     }
   }
 
-  CheckedCastKind Kind =
-      TypeChecker::typeCheckCheckedCast(fromType, toType,
-                                        CheckedCastContextKind::None, getDC(),
-                                        SourceLoc(), nullptr, SourceRange());
+  CheckedCastKind Kind = TypeChecker::typeCheckCheckedCast(
+      fromType, toType, CheckedCastContextKind::None, getDC());
 
   if (Kind != CheckedCastKind::Unresolved) {
     bool canUseAs = Kind == CheckedCastKind::Coercion ||
@@ -7982,7 +7980,7 @@ bool CoercibleOptionalCheckedCastFailure::diagnoseConditionalCastExpr() const {
   return true;
 }
 
-bool NoopCheckedCast::diagnoseIfExpr() const {
+bool NoopCheckedCast::diagnoseIsExpr() const {
   auto *expr = getAsExpr<IsExpr>(CastExpr);
   if (!expr)
     return false;
@@ -8028,7 +8026,7 @@ bool NoopCheckedCast::diagnoseForcedCastExpr() const {
 }
 
 bool NoopCheckedCast::diagnoseAsError() {
-  if (diagnoseIfExpr())
+  if (diagnoseIsExpr())
     return true;
 
   if (diagnoseForcedCastExpr())
@@ -8040,6 +8038,11 @@ bool NoopCheckedCast::diagnoseAsError() {
   llvm_unreachable("Shouldn't reach here");
 }
 
+bool NoopExistentialToCFTypeCheckedCast::diagnoseAsError() {
+  emitDiagnostic(diag::isa_is_foreign_check, getToType());
+  return true;
+}
+
 bool CoercibleOptionalCheckedCastFailure::diagnoseAsError() {
   if (diagnoseIfExpr())
     return true;
@@ -8062,6 +8065,42 @@ bool UnsupportedRuntimeCheckedCastFailure::diagnoseAsError() {
   return true;
 }
 
+bool CheckedCastToUnrelatedFailure::diagnoseAsError() {
+  const auto toType = getToType();
+  auto *sub = CastExpr->getSubExpr()->getSemanticsProvidingExpr();
+  // FIXME: This literal diagnostics needs to be revisited by a proposal
+  // to unify casting semantics for literals.
+  // https://bugs.swift.org/browse/SR-12093
+  auto &ctx = getASTContext();
+  auto *dc = getDC();
+  if (isa<LiteralExpr>(sub)) {
+    auto *protocol = TypeChecker::getLiteralProtocol(ctx, sub);
+    // Special handle for literals conditional checked cast when they can
+    // be statically coerced to the cast type.
+    if (protocol && TypeChecker::conformsToProtocol(toType, protocol,
+                                                    dc->getParentModule())) {
+      emitDiagnostic(diag::literal_conditional_downcast_to_coercion, toType);
+      return true;
+    }
+  }
+
+  emitDiagnostic(diag::downcast_to_unrelated, getFromType(), toType)
+      .highlight(getFromRange())
+      .highlight(getToRange());
+  // If we're referring to a function with a return value (not Void) then
+  // emit a fix-it suggesting to add `()` to call the function
+  if (auto DRE = dyn_cast<DeclRefExpr>(sub)) {
+    if (auto FD = dyn_cast<FuncDecl>(DRE->getDecl())) {
+      if (!FD->getResultInterfaceType()->isVoid()) {
+        emitDiagnostic(diag::downcast_to_unrelated_fixit,
+                       FD->getBaseIdentifier())
+            .fixItInsertAfter(sub->getEndLoc(), "()");
+      }
+    }
+  }
+  return true;
+}
+
 bool InvalidWeakAttributeUse::diagnoseAsError() {
   auto *pattern =
       dyn_cast_or_null<NamedPattern>(getAnchor().dyn_cast<Pattern *>());

lib/Sema/CSDiagnostics.h

@@ -2574,13 +2574,25 @@ class NoopCheckedCast final : public CheckedCastBaseFailure {
   bool diagnoseAsError() override;
 
 private:
-  bool diagnoseIfExpr() const;
+  bool diagnoseIsExpr() const;
 
   bool diagnoseForcedCastExpr() const;
 
   bool diagnoseConditionalCastExpr() const;
 };
 
+/// Warn situations where the compiler can statically know a runtime
+/// checked cast always succeed.
+class NoopExistentialToCFTypeCheckedCast final : public CheckedCastBaseFailure {
+public:
+  NoopExistentialToCFTypeCheckedCast(const Solution &solution, Type fromType,
+                                     Type toType, CheckedCastKind kind,
+                                     ConstraintLocator *locator)
+      : CheckedCastBaseFailure(solution, fromType, toType, kind, locator) {}
+
+  bool diagnoseAsError() override;
+};
+
 /// Warn situations where the compiler can statically know a runtime
 /// check is not supported.
 class UnsupportedRuntimeCheckedCastFailure final
@@ -2594,6 +2606,18 @@ class UnsupportedRuntimeCheckedCastFailure final
   bool diagnoseAsError() override;
 };
 
+/// Emit a warning when compiler can detect that checked cast would fail at
+/// runtime based on statically known types.
+class CheckedCastToUnrelatedFailure final : public CheckedCastBaseFailure {
+public:
+  CheckedCastToUnrelatedFailure(const Solution &solution, Type fromType,
+                                Type toType, CheckedCastKind kind,
+                                ConstraintLocator *locator)
+      : CheckedCastBaseFailure(solution, fromType, toType, kind, locator) {}
+
+  bool diagnoseAsError() override;
+};
+
 /// Diagnose situations when static member reference has invalid result
 /// type which disqualifies it from being used on a protocol metatype base.
 ///