Merge pull request swiftlang#78949 from xedin/rdar-143475850-part-2

[CSOptimizer] Don't consider CGFloat widening when explicit initializ…

Author: xedin

lib/Sema/CSOptimizer.cpp

@@ -370,22 +370,36 @@ static void determineBestChoicesInContext(
       FunctionType::relabelParams(argsWithLabels, argumentList);
     }
 
-    SmallVector<SmallVector<std::pair<Type, /*fromLiteral=*/bool>, 2>, 2>
-        candidateArgumentTypes;
-    candidateArgumentTypes.resize(argFuncType->getNumParams());
+    struct ArgumentCandidate {
+      Type type;
+      // The candidate type is derived from a literal expression.
+      bool fromLiteral : 1;
+      // The candidate type is derived from a call to an
+      // initializer i.e. `Double(...)`.
+      bool fromInitializerCall : 1;
+
+      ArgumentCandidate(Type type, bool fromLiteral = false,
+                        bool fromInitializerCall = false)
+          : type(type), fromLiteral(fromLiteral),
+            fromInitializerCall(fromInitializerCall) {}
+    };
+
+    SmallVector<SmallVector<ArgumentCandidate, 2>, 2>
+        argumentCandidates;
+    argumentCandidates.resize(argFuncType->getNumParams());
 
     llvm::TinyPtrVector<Type> resultTypes;
 
     for (unsigned i = 0, n = argFuncType->getNumParams(); i != n; ++i) {
       const auto &param = argFuncType->getParams()[i];
       auto argType = cs.simplifyType(param.getPlainType());
 
-      SmallVector<std::pair<Type, bool>, 2> types;
+      SmallVector<ArgumentCandidate, 2> types;
       if (auto *typeVar = argType->getAs<TypeVariableType>()) {
         auto bindingSet = cs.getBindingsFor(typeVar);
 
         for (const auto &binding : bindingSet.Bindings) {
-          types.push_back({binding.BindingType, /*fromLiteral=*/false});
+          types.push_back({binding.BindingType});
         }
 
         for (const auto &literal : bindingSet.Literals) {
@@ -408,15 +422,16 @@ static void determineBestChoicesInContext(
             if (auto *typeExpr = dyn_cast<TypeExpr>(call->getFn())) {
               auto instanceTy = cs.getType(typeExpr)->getMetatypeInstanceType();
               if (instanceTy->isDouble() || instanceTy->isCGFloat())
-                types.push_back({instanceTy, /*fromLiteral=*/false});
+                types.push_back({instanceTy, /*fromLiteral=*/false,
+                                 /*fromInitializerCall=*/true});
             }
           }
         }
       } else {
         types.push_back({argType, /*fromLiteral=*/false});
       }
 
-      candidateArgumentTypes[i].append(types);
+      argumentCandidates[i].append(types);
     }
 
     auto resultType = cs.simplifyType(argFuncType->getResult());
@@ -437,9 +452,9 @@ static void determineBestChoicesInContext(
         argFuncType->getNumParams() > 0 &&
         llvm::none_of(
             indices(argFuncType->getParams()), [&](const unsigned argIdx) {
-              auto &candidates = candidateArgumentTypes[argIdx];
+              auto &candidates = argumentCandidates[argIdx];
               return llvm::any_of(candidates, [&](const auto &candidate) {
-                return !candidate.second;
+                return !candidate.fromLiteral;
               });
             });
 
@@ -846,7 +861,7 @@ static void determineBestChoicesInContext(
             auto argIdx = argIndices.front();
 
             // Looks like there is nothing know about the argument.
-            if (candidateArgumentTypes[argIdx].empty())
+            if (argumentCandidates[argIdx].empty())
               continue;
 
             const auto paramFlags = param.getParameterFlags();
@@ -873,32 +888,25 @@ static void determineBestChoicesInContext(
             // at this parameter position and remove the overload choice
             // from consideration.
             double bestCandidateScore = 0;
-            llvm::BitVector mismatches(candidateArgumentTypes[argIdx].size());
+            llvm::BitVector mismatches(argumentCandidates[argIdx].size());
 
             for (unsigned candidateIdx :
-                 indices(candidateArgumentTypes[argIdx])) {
+                 indices(argumentCandidates[argIdx])) {
               // If one of the candidates matched exactly there is no reason
               // to continue checking.
               if (bestCandidateScore == 1)
                 break;
 
-              Type candidateType;
-              bool isLiteralDefault;
-
-              std::tie(candidateType, isLiteralDefault) =
-                  candidateArgumentTypes[argIdx][candidateIdx];
+              auto candidate = argumentCandidates[argIdx][candidateIdx];
 
               // `inout` parameter accepts only l-value argument.
-              if (paramFlags.isInOut() && !candidateType->is<LValueType>()) {
+              if (paramFlags.isInOut() && !candidate.type->is<LValueType>()) {
                 mismatches.set(candidateIdx);
                 continue;
               }
 
-              // The specifier only matters for `inout` check.
-              candidateType = candidateType->getWithoutSpecifierType();
-
               MatchOptions options(MatchFlag::OnParam);
-              if (isLiteralDefault)
+              if (candidate.fromLiteral)
                 options |= MatchFlag::Literal;
               if (favorExactMatchesOnly)
                 options |= MatchFlag::ExactOnly;
@@ -913,8 +921,16 @@ static void determineBestChoicesInContext(
               if (n == 1 && decl->isOperator())
                 options |= MatchFlag::DisableCGFloatDoubleConversion;
 
+              // Disable implicit CGFloat -> Double widening conversion if
+              // argument is an explicit call to `CGFloat` initializer.
+              if (candidate.type->isCGFloat() &&
+                  candidate.fromInitializerCall)
+                options |= MatchFlag::DisableCGFloatDoubleConversion;
+
+              // The specifier for a candidate only matters for `inout` check.
               auto candidateScore = scoreCandidateMatch(
-                  genericSig, decl, candidateType, paramType, options);
+                  genericSig, decl, candidate.type->getWithoutSpecifierType(),
+                  paramType, options);
 
               if (!candidateScore)
                 continue;
@@ -928,7 +944,7 @@ static void determineBestChoicesInContext(
               // Only established arguments could be considered mismatches,
               // literal default types should be regarded as holes if they
               // didn't match.
-              if (!isLiteralDefault && !candidateType->hasTypeVariable())
+              if (!candidate.fromLiteral && !candidate.type->hasTypeVariable())
                 mismatches.set(candidateIdx);
             }
 

test/Constraints/implicit_double_cgfloat_conversion.swift

@@ -366,3 +366,18 @@ func test_cgfloat_operator_is_attempted_with_literal_arguments(v: CGFloat?) {
   let ratio = v ?? (2.0 / 16.0)
   let _: CGFloat = ratio // Ok
 }
+
+// Make sure that optimizer doesn't favor CGFloat -> Double conversion
+// in presence of CGFloat initializer, otherwise it could lead to ambiguities.
+func test_explicit_cgfloat_use_avoids_ambiguity(v: Int) {
+  func test(_: CGFloat) -> CGFloat { 0 }
+  func test(_: Double) -> Double { 0 }
+
+  func hasCGFloatElement<C: Collection>(_: C) where C.Element == CGFloat {}
+
+  let arr = [test(CGFloat(v))]
+  hasCGFloatElement(arr) // Ok
+
+  var total = 0.0 // This is Double by default
+  total += test(CGFloat(v)) + CGFloat(v) // Ok
+}