From f8dbb20a19c9022b486da9544a731d25f1eba7c2 Mon Sep 17 00:00:00 2001 From: gregomni Date: Mon, 15 Feb 2016 14:05:23 -0800 Subject: [PATCH] [Sema] Extend callee diagnosis to multiple generics Straightforward extension of the previous work here to handle callees with multiple generic parameters. Same type requirements are already handled by the ArchetypeBuilder, and protocol conformance is handled explicitly. This is still bailing on nested archetypes. Pre-existing tests updated that now give better diagnoses. --- lib/Sema/CSDiag.cpp | 185 ++++++++++++++++-------------- test/Constraints/generics.swift | 3 +- test/Generics/deduction.swift | 4 +- test/Generics/function_defs.swift | 14 ++- test/Generics/generic_types.swift | 3 +- 5 files changed, 113 insertions(+), 96 deletions(-) diff --git a/lib/Sema/CSDiag.cpp b/lib/Sema/CSDiag.cpp index c4eb776bcec12..4dff2a85fd343 100644 --- a/lib/Sema/CSDiag.cpp +++ b/lib/Sema/CSDiag.cpp @@ -987,18 +987,14 @@ static bool argumentMismatchIsNearMiss(Type argType, Type paramType) { /// 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) { + TypeSubstitutionMap &archetypesMap) { class GenericVisitor : public TypeMatcher { DeclContext *dc; - SmallVector &archetypes; - SmallVector &substitutions; + TypeSubstitutionMap &archetypesMap; public: - GenericVisitor(DeclContext *dc, - SmallVector &archetypes, - SmallVector &substitutions) - : dc(dc), archetypes(archetypes), substitutions(substitutions) {} + GenericVisitor(DeclContext *dc, TypeSubstitutionMap &archetypesMap) + : dc(dc), archetypesMap(archetypesMap) {} bool mismatch(TypeBase *paramType, TypeBase *argType) { return paramType->isEqual(argType); @@ -1010,12 +1006,10 @@ static bool findGenericSubstitutions(DeclContext *dc, Type paramType, Type actua 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); + auto existing = archetypesMap[archetype]; + if (existing) + return existing->isEqual(argType); + archetypesMap[archetype] = argType; return true; } return false; @@ -1030,15 +1024,22 @@ static bool findGenericSubstitutions(DeclContext *dc, Type paramType, Type actua if (dc && original->isTypeParameter()) original = ArchetypeBuilder::mapTypeIntoContext(dc, original); - if (auto archetype = original->getAs()) { - archetypes.push_back(archetype); - substitutions.push_back(substitution->getReplacementType()); - } + Type replacement = substitution->getReplacementType(); + // If the replacement is itself an archetype, then the constraint + // system was asserting equivalencies between different levels of + // generics, rather than binding a generic to a concrete type (and we + // don't/won't have a concrete type). In which case, it is the + // replacement we are interested in, since it is the one in our current + // context. That generic type should equal itself. + if (auto ourGeneric = replacement->getAs()) + archetypesMap[ourGeneric] = replacement; + else if (auto archetype = original->getAs()) + archetypesMap[archetype] = replacement; } return false; }); - GenericVisitor visitor(dc, archetypes, substitutions); + GenericVisitor visitor(dc, archetypesMap); return visitor.match(paramType, actualArgType); } @@ -1085,18 +1086,20 @@ CalleeCandidateInfo::evaluateCloseness(DeclContext *dc, Type candArgListType, // their type and count the number of mismatched arguments. unsigned mismatchingArgs = 0; - // Checking of archetypes. - // FIXME: For now just trying to verify applicability of arguments with only - // a single generic variable. Ideally we'd create a ConstraintSystem with - // type variables for all generics and solve it with the given argument types. - Type singleArchetype = nullptr; - Type matchingArgType = nullptr; + // Known mapping of archetypes in all arguments so far. An archetype may map + // to another archetype if the constraint system substituted one for another. + TypeSubstitutionMap allGenericSubstitutions; - // Number of args of generic archetype which are mismatched because + // Number of args of one generic archetype which are mismatched because // isSubstitutableFor() has failed. If all mismatches are of this type, we'll // return a different closeness for better diagnoses. + Type nonSubstitutableArchetype = nullptr; unsigned nonSubstitutableArgs = 0; + // The type of failure is that multiple occurrences of the same generic are + // being passed arguments with different concrete types. + bool genericWithDifferingConcreteTypes = false; + // We classify an argument mismatch as being a "near" miss if it is a very // likely match due to a common sort of problem (e.g. wrong flags on a // function type, optional where none was expected, etc). This allows us to @@ -1122,75 +1125,77 @@ CalleeCandidateInfo::evaluateCloseness(DeclContext *dc, Type candArgListType, // FIXME: Right now, a "matching" overload is one with a parameter whose // type is identical to the argument type, or substitutable via handling - // of functions with a single archetype in one or more parameters. + // of functions with primary archetypes in one or more parameters. // We can still do something more sophisticated with this. // FIXME: Use TC.isConvertibleTo? - SmallVector archetypes; - SmallVector substitutions; + TypeSubstitutionMap archetypesMap; bool matched; - if (paramType->is()) + if (paramType->is() || rArgType->hasTypeVariable()) matched = false; else - matched = findGenericSubstitutions(dc, paramType, rArgType, - archetypes, substitutions); + matched = findGenericSubstitutions(dc, paramType, rArgType, archetypesMap); - if (matched && archetypes.size() == 0) - continue; - if (matched && archetypes.size() == 1 && !rArgType->hasTypeVariable()) { - auto archetype = archetypes[0]; - auto substitution = substitutions[0]; - - if (singleArchetype) { - if (!archetype->isEqual(singleArchetype)) - // Multiple archetypes, too complicated. - return { CC_ArgumentMismatch, {}}; + if (matched) { + for (auto pair : archetypesMap) { + auto archetype = pair.first->castTo(); + auto substitution = pair.second; - if (substitution->isEqual(matchingArgType)) { - if (nonSubstitutableArgs == 0) - continue; - ++nonSubstitutableArgs; - mismatchesAreNearMisses = false; + auto existingSubstitution = allGenericSubstitutions[archetype]; + if (!existingSubstitution) { + // New substitution for this callee. + allGenericSubstitutions[archetype] = substitution; + + // Not yet handling nested archetypes. + if (!archetype->isPrimary()) + return { CC_ArgumentMismatch, {}}; + + if (!CS->TC.isSubstitutableFor(substitution, archetype, CS->DC)) { + // If we have multiple non-substitutable types, this is just a mismatched mess. + if (!nonSubstitutableArchetype.isNull()) + return { CC_ArgumentMismatch, {}}; + + if (auto argOptType = argType->getOptionalObjectType()) + mismatchesAreNearMisses &= CS->TC.isSubstitutableFor(argOptType, archetype, CS->DC); + else + mismatchesAreNearMisses = false; + + nonSubstitutableArchetype = archetype; + nonSubstitutableArgs = 1; + matched = false; + } } else { - if (nonSubstitutableArgs == 1) { + // Substitution for the same archetype as in a previous argument. + bool isNonSubstitutableArchetype = !nonSubstitutableArchetype.isNull() && + nonSubstitutableArchetype->isEqual(archetype); + if (substitution->isEqual(existingSubstitution)) { + if (isNonSubstitutableArchetype) { + ++nonSubstitutableArgs; + matched = false; + } + } else { // 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. - if (CS->TC.isSubstitutableFor(substitution, archetype, CS->DC)) { - mismatchesAreNearMisses = argumentMismatchIsNearMiss(matchingArgType, substitution); - matchingArgType = substitution; - continue; + if (isNonSubstitutableArchetype && nonSubstitutableArgs == 1 && + CS->TC.isSubstitutableFor(substitution, archetype, CS->DC)) { + mismatchesAreNearMisses = argumentMismatchIsNearMiss(existingSubstitution, substitution); + allGenericSubstitutions[archetype] = substitution; + } else { + genericWithDifferingConcreteTypes = true; + matched = false; } } - - // 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()) - return matchingArgType; - return type; - })); } - } else { - matchingArgType = substitution; - singleArchetype = archetype; - - 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); } + if (matched) + continue; + + if (archetypesMap.empty()) + mismatchesAreNearMisses &= argumentMismatchIsNearMiss(argType, paramType); + ++mismatchingArgs; failureInfo.argumentNumber = argNo; @@ -1213,10 +1218,23 @@ CalleeCandidateInfo::evaluateCloseness(DeclContext *dc, Type candArgListType, // close matches are prioritized against obviously wrong ones. if (mismatchingArgs == 1) { CandidateCloseness closeness; - if (singleArchetype.isNull()) { + if (allGenericSubstitutions.empty()) { closeness = mismatchesAreNearMisses ? CC_OneArgumentNearMismatch : CC_OneArgumentMismatch; } else { + // If the failure is that different occurrences of the same generic have + // different concrete types, substitute in all the concrete types we've found + // into the failureInfo to improve diagnosis. + if (genericWithDifferingConcreteTypes) { + auto newType = failureInfo.parameterType.transform([&](Type type) -> Type { + if (auto archetype = type->getAs()) + if (auto replacement = allGenericSubstitutions[archetype]) + return replacement; + return type; + }); + failureInfo.parameterType = newType; + } + closeness = mismatchesAreNearMisses ? CC_OneGenericArgumentNearMismatch : CC_OneGenericArgumentMismatch; } @@ -1589,16 +1607,15 @@ bool CalleeCandidateInfo::diagnoseGenericParameterErrors(Expr *badArgExpr) { return false; bool foundFailure = false; - SmallVector archetypes; - SmallVector substitutions; + TypeSubstitutionMap archetypesMap; if (!findGenericSubstitutions(failedArgument.declContext, failedArgument.parameterType, - argType, archetypes, substitutions)) + argType, archetypesMap)) return false; - for (unsigned i = 0, c = archetypes.size(); i < c; i++) { - auto archetype = archetypes[i]; - auto substitution = substitutions[i]; + for (auto pair : archetypesMap) { + auto archetype = pair.first->castTo(); + auto substitution = pair.second; // FIXME: Add specific error for not subclass, if the archetype has a superclass? diff --git a/test/Constraints/generics.swift b/test/Constraints/generics.swift index c4ac9057ad43f..30cd877d6ede1 100644 --- a/test/Constraints/generics.swift +++ b/test/Constraints/generics.swift @@ -14,8 +14,7 @@ func min(x: T, y: T) -> T { func weirdConcat(t: T, u: U) { t +++ u t +++ 1 - u +++ t // expected-error{{binary operator '+++' cannot be applied to operands of type 'U' and 'T'}} - // expected-note @-1 {{expected an argument list of type '(Self, T)'}} + u +++ t // expected-error{{argument type 'U' does not conform to expected type 'ConcatToAnything'}} } // Make sure that the protocol operators don't get in the way. diff --git a/test/Generics/deduction.swift b/test/Generics/deduction.swift index 79ddd19692d2b..82336531ffeb1 100644 --- a/test/Generics/deduction.swift +++ b/test/Generics/deduction.swift @@ -65,7 +65,7 @@ func takeTuples(_: (T, U), _: (U, T)) { func useTuples(x: Int, y: Float, z: (Float, Int)) { takeTuples((x, y), (y, x)) - takeTuples((x, y), (x, y)) // expected-error{{cannot convert value of type '(Int, Float)' to expected argument type '(_, _)'}} + takeTuples((x, y), (x, y)) // expected-error{{cannot convert value of type 'Int' to expected argument type 'Float'}} // FIXME: Use 'z', which requires us to fix our tuple-conversion // representation. @@ -75,7 +75,7 @@ func acceptFunction(f: (T) -> U, _ t: T, _ u: U) {} func passFunction(f: (Int) -> Float, x: Int, y: Float) { acceptFunction(f, x, y) - acceptFunction(f, y, y) // expected-error{{cannot convert value of type '(Int) -> Float' to expected argument type '(_) -> _'}} + acceptFunction(f, y, y) // expected-error{{cannot convert value of type 'Float' to expected argument type 'Int'}} } func returnTuple(_: T) -> (T, U) { } // expected-note {{in call to function 'returnTuple'}} diff --git a/test/Generics/function_defs.swift b/test/Generics/function_defs.swift index c5270dd9f9797..6be23af8efef9 100644 --- a/test/Generics/function_defs.swift +++ b/test/Generics/function_defs.swift @@ -75,8 +75,8 @@ protocol Overload { associatedtype B func getA() -> A func getB() -> B - func f1(_: A) -> A // expected-note{{found this candidate}} - func f1(_: B) -> B // expected-note{{found this candidate}} + func f1(_: A) -> A + func f1(_: B) -> B func f2(_: Int) -> A // expected-note{{found this candidate}} func f2(_: Int) -> B // expected-note{{found this candidate}} func f3(_: Int) -> Int // expected-note {{found this candidate}} @@ -106,7 +106,8 @@ func testOverload(ovl: Ovl, ovl2: Ovl, a = ovl2.f2(17) a = ovl2.f1(a) - other.f1(a) // expected-error{{ambiguous reference to member 'f1'}} + other.f1(a) // expected-error{{cannot invoke 'f1' with an argument list of type '(Ovl.A)'}} + // expected-note @-1 {{overloads for 'f1' exist with these partially matching parameter lists: (Self.A), (Self.B)}} // Overloading based on context var f3i : (Int) -> Int = ovl.f3 @@ -132,7 +133,7 @@ protocol Subscriptable { func getIndex() -> Index func getValue() -> Value - subscript (index : Index) -> Value { get set } // expected-note{{found this candidate}} + subscript (index : Index) -> Value { get set } } protocol IntSubscriptable { @@ -140,7 +141,7 @@ protocol IntSubscriptable { func getElement() -> ElementType - subscript (index : Int) -> ElementType { get } // expected-note{{found this candidate}} + subscript (index : Int) -> ElementType { get } } func subscripting>(t: T) { @@ -153,7 +154,8 @@ func subscripting>(t: T) { element = t[17] t[42] = element // expected-error{{cannot assign through subscript: subscript is get-only}} - t[value] = 17 // expected-error{{ambiguous reference to member 'subscript'}} + // Suggests the Int form because we prefer concrete matches to generic matches in diagnosis. + t[value] = 17 // expected-error{{cannot convert value of type 'T.Value' to expected argument type 'Int'}} } //===----------------------------------------------------------------------===// diff --git a/test/Generics/generic_types.swift b/test/Generics/generic_types.swift index a8d95efd251e5..53dde737cad6a 100644 --- a/test/Generics/generic_types.swift +++ b/test/Generics/generic_types.swift @@ -200,8 +200,7 @@ func useNested(ii: Int, hni: HasNested, // Generic constructor of a generic struct HNI(1, 2.71828) // expected-warning{{unused}} - // FIXME: Should report this error: {{cannot convert the expression's type 'HNI' to type 'Int'}} - HNI(1.5, 2.71828) // expected-error{{cannot invoke initializer for type 'HNI' with an argument list of type '(Double, Double)'}} expected-note{{expected an argument list of type '(T, U)'}} + HNI(1.5, 2.71828) // expected-error{{'Double' is not convertible to 'Int'}} // Generic function in a nested generic struct var ids = xis.g(1, u: "Hello", v: 3.14159)