Skip to content

Commit 8741fd2

Browse files
committed
[ConstraintSystem] Reinstate favoring more specialized of two generic overloads.
This was originally commited in 3d32e89, and then backed out in c40fd39 due to concern over behavioral changes. This version avoids the optimization when there are any optional types involved in the signatures of the functions. For these cases, compareDeclarations can return the wrong order at the moment. I have another PR in the works that attempts to begin unraveling some of the issues around fixing the overload comparisons. (cherry picked from commit 1a2d7f5)
1 parent 172972d commit 8741fd2

File tree

3 files changed

+61
-1
lines changed

3 files changed

+61
-1
lines changed

lib/Sema/ConstraintSystem.cpp

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1411,6 +1411,64 @@ ConstraintSystem::getTypeOfMemberReference(
14111411
return { openedType, type };
14121412
}
14131413

1414+
// Performance hack: if there are two generic overloads, and one is
1415+
// more specialized than the other, prefer the more-specialized one.
1416+
static void tryOptimizeGenericDisjunction(ConstraintSystem &cs,
1417+
ArrayRef<OverloadChoice> choices,
1418+
OverloadChoice *&favoredChoice) {
1419+
if (favoredChoice || choices.size() != 2)
1420+
return;
1421+
1422+
const auto &choiceA = choices[0];
1423+
const auto &choiceB = choices[1];
1424+
1425+
if (!choiceA.isDecl() || !choiceB.isDecl())
1426+
return;
1427+
1428+
auto isViable = [](ValueDecl *decl) -> bool {
1429+
assert(decl);
1430+
1431+
auto *AFD = dyn_cast<AbstractFunctionDecl>(decl);
1432+
if (!AFD || !AFD->isGeneric())
1433+
return false;
1434+
1435+
auto funcType = AFD->getInterfaceType();
1436+
auto hasAnyOrOptional = funcType.findIf([](Type type) -> bool {
1437+
if (auto objType = type->getOptionalObjectType())
1438+
return true;
1439+
1440+
return type->isAny();
1441+
});
1442+
1443+
// If function declaration references `Any` or `Any?` type
1444+
// let's not attempt it, because it's unclear
1445+
// without solving which overload is going to be better.
1446+
return !hasAnyOrOptional;
1447+
};
1448+
1449+
auto *declA = choiceA.getDecl();
1450+
auto *declB = choiceB.getDecl();
1451+
1452+
if (!isViable(declA) || !isViable(declB))
1453+
return;
1454+
1455+
auto &TC = cs.TC;
1456+
auto *DC = cs.DC;
1457+
1458+
switch (TC.compareDeclarations(DC, declA, declB)) {
1459+
case Comparison::Better:
1460+
favoredChoice = const_cast<OverloadChoice *>(&choiceA);
1461+
break;
1462+
1463+
case Comparison::Worse:
1464+
favoredChoice = const_cast<OverloadChoice *>(&choiceB);
1465+
break;
1466+
1467+
case Comparison::Unordered:
1468+
break;
1469+
}
1470+
}
1471+
14141472
void ConstraintSystem::addOverloadSet(Type boundType,
14151473
ArrayRef<OverloadChoice> choices,
14161474
DeclContext *useDC,
@@ -1425,6 +1483,8 @@ void ConstraintSystem::addOverloadSet(Type boundType,
14251483
return;
14261484
}
14271485

1486+
tryOptimizeGenericDisjunction(*this, choices, favoredChoice);
1487+
14281488
SmallVector<Constraint *, 4> overloads;
14291489

14301490
// As we do for other favored constraints, if a favored overload has been

validation-test/Sema/type_checker_perf/slow/more_specialized_generic_func.swift.gyb renamed to validation-test/Sema/type_checker_perf/fast/more_specialized_generic_func.swift.gyb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %scale-test --invert-result --begin 1 --end 10 --step 1 --select incrementScopeCounter %s --expected-exit-code 0
1+
// RUN: %scale-test --begin 1 --end 10 --step 1 --select incrementScopeCounter %s --expected-exit-code 0
22
// REQUIRES: OS=macosx
33
// REQUIRES: asserts
44

0 commit comments

Comments
 (0)