From 6b30695869743fc6d688dd6b14b9d71161b16c24 Mon Sep 17 00:00:00 2001 From: gregomni Date: Mon, 1 Feb 2016 17:49:21 -0800 Subject: [PATCH] Extend callee diagnosis to complex args including generics, e.g. (Void) -> T Correctly determine callee closeness for func/ops that include generics as part of more complicated parameters, i.e. tuple or closure args containing generics as elements or args/results. Still only handling single archetypes. Also added code to check generic substitutions already made in the callee parameters, which further helps diagnosis. --- include/swift/AST/TypeMatcher.h | 6 + lib/Sema/CSDiag.cpp | 112 +++++++++++++++--- test/Constraints/diagnostics.swift | 10 ++ test/Generics/function_defs.swift | 8 +- test/Misc/misc_diagnostics.swift | 4 +- .../StdlibUnittestStaticAssertions.swift | 3 +- 6 files changed, 115 insertions(+), 28 deletions(-) diff --git a/include/swift/AST/TypeMatcher.h b/include/swift/AST/TypeMatcher.h index 1cf6c940f176d..674dae143c49e 100644 --- a/include/swift/AST/TypeMatcher.h +++ b/include/swift/AST/TypeMatcher.h @@ -230,6 +230,12 @@ class TypeMatcher { /// FIXME: Split this out into cases? bool visitAnyFunctionType(CanAnyFunctionType firstFunc, Type secondType) { if (auto secondFunc = secondType->getAs()) { + // FIXME: Compare throws()? Both existing subclasses would prefer + // to mismatch on (!firstFunc->throws() && secondFunc->throws()), but + // embedding that non-commutativity in this general matcher is icky. + if (firstFunc->isNoEscape() != secondFunc->isNoEscape()) + return mismatch(firstFunc.getPointer(), secondFunc); + return this->visit(firstFunc.getInput(), secondFunc->getInput()) && this->visit(firstFunc.getResult(), secondFunc->getResult()); } diff --git a/lib/Sema/CSDiag.cpp b/lib/Sema/CSDiag.cpp index e0f7c06e37409..e7805518c6ab7 100644 --- a/lib/Sema/CSDiag.cpp +++ b/lib/Sema/CSDiag.cpp @@ -17,6 +17,8 @@ #include "ConstraintSystem.h" #include "llvm/Support/SaveAndRestore.h" #include "swift/AST/ASTWalker.h" +#include "swift/AST/TypeWalker.h" +#include "swift/AST/TypeMatcher.h" using namespace swift; using namespace constraints; @@ -836,7 +838,7 @@ namespace { /// obviously mismatching candidates and compute a "closeness" for the /// resultant set. std::pair - evaluateCloseness(Type candArgListType, ArrayRef actualArgs); + evaluateCloseness(DeclContext *dc, Type candArgListType, ArrayRef actualArgs); void filterList(ArrayRef actualArgs); void filterList(Type actualArgsType) { @@ -980,11 +982,71 @@ static bool argumentMismatchIsNearMiss(Type argType, Type paramType) { return false; } +/// Given a parameter type that may contain generic type params and an actual +/// argument type, decide whether the param and actual arg have the same shape +/// and equal fixed type portions, and return by reference each archetype and +/// the matching portion of the actual arg type where that archetype appears. +static bool findGenericSubstitutions(DeclContext *dc, Type paramType, Type actualArgType, + SmallVector &archetypes, + SmallVector &substitutions) { + class GenericVisitor : public TypeMatcher { + DeclContext *dc; + SmallVector &archetypes; + SmallVector &substitutions; + + public: + GenericVisitor(DeclContext *dc, + SmallVector &archetypes, + SmallVector &substitutions) + : dc(dc), archetypes(archetypes), substitutions(substitutions) {} + + bool mismatch(TypeBase *paramType, TypeBase *argType) { + return paramType->isEqual(argType); + } + + bool mismatch(SubstitutableType *paramType, TypeBase *argType) { + Type type = paramType; + if (dc && type->isTypeParameter()) + type = ArchetypeBuilder::mapTypeIntoContext(dc, paramType); + + if (auto archetype = type->getAs()) { + for (unsigned i = 0, c = archetypes.size(); i < c; i++) { + if (archetypes[i]->isEqual(archetype)) + return substitutions[i]->isEqual(argType); + } + archetypes.push_back(archetype); + substitutions.push_back(argType); + return true; + } + return false; + } + }; + + // If paramType contains any substitutions already, find them and add them + // to our list before matching the two types to find more. + paramType.findIf([&](Type type) -> bool { + if (auto substitution = dyn_cast(type.getPointer())) { + Type original = substitution->getOriginal(); + if (dc && original->isTypeParameter()) + original = ArchetypeBuilder::mapTypeIntoContext(dc, original); + + if (auto archetype = original->getAs()) { + archetypes.push_back(archetype); + substitutions.push_back(substitution->getReplacementType()); + } + } + return false; + }); + + GenericVisitor visitor(dc, archetypes, substitutions); + return visitor.match(paramType, actualArgType); +} + /// Determine how close an argument list is to an already decomposed argument /// list. If the closeness is a miss by a single argument, then this returns /// information about that failure. std::pair -CalleeCandidateInfo::evaluateCloseness(Type candArgListType, +CalleeCandidateInfo::evaluateCloseness(DeclContext *dc, Type candArgListType, ArrayRef actualArgs) { auto candArgs = decomposeArgParamType(candArgListType); @@ -1064,16 +1126,28 @@ CalleeCandidateInfo::evaluateCloseness(Type candArgListType, // more parameters. // We can still do something more sophisticated with this. // FIXME: Use TC.isConvertibleTo? - if (rArgType->isEqual(paramType)) + + SmallVector archetypes; + SmallVector substitutions; + bool matched; + if (paramType->is()) + matched = false; + else + matched = findGenericSubstitutions(dc, paramType, rArgType, + archetypes, substitutions); + + if (matched && archetypes.size() == 0) continue; - if (auto genericParam = paramType->getAs()) - paramType = genericParam->getDecl()->getArchetype(); - if (paramType->is() && !rArgType->hasTypeVariable()) { + if (matched && archetypes.size() == 1 && !rArgType->hasTypeVariable()) { + auto archetype = archetypes[0]; + auto substitution = substitutions[0]; + if (singleArchetype) { - if (!paramType->isEqual(singleArchetype)) + if (!archetype->isEqual(singleArchetype)) // Multiple archetypes, too complicated. return { CC_ArgumentMismatch, {}}; - if (rArgType->isEqual(matchingArgType)) { + + if (substitution->isEqual(matchingArgType)) { if (nonSubstitutableArgs == 0) continue; ++nonSubstitutableArgs; @@ -1087,20 +1161,18 @@ CalleeCandidateInfo::evaluateCloseness(Type candArgListType, // 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. - auto archetype = paramType->castTo(); - if (CS->TC.isSubstitutableFor(rArgType, archetype, CS->DC)) { - mismatchesAreNearMisses = argumentMismatchIsNearMiss(matchingArgType, rArgType); - matchingArgType = rArgType; + if (CS->TC.isSubstitutableFor(substitution, archetype, CS->DC)) { + mismatchesAreNearMisses = argumentMismatchIsNearMiss(matchingArgType, substitution); + matchingArgType = substitution; continue; } } } } else { - matchingArgType = rArgType; - singleArchetype = paramType; + matchingArgType = substitution; + singleArchetype = archetype; - auto archetype = paramType->getAs(); - if (CS->TC.isSubstitutableFor(rArgType, archetype, CS->DC)) { + if (CS->TC.isSubstitutableFor(substitution, archetype, CS->DC)) { continue; } ++nonSubstitutableArgs; @@ -1325,7 +1397,9 @@ void CalleeCandidateInfo::filterList(ArrayRef actualArgs) { // If this isn't a function or isn't valid at this uncurry level, treat it // as a general mismatch. if (!inputType) return { CC_GeneralMismatch, {}}; - return evaluateCloseness(inputType, actualArgs); + Decl *decl = candidate.getDecl(); + return evaluateCloseness(decl ? decl->getInnermostDeclContext() : nullptr, + inputType, actualArgs); }); } @@ -3497,8 +3571,10 @@ bool FailureDiagnosis::visitSubscriptExpr(SubscriptExpr *SE) { } // Explode out multi-index subscripts to find the best match. + Decl *decl = cand.getDecl(); auto indexResult = - calleeInfo.evaluateCloseness(cand.getArgumentType(), decomposedIndexType); + calleeInfo.evaluateCloseness(decl ? decl->getInnermostDeclContext() : nullptr, + cand.getArgumentType(), decomposedIndexType); if (selfConstraint > indexResult.first) return {selfConstraint, {}}; return indexResult; diff --git a/test/Constraints/diagnostics.swift b/test/Constraints/diagnostics.swift index 389259edd42c7..39af0540dca4a 100644 --- a/test/Constraints/diagnostics.swift +++ b/test/Constraints/diagnostics.swift @@ -71,6 +71,7 @@ i.wobble() // expected-error{{value of type 'Int' has no member 'wobble'}} // Generic member does not conform. extension Int { func wibble(x: T, _ y: T) -> T { return x } + func wubble(x: Int -> T) -> T { return x(self) } } i.wibble(3, 4) // expected-error {{argument type 'Int' does not conform to expected type 'P2'}} @@ -82,6 +83,15 @@ let a = A() for j in i.wibble(a, a) { // expected-error {{type 'A' does not conform to protocol 'SequenceType'}} } +// Generic as part of function/tuple types +func f6(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)'}} +} + // 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'}} diff --git a/test/Generics/function_defs.swift b/test/Generics/function_defs.swift index 8f68c5b560d24..c5270dd9f9797 100644 --- a/test/Generics/function_defs.swift +++ b/test/Generics/function_defs.swift @@ -17,9 +17,7 @@ func doCompare(t1: T, t2: T, u: U) -> return true } - // FIXME: This is not ambiguous, the actual problem is that 'u' has the wrong - // type - "U" is not the same as "T". - return t1.isEqual(u) // expected-error {{type of expression is ambiguous without more context}} + return t1.isEqual(u) // expected-error {{cannot invoke 'isEqual' with an argument list of type '(U)'}} expected-note {{expected an argument list of type '(T)'}} } protocol MethodLessComparable { @@ -171,9 +169,7 @@ func staticEqCheck(t: T, u: U) { if T.isEqual(t, y: t) { return } if U.isEqual(u, y: u) { return } - // FIXME: This is not ambiguous, the actual problem is that 'u' has the wrong - // type - "U" is not the same as "T". - T.isEqual(t, y: u) // expected-error{{type of expression is ambiguous without more context}} + T.isEqual(t, y: u) // expected-error{{cannot invoke 'isEqual' with an argument list of type '(T, y: U)'}} expected-note {{expected an argument list of type '(T, y: T)'}} } //===----------------------------------------------------------------------===// diff --git a/test/Misc/misc_diagnostics.swift b/test/Misc/misc_diagnostics.swift index 6c5369c356ae9..034c1e35a1a46 100644 --- a/test/Misc/misc_diagnostics.swift +++ b/test/Misc/misc_diagnostics.swift @@ -91,7 +91,7 @@ func testIS1() -> Int { return 0 } let _: String = testIS1() // expected-error {{cannot convert value of type 'Int' to specified type 'String'}} func insertA(inout array : [T], elt : T) { - array.append(T); // expected-error {{ambiguous reference to member 'append'}} + array.append(T); // expected-error {{cannot invoke 'append' with an argument list of type '((T).Type)'}} expected-note {{expected an argument list of type '(T)'}} } // can't append to array of tuples @@ -130,6 +130,6 @@ func test17875634() { // Pattern matching ranges against tuples crashes the compiler func test20770032() { - if case let 1...10 = (1, 1) { // expected-warning{{'let' pattern has no effect; sub-pattern didn't bind any variables}} {{11-15=}} expected-error{{cannot convert value of type '(Int, Int)' to expected argument type 'Range'}} + if case let 1...10 = (1, 1) { // expected-warning{{'let' pattern has no effect; sub-pattern didn't bind any variables}} {{11-15=}} expected-error{{expression pattern of type 'Range' cannot match values of type '(Int, Int)'}} } } diff --git a/validation-test/stdlib/StdlibUnittestStaticAssertions.swift b/validation-test/stdlib/StdlibUnittestStaticAssertions.swift index ca4a4e95eedb8..ccc978e3f141e 100644 --- a/validation-test/stdlib/StdlibUnittestStaticAssertions.swift +++ b/validation-test/stdlib/StdlibUnittestStaticAssertions.swift @@ -21,7 +21,6 @@ func test_expectType() { func test_expectEqualType() { expectEqualType(S1.self, S1.self) - expectEqualType(S1.self, S2.self) // expected-error {{cannot invoke 'expectEqualType' with an argument list of type '(S1.Type, S2.Type)'}} - // expected-note @-1 {{expected an argument list of type '(T.Type, T.Type)'}} + expectEqualType(S1.self, S2.self) // expected-error {{cannot convert value of type 'S2.Type' to expected argument type 'S1'}} }