Skip to content

Commit 2120a31

Browse files
committed
[Diagnostics] Correctly diagnose misplaced missing argument
Due to the fact that `matchCallArgument` can't and doesn't take types into consideration while matching arguments to parameters, when both arguments are un-labeled, it's impossible to say which one is missing: func foo(_: Int, _: String) {} foo("") In this case first argument is missing, but we end up with two fixes - argument mismatch (for #1) and missing argument (for #2), which is incorrect so it has to be handled specially.
1 parent 73b6427 commit 2120a31

File tree

2 files changed

+124
-1
lines changed

2 files changed

+124
-1
lines changed

lib/Sema/CSDiagnostics.cpp

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3565,6 +3565,7 @@ bool ImplicitInitOnNonConstMetatypeFailure::diagnoseAsError() {
35653565
}
35663566

35673567
bool MissingArgumentsFailure::diagnoseAsError() {
3568+
auto &cs = getConstraintSystem();
35683569
auto *locator = getLocator();
35693570
auto path = locator->getPath();
35703571

@@ -3574,6 +3575,11 @@ bool MissingArgumentsFailure::diagnoseAsError() {
35743575
path.back().getKind() == ConstraintLocator::ApplyArgument))
35753576
return false;
35763577

3578+
// If this is a misplaced `missng argument` situation, it would be
3579+
// diagnosed by invalid conversion fix.
3580+
if (isMisplacedMissingArgument(cs, locator))
3581+
return false;
3582+
35773583
auto *anchor = getAnchor();
35783584
if (auto *captureList = dyn_cast<CaptureListExpr>(anchor))
35793585
anchor = captureList->getClosureBody();
@@ -3901,6 +3907,68 @@ bool MissingArgumentsFailure::isPropertyWrapperInitialization() const {
39013907
return NTD && NTD->getAttrs().hasAttribute<PropertyWrapperAttr>();
39023908
}
39033909

3910+
bool MissingArgumentsFailure::isMisplacedMissingArgument(
3911+
ConstraintSystem &cs, ConstraintLocator *locator) {
3912+
auto *calleeLocator = cs.getCalleeLocator(locator);
3913+
auto overloadChoice = cs.findSelectedOverloadFor(calleeLocator);
3914+
if (!overloadChoice)
3915+
return false;
3916+
3917+
auto *fnType =
3918+
cs.simplifyType(overloadChoice->ImpliedType)->getAs<FunctionType>();
3919+
if (!(fnType && fnType->getNumParams() == 2))
3920+
return false;
3921+
3922+
auto *anchor = locator->getAnchor();
3923+
3924+
auto hasFixFor = [&](FixKind kind, ConstraintLocator *locator) -> bool {
3925+
auto fix = llvm::find_if(cs.getFixes(), [&](const ConstraintFix *fix) {
3926+
return fix->getLocator() == locator;
3927+
});
3928+
3929+
if (fix == cs.getFixes().end())
3930+
return false;
3931+
3932+
return (*fix)->getKind() == kind;
3933+
};
3934+
3935+
auto *callLocator =
3936+
cs.getConstraintLocator(anchor, ConstraintLocator::ApplyArgument);
3937+
3938+
auto argFlags = fnType->getParams()[0].getParameterFlags();
3939+
auto *argLoc = cs.getConstraintLocator(
3940+
callLocator, LocatorPathElt::ApplyArgToParam(0, 0, argFlags));
3941+
3942+
if (!(hasFixFor(FixKind::AllowArgumentTypeMismatch, argLoc) &&
3943+
hasFixFor(FixKind::AddMissingArguments, callLocator)))
3944+
return false;
3945+
3946+
Expr *argExpr = nullptr;
3947+
if (auto *call = dyn_cast<CallExpr>(anchor)) {
3948+
argExpr = call->getArg();
3949+
} else if (auto *subscript = dyn_cast<SubscriptExpr>(anchor)) {
3950+
argExpr = subscript->getIndex();
3951+
} else {
3952+
return false;
3953+
}
3954+
3955+
Expr *argument = nullptr;
3956+
if (auto *PE = dyn_cast<ParenExpr>(argExpr)) {
3957+
argument = PE->getSubExpr();
3958+
} else {
3959+
auto *tuple = cast<TupleExpr>(argExpr);
3960+
if (tuple->getNumElements() != 1)
3961+
return false;
3962+
argument = tuple->getElement(0);
3963+
}
3964+
3965+
auto argType = cs.simplifyType(cs.getType(argument));
3966+
auto paramType = fnType->getParams()[1].getPlainType();
3967+
3968+
auto &TC = cs.getTypeChecker();
3969+
return TC.isConvertibleTo(argType, paramType, cs.DC);
3970+
}
3971+
39043972
bool ClosureParamDestructuringFailure::diagnoseAsError() {
39053973
auto *closure = cast<ClosureExpr>(getAnchor());
39063974
auto params = closure->getParameters();
@@ -4845,6 +4913,9 @@ void InOutConversionFailure::fixItChangeArgumentType() const {
48454913
}
48464914

48474915
bool ArgumentMismatchFailure::diagnoseAsError() {
4916+
if (diagnoseMisplacedMissingArgument())
4917+
return true;
4918+
48484919
if (diagnoseConversionToBool())
48494920
return true;
48504921

@@ -5074,6 +5145,33 @@ bool ArgumentMismatchFailure::diagnoseArchetypeMismatch() const {
50745145
return true;
50755146
}
50765147

5148+
bool ArgumentMismatchFailure::diagnoseMisplacedMissingArgument() const {
5149+
auto &cs = getConstraintSystem();
5150+
auto *locator = getLocator();
5151+
5152+
if (!MissingArgumentsFailure::isMisplacedMissingArgument(cs, locator))
5153+
return false;
5154+
5155+
auto info = *getFunctionArgApplyInfo(locator);
5156+
5157+
auto *argType = cs.createTypeVariable(
5158+
cs.getConstraintLocator(locator, LocatorPathElt::SynthesizedArgument(1)),
5159+
/*flags=*/0);
5160+
5161+
// Assign new type variable to a type of a parameter.
5162+
auto *fnType = info.getFnType();
5163+
const auto &param = fnType->getParams()[0];
5164+
cs.assignFixedType(argType, param.getOldType());
5165+
5166+
auto *anchor = getRawAnchor();
5167+
5168+
MissingArgumentsFailure failure(
5169+
getParentExpr(), cs, {param.withType(argType)},
5170+
cs.getConstraintLocator(anchor, ConstraintLocator::ApplyArgument));
5171+
5172+
return failure.diagnoseSingleMissingArgument();
5173+
}
5174+
50775175
void ExpandArrayIntoVarargsFailure::tryDropArrayBracketsFixIt(
50785176
Expr *anchor) const {
50795177
// If this is an array literal, offer to remove the brackets and pass the

lib/Sema/CSDiagnostics.h

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1197,9 +1197,9 @@ class MissingArgumentsFailure final : public FailureDiagnostic {
11971197

11981198
bool diagnoseAsError() override;
11991199

1200-
private:
12011200
bool diagnoseSingleMissingArgument() const;
12021201

1202+
private:
12031203
/// If missing arguments come from a closure,
12041204
/// let's produce tailored diagnostics.
12051205
bool diagnoseClosure(ClosureExpr *closure);
@@ -1212,6 +1212,21 @@ class MissingArgumentsFailure final : public FailureDiagnostic {
12121212
/// an implicit call to a property wrapper initializer e.g.
12131213
/// `@Foo(answer: 42) var question = "ultimate question"`
12141214
bool isPropertyWrapperInitialization() const;
1215+
1216+
public:
1217+
/// Due to the fact that `matchCallArgument` can't and
1218+
/// doesn't take types into consideration while matching
1219+
/// arguments to parameters, for cases where both arguments
1220+
/// are un-labeled, it's impossible to say which one is missing:
1221+
///
1222+
/// func foo(_: Int, _: String) {}
1223+
/// foo("")
1224+
///
1225+
/// In this case first argument is missing, but we end up with
1226+
/// two fixes - argument mismatch (for #1) and missing argument
1227+
/// (for #2), which is incorrect so it has to be handled specially.
1228+
static bool isMisplacedMissingArgument(ConstraintSystem &cs,
1229+
ConstraintLocator *locator);
12151230
};
12161231

12171232
class OutOfOrderArgumentFailure final : public FailureDiagnostic {
@@ -1648,6 +1663,16 @@ class ArgumentMismatchFailure : public ContextualFailure {
16481663
bool diagnoseUseOfReferenceEqualityOperator() const;
16491664

16501665
protected:
1666+
1667+
/// Situations like this:
1668+
///
1669+
/// func foo(_: Int, _: String) {}
1670+
/// foo("")
1671+
///
1672+
/// Are currently impossible to fix correctly,
1673+
/// so we have to attend to that in diagnostics.
1674+
bool diagnoseMisplacedMissingArgument() const;
1675+
16511676
SourceLoc getLoc() const { return getAnchor()->getLoc(); }
16521677

16531678
ValueDecl *getDecl() const {

0 commit comments

Comments
 (0)