Skip to content

Commit e333291

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 d572e13 commit e333291

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
@@ -5409,6 +5409,7 @@ class KeyPathExpr : public Expr {
54095409
/// - a resolved ValueDecl, referring to
54105410
/// - a subscript index expression, which may or may not be resolved
54115411
/// - an optional chaining, forcing, or wrapping component
5412+
/// - a code completion token
54125413
class Component {
54135414
public:
54145415
enum class Kind: unsigned {
@@ -5423,6 +5424,7 @@ class KeyPathExpr : public Expr {
54235424
Identity,
54245425
TupleElement,
54255426
DictionaryKey,
5427+
CodeCompletion,
54265428
};
54275429

54285430
private:
@@ -5598,8 +5600,12 @@ class KeyPathExpr : public Expr {
55985600
SourceLoc loc) {
55995601
return Component(fieldNumber, elementType, loc);
56005602
}
5601-
5602-
5603+
5604+
static Component forCodeCompletion(SourceLoc Loc) {
5605+
return Component(nullptr, {}, nullptr, {}, {}, Kind::CodeCompletion,
5606+
Type(), Loc);
5607+
}
5608+
56035609
SourceLoc getLoc() const {
56045610
return Loc;
56055611
}
@@ -5637,6 +5643,7 @@ class KeyPathExpr : public Expr {
56375643
case Kind::UnresolvedSubscript:
56385644
case Kind::UnresolvedProperty:
56395645
case Kind::Invalid:
5646+
case Kind::CodeCompletion:
56405647
return false;
56415648
}
56425649
llvm_unreachable("unhandled kind");
@@ -5657,6 +5664,7 @@ class KeyPathExpr : public Expr {
56575664
case Kind::Identity:
56585665
case Kind::TupleElement:
56595666
case Kind::DictionaryKey:
5667+
case Kind::CodeCompletion:
56605668
return nullptr;
56615669
}
56625670
llvm_unreachable("unhandled kind");
@@ -5677,6 +5685,7 @@ class KeyPathExpr : public Expr {
56775685
case Kind::Identity:
56785686
case Kind::TupleElement:
56795687
case Kind::DictionaryKey:
5688+
case Kind::CodeCompletion:
56805689
llvm_unreachable("no subscript labels for this kind");
56815690
}
56825691
llvm_unreachable("unhandled kind");
@@ -5700,6 +5709,7 @@ class KeyPathExpr : public Expr {
57005709
case Kind::Identity:
57015710
case Kind::TupleElement:
57025711
case Kind::DictionaryKey:
5712+
case Kind::CodeCompletion:
57035713
return {};
57045714
}
57055715
llvm_unreachable("unhandled kind");
@@ -5723,6 +5733,7 @@ class KeyPathExpr : public Expr {
57235733
case Kind::Property:
57245734
case Kind::Identity:
57255735
case Kind::TupleElement:
5736+
case Kind::CodeCompletion:
57265737
llvm_unreachable("no unresolved name for this kind");
57275738
}
57285739
llvm_unreachable("unhandled kind");
@@ -5743,6 +5754,7 @@ class KeyPathExpr : public Expr {
57435754
case Kind::Identity:
57445755
case Kind::TupleElement:
57455756
case Kind::DictionaryKey:
5757+
case Kind::CodeCompletion:
57465758
return false;
57475759
}
57485760
llvm_unreachable("unhandled kind");
@@ -5763,6 +5775,7 @@ class KeyPathExpr : public Expr {
57635775
case Kind::Identity:
57645776
case Kind::TupleElement:
57655777
case Kind::DictionaryKey:
5778+
case Kind::CodeCompletion:
57665779
llvm_unreachable("no decl ref for this kind");
57675780
}
57685781
llvm_unreachable("unhandled kind");
@@ -5783,6 +5796,7 @@ class KeyPathExpr : public Expr {
57835796
case Kind::Property:
57845797
case Kind::Subscript:
57855798
case Kind::DictionaryKey:
5799+
case Kind::CodeCompletion:
57865800
llvm_unreachable("no field number for this kind");
57875801
}
57885802
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
@@ -2880,6 +2880,9 @@ class PrintExpr : public ExprVisitor<PrintExpr> {
28802880
PrintWithColorRAII(OS, IdentifierColor)
28812881
<< " key='" << component.getUnresolvedDeclName() << "'";
28822882
break;
2883+
case KeyPathExpr::Component::Kind::CodeCompletion:
2884+
PrintWithColorRAII(OS, ASTNodeColor) << "completion";
2885+
break;
28832886
}
28842887
PrintWithColorRAII(OS, TypeColor)
28852888
<< " type='" << GetTypeOfKeyPathComponent(E, i) << "'";

‎lib/AST/ASTWalker.cpp

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

‎lib/AST/Expr.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2399,6 +2399,7 @@ void KeyPathExpr::Component::setSubscriptIndexHashableConformances(
23992399
case Kind::Identity:
24002400
case Kind::TupleElement:
24012401
case Kind::DictionaryKey:
2402+
case Kind::CodeCompletion:
24022403
llvm_unreachable("no hashable conformances for this kind");
24032404
}
24042405
}

‎lib/IDE/CodeCompletion.cpp

Lines changed: 38 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -6727,6 +6727,28 @@ void deliverUnresolvedMemberResults(
67276727
deliverCompletionResults(CompletionCtx, Lookup, DC, Consumer);
67286728
}
67296729

6730+
void deliverKeyPathResults(
6731+
ArrayRef<KeyPathTypeCheckCompletionCallback::Result> Results,
6732+
DeclContext *DC, SourceLoc DotLoc,
6733+
ide::CodeCompletionContext &CompletionCtx,
6734+
CodeCompletionConsumer &Consumer) {
6735+
ASTContext &Ctx = DC->getASTContext();
6736+
CompletionLookup Lookup(CompletionCtx.getResultSink(), Ctx, DC,
6737+
&CompletionCtx);
6738+
6739+
if (DotLoc.isValid()) {
6740+
Lookup.setHaveDot(DotLoc);
6741+
}
6742+
Lookup.shouldCheckForDuplicates(Results.size() > 1);
6743+
6744+
for (auto &Result : Results) {
6745+
Lookup.setIsSwiftKeyPathExpr(Result.OnRoot);
6746+
Lookup.getValueExprCompletions(Result.BaseType);
6747+
}
6748+
6749+
deliverCompletionResults(CompletionCtx, Lookup, DC, Consumer);
6750+
}
6751+
67306752
void deliverDotExprResults(
67316753
ArrayRef<DotExprTypeCheckCompletionCallback::Result> Results,
67326754
Expr *BaseExpr, DeclContext *DC, SourceLoc DotLoc, bool IsInSelector,
@@ -6818,6 +6840,21 @@ bool CodeCompletionCallbacksImpl::trySolverCompletion(bool MaybeFuncBody) {
68186840
CompletionContext, Consumer);
68196841
return true;
68206842
}
6843+
case CompletionKind::KeyPathExprSwift: {
6844+
assert(CurDeclContext);
6845+
6846+
// CodeCompletionCallbacks::completeExprKeyPath takes a \c KeyPathExpr,
6847+
// so we can safely cast the \c ParsedExpr back to a \c KeyPathExpr.
6848+
auto KeyPath = cast<KeyPathExpr>(ParsedExpr);
6849+
KeyPathTypeCheckCompletionCallback Lookup(KeyPath);
6850+
llvm::SaveAndRestore<TypeCheckCompletionCallback *> CompletionCollector(
6851+
Context.CompletionCallback, &Lookup);
6852+
typeCheckContextAt(CurDeclContext, CompletionLoc);
6853+
6854+
deliverKeyPathResults(Lookup.getResults(), CurDeclContext, DotLoc,
6855+
CompletionContext, Consumer);
6856+
return true;
6857+
}
68216858
default:
68226859
return false;
68236860
}
@@ -6938,60 +6975,10 @@ void CodeCompletionCallbacksImpl::doneParsing() {
69386975
case CompletionKind::None:
69396976
case CompletionKind::DotExpr:
69406977
case CompletionKind::UnresolvedMember:
6978+
case CompletionKind::KeyPathExprSwift:
69416979
llvm_unreachable("should be already handled");
69426980
return;
69436981

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

‎lib/IDE/SourceEntityWalker.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -494,6 +494,7 @@ std::pair<bool, Expr *> SemaAnnotator::walkToExprPre(Expr *E) {
494494
case KeyPathExpr::Component::Kind::OptionalForce:
495495
case KeyPathExpr::Component::Kind::Identity:
496496
case KeyPathExpr::Component::Kind::DictionaryKey:
497+
case KeyPathExpr::Component::Kind::CodeCompletion:
497498
break;
498499
}
499500
}

‎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
@@ -3748,6 +3748,7 @@ RValue RValueEmitter::visitKeyPathExpr(KeyPathExpr *E, SGFContext C) {
37483748
case KeyPathExpr::Component::Kind::Invalid:
37493749
case KeyPathExpr::Component::Kind::UnresolvedProperty:
37503750
case KeyPathExpr::Component::Kind::UnresolvedSubscript:
3751+
case KeyPathExpr::Component::Kind::CodeCompletion:
37513752
llvm_unreachable("not resolved");
37523753
break;
37533754

‎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;
@@ -4938,7 +4939,8 @@ namespace {
49384939
}
49394940
break;
49404941
}
4941-
case KeyPathExpr::Component::Kind::Invalid: {
4942+
case KeyPathExpr::Component::Kind::Invalid:
4943+
case KeyPathExpr::Component::Kind::CodeCompletion: {
49424944
auto component = origComponent;
49434945
component.setComponentType(leafTy);
49444946
resolvedComponents.push_back(component);

‎lib/Sema/CSGen.cpp

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

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

32453253
auto optTy = OptionalType::get(objTy);
@@ -3250,8 +3258,8 @@ namespace {
32503258

32513259
auto baseLocator =
32523260
CS.getConstraintLocator(E, ConstraintLocator::KeyPathValue);
3253-
auto rvalueBase = CS.createTypeVariable(baseLocator,
3254-
TVO_CanBindToNoEscape);
3261+
auto rvalueBase = CS.createTypeVariable(
3262+
baseLocator, TVO_CanBindToNoEscape | TVO_CanBindToHole);
32553263
CS.addConstraint(ConstraintKind::Equal, base, rvalueBase, locator);
32563264

32573265
// 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
@@ -9488,7 +9488,12 @@ ConstraintSystem::simplifyKeyPathConstraint(
94889488
case KeyPathExpr::Component::Kind::Invalid:
94899489
case KeyPathExpr::Component::Kind::Identity:
94909490
break;
9491-
9491+
9492+
case KeyPathExpr::Component::Kind::CodeCompletion: {
9493+
anyComponentsUnresolved = true;
9494+
capability = ReadOnly;
9495+
break;
9496+
}
94929497
case KeyPathExpr::Component::Kind::Property:
94939498
case KeyPathExpr::Component::Kind::Subscript:
94949499
case KeyPathExpr::Component::Kind::UnresolvedProperty:

0 commit comments

Comments
 (0)