[ConstraintSystem] Start to allow diagnosing ambiguity with fixes for solution sets with more than one fix. #28979
Conversation
|
@swift-ci please smoke test |
| auto overload = viable->getOverloadChoice(commonCalleeLocator); | ||
| auto *fn = overload.openedType->getAs<AnyFunctionType>(); | ||
| assert(fn); | ||
| DE.diagnose(overload.choice.getDecl()->getLoc(), |
There was a problem hiding this comment.
Just a question, I wonder if with this we might be able to get rid of the wrong_argument_labels_overload in CSDiag here? I'm asking because I was looking into this just yesterday :))
There was a problem hiding this comment.
This change does cover ambiguity due to argument label mismatches so it's very possible that wrong_argument_labels_overload in CSDiag is obsolete. I'm testing it now!
There was a problem hiding this comment.
Seems to be obsolete - thanks for catching this!
lib/Sema/ConstraintSystem.cpp
Outdated
| return false; | ||
|
|
||
| const auto *fix = fixes.front(); | ||
| auto *locator = fix->getLocator(); | ||
| if (ambiguityKind == AmbiguityKind::CloseMatch && fixes.size() > 1) |
There was a problem hiding this comment.
What do you think about this condition:
if (fixes.size() > 1) {
ambiguityKind = (ambiguityKind == AmbiguityKind::CloseMatch ||
ambiguityKind == AmbiguityKind::ParameterList) &&
llvm::all_of(fixes, [](const ConstraintFix *fix) -> bool {
auto *locator = fix->getLocator();
return locator->findLast<LocatorPathElt::ApplyArgument>();
}) ? AmbiguityKind::ParameterList
: AmbiguityKind::General;
}I think it's okay to check for the common locator as a separate pass to keep this simple since it's not that performance sensitive.
There was a problem hiding this comment.
I do think it's better to determine the ambiguity kind separately, thanks! I'll make this change
hborla
force-pushed
the
ambiguous-call-diagnostics
branch
from
04da66f to
221cdce
Compare
for solution sets with more than one fix.
hborla
force-pushed
the
ambiguous-call-diagnostics
branch
from
221cdce to
9e73bb3
Compare
|
@swift-ci please smoke test |
For now, we only do this for for ambiguous function calls that only have argument mismatches in the solutions. All fixes still must have the same callee locator.