Skip to content

[Sema] Extend callee diagnoses to non-conforming complex args including generics. #1229

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 1 commit into from
Feb 15, 2016
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
3 changes: 3 additions & 0 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,9 @@ ERROR(cannot_convert_argument_value,none,
(Type,Type))
ERROR(cannot_convert_argument_value_protocol,none,
"argument type %0 does not conform to expected type %1", (Type, Type))
ERROR(cannot_convert_partial_argument_value_protocol,none,
"in argument type %0, %1 does not conform to expected type %2", (Type, Type, Type))

ERROR(cannot_convert_argument_value_nil,none,
"nil is not compatible with expected argument type %0", (Type))

Expand Down
103 changes: 69 additions & 34 deletions lib/Sema/CSDiag.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1121,9 +1121,8 @@ CalleeCandidateInfo::evaluateCloseness(DeclContext *dc, Type candArgListType,
continue;

// FIXME: Right now, a "matching" overload is one with a parameter whose
// type is identical to the argument type, or substitutable via
// rudimentary handling of functions with a single archetype in one or
// more parameters.
// type is identical to the argument type, or substitutable via handling
// of functions with a single archetype in one or more parameters.
// We can still do something more sophisticated with this.
// FIXME: Use TC.isConvertibleTo?

Expand Down Expand Up @@ -1151,13 +1150,9 @@ CalleeCandidateInfo::evaluateCloseness(DeclContext *dc, Type candArgListType,
if (nonSubstitutableArgs == 0)
continue;
++nonSubstitutableArgs;
// Fallthrough, this is nonsubstitutable, so mismatches as well.
mismatchesAreNearMisses = false;
} else {
if (nonSubstitutableArgs == 0) {
paramType = matchingArgType;
// Fallthrough as mismatched arg, comparing nearness to archetype
// bound type.
} else if (nonSubstitutableArgs == 1) {
if (nonSubstitutableArgs == 1) {
// If we have only one nonSubstitutableArg so far, then this different
// type might be the one that we should be substituting for instead.
// Note that failureInfo is already set correctly for that case.
Expand All @@ -1167,23 +1162,37 @@ CalleeCandidateInfo::evaluateCloseness(DeclContext *dc, Type candArgListType,
continue;
}
}

// This substitution doesn't match a previous substitution. Set up the nearMiss
// and failureInfo.paramType with the expected substitution inserted.
// (Note that this transform assumes only a single archetype.)
mismatchesAreNearMisses &= argumentMismatchIsNearMiss(substitution, matchingArgType);
paramType = paramType.transform(([&](Type type) -> Type {
if (type->is<SubstitutableType>())
return matchingArgType;
return type;
}));
}
} else {
matchingArgType = substitution;
singleArchetype = archetype;

if (CS->TC.isSubstitutableFor(substitution, archetype, CS->DC)) {
if (CS->TC.isSubstitutableFor(substitution, archetype, CS->DC))
continue;
}

if (auto argOptType = argType->getOptionalObjectType())
mismatchesAreNearMisses &= CS->TC.isSubstitutableFor(argOptType, archetype, CS->DC);
else
mismatchesAreNearMisses = false;
++nonSubstitutableArgs;
}
} else {
// Keep track of whether this argument was a near miss or not.
mismatchesAreNearMisses &= argumentMismatchIsNearMiss(argType, paramType);
}

++mismatchingArgs;

// Keep track of whether this argument was a near miss or not.
mismatchesAreNearMisses &= argumentMismatchIsNearMiss(argType, paramType);


failureInfo.argumentNumber = argNo;
failureInfo.parameterType = paramType;
if (paramType->hasTypeParameter())
Expand All @@ -1196,7 +1205,8 @@ CalleeCandidateInfo::evaluateCloseness(DeclContext *dc, Type candArgListType,

// Check to see if the first argument expects an inout argument, but is not
// an lvalue.
if (candArgs[0].Ty->is<InOutType>() && !actualArgs[0].Ty->isLValueType())
Type firstArg = actualArgs[0].Ty;
if (candArgs[0].Ty->is<InOutType>() && !(firstArg->isLValueType() || firstArg->is<InOutType>()))
return { CC_NonLValueInOut, {}};

// If we have exactly one argument mismatching, classify it specially, so that
Expand Down Expand Up @@ -1588,14 +1598,28 @@ bool CalleeCandidateInfo::diagnoseGenericParameterErrors(Expr *badArgExpr) {

for (unsigned i = 0, c = archetypes.size(); i < c; i++) {
auto archetype = archetypes[i];
auto argType = substitutions[i];
auto substitution = substitutions[i];

// FIXME: Add specific error for not subclass, if the archetype has a superclass?

// Check for optional near miss.
if (auto argOptType = substitution->getOptionalObjectType()) {
if (CS->TC.isSubstitutableFor(argOptType, archetype, CS->DC)) {
CS->TC.diagnose(badArgExpr->getLoc(), diag::missing_unwrap_optional, argType);
foundFailure = true;
continue;
}
}

for (auto proto : archetype->getConformsTo()) {
if (!CS->TC.conformsToProtocol(argType, proto, CS->DC, ConformanceCheckOptions(TR_InExpression))) {
CS->TC.diagnose(badArgExpr->getLoc(), diag::cannot_convert_argument_value_protocol,
argType, proto->getDeclaredType());
if (!CS->TC.conformsToProtocol(substitution, proto, CS->DC, ConformanceCheckOptions(TR_InExpression))) {
if (substitution->isEqual(argType)) {
CS->TC.diagnose(badArgExpr->getLoc(), diag::cannot_convert_argument_value_protocol,
substitution, proto->getDeclaredType());
} else {
CS->TC.diagnose(badArgExpr->getLoc(), diag::cannot_convert_partial_argument_value_protocol,
argType, substitution, proto->getDeclaredType());
}
foundFailure = true;
}
}
Expand Down Expand Up @@ -1647,7 +1671,11 @@ enum TCCFlags {
/// Re-type-check the given subexpression even if the expression has already
/// been checked already. The client is asserting that infinite recursion is
/// not possible because it has relaxed a constraint on the system.
TCC_ForceRecheck = 0x02
TCC_ForceRecheck = 0x02,

/// tell typeCheckExpression that it is ok to produce an ambiguous result,
/// it can just fill in holes with UnresolvedType and we'll deal with it.
TCC_AllowUnresolvedTypeVariables = 0x04
Copy link
Contributor

Choose a reason for hiding this comment

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

I vaguely recall we used to have this and it got taken out. @lattner ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is just plumbing to pass down through a couple layers of calls to set the pre-existing TypeCheckExprFlags::AllowUnresolvedTypeVariables.

};

typedef OptionSet<TCCFlags> TCCOptions;
Expand Down Expand Up @@ -1716,7 +1744,8 @@ class FailureDiagnosis :public ASTVisitor<FailureDiagnosis, /*exprresult*/bool>{

/// Special magic to handle inout exprs and tuples in argument lists.
Expr *typeCheckArgumentChildIndependently(Expr *argExpr, Type argType,
const CalleeCandidateInfo &candidates);
const CalleeCandidateInfo &candidates,
TCCOptions options = TCCOptions());

/// Diagnose common failures due to applications of an argument list to an
/// ApplyExpr or SubscriptExpr.
Expand Down Expand Up @@ -2781,7 +2810,7 @@ typeCheckChildIndependently(Expr *subExpr, Type convertType,
// If there is no contextual type available, tell typeCheckExpression that it
// is ok to produce an ambiguous result, it can just fill in holes with
// UnresolvedType and we'll deal with it.
if (!convertType)
if (!convertType || options.contains(TCC_AllowUnresolvedTypeVariables))
TCEOptions |= TypeCheckExprFlags::AllowUnresolvedTypeVariables;

bool hadError = CS->TC.typeCheckExpression(subExpr, CS->DC, convertType,
Expand Down Expand Up @@ -3169,7 +3198,8 @@ void ConstraintSystem::diagnoseAssignmentFailure(Expr *dest, Type destTy,
/// Special magic to handle inout exprs and tuples in argument lists.
Expr *FailureDiagnosis::
typeCheckArgumentChildIndependently(Expr *argExpr, Type argType,
const CalleeCandidateInfo &candidates) {
const CalleeCandidateInfo &candidates,
TCCOptions options) {
// Grab one of the candidates (if present) and get its input list to help
// identify operators that have implicit inout arguments.
Type exampleInputType;
Expand Down Expand Up @@ -3203,7 +3233,6 @@ typeCheckArgumentChildIndependently(Expr *argExpr, Type argType,
if (!TE) {
// If the argument isn't a tuple, it is some scalar value for a
// single-argument call.
TCCOptions options;
if (exampleInputType && exampleInputType->is<InOutType>())
options |= TCC_AllowLValue;

Expand Down Expand Up @@ -3276,7 +3305,6 @@ typeCheckArgumentChildIndependently(Expr *argExpr, Type argType,
unsigned inArgNo = sources[i];
auto actualType = argTypeTT->getElementType(i);

TCCOptions options;
if (actualType->is<InOutType>())
options |= TCC_AllowLValue;

Expand Down Expand Up @@ -3341,7 +3369,6 @@ typeCheckArgumentChildIndependently(Expr *argExpr, Type argType,
exampleInputTuple = exampleInputType->getAs<TupleType>();

for (unsigned i = 0, e = TE->getNumElements(); i != e; i++) {
TCCOptions options;
if (exampleInputTuple && i < exampleInputTuple->getNumElements() &&
exampleInputTuple->getElementType(i)->is<InOutType>())
options |= TCC_AllowLValue;
Expand Down Expand Up @@ -3761,18 +3788,17 @@ bool FailureDiagnosis::diagnoseParameterErrors(CalleeCandidateInfo &CCI,
badArgExpr = argExpr;
}

// It could be that the argument doesn't conform to an archetype.
if (CCI.diagnoseGenericParameterErrors(badArgExpr))
return true;

// Re-type-check the argument with the expected type of the candidate set.
// This should produce a specific and tailored diagnostic saying that the
// type mismatches with expectations.
Type paramType = CCI.failedArgument.parameterType;
if (!typeCheckChildIndependently(badArgExpr, paramType,
CTP_CallArgument, options))
return true;

// If that fails, it could be that the argument doesn't conform to an
// archetype.
if (CCI.diagnoseGenericParameterErrors(badArgExpr))
return true;
}

return false;
Expand Down Expand Up @@ -4059,8 +4085,9 @@ bool FailureDiagnosis::visitApplyExpr(ApplyExpr *callExpr) {
// Get the expression result of type checking the arguments to the call
// independently, so we have some idea of what we're working with.
//
auto argExpr = typeCheckArgumentChildIndependently(callExpr->getArg(),
argType, calleeInfo);
auto argExpr = typeCheckArgumentChildIndependently(callExpr->getArg(), argType,
calleeInfo,
TCC_AllowUnresolvedTypeVariables);
if (!argExpr)
return true; // already diagnosed.

Expand All @@ -4069,6 +4096,14 @@ bool FailureDiagnosis::visitApplyExpr(ApplyExpr *callExpr) {
if (diagnoseParameterErrors(calleeInfo, callExpr->getFn(), argExpr))
return true;

// Force recheck of the arg expression because we allowed unresolved types
// before, and that turned out not to help, and now we want any diagnoses
// from disallowing them.
argExpr = typeCheckArgumentChildIndependently(callExpr->getArg(), argType,
calleeInfo, TCC_ForceRecheck);
if (!argExpr)
return true; // already diagnosed.

// Diagnose some simple and common errors.
if (calleeInfo.diagnoseSimpleErrors(callExpr->getLoc()))
return true;
Expand Down
10 changes: 9 additions & 1 deletion lib/Sema/TypeCheckConstraints.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1141,6 +1141,14 @@ solveForExpression(Expr *&expr, DeclContext *dc, Type convertType,
auto constraintKind = ConstraintKind::Conversion;
if (cs.getContextualTypePurpose() == CTP_CallArgument)
constraintKind = ConstraintKind::ArgumentConversion;

if (allowFreeTypeVariables == FreeTypeVariableBinding::UnresolvedType) {
convertType = convertType.transform([&](Type type) -> Type {
if (type->is<UnresolvedType>())
return cs.createTypeVariable(cs.getConstraintLocator(expr), 0);
return type;
});
}

cs.addConstraint(constraintKind, expr->getType(), convertType,
cs.getConstraintLocator(expr), /*isFavored*/ true);
Expand Down Expand Up @@ -1318,7 +1326,7 @@ bool TypeChecker::typeCheckExpression(Expr *&expr, DeclContext *dc,
// check for them now. We cannot apply the solution with unresolved TypeVars,
// because they will leak out into arbitrary places in the resultant AST.
if (options.contains(TypeCheckExprFlags::AllowUnresolvedTypeVariables) &&
viable.size() != 1) {
(viable.size() != 1 || (convertType && convertType->hasUnresolvedType()))) {
expr->setType(ErrorType::get(Context));
return false;
}
Expand Down
9 changes: 8 additions & 1 deletion test/Constraints/diagnostics.swift
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,18 @@ for j in i.wibble(a, a) { // expected-error {{type 'A' does not conform to proto
func f6<T:P2>(g: Void -> T) -> (c: Int, i: T) {
return (c: 0, i: g())
}

func f7() -> (c: Int, v: A) {
let g: Void -> A = { return A() }
return f6(g) // expected-error {{cannot convert return expression of type '(c: Int, i: A)' to return type '(c: Int, v: A)'}}
}

func f8<T:P2>(n: T, _ f: (T) -> T) {}
f8(3, f4) // expected-error {{in argument type '(Int) -> Int', 'Int' does not conform to expected type 'P2'}}
typealias Tup = (Int, Double)
func f9(x: Tup) -> Tup { return x }
f8((1,2.0), f9) // expected-error {{in argument type '(Tup) -> Tup', 'Tup' (aka '(Int, Double)') does not conform to expected type 'P2'}}

// <rdar://problem/19658691> QoI: Incorrect diagnostic for calling nonexistent members on literals
1.doesntExist(0) // expected-error {{value of type 'Int' has no member 'doesntExist'}}
[1, 2, 3].doesntExist(0) // expected-error {{value of type '[Int]' has no member 'doesntExist'}}
Expand Down Expand Up @@ -643,7 +650,7 @@ let a = safeAssign // expected-error {{generic parameter 'T' could not be inferr

// <rdar://problem/21692808> QoI: Incorrect 'add ()' fixit with trailing closure
func foo() -> [Int] {
return Array <Int> (count: 1) { // expected-error {{cannot invoke initializer for type 'Array<Int>' with an argument list of type '(count: Int, () -> _)'}}
return Array <Int> (count: 1) { // expected-error {{cannot invoke initializer for type 'Array<Int>' with an argument list of type '(count: Int, () -> Int)'}}
// expected-note @-1 {{expected an argument list of type '(count: Int, repeatedValue: Element)'}}
return 1
}
Expand Down
6 changes: 3 additions & 3 deletions test/Generics/deduction.swift
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func useSwap(xi: Int, yi: Float) {
mySwap(x, x) // expected-error {{passing value of type 'Int' to an inout parameter requires explicit '&'}} {{10-10=&}}
// expected-error @-1 {{passing value of type 'Int' to an inout parameter requires explicit '&'}} {{13-13=&}}

mySwap(&x, &y) // expected-error{{cannot convert value of type 'inout Int' to expected argument type 'inout _'}}
mySwap(&x, &y) // expected-error{{cannot convert value of type 'Float' to expected argument type 'Int'}}
}

func takeTuples<T, U>(_: (T, U), _: (U, T)) {
Expand Down Expand Up @@ -208,14 +208,14 @@ func callMin(x: Int, y: Int, a: Float, b: Float) {
min2(a, b) // expected-error{{argument type 'Float' does not conform to expected type 'IsBefore'}}
}

func rangeOfIsBefore< // expected-note {{in call to function 'rangeOfIsBefore'}}
func rangeOfIsBefore<
R : GeneratorType where R.Element : IsBefore
>(range : R) { }


func callRangeOfIsBefore(ia: [Int], da: [Double]) {
rangeOfIsBefore(ia.generate())
rangeOfIsBefore(da.generate()) // expected-error{{generic parameter 'R' could not be inferred}}
rangeOfIsBefore(da.generate()) // expected-error{{ambiguous reference to member 'generate()'}}
}

//===----------------------------------------------------------------------===//
Expand Down
4 changes: 2 additions & 2 deletions validation-test/stdlib/StdlibUnittestStaticAssertions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@ struct S2 {}
func test_expectType() {
var s1 = S1()
expectType(S1.self, &s1)
expectType(S2.self, &s1) // expected-error {{cannot convert value of type 'inout S1' to expected argument type 'inout _'}}
expectType(S2.self, &s1) // expected-error {{cannot convert value of type 'S1' to expected argument type 'S2'}}
}

func test_expectEqualType() {
expectEqualType(S1.self, S1.self)
expectEqualType(S1.self, S2.self) // expected-error {{cannot convert value of type 'S2.Type' to expected argument type 'S1'}}
expectEqualType(S1.self, S2.self) // expected-error {{cannot convert value of type 'S2.Type' to expected argument type 'S1.Type'}}
}