[ASTWalker] Don't visit capture list vars twice

Previously we were walking them once when visiting
the capture list, and then again as a part of the
pattern binding decl. Change the logic to only
visit them as a part of their pattern binding decl.

Author: hamishknight

lib/AST/ASTWalker.cpp

@@ -802,8 +802,11 @@ class Traversal : public ASTVisitor<Traversal, Expr*, Stmt*,
           else
             return nullptr;
         }
-      } else if (doIt(c.Var) || doIt(c.Init)) {
-        return nullptr;
+      } else {
+        // Note we do not walk c.Var here, as it'll be visited as a part of the
+        // PatternBindingDecl.
+        if (doIt(c.Init))
+          return nullptr;
       }
     }
 

lib/IDE/SyntaxModel.cpp

@@ -668,23 +668,6 @@ std::pair<bool, Expr *> ModelASTWalker::walkToExprPre(Expr *E) {
 
     pushStructureNode(SN, Closure);
 
-  } else if (auto *CLE = dyn_cast<CaptureListExpr>(E)) {
-    // The ASTWalker visits captured variables twice, from a `CaptureListEntry` they are visited
-    // from the `VarDecl` and the `PatternBindingDecl` entries.
-    // We take over visitation here to avoid walking the `PatternBindingDecl` ones.
-    for (auto c : CLE->getCaptureList()) {
-      if (auto *VD = c.Var) {
-        // We're skipping over the PatternBindingDecl so we need to handle the
-        // the VarDecl's attributes that we'd normally process visiting the PBD.
-        if (!handleAttrs(VD->getAttrs()))
-          return { false, nullptr };
-        VD->walk(*this);
-      }
-    }
-    if (auto *CE = CLE->getClosureBody())
-      CE->walk(*this);
-    return { false, walkToExprPost(E) };
-
   } else if (auto SE = dyn_cast<SequenceExpr>(E)) {
     // In SequenceExpr, explicit cast expressions (e.g. 'as', 'is') appear
     // twice. Skip pointers we've already seen.

lib/SILGen/SILGenExpr.cpp

@@ -1568,10 +1568,8 @@ ManagedValue emitCFunctionPointer(SILGenFunction &SGF,
     // Ensure that weak captures are in a separate scope.
     DebugScope scope(SGF, CleanupLocation(captureList));
     // CaptureListExprs evaluate their bound variables.
-    for (auto capture : captureList->getCaptureList()) {
-      SGF.visit(capture.Var);
+    for (auto capture : captureList->getCaptureList())
       SGF.visit(capture.Init);
-    }
 
     // Emit the closure body.
     auto *closure = captureList->getClosureBody();
@@ -2418,10 +2416,8 @@ RValue RValueEmitter::visitCaptureListExpr(CaptureListExpr *E, SGFContext C) {
   // Ensure that weak captures are in a separate scope.
   DebugScope scope(SGF, CleanupLocation(E));
   // CaptureListExprs evaluate their bound variables.
-  for (auto capture : E->getCaptureList()) {
-    SGF.visit(capture.Var);
+  for (auto capture : E->getCaptureList())
     SGF.visit(capture.Init);
-  }
 
   // Then they evaluate to their body.
   return visit(E->getClosureBody(), C);

lib/Sema/MiscDiagnostics.cpp

@@ -2492,26 +2492,16 @@ class VarDeclUsageChecker : public ASTWalker {
     // If this is a VarDecl, then add it to our list of things to track.
     if (auto *vd = dyn_cast<VarDecl>(D)) {
       if (shouldTrackVarDecl(vd)) {
-        // Inline constructor.
-        auto defaultFlags = [&]() -> unsigned {
-          // If this VarDecl is nested inside of a CaptureListExpr, remember
-          // that fact for better diagnostics.
-          auto parentAsExpr = Parent.getAsExpr();
-          if (parentAsExpr && isa<CaptureListExpr>(parentAsExpr))
-            return RK_CaptureList | RK_Defined;
-          // Otherwise, return none.
-          return RK_Defined;
-        }();
-
-        if (!vd->isImplicit()) {
-          if (auto *childVd =
-                  vd->getCorrespondingCaseBodyVariable().getPtrOrNull()) {
-            // Child vars are never in capture lists.
-            assert(defaultFlags == RK_Defined);
-            VarDecls[childVd] |= RK_Defined;
-          }
+        unsigned flags = RK_Defined;
+        if (vd->isCaptureList())
+          flags |= RK_CaptureList;
+
+        if (auto childVd = vd->getCorrespondingCaseBodyVariable()) {
+          // Child vars are never in capture lists.
+          assert(flags == RK_Defined);
+          addMark(childVd.get(), flags);
         }
-        VarDecls[vd] |= defaultFlags;
+        addMark(vd, flags);
       }
     }