From 1dd6d349d1b12757c39a55e9b28a3b5546e9a9a7 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Wed, 1 Mar 2023 11:31:08 -0800 Subject: [PATCH 1/4] [ConstraintSystem] NFC: Pass down matcher flags to `repairFailures` --- include/swift/Sema/ConstraintSystem.h | 1 + lib/Sema/CSSimplify.cpp | 11 ++++++----- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 3db194e0b25e6..7d1413c4a07c7 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -5257,6 +5257,7 @@ class ConstraintSystem { /// \return true if at least some of the failures has been repaired /// successfully, which allows type matcher to continue. bool repairFailures(Type lhs, Type rhs, ConstraintKind matchKind, + TypeMatchOptions flags, SmallVectorImpl &conversionsOrFixes, ConstraintLocatorBuilder locator); diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index c1d378bd810ae..eec65b072176e 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -4843,7 +4843,7 @@ static bool repairOutOfOrderArgumentsInBinaryFunction( /// \return true if at least some of the failures has been repaired /// successfully, which allows type matcher to continue. bool ConstraintSystem::repairFailures( - Type lhs, Type rhs, ConstraintKind matchKind, + Type lhs, Type rhs, ConstraintKind matchKind, TypeMatchOptions flags, SmallVectorImpl &conversionsOrFixes, ConstraintLocatorBuilder locator) { SmallVector path; @@ -5344,7 +5344,7 @@ bool ConstraintSystem::repairFailures( // let's re-attempt to repair without l-value conversion in the // locator to fix underlying type mismatch. if (path.back().is()) { - return repairFailures(lhs, rhs, matchKind, conversionsOrFixes, + return repairFailures(lhs, rhs, matchKind, flags, conversionsOrFixes, getConstraintLocator(anchor, path)); } @@ -6133,7 +6133,7 @@ bool ConstraintSystem::repairFailures( if (!path.empty() && path.back().is()) path.pop_back(); - return repairFailures(lhs, rhs, matchKind, conversionsOrFixes, + return repairFailures(lhs, rhs, matchKind, flags, conversionsOrFixes, getConstraintLocator(anchor, path)); } @@ -6377,7 +6377,7 @@ bool ConstraintSystem::repairFailures( path.pop_back(); if (!path.empty() && path.back().is()) { - return repairFailures(lhs, rhs, matchKind, conversionsOrFixes, + return repairFailures(lhs, rhs, matchKind, flags, conversionsOrFixes, getConstraintLocator(anchor, path)); } @@ -7479,7 +7479,8 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind, // Attempt fixes iff it's allowed, both types are concrete and // we are not in the middle of attempting one already. if (shouldAttemptFixes() && !flags.contains(TMF_ApplyingFix)) { - if (repairFailures(type1, type2, kind, conversionsOrFixes, locator)) { + if (repairFailures(type1, type2, kind, flags, conversionsOrFixes, + locator)) { if (conversionsOrFixes.empty()) return getTypeMatchSuccess(); } From 17b09a801ec5121bc4910a34755f768573cf9b3c Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Fri, 3 Mar 2023 18:50:19 -0800 Subject: [PATCH 2/4] [ConstraintSystem] Add a locator element to represent a generic type This locator is going to be used as a "parent" element for "generic argument" elements. --- include/swift/Sema/ConstraintLocator.h | 14 ++++++++++++++ include/swift/Sema/ConstraintLocatorPathElts.def | 4 ++++ lib/Sema/ConstraintLocator.cpp | 7 +++++++ lib/Sema/ConstraintSystem.cpp | 1 + 4 files changed, 26 insertions(+) diff --git a/include/swift/Sema/ConstraintLocator.h b/include/swift/Sema/ConstraintLocator.h index 801ed3645cd06..b034f4db4c0d7 100644 --- a/include/swift/Sema/ConstraintLocator.h +++ b/include/swift/Sema/ConstraintLocator.h @@ -643,6 +643,20 @@ class LocatorPathElt::TupleType : public StoredPointerElement { } }; +class LocatorPathElt::GenericType : public StoredPointerElement { +public: + GenericType(Type type) + : StoredPointerElement(PathElementKind::GenericType, type.getPointer()) { + assert(type->getDesugaredType()->is()); + } + + Type getType() const { return getStoredPointer(); } + + static bool classof(const LocatorPathElt *elt) { + return elt->getKind() == PathElementKind::GenericType; + } +}; + /// Abstract superclass for any kind of tuple element. class LocatorPathElt::AnyTupleElement : public StoredIntegerElement<1> { protected: diff --git a/include/swift/Sema/ConstraintLocatorPathElts.def b/include/swift/Sema/ConstraintLocatorPathElts.def index 8627a12c5f2b4..4af99948d5a94 100644 --- a/include/swift/Sema/ConstraintLocatorPathElts.def +++ b/include/swift/Sema/ConstraintLocatorPathElts.def @@ -181,6 +181,10 @@ CUSTOM_LOCATOR_PATH_ELT(SynthesizedArgument) /// path components. CUSTOM_LOCATOR_PATH_ELT(TupleType) +/// A generic type, which provides context for subsequent generic +/// argument path components. +CUSTOM_LOCATOR_PATH_ELT(GenericType) + /// Tuple elements. ABSTRACT_LOCATOR_PATH_ELT(AnyTupleElement) /// A tuple element referenced by position. diff --git a/lib/Sema/ConstraintLocator.cpp b/lib/Sema/ConstraintLocator.cpp index e711f03fa9a45..8054d4534e1ba 100644 --- a/lib/Sema/ConstraintLocator.cpp +++ b/lib/Sema/ConstraintLocator.cpp @@ -67,6 +67,7 @@ unsigned LocatorPathElt::getNewSummaryFlags() const { case ConstraintLocator::GenericParameter: case ConstraintLocator::GenericArgument: case ConstraintLocator::TupleType: + case ConstraintLocator::GenericType: case ConstraintLocator::NamedTupleElement: case ConstraintLocator::TupleElement: case ConstraintLocator::ProtocolRequirement: @@ -242,6 +243,12 @@ void LocatorPathElt::dump(raw_ostream &out) const { break; } + case ConstraintLocator::GenericType: { + auto genericTyElt = elt.castTo(); + out << "generic type '" << genericTyElt.getType()->getString(PO) << "'"; + break; + } + case ConstraintLocator::NamedTupleElement: { auto tupleElt = elt.castTo(); out << "named tuple element #" << llvm::utostr(tupleElt.getIndex()); diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index f5457bb5856fa..8baeeebe89ed1 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -5372,6 +5372,7 @@ void constraints::simplifyLocator(ASTNode &anchor, continue; case ConstraintLocator::TupleType: + case ConstraintLocator::GenericType: path = path.slice(1); continue; From 059891771fb95b4e54e6413062701ac511bf8f4e Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Wed, 1 Mar 2023 15:14:36 -0800 Subject: [PATCH 3/4] [CSFix] Implement coalescing for generic argument mismatch fixes --- include/swift/Sema/CSFix.h | 7 +++++++ lib/Sema/CSFix.cpp | 23 ++++++++++++++++++++++- 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/include/swift/Sema/CSFix.h b/include/swift/Sema/CSFix.h index 6b77469cc320b..58698804345c4 100644 --- a/include/swift/Sema/CSFix.h +++ b/include/swift/Sema/CSFix.h @@ -1100,6 +1100,10 @@ class GenericArgumentsMismatch final return {getTrailingObjects(), NumMismatches}; } + bool coalesceAndDiagnose(const Solution &solution, + ArrayRef secondaryFixes, + bool asNote = false) const override; + bool diagnose(const Solution &solution, bool asNote = false) const override; static GenericArgumentsMismatch *create(ConstraintSystem &cs, Type actual, @@ -1112,6 +1116,9 @@ class GenericArgumentsMismatch final } private: + bool diagnose(const Solution &solution, ArrayRef mismatches, + bool asNote = false) const; + MutableArrayRef getMismatchesBuf() { return {getTrailingObjects(), NumMismatches}; } diff --git a/lib/Sema/CSFix.cpp b/lib/Sema/CSFix.cpp index 8412c35890c53..fbaa31d653a80 100644 --- a/lib/Sema/CSFix.cpp +++ b/lib/Sema/CSFix.cpp @@ -732,10 +732,31 @@ AllowFunctionTypeMismatch::create(ConstraintSystem &cs, Type lhs, Type rhs, AllowFunctionTypeMismatch(cs, lhs, rhs, locator, index); } +bool GenericArgumentsMismatch::coalesceAndDiagnose( + const Solution &solution, ArrayRef secondaryFixes, + bool asNote) const { + std::set scratch(getMismatches().begin(), getMismatches().end()); + + for (auto *fix : secondaryFixes) { + auto *genericArgsFix = fix->castTo(); + for (auto mismatchIdx : genericArgsFix->getMismatches()) + scratch.insert(mismatchIdx); + } + + SmallVector mismatches(scratch.begin(), scratch.end()); + return diagnose(solution, mismatches, asNote); +} + +bool GenericArgumentsMismatch::diagnose(const Solution &solution, + bool asNote) const { + return diagnose(solution, getMismatches(), asNote); +} + bool GenericArgumentsMismatch::diagnose(const Solution &solution, + ArrayRef mismatches, bool asNote) const { GenericArgumentsMismatchFailure failure(solution, getFromType(), getToType(), - getMismatches(), getLocator()); + mismatches, getLocator()); return failure.diagnose(asNote); } From b7745b04bd21118a69a6e3891adc09a05947c650 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Thu, 2 Mar 2023 10:59:20 -0800 Subject: [PATCH 4/4] [CSSimplify] Detect and diagnose generic argument mismatches individually Generic arguments types are not always resolved enough to enable aggregated mismatch fixes, which means that the solver should be able to handle standalone generic argument matching constraints and create a fix per mismatch location to coalesce them during diagnostics. Resolves: rdar://106054263 --- include/swift/Sema/ConstraintSystem.h | 4 ++ lib/Sema/CSSimplify.cpp | 71 ++++++++++++++++++++++++--- test/Constraints/generics.swift | 24 +++++++++ test/Generics/issue-55666.swift | 16 +++--- 4 files changed, 101 insertions(+), 14 deletions(-) diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 7d1413c4a07c7..d50ec5bceecad 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -4655,6 +4655,10 @@ class ConstraintSystem { /// Indicates that we are attempting a possible type for /// a type variable. TMF_BindingTypeVariable = 0x04, + + /// Indicates that the solver is matching one of the + /// generic argument pairs as part of matching two generic types. + TMF_MatchingGenericArguments = 0x08, }; /// Options that govern how type matching should proceed. diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index eec65b072176e..05fdd63355038 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -3748,15 +3748,23 @@ ConstraintSystem::matchDeepEqualityTypes(Type type1, Type type2, // Match up the generic arguments, exactly. if (shouldAttemptFixes()) { + auto *baseLoc = + getConstraintLocator(locator, {LocatorPathElt::GenericType(bound1), + LocatorPathElt::GenericType(bound2)}); + + auto argMatchingFlags = + subflags | TMF_ApplyingFix | TMF_MatchingGenericArguments; + // Optionals have a lot of special diagnostics and only one // generic argument so if we' re dealing with one, don't produce generic // arguments mismatch fixes. if (bound1->getDecl()->isOptionalDecl()) - return matchDeepTypeArguments(*this, subflags, args1, args2, locator); + return matchDeepTypeArguments(*this, argMatchingFlags, args1, args2, + baseLoc); SmallVector mismatches; auto result = matchDeepTypeArguments( - *this, subflags | TMF_ApplyingFix, args1, args2, locator, + *this, argMatchingFlags, args1, args2, baseLoc, [&mismatches](unsigned position) { mismatches.push_back(position); }); if (mismatches.empty()) @@ -6369,7 +6377,8 @@ bool ConstraintSystem::repairFailures( // failure e.g. `String bind T.Element`, so let's drop the generic argument // path element and recurse in repairFailures to check and potentially // record the requirement failure fix. - path.pop_back(); + auto genericArgElt = + path.pop_back_val().castTo(); // If we have something like ... -> type req # -> pack element #, we're // solving a requirement of the form T : P where T is a type parameter pack @@ -6381,7 +6390,57 @@ bool ConstraintSystem::repairFailures( getConstraintLocator(anchor, path)); } - break; + // When the solver sets `TMF_MatchingGenericArguments` it means + // that it's matching generic argument pairs to identify any mismatches + // as part of larger matching of two generic types. Letting this + // fail results in a single fix that aggregates all mismatch locations. + // + // Types are not always resolved enough to enable that which means + // that the comparison should be delayed, which brings us here - a + // standalone constraint that represents such a match, in such cases + // we create a fix per mismatch location and coalesce them during + // diagnostics. + if (flags.contains(TMF_MatchingGenericArguments)) + break; + + Type fromType; + Type toType; + + if (path.size() >= 2) { + if (path[path.size() - 2].is()) { + fromType = path[path.size() - 2] + .castTo() + .getType(); + } + + if (path[path.size() - 1].is()) { + toType = path[path.size() - 1] + .castTo() + .getType(); + } + } + + if (!fromType || !toType) + break; + + // Drop both `GenericType` elements. + path.pop_back(); + path.pop_back(); + + ConstraintFix *fix = nullptr; + if (!path.empty() && path.back().is()) { + fix = fixRequirementFailure(*this, fromType, toType, anchor, path); + } else { + fix = GenericArgumentsMismatch::create( + *this, fromType, toType, {genericArgElt.getIndex()}, + getConstraintLocator(anchor, path)); + } + + if (!fix) + break; + + conversionsOrFixes.push_back(fix); + return true; } case ConstraintLocator::ResultBuilderBodyResult: { @@ -14023,7 +14082,8 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint( case FixKind::RenameConflictingPatternVariables: case FixKind::MustBeCopyable: case FixKind::MacroMissingPound: - case FixKind::AllowGlobalActorMismatch: { + case FixKind::AllowGlobalActorMismatch: + case FixKind::GenericArgumentsMismatch: { return recordFix(fix) ? SolutionKind::Error : SolutionKind::Solved; } case FixKind::IgnoreInvalidASTNode: { @@ -14243,7 +14303,6 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint( case FixKind::TreatKeyPathSubscriptIndexAsHashable: case FixKind::AllowInvalidRefInKeyPath: case FixKind::DefaultGenericArgument: - case FixKind::GenericArgumentsMismatch: case FixKind::AllowMutatingMemberOnRValueBase: case FixKind::AllowTupleSplatForSingleParameter: case FixKind::AllowNonClassTypeToConvertToAnyObject: diff --git a/test/Constraints/generics.swift b/test/Constraints/generics.swift index 8cfa0bdcba833..1bec0aa37527b 100644 --- a/test/Constraints/generics.swift +++ b/test/Constraints/generics.swift @@ -1031,3 +1031,27 @@ func test_requirement_failures_in_ambiguous_context() { f3(A()) // expected-error {{no exact matches in call to local function 'f3'}} } + +// rdar://106054263 - failed to produce a diagnostic upon generic argument mismatch +func test_mismatches_with_dependent_member_generic_arguments() { + struct Binding {} + // expected-note@-1 {{arguments to generic parameter 'T' ('Double?' and 'Data.SomeAssociated') are expected to be equal}} + // expected-note@-2 {{arguments to generic parameter 'U' ('Int' and 'Data.SomeAssociated') are expected to be equal}} + + struct Data : SomeProtocol { + typealias SomeAssociated = String + } + + func test1(_: Binding, _: T) where T.SomeAssociated == String { + } + + func test2(_: Optional, _: T) where T.SomeAssociated == String { + } + + test1(Binding(), Data()) + // expected-error@-1 {{cannot convert value of type 'Binding' to expected argument type 'Binding'}} + + test2(Optional(nil), Data()) + // expected-error@-1 {{cannot convert value of type 'Optional' to expected argument type 'Optional'}} + // expected-note@-2 {{arguments to generic parameter 'Wrapped' ('Int' and 'Data.SomeAssociated') are expected to be equal}} +} diff --git a/test/Generics/issue-55666.swift b/test/Generics/issue-55666.swift index bd829def873c9..812c22d6f7478 100644 --- a/test/Generics/issue-55666.swift +++ b/test/Generics/issue-55666.swift @@ -6,19 +6,19 @@ struct W {} struct S { init(){} - // expected-note@+2 {{where 'C1.Element' = 'String', 'W' = 'Int'}} - // expected-note@+1 {{where 'C1.Element' = 'C1', 'W' = 'C2.Element'}} + // expected-note@+2 {{where 'C1.Element' = 'W', 'main.W' = 'main.W'}} + // expected-note@+1 {{where 'C1.Element' = 'W', 'W' = 'W.Element>'}} init(_ c2: W) where C2: Collection, C1.Element == W {} - // expected-note@+1 {{where 'C1.Element' = 'String', 'W' = 'Int'}} + // expected-note@+1 {{where 'C1.Element' = 'W', 'W' = 'W.Element>'}} static func f(_ c2: W) where C2: Collection, C1.Element == W {} - // expected-note@+1 {{where 'C1.Element' = 'String', 'W' = 'Int'}} + // expected-note@+1 {{where 'C1.Element' = 'W', 'W' = 'W.Element>'}} func instancef(_ c2: W) where C2: Collection, C1.Element == W {} } -let _ = S<[W]>(W<[Int]>()) // expected-error{{initializer 'init(_:)' requires the types 'String' and 'Int' be equivalent}} -let _ = S<[W]>.f(W<[Int]>()) // expected-error{{static method 'f' requires the types 'String' and 'Int' be equivalent}} -let _ = S<[W]>().instancef(W<[Int]>()) // expected-error{{instance method 'instancef' requires the types 'String' and 'Int' be equivalent}} +let _ = S<[W]>(W<[Int]>()) // expected-error{{initializer 'init(_:)' requires the types 'W' and 'W.Element>' be equivalent}} +let _ = S<[W]>.f(W<[Int]>()) // expected-error{{static method 'f' requires the types 'W' and 'W.Element>' be equivalent}} +let _ = S<[W]>().instancef(W<[Int]>()) // expected-error{{instance method 'instancef' requires the types 'W' and 'W.Element>' be equivalent}} // Archetypes requirement failure func genericFunc(_ c2: W, c1: C1.Type) where C1.Element == W { - let _ = S<[W]>(W()) // expected-error{{initializer 'init(_:)' requires the types 'C1' and 'C2.Element' be equivalent}} + let _ = S<[W]>(W()) // expected-error{{initializer 'init(_:)' requires the types 'W' and 'W' be equivalent}} }