From e3ad92fbfbfd3989de25dfc3dfb07be90aff9639 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Wed, 3 Apr 2019 11:01:48 -0700 Subject: [PATCH] [ConstraintSystem] Allow to use keypath dynamic member lookup inside keypath expressions --- lib/Sema/CSApply.cpp | 123 +++++++++++++----- lib/Sema/ConstraintLocator.cpp | 12 ++ lib/Sema/ConstraintLocator.h | 4 + lib/Sema/ConstraintSystem.cpp | 2 +- .../keypath_dynamic_member_lookup.swift | 20 +++ test/attr/attr_dynamic_member_lookup.swift | 12 +- 6 files changed, 140 insertions(+), 33 deletions(-) diff --git a/lib/Sema/CSApply.cpp b/lib/Sema/CSApply.cpp index db858940ebe..e03a04e0be1 100644 --- a/lib/Sema/CSApply.cpp +++ b/lib/Sema/CSApply.cpp @@ -1555,32 +1555,81 @@ namespace { // based diagnostics could hold the reference to original AST. Expr *componentExpr = nullptr; auto *dotExpr = new (ctx) KeyPathDotExpr(dotLoc); + + // Determines whether this index is built to be used for + // one of the existing keypath components e.g. `\Lens<[Int]>.count` + // instead of a regular expression e.g. `lens[0]`. + bool forKeyPathComponent = false; + // Looks like keypath dynamic member lookup was used inside + // of a keypath expression e.g. `\Lens<[Int]>.count` where + // `count` is referenced using dynamic lookup. + if (auto *KPE = dyn_cast(anchor)) { + auto path = memberLoc->getPath(); + if (memberLoc->isSubscriptMemberRef()) + path = path.drop_back(); + + auto &componentIdx = path.back(); + assert(componentIdx.getKind() == ConstraintLocator::KeyPathComponent); + auto &origComponent = KPE->getComponents()[componentIdx.getValue()]; + + using ComponentKind = KeyPathExpr::Component::Kind; + if (origComponent.getKind() == ComponentKind::UnresolvedProperty) { + anchor = new (ctx) UnresolvedDotExpr( + dotExpr, dotLoc, origComponent.getUnresolvedDeclName(), + DeclNameLoc(origComponent.getLoc()), + /*Implicit=*/true); + } else if (origComponent.getKind() == + ComponentKind::UnresolvedSubscript) { + anchor = SubscriptExpr::create( + ctx, dotExpr, origComponent.getIndexExpr(), ConcreteDeclRef(), + /*implicit=*/true, AccessSemantics::Ordinary, + [&](const Expr *expr) { return simplifyType(cs.getType(expr)); }); + } else { + return nullptr; + } + + anchor->setType(simplifyType(overload.openedType)); + cs.cacheType(anchor); + forKeyPathComponent = true; + } + if (auto *UDE = dyn_cast(anchor)) { - componentExpr = new (ctx) UnresolvedDotExpr( - dotExpr, dotLoc, UDE->getName(), UDE->getNameLoc(), - /*Implicit=*/true); + componentExpr = + forKeyPathComponent + ? UDE + : new (ctx) UnresolvedDotExpr(dotExpr, dotLoc, UDE->getName(), + UDE->getNameLoc(), + /*Implicit=*/true); component = buildKeyPathPropertyComponent(overload, UDE->getLoc(), componentLoc); } else if (auto *SE = dyn_cast(anchor)) { - SmallVector arguments; - if (auto *TE = dyn_cast(SE->getIndex())) { - auto args = TE->getElements(); - arguments.append(args.begin(), args.end()); - } else { - arguments.push_back(SE->getIndex()->getSemanticsProvidingExpr()); + componentExpr = SE; + // If this is not for a keypath component, we have to copy + // original subscript expression because expression based + // diagnostics might have a reference to it, so it couldn't + // be modified. + if (!forKeyPathComponent) { + SmallVector arguments; + if (auto *TE = dyn_cast(SE->getIndex())) { + auto args = TE->getElements(); + arguments.append(args.begin(), args.end()); + } else { + arguments.push_back(SE->getIndex()->getSemanticsProvidingExpr()); + } + + Expr *trailingClosure = nullptr; + if (SE->hasTrailingClosure()) + trailingClosure = arguments.back(); + + componentExpr = SubscriptExpr::create( + ctx, dotExpr, SE->getStartLoc(), arguments, + SE->getArgumentLabels(), SE->getArgumentLabelLocs(), + SE->getEndLoc(), trailingClosure, + SE->hasDecl() ? SE->getDecl() : ConcreteDeclRef(), + /*implicit=*/true, SE->getAccessSemantics()); } - Expr *trailingClosure = nullptr; - if (SE->hasTrailingClosure()) - trailingClosure = arguments.back(); - - componentExpr = SubscriptExpr::create( - ctx, dotExpr, SE->getStartLoc(), arguments, SE->getArgumentLabels(), - SE->getArgumentLabelLocs(), SE->getEndLoc(), trailingClosure, - SE->hasDecl() ? SE->getDecl() : ConcreteDeclRef(), - /*implicit=*/true, SE->getAccessSemantics()); - component = buildKeyPathSubscriptComponent( overload, SE->getLoc(), SE->getIndex(), SE->getArgumentLabels(), componentLoc); @@ -4300,16 +4349,22 @@ namespace { ConstraintLocator::SubscriptMember); } + bool isDynamicMember = false; // If this is an unresolved link, make sure we resolved it. if (kind == KeyPathExpr::Component::Kind::UnresolvedProperty || kind == KeyPathExpr::Component::Kind::UnresolvedSubscript) { foundDecl = solution.getOverloadChoiceIfAvailable(locator); // Leave the component unresolved if the overload was not resolved. if (foundDecl) { + isDynamicMember = + foundDecl->choice.getKind() == + OverloadChoiceKind::DynamicMemberLookup || + foundDecl->choice.getKind() == + OverloadChoiceKind::KeyPathDynamicMemberLookup; + // If this was a @dynamicMemberLookup property, then we actually // form a subscript reference, so switch the kind. - if (foundDecl->choice.getKind() == - OverloadChoiceKind::DynamicMemberLookup) { + if (isDynamicMember) { kind = KeyPathExpr::Component::Kind::UnresolvedSubscript; } } @@ -4347,8 +4402,7 @@ namespace { } ArrayRef subscriptLabels; - if (foundDecl->choice.getKind() != - OverloadChoiceKind::DynamicMemberLookup) + if (!isDynamicMember) subscriptLabels = origComponent.getSubscriptLabels(); component = buildKeyPathSubscriptComponent( @@ -4535,18 +4589,25 @@ namespace { // openedType and origComponent to its full reference as if the user // wrote out the subscript manually. if (overload.choice.getKind() == - OverloadChoiceKind::DynamicMemberLookup) { + OverloadChoiceKind::DynamicMemberLookup || + overload.choice.getKind() == + OverloadChoiceKind::KeyPathDynamicMemberLookup) { overload.openedType = overload.openedFullType->castTo()->getResult(); - auto &ctx = cs.TC.Context; - auto fieldName = overload.choice.getName().getBaseIdentifier().str(); + labels = cs.getASTContext().Id_dynamicMember; - labels = ctx.Id_dynamicMember; - indexExpr = new (ctx) StringLiteralExpr(fieldName, componentLoc, - /*implicit*/ true); - (void)cs.TC.typeCheckExpression(indexExpr, dc); - cs.cacheExprTypes(indexExpr); + if (overload.choice.getKind() == + OverloadChoiceKind::KeyPathDynamicMemberLookup) { + auto fnType = overload.openedType->castTo(); + auto keyPathTy = simplifyType(fnType->getParams()[0].getPlainType()); + indexExpr = buildKeyPathDynamicMemberIndexExpr( + keyPathTy->castTo(), componentLoc, locator); + } else { + auto fieldName = overload.choice.getName().getBaseIdentifier().str(); + indexExpr = buildDynamicMemberLookupIndexExpr(fieldName, componentLoc, + dc, cs); + } } auto subscriptType = diff --git a/lib/Sema/ConstraintLocator.cpp b/lib/Sema/ConstraintLocator.cpp index cd2bf734777..3e11a9ed728 100644 --- a/lib/Sema/ConstraintLocator.cpp +++ b/lib/Sema/ConstraintLocator.cpp @@ -89,6 +89,18 @@ void ConstraintLocator::Profile(llvm::FoldingSetNodeID &id, Expr *anchor, } } +/// Determine whether given locator points to the subscript reference +/// e.g. `foo[0]` or `\Foo.[0]` +bool ConstraintLocator::isSubscriptMemberRef() const { + auto *anchor = getAnchor(); + auto path = getPath(); + + if (!anchor || path.empty()) + return false; + + return path.back().getKind() == ConstraintLocator::SubscriptMember; +} + void ConstraintLocator::dump(SourceManager *sm) { dump(sm, llvm::errs()); llvm::errs() << "\n"; diff --git a/lib/Sema/ConstraintLocator.h b/lib/Sema/ConstraintLocator.h index fb88465c7b7..23d836cd5ef 100644 --- a/lib/Sema/ConstraintLocator.h +++ b/lib/Sema/ConstraintLocator.h @@ -514,6 +514,10 @@ public: return (getSummaryFlags() & IsFunctionConversion); } + /// Determine whether given locator points to the subscript reference + /// e.g. `foo[0]` or `\Foo.[0]` + bool isSubscriptMemberRef() const; + /// Produce a profile of this locator, for use in a folding set. static void Profile(llvm::FoldingSetNodeID &id, Expr *anchor, ArrayRef path); diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index 90c78f52a85..9d2d566a824 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -1912,7 +1912,7 @@ void ConstraintSystem::resolveOverload(ConstraintLocator *locator, auto memberTy = createTypeVariable(keyPathLoc, TVO_CanBindToLValue); // Attempt to lookup a member with a give name in the root type and // assign result to the leaf type of the keypath. - bool isSubscriptRef = isa(locator->getAnchor()); + bool isSubscriptRef = locator->isSubscriptMemberRef(); DeclName memberName = isSubscriptRef ? DeclBaseName::createSubscript() : choice.getName(); diff --git a/test/Constraints/keypath_dynamic_member_lookup.swift b/test/Constraints/keypath_dynamic_member_lookup.swift index 404019e7da8..09e61ae17db 100644 --- a/test/Constraints/keypath_dynamic_member_lookup.swift +++ b/test/Constraints/keypath_dynamic_member_lookup.swift @@ -206,3 +206,23 @@ func test_direct_subscript_ref(_ lens: OverloadedLens) { // CHECK: function_ref @$s29keypath_dynamic_member_lookup14OverloadedLensV0B6Memberqd__s7KeyPathCyxqd__G_tcluig _ = lens.y } + +func test_keypath_dynamic_lookup_inside_keypath() { + // CHECK: keypath $KeyPath, (root $Point; stored_property #Point.x : $Int) + // CHECK-NEXT: keypath $KeyPath, Lens>, (root $Lens; gettable_property $Lens, id @$s29keypath_dynamic_member_lookup4LensV0B6MemberACyqd__Gs7KeyPathCyxqd__G_tcluig : {{.*}}) + _ = \Lens.x + // CHECK: keypath $WritableKeyPath, (root $Rectangle; stored_property #Rectangle.topLeft : $Point) + // CHECK-NEXT: keypath $WritableKeyPath, (root $Point; stored_property #Point.y : $Int) + // CHECK-NEXT: keypath $WritableKeyPath, Lens>, (root $Lens; settable_property $Lens, id @$s29keypath_dynamic_member_lookup4LensV0B6MemberACyqd__Gs15WritableKeyPathCyxqd__G_tcluig : {{.*}}) + _ = \Lens.topLeft.y + // CHECK: keypath $KeyPath, Int>, (root $Array; gettable_property $Int, id @$sSa5countSivg : {{.*}}) + // CHECK-NEXT: keypath $KeyPath>, Lens>, (root $Lens>; gettable_property $Lens, id @$s29keypath_dynamic_member_lookup4LensV0B6MemberACyqd__Gs7KeyPathCyxqd__G_tcluig : {{.*}}) + _ = \Lens<[Int]>.count + // CHECK: keypath $WritableKeyPath, Int>, (root $Array; settable_property $Int, id @$sSayxSicig : {{.*}}) + // CHECK-NEXT: keypath $WritableKeyPath>, Lens>, (root $Lens>; settable_property $Lens, id @$s29keypath_dynamic_member_lookup4LensV0B6MemberACyqd__Gs15WritableKeyPathCyxqd__G_tcluig : {{.*}}) + _ = \Lens<[Int]>.[0] + // CHECK: keypath $WritableKeyPath>, Array>, (root $Array>; settable_property $Array, id @$sSayxSicig : {{.*}}) + // CHECK-NEXT: keypath $KeyPath, Int>, (root $Array; gettable_property $Int, id @$sSa5countSivg : {{.*}}) + // CHECK-NEXT: keypath $KeyPath>>, Lens>, (root $Lens>>; settable_property $Lens>, id @$s29keypath_dynamic_member_lookup4LensV0B6MemberACyqd__Gs15WritableKeyPathCyxqd__G_tcluig : {{.*}}) + _ = \Lens<[[Int]]>.[0].count +} diff --git a/test/attr/attr_dynamic_member_lookup.swift b/test/attr/attr_dynamic_member_lookup.swift index 686de4d5755..7054d025f8e 100644 --- a/test/attr/attr_dynamic_member_lookup.swift +++ b/test/attr/attr_dynamic_member_lookup.swift @@ -404,7 +404,7 @@ struct Point { var x: Int let y: Int // expected-note 2 {{change 'let' to 'var' to make it mutable}} - private let z: Int = 0 // expected-note 7 {{declared here}} + private let z: Int = 0 // expected-note 9 {{declared here}} } struct Rectangle { @@ -445,6 +445,16 @@ _ = lens.bottomRight.x _ = lens.bottomRight.y _ = lens.bottomRight.z // expected-error {{'z' is inaccessible due to 'private' protection level}} +_ = \Lens.x +_ = \Lens.y +_ = \Lens.z // expected-error {{'z' is inaccessible due to 'private' protection level}} +_ = \Lens.topLeft.x +_ = \Lens.topLeft.y +_ = \Lens.topLeft.z // expected-error {{'z' is inaccessible due to 'private' protection level}} +_ = \Lens<[Int]>.count +_ = \Lens<[Int]>.[0] +_ = \Lens<[[Int]]>.[0].count + lens.topLeft = Lens(Point(x: 1, y: 2)) // Ok lens.bottomRight.x = Lens(11) // Ok lens.bottomRight.y = Lens(12) // expected-error {{cannot assign to property: 'y' is a 'let' constant}}