Skip to content

Commit 6264792

Browse files
committed
[SourceKit] Don't use SourceEntitityWalker for placeholder expansion
Placeholder expansion should be a syntactic operation, but `SourceEntityWalker` can invoke type checking operations, which causes unexpected bahaviors including crashes. rdar://121360941`
1 parent 73ed03c commit 6264792

File tree

2 files changed

+44
-11
lines changed

2 files changed

+44
-11
lines changed
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// RUN: %sourcekitd-test -req=expand-placeholder %s | %FileCheck %s
2+
3+
// FIXME: Make it acccept '-debug-forbid-typecheck-prefix' and ensure no typecheck happens.'
4+
5+
protocol MyProto {}
6+
7+
enum MyEnum: Hashable, MyProto {
8+
case foo
9+
case bar
10+
}
11+
12+
func test(array: [MyEnum]) -> [NyEnum] {
13+
array.filter(<#T##isIncluded: (MyEnum) throws -> Bool##(MyEnum) throws -> Bool#>)
14+
// CHECK: array.filter { <#MyEnum#> in
15+
// CHECK-NEXT: <#code#>
16+
// CHECK-NEXT: }
17+
}

tools/SourceKit/lib/SwiftLang/SwiftEditor.cpp

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1695,7 +1695,7 @@ class PlaceholderExpansionScanner {
16951695
std::pair<ArgumentList *, bool> enclosingCallExprArg(SourceFile &SF,
16961696
SourceLoc SL) {
16971697

1698-
class CallExprFinder : public SourceEntityWalker {
1698+
class CallExprFinder : public ASTWalker {
16991699
public:
17001700
const SourceManager &SM;
17011701
SourceLoc TargetLoc;
@@ -1719,7 +1719,11 @@ class PlaceholderExpansionScanner {
17191719
return true;
17201720
}
17211721

1722-
bool walkToExprPre(Expr *E) override {
1722+
MacroWalking getMacroWalkingBehavior() const override {
1723+
return MacroWalking::Arguments;
1724+
}
1725+
1726+
PreWalkResult<Expr *> walkToExprPre(Expr *E) override {
17231727
auto SR = E->getSourceRange();
17241728
if (SR.isValid() && SM.rangeContainsTokenLoc(SR, TargetLoc) &&
17251729
!checkCallExpr(E) && !EnclosingCallAndArg.first) {
@@ -1732,13 +1736,13 @@ class PlaceholderExpansionScanner {
17321736
OuterExpr = E;
17331737
}
17341738
}
1735-
return true;
1739+
return Action::Continue(E);
17361740
}
17371741

1738-
bool walkToExprPost(Expr *E) override {
1742+
PostWalkResult<Expr *> walkToExprPost(Expr *E) override {
17391743
if (E->getStartLoc() == TargetLoc)
1740-
return false; // found what we needed to find, stop walking.
1741-
return true;
1744+
return Action::Stop(); // found what we needed to find, stop walking.
1745+
return Action::Continue(E);
17421746
}
17431747

17441748
/// Whether this statement body consists of only an implicit "return",
@@ -1756,7 +1760,7 @@ class PlaceholderExpansionScanner {
17561760

17571761
// Before pre-checking, the implicit return will not have been
17581762
// inserted. Look for a single expression body in a closure.
1759-
if (auto *ParentE = getWalker().Parent.getAsExpr()) {
1763+
if (auto *ParentE = Parent.getAsExpr()) {
17601764
if (isa<ClosureExpr>(ParentE)) {
17611765
if (auto *innerE = BS->getSingleActiveExpression())
17621766
return innerE->getStartLoc() == TargetLoc;
@@ -1767,7 +1771,7 @@ class PlaceholderExpansionScanner {
17671771
return false;
17681772
}
17691773

1770-
bool walkToStmtPre(Stmt *S) override {
1774+
PreWalkResult<Stmt *> walkToStmtPre(Stmt *S) override {
17711775
auto SR = S->getSourceRange();
17721776
if (SR.isValid() && SM.rangeContainsTokenLoc(SR, TargetLoc) &&
17731777
!isImplicitReturnBody(S)) {
@@ -1791,17 +1795,29 @@ class PlaceholderExpansionScanner {
17911795
break;
17921796
}
17931797
}
1794-
return true;
1798+
return Action::Continue(S);
17951799
}
17961800

1797-
bool shouldWalkInactiveConfigRegion() override { return true; }
1801+
PreWalkAction walkToDeclPre(Decl *D) override {
1802+
if (auto *ICD = dyn_cast<IfConfigDecl>(D)) {
1803+
for (auto Clause : ICD->getClauses()) {
1804+
// Active clase elements are visited normally.
1805+
if (Clause.isActive)
1806+
continue;
1807+
for (auto Member : Clause.Elements)
1808+
Member.walk(*this);
1809+
}
1810+
return Action::SkipNode();
1811+
}
1812+
return Action::Continue();
1813+
}
17981814

17991815
ArgumentList *findEnclosingCallArg(SourceFile &SF, SourceLoc SL) {
18001816
EnclosingCallAndArg = {nullptr, nullptr};
18011817
OuterExpr = nullptr;
18021818
OuterStmt = nullptr;
18031819
TargetLoc = SL;
1804-
walk(SF);
1820+
SF.walk(*this);
18051821
return EnclosingCallAndArg.second;
18061822
}
18071823
};

0 commit comments

Comments
 (0)