Skip to content

Commit 4716f61

Browse files
committed
[AST] Introduce explicit actions for ASTWalker
Replace the use of bool and pointer returns for `walkToXXXPre`/`walkToXXXPost`, and instead use explicit actions such as `Action::Continue(E)`, `Action::SkipChildren(E)`, and `Action::Stop()`. There are also conditional variants, e.g `Action::SkipChildrenIf`, `Action::VisitChildrenIf`, and `Action::StopIf`. There is still more work that can be done here, in particular: - SourceEntityWalker still needs to be migrated. - Some uses of `return false` in pre-visitation methods can likely now be replaced by `Action::Stop`. - We still use bool and pointer returns internally within the ASTWalker traversal, which could likely be improved. But I'm leaving those as future work for now as this patch is already large enough.
1 parent 6804623 commit 4716f61

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

68 files changed

+1826
-1374
lines changed

include/swift/AST/ASTWalker.h

Lines changed: 324 additions & 64 deletions
Large diffs are not rendered by default.

include/swift/AST/TypeDeclFinder.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,8 @@ class TypeReprIdentFinder : public ASTWalker {
6666
/// The function to call when a ComponentIdentTypeRepr is seen.
6767
llvm::function_ref<bool(const ComponentIdentTypeRepr *)> Callback;
6868

69-
bool walkToTypeReprPost(TypeRepr *TR) override;
69+
PostWalkAction walkToTypeReprPost(TypeRepr *TR) override;
70+
7071
public:
7172
explicit TypeReprIdentFinder(
7273
llvm::function_ref<bool(const ComponentIdentTypeRepr *)> callback)

include/swift/IDE/SourceEntityWalker.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ namespace swift {
5454
/// call will *not* receive a \c walkTo*Post call.
5555
/// If \c walkTo*Post returns \c true, the traversal continues.
5656
class SourceEntityWalker {
57+
// TODO: Switch to using explicit ASTWalker actions.
5758
public:
5859
/// Walks the provided source file.
5960
/// \returns true if traversal was aborted, false otherwise.

include/swift/IDE/Utils.h

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -284,18 +284,18 @@ class NameMatcher: public ASTWalker {
284284
bool handleCustomAttrs(Decl *D);
285285
ArgumentList *getApplicableArgsFor(Expr* E);
286286

287-
std::pair<bool, Expr*> walkToExprPre(Expr *E) override;
288-
Expr* walkToExprPost(Expr *E) override;
289-
bool walkToDeclPre(Decl *D) override;
290-
bool walkToDeclPost(Decl *D) override;
291-
std::pair<bool, Stmt*> walkToStmtPre(Stmt *S) override;
292-
Stmt* walkToStmtPost(Stmt *S) override;
293-
bool walkToTypeReprPre(TypeRepr *T) override;
294-
bool walkToTypeReprPost(TypeRepr *T) override;
295-
std::pair<bool, Pattern*> walkToPatternPre(Pattern *P) override;
287+
PreWalkResult<Expr *> walkToExprPre(Expr *E) override;
288+
PostWalkResult<Expr *> walkToExprPost(Expr *E) override;
289+
PreWalkAction walkToDeclPre(Decl *D) override;
290+
PostWalkAction walkToDeclPost(Decl *D) override;
291+
PreWalkResult<Stmt *> walkToStmtPre(Stmt *S) override;
292+
PostWalkResult<Stmt *> walkToStmtPost(Stmt *S) override;
293+
PreWalkAction walkToTypeReprPre(TypeRepr *T) override;
294+
PostWalkAction walkToTypeReprPost(TypeRepr *T) override;
295+
PreWalkResult<Pattern *> walkToPatternPre(Pattern *P) override;
296296
bool shouldWalkIntoGenericParams() override { return true; }
297297

298-
std::pair<bool, ArgumentList *>
298+
PreWalkResult<ArgumentList *>
299299
walkToArgumentListPre(ArgumentList *ArgList) override;
300300

301301
// FIXME: Remove this

include/swift/Sema/CompletionContextFinder.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,9 +64,9 @@ class CompletionContextFinder : public ASTWalker {
6464
InitialDC->walkContext(*this);
6565
}
6666

67-
std::pair<bool, Expr *> walkToExprPre(Expr *E) override;
67+
PreWalkResult<Expr *> walkToExprPre(Expr *E) override;
6868

69-
Expr *walkToExprPost(Expr *E) override;
69+
PostWalkResult<Expr *> walkToExprPost(Expr *E) override;
7070

7171
/// Check whether code completion expression is located inside of a
7272
/// multi-statement closure.

include/swift/Sema/ConstraintSystem.h

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3307,10 +3307,10 @@ class ConstraintSystem {
33073307
CacheExprTypes(Expr *expr, ConstraintSystem &cs, bool excludeRoot)
33083308
: RootExpr(expr), CS(cs), ExcludeRoot(excludeRoot) {}
33093309

3310-
Expr *walkToExprPost(Expr *expr) override {
3310+
PostWalkResult<Expr *> walkToExprPost(Expr *expr) override {
33113311
if (ExcludeRoot && expr == RootExpr) {
33123312
assert(!expr->getType() && "Unexpected type in root of expression!");
3313-
return expr;
3313+
return Action::Continue(expr);
33143314
}
33153315

33163316
if (expr->getType())
@@ -3321,16 +3321,18 @@ class ConstraintSystem {
33213321
if (kp->getComponents()[i].getComponentType())
33223322
CS.cacheType(kp, i);
33233323

3324-
return expr;
3324+
return Action::Continue(expr);
33253325
}
33263326

33273327
/// Ignore statements.
3328-
std::pair<bool, Stmt *> walkToStmtPre(Stmt *stmt) override {
3329-
return { false, stmt };
3328+
PreWalkResult<Stmt *> walkToStmtPre(Stmt *stmt) override {
3329+
return Action::SkipChildren(stmt);
33303330
}
33313331

33323332
/// Ignore declarations.
3333-
bool walkToDeclPre(Decl *decl) override { return false; }
3333+
PreWalkAction walkToDeclPre(Decl *decl) override {
3334+
return Action::SkipChildren();
3335+
}
33343336
};
33353337

33363338
public:
@@ -6751,7 +6753,7 @@ class OverloadSetCounter : public ASTWalker {
67516753
: NumOverloads(overloads)
67526754
{}
67536755

6754-
std::pair<bool, Expr *> walkToExprPre(Expr *expr) override {
6756+
PreWalkResult<Expr *> walkToExprPre(Expr *expr) override {
67556757
if (auto applyExpr = dyn_cast<ApplyExpr>(expr)) {
67566758
// If we've found function application and it's
67576759
// function is an overload set, count it.
@@ -6760,7 +6762,7 @@ class OverloadSetCounter : public ASTWalker {
67606762
}
67616763

67626764
// Always recur into the children.
6763-
return { true, expr };
6765+
return Action::Continue(expr);
67646766
}
67656767
};
67666768

lib/APIDigester/ModuleAnalyzerNodes.cpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2387,16 +2387,17 @@ class ConstExtractor: public ASTWalker {
23872387
}
23882388
return false;
23892389
}
2390-
std::pair<bool, Expr *> walkToExprPre(Expr *E) override {
2390+
PreWalkResult<Expr *> walkToExprPre(Expr *E) override {
23912391
if (E->isSemanticallyConstExpr()) {
23922392
record(E, E);
2393-
return { false, E };
2393+
return Action::SkipChildren(E);
23942394
}
23952395
if (handleSimpleReference(E)) {
2396-
return { false, E };
2396+
return Action::SkipChildren(E);
23972397
}
2398-
return { true, E };
2398+
return Action::Continue(E);
23992399
}
2400+
24002401
public:
24012402
ConstExtractor(SDKContext &SCtx, ASTContext &Ctx): SCtx(SCtx),
24022403
SM(Ctx.SourceMgr) {}

lib/AST/ASTScopeCreation.cpp

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -108,33 +108,39 @@ class ScopeCreator final : public ASTAllocated<ScopeCreator> {
108108
ClosureFinder(ScopeCreator &scopeCreator, ASTScopeImpl *parent)
109109
: scopeCreator(scopeCreator), parent(parent) {}
110110

111-
std::pair<bool, Expr *> walkToExprPre(Expr *E) override {
111+
PreWalkResult<Expr *> walkToExprPre(Expr *E) override {
112112
if (auto *closure = dyn_cast<ClosureExpr>(E)) {
113113
scopeCreator
114114
.constructExpandAndInsert<ClosureParametersScope>(
115115
parent, closure);
116-
return {false, E};
116+
return Action::SkipChildren(E);
117117
}
118118
if (auto *capture = dyn_cast<CaptureListExpr>(E)) {
119119
scopeCreator
120120
.constructExpandAndInsert<CaptureListScope>(
121121
parent, capture);
122-
return {false, E};
122+
return Action::SkipChildren(E);
123123
}
124-
return {true, E};
124+
return Action::Continue(E);
125125
}
126-
std::pair<bool, Stmt *> walkToStmtPre(Stmt *S) override {
126+
PreWalkResult<Stmt *> walkToStmtPre(Stmt *S) override {
127127
if (isa<BraceStmt>(S)) { // closures hidden in here
128-
return {true, S};
128+
return Action::Continue(S);
129129
}
130-
return {false, S};
130+
return Action::SkipChildren(S);
131131
}
132-
std::pair<bool, Pattern *> walkToPatternPre(Pattern *P) override {
133-
return {false, P};
132+
PreWalkResult<Pattern *> walkToPatternPre(Pattern *P) override {
133+
return Action::SkipChildren(P);
134+
}
135+
PreWalkAction walkToDeclPre(Decl *D) override {
136+
return Action::SkipChildren();
137+
}
138+
PreWalkAction walkToTypeReprPre(TypeRepr *T) override {
139+
return Action::SkipChildren();
140+
}
141+
PreWalkAction walkToParameterListPre(ParameterList *PL) override {
142+
return Action::SkipChildren();
134143
}
135-
bool walkToDeclPre(Decl *D) override { return false; }
136-
bool walkToTypeReprPre(TypeRepr *T) override { return false; }
137-
bool walkToParameterListPre(ParameterList *PL) override { return false; }
138144
};
139145

140146
expr->walk(ClosureFinder(*this, parent));

0 commit comments

Comments
 (0)