IDE: Clean up traversal logic in SourceEntityWalker

Try to make this a bit more self-documenting.

Author: slavapestov

lib/IDE/SourceEntityWalker.cpp

@@ -246,14 +246,23 @@ static SemaReferenceKind getReferenceKind(Expr *Parent, Expr *E) {
 std::pair<bool, Expr *> SemaAnnotator::walkToExprPre(Expr *E) {
   assert(E);
 
+  std::pair<bool, Expr *> stopTraversal = { false, nullptr };
+  std::pair<bool, Expr *> skipChildren = { false, E };
+
+  auto doSkipChildren = [&]() -> std::pair<bool, Expr *> {
+    if (!SEWalker.walkToExprPost(E))
+      return stopTraversal;
+    return skipChildren;
+  };
+
   if (isDone())
-    return { false, nullptr };
+    return stopTraversal;
 
   if (ExprsToSkip.count(E) != 0)
-    return { false, E };
+    return skipChildren;
 
   if (!SEWalker.walkToExprPre(E))
-    return { false, E };
+    return skipChildren;
 
   if (auto *CtorRefE = dyn_cast<ConstructorRefCallExpr>(E))
     CtorRefs.push_back(CtorRefE);
@@ -262,11 +271,10 @@ std::pair<bool, Expr *> SemaAnnotator::walkToExprPre(Expr *E) {
     if (auto *SubExpr = ACE->getUnwrappedCurryThunkExpr()) {
       if (auto *DRE = dyn_cast<DeclRefExpr>(SubExpr)) {
         if (!passReference(DRE->getDecl(), DRE->getType(),
-                                  DRE->getNameLoc(),
-                          ReferenceMetaData(getReferenceKind(Parent.getAsExpr(), DRE),
-                                            OpAccess))) {
-          return { false, nullptr };
-        }
+                           DRE->getNameLoc(),
+                           ReferenceMetaData(getReferenceKind(Parent.getAsExpr(), DRE),
+                                             OpAccess)))
+          return stopTraversal;
 
         return { true, E };
       }
@@ -300,12 +308,12 @@ std::pair<bool, Expr *> SemaAnnotator::walkToExprPre(Expr *E) {
     if (auto *module = dyn_cast<ModuleDecl>(DRE->getDecl())) {
       if (!passReference(ModuleEntity(module),
                          {module->getName(), E->getLoc()}))
-        return { false, nullptr };
+        return stopTraversal;
     } else if (!passReference(DRE->getDecl(), DRE->getType(),
                               DRE->getNameLoc(),
                       ReferenceMetaData(getReferenceKind(Parent.getAsExpr(), DRE),
                                         OpAccess))) {
-      return { false, nullptr };
+      return stopTraversal;
     }
   } else if (auto *MRE = dyn_cast<MemberRefExpr>(E)) {
     {
@@ -324,31 +332,29 @@ std::pair<bool, Expr *> SemaAnnotator::walkToExprPre(Expr *E) {
 
       // Visit in source order.
       if (!MRE->getBase()->walk(*this))
-        return { false, nullptr };
+        return stopTraversal;
     }
 
     if (!passReference(MRE->getMember().getDecl(), MRE->getType(),
                        MRE->getNameLoc(),
                        ReferenceMetaData(SemaReferenceKind::DeclMemberRef,
                                          OpAccess)))
-      return { false, nullptr };
+      return stopTraversal;
 
     // We already visited the children.
-    if (!walkToExprPost(E))
-      return { false, nullptr };
-    return { false, E };
+    return doSkipChildren();
 
   } else if (auto OtherCtorE = dyn_cast<OtherConstructorDeclRefExpr>(E)) {
     if (!passReference(OtherCtorE->getDecl(), OtherCtorE->getType(),
                        OtherCtorE->getConstructorLoc(),
                        ReferenceMetaData(SemaReferenceKind::DeclConstructorRef,
                                          OpAccess)))
-      return { false, nullptr };
+      return stopTraversal;
 
   } else if (auto *SE = dyn_cast<SubscriptExpr>(E)) {
     // Visit in source order.
     if (!SE->getBase()->walk(*this))
-      return { false, nullptr };
+      return stopTraversal;
 
     ValueDecl *SubscrD = nullptr;
     if (SE->hasDecl())
@@ -359,21 +365,19 @@ std::pair<bool, Expr *> SemaAnnotator::walkToExprPre(Expr *E) {
 
     if (SubscrD) {
       if (!passSubscriptReference(SubscrD, E->getLoc(), data, true))
-        return { false, nullptr };
+        return stopTraversal;
     }
 
     if (!SE->getIndex()->walk(*this))
-      return { false, nullptr };
+      return stopTraversal;
 
     if (SubscrD) {
       if (!passSubscriptReference(SubscrD, E->getEndLoc(), data, false))
-        return { false, nullptr };
+        return stopTraversal;
     }
 
     // We already visited the children.
-    if (!walkToExprPost(E))
-      return { false, nullptr };
-    return { false, E };
+    return doSkipChildren();
 
   } else if (auto *KPE = dyn_cast<KeyPathExpr>(E)) {
     for (auto &component : KPE->getComponents()) {
@@ -406,60 +410,54 @@ std::pair<bool, Expr *> SemaAnnotator::walkToExprPre(Expr *E) {
   } else if (auto *BinE = dyn_cast<BinaryExpr>(E)) {
     // Visit in source order.
     if (!BinE->getArg()->getElement(0)->walk(*this))
-      return { false, nullptr };
+      return stopTraversal;
     if (!BinE->getFn()->walk(*this))
-      return { false, nullptr };
+      return stopTraversal;
     if (!BinE->getArg()->getElement(1)->walk(*this))
-      return { false, nullptr };
+      return stopTraversal;
 
     // We already visited the children.
-    if (!walkToExprPost(E))
-      return { false, nullptr };
-    return { false, E };
+    return doSkipChildren();
 
   } else if (auto TupleE = dyn_cast<TupleExpr>(E)) {
     if (auto CallE = dyn_cast_or_null<CallExpr>(Parent.getAsExpr())) {
       if (!passCallArgNames(CallE->getFn(), TupleE))
-        return { false, nullptr };
+        return stopTraversal;
     }
   } else if (auto IOE = dyn_cast<InOutExpr>(E)) {
     llvm::SaveAndRestore<Optional<AccessKind>>
       C(this->OpAccess, AccessKind::ReadWrite);
 
     if (!IOE->getSubExpr()->walk(*this))
-      return { false, nullptr };
+      return stopTraversal;
 
     // We already visited the children.
     if (!walkToExprPost(E))
-      return { false, nullptr };
-    return { false, E };
+      return stopTraversal;
+    return skipChildren;
   } else if (auto LE = dyn_cast<LoadExpr>(E)) {
     llvm::SaveAndRestore<Optional<AccessKind>>
       C(this->OpAccess, AccessKind::Read);
 
     if (!LE->getSubExpr()->walk(*this))
-      return { false, nullptr };
+      return stopTraversal;
 
     // We already visited the children.
-    if (!walkToExprPost(E))
-      return { false, nullptr };
-    return { false, E };
+    return doSkipChildren();
   } else if (auto AE = dyn_cast<AssignExpr>(E)) {
     {
       llvm::SaveAndRestore<Optional<AccessKind>>
         C(this->OpAccess, AccessKind::Write);
 
       if (AE->getDest() && !AE->getDest()->walk(*this))
-        return { false, nullptr };
+        return stopTraversal;
     }
 
     if (AE->getSrc() && !AE->getSrc()->walk(*this))
-      return { false, nullptr };
+      return stopTraversal;
 
     // We already visited the children.
-    if (!walkToExprPost(E))
-      return { false, nullptr };
-    return { false, E };
+    return doSkipChildren();
   } else if (auto OEE = dyn_cast<OpenExistentialExpr>(E)) {
     // Record opaque value.
     OpaqueValueMap[OEE->getOpaqueValue()] = OEE->getExistentialValue();
@@ -468,44 +466,39 @@ std::pair<bool, Expr *> SemaAnnotator::walkToExprPre(Expr *E) {
     };
 
     if (!OEE->getSubExpr()->walk(*this))
-      return { false, nullptr };
-    if (!walkToExprPost(E))
-      return { false, nullptr };
-    return { false, E };
+      return stopTraversal;
+
+    return doSkipChildren();
   } else if (auto MTEE = dyn_cast<MakeTemporarilyEscapableExpr>(E)) {
     // Manually walk to original arguments in order. We don't handle
     // OpaqueValueExpr here.
 
     // Original non-escaping closure.
     if (!MTEE->getNonescapingClosureValue()->walk(*this))
-      return { false, nullptr };
+      return stopTraversal;
 
     // Body, which is called by synthesized CallExpr.
     auto *callExpr = cast<CallExpr>(MTEE->getSubExpr());
     if (!callExpr->getFn()->walk(*this))
-      return { false, nullptr };
+      return stopTraversal;
 
-    if (!walkToExprPost(E))
-      return { false, nullptr };
-    return { false, E };
+    return doSkipChildren();
   } else if (auto CUCE = dyn_cast<CollectionUpcastConversionExpr>(E)) {
     // Ignore conversion expressions. We don't handle OpaqueValueExpr here
     // because it's only in conversion expressions. Instead, just walk into
     // sub expression.
     if (!CUCE->getSubExpr()->walk(*this))
-      return { false, nullptr };
-    if (!walkToExprPost(E))
-      return { false, nullptr };
-    return { false, E };
+      return stopTraversal;
+
+    return doSkipChildren();
   } else if (auto OVE = dyn_cast<OpaqueValueExpr>(E)) {
     // Walk into mapped value.
     auto value = OpaqueValueMap.find(OVE);
     if (value != OpaqueValueMap.end()) {
       if (!value->second->walk(*this))
-        return { false, nullptr };
-      if (!walkToExprPost(E))
-        return { false, nullptr };
-      return { false, E };
+        return stopTraversal;
+
+      return doSkipChildren();
     }
   }