Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 28, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 40 additions & 24 deletions lib/Sema/CSOptimizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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());
Expand All @@ -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;
});
});

Expand Down Expand Up @@ -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();
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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);
}

Expand Down
15 changes: 15 additions & 0 deletions test/Constraints/implicit_double_cgfloat_conversion.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
}