Skip to content

[Diagnostics] Add a diagnostic for single parameter tuple splat #26165

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Jul 17, 2019
Merged
9 changes: 6 additions & 3 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -3388,9 +3388,12 @@ ERROR(closure_tuple_parameter_destructuring_implicit,none,
ERROR(nested_tuple_parameter_destructuring,none,
"nested tuple parameter %0 of function %1 "
"does not support destructuring", (Type, Type))
ERROR(single_tuple_parameter_mismatch,none,
"%0 %select{|%1 }3expects a single parameter of type %2",
(DescriptiveDeclKind, Identifier, Type, bool))
ERROR(single_tuple_parameter_mismatch_special,none,
"%0 expects a single parameter of type %1%2",
(DescriptiveDeclKind, Type, StringRef))
ERROR(single_tuple_parameter_mismatch_normal,none,
"%0 %1 expects a single parameter of type %2%3",
(DescriptiveDeclKind, DeclBaseName, Type, StringRef))
ERROR(unknown_single_tuple_parameter_mismatch,none,
"single parameter of type %0 is expected in call", (Type))

Expand Down
114 changes: 0 additions & 114 deletions lib/Sema/CSDiag.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3044,116 +3044,6 @@ bool FailureDiagnosis::diagnoseImplicitSelfErrors(
return false;
}

static bool diagnoseTupleParameterMismatch(CalleeCandidateInfo &CCI,
ArrayRef<AnyFunctionType::Param> params,
ArrayRef<AnyFunctionType::Param> args,
Expr *fnExpr, Expr *argExpr,
bool isTopLevel = true) {
// Try to diagnose function call tuple parameter splat only if
// there is no trailing or argument closure, because
// FailureDiagnosis::visitClosureExpr will produce better
// diagnostic and fix-it for trailing closure case.
if (isTopLevel) {
if (CCI.hasTrailingClosure)
return false;

if (auto *parenExpr = dyn_cast<ParenExpr>(argExpr)) {
if (isa<ClosureExpr>(parenExpr->getSubExpr()))
return false;
}
}

if (params.size() == 1 && args.size() == 1) {
auto paramType = params.front().getOldType();
auto argType = args.front().getOldType();

if (auto *paramFnType = paramType->getAs<AnyFunctionType>()) {
// Only if both of the parameter and argument types are functions
// let's recur into diagnosing their arguments.
if (auto *argFnType = argType->getAs<AnyFunctionType>())
return diagnoseTupleParameterMismatch(CCI, paramFnType->getParams(),
argFnType->getParams(), fnExpr,
argExpr, /* isTopLevel */ false);
return false;
}
}

if (params.size() != 1 || args.empty())
return false;

auto paramType = params.front().getOldType();

if (args.size() == 1) {
auto argType = args.front().getOldType();
if (auto *paramFnType = paramType->getAs<AnyFunctionType>()) {
// Only if both of the parameter and argument types are functions
// let's recur into diagnosing their arguments.
if (auto *argFnType = argType->getAs<AnyFunctionType>())
return diagnoseTupleParameterMismatch(CCI, paramFnType->getParams(),
argFnType->getParams(), fnExpr,
argExpr, /* isTopLevel */ false);
}

return false;
}

// Let's see if inferred argument is actually a tuple inside of Paren.
auto *paramTupleTy = paramType->getAs<TupleType>();
if (!paramTupleTy)
return false;

if (paramTupleTy->getNumElements() != args.size())
return false;

// Looks like the number of tuple elements matches number
// of function arguments, which means we can we can emit an
// error about an attempt to make use of tuple splat or tuple
// destructuring, unfortunately we can't provide a fix-it for
// this case.
auto &TC = CCI.CS.TC;
if (isTopLevel) {
if (auto *decl = CCI[0].getDecl()) {
Identifier name;
auto kind = decl->getDescriptiveKind();
// Constructors/descructors and subscripts don't really have names.
if (!(isa<ConstructorDecl>(decl) || isa<DestructorDecl>(decl) ||
isa<SubscriptDecl>(decl))) {
name = decl->getBaseName().getIdentifier();
}

TC.diagnose(argExpr->getLoc(), diag::single_tuple_parameter_mismatch,
kind, name, paramTupleTy, !name.empty())
.highlight(argExpr->getSourceRange())
.fixItInsertAfter(argExpr->getStartLoc(), "(")
.fixItInsert(argExpr->getEndLoc(), ")");
} else {
TC.diagnose(argExpr->getLoc(),
diag::unknown_single_tuple_parameter_mismatch, paramTupleTy)
.highlight(argExpr->getSourceRange())
.fixItInsertAfter(argExpr->getStartLoc(), "(")
.fixItInsert(argExpr->getEndLoc(), ")");
}
} else {
TC.diagnose(argExpr->getLoc(),
diag::nested_tuple_parameter_destructuring, paramTupleTy,
CCI.CS.getType(fnExpr));
}

return true;
}

static bool diagnoseTupleParameterMismatch(CalleeCandidateInfo &CCI,
ArrayRef<FunctionType::Param> params,
Type argType, Expr *fnExpr,
Expr *argExpr,
bool isTopLevel = true) {
llvm::SmallVector<AnyFunctionType::Param, 4> args;
FunctionType::decomposeInput(argType, args);

return diagnoseTupleParameterMismatch(CCI, params, args, fnExpr, argExpr,
isTopLevel);
}

class ArgumentMatcher : public MatchCallArgumentListener {
TypeChecker &TC;
Expr *FnExpr;
Expand Down Expand Up @@ -3469,10 +3359,6 @@ diagnoseSingleCandidateFailures(CalleeCandidateInfo &CCI, Expr *fnExpr,
}
}

if (diagnoseTupleParameterMismatch(CCI, candidate.getParameters(),
CCI.CS.getType(argExpr), fnExpr, argExpr))
return true;

// We only handle structural errors here.
if (CCI.closeness != CC_ArgumentLabelMismatch &&
CCI.closeness != CC_ArgumentCountMismatch)
Expand Down
59 changes: 59 additions & 0 deletions lib/Sema/CSDiagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3493,3 +3493,62 @@ bool MutatingMemberRefOnImmutableBase::diagnoseAsError() {
diagIDmember);
return failure.diagnoseAsError();
}

bool InvalidTupleSplatWithSingleParameterFailure::diagnoseAsError() {
auto *anchor = getRawAnchor();

auto selectedOverload = getChoiceFor(anchor);
if (!selectedOverload || !selectedOverload->choice.isDecl())
return false;

auto *choice = selectedOverload->choice.getDecl();

auto *argExpr = getArgumentExprFor(anchor);
if (!argExpr)
return false;

using Substitution = std::pair<GenericTypeParamType *, Type>;
llvm::SetVector<Substitution> substitutions;
auto paramTy = ParamType.transform([&](Type type) -> Type {
if (auto *typeVar = type->getAs<TypeVariableType>()) {
type = resolveType(typeVar);

if (auto *GP = typeVar->getImpl().getGenericParameter()) {
substitutions.insert(std::make_pair(GP, type));
return GP;
}
}

return type;
});

DeclBaseName name = choice->getBaseName();

std::string subsStr;
if (!substitutions.empty()) {
subsStr += " [where ";
interleave(
substitutions,
[&subsStr](const Substitution &substitution) {
subsStr += substitution.first->getString();
subsStr += " = ";
subsStr += substitution.second->getString();
},
[&subsStr] { subsStr += ", "; });
subsStr += ']';
}

auto diagnostic =
name.isSpecial()
? emitDiagnostic(argExpr->getLoc(),
diag::single_tuple_parameter_mismatch_special,
choice->getDescriptiveKind(), paramTy, subsStr)
: emitDiagnostic(
argExpr->getLoc(), diag::single_tuple_parameter_mismatch_normal,
choice->getDescriptiveKind(), name, paramTy, subsStr);

diagnostic.highlight(argExpr->getSourceRange())
.fixItInsertAfter(argExpr->getStartLoc(), "(")
.fixItInsert(argExpr->getEndLoc(), ")");
return true;
}
19 changes: 19 additions & 0 deletions lib/Sema/CSDiagnostics.h
Original file line number Diff line number Diff line change
Expand Up @@ -1457,6 +1457,25 @@ class SkipUnhandledConstructInFunctionBuilderFailure final
bool diagnoseAsNote() override;
};

/// Diagnose situation when a single "tuple" parameter is given N arguments e.g.
///
/// ```swift
/// func foo<T>(_ x: (T, Bool)) {}
/// foo(1, false) // foo exptects a single argument of tuple type `(1, false)`
/// ```
class InvalidTupleSplatWithSingleParameterFailure final
: public FailureDiagnostic {
Type ParamType;

public:
InvalidTupleSplatWithSingleParameterFailure(Expr *root, ConstraintSystem &cs,
Type paramTy,
ConstraintLocator *locator)
: FailureDiagnostic(root, cs, locator), ParamType(paramTy) {}

bool diagnoseAsError() override;
};

/// Provides information about the application of a function argument to a
/// parameter.
class FunctionArgApplyInfo {
Expand Down
40 changes: 40 additions & 0 deletions lib/Sema/CSFix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -657,3 +657,43 @@ AllowMutatingMemberOnRValueBase::create(ConstraintSystem &cs, Type baseType,
return new (cs.getAllocator())
AllowMutatingMemberOnRValueBase(cs, baseType, member, name, locator);
}

bool AllowTupleSplatForSingleParameter::diagnose(Expr *root, bool asNote) const {
auto &cs = getConstraintSystem();
InvalidTupleSplatWithSingleParameterFailure failure(root, cs, ParamType,
getLocator());
return failure.diagnose(asNote);
}

bool AllowTupleSplatForSingleParameter::attempt(
ConstraintSystem &cs, SmallVectorImpl<Param> &args, ArrayRef<Param> params,
SmallVectorImpl<SmallVector<unsigned, 1>> &bindings,
ConstraintLocatorBuilder locator) {
if (params.size() != 1 || args.size() <= 1)
return true;

const auto &param = params.front();

auto *paramTy = param.getOldType()->getAs<TupleType>();
if (!paramTy || paramTy->getNumElements() != args.size())
return true;

SmallVector<TupleTypeElt, 4> argElts;
for (const auto &arg : args) {
argElts.push_back(
{arg.getPlainType(), arg.getLabel(), arg.getParameterFlags()});
}

bindings[0].clear();
bindings[0].push_back(0);

auto newArgType = TupleType::get(argElts, cs.getASTContext());

args.clear();
args.push_back(AnyFunctionType::Param(newArgType, param.getLabel()));

auto *fix = new (cs.getAllocator()) AllowTupleSplatForSingleParameter(
cs, paramTy, cs.getConstraintLocator(locator));

return cs.recordFix(fix);
}
31 changes: 31 additions & 0 deletions lib/Sema/CSFix.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ namespace constraints {
class OverloadChoice;
class ConstraintSystem;
class ConstraintLocator;
class ConstraintLocatorBuilder;
class Solution;

/// Describes the kind of fix to apply to the given constraint before
Expand Down Expand Up @@ -183,6 +184,10 @@ enum class FixKind : uint8_t {
/// Allow invalid reference to a member declared as `mutating`
/// when base is an r-value type.
AllowMutatingMemberOnRValueBase,

/// Allow a single tuple parameter to be matched with N arguments
/// by forming all of the given arguments into a single tuple.
AllowTupleSplatForSingleParameter,
};

class ConstraintFix {
Expand Down Expand Up @@ -1181,6 +1186,32 @@ class SkipUnhandledConstructInFunctionBuilder final : public ConstraintFix {
NominalTypeDecl *builder, ConstraintLocator *locator);
};

class AllowTupleSplatForSingleParameter final : public ConstraintFix {
using Param = AnyFunctionType::Param;

Type ParamType;

AllowTupleSplatForSingleParameter(ConstraintSystem &cs, Type paramTy,
ConstraintLocator *locator)
: ConstraintFix(cs, FixKind::AllowTupleSplatForSingleParameter, locator),
ParamType(paramTy) {}

public:
std::string getName() const override {
return "allow single parameter tuple splat";
}

bool diagnose(Expr *root, bool asNote = false) const override;

/// Apply this fix to given arguments/parameters and return `true`
/// this fix is not applicable and solver can't continue, `false`
/// otherwise.
static bool attempt(ConstraintSystem &cs, SmallVectorImpl<Param> &args,
ArrayRef<Param> params,
SmallVectorImpl<SmallVector<unsigned, 1>> &bindings,
ConstraintLocatorBuilder locator);
};

} // end namespace constraints
} // end namespace swift

Expand Down
11 changes: 9 additions & 2 deletions lib/Sema/CSSimplify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -949,8 +949,14 @@ ConstraintSystem::TypeMatchResult constraints::matchCallArguments(
paramInfo,
hasTrailingClosure,
cs.shouldAttemptFixes(), listener,
parameterBindings))
return cs.getTypeMatchFailure(locator);
parameterBindings)) {
if (!cs.shouldAttemptFixes())
return cs.getTypeMatchFailure(locator);

if (AllowTupleSplatForSingleParameter::attempt(cs, argsWithLabels, params,
parameterBindings, locator))
return cs.getTypeMatchFailure(locator);
}

// If this application is part of an operator, then we allow an implicit
// lvalue to be compatible with inout arguments. This is used by
Expand Down Expand Up @@ -6984,6 +6990,7 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
case FixKind::ExplicitlySpecifyGenericArguments:
case FixKind::GenericArgumentsMismatch:
case FixKind::AllowMutatingMemberOnRValueBase:
case FixKind::AllowTupleSplatForSingleParameter:
llvm_unreachable("handled elsewhere");
}

Expand Down
2 changes: 1 addition & 1 deletion lib/Sema/ConstraintSystem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,7 @@ ConstraintLocator *ConstraintSystem::getCalleeLocator(Expr *expr) {
ConstraintLocator::ConstructorMember);
}
// Otherwise fall through and look for locators anchored on the fn expr.
expr = fnExpr;
expr = fnExpr->getSemanticsProvidingExpr();
}

auto *locator = getConstraintLocator(expr);
Expand Down
2 changes: 1 addition & 1 deletion test/Constraints/keyword_arguments.swift
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@ func acceptTuple1<T>(_ x: (T, Bool)) { }

acceptTuple1(produceTuple1())
acceptTuple1((1, false))
acceptTuple1(1, false) // expected-error {{global function 'acceptTuple1' expects a single parameter of type '(T, Bool)'}} {{14-14=(}} {{22-22=)}}
acceptTuple1(1, false) // expected-error {{global function 'acceptTuple1' expects a single parameter of type '(T, Bool)' [where T = Int]}} {{14-14=(}} {{22-22=)}}

func acceptTuple2<T>(_ input : T) -> T { return input }
var tuple1 = (1, "hello")
Expand Down
Loading