Skip to content

[Completion] Fix Sendable KeyPath dynamic member subscripts #77691

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions lib/IDE/CompletionLookup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -643,8 +643,7 @@ Type CompletionLookup::getTypeOfMember(const ValueDecl *VD,

// If the keyPath result type has type parameters, that might affect the
// subscript result type.
auto keyPathResultTy =
getResultTypeOfKeypathDynamicMember(SD)->mapTypeOutOfContext();
auto keyPathResultTy = getResultTypeOfKeypathDynamicMember(SD);
if (keyPathResultTy->hasTypeParameter()) {
auto keyPathRootTy = getRootTypeOfKeypathDynamicMember(SD).subst(
QueryTypeSubstitutionMap{subs},
Expand Down
11 changes: 4 additions & 7 deletions lib/Sema/IDETypeCheckingRequests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "swift/AST/ASTPrinter.h"
#include "swift/AST/ConformanceLookup.h"
#include "swift/AST/Decl.h"
#include "swift/AST/ExistentialLayout.h"
#include "swift/AST/NameLookup.h"
#include "swift/Basic/Assertions.h"
#include "swift/Basic/SourceManager.h"
Expand Down Expand Up @@ -257,15 +258,11 @@ TypeRelationCheckRequest::evaluate(Evaluator &evaluator,
TypePair
RootAndResultTypeOfKeypathDynamicMemberRequest::evaluate(Evaluator &evaluator,
SubscriptDecl *subscript) const {
if (!isValidKeyPathDynamicMemberLookup(subscript))
return TypePair();

const auto *param = subscript->getIndices()->get(0);
auto keyPathType = param->getTypeInContext()->getAs<BoundGenericType>();
auto keyPathType = getKeyPathTypeForDynamicMemberLookup(subscript);
if (!keyPathType)
return TypePair();

auto genericArgs = keyPathType->getGenericArgs();
assert(!genericArgs.empty() && genericArgs.size() == 2 &&
"invalid keypath dynamic member");
assert(genericArgs.size() == 2 && "invalid keypath dynamic member");
return TypePair(genericArgs[0], genericArgs[1]);
}
25 changes: 17 additions & 8 deletions lib/Sema/TypeCheckAttr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1969,12 +1969,13 @@ bool swift::isValidStringDynamicMemberLookup(SubscriptDecl *decl,
paramType, KnownProtocolKind::ExpressibleByStringLiteral);
}

bool swift::isValidKeyPathDynamicMemberLookup(SubscriptDecl *decl,
bool ignoreLabel) {
BoundGenericType *
swift::getKeyPathTypeForDynamicMemberLookup(SubscriptDecl *decl,
bool ignoreLabel) {
auto &ctx = decl->getASTContext();
if (!hasSingleNonVariadicParam(decl, ctx.Id_dynamicMember,
ignoreLabel))
return false;
return nullptr;

auto paramTy = decl->getIndices()->get(0)->getInterfaceType();

Expand All @@ -1994,17 +1995,25 @@ bool swift::isValidKeyPathDynamicMemberLookup(SubscriptDecl *decl,

return false;
})) {
return false;
return nullptr;
}

paramTy = layout.getSuperclass();
if (!paramTy)
return false;
return nullptr;
}

return paramTy->isKeyPath() ||
paramTy->isWritableKeyPath() ||
paramTy->isReferenceWritableKeyPath();
if (!paramTy->isKeyPath() &&
!paramTy->isWritableKeyPath() &&
!paramTy->isReferenceWritableKeyPath()) {
return nullptr;
}
return paramTy->getAs<BoundGenericType>();
}

bool swift::isValidKeyPathDynamicMemberLookup(SubscriptDecl *decl,
bool ignoreLabel) {
return bool(getKeyPathTypeForDynamicMemberLookup(decl, ignoreLabel));
}

/// The @dynamicMemberLookup attribute is only allowed on types that have at
Expand Down
12 changes: 12 additions & 0 deletions lib/Sema/TypeChecker.h
Original file line number Diff line number Diff line change
Expand Up @@ -1271,6 +1271,18 @@ bool isValidDynamicMemberLookupSubscript(SubscriptDecl *decl,
bool isValidStringDynamicMemberLookup(SubscriptDecl *decl,
bool ignoreLabel = false);

/// Returns the KeyPath parameter type for a valid implementation of
/// the `subscript(dynamicMember: {Writable}KeyPath<...>)` requirement for
/// @dynamicMemberLookup.
/// The method is given to be defined as `subscript(dynamicMember:)` which
/// takes a single non-variadic parameter of `{Writable}KeyPath<T, U>` type.
///
/// Returns null if the given subscript is not a valid dynamic member lookup
/// implementation.
BoundGenericType *
getKeyPathTypeForDynamicMemberLookup(SubscriptDecl *decl,
bool ignoreLabel = false);

/// Returns true if the given subscript method is an valid implementation of
/// the `subscript(dynamicMember: {Writable}KeyPath<...>)` requirement for
/// @dynamicMemberLookup.
Expand Down
43 changes: 16 additions & 27 deletions test/IDE/complete_keypath_member_lookup.swift
Original file line number Diff line number Diff line change
@@ -1,29 +1,4 @@
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=testMembersPostfix1 | %FileCheck %s -check-prefix=testMembersPostfix1
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=testMembersDot1 | %FileCheck %s -check-prefix=testMembersDot1
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=testMembersDot2 | %FileCheck %s -check-prefix=testMembersDot2
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=testMultipleSubscript1 | %FileCheck %s -check-prefix=testMultipleSubscript1
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=testInherit1 | %FileCheck %s -check-prefix=testInherit1
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=testInherit2 | %FileCheck %s -check-prefix=testInherit2
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=testShadow1 | %FileCheck %s -check-prefix=testShadow1
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=testGeneric1 | %FileCheck %s -check-prefix=testGeneric1
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=testGenericUnderconstrained1 | %FileCheck %s -check-prefix=testGenericUnderconstrained1
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=testExistential1 | %FileCheck %s -check-prefix=testGenericUnderconstrained1
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=testExistential2 | %FileCheck %s -check-prefix=testExistential2
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=testProtocolConform1 | %FileCheck %s -check-prefix=testProtocolConform1
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=OnSelf1 | %FileCheck %s -check-prefix=OnSelf1
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=testSelfExtension1 | %FileCheck %s -check-prefix=testSelfExtension1
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=testInvalid1 | %FileCheck %s -check-prefix=testInvalid1
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=testInvalid2 | %FileCheck %s -check-prefix=testInvalid2
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=testInvalid3 | %FileCheck %s -check-prefix=testInvalid3
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=testInvalid4 | %FileCheck %s -check-prefix=testInvalid4
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=testGenericRoot1 | %FileCheck %s -check-prefix=testGenericRoot1
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=testGenericResult1 | %FileCheck %s -check-prefix=testGenericResult1
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=testAnyObjectRoot1 | %FileCheck %s -check-prefix=testAnyObjectRoot1
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=testNested1 | %FileCheck %s -check-prefix=testNested1
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=testNested2 | %FileCheck %s -check-prefix=testNested2
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=testCycle1 | %FileCheck %s -check-prefix=testCycle1
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=testCycle2 | %FileCheck %s -check-prefix=testCycle2
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=testSubscriptOnProtocolExt | %FileCheck %s -check-prefix=testSubscriptOnProtocolExt
// RUN: %batch-code-completion

struct Point {
var x: Int
Expand Down Expand Up @@ -154,7 +129,7 @@ func testGenericUnderconstrained1<G: P>(r: G) {
// testGenericUnderconstrained1-NOT: CurrNominal

func testExistential1(r: P) {
r.#^testExistential1^#
r.#^testExistential1?check=testGenericUnderconstrained1^#
}

@dynamicMemberLookup
Expand Down Expand Up @@ -381,3 +356,17 @@ func testSubscriptOnProtocolExtension(dyn: DynamicLookupConcrete) {
// testSubscriptOnProtocolExt: Decl[InstanceVar]/CurrNominal: x[#Int#];
// testSubscriptOnProtocolExt: Decl[InstanceVar]/CurrNominal: y[#Int#];
}

// https://github.com/swiftlang/swift/issues/77035
@dynamicMemberLookup
struct HasSendableKeyPath<T> {
subscript<U>(dynamicMember keyPath: KeyPath<T, U> & Sendable) -> HasSendableKeyPath<U> {
fatalError()
}
}

func testSendableKeyPath(_ x: HasSendableKeyPath<Point>) {
x.#^SENDABLE_KEYPATH_POINT^#
// SENDABLE_KEYPATH_POINT-DAG: Decl[InstanceVar]/CurrNominal: x[#HasSendableKeyPath<Int>#]; name=x
// SENDABLE_KEYPATH_POINT-DAG: Decl[InstanceVar]/CurrNominal: y[#HasSendableKeyPath<Int>#]; name=y
}