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)