Skip to content

Commit c7ba055

Browse files
committed
[ConstraintSystem] Handle ambiguities caused by requirement failures
Aggregate all requirement failures (regardless of kind) that belong to the same locator and diagnose them as an ambiguity (if there is more than one overload) or as the singular failure if all solutions point to the same overload.
1 parent 9b688e9 commit c7ba055

File tree

2 files changed

+162
-2
lines changed

2 files changed

+162
-2
lines changed

lib/Sema/ConstraintSystem.cpp

Lines changed: 127 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4427,6 +4427,67 @@ static bool diagnoseAmbiguityWithContextualType(
44274427
return true;
44284428
}
44294429

4430+
/// Diagnose problems with generic requirement fixes that are anchored on
4431+
/// one callee location. The list could contain different kinds of fixes
4432+
/// i.e. missing protocol conformances at different positions,
4433+
/// same-type requirement mismatches, etc.
4434+
static bool diagnoseAmbiguityWithGenericRequirements(
4435+
ConstraintSystem &cs,
4436+
ArrayRef<std::pair<const Solution *, const ConstraintFix *>> aggregate) {
4437+
// If all of the fixes point to the same overload choice,
4438+
// we can diagnose this an a single error.
4439+
bool hasNonDeclOverloads = false;
4440+
4441+
llvm::SmallSet<ValueDecl *, 4> overloadChoices;
4442+
for (const auto &entry : aggregate) {
4443+
const auto &solution = *entry.first;
4444+
auto *calleeLocator = solution.getCalleeLocator(entry.second->getLocator());
4445+
4446+
if (auto overload = solution.getOverloadChoiceIfAvailable(calleeLocator)) {
4447+
if (auto *D = overload->choice.getDeclOrNull()) {
4448+
overloadChoices.insert(D);
4449+
} else {
4450+
hasNonDeclOverloads = true;
4451+
}
4452+
}
4453+
}
4454+
4455+
auto &primaryFix = aggregate.front();
4456+
{
4457+
if (overloadChoices.size() > 0) {
4458+
// Some of the choices are non-declaration,
4459+
// let's delegate that to ambiguity diagnostics.
4460+
if (hasNonDeclOverloads)
4461+
return false;
4462+
4463+
if (overloadChoices.size() == 1)
4464+
return primaryFix.second->diagnose(*primaryFix.first);
4465+
4466+
// fall through to the tailored ambiguity diagnostic.
4467+
} else {
4468+
// If there are no overload choices it means that
4469+
// the issue is with types, delegate that to the primary fix.
4470+
return primaryFix.second->diagnoseForAmbiguity(aggregate);
4471+
}
4472+
}
4473+
4474+
// Produce "no exact matches" diagnostic.
4475+
auto &ctx = cs.getASTContext();
4476+
auto *choice = *overloadChoices.begin();
4477+
auto name = choice->getName();
4478+
4479+
ctx.Diags.diagnose(getLoc(primaryFix.second->getLocator()->getAnchor()),
4480+
diag::no_overloads_match_exactly_in_call,
4481+
/*isApplication=*/false, choice->getDescriptiveKind(),
4482+
name.isSpecial(), name.getBaseName());
4483+
4484+
for (const auto &entry : aggregate) {
4485+
entry.second->diagnose(*entry.first, /*asNote=*/true);
4486+
}
4487+
4488+
return true;
4489+
}
4490+
44304491
static bool diagnoseAmbiguity(
44314492
ConstraintSystem &cs, const SolutionDiff::OverloadDiff &ambiguity,
44324493
ArrayRef<std::pair<const Solution *, const ConstraintFix *>> aggregateFix,
@@ -4850,14 +4911,78 @@ bool ConstraintSystem::diagnoseAmbiguityWithFixes(
48504911
// overload choices.
48514912
fixes.set_subtract(consideredFixes);
48524913

4914+
// Aggregate all requirement fixes that belong to the same callee
4915+
// and attempt to diagnose possible ambiguities.
4916+
{
4917+
auto isResultBuilderRef = [&](ASTNode node) {
4918+
auto *UDE = getAsExpr<UnresolvedDotExpr>(node);
4919+
if (!UDE)
4920+
return false;
4921+
4922+
auto &ctx = getASTContext();
4923+
return UDE->isImplicit() &&
4924+
UDE->getName().compare(DeclNameRef(ctx.Id_buildBlock)) == 0;
4925+
};
4926+
4927+
llvm::MapVector<SourceLoc,
4928+
SmallVector<FixInContext, 4>>
4929+
builderFixes;
4930+
4931+
llvm::MapVector<ConstraintLocator *, SmallVector<FixInContext, 4>>
4932+
requirementFixes;
4933+
4934+
// TODO(diagnostics): This approach doesn't work for synthesized code
4935+
// because i.e. result builders would inject a new `buildBlock` or
4936+
// `buildExpression` for every kind of builder. We need to come up
4937+
// with the strategy to unify locators in such cases.
4938+
for (const auto &entry : fixes) {
4939+
auto *fix = entry.second;
4940+
if (!fix->getLocator()->isLastElement<LocatorPathElt::AnyRequirement>())
4941+
continue;
4942+
4943+
auto *calleeLoc = entry.first->getCalleeLocator(fix->getLocator());
4944+
4945+
if (isResultBuilderRef(calleeLoc->getAnchor())) {
4946+
auto *anchor = castToExpr<Expr>(calleeLoc->getAnchor());
4947+
builderFixes[anchor->getLoc()].push_back(entry);
4948+
} else {
4949+
requirementFixes[calleeLoc].push_back(entry);
4950+
}
4951+
}
4952+
4953+
SmallVector<FixInContext, 4> diagnosedFixes;
4954+
4955+
for (auto &entry : builderFixes) {
4956+
auto &aggregate = entry.second;
4957+
4958+
if (diagnoseAmbiguityWithGenericRequirements(*this, aggregate))
4959+
diagnosedFixes.append(aggregate.begin(), aggregate.end());
4960+
}
4961+
4962+
for (auto &entry : requirementFixes) {
4963+
auto &aggregate = entry.second;
4964+
4965+
// Ambiguity only if all of the solutions have a requirement
4966+
// fix at the given location.
4967+
if (aggregate.size() != solutions.size())
4968+
continue;
4969+
4970+
if (diagnoseAmbiguityWithGenericRequirements(*this, aggregate))
4971+
diagnosedFixes.append(aggregate.begin(), aggregate.end());
4972+
}
4973+
4974+
diagnosed |= !diagnosedFixes.empty();
4975+
// Remove any diagnosed fixes.
4976+
fixes.set_subtract(diagnosedFixes);
4977+
}
4978+
48534979
llvm::MapVector<std::pair<FixKind, ConstraintLocator *>,
48544980
SmallVector<FixInContext, 4>>
48554981
fixesByKind;
48564982

48574983
for (const auto &entry : fixes) {
48584984
const auto *fix = entry.second;
4859-
fixesByKind[{fix->getKind(), fix->getLocator()}].push_back(
4860-
{entry.first, fix});
4985+
fixesByKind[{fix->getKind(), fix->getLocator()}].push_back(entry);
48614986
}
48624987

48634988
// If leftover fix is contained in all of the solutions let's

test/Constraints/generics.swift

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -996,3 +996,38 @@ do {
996996
// expected-error@-3 {{local function 'foo' requires the types 'Set<Int>.Type.Element' and 'Array<String>.Type.Element' be equivalent}}
997997
// expected-note@-4 2 {{only concrete types such as structs, enums and classes can conform to protocols}}
998998
}
999+
1000+
// https://github.com/apple/swift/issues/56173
1001+
protocol P_56173 {
1002+
associatedtype Element
1003+
}
1004+
protocol Q_56173 {
1005+
associatedtype Element
1006+
}
1007+
1008+
func test_requirement_failures_in_ambiguous_context() {
1009+
struct A : P_56173 {
1010+
typealias Element = String
1011+
}
1012+
struct B : Q_56173 {
1013+
typealias Element = Int
1014+
}
1015+
1016+
func f1<T: Equatable>(_: T, _: T) {} // expected-note {{where 'T' = 'A'}}
1017+
1018+
f1(A(), B()) // expected-error {{local function 'f1' requires that 'A' conform to 'Equatable'}}
1019+
1020+
func f2<T: P_56173, U: P_56173>(_: T, _: U) {}
1021+
// expected-note@-1 {{candidate requires that 'B' conform to 'P_56173' (requirement specified as 'U' : 'P_56173')}}
1022+
func f2<T: Q_56173, U: Q_56173>(_: T, _: U) {}
1023+
// expected-note@-1 {{candidate requires that 'A' conform to 'Q_56173' (requirement specified as 'T' : 'Q_56173')}}
1024+
1025+
f2(A(), B()) // expected-error {{no exact matches in call to local function 'f2'}}
1026+
1027+
func f3<T: P_56173>(_: T) where T.Element == Int {}
1028+
// expected-note@-1 {{candidate requires that the types 'A.Element' (aka 'String') and 'Int' be equivalent (requirement specified as 'T.Element' == 'Int')}}
1029+
func f3<U: Q_56173>(_: U) where U.Element == String {}
1030+
// expected-note@-1 {{candidate requires that 'A' conform to 'Q_56173' (requirement specified as 'U' : 'Q_56173')}}
1031+
1032+
f3(A()) // expected-error {{no exact matches in call to local function 'f3'}}
1033+
}

0 commit comments

Comments
 (0)