Skip to content

[ConstraintSystem] Port function parameter type mismatch diagnostics. #28712

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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
116 changes: 0 additions & 116 deletions lib/Sema/CSDiag.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -248,8 +248,6 @@ class FailureDiagnosis :public ASTVisitor<FailureDiagnosis, /*exprresult*/bool>{
Optional<std::function<bool(ArrayRef<OverloadChoice>)>> callback = None,
bool includeInaccessibleMembers = true);

bool diagnoseTrailingClosureErrors(ApplyExpr *expr);

bool diagnoseSubscriptErrors(SubscriptExpr *SE, bool performingSet);

bool visitExpr(Expr *E);
Expand Down Expand Up @@ -1709,113 +1707,6 @@ static bool diagnoseClosureExplicitParameterMismatch(
return false;
}

bool FailureDiagnosis::diagnoseTrailingClosureErrors(ApplyExpr *callExpr) {
if (!callExpr->hasTrailingClosure())
return false;

auto *fnExpr = callExpr->getFn();
auto *argExpr = callExpr->getArg();

ClosureExpr *closureExpr = nullptr;
if (auto *PE = dyn_cast<ParenExpr>(argExpr)) {
closureExpr = dyn_cast<ClosureExpr>(PE->getSubExpr());
} else {
return false;
}

if (!closureExpr)
return false;

class CallResultListener : public ExprTypeCheckListener {
Type expectedResultType;

public:
explicit CallResultListener(Type resultType)
: expectedResultType(resultType) {}

bool builtConstraints(ConstraintSystem &cs, Expr *expr) override {
if (!expectedResultType)
return false;

auto resultType = cs.getType(expr);
auto *locator = cs.getConstraintLocator(expr);

// Since we know that this is trailing closure, format of the
// type could be like this - ((Input) -> Result) -> ClosureResult
// which we can leverage to create specific conversion for
// result type of the call itself, this might help us gain
// some valuable contextual information.
if (auto *fnType = resultType->getAs<AnyFunctionType>()) {
cs.addConstraint(ConstraintKind::Conversion, fnType->getResult(),
expectedResultType, locator);
} else if (auto *typeVar = resultType->getAs<TypeVariableType>()) {
auto tv = cs.createTypeVariable(cs.getConstraintLocator(expr),
TVO_CanBindToLValue |
TVO_PrefersSubtypeBinding |
TVO_CanBindToNoEscape);

auto extInfo = FunctionType::ExtInfo().withThrows();

FunctionType::Param tvParam(tv);
auto fTy = FunctionType::get({tvParam}, expectedResultType, extInfo);

// Add a conversion constraint between the types.
cs.addConstraint(ConstraintKind::Conversion, typeVar, fTy, locator,
/*isFavored*/ true);
}

return false;
}
};

SmallPtrSet<TypeBase *, 4> possibleTypes;
auto currentType = CS.simplifyType(CS.getType(fnExpr));

// If current type has type variables or unresolved types
// let's try to re-typecheck it to see if we can get some
// more information about what is going on.
if (currentType->hasTypeVariable() || currentType->hasUnresolvedType()) {
auto contextualType = CS.getContextualType();
CallResultListener listener(contextualType);
getPossibleTypesOfExpressionWithoutApplying(
fnExpr, CS.DC, possibleTypes, FreeTypeVariableBinding::UnresolvedType,
&listener);

// Looks like there is there a contextual mismatch
// related to function type, let's try to diagnose it.
if (possibleTypes.empty() && contextualType &&
!contextualType->hasUnresolvedType())
return diagnoseContextualConversionError(callExpr, contextualType,
CS.getContextualTypePurpose());
} else {
possibleTypes.insert(currentType.getPointer());
}

for (Type type : possibleTypes) {
auto *fnType = type->getAs<AnyFunctionType>();
if (!fnType)
continue;

auto params = fnType->getParams();
if (params.size() != 1)
return false;

Type paramType = params.front().getOldType();
if (auto paramFnType = paramType->getAs<AnyFunctionType>()) {
auto closureType = CS.getType(closureExpr);
if (auto *argFnType = closureType->getAs<AnyFunctionType>()) {
auto *params = closureExpr->getParameters();
auto loc = params ? params->getStartLoc() : closureExpr->getStartLoc();
if (diagnoseClosureExplicitParameterMismatch(
CS, loc, argFnType->getParams(), paramFnType->getParams()))
return true;
}
}
}

return false;
}

/// Check if there failure associated with expression is related
/// to given contextual type.
bool FailureDiagnosis::diagnoseCallContextualConversionErrors(
Expand Down Expand Up @@ -1909,13 +1800,6 @@ static bool isViableOverloadSet(const CalleeCandidateInfo &CCI,
}

bool FailureDiagnosis::visitApplyExpr(ApplyExpr *callExpr) {
// If this call involves trailing closure as an argument,
// let's treat it specially, because re-typecheck of the
// either function or arguments might results in diagnosing
// of the unrelated problems due to luck of context.
if (diagnoseTrailingClosureErrors(callExpr))
return true;

if (diagnoseCallContextualConversionErrors(callExpr, CS.getContextualType(),
CS.getContextualTypePurpose()))
return true;
Expand Down
10 changes: 10 additions & 0 deletions lib/Sema/CSDiagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2826,6 +2826,16 @@ bool TupleContextualFailure::diagnoseAsError() {
return true;
}

bool FunctionTypeMismatch::diagnoseAsError() {
auto purpose = getContextualTypePurpose();
auto diagnostic = getDiagnosticFor(purpose, /*forProtocol=*/false);
if (!diagnostic)
return false;

emitDiagnostic(getAnchor()->getLoc(), *diagnostic, getFromType(), getToType());
return true;
}

bool AutoClosureForwardingFailure::diagnoseAsError() {
auto *loc = getLocator();
auto last = loc->castLastElementTo<LocatorPathElt::ApplyArgToParam>();
Expand Down
17 changes: 17 additions & 0 deletions lib/Sema/CSDiagnostics.h
Original file line number Diff line number Diff line change
Expand Up @@ -852,6 +852,23 @@ class TupleContextualFailure final : public ContextualFailure {
}
};

class FunctionTypeMismatch final : public ContextualFailure {
/// Indices of the parameters whose types do not match.
llvm::SmallVector<unsigned, 4> Indices;

public:
FunctionTypeMismatch(ConstraintSystem &cs, ContextualTypePurpose purpose,
Type lhs, Type rhs, llvm::ArrayRef<unsigned> indices,
ConstraintLocator *locator)
: ContextualFailure(cs, purpose, lhs, rhs, locator),
Indices(indices.begin(), indices.end()) {
std::sort(Indices.begin(), Indices.end());
assert(getFromType()->is<AnyFunctionType>() && getToType()->is<AnyFunctionType>());
}

bool diagnoseAsError() override;
};

/// Diagnose situations when @autoclosure argument is passed to @autoclosure
/// parameter directly without calling it first.
class AutoClosureForwardingFailure final : public FailureDiagnostic {
Expand Down
87 changes: 68 additions & 19 deletions lib/Sema/CSFix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,35 @@ ContextualMismatch *ContextualMismatch::create(ConstraintSystem &cs, Type lhs,
return new (cs.getAllocator()) ContextualMismatch(cs, lhs, rhs, locator);
}

/// Computes the contextual type information for a type mismatch of a
/// component in a structural type (tuple or function type).
///
/// \returns A tuple containing the contextual type purpose, the source type,
/// and the contextual type.
static Optional<std::tuple<ContextualTypePurpose, Type, Type>>
getStructuralTypeContext(ConstraintSystem &cs, ConstraintLocator *locator) {
if (auto contextualType = cs.getContextualType()) {
if (auto *anchor = simplifyLocatorToAnchor(locator))
return std::make_tuple(cs.getContextualTypePurpose(),
cs.getType(anchor),
contextualType);
} else if (auto argApplyInfo = cs.getFunctionArgApplyInfo(locator)) {
return std::make_tuple(CTP_CallArgument,
argApplyInfo->getArgType(),
argApplyInfo->getParamType());
} else if (auto *coerceExpr = dyn_cast<CoerceExpr>(locator->getAnchor())) {
return std::make_tuple(CTP_CoerceOperand,
cs.getType(coerceExpr->getSubExpr()),
cs.getType(coerceExpr));
} else if (auto *assignExpr = dyn_cast<AssignExpr>(locator->getAnchor())) {
return std::make_tuple(CTP_AssignSource,
cs.getType(assignExpr->getSrc()),
cs.getType(assignExpr->getDest()));
}

return None;
}

bool AllowTupleTypeMismatch::coalesceAndDiagnose(
ArrayRef<ConstraintFix *> fixes, bool asNote) const {
llvm::SmallVector<unsigned, 4> indices;
Expand All @@ -270,31 +299,16 @@ bool AllowTupleTypeMismatch::coalesceAndDiagnose(

auto &cs = getConstraintSystem();
auto *locator = getLocator();
auto purpose = cs.getContextualTypePurpose();
ContextualTypePurpose purpose;
Type fromType;
Type toType;

if (getFromType()->is<TupleType>() && getToType()->is<TupleType>()) {
purpose = cs.getContextualTypePurpose();
fromType = getFromType();
toType = getToType();
} else if (auto contextualType = cs.getContextualType()) {
auto *tupleExpr = simplifyLocatorToAnchor(locator);
if (!tupleExpr)
return false;
fromType = cs.getType(tupleExpr);
toType = contextualType;
} else if (auto argApplyInfo = cs.getFunctionArgApplyInfo(locator)) {
purpose = CTP_CallArgument;
fromType = argApplyInfo->getArgType();
toType = argApplyInfo->getParamType();
} else if (auto *coerceExpr = dyn_cast<CoerceExpr>(locator->getAnchor())) {
purpose = CTP_CoerceOperand;
fromType = cs.getType(coerceExpr->getSubExpr());
toType = cs.getType(coerceExpr);
} else if (auto *assignExpr = dyn_cast<AssignExpr>(locator->getAnchor())) {
purpose = CTP_AssignSource;
fromType = cs.getType(assignExpr->getSrc());
toType = cs.getType(assignExpr->getDest());
} else if (auto contextualTypeInfo = getStructuralTypeContext(cs, locator)) {
std::tie(purpose, fromType, toType) = *contextualTypeInfo;
} else {
return false;
}
Expand All @@ -315,6 +329,41 @@ AllowTupleTypeMismatch::create(ConstraintSystem &cs, Type lhs, Type rhs,
AllowTupleTypeMismatch(cs, lhs, rhs, locator, index);
}

bool AllowFunctionTypeMismatch::coalesceAndDiagnose(
ArrayRef<ConstraintFix *> fixes, bool asNote) const {
llvm::SmallVector<unsigned, 4> indices{ParamIndex};

for (auto fix : fixes) {
if (auto *fnFix = fix->getAs<AllowFunctionTypeMismatch>())
indices.push_back(fnFix->ParamIndex);
}

auto &cs = getConstraintSystem();
auto *locator = getLocator();
ContextualTypePurpose purpose;
Type fromType;
Type toType;

auto contextualTypeInfo = getStructuralTypeContext(cs, locator);
if (!contextualTypeInfo)
return false;

std::tie(purpose, fromType, toType) = *contextualTypeInfo;
FunctionTypeMismatch failure(cs, purpose, fromType, toType, indices, locator);
return failure.diagnose(asNote);
}

bool AllowFunctionTypeMismatch::diagnose(bool asNote) const {
return coalesceAndDiagnose({}, asNote);
}

AllowFunctionTypeMismatch *
AllowFunctionTypeMismatch::create(ConstraintSystem &cs, Type lhs, Type rhs,
ConstraintLocator *locator, unsigned index) {
return new (cs.getAllocator())
AllowFunctionTypeMismatch(cs, lhs, rhs, locator, index);
}

bool GenericArgumentsMismatch::diagnose(bool asNote) const {
auto &cs = getConstraintSystem();
GenericArgumentsMismatchFailure failure(cs, getFromType(), getToType(),
Expand Down
33 changes: 33 additions & 0 deletions lib/Sema/CSFix.h
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,10 @@ enum class FixKind : uint8_t {
/// types.
AllowTupleTypeMismatch,

/// Allow a function type to be destructured with mismatched parameter types
/// or return type.
AllowFunctionTypeMismatch,

/// Allow an invalid member access on a value of protocol type as if
/// that protocol type were a generic constraint requiring conformance
/// to that protocol.
Expand Down Expand Up @@ -987,6 +991,35 @@ class AllowTupleTypeMismatch final : public ContextualMismatch {
bool diagnose(bool asNote = false) const override;
};

class AllowFunctionTypeMismatch final : public ContextualMismatch {
/// The index of the parameter where the type mismatch occurred.
unsigned ParamIndex;

AllowFunctionTypeMismatch(ConstraintSystem &cs, Type lhs, Type rhs,
ConstraintLocator *locator, unsigned index)
: ContextualMismatch(cs, FixKind::AllowFunctionTypeMismatch, lhs, rhs,
locator), ParamIndex(index) {}

public:
static AllowFunctionTypeMismatch *create(ConstraintSystem &cs, Type lhs,
Type rhs, ConstraintLocator *locator,
unsigned index);

static bool classof(const ConstraintFix *fix) {
return fix->getKind() == FixKind::AllowFunctionTypeMismatch;
}

std::string getName() const override {
return "allow function type mismatch";
}

bool coalesceAndDiagnose(ArrayRef<ConstraintFix *> secondaryFixes,
bool asNote = false) const override;

bool diagnose(bool asNote = false) const override;
};


class AllowMutatingMemberOnRValueBase final : public AllowInvalidMemberRef {
AllowMutatingMemberOnRValueBase(ConstraintSystem &cs, Type baseType,
ValueDecl *member, DeclName name,
Expand Down
20 changes: 14 additions & 6 deletions lib/Sema/CSSimplify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3543,19 +3543,21 @@ bool ConstraintSystem::repairFailures(

// Drop the `tuple element` locator element so that all tuple element
// mismatches within the same tuple type can be coalesced later.
auto index = elt.getAs<LocatorPathElt::TupleElement>()->getIndex();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to double-check - this is an intermediate step until we get ApplyArgToParam generalization, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Much of this is intermediate until we get a generalized ApplyArgToParam locator - especially the getStructuralTypeContext function in CSFix.cpp. That function is filling in the contextual information that we're missing.

That said, this particular line of code is just to keep track of which tuple elements/parameters don't match (it was there before, previously on line 3556, I just moved it up).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good! I just needed to put this comment on one of the lines because otherwise GitHub doesn't support discussion if I just leave it as review comment :)

path.pop_back();
auto *tupleLocator = getConstraintLocator(locator.getAnchor(), path);

// TODO: Add a tailored fix for function parameter type mismatches.
// Let this fail if it's a contextual mismatch with sequence element types,
// as there's a special fix for that.
if (tupleLocator->isLastElement<LocatorPathElt::FunctionArgument>() ||
tupleLocator->isLastElement<LocatorPathElt::SequenceElementType>())
if (tupleLocator->isLastElement<LocatorPathElt::SequenceElementType>())
break;

auto index = elt.getAs<LocatorPathElt::TupleElement>()->getIndex();
auto *fix =
AllowTupleTypeMismatch::create(*this, lhs, rhs, tupleLocator, index);
ConstraintFix *fix;
if (tupleLocator->isLastElement<LocatorPathElt::FunctionArgument>()) {
fix = AllowFunctionTypeMismatch::create(*this, lhs, rhs, tupleLocator, index);
} else {
fix = AllowTupleTypeMismatch::create(*this, lhs, rhs, tupleLocator, index);
}
conversionsOrFixes.push_back(fix);
break;
}
Expand Down Expand Up @@ -8533,6 +8535,12 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
return matchTupleTypes(matchingType, smaller, matchKind, subflags, locator);
}

case FixKind::AllowFunctionTypeMismatch: {
if (recordFix(fix, /*impact=*/5))
return SolutionKind::Error;
return SolutionKind::Solved;
}

case FixKind::TreatEphemeralAsNonEphemeral: {
auto *theFix = static_cast<TreatEphemeralAsNonEphemeral *>(fix);
// If we have a non-ephemeral locator for an ephemeral conversion, make a
Expand Down
2 changes: 1 addition & 1 deletion test/Constraints/trailing_closures_objc.swift
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ func foo(options: [AVMediaSelectionOption]) {

func rdar28004686(a: [IndexPath]) {
_ = a.sorted { (lhs: NSIndexPath, rhs: NSIndexPath) -> Bool in true }
// expected-error@-1 {{'NSIndexPath' is not convertible to 'IndexPath'}}
// expected-error@-1 {{cannot convert value of type '(NSIndexPath, NSIndexPath) -> Bool' to expected argument type '(IndexPath, IndexPath) throws -> Bool'}}
}

class Test: NSObject {
Expand Down
2 changes: 1 addition & 1 deletion test/expr/closure/closures.swift
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func funcdecl5(_ a: Int, _ y: Int) {
func6({a,b in 4.0 }) // expected-error {{cannot convert value of type 'Double' to closure result type 'Int'}}

// TODO: This diagnostic can be improved: rdar://22128205
func6({(a : Float, b) in 4 }) // expected-error {{cannot convert value of type '(Float, _) -> Int' to expected argument type '(Int, Int) -> Int'}}
func6({(a : Float, b) in 4 }) // expected-error {{cannot convert value of type '(Float, Int) -> Int' to expected argument type '(Int, Int) -> Int'}}



Expand Down