Skip to content

Commit d64b8ec

Browse files
ahoppenahoppen
committed
[CodeCompletion] Migrate key path completion to be solver based
This commit essentially consistes of the following steps: - Add a new code completion key path component that represents the code completion token inside a key path. Previously, the key path would have an invalid component at the end if it contained a code completion token. - When type checking the key path, model the code completion token’s result type by a new type variable that is unrelated to the previous components (because the code completion token might resolve to anything). - Since the code completion token is now properly modelled in the constraint system, we can use the solver based code completion implementation and inspect any solution determined by the constraint solver. The base type for code completion is now the result type of the key path component that preceeds the code completion component. This resolves bugs where code completion was not working correctly if the key path’s type had a generic base or result type. It’s also nice to have moved another completion type over to the solver-based implementation. Resolves rdar://78779234 [SR-14685] and rdar://78779335 [SR-14703]
1 parent 38a8b00 commit d64b8ec

19 files changed

+238
-74
lines changed

‎include/swift/AST/Expr.h

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5462,6 +5462,7 @@ class KeyPathExpr : public Expr {
54625462
/// - a resolved ValueDecl, referring to
54635463
/// - a subscript index expression, which may or may not be resolved
54645464
/// - an optional chaining, forcing, or wrapping component
5465+
/// - a code completion token
54655466
class Component {
54665467
public:
54675468
enum class Kind: unsigned {
@@ -5476,6 +5477,7 @@ class KeyPathExpr : public Expr {
54765477
Identity,
54775478
TupleElement,
54785479
DictionaryKey,
5480+
CodeCompletion,
54795481
};
54805482

54815483
private:
@@ -5651,8 +5653,12 @@ class KeyPathExpr : public Expr {
56515653
SourceLoc loc) {
56525654
return Component(fieldNumber, elementType, loc);
56535655
}
5654-
5655-
5656+
5657+
static Component forCodeCompletion(SourceLoc Loc) {
5658+
return Component(nullptr, {}, nullptr, {}, {}, Kind::CodeCompletion,
5659+
Type(), Loc);
5660+
}
5661+
56565662
SourceLoc getLoc() const {
56575663
return Loc;
56585664
}
@@ -5690,6 +5696,7 @@ class KeyPathExpr : public Expr {
56905696
case Kind::UnresolvedSubscript:
56915697
case Kind::UnresolvedProperty:
56925698
case Kind::Invalid:
5699+
case Kind::CodeCompletion:
56935700
return false;
56945701
}
56955702
llvm_unreachable("unhandled kind");
@@ -5710,6 +5717,7 @@ class KeyPathExpr : public Expr {
57105717
case Kind::Identity:
57115718
case Kind::TupleElement:
57125719
case Kind::DictionaryKey:
5720+
case Kind::CodeCompletion:
57135721
return nullptr;
57145722
}
57155723
llvm_unreachable("unhandled kind");
@@ -5730,6 +5738,7 @@ class KeyPathExpr : public Expr {
57305738
case Kind::Identity:
57315739
case Kind::TupleElement:
57325740
case Kind::DictionaryKey:
5741+
case Kind::CodeCompletion:
57335742
llvm_unreachable("no subscript labels for this kind");
57345743
}
57355744
llvm_unreachable("unhandled kind");
@@ -5753,6 +5762,7 @@ class KeyPathExpr : public Expr {
57535762
case Kind::Identity:
57545763
case Kind::TupleElement:
57555764
case Kind::DictionaryKey:
5765+
case Kind::CodeCompletion:
57565766
return {};
57575767
}
57585768
llvm_unreachable("unhandled kind");
@@ -5776,6 +5786,7 @@ class KeyPathExpr : public Expr {
57765786
case Kind::Property:
57775787
case Kind::Identity:
57785788
case Kind::TupleElement:
5789+
case Kind::CodeCompletion:
57795790
llvm_unreachable("no unresolved name for this kind");
57805791
}
57815792
llvm_unreachable("unhandled kind");
@@ -5796,6 +5807,7 @@ class KeyPathExpr : public Expr {
57965807
case Kind::Identity:
57975808
case Kind::TupleElement:
57985809
case Kind::DictionaryKey:
5810+
case Kind::CodeCompletion:
57995811
return false;
58005812
}
58015813
llvm_unreachable("unhandled kind");
@@ -5816,6 +5828,7 @@ class KeyPathExpr : public Expr {
58165828
case Kind::Identity:
58175829
case Kind::TupleElement:
58185830
case Kind::DictionaryKey:
5831+
case Kind::CodeCompletion:
58195832
llvm_unreachable("no decl ref for this kind");
58205833
}
58215834
llvm_unreachable("unhandled kind");
@@ -5836,6 +5849,7 @@ class KeyPathExpr : public Expr {
58365849
case Kind::Property:
58375850
case Kind::Subscript:
58385851
case Kind::DictionaryKey:
5852+
case Kind::CodeCompletion:
58395853
llvm_unreachable("no field number for this kind");
58405854
}
58415855
llvm_unreachable("unhandled kind");

‎include/swift/Sema/CodeCompletionTypeChecking.h

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,29 @@ namespace swift {
116116
void sawSolution(const constraints::Solution &solution) override;
117117
};
118118

119+
class KeyPathTypeCheckCompletionCallback
120+
: public TypeCheckCompletionCallback {
121+
public:
122+
struct Result {
123+
/// The type on which completion should occur, i.e. a result type of the
124+
/// previous component.
125+
Type BaseType;
126+
/// Whether code completion happens on the key path's root.
127+
bool OnRoot;
128+
};
129+
130+
private:
131+
KeyPathExpr *KeyPath;
132+
SmallVector<Result, 4> Results;
133+
134+
public:
135+
KeyPathTypeCheckCompletionCallback(KeyPathExpr *KeyPath)
136+
: KeyPath(KeyPath) {}
137+
138+
ArrayRef<Result> getResults() const { return Results; }
139+
140+
void sawSolution(const constraints::Solution &solution) override;
141+
};
119142
}
120143

121144
#endif

‎lib/AST/ASTDumper.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2877,6 +2877,9 @@ class PrintExpr : public ExprVisitor<PrintExpr> {
28772877
PrintWithColorRAII(OS, IdentifierColor)
28782878
<< " key='" << component.getUnresolvedDeclName() << "'";
28792879
break;
2880+
case KeyPathExpr::Component::Kind::CodeCompletion:
2881+
PrintWithColorRAII(OS, ASTNodeColor) << "completion";
2882+
break;
28802883
}
28812884
PrintWithColorRAII(OS, TypeColor)
28822885
<< " type='" << GetTypeOfKeyPathComponent(E, i) << "'";

‎lib/AST/ASTWalker.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1126,6 +1126,7 @@ class Traversal : public ASTVisitor<Traversal, Expr*, Stmt*,
11261126
case KeyPathExpr::Component::Kind::Identity:
11271127
case KeyPathExpr::Component::Kind::TupleElement:
11281128
case KeyPathExpr::Component::Kind::DictionaryKey:
1129+
case KeyPathExpr::Component::Kind::CodeCompletion:
11291130
// No subexpr to visit.
11301131
break;
11311132
}

‎lib/AST/Expr.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2418,6 +2418,7 @@ void KeyPathExpr::Component::setSubscriptIndexHashableConformances(
24182418
case Kind::Identity:
24192419
case Kind::TupleElement:
24202420
case Kind::DictionaryKey:
2421+
case Kind::CodeCompletion:
24212422
llvm_unreachable("no hashable conformances for this kind");
24222423
}
24232424
}

‎lib/IDE/CodeCompletion.cpp

Lines changed: 38 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -6742,6 +6742,28 @@ void deliverUnresolvedMemberResults(
67426742
deliverCompletionResults(CompletionCtx, Lookup, DC, Consumer);
67436743
}
67446744

6745+
void deliverKeyPathResults(
6746+
ArrayRef<KeyPathTypeCheckCompletionCallback::Result> Results,
6747+
DeclContext *DC, SourceLoc DotLoc,
6748+
ide::CodeCompletionContext &CompletionCtx,
6749+
CodeCompletionConsumer &Consumer) {
6750+
ASTContext &Ctx = DC->getASTContext();
6751+
CompletionLookup Lookup(CompletionCtx.getResultSink(), Ctx, DC,
6752+
&CompletionCtx);
6753+
6754+
if (DotLoc.isValid()) {
6755+
Lookup.setHaveDot(DotLoc);
6756+
}
6757+
Lookup.shouldCheckForDuplicates(Results.size() > 1);
6758+
6759+
for (auto &Result : Results) {
6760+
Lookup.setIsSwiftKeyPathExpr(Result.OnRoot);
6761+
Lookup.getValueExprCompletions(Result.BaseType);
6762+
}
6763+
6764+
deliverCompletionResults(CompletionCtx, Lookup, DC, Consumer);
6765+
}
6766+
67456767
void deliverDotExprResults(
67466768
ArrayRef<DotExprTypeCheckCompletionCallback::Result> Results,
67476769
Expr *BaseExpr, DeclContext *DC, SourceLoc DotLoc, bool IsInSelector,
@@ -6833,6 +6855,21 @@ bool CodeCompletionCallbacksImpl::trySolverCompletion(bool MaybeFuncBody) {
68336855
CompletionContext, Consumer);
68346856
return true;
68356857
}
6858+
case CompletionKind::KeyPathExprSwift: {
6859+
assert(CurDeclContext);
6860+
6861+
// CodeCompletionCallbacks::completeExprKeyPath takes a \c KeyPathExpr,
6862+
// so we can safely cast the \c ParsedExpr back to a \c KeyPathExpr.
6863+
auto KeyPath = cast<KeyPathExpr>(ParsedExpr);
6864+
KeyPathTypeCheckCompletionCallback Lookup(KeyPath);
6865+
llvm::SaveAndRestore<TypeCheckCompletionCallback *> CompletionCollector(
6866+
Context.CompletionCallback, &Lookup);
6867+
typeCheckContextAt(CurDeclContext, CompletionLoc);
6868+
6869+
deliverKeyPathResults(Lookup.getResults(), CurDeclContext, DotLoc,
6870+
CompletionContext, Consumer);
6871+
return true;
6872+
}
68366873
default:
68376874
return false;
68386875
}
@@ -6953,60 +6990,10 @@ void CodeCompletionCallbacksImpl::doneParsing() {
69536990
case CompletionKind::None:
69546991
case CompletionKind::DotExpr:
69556992
case CompletionKind::UnresolvedMember:
6993+
case CompletionKind::KeyPathExprSwift:
69566994
llvm_unreachable("should be already handled");
69576995
return;
69586996

6959-
case CompletionKind::KeyPathExprSwift: {
6960-
auto KPE = dyn_cast<KeyPathExpr>(ParsedExpr);
6961-
auto BGT = (*ExprType)->getAs<BoundGenericType>();
6962-
if (!KPE || !BGT || BGT->getGenericArgs().size() != 2)
6963-
break;
6964-
assert(!KPE->isObjC());
6965-
6966-
if (DotLoc.isValid())
6967-
Lookup.setHaveDot(DotLoc);
6968-
6969-
bool OnRoot = !KPE->getComponents().front().isValid();
6970-
Lookup.setIsSwiftKeyPathExpr(OnRoot);
6971-
6972-
Type baseType = BGT->getGenericArgs()[OnRoot ? 0 : 1];
6973-
if (OnRoot && baseType->is<UnresolvedType>()) {
6974-
// Infer the root type of the keypath from the context type.
6975-
ExprContextInfo ContextInfo(CurDeclContext, ParsedExpr);
6976-
for (auto T : ContextInfo.getPossibleTypes()) {
6977-
if (auto unwrapped = T->getOptionalObjectType())
6978-
T = unwrapped;
6979-
6980-
// If the context type is any of the KeyPath types, use it.
6981-
if (T->getAnyNominal() && T->getAnyNominal()->getKeyPathTypeKind() &&
6982-
!T->hasUnresolvedType() && T->is<BoundGenericType>()) {
6983-
baseType = T->castTo<BoundGenericType>()->getGenericArgs()[0];
6984-
break;
6985-
}
6986-
6987-
// KeyPath can be used as a function that receives its root type.
6988-
if (T->is<AnyFunctionType>()) {
6989-
auto *fnType = T->castTo<AnyFunctionType>();
6990-
if (fnType->getNumParams() == 1) {
6991-
const AnyFunctionType::Param &param = fnType->getParams()[0];
6992-
baseType = param.getParameterType();
6993-
break;
6994-
}
6995-
}
6996-
}
6997-
}
6998-
if (!OnRoot && KPE->getComponents().back().getKind() ==
6999-
KeyPathExpr::Component::Kind::OptionalWrap) {
7000-
// KeyPath expr with '?' (e.g. '\Ty.[0].prop?.another').
7001-
// Although expected type is optional, we should unwrap it because it's
7002-
// unwrapped.
7003-
baseType = baseType->getOptionalObjectType();
7004-
}
7005-
7006-
Lookup.getValueExprCompletions(baseType);
7007-
break;
7008-
}
7009-
70106997
case CompletionKind::StmtOrExpr:
70116998
case CompletionKind::ForEachSequence:
70126999
case CompletionKind::PostfixExprBeginning: {

‎lib/IDE/SourceEntityWalker.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -435,6 +435,7 @@ std::pair<bool, Expr *> SemaAnnotator::walkToExprPre(Expr *E) {
435435
case KeyPathExpr::Component::Kind::OptionalForce:
436436
case KeyPathExpr::Component::Kind::Identity:
437437
case KeyPathExpr::Component::Kind::DictionaryKey:
438+
case KeyPathExpr::Component::Kind::CodeCompletion:
438439
break;
439440
}
440441
}

‎lib/IDE/SwiftSourceDocInfo.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -440,6 +440,7 @@ std::pair<bool, Expr*> NameMatcher::walkToExprPre(Expr *E) {
440440
break;
441441
case KeyPathExpr::Component::Kind::DictionaryKey:
442442
case KeyPathExpr::Component::Kind::Invalid:
443+
case KeyPathExpr::Component::Kind::CodeCompletion:
443444
break;
444445
case KeyPathExpr::Component::Kind::OptionalForce:
445446
case KeyPathExpr::Component::Kind::OptionalChain:

‎lib/Parse/ParseExpr.cpp

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -659,21 +659,26 @@ ParserResult<Expr> Parser::parseExprKeyPath() {
659659
if (rootResult.isNull() && pathResult.isNull())
660660
return nullptr;
661661

662-
auto keypath =
663-
new (Context) KeyPathExpr(backslashLoc, rootResult.getPtrOrNull(),
664-
pathResult.getPtrOrNull(), hasLeadingDot);
665-
666662
// Handle code completion.
667663
if ((Tok.is(tok::code_complete) && !Tok.isAtStartOfLine()) ||
668664
(Tok.is(tok::period) && peekToken().isAny(tok::code_complete))) {
669665
SourceLoc DotLoc;
670666
consumeIf(tok::period, DotLoc);
667+
668+
// Add the code completion expression to the path result.
669+
CodeCompletionExpr *CC = new (Context)
670+
CodeCompletionExpr(pathResult.getPtrOrNull(), Tok.getLoc());
671+
auto keypath = new (Context)
672+
KeyPathExpr(backslashLoc, rootResult.getPtrOrNull(), CC, hasLeadingDot);
671673
if (CodeCompletion)
672674
CodeCompletion->completeExprKeyPath(keypath, DotLoc);
673675
consumeToken(tok::code_complete);
674676
return makeParserCodeCompletionResult(keypath);
675677
}
676678

679+
auto keypath =
680+
new (Context) KeyPathExpr(backslashLoc, rootResult.getPtrOrNull(),
681+
pathResult.getPtrOrNull(), hasLeadingDot);
677682
return makeParserResult(parseStatus, keypath);
678683
}
679684

‎lib/SILGen/SILGenExpr.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3741,6 +3741,7 @@ RValue RValueEmitter::visitKeyPathExpr(KeyPathExpr *E, SGFContext C) {
37413741
case KeyPathExpr::Component::Kind::Invalid:
37423742
case KeyPathExpr::Component::Kind::UnresolvedProperty:
37433743
case KeyPathExpr::Component::Kind::UnresolvedSubscript:
3744+
case KeyPathExpr::Component::Kind::CodeCompletion:
37443745
llvm_unreachable("not resolved");
37453746
break;
37463747

‎lib/Sema/CSApply.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -377,6 +377,7 @@ static bool buildObjCKeyPathString(KeyPathExpr *E,
377377
case KeyPathExpr::Component::Kind::Invalid:
378378
case KeyPathExpr::Component::Kind::UnresolvedProperty:
379379
case KeyPathExpr::Component::Kind::UnresolvedSubscript:
380+
case KeyPathExpr::Component::Kind::CodeCompletion:
380381
// Don't bother building the key path string if the key path didn't even
381382
// resolve.
382383
return false;
@@ -4932,7 +4933,8 @@ namespace {
49324933
}
49334934
break;
49344935
}
4935-
case KeyPathExpr::Component::Kind::Invalid: {
4936+
case KeyPathExpr::Component::Kind::Invalid:
4937+
case KeyPathExpr::Component::Kind::CodeCompletion: {
49364938
auto component = origComponent;
49374939
component.setComponentType(leafTy);
49384940
resolvedComponents.push_back(component);

‎lib/Sema/CSGen.cpp

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3123,6 +3123,7 @@ namespace {
31233123
if (!rootObjectTy || rootObjectTy->hasError())
31243124
return Type();
31253125

3126+
CS.setType(rootRepr, rootObjectTy);
31263127
// Allow \Derived.property to be inferred as \Base.property to
31273128
// simulate a sort of covariant conversion from
31283129
// KeyPath<Derived, T> to KeyPath<Base, T>.
@@ -3144,7 +3145,13 @@ namespace {
31443145
switch (auto kind = component.getKind()) {
31453146
case KeyPathExpr::Component::Kind::Invalid:
31463147
break;
3147-
3148+
case KeyPathExpr::Component::Kind::CodeCompletion:
3149+
// We don't know what the code completion might resolve to, so we are
3150+
// creating a new type variable for its result, which might be a hole.
3151+
base = CS.createTypeVariable(
3152+
resultLocator,
3153+
TVO_CanBindToLValue | TVO_CanBindToNoEscape | TVO_CanBindToHole);
3154+
break;
31483155
case KeyPathExpr::Component::Kind::UnresolvedProperty:
31493156
// This should only appear in resolved ASTs, but we may need to
31503157
// re-type-check the constraints during failure diagnosis.
@@ -3240,7 +3247,8 @@ namespace {
32403247
// If there was an optional chaining component, the end result must be
32413248
// optional.
32423249
if (didOptionalChain) {
3243-
auto objTy = CS.createTypeVariable(locator, TVO_CanBindToNoEscape);
3250+
auto objTy = CS.createTypeVariable(locator, TVO_CanBindToNoEscape |
3251+
TVO_CanBindToHole);
32443252
componentTypeVars.push_back(objTy);
32453253

32463254
auto optTy = OptionalType::get(objTy);
@@ -3251,8 +3259,8 @@ namespace {
32513259

32523260
auto baseLocator =
32533261
CS.getConstraintLocator(E, ConstraintLocator::KeyPathValue);
3254-
auto rvalueBase = CS.createTypeVariable(baseLocator,
3255-
TVO_CanBindToNoEscape);
3262+
auto rvalueBase = CS.createTypeVariable(
3263+
baseLocator, TVO_CanBindToNoEscape | TVO_CanBindToHole);
32563264
CS.addConstraint(ConstraintKind::Equal, base, rvalueBase, locator);
32573265

32583266
// The result is a KeyPath from the root to the end component.

‎lib/Sema/CSSimplify.cpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9468,7 +9468,12 @@ ConstraintSystem::simplifyKeyPathConstraint(
94689468
case KeyPathExpr::Component::Kind::Invalid:
94699469
case KeyPathExpr::Component::Kind::Identity:
94709470
break;
9471-
9471+
9472+
case KeyPathExpr::Component::Kind::CodeCompletion: {
9473+
anyComponentsUnresolved = true;
9474+
capability = ReadOnly;
9475+
break;
9476+
}
94729477
case KeyPathExpr::Component::Kind::Property:
94739478
case KeyPathExpr::Component::Kind::Subscript:
94749479
case KeyPathExpr::Component::Kind::UnresolvedProperty:

0 commit comments

Comments
 (0)