Merge pull request swiftlang#34401 from xedin/implicit-cgfloat-conversion

[DNM][TypeChecker] Implement Double <-> CGFloat implicit conversion

Author: xedin

include/swift/AST/Types.h

@@ -800,7 +800,14 @@ class alignas(1 << TypeAlignInBits) TypeBase {
 
   /// Check if this is a nominal type defined at the top level of the Swift module
   bool isStdlibType();
-  
+
+  /// Check if this is a CGFloat type from CoreGraphics framework
+  /// on macOS or Foundation on Linux.
+  bool isCGFloatType();
+
+  /// Check if this is a Double type from standard library.
+  bool isDoubleType();
+
   /// Check if this is either an Array, Set or Dictionary collection type defined
   /// at the top level of the Swift module
   bool isKnownStdlibCollectionType();

include/swift/Basic/LangOptions.h

@@ -592,6 +592,9 @@ namespace swift {
 
     /// See \ref FrontendOptions.PrintFullConvention
     bool PrintFullConvention = false;
+
+    /// Disallow Double and CGFloat types from being used interchangeably.
+    bool DisableImplicitDoubleCGFloatConversion = false;
   };
 
   /// Options for controlling the behavior of the Clang importer.

include/swift/Option/FrontendOptions.td

@@ -248,6 +248,9 @@ def enable_resilience : Flag<["-"], "enable-resilience">,
 // HIDDEN FLAGS
 let Flags = [FrontendOption, NoDriverOption, HelpHidden] in {
 
+def disable_implicit_double_cgfloat_conversion : Flag<["-"], "disable-implicit-double-cgfloat-conversion">,
+  HelpText<"Disable implicit conversion between Double and CGFloat types">;
+
 def debug_constraints : Flag<["-"], "debug-constraints">,
   HelpText<"Debug the constraint-based type checker">;
 

include/swift/Sema/Constraint.h

@@ -261,6 +261,12 @@ enum class ConversionRestrictionKind {
   /// Implicit conversion from an Objective-C class type to its
   /// toll-free-bridged CF type.
   ObjCTollFreeBridgeToCF,
+  /// Implicit conversion from a value of Double to a value of CGFloat type via
+  /// an implicit CGFloat initializer call.
+  DoubleToCGFloat,
+  /// Implicit conversion from a value of CGFloat type to a value of Double type
+  /// via an implicit Double initializer call passing a CGFloat value.
+  CGFloatToDouble,
 };
 
 /// Specifies whether a given conversion requires the creation of a temporary

include/swift/Sema/ConstraintLocator.h

@@ -44,6 +44,7 @@ class ProtocolConformance;
 namespace constraints {
 
 class ConstraintSystem;
+enum class ConversionRestrictionKind;
 
 /// Locates a given constraint within the expression being
 /// type-checked, which may refer down into subexpressions and parts of
@@ -805,6 +806,26 @@ class LocatorPathElt::PlaceholderType final
   }
 };
 
+class LocatorPathElt::ImplicitConversion final
+    : public StoredIntegerElement<1> {
+public:
+  ImplicitConversion(ConversionRestrictionKind kind)
+      : StoredIntegerElement(ConstraintLocator::ImplicitConversion,
+                             static_cast<unsigned>(kind)) {}
+
+  ConversionRestrictionKind getConversionKind() const {
+    return static_cast<ConversionRestrictionKind>(getValue());
+  }
+
+  bool is(ConversionRestrictionKind kind) const {
+    return getConversionKind() == kind;
+  }
+
+  static bool classof(const LocatorPathElt *elt) {
+    return elt->getKind() == ConstraintLocator::ImplicitConversion;
+  }
+};
+
 namespace details {
   template <typename CustomPathElement>
   class PathElement {

include/swift/Sema/ConstraintLocatorPathElts.def

@@ -201,6 +201,9 @@ CUSTOM_LOCATOR_PATH_ELT(ConformanceRequirement)
 /// A source-specified placeholder.
 CUSTOM_LOCATOR_PATH_ELT(PlaceholderType)
 
+/// The implicit conversion applied at a given location.
+CUSTOM_LOCATOR_PATH_ELT(ImplicitConversion)
+
 #undef LOCATOR_PATH_ELT
 #undef CUSTOM_LOCATOR_PATH_ELT
 #undef SIMPLE_LOCATOR_PATH_ELT

include/swift/Sema/ConstraintSystem.h

@@ -553,8 +553,12 @@ template <typename T = Expr> T *castToExpr(ASTNode node) {
 }
 
 template <typename T = Expr> T *getAsExpr(ASTNode node) {
+  if (node.isNull())
+    return nullptr;
+
   if (auto *E = node.dyn_cast<Expr *>())
     return dyn_cast_or_null<T>(E);
+
   return nullptr;
 }
 
@@ -788,6 +792,10 @@ enum ScoreKind {
   SK_UnresolvedMemberViaOptional,
   /// An implicit force of an implicitly unwrapped optional value.
   SK_ForceUnchecked,
+  /// An implicit conversion from a value of one type (lhs)
+  /// to another type (rhs) via implicit initialization of
+  /// `rhs` type with an argument of `lhs` value.
+  SK_ImplicitValueConversion,
   /// A user-defined conversion.
   SK_UserConversion,
   /// A non-trivial function conversion.
@@ -1169,6 +1177,10 @@ class Solution {
   /// The locators of \c Defaultable constraints whose defaults were used.
   llvm::SmallPtrSet<ConstraintLocator *, 2> DefaultedConstraints;
 
+  /// Implicit value conversions applied for a given locator.
+  SmallVector<std::pair<ConstraintLocator *, ConversionRestrictionKind>, 2>
+      ImplicitValueConversions;
+
   /// The node -> type mappings introduced by this solution.
   llvm::DenseMap<ASTNode, Type> nodeTypes;
 
@@ -1393,6 +1405,10 @@ enum class ConstraintSystemFlags {
   /// calling conventions, say due to Clang attributes such as
   /// `__attribute__((ns_consumed))`.
   UseClangFunctionTypes = 0x80,
+
+  /// Disallow using Double and CGFloat interchangeably by means of
+  /// an implicit value conversion.
+  DisableImplicitDoubleCGFloatConversion = 0x100,
 };
 
 /// Options that affect the constraint system as a whole.
@@ -2202,6 +2218,11 @@ class ConstraintSystem {
   std::vector<std::pair<ConstraintLocator*, TrailingClosureMatching>>
       trailingClosureMatchingChoices;
 
+  /// The set of implicit value conversions performed by the solver on
+  /// a current path to reach a solution.
+  SmallVector<std::pair<ConstraintLocator *, ConversionRestrictionKind>, 2>
+      ImplicitValueConversions;
+
   /// The worklist of "active" constraints that should be revisited
   /// due to a change.
   ConstraintList ActiveConstraints;
@@ -2730,6 +2751,9 @@ class ConstraintSystem {
     /// The length of \c caseLabelItems.
     unsigned numCaseLabelItems;
 
+    /// The length of \c ImplicitValueConversions.
+    unsigned numImplicitValueConversions;
+
     /// The previous score.
     Score PreviousScore;
 
@@ -3996,7 +4020,7 @@ class ConstraintSystem {
   /// \returns \c true if an error was encountered, \c false otherwise.
   bool simplifyAppliedOverloadsImpl(Constraint *disjunction,
                                     TypeVariableType *fnTypeVar,
-                                    const FunctionType *argFnType,
+                                    FunctionType *argFnType,
                                     unsigned numOptionalUnwraps,
                                     ConstraintLocatorBuilder locator);
 
@@ -4021,7 +4045,7 @@ class ConstraintSystem {
   /// call.
   ///
   /// \returns \c true if an error was encountered, \c false otherwise.
-  bool simplifyAppliedOverloads(Type fnType, const FunctionType *argFnType,
+  bool simplifyAppliedOverloads(Type fnType, FunctionType *argFnType,
                                 ConstraintLocatorBuilder locator);
 
   /// Retrieve the type that will be used when matching the given overload.
@@ -5334,11 +5358,12 @@ class DisjunctionChoice {
 
   bool isBeginningOfPartition() const { return IsBeginningOfPartition; }
 
-  // FIXME: Both of the accessors below are required to support
+  // FIXME: All three of the accessors below are required to support
   //        performance optimization hacks in constraint solver.
 
   bool isGenericOperator() const;
   bool isSymmetricOperator() const;
+  bool isUnaryOperator() const;
 
   void print(llvm::raw_ostream &Out, SourceManager *SM) const {
     Out << "disjunction choice ";

lib/AST/Type.cpp

@@ -813,6 +813,28 @@ bool TypeBase::isStdlibType() {
   return false;
 }
 
+bool TypeBase::isCGFloatType() {
+  auto *NTD = getAnyNominal();
+  if (!NTD)
+    return false;
+
+  auto *DC = NTD->getDeclContext();
+  if (!DC->isModuleScopeContext())
+    return false;
+
+  auto *module = DC->getParentModule();
+  // On macOS `CGFloat` is part of a `CoreGraphics` module,
+  // but on Linux it could be found in `Foundation`.
+  return (module->getName().is("CoreGraphics") ||
+          module->getName().is("Foundation")) &&
+         NTD->getName().is("CGFloat");
+}
+
+bool TypeBase::isDoubleType() {
+  auto *NTD = getAnyNominal();
+  return NTD ? NTD->getDecl() == getASTContext().getDoubleDecl() : false;
+}
+
 bool TypeBase::isKnownStdlibCollectionType() {
   if (auto *structType = getAs<BoundGenericStructType>()) {
     auto &ctx = getASTContext();

lib/Frontend/CompilerInvocation.cpp

@@ -830,6 +830,9 @@ static bool ParseTypeCheckerArgs(TypeCheckerOptions &Opts, ArgList &Args,
   Opts.DebugConstraintSolver |= Args.hasArg(OPT_debug_constraints);
   Opts.DebugGenericSignatures |= Args.hasArg(OPT_debug_generic_signatures);
 
+  Opts.DisableImplicitDoubleCGFloatConversion |=
+      Args.hasArg(OPT_disable_implicit_double_cgfloat_conversion);
+
   for (const Arg *A : Args.filtered(OPT_debug_constraints_on_line)) {
     unsigned line;
     if (StringRef(A->getValue()).getAsInteger(/*radix*/ 10, line)) {

lib/Sema/CSApply.cpp

@@ -5743,7 +5743,7 @@ Expr *ExprRewriter::coerceCallArguments(
   // wrapped value placeholder, the first non-defaulted argument must be
   // wrapped in an OpaqueValueExpr.
   bool shouldInjectWrappedValuePlaceholder =
-     target->shouldInjectWrappedValuePlaceholder(apply);
+      target && target->shouldInjectWrappedValuePlaceholder(apply);
 
   auto injectWrappedValuePlaceholder =
       [&](Expr *arg, bool isAutoClosure = false) -> Expr * {
@@ -6749,6 +6749,59 @@ Expr *ExprRewriter::coerceToType(Expr *expr, Type toType,
       return cs.cacheType(new (ctx)
                               ForeignObjectConversionExpr(result, toType));
     }
+
+    case ConversionRestrictionKind::CGFloatToDouble:
+    case ConversionRestrictionKind::DoubleToCGFloat: {
+      auto conversionKind = knownRestriction->second;
+
+      auto *argExpr = locator.trySimplifyToExpr();
+      assert(argExpr);
+
+      // Load the value for conversion.
+      argExpr = cs.coerceToRValue(argExpr);
+
+      auto *implicitInit =
+          CallExpr::createImplicit(ctx, TypeExpr::createImplicit(toType, ctx),
+                                   /*args=*/{argExpr},
+                                   /*argLabels=*/{Identifier()});
+
+      cs.cacheExprTypes(implicitInit->getFn());
+      cs.setType(argExpr, fromType);
+      cs.setType(implicitInit->getArg(),
+                 ParenType::get(cs.getASTContext(), fromType));
+
+      auto *callLocator = cs.getConstraintLocator(
+          implicitInit, LocatorPathElt::ImplicitConversion(conversionKind));
+
+      // HACK: Temporarily push the call expr onto the expr stack to make sure
+      // we don't try to prematurely close an existential when applying the
+      // curried member ref. This can be removed once existential opening is
+      // refactored not to rely on the shape of the AST prior to rewriting.
+      ExprStack.push_back(implicitInit);
+      SWIFT_DEFER { ExprStack.pop_back(); };
+
+      // We need to take information recorded for all conversions of this
+      // kind and move it to a specific location where restriction is applied.
+      {
+        auto *memberLoc = solution.getConstraintLocator(
+            callLocator, {ConstraintLocator::ApplyFunction,
+                          ConstraintLocator::ConstructorMember});
+
+        auto overload = solution.getOverloadChoice(cs.getConstraintLocator(
+            ASTNode(), {LocatorPathElt::ImplicitConversion(conversionKind),
+                        ConstraintLocator::ApplyFunction,
+                        ConstraintLocator::ConstructorMember}));
+
+        solution.overloadChoices.insert({memberLoc, overload});
+        solution.trailingClosureMatchingChoices.insert(
+            {cs.getConstraintLocator(callLocator,
+                                     ConstraintLocator::ApplyArgument),
+             TrailingClosureMatching::Forward});
+      }
+
+      finishApply(implicitInit, toType, callLocator, callLocator);
+      return implicitInit;
+    }
     }
   }
 
@@ -8588,16 +8641,15 @@ ExprWalker::rewriteTarget(SolutionApplicationTarget target) {
     // If we're supposed to convert the expression to some particular type,
     // do so now.
     if (shouldCoerceToContextualType()) {
-      resultExpr = Rewriter.coerceToType(resultExpr,
-                                         solution.simplifyType(convertType),
-                                         cs.getConstraintLocator(expr));
+      resultExpr =
+          Rewriter.coerceToType(resultExpr, solution.simplifyType(convertType),
+                                cs.getConstraintLocator(resultExpr));
     } else if (cs.getType(resultExpr)->hasLValueType() &&
                !target.isDiscardedExpr()) {
       // We referenced an lvalue. Load it.
       resultExpr = Rewriter.coerceToType(
-          resultExpr,
-          cs.getType(resultExpr)->getRValueType(),
-          cs.getConstraintLocator(expr));
+          resultExpr, cs.getType(resultExpr)->getRValueType(),
+          cs.getConstraintLocator(resultExpr));
     }
 
     if (!resultExpr)