Skip to content

[ConstraintSystem] Detect and fix invalid refs in dynamic key path me… #24454

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 1 commit into from
May 3, 2019
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
12 changes: 7 additions & 5 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -473,13 +473,15 @@ ERROR(expr_keypath_type_of_property,none,
ERROR(expr_keypath_generic_type,none,
"key path cannot refer to generic type %0", (DeclName))
ERROR(expr_keypath_not_property,none,
"key path cannot refer to %0 %1", (DescriptiveDeclKind, DeclName))
"%select{key path|dynamic key path member lookup}2 cannot refer to %0 %1",
(DescriptiveDeclKind, DeclName, bool))
ERROR(expr_keypath_mutating_getter,none,
"key path cannot refer to %0, which has a mutating getter",
(DeclName))
"%select{key path|dynamic key path member lookup}1 cannot refer to %0, "
"which has a mutating getter",
(DeclName, bool))
ERROR(expr_keypath_static_member,none,
"key path cannot refer to static member %0",
(DeclName))
"%select{key path|dynamic key path member lookup}1 cannot refer to static member %0",
(DeclName, bool))
ERROR(expr_keypath_empty,none,
"empty key path does not refer to a property", ())
ERROR(expr_unsupported_objc_key_path_component,none,
Expand Down
8 changes: 5 additions & 3 deletions lib/Sema/CSDiagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2532,17 +2532,19 @@ SourceLoc InvalidMemberRefInKeyPath::getLoc() const {
}

bool InvalidStaticMemberRefInKeyPath::diagnoseAsError() {
emitDiagnostic(getLoc(), diag::expr_keypath_static_member, getName());
emitDiagnostic(getLoc(), diag::expr_keypath_static_member, getName(),
isForKeyPathDynamicMemberLookup());
return true;
}

bool InvalidMemberWithMutatingGetterInKeyPath::diagnoseAsError() {
emitDiagnostic(getLoc(), diag::expr_keypath_mutating_getter, getName());
emitDiagnostic(getLoc(), diag::expr_keypath_mutating_getter, getName(),
isForKeyPathDynamicMemberLookup());
return true;
}

bool InvalidMethodRefInKeyPath::diagnoseAsError() {
emitDiagnostic(getLoc(), diag::expr_keypath_not_property, getKind(),
getName());
getName(), isForKeyPathDynamicMemberLookup());
return true;
}
7 changes: 6 additions & 1 deletion lib/Sema/CSDiagnostics.h
Original file line number Diff line number Diff line change
Expand Up @@ -1039,7 +1039,8 @@ class InvalidMemberRefInKeyPath : public FailureDiagnostic {
ConstraintLocator *locator)
: FailureDiagnostic(root, cs, locator), Member(member) {
assert(member->hasName());
assert(locator->isForKeyPathComponent());
assert(locator->isForKeyPathComponent() ||
locator->isForKeyPathDynamicMemberLookup());
}

DescriptiveDeclKind getKind() const { return Member->getDescriptiveKind(); }
Expand All @@ -1051,6 +1052,10 @@ class InvalidMemberRefInKeyPath : public FailureDiagnostic {
protected:
/// Compute location of the failure for diagnostic.
SourceLoc getLoc() const;

bool isForKeyPathDynamicMemberLookup() const {
return getLocator()->isForKeyPathDynamicMemberLookup();
}
};

/// Diagnose an attempt to reference a static member as a key path component
Expand Down
18 changes: 10 additions & 8 deletions lib/Sema/CSSimplify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3710,11 +3710,6 @@ bool swift::hasDynamicMemberLookupAttribute(Type type) {
return ::hasDynamicMemberLookupAttribute(type, DynamicMemberLookupCache);
}

static bool isKeyPathDynamicMemberLookup(ConstraintLocator *locator) {
auto path = locator ? locator->getPath() : None;
return !path.empty() && path.back().isKeyPathDynamicMember();
}

/// Given a ValueMember, UnresolvedValueMember, or TypeMember constraint,
/// perform a lookup into the specified base type to find a candidate list.
/// The list returned includes the viable candidates as well as the unviable
Expand Down Expand Up @@ -3753,7 +3748,7 @@ performMemberLookup(ConstraintKind constraintKind, DeclName memberName,
// is a single argument with `keypath:` label or `\.` syntax is used.
if (memberName.isSimpleName() &&
memberName.getBaseName().getKind() == DeclBaseName::Kind::Subscript &&
!isKeyPathDynamicMemberLookup(memberLocator)) {
!(memberLocator && memberLocator->isForKeyPathDynamicMemberLookup())) {
if (baseTy->isAnyObject()) {
result.addUnviable(
OverloadChoice(baseTy, OverloadChoiceKind::KeyPathApplication),
Expand Down Expand Up @@ -4019,7 +4014,7 @@ performMemberLookup(ConstraintKind constraintKind, DeclName memberName,
// based dynamic member lookup. Since it's unknown upfront
// what kind of declaration lookup is going to find, let's
// double check here that given keypath is appropriate for it.
if (isKeyPathDynamicMemberLookup(memberLocator)) {
if (memberLocator && memberLocator->isForKeyPathDynamicMemberLookup()) {
auto path = memberLocator->getPath();
auto *keyPath = path.back().getKeyPath();
if (auto *storage = dyn_cast<AbstractStorageDecl>(decl)) {
Expand Down Expand Up @@ -4416,10 +4411,17 @@ fixMemberRef(ConstraintSystem &cs, Type baseTy,
// Not all of the choices handled here are going
// to refer to a declaration.
if (choice.isDecl()) {
if (auto *CD = dyn_cast<ConstructorDecl>(choice.getDecl())) {
auto *decl = choice.getDecl();

if (auto *CD = dyn_cast<ConstructorDecl>(decl)) {
if (auto *fix = validateInitializerRef(cs, CD, locator))
return fix;
}

if (locator->isForKeyPathDynamicMemberLookup()) {
if (auto *fix = AllowInvalidRefInKeyPath::forRef(cs, decl, locator))
return fix;
}
}

if (reason) {
Expand Down
5 changes: 5 additions & 0 deletions lib/Sema/ConstraintLocator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,11 @@ bool ConstraintLocator::isKeyPathSubscriptComponent() const {
});
}

bool ConstraintLocator::isForKeyPathDynamicMemberLookup() const {
auto path = getPath();
return !path.empty() && path.back().isKeyPathDynamicMember();
}

bool ConstraintLocator::isForKeyPathComponent() const {
return llvm::any_of(getPath(), [&](const LocatorPathElt &elt) {
return elt.isKeyPathComponent();
Expand Down
4 changes: 4 additions & 0 deletions lib/Sema/ConstraintLocator.h
Original file line number Diff line number Diff line change
Expand Up @@ -560,6 +560,10 @@ class ConstraintLocator : public llvm::FoldingSetNode {
/// of the key path at some index.
bool isKeyPathSubscriptComponent() const;

/// Determine whether this locator points to the member found
/// via key path dynamic member lookup.
bool isForKeyPathDynamicMemberLookup() const;

/// Determine whether this locator points to one of the key path
/// components.
bool isForKeyPathComponent() const;
Expand Down
3 changes: 2 additions & 1 deletion lib/Sema/TypeCheckExprObjC.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,8 @@ Optional<Type> TypeChecker::checkObjCKeyPathExpr(DeclContext *dc,

// Declarations that cannot be part of a key-path.
diagnose(componentNameLoc, diag::expr_keypath_not_property,
found->getDescriptiveKind(), found->getFullName());
found->getDescriptiveKind(), found->getFullName(),
/*isForDynamicKeyPathMemberLookup=*/false);
isInvalid = true;
break;
}
Expand Down
21 changes: 21 additions & 0 deletions test/attr/attr_dynamic_member_lookup.swift
Original file line number Diff line number Diff line change
Expand Up @@ -667,3 +667,24 @@ func test_chain_of_recursive_lookups(_ lens: Lens<Lens<Lens<Point>>>) {
_ = \Lens<Lens<Point>>.x
_ = \Lens<Lens<Point>>.obj.x
}

// KeyPath Dynamic Member Lookup can't refer to methods, mutating setters and static members
// because of the KeyPath limitations
func invalid_refs_through_dynamic_lookup() {
struct S {
static var foo: Int = 42
func bar() -> Q { return Q() }
static func baz(_: String) -> Int { return 0 }
}

struct Q {
var faz: Int = 0
}

func test(_ lens: A<S>) {
_ = lens.foo // expected-error {{dynamic key path member lookup cannot refer to static member 'foo'}}
_ = lens.bar() // expected-error {{dynamic key path member lookup cannot refer to instance method 'bar()'}}
_ = lens.bar().faz + 1 // expected-error {{dynamic key path member lookup cannot refer to instance method 'bar()'}}
_ = lens.baz("hello") // expected-error {{dynamic key path member lookup cannot refer to static method 'baz'}}
}
}