Merge pull request #40047 from xedin/rdar-83056143

[Diagnostics] A tailored diagnostic for Double<->CGFloat conversion via optional chaining

Author: xedin

include/swift/AST/DiagnosticsSema.def

@@ -5952,6 +5952,13 @@ ERROR(cannot_declare_computed_var_in_result_builder,none,
         "expression shuffles the elements of this tuple; "
         "this behavior is deprecated", ())
 
+//------------------------------------------------------------------------------
+// MARK: Implicit conversion diagnostics
+//------------------------------------------------------------------------------
+ERROR(cannot_implicitly_convert_in_optional_context,none,
+      "cannot implicitly convert value of type %0 to expected type %1",
+      (Type, Type))
+
 //------------------------------------------------------------------------------
 // MARK: marker protocol diagnostics
 //------------------------------------------------------------------------------

include/swift/AST/Types.h

@@ -802,7 +802,7 @@ class alignas(1 << TypeAlignInBits) TypeBase
 
   /// Check if this is a CGFloat type from CoreGraphics framework
   /// on macOS or Foundation on Linux.
-  bool isCGFloatType();
+  bool isCGFloat();
 
   /// Check if this is either an Array, Set or Dictionary collection type defined
   /// at the top level of the Swift module

lib/AST/Type.cpp

@@ -813,7 +813,7 @@ bool TypeBase::isStdlibType() {
   return false;
 }
 
-bool TypeBase::isCGFloatType() {
+bool TypeBase::isCGFloat() {
   auto *NTD = getAnyNominal();
   if (!NTD)
     return false;

lib/Sema/CSBindings.cpp

@@ -548,7 +548,7 @@ void BindingSet::addBinding(PotentialBinding binding) {
   if (!TypeVar->getImpl().isClosureParameterType()) {
     auto type = binding.BindingType;
 
-    if (type->isCGFloatType() &&
+    if (type->isCGFloat() &&
         llvm::any_of(Bindings, [](const PotentialBinding &binding) {
           return binding.BindingType->isDouble();
         }))
@@ -557,7 +557,7 @@ void BindingSet::addBinding(PotentialBinding binding) {
     if (type->isDouble()) {
       auto inferredCGFloat =
           llvm::find_if(Bindings, [](const PotentialBinding &binding) {
-            return binding.BindingType->isCGFloatType();
+            return binding.BindingType->isCGFloat();
           });
 
       if (inferredCGFloat != Bindings.end()) {

lib/Sema/CSDiagnostics.cpp

@@ -2418,6 +2418,22 @@ bool ContextualFailure::diagnoseAsError() {
     break;
   }
 
+  case ConstraintLocator::OptionalPayload: {
+    // If this is an attempt at a Double <-> CGFloat conversion
+    // through optional chaining, let's produce a tailored diagnostic.
+    if (isExpr<OptionalEvaluationExpr>(getAnchor())) {
+      if ((fromType->isDouble() || fromType->isCGFloat()) &&
+          (toType->isDouble() || toType->isCGFloat())) {
+        fromType = OptionalType::get(fromType);
+        toType = OptionalType::get(toType);
+        diagnostic = diag::cannot_implicitly_convert_in_optional_context;
+        break;
+      }
+    }
+
+    return false;
+  }
+
   default:
     return false;
   }

lib/Sema/CSGen.cpp

@@ -383,7 +383,7 @@ namespace {
       return true;
 
     // Don't favor narrowing conversions.
-    if (argTy->isDouble() && paramTy->isCGFloatType())
+    if (argTy->isDouble() && paramTy->isCGFloat())
       return false;
 
     llvm::SmallSetVector<ProtocolDecl *, 2> literalProtos;
@@ -420,7 +420,7 @@ namespace {
         // it is the same as the parameter type.
         // Check whether there is a default type to compare against.
         if (paramTy->isEqual(defaultType) ||
-            (defaultType->isDouble() && paramTy->isCGFloatType()))
+            (defaultType->isDouble() && paramTy->isCGFloat()))
           return true;
       }
     }
@@ -560,7 +560,7 @@ namespace {
       // in order to preserve current behavior, let's not favor overloads
       // which would result in conversion from CGFloat to Double; otherwise
       // it would lead to ambiguities.
-      if (argTy->isCGFloatType() && paramTy->isDouble())
+      if (argTy->isCGFloat() && paramTy->isDouble())
         return false;
 
       return isFavoredParamAndArg(CS, paramTy, argTy) &&
@@ -719,10 +719,10 @@ namespace {
       // Avoid favoring overloads that would require narrowing conversion
       // to match the arguments.
       {
-        if (firstArgTy->isDouble() && firstParamTy->isCGFloatType())
+        if (firstArgTy->isDouble() && firstParamTy->isCGFloat())
           return false;
 
-        if (secondArgTy->isDouble() && secondParamTy->isCGFloatType())
+        if (secondArgTy->isDouble() && secondParamTy->isCGFloat())
           return false;
       }
 

lib/Sema/CSSimplify.cpp

@@ -5474,7 +5474,7 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
 
       if (kind >= ConstraintKind::Subtype &&
           nominal1->getDecl() != nominal2->getDecl() &&
-          ((nominal1->isCGFloatType() || nominal2->isCGFloatType()) &&
+          ((nominal1->isCGFloat() || nominal2->isCGFloat()) &&
            (nominal1->isDouble() || nominal2->isDouble()))) {
         ConstraintLocatorBuilder location{locator};
         // Look through all value-to-optional promotions to allow
@@ -5485,6 +5485,19 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
             SmallVector<LocatorPathElt, 4> path;
             auto anchor = location.getLocatorParts(path);
 
+            // An attempt at Double/CGFloat conversion through
+            // optional chaining. This is not supported at the
+            // moment because solution application doesn't know
+            // how to map Double to/from CGFloat through optionals.
+            if (isExpr<OptionalEvaluationExpr>(anchor)) {
+              if (!shouldAttemptFixes())
+                return getTypeMatchFailure(locator);
+
+              conversionsOrFixes.push_back(ContextualMismatch::create(
+                  *this, nominal1, nominal2, getConstraintLocator(locator)));
+              break;
+            }
+
             // Drop all of the applied `value-to-optional` promotions.
             path.erase(llvm::remove_if(
                            path,
@@ -5519,7 +5532,7 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
         auto isCGFloatInit = [&](ASTNode location) {
           if (auto *call = getAsExpr<CallExpr>(location)) {
             if (auto *typeExpr = dyn_cast<TypeExpr>(call->getFn())) {
-              return getInstanceType(typeExpr)->isCGFloatType();
+              return getInstanceType(typeExpr)->isCGFloat();
             }
           }
           return false;
@@ -5548,7 +5561,7 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
               return false;
             })) {
           conversionsOrFixes.push_back(
-              desugar1->isCGFloatType()
+              desugar1->isCGFloat()
                   ? ConversionRestrictionKind::CGFloatToDouble
                   : ConversionRestrictionKind::DoubleToCGFloat);
         }

validation-test/Sema/type_checker_crashers_fixed/rdar83666783.swift

@@ -0,0 +1,16 @@
+// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) %s -typecheck -verify
+// REQUIRES: objc_interop
+
+import Foundation
+
+struct S {
+  var test: CGFloat
+}
+
+func compute(_: Double?) -> Double? {
+  return 42
+}
+
+func test(s: S?) {
+  _ = compute(s?.test) // expected-error {{cannot implicitly convert value of type 'CGFloat?' to expected type 'Double?'}}
+}