Skip to content

Commit

Permalink
[Diagnostics] Correctly diagnose misplaced missing argument
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
xedin committed Sep 24, 2019
1 parent 73b6427 commit 2120a31
Show file tree
Hide file tree
Showing 2 changed files with 124 additions and 1 deletion.
98 changes: 98 additions & 0 deletions lib/Sema/CSDiagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3565,6 +3565,7 @@ bool ImplicitInitOnNonConstMetatypeFailure::diagnoseAsError() {
}

bool MissingArgumentsFailure::diagnoseAsError() {
auto &cs = getConstraintSystem();
auto *locator = getLocator();
auto path = locator->getPath();

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

// If this is a misplaced `missng argument` situation, it would be
// diagnosed by invalid conversion fix.
if (isMisplacedMissingArgument(cs, locator))
return false;

auto *anchor = getAnchor();
if (auto *captureList = dyn_cast<CaptureListExpr>(anchor))
anchor = captureList->getClosureBody();
Expand Down Expand Up @@ -3901,6 +3907,68 @@ bool MissingArgumentsFailure::isPropertyWrapperInitialization() const {
return NTD && NTD->getAttrs().hasAttribute<PropertyWrapperAttr>();
}

bool MissingArgumentsFailure::isMisplacedMissingArgument(
ConstraintSystem &cs, ConstraintLocator *locator) {
auto *calleeLocator = cs.getCalleeLocator(locator);
auto overloadChoice = cs.findSelectedOverloadFor(calleeLocator);
if (!overloadChoice)
return false;

auto *fnType =
cs.simplifyType(overloadChoice->ImpliedType)->getAs<FunctionType>();
if (!(fnType && fnType->getNumParams() == 2))
return false;

auto *anchor = locator->getAnchor();

auto hasFixFor = [&](FixKind kind, ConstraintLocator *locator) -> bool {
auto fix = llvm::find_if(cs.getFixes(), [&](const ConstraintFix *fix) {
return fix->getLocator() == locator;
});

if (fix == cs.getFixes().end())
return false;

return (*fix)->getKind() == kind;
};

auto *callLocator =
cs.getConstraintLocator(anchor, ConstraintLocator::ApplyArgument);

auto argFlags = fnType->getParams()[0].getParameterFlags();
auto *argLoc = cs.getConstraintLocator(
callLocator, LocatorPathElt::ApplyArgToParam(0, 0, argFlags));

if (!(hasFixFor(FixKind::AllowArgumentTypeMismatch, argLoc) &&
hasFixFor(FixKind::AddMissingArguments, callLocator)))
return false;

Expr *argExpr = nullptr;
if (auto *call = dyn_cast<CallExpr>(anchor)) {
argExpr = call->getArg();
} else if (auto *subscript = dyn_cast<SubscriptExpr>(anchor)) {
argExpr = subscript->getIndex();
} else {
return false;
}

Expr *argument = nullptr;
if (auto *PE = dyn_cast<ParenExpr>(argExpr)) {
argument = PE->getSubExpr();
} else {
auto *tuple = cast<TupleExpr>(argExpr);
if (tuple->getNumElements() != 1)
return false;
argument = tuple->getElement(0);
}

auto argType = cs.simplifyType(cs.getType(argument));
auto paramType = fnType->getParams()[1].getPlainType();

auto &TC = cs.getTypeChecker();
return TC.isConvertibleTo(argType, paramType, cs.DC);
}

bool ClosureParamDestructuringFailure::diagnoseAsError() {
auto *closure = cast<ClosureExpr>(getAnchor());
auto params = closure->getParameters();
Expand Down Expand Up @@ -4845,6 +4913,9 @@ void InOutConversionFailure::fixItChangeArgumentType() const {
}

bool ArgumentMismatchFailure::diagnoseAsError() {
if (diagnoseMisplacedMissingArgument())
return true;

if (diagnoseConversionToBool())
return true;

Expand Down Expand Up @@ -5074,6 +5145,33 @@ bool ArgumentMismatchFailure::diagnoseArchetypeMismatch() const {
return true;
}

bool ArgumentMismatchFailure::diagnoseMisplacedMissingArgument() const {
auto &cs = getConstraintSystem();
auto *locator = getLocator();

if (!MissingArgumentsFailure::isMisplacedMissingArgument(cs, locator))
return false;

auto info = *getFunctionArgApplyInfo(locator);

auto *argType = cs.createTypeVariable(
cs.getConstraintLocator(locator, LocatorPathElt::SynthesizedArgument(1)),
/*flags=*/0);

// Assign new type variable to a type of a parameter.
auto *fnType = info.getFnType();
const auto &param = fnType->getParams()[0];
cs.assignFixedType(argType, param.getOldType());

auto *anchor = getRawAnchor();

MissingArgumentsFailure failure(
getParentExpr(), cs, {param.withType(argType)},
cs.getConstraintLocator(anchor, ConstraintLocator::ApplyArgument));

return failure.diagnoseSingleMissingArgument();
}

void ExpandArrayIntoVarargsFailure::tryDropArrayBracketsFixIt(
Expr *anchor) const {
// If this is an array literal, offer to remove the brackets and pass the
Expand Down
27 changes: 26 additions & 1 deletion lib/Sema/CSDiagnostics.h
Original file line number Diff line number Diff line change
Expand Up @@ -1197,9 +1197,9 @@ class MissingArgumentsFailure final : public FailureDiagnostic {

bool diagnoseAsError() override;

private:
bool diagnoseSingleMissingArgument() const;

private:
/// If missing arguments come from a closure,
/// let's produce tailored diagnostics.
bool diagnoseClosure(ClosureExpr *closure);
Expand All @@ -1212,6 +1212,21 @@ class MissingArgumentsFailure final : public FailureDiagnostic {
/// an implicit call to a property wrapper initializer e.g.
/// `@Foo(answer: 42) var question = "ultimate question"`
bool isPropertyWrapperInitialization() const;

public:
/// Due to the fact that `matchCallArgument` can't and
/// doesn't take types into consideration while matching
/// arguments to parameters, for cases where 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.
static bool isMisplacedMissingArgument(ConstraintSystem &cs,
ConstraintLocator *locator);
};

class OutOfOrderArgumentFailure final : public FailureDiagnostic {
Expand Down Expand Up @@ -1648,6 +1663,16 @@ class ArgumentMismatchFailure : public ContextualFailure {
bool diagnoseUseOfReferenceEqualityOperator() const;

protected:

/// Situations like this:
///
/// func foo(_: Int, _: String) {}
/// foo("")
///
/// Are currently impossible to fix correctly,
/// so we have to attend to that in diagnostics.
bool diagnoseMisplacedMissingArgument() const;

SourceLoc getLoc() const { return getAnchor()->getLoc(); }

ValueDecl *getDecl() const {
Expand Down

0 comments on commit 2120a31

Please sign in to comment.