Skip to content
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

[SE-0252][TypeChecker] Keypath Dynamic Member Lookup #23436

Merged
merged 27 commits into from
Apr 5, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
b3cb1a1
[TypeChecker] Adjust dynamic member lookup to support keypaths
xedin Mar 17, 2019
f95d9b0
[TypeChecker] Add new type of overload choice to support keypath dyna…
xedin Mar 18, 2019
cde23ec
[ConstraintSystem] Implement keypath dynamic member lookup
xedin Mar 19, 2019
71e8148
[CSApply] Implement AST transformation for keypath dynamic member lookup
xedin Mar 20, 2019
7e07644
[CSRanking] Dynamic member look - rank keypath choice higher than str…
xedin Mar 20, 2019
5295bdc
[TypeChecker] NFC: Add tests for keypath dynamic member lookup
xedin Mar 20, 2019
fb30555
[ConstraintSystem] NFC: Extract check whether storage for keypath is …
xedin Mar 20, 2019
84167ed
[ConstraintSystem] Add a special locator for keypath dynamic member l…
xedin Mar 20, 2019
3e7a7a2
[CSApply] Extract keypath property resolution logic
xedin Mar 20, 2019
9eed013
[CSSolver] Don't generate implicit argument for keypath dynamic lookup
xedin Mar 21, 2019
2c82882
[ConstraintSystem] Move unviable keypath dynamic member check into `p…
xedin Mar 21, 2019
0235ff1
[Sema] Adjust invalid @dynamicMemberLookup to mention keypath parameter
xedin Mar 21, 2019
32e0c80
[TypeChecker] Add more tests for keypath dynamic member lookup
xedin Mar 21, 2019
4d225c7
[CSApply] Extract keypath subscript resolution logic
xedin Mar 22, 2019
d4bbcc1
[TypeChecker] Add subscript support keypath dynamic member lookup
xedin Mar 26, 2019
589283b
[ConstraintSystem] Teach sanitizer about dynamic member lookup expres…
xedin Mar 26, 2019
99c27d2
[CSApply] Avoid reusing expressions for keypath dynamic member lookup…
xedin Mar 26, 2019
675c5f4
[TypeChecker] Add subscript + trailing closure tests for keypath dyna…
xedin Mar 27, 2019
7032095
[TypeChecker] Tighten up validation of dynamic member decls
xedin Mar 27, 2019
41f6eb9
[ConstraintSystem] Don't introduce implicit keypath choice for keypat…
xedin Mar 27, 2019
d602cdf
[TypeChecker] NFC: Add more tests to keypath dynamic member lookup (t…
xedin Mar 27, 2019
dd810a7
[Diagnostics] Improve mutability diagnostics related to dynamic membe…
xedin Mar 28, 2019
8f88054
[TypeChecker] Add `ReferenceWritableKeyPath` support to keypath dynam…
xedin Mar 29, 2019
bc4d016
[TypeChecker] Fix dynamic member declaration validation to check labels
xedin Mar 30, 2019
e3ad92f
[ConstraintSystem] Allow to use keypath dynamic member lookup inside …
xedin Apr 3, 2019
2e8d163
[ConstraintSystem] Special case keypath dynamic member lookup in disj…
xedin Apr 3, 2019
b388d83
[ConstraintSystem] Make sure that keypath dynamic member lookup could…
xedin Apr 4, 2019
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
4 changes: 2 additions & 2 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -1057,8 +1057,8 @@ ERROR(missing_dynamic_callable_kwargs_method,none,

ERROR(invalid_dynamic_member_lookup_type,none,
"@dynamicMemberLookup attribute requires %0 to have a "
"'subscript(dynamicMember:)' method with an "
"'ExpressibleByStringLiteral' parameter", (Type))
"'subscript(dynamicMember:)' method that accepts either "
"'ExpressibleByStringLiteral' or a keypath", (Type))

ERROR(string_index_not_integer,none,
"String must not be indexed with %0, it has variable size elements",
Expand Down
557 changes: 381 additions & 176 deletions lib/Sema/CSApply.cpp

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions lib/Sema/CSDiag.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -811,6 +811,8 @@ void FailureDiagnosis::diagnoseUnviableLookupResults(
if (sameProblem) {
switch (firstProblem) {
case MemberLookupResult::UR_LabelMismatch:
case MemberLookupResult::UR_WritableKeyPathOnReadOnlyMember:
case MemberLookupResult::UR_ReferenceWritableKeyPathOnMutatingMember:
break;
case MemberLookupResult::UR_UnavailableInExistential:
diagnose(loc, diag::could_not_use_member_on_existential,
Expand Down
54 changes: 50 additions & 4 deletions lib/Sema/CSDiagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -776,7 +776,7 @@ bool MissingOptionalUnwrapFailure::diagnoseAsError() {
bool RValueTreatedAsLValueFailure::diagnoseAsError() {
Diag<StringRef> subElementDiagID;
Diag<Type> rvalueDiagID = diag::assignment_lhs_not_lvalue;
Expr *diagExpr = getLocator()->getAnchor();
Expr *diagExpr = getRawAnchor();
SourceLoc loc = diagExpr->getLoc();

if (auto assignExpr = dyn_cast<AssignExpr>(diagExpr)) {
Expand Down Expand Up @@ -854,10 +854,18 @@ bool RValueTreatedAsLValueFailure::diagnoseAsError() {
}
}

if (auto resolvedOverload = getResolvedOverload(getLocator()))
if (auto resolvedOverload = getResolvedOverload(getLocator())) {
if (resolvedOverload->Choice.getKind() ==
OverloadChoiceKind::DynamicMemberLookup)
subElementDiagID = diag::assignment_dynamic_property_has_immutable_base;

if (resolvedOverload->Choice.getKind() ==
OverloadChoiceKind::KeyPathDynamicMemberLookup) {
if (!getType(member->getBase())->hasLValueType())
subElementDiagID =
diag::assignment_dynamic_property_has_immutable_base;
}
}
} else if (auto sub = dyn_cast<SubscriptExpr>(diagExpr)) {
subElementDiagID = diag::assignment_subscript_has_immutable_base;
} else {
Expand Down Expand Up @@ -1202,7 +1210,7 @@ AssignmentFailure::resolveImmutableBase(Expr *expr) const {
if (!member) {
auto loc =
cs.getConstraintLocator(SE, ConstraintLocator::SubscriptMember);
member = dyn_cast_or_null<SubscriptDecl>(cs.findResolvedMemberRef(loc));
member = dyn_cast_or_null<SubscriptDecl>(getMemberRef(loc));
}

// If it isn't settable, return it.
Expand Down Expand Up @@ -1231,9 +1239,10 @@ AssignmentFailure::resolveImmutableBase(Expr *expr) const {
// If we found a decl for the UDE, check it.
auto loc = cs.getConstraintLocator(UDE, ConstraintLocator::Member);

auto *member = getMemberRef(loc);
// If we can resolve a member, we can determine whether it is settable in
// this context.
if (auto *member = cs.findResolvedMemberRef(loc)) {
if (member) {
auto *memberVD = dyn_cast<VarDecl>(member);

// If the member isn't a vardecl (e.g. its a funcdecl), or it isn't
Expand Down Expand Up @@ -1281,6 +1290,43 @@ AssignmentFailure::resolveImmutableBase(Expr *expr) const {
return {expr, nullptr};
}

ValueDecl *AssignmentFailure::getMemberRef(ConstraintLocator *locator) const {
auto member = getOverloadChoiceIfAvailable(locator);
if (!member || !member->choice.isDecl())
return nullptr;

auto *DC = getDC();
auto &TC = getTypeChecker();

auto *decl = member->choice.getDecl();
if (isa<SubscriptDecl>(decl) &&
isValidDynamicMemberLookupSubscript(cast<SubscriptDecl>(decl), DC, TC)) {
auto *subscript = cast<SubscriptDecl>(decl);
// If this is a keypath dynamic member lookup, we have to
// adjust the locator to find member referred by it.
if (isValidKeyPathDynamicMemberLookup(subscript, TC)) {
auto &cs = getConstraintSystem();
// Type has a following format:
// `(Self) -> (dynamicMember: {Writable}KeyPath<T, U>) -> U`
auto *fullType = member->openedFullType->castTo<FunctionType>();
auto *fnType = fullType->getResult()->castTo<FunctionType>();

auto paramTy = fnType->getParams()[0].getPlainType();
auto keyPath = paramTy->getAnyNominal();
auto memberLoc = cs.getConstraintLocator(
locator, LocatorPathElt::getKeyPathDynamicMember(keyPath));

auto memberRef = getOverloadChoiceIfAvailable(memberLoc);
return memberRef ? memberRef->choice.getDecl() : nullptr;
}

// If this is a string based dynamic lookup, there is no member declaration.
return nullptr;
}

return decl;
}

Diag<StringRef> AssignmentFailure::findDeclDiagonstic(ASTContext &ctx,
Expr *destExpr) {
if (isa<ApplyExpr>(destExpr) || isa<SelfApplyExpr>(destExpr))
Expand Down
4 changes: 4 additions & 0 deletions lib/Sema/CSDiagnostics.h
Original file line number Diff line number Diff line change
Expand Up @@ -647,6 +647,10 @@ class AssignmentFailure final : public FailureDiagnostic {
isLoadedLValue(ifExpr->getElseExpr());
return false;
}

/// Retrive an member reference associated with given member
/// looking through dynamic member lookup on the way.
ValueDecl *getMemberRef(ConstraintLocator *locator) const;
};

/// Intended to diagnose any possible contextual failure
Expand Down
34 changes: 33 additions & 1 deletion lib/Sema/CSGen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3274,6 +3274,38 @@ namespace {
expr = sanitizeArgumentList(expr);
}

// If this expression represents keypath based dynamic member
// lookup, let's convert it back to the original form of
// member or subscript reference.
if (auto *SE = dyn_cast<SubscriptExpr>(expr)) {
if (auto *TE = dyn_cast<TupleExpr>(SE->getIndex())) {
auto isImplicitKeyPathExpr = [](Expr *argExpr) -> bool {
if (auto *KP = dyn_cast<KeyPathExpr>(argExpr))
return KP->isImplicit();
return false;
};

if (TE->isImplicit() && TE->getNumElements() == 1 &&
TE->getElementName(0) == TC.Context.Id_dynamicMember &&
isImplicitKeyPathExpr(TE->getElement(0))) {
auto *keyPathExpr = cast<KeyPathExpr>(TE->getElement(0));
auto *componentExpr = keyPathExpr->getParsedPath();

if (auto *UDE = dyn_cast<UnresolvedDotExpr>(componentExpr)) {
UDE->setBase(SE->getBase());
return {true, UDE};
}

if (auto *subscript = dyn_cast<SubscriptExpr>(componentExpr)) {
subscript->setBase(SE->getBase());
return {true, subscript};
}

llvm_unreachable("unknown keypath component type");
}
}
}

// Now, we're ready to walk into sub expressions.
return {true, expr};
}
Expand Down Expand Up @@ -3898,4 +3930,4 @@ swift::getOriginalArgumentList(Expr *expr) {
}

return result;
}
}
16 changes: 16 additions & 0 deletions lib/Sema/CSRanking.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ static bool sameOverloadChoice(const OverloadChoice &x,
case OverloadChoiceKind::DeclViaBridge:
case OverloadChoiceKind::DeclViaUnwrappedOptional:
case OverloadChoiceKind::DynamicMemberLookup:
case OverloadChoiceKind::KeyPathDynamicMemberLookup:
return sameDecl(x.getDecl(), y.getDecl());

case OverloadChoiceKind::TupleIndex:
Expand Down Expand Up @@ -875,6 +876,20 @@ SolutionCompareResult ConstraintSystem::compareSolutions(
continue;
}

// Dynamic member lookup through a keypath is better than one using string
// because it carries more type information.
if (choice1.getKind() == OverloadChoiceKind::KeyPathDynamicMemberLookup &&
choice2.getKind() == OverloadChoiceKind::DynamicMemberLookup) {
score1 += weight;
continue;
}

if (choice1.getKind() == OverloadChoiceKind::DynamicMemberLookup &&
choice2.getKind() == OverloadChoiceKind::KeyPathDynamicMemberLookup) {
score2 += weight;
continue;
}

continue;
}

Expand All @@ -893,6 +908,7 @@ SolutionCompareResult ConstraintSystem::compareSolutions(
case OverloadChoiceKind::DeclViaBridge:
case OverloadChoiceKind::DeclViaUnwrappedOptional:
case OverloadChoiceKind::DynamicMemberLookup:
case OverloadChoiceKind::KeyPathDynamicMemberLookup:
break;
}

Expand Down
Loading