From 61d0486d12af42fa9cd344467fcb35339fb388a1 Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Fri, 15 Aug 2025 21:30:22 +0100 Subject: [PATCH 1/2] [CS] NFC: Factor out `hasKeyPathComponent` --- lib/Sema/ConstraintLocator.cpp | 41 ++++++++++++++-------------------- 1 file changed, 17 insertions(+), 24 deletions(-) diff --git a/lib/Sema/ConstraintLocator.cpp b/lib/Sema/ConstraintLocator.cpp index fbe86696bd27a..0ec690e88a190 100644 --- a/lib/Sema/ConstraintLocator.cpp +++ b/lib/Sema/ConstraintLocator.cpp @@ -596,41 +596,34 @@ bool ConstraintLocator::isResultOfKeyPathDynamicMemberLookup() const { }); } -bool ConstraintLocator::isKeyPathSubscriptComponent() const { - auto *anchor = getAsExpr(getAnchor()); - auto *KPE = dyn_cast_or_null(anchor); +static bool hasKeyPathComponent( + const ConstraintLocator *loc, + llvm::function_ref predicate) { + auto *KPE = getAsExpr(loc->getAnchor()); if (!KPE) return false; - using ComponentKind = KeyPathExpr::Component::Kind; - return llvm::any_of(getPath(), [&](const LocatorPathElt &elt) { + return llvm::any_of(loc->getPath(), [&](const LocatorPathElt &elt) { auto keyPathElt = elt.getAs(); if (!keyPathElt) return false; - auto index = keyPathElt->getIndex(); - auto &component = KPE->getComponents()[index]; - return component.getKind() == ComponentKind::Subscript || - component.getKind() == ComponentKind::UnresolvedSubscript; + auto &component = KPE->getComponents()[keyPathElt->getIndex()]; + return predicate(component.getKind()); }); } -bool ConstraintLocator::isKeyPathMemberComponent() const { - auto *anchor = getAsExpr(getAnchor()); - auto *KPE = dyn_cast_or_null(anchor); - if (!KPE) - return false; - - using ComponentKind = KeyPathExpr::Component::Kind; - return llvm::any_of(getPath(), [&](const LocatorPathElt &elt) { - auto keyPathElt = elt.getAs(); - if (!keyPathElt) - return false; +bool ConstraintLocator::isKeyPathSubscriptComponent() const { + return hasKeyPathComponent(this, [](auto kind) { + return kind == KeyPathExpr::Component::Kind::Subscript || + kind == KeyPathExpr::Component::Kind::UnresolvedSubscript; + }); +} - auto index = keyPathElt->getIndex(); - auto &component = KPE->getComponents()[index]; - return component.getKind() == ComponentKind::Member || - component.getKind() == ComponentKind::UnresolvedMember; +bool ConstraintLocator::isKeyPathMemberComponent() const { + return hasKeyPathComponent(this, [](auto kind) { + return kind == KeyPathExpr::Component::Kind::Member || + kind == KeyPathExpr::Component::Kind::UnresolvedMember; }); } From 4423399fce35d813a2607048fe1fb6afaa060d00 Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Fri, 15 Aug 2025 21:30:22 +0100 Subject: [PATCH 2/2] [CS] Use apply component locator for `verifyThatArgumentIsHashable` For a method key path use the locator for the apply itself rather than the member, ensuring we handle invalid cases where the apply is the first component, and providing more accurate location info. --- include/swift/Sema/ConstraintLocator.h | 4 ++++ lib/Sema/CSDiagnostics.cpp | 5 +++-- lib/Sema/CSDiagnostics.h | 7 ++++--- lib/Sema/CSGen.cpp | 8 ++------ lib/Sema/CSSimplify.cpp | 7 ++++--- lib/Sema/ConstraintLocator.cpp | 7 +++++++ .../3c95e32711989555.swift | 2 +- .../5f26ba86c56cc88.swift | 2 +- 8 files changed, 26 insertions(+), 16 deletions(-) rename validation-test/{compiler_crashers_2 => compiler_crashers_2_fixed}/3c95e32711989555.swift (88%) rename validation-test/{compiler_crashers_2 => compiler_crashers_2_fixed}/5f26ba86c56cc88.swift (89%) diff --git a/include/swift/Sema/ConstraintLocator.h b/include/swift/Sema/ConstraintLocator.h index d5f099e3b3518..e1ed7f2d20a4c 100644 --- a/include/swift/Sema/ConstraintLocator.h +++ b/include/swift/Sema/ConstraintLocator.h @@ -260,6 +260,10 @@ class ConstraintLocator : public llvm::FoldingSetNode { /// of the key path at some index. bool isKeyPathMemberComponent() const; + /// Determine whether this locator points to an apply component of the key + /// path at some index. + bool isKeyPathApplyComponent() const; + /// Determine whether this locator points to the member found /// via key path dynamic member lookup. bool isForKeyPathDynamicMemberLookup() const; diff --git a/lib/Sema/CSDiagnostics.cpp b/lib/Sema/CSDiagnostics.cpp index 525c8ec622840..b057df3bda878 100644 --- a/lib/Sema/CSDiagnostics.cpp +++ b/lib/Sema/CSDiagnostics.cpp @@ -6405,7 +6405,8 @@ SourceLoc KeyPathSubscriptIndexHashableFailure::getLoc() const { auto *locator = getLocator(); if (locator->isKeyPathSubscriptComponent() || - locator->isKeyPathMemberComponent()) { + locator->isKeyPathMemberComponent() || + locator->isKeyPathApplyComponent()) { auto *KPE = castToExpr(getAnchor()); if (auto kpElt = locator->findFirst()) return KPE->getComponents()[kpElt->getIndex()].getLoc(); @@ -6417,7 +6418,7 @@ SourceLoc KeyPathSubscriptIndexHashableFailure::getLoc() const { bool KeyPathSubscriptIndexHashableFailure::diagnoseAsError() { auto *locator = getLocator(); emitDiagnostic(diag::expr_keypath_arg_or_index_not_hashable, - !locator->isKeyPathMemberComponent(), + !locator->isKeyPathApplyComponent(), resolveType(NonConformingType)); return true; } diff --git a/lib/Sema/CSDiagnostics.h b/lib/Sema/CSDiagnostics.h index 5c65d8a23c067..5ed4449a89f9d 100644 --- a/lib/Sema/CSDiagnostics.h +++ b/lib/Sema/CSDiagnostics.h @@ -1707,9 +1707,10 @@ class KeyPathSubscriptIndexHashableFailure final : public FailureDiagnostic { KeyPathSubscriptIndexHashableFailure(const Solution &solution, Type type, ConstraintLocator *locator) : FailureDiagnostic(solution, locator), NonConformingType(type) { - assert((locator->isResultOfKeyPathDynamicMemberLookup() || - locator->isKeyPathSubscriptComponent()) || - locator->isKeyPathMemberComponent()); + assert(locator->isResultOfKeyPathDynamicMemberLookup() || + locator->isKeyPathSubscriptComponent() || + locator->isKeyPathMemberComponent() || + locator->isKeyPathApplyComponent()); } SourceLoc getLoc() const override; diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp index d1791809bc69b..f6d348dea9c02 100644 --- a/lib/Sema/CSGen.cpp +++ b/lib/Sema/CSGen.cpp @@ -1198,7 +1198,6 @@ namespace { /// component kind. Type addApplyConstraints( Expr *anchor, Type memberTy, ArgumentList *argList, - ConstraintLocator *memberComponentLoc, ConstraintLocator *applyComponentLoc, SmallVectorImpl *addedTypeVars = nullptr) { // Locators used in this expression. @@ -1225,7 +1224,7 @@ namespace { for (auto index : indices(params)) { const auto ¶m = params[index]; CS.verifyThatArgumentIsHashable(index, param.getParameterType(), - memberComponentLoc, loc); + applyComponentLoc, loc); } // Add the constraint that the index expression's type be convertible @@ -3895,11 +3894,8 @@ namespace { case KeyPathExpr::Component::Kind::UnresolvedApply: case KeyPathExpr::Component::Kind::Apply: { - auto prevMemberLocator = CS.getConstraintLocator( - locator, LocatorPathElt::KeyPathComponent(i - 1)); base = addApplyConstraints(E, base, component.getArgs(), - prevMemberLocator, memberLocator, - &componentTypeVars); + memberLocator, &componentTypeVars); break; } diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index d4d24b90b5573..e67332dda9e8d 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -9221,9 +9221,10 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyConformsToConstraint( // If this is an implicit Hashable conformance check generated for each // index argument of the keypath subscript component, we could just treat // it as though it conforms. - if ((loc->isResultOfKeyPathDynamicMemberLookup() || - loc->isKeyPathSubscriptComponent()) || - loc->isKeyPathMemberComponent()) { + if (loc->isResultOfKeyPathDynamicMemberLookup() || + loc->isKeyPathSubscriptComponent() || + loc->isKeyPathMemberComponent() || + loc->isKeyPathApplyComponent()) { if (protocol == getASTContext().getProtocol(KnownProtocolKind::Hashable)) { auto *fix = diff --git a/lib/Sema/ConstraintLocator.cpp b/lib/Sema/ConstraintLocator.cpp index 0ec690e88a190..dd324fd379612 100644 --- a/lib/Sema/ConstraintLocator.cpp +++ b/lib/Sema/ConstraintLocator.cpp @@ -627,6 +627,13 @@ bool ConstraintLocator::isKeyPathMemberComponent() const { }); } +bool ConstraintLocator::isKeyPathApplyComponent() const { + return hasKeyPathComponent(this, [](auto kind) { + return kind == KeyPathExpr::Component::Kind::Apply || + kind == KeyPathExpr::Component::Kind::UnresolvedApply; + }); +} + bool ConstraintLocator::isForKeyPathDynamicMemberLookup() const { auto path = getPath(); return !path.empty() && path.back().isKeyPathDynamicMember(); diff --git a/validation-test/compiler_crashers_2/3c95e32711989555.swift b/validation-test/compiler_crashers_2_fixed/3c95e32711989555.swift similarity index 88% rename from validation-test/compiler_crashers_2/3c95e32711989555.swift rename to validation-test/compiler_crashers_2_fixed/3c95e32711989555.swift index cef6f0bebb23e..c268126937154 100644 --- a/validation-test/compiler_crashers_2/3c95e32711989555.swift +++ b/validation-test/compiler_crashers_2_fixed/3c95e32711989555.swift @@ -1,3 +1,3 @@ // {"kind":"typecheck","signature":"swift::constraints::ConstraintSystem::simplifyConformsToConstraint(swift::Type, swift::ProtocolDecl*, swift::constraints::ConstraintKind, swift::constraints::ConstraintLocatorBuilder, swift::optionset::OptionSet)","signatureAssert":"Assertion failed: (Index < Length && \"Invalid index!\"), function operator[]"} -// RUN: not --crash %target-swift-frontend -typecheck %s +// RUN: not %target-swift-frontend -typecheck %s \_(error) diff --git a/validation-test/compiler_crashers_2/5f26ba86c56cc88.swift b/validation-test/compiler_crashers_2_fixed/5f26ba86c56cc88.swift similarity index 89% rename from validation-test/compiler_crashers_2/5f26ba86c56cc88.swift rename to validation-test/compiler_crashers_2_fixed/5f26ba86c56cc88.swift index 9ad200c184e2f..047b1ddc71b4c 100644 --- a/validation-test/compiler_crashers_2/5f26ba86c56cc88.swift +++ b/validation-test/compiler_crashers_2_fixed/5f26ba86c56cc88.swift @@ -1,4 +1,4 @@ // {"kind":"typecheck","signature":"swift::constraints::ConstraintSystem::getCalleeLocator(swift::constraints::ConstraintLocator*, bool, llvm::function_ref, llvm::function_ref, llvm::function_ref (swift::constraints::ConstraintLocator*)>)","signatureAssert":"Assertion failed: (Index < Length && \"Invalid index!\"), function operator[]"} -// RUN: not --crash %target-swift-frontend -typecheck %s +// RUN: not %target-swift-frontend -typecheck %s struct a func b(c : [Int]) { \ a(c.map{})