Skip to content

Commit 15fa06d

Browse files
hamishknighthamishknight
committed
[ASTWalker] Cleanup some cases manually doing post-walks
We can replace these with `Action::SkipChildren`.
1 parent e80e59c commit 15fa06d

File tree

4 files changed

+29
-95
lines changed

4 files changed

+29
-95
lines changed

‎lib/IDE/SourceEntityWalker.cpp‎

Lines changed: 15 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -347,23 +347,6 @@ ASTWalker::PreWalkResult<Expr *> SemaAnnotator::walkToExprPre(Expr *E) {
347347
return Action::SkipNode(E);
348348
}
349349

350-
auto doSkipChildren = [&]() -> PreWalkResult<Expr *> {
351-
// If we decide to skip the children after having issued the call to
352-
// walkToExprPre, we need to simulate a corresponding call to walkToExprPost
353-
// which will not be issued by the ASTWalker if we return false in the first
354-
// component.
355-
// TODO: We should consider changing Action::SkipChildren to still call
356-
// walkToExprPost, which would eliminate the need for this.
357-
auto postWalkResult = walkToExprPost(E);
358-
switch (postWalkResult.Action.Action) {
359-
case PostWalkAction::Stop:
360-
return Action::Stop();
361-
case PostWalkAction::Continue:
362-
return Action::SkipNode(*postWalkResult.Value);
363-
}
364-
llvm_unreachable("Unhandled case in switch!");
365-
};
366-
367350
if (auto *CtorRefE = dyn_cast<ConstructorRefCallExpr>(E))
368351
CtorRefs.push_back(CtorRefE);
369352

@@ -439,11 +422,11 @@ ASTWalker::PreWalkResult<Expr *> SemaAnnotator::walkToExprPre(Expr *E) {
439422
if (!passReference(MRE->getMember().getDecl(), MRE->getType(),
440423
MRE->getNameLoc(),
441424
ReferenceMetaData(SemaReferenceKind::DeclMemberRef,
442-
OpAccess)))
425+
OpAccess))) {
443426
return Action::Stop();
444-
427+
}
445428
// We already visited the children.
446-
return doSkipChildren();
429+
return Action::SkipChildren(E);
447430

448431
} else if (auto OtherCtorE = dyn_cast<OtherConstructorDeclRefExpr>(E)) {
449432
if (!passReference(OtherCtorE->getDecl(), OtherCtorE->getType(),
@@ -478,7 +461,7 @@ ASTWalker::PreWalkResult<Expr *> SemaAnnotator::walkToExprPre(Expr *E) {
478461
}
479462

480463
// We already visited the children.
481-
return doSkipChildren();
464+
return Action::SkipChildren(E);
482465

483466
} else if (auto *KPE = dyn_cast<KeyPathExpr>(E)) {
484467
for (auto &component : KPE->getComponents()) {
@@ -522,7 +505,7 @@ ASTWalker::PreWalkResult<Expr *> SemaAnnotator::walkToExprPre(Expr *E) {
522505
return Action::Stop();
523506

524507
// We already visited the children.
525-
return doSkipChildren();
508+
return Action::SkipChildren(E);
526509
} else if (auto IOE = dyn_cast<InOutExpr>(E)) {
527510
llvm::SaveAndRestore<llvm::Optional<AccessKind>> C(this->OpAccess,
528511
AccessKind::ReadWrite);
@@ -531,7 +514,7 @@ ASTWalker::PreWalkResult<Expr *> SemaAnnotator::walkToExprPre(Expr *E) {
531514
return Action::Stop();
532515

533516
// We already visited the children.
534-
return doSkipChildren();
517+
return Action::SkipChildren(E);
535518
} else if (auto LE = dyn_cast<LoadExpr>(E)) {
536519
llvm::SaveAndRestore<llvm::Optional<AccessKind>> C(this->OpAccess,
537520
AccessKind::Read);
@@ -540,7 +523,7 @@ ASTWalker::PreWalkResult<Expr *> SemaAnnotator::walkToExprPre(Expr *E) {
540523
return Action::Stop();
541524

542525
// We already visited the children.
543-
return doSkipChildren();
526+
return Action::SkipChildren(E);
544527
} else if (auto AE = dyn_cast<AssignExpr>(E)) {
545528
{
546529
llvm::SaveAndRestore<llvm::Optional<AccessKind>> C(this->OpAccess,
@@ -554,7 +537,7 @@ ASTWalker::PreWalkResult<Expr *> SemaAnnotator::walkToExprPre(Expr *E) {
554537
return Action::Stop();
555538

556539
// We already visited the children.
557-
return doSkipChildren();
540+
return Action::SkipChildren(E);
558541
} else if (auto OEE = dyn_cast<OpenExistentialExpr>(E)) {
559542
// Record opaque value.
560543
OpaqueValueMap[OEE->getOpaqueValue()] = OEE->getExistentialValue();
@@ -565,7 +548,7 @@ ASTWalker::PreWalkResult<Expr *> SemaAnnotator::walkToExprPre(Expr *E) {
565548
if (!OEE->getSubExpr()->walk(*this))
566549
return Action::Stop();
567550

568-
return doSkipChildren();
551+
return Action::SkipChildren(E);
569552
} else if (auto MTEE = dyn_cast<MakeTemporarilyEscapableExpr>(E)) {
570553
// Manually walk to original arguments in order. We don't handle
571554
// OpaqueValueExpr here.
@@ -579,23 +562,23 @@ ASTWalker::PreWalkResult<Expr *> SemaAnnotator::walkToExprPre(Expr *E) {
579562
if (!callExpr->getFn()->walk(*this))
580563
return Action::Stop();
581564

582-
return doSkipChildren();
565+
return Action::SkipChildren(E);
583566
} else if (auto CUCE = dyn_cast<CollectionUpcastConversionExpr>(E)) {
584567
// Ignore conversion expressions. We don't handle OpaqueValueExpr here
585568
// because it's only in conversion expressions. Instead, just walk into
586569
// sub expression.
587570
if (!CUCE->getSubExpr()->walk(*this))
588571
return Action::Stop();
589572

590-
return doSkipChildren();
573+
return Action::SkipChildren(E);
591574
} else if (auto OVE = dyn_cast<OpaqueValueExpr>(E)) {
592575
// Walk into mapped value.
593576
auto value = OpaqueValueMap.find(OVE);
594577
if (value != OpaqueValueMap.end()) {
595578
if (!value->second->walk(*this))
596579
return Action::Stop();
597580

598-
return doSkipChildren();
581+
return Action::SkipChildren(E);
599582
}
600583
} else if (auto DMRE = dyn_cast<DynamicMemberRefExpr>(E)) {
601584
// Visit in source order.
@@ -604,10 +587,11 @@ ASTWalker::PreWalkResult<Expr *> SemaAnnotator::walkToExprPre(Expr *E) {
604587
if (!passReference(DMRE->getMember().getDecl(), DMRE->getType(),
605588
DMRE->getNameLoc(),
606589
ReferenceMetaData(SemaReferenceKind::DynamicMemberRef,
607-
OpAccess)))
590+
OpAccess))) {
608591
return Action::Stop();
592+
}
609593
// We already visited the children.
610-
return doSkipChildren();
594+
return Action::SkipChildren(E);
611595
} else if (auto ME = dyn_cast<MacroExpansionExpr>(E)) {
612596
// Add a reference to the macro if this is a true macro expansion *expression*.
613597
// If this is a `MacroExpansionExpr` that expands a declaration macro, the

‎lib/IDE/SyntaxModel.cpp‎

Lines changed: 4 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -672,17 +672,8 @@ ASTWalker::PreWalkResult<Expr *> ModelASTWalker::walkToExprPre(Expr *E) {
672672
llvm::SaveAndRestore<ASTWalker::ParentTy> SetParent(Parent, E);
673673
subExpr->walk(*this);
674674
}
675-
// TODO: We should consider changing Action::SkipChildren to still call
676-
// walkToExprPost, which would eliminate the need for this.
677-
auto postWalkResult = walkToExprPost(SE);
678-
switch (postWalkResult.Action.Action) {
679-
case PostWalkAction::Stop:
680-
return Action::Stop();
681-
case PostWalkAction::Continue:
682-
// We already visited the children.
683-
return Action::SkipNode(*postWalkResult.Value);
684-
}
685-
llvm_unreachable("Unhandled case in switch!");
675+
// We already visited the children.
676+
return Action::SkipChildren(SE);
686677
} else if (auto *ISL = dyn_cast<InterpolatedStringLiteralExpr>(E)) {
687678
// Don't visit the child expressions directly. Instead visit the arguments
688679
// of each appendStringLiteral/appendInterpolation CallExpr so we don't
@@ -694,17 +685,7 @@ ASTWalker::PreWalkResult<Expr *> ModelASTWalker::walkToExprPre(Expr *E) {
694685
arg.getExpr()->walk(*this);
695686
}
696687
});
697-
// TODO: We should consider changing Action::SkipChildren to still call
698-
// walkToExprPost, which would eliminate the need for this.
699-
auto postWalkResult = walkToExprPost(E);
700-
switch (postWalkResult.Action.Action) {
701-
case PostWalkAction::Stop:
702-
return Action::Stop();
703-
case PostWalkAction::Continue:
704-
// We already visited the children.
705-
return Action::SkipNode(*postWalkResult.Value);
706-
}
707-
llvm_unreachable("Unhandled case in switch!");
688+
return Action::SkipChildren(E);
708689
}
709690

710691
return Action::Continue(E);
@@ -845,10 +826,9 @@ ASTWalker::PreWalkResult<Stmt *> ModelASTWalker::walkToStmtPre(Stmt *S) {
845826
assert(RetS == Body);
846827
(void)RetS;
847828
}
848-
walkToStmtPost(DeferS);
849829
}
850830
// Already walked children.
851-
return Action::SkipNode(DeferS);
831+
return Action::SkipChildren(DeferS);
852832
}
853833

854834
return Action::Continue(S);

‎lib/SIL/IR/SILProfiler.cpp‎

Lines changed: 6 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,7 @@ shouldWalkIntoExpr(Expr *E, ASTWalker::ParentTy Parent, SILDeclRef Constant) {
232232
// SILDeclRef is not for a closure, as it could be for a property
233233
// initializer instead.
234234
if (!Parent.isNull() || !Constant || !Constant.getAbstractClosureExpr())
235-
return Action::SkipNode(E);
235+
return Action::SkipChildren(E);
236236
}
237237
return Action::Continue(E);
238238
}
@@ -373,14 +373,7 @@ struct MapRegionCounters : public ASTWalker {
373373
if (isa<LazyInitializerExpr>(E))
374374
mapRegion(E);
375375

376-
auto WalkResult = shouldWalkIntoExpr(E, Parent, Constant);
377-
if (WalkResult.Action.Action == PreWalkAction::SkipNode) {
378-
// We need to manually do the post-visit here since the ASTWalker will
379-
// skip it.
380-
// FIXME: The ASTWalker should do a post-visit.
381-
walkToExprPost(E);
382-
}
383-
return WalkResult;
376+
return shouldWalkIntoExpr(E, Parent, Constant);
384377
}
385378

386379
PostWalkResult<Expr *> walkToExprPost(Expr *E) override {
@@ -795,14 +788,7 @@ struct PGOMapping : public ASTWalker {
795788
if (isa<LazyInitializerExpr>(E))
796789
setKnownExecutionCount(E);
797790

798-
auto WalkResult = shouldWalkIntoExpr(E, Parent, Constant);
799-
if (WalkResult.Action.Action == PreWalkAction::SkipNode) {
800-
// We need to manually do the post-visit here since the ASTWalker will
801-
// skip it.
802-
// FIXME: The ASTWalker should do a post-visit.
803-
walkToExprPost(E);
804-
}
805-
return WalkResult;
791+
return shouldWalkIntoExpr(E, Parent, Constant);
806792
}
807793

808794
PostWalkResult<Expr *> walkToExprPost(Expr *E) override {
@@ -1326,9 +1312,7 @@ struct CoverageMapping : public ASTWalker {
13261312
replaceCount(AfterIf, getEndLoc(IS));
13271313
}
13281314
// Already visited the children.
1329-
// FIXME: The ASTWalker should do a post-walk for SkipChildren.
1330-
walkToStmtPost(S);
1331-
return Action::SkipNode(S);
1315+
return Action::SkipChildren(S);
13321316

13331317
} else if (auto *GS = dyn_cast<GuardStmt>(S)) {
13341318
assignCounter(GS, CounterExpr::Zero());
@@ -1549,18 +1533,9 @@ struct CoverageMapping : public ASTWalker {
15491533
Else->walk(*this);
15501534
}
15511535
// Already visited the children.
1552-
// FIXME: The ASTWalker should do a post-walk for SkipChildren.
1553-
walkToExprPost(TE);
1554-
return Action::SkipNode(TE);
1555-
}
1556-
auto WalkResult = shouldWalkIntoExpr(E, Parent, Constant);
1557-
if (WalkResult.Action.Action == PreWalkAction::SkipNode) {
1558-
// We need to manually do the post-visit here since the ASTWalker will
1559-
// skip it.
1560-
// FIXME: The ASTWalker should do a post-visit.
1561-
walkToExprPost(E);
1536+
return Action::SkipChildren(TE);
15621537
}
1563-
return WalkResult;
1538+
return shouldWalkIntoExpr(E, Parent, Constant);
15641539
}
15651540

15661541
PostWalkResult<Expr *> walkToExprPost(Expr *E) override {

‎lib/Sema/TypeCheckAvailability.cpp‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3188,19 +3188,14 @@ class ExprAvailabilityWalker : public ASTWalker {
31883188

31893189
ExprStack.push_back(E);
31903190

3191-
auto skipChildren = [&]() {
3192-
ExprStack.pop_back();
3193-
return Action::SkipNode(E);
3194-
};
3195-
31963191
if (auto DR = dyn_cast<DeclRefExpr>(E)) {
31973192
diagnoseDeclRefAvailability(DR->getDeclRef(), DR->getSourceRange(),
31983193
getEnclosingApplyExpr(), llvm::None);
31993194
maybeDiagStorageAccess(DR->getDecl(), DR->getSourceRange(), DC);
32003195
}
32013196
if (auto MR = dyn_cast<MemberRefExpr>(E)) {
32023197
walkMemberRef(MR);
3203-
return skipChildren();
3198+
return Action::SkipChildren(E);
32043199
}
32053200
if (auto OCDR = dyn_cast<OtherConstructorDeclRefExpr>(E))
32063201
diagnoseDeclRefAvailability(OCDR->getDeclRef(),
@@ -3252,11 +3247,11 @@ class ExprAvailabilityWalker : public ASTWalker {
32523247
}
32533248
if (auto A = dyn_cast<AssignExpr>(E)) {
32543249
walkAssignExpr(A);
3255-
return skipChildren();
3250+
return Action::SkipChildren(E);
32563251
}
32573252
if (auto IO = dyn_cast<InOutExpr>(E)) {
32583253
walkInOutExpr(IO);
3259-
return skipChildren();
3254+
return Action::SkipChildren(E);
32603255
}
32613256
if (auto T = dyn_cast<TypeExpr>(E)) {
32623257
if (!T->isImplicit()) {
@@ -3282,7 +3277,7 @@ class ExprAvailabilityWalker : public ASTWalker {
32823277
if (AbstractClosureExpr *closure = dyn_cast<AbstractClosureExpr>(E)) {
32833278
if (shouldWalkIntoClosure(closure)) {
32843279
walkAbstractClosure(closure);
3285-
return skipChildren();
3280+
return Action::SkipChildren(E);
32863281
}
32873282
}
32883283

0 commit comments

Comments
 (0)