[PATCH] D82486: RecursiveASTVisitor: don't call WalkUp unnecessarily in post-order traversal

2020-07-03 Thread Dmitri Gribenko via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG94454442c3c1: RecursiveASTVisitor: dont call WalkUp 
unnecessarily in post-order traversal (authored by gribozavr).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82486/new/

https://reviews.llvm.org/D82486

Files:
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp

Index: clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp
===
--- clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp
+++ clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp
@@ -154,22 +154,17 @@
   R"txt(
 TraverseIntegerLiteral IntegerLiteral(1)
   WalkUpFromStmt IntegerLiteral(1)
-WalkUpFromStmt IntegerLiteral(1)
 TraverseIntegerLiteral IntegerLiteral(2)
   WalkUpFromStmt IntegerLiteral(2)
-WalkUpFromStmt IntegerLiteral(2)
 TraverseIntegerLiteral IntegerLiteral(3)
   WalkUpFromStmt IntegerLiteral(3)
-WalkUpFromStmt IntegerLiteral(3)
 WalkUpFromStmt BinaryOperator(+)
 WalkUpFromStmt DeclRefExpr(add)
 WalkUpFromStmt ImplicitCastExpr
 TraverseIntegerLiteral IntegerLiteral(4)
   WalkUpFromStmt IntegerLiteral(4)
-WalkUpFromStmt IntegerLiteral(4)
 TraverseIntegerLiteral IntegerLiteral(5)
   WalkUpFromStmt IntegerLiteral(5)
-WalkUpFromStmt IntegerLiteral(5)
 WalkUpFromStmt CallExpr(add)
 WalkUpFromStmt CompoundStmt
 )txt"));
@@ -258,23 +253,14 @@
   WalkUpFromIntegerLiteral IntegerLiteral(1)
 WalkUpFromExpr IntegerLiteral(1)
   WalkUpFromStmt IntegerLiteral(1)
-WalkUpFromIntegerLiteral IntegerLiteral(1)
-  WalkUpFromExpr IntegerLiteral(1)
-WalkUpFromStmt IntegerLiteral(1)
 TraverseIntegerLiteral IntegerLiteral(2)
   WalkUpFromIntegerLiteral IntegerLiteral(2)
 WalkUpFromExpr IntegerLiteral(2)
   WalkUpFromStmt IntegerLiteral(2)
-WalkUpFromIntegerLiteral IntegerLiteral(2)
-  WalkUpFromExpr IntegerLiteral(2)
-WalkUpFromStmt IntegerLiteral(2)
 TraverseIntegerLiteral IntegerLiteral(3)
   WalkUpFromIntegerLiteral IntegerLiteral(3)
 WalkUpFromExpr IntegerLiteral(3)
   WalkUpFromStmt IntegerLiteral(3)
-WalkUpFromIntegerLiteral IntegerLiteral(3)
-  WalkUpFromExpr IntegerLiteral(3)
-WalkUpFromStmt IntegerLiteral(3)
 WalkUpFromExpr BinaryOperator(+)
   WalkUpFromStmt BinaryOperator(+)
 WalkUpFromExpr DeclRefExpr(add)
@@ -285,16 +271,10 @@
   WalkUpFromIntegerLiteral IntegerLiteral(4)
 WalkUpFromExpr IntegerLiteral(4)
   WalkUpFromStmt IntegerLiteral(4)
-WalkUpFromIntegerLiteral IntegerLiteral(4)
-  WalkUpFromExpr IntegerLiteral(4)
-WalkUpFromStmt IntegerLiteral(4)
 TraverseIntegerLiteral IntegerLiteral(5)
   WalkUpFromIntegerLiteral IntegerLiteral(5)
 WalkUpFromExpr IntegerLiteral(5)
   WalkUpFromStmt IntegerLiteral(5)
-WalkUpFromIntegerLiteral IntegerLiteral(5)
-  WalkUpFromExpr IntegerLiteral(5)
-WalkUpFromStmt IntegerLiteral(5)
 WalkUpFromExpr CallExpr(add)
   WalkUpFromStmt CallExpr(add)
 WalkUpFromStmt CompoundStmt
@@ -436,12 +416,13 @@
 WalkUpFromStmt IntegerLiteral(3)
 )txt"));
 
+  // FIXME: The following log should include a call to WalkUpFromStmt for
+  // UnaryOperator(-).
   EXPECT_TRUE(visitorCallbackLogEqual(
   RecordingVisitor(ShouldTraversePostOrder::Yes), Code,
   R"txt(
 WalkUpFromStmt IntegerLiteral(1)
 WalkUpFromStmt IntegerLiteral(2)
-WalkUpFromStmt UnaryOperator(-)
 WalkUpFromStmt IntegerLiteral(3)
 WalkUpFromStmt CompoundStmt
 )txt"));
@@ -507,6 +488,8 @@
   WalkUpFromStmt IntegerLiteral(3)
 )txt"));
 
+  // FIXME: The following log should include a call to WalkUpFromUnaryOperator
+  // for UnaryyOperator(-).
   EXPECT_TRUE(visitorCallbackLogEqual(
   RecordingVisitor(ShouldTraversePostOrder::Yes), Code,
   R"txt(
@@ -514,9 +497,6 @@
   WalkUpFromStmt IntegerLiteral(1)
 WalkUpFromExpr IntegerLiteral(2)
   WalkUpFromStmt IntegerLiteral(2)
-WalkUpFromUnaryOperator UnaryOperator(-)
-  WalkUpFromExpr UnaryOperator(-)
-WalkUpFromStmt UnaryOperator(-)
 WalkUpFromExpr IntegerLiteral(3)
   WalkUpFromStmt IntegerLiteral(3)
 WalkUpFromStmt CompoundStmt
@@ -817,13 +797,14 @@
 WalkUpFromStmt IntegerLiteral(4)
 )txt"));
 
+  // FIXME: The following log should include a call to WalkUpFromStmt for
+  // BinaryOperator(+).
   EXPECT_TRUE(visitorCallbackLogEqual(
   RecordingVisitor(ShouldTraversePostOrder::Yes), Code,
   R"txt(
 WalkUpFromStmt IntegerLiteral(1)
 WalkUpFromStmt IntegerLiteral(2)
 WalkUpFromStmt IntegerLiteral(3)
-WalkUpFromStmt BinaryOperator(+)
 WalkUpFromStmt IntegerLiteral(4)
 WalkUpFromStmt CompoundStmt
 )txt"));
@@ -891,6 +872,7 @@
   WalkUpFromStmt IntegerLiteral(4)
 )txt"));
 
+  // FIXME: The following log should include a call to WalkUpFromBinaryOperator.
   EXPECT_TRUE(visitorCallbackLogEqual(
   RecordingVisitor(ShouldTraversePostOrder::Yes), Code,
   R"txt(
@@ -900,9 +882,6 @@

[PATCH] D82486: RecursiveASTVisitor: don't call WalkUp unnecessarily in post-order traversal

2020-06-30 Thread Dmitri Gribenko via Phabricator via cfe-commits 
gribozavr updated this revision to Diff 274481.
gribozavr added a comment.

Rebased the patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82486/new/

https://reviews.llvm.org/D82486

Files:
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp

Index: clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp
===
--- clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp
+++ clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp
@@ -154,22 +154,17 @@
   R"txt(
 TraverseIntegerLiteral IntegerLiteral(1)
   WalkUpFromStmt IntegerLiteral(1)
-WalkUpFromStmt IntegerLiteral(1)
 TraverseIntegerLiteral IntegerLiteral(2)
   WalkUpFromStmt IntegerLiteral(2)
-WalkUpFromStmt IntegerLiteral(2)
 TraverseIntegerLiteral IntegerLiteral(3)
   WalkUpFromStmt IntegerLiteral(3)
-WalkUpFromStmt IntegerLiteral(3)
 WalkUpFromStmt BinaryOperator(+)
 WalkUpFromStmt DeclRefExpr(add)
 WalkUpFromStmt ImplicitCastExpr
 TraverseIntegerLiteral IntegerLiteral(4)
   WalkUpFromStmt IntegerLiteral(4)
-WalkUpFromStmt IntegerLiteral(4)
 TraverseIntegerLiteral IntegerLiteral(5)
   WalkUpFromStmt IntegerLiteral(5)
-WalkUpFromStmt IntegerLiteral(5)
 WalkUpFromStmt CallExpr(add)
 WalkUpFromStmt CompoundStmt
 )txt"));
@@ -258,23 +253,14 @@
   WalkUpFromIntegerLiteral IntegerLiteral(1)
 WalkUpFromExpr IntegerLiteral(1)
   WalkUpFromStmt IntegerLiteral(1)
-WalkUpFromIntegerLiteral IntegerLiteral(1)
-  WalkUpFromExpr IntegerLiteral(1)
-WalkUpFromStmt IntegerLiteral(1)
 TraverseIntegerLiteral IntegerLiteral(2)
   WalkUpFromIntegerLiteral IntegerLiteral(2)
 WalkUpFromExpr IntegerLiteral(2)
   WalkUpFromStmt IntegerLiteral(2)
-WalkUpFromIntegerLiteral IntegerLiteral(2)
-  WalkUpFromExpr IntegerLiteral(2)
-WalkUpFromStmt IntegerLiteral(2)
 TraverseIntegerLiteral IntegerLiteral(3)
   WalkUpFromIntegerLiteral IntegerLiteral(3)
 WalkUpFromExpr IntegerLiteral(3)
   WalkUpFromStmt IntegerLiteral(3)
-WalkUpFromIntegerLiteral IntegerLiteral(3)
-  WalkUpFromExpr IntegerLiteral(3)
-WalkUpFromStmt IntegerLiteral(3)
 WalkUpFromExpr BinaryOperator(+)
   WalkUpFromStmt BinaryOperator(+)
 WalkUpFromExpr DeclRefExpr(add)
@@ -285,16 +271,10 @@
   WalkUpFromIntegerLiteral IntegerLiteral(4)
 WalkUpFromExpr IntegerLiteral(4)
   WalkUpFromStmt IntegerLiteral(4)
-WalkUpFromIntegerLiteral IntegerLiteral(4)
-  WalkUpFromExpr IntegerLiteral(4)
-WalkUpFromStmt IntegerLiteral(4)
 TraverseIntegerLiteral IntegerLiteral(5)
   WalkUpFromIntegerLiteral IntegerLiteral(5)
 WalkUpFromExpr IntegerLiteral(5)
   WalkUpFromStmt IntegerLiteral(5)
-WalkUpFromIntegerLiteral IntegerLiteral(5)
-  WalkUpFromExpr IntegerLiteral(5)
-WalkUpFromStmt IntegerLiteral(5)
 WalkUpFromExpr CallExpr(add)
   WalkUpFromStmt CallExpr(add)
 WalkUpFromStmt CompoundStmt
@@ -436,12 +416,13 @@
 WalkUpFromStmt IntegerLiteral(3)
 )txt"));
 
+  // FIXME: The following log should include a call to WalkUpFromStmt for
+  // UnaryOperator(-).
   EXPECT_TRUE(visitorCallbackLogEqual(
   RecordingVisitor(ShouldTraversePostOrder::Yes), Code,
   R"txt(
 WalkUpFromStmt IntegerLiteral(1)
 WalkUpFromStmt IntegerLiteral(2)
-WalkUpFromStmt UnaryOperator(-)
 WalkUpFromStmt IntegerLiteral(3)
 WalkUpFromStmt CompoundStmt
 )txt"));
@@ -507,6 +488,8 @@
   WalkUpFromStmt IntegerLiteral(3)
 )txt"));
 
+  // FIXME: The following log should include a call to WalkUpFromUnaryOperator
+  // for UnaryyOperator(-).
   EXPECT_TRUE(visitorCallbackLogEqual(
   RecordingVisitor(ShouldTraversePostOrder::Yes), Code,
   R"txt(
@@ -514,9 +497,6 @@
   WalkUpFromStmt IntegerLiteral(1)
 WalkUpFromExpr IntegerLiteral(2)
   WalkUpFromStmt IntegerLiteral(2)
-WalkUpFromUnaryOperator UnaryOperator(-)
-  WalkUpFromExpr UnaryOperator(-)
-WalkUpFromStmt UnaryOperator(-)
 WalkUpFromExpr IntegerLiteral(3)
   WalkUpFromStmt IntegerLiteral(3)
 WalkUpFromStmt CompoundStmt
@@ -817,13 +797,14 @@
 WalkUpFromStmt IntegerLiteral(4)
 )txt"));
 
+  // FIXME: The following log should include a call to WalkUpFromStmt for
+  // BinaryOperator(+).
   EXPECT_TRUE(visitorCallbackLogEqual(
   RecordingVisitor(ShouldTraversePostOrder::Yes), Code,
   R"txt(
 WalkUpFromStmt IntegerLiteral(1)
 WalkUpFromStmt IntegerLiteral(2)
 WalkUpFromStmt IntegerLiteral(3)
-WalkUpFromStmt BinaryOperator(+)
 WalkUpFromStmt IntegerLiteral(4)
 WalkUpFromStmt CompoundStmt
 )txt"));
@@ -891,6 +872,7 @@
   WalkUpFromStmt IntegerLiteral(4)
 )txt"));
 
+  // FIXME: The following log should include a call to WalkUpFromBinaryOperator.
   EXPECT_TRUE(visitorCallbackLogEqual(
   RecordingVisitor(ShouldTraversePostOrder::Yes), Code,
   R"txt(
@@ -900,9 +882,6 @@
   WalkUpFromStmt IntegerLiteral(2)
 WalkUpFromExpr IntegerLiteral(3)
   WalkUpFromStmt IntegerLiteral(3)

[PATCH] D82486: RecursiveASTVisitor: don't call WalkUp unnecessarily in post-order traversal

2020-06-29 Thread Dmitri Gribenko via Phabricator via cfe-commits 
gribozavr updated this revision to Diff 274032.
gribozavr added a comment.

Added a FIXME about a regression.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82486/new/

https://reviews.llvm.org/D82486

Files:
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp

Index: clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp
===
--- clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp
+++ clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp
@@ -149,22 +149,17 @@
   R"txt(
 TraverseIntegerLiteral IntegerLiteral(1)
   WalkUpFromStmt IntegerLiteral(1)
-WalkUpFromStmt IntegerLiteral(1)
 TraverseIntegerLiteral IntegerLiteral(2)
   WalkUpFromStmt IntegerLiteral(2)
-WalkUpFromStmt IntegerLiteral(2)
 TraverseIntegerLiteral IntegerLiteral(3)
   WalkUpFromStmt IntegerLiteral(3)
-WalkUpFromStmt IntegerLiteral(3)
 WalkUpFromStmt BinaryOperator(+)
 WalkUpFromStmt DeclRefExpr(add)
 WalkUpFromStmt ImplicitCastExpr
 TraverseIntegerLiteral IntegerLiteral(4)
   WalkUpFromStmt IntegerLiteral(4)
-WalkUpFromStmt IntegerLiteral(4)
 TraverseIntegerLiteral IntegerLiteral(5)
   WalkUpFromStmt IntegerLiteral(5)
-WalkUpFromStmt IntegerLiteral(5)
 WalkUpFromStmt CallExpr(add)
 WalkUpFromStmt CompoundStmt
 )txt"));
@@ -253,23 +248,14 @@
   WalkUpFromIntegerLiteral IntegerLiteral(1)
 WalkUpFromExpr IntegerLiteral(1)
   WalkUpFromStmt IntegerLiteral(1)
-WalkUpFromIntegerLiteral IntegerLiteral(1)
-  WalkUpFromExpr IntegerLiteral(1)
-WalkUpFromStmt IntegerLiteral(1)
 TraverseIntegerLiteral IntegerLiteral(2)
   WalkUpFromIntegerLiteral IntegerLiteral(2)
 WalkUpFromExpr IntegerLiteral(2)
   WalkUpFromStmt IntegerLiteral(2)
-WalkUpFromIntegerLiteral IntegerLiteral(2)
-  WalkUpFromExpr IntegerLiteral(2)
-WalkUpFromStmt IntegerLiteral(2)
 TraverseIntegerLiteral IntegerLiteral(3)
   WalkUpFromIntegerLiteral IntegerLiteral(3)
 WalkUpFromExpr IntegerLiteral(3)
   WalkUpFromStmt IntegerLiteral(3)
-WalkUpFromIntegerLiteral IntegerLiteral(3)
-  WalkUpFromExpr IntegerLiteral(3)
-WalkUpFromStmt IntegerLiteral(3)
 WalkUpFromExpr BinaryOperator(+)
   WalkUpFromStmt BinaryOperator(+)
 WalkUpFromExpr DeclRefExpr(add)
@@ -280,16 +266,10 @@
   WalkUpFromIntegerLiteral IntegerLiteral(4)
 WalkUpFromExpr IntegerLiteral(4)
   WalkUpFromStmt IntegerLiteral(4)
-WalkUpFromIntegerLiteral IntegerLiteral(4)
-  WalkUpFromExpr IntegerLiteral(4)
-WalkUpFromStmt IntegerLiteral(4)
 TraverseIntegerLiteral IntegerLiteral(5)
   WalkUpFromIntegerLiteral IntegerLiteral(5)
 WalkUpFromExpr IntegerLiteral(5)
   WalkUpFromStmt IntegerLiteral(5)
-WalkUpFromIntegerLiteral IntegerLiteral(5)
-  WalkUpFromExpr IntegerLiteral(5)
-WalkUpFromStmt IntegerLiteral(5)
 WalkUpFromExpr CallExpr(add)
   WalkUpFromStmt CallExpr(add)
 WalkUpFromStmt CompoundStmt
@@ -434,13 +414,14 @@
 WalkUpFromStmt IntegerLiteral(5)
 )txt"));
 
+  // FIXME: The following log should include a call to WalkUpFromStmt for
+  // BinaryOperator(+).
   EXPECT_TRUE(visitorCallbackLogEqual(
   RecordingVisitor(ShouldTraversePostOrder::Yes), Code,
   R"txt(
 WalkUpFromStmt IntegerLiteral(1)
 WalkUpFromStmt IntegerLiteral(2)
 WalkUpFromStmt IntegerLiteral(3)
-WalkUpFromStmt BinaryOperator(+)
 WalkUpFromStmt DeclRefExpr(add)
 WalkUpFromStmt ImplicitCastExpr
 WalkUpFromStmt IntegerLiteral(4)
@@ -521,6 +502,8 @@
   WalkUpFromStmt IntegerLiteral(5)
 )txt"));
 
+  // FIXME: The following log should include a call to WalkUpFromStmt for
+  // BinaryOperator(+).
   EXPECT_TRUE(visitorCallbackLogEqual(
   RecordingVisitor(ShouldTraversePostOrder::Yes), Code,
   R"txt(
@@ -530,9 +513,6 @@
   WalkUpFromStmt IntegerLiteral(2)
 WalkUpFromExpr IntegerLiteral(3)
   WalkUpFromStmt IntegerLiteral(3)
-WalkUpFromBinaryOperator BinaryOperator(+)
-  WalkUpFromExpr BinaryOperator(+)
-WalkUpFromStmt BinaryOperator(+)
 WalkUpFromExpr DeclRefExpr(add)
   WalkUpFromStmt DeclRefExpr(add)
 WalkUpFromExpr ImplicitCastExpr
@@ -690,7 +670,6 @@
   WalkUpFromStmt IntegerLiteral(4)
   WalkUpFromStmt IntegerLiteral(5)
   WalkUpFromStmt CallExpr(add)
-WalkUpFromStmt CallExpr(add)
 WalkUpFromStmt CompoundStmt
 )txt"));
 }
@@ -784,9 +763,6 @@
   WalkUpFromCallExpr CallExpr(add)
 WalkUpFromExpr CallExpr(add)
   WalkUpFromStmt CallExpr(add)
-WalkUpFromCallExpr CallExpr(add)
-  WalkUpFromExpr CallExpr(add)
-WalkUpFromStmt CallExpr(add)
 WalkUpFromStmt CompoundStmt
 )txt"));
 }
Index: clang/lib/Tooling/Syntax/BuildTree.cpp
===
--- clang/lib/Tooling/Syntax/BuildTree.cpp
+++ clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -578,15 +578,19 @@
 // RAV traverses it as a statement, we produce invalid node kinds in that
 // case.
 // FIXME: should do this in RAV instead?
-

[PATCH] D82486: RecursiveASTVisitor: don't call WalkUp unnecessarily in post-order traversal

2020-06-29 Thread Dmitri Gribenko via Phabricator via cfe-commits 
gribozavr updated this revision to Diff 274027.
gribozavr added a comment.

Rebased the patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82486/new/

https://reviews.llvm.org/D82486

Files:
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp

Index: clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp
===
--- clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp
+++ clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp
@@ -149,22 +149,17 @@
   R"txt(
 TraverseIntegerLiteral IntegerLiteral(1)
   WalkUpFromStmt IntegerLiteral(1)
-WalkUpFromStmt IntegerLiteral(1)
 TraverseIntegerLiteral IntegerLiteral(2)
   WalkUpFromStmt IntegerLiteral(2)
-WalkUpFromStmt IntegerLiteral(2)
 TraverseIntegerLiteral IntegerLiteral(3)
   WalkUpFromStmt IntegerLiteral(3)
-WalkUpFromStmt IntegerLiteral(3)
 WalkUpFromStmt BinaryOperator(+)
 WalkUpFromStmt DeclRefExpr(add)
 WalkUpFromStmt ImplicitCastExpr
 TraverseIntegerLiteral IntegerLiteral(4)
   WalkUpFromStmt IntegerLiteral(4)
-WalkUpFromStmt IntegerLiteral(4)
 TraverseIntegerLiteral IntegerLiteral(5)
   WalkUpFromStmt IntegerLiteral(5)
-WalkUpFromStmt IntegerLiteral(5)
 WalkUpFromStmt CallExpr(add)
 WalkUpFromStmt CompoundStmt
 )txt"));
@@ -253,23 +248,14 @@
   WalkUpFromIntegerLiteral IntegerLiteral(1)
 WalkUpFromExpr IntegerLiteral(1)
   WalkUpFromStmt IntegerLiteral(1)
-WalkUpFromIntegerLiteral IntegerLiteral(1)
-  WalkUpFromExpr IntegerLiteral(1)
-WalkUpFromStmt IntegerLiteral(1)
 TraverseIntegerLiteral IntegerLiteral(2)
   WalkUpFromIntegerLiteral IntegerLiteral(2)
 WalkUpFromExpr IntegerLiteral(2)
   WalkUpFromStmt IntegerLiteral(2)
-WalkUpFromIntegerLiteral IntegerLiteral(2)
-  WalkUpFromExpr IntegerLiteral(2)
-WalkUpFromStmt IntegerLiteral(2)
 TraverseIntegerLiteral IntegerLiteral(3)
   WalkUpFromIntegerLiteral IntegerLiteral(3)
 WalkUpFromExpr IntegerLiteral(3)
   WalkUpFromStmt IntegerLiteral(3)
-WalkUpFromIntegerLiteral IntegerLiteral(3)
-  WalkUpFromExpr IntegerLiteral(3)
-WalkUpFromStmt IntegerLiteral(3)
 WalkUpFromExpr BinaryOperator(+)
   WalkUpFromStmt BinaryOperator(+)
 WalkUpFromExpr DeclRefExpr(add)
@@ -280,16 +266,10 @@
   WalkUpFromIntegerLiteral IntegerLiteral(4)
 WalkUpFromExpr IntegerLiteral(4)
   WalkUpFromStmt IntegerLiteral(4)
-WalkUpFromIntegerLiteral IntegerLiteral(4)
-  WalkUpFromExpr IntegerLiteral(4)
-WalkUpFromStmt IntegerLiteral(4)
 TraverseIntegerLiteral IntegerLiteral(5)
   WalkUpFromIntegerLiteral IntegerLiteral(5)
 WalkUpFromExpr IntegerLiteral(5)
   WalkUpFromStmt IntegerLiteral(5)
-WalkUpFromIntegerLiteral IntegerLiteral(5)
-  WalkUpFromExpr IntegerLiteral(5)
-WalkUpFromStmt IntegerLiteral(5)
 WalkUpFromExpr CallExpr(add)
   WalkUpFromStmt CallExpr(add)
 WalkUpFromStmt CompoundStmt
@@ -434,13 +414,14 @@
 WalkUpFromStmt IntegerLiteral(5)
 )txt"));
 
+  // FIXME: The following log should include a call to WalkUpFromStmt for
+  // BinaryOperator(+).
   EXPECT_TRUE(visitorCallbackLogEqual(
   RecordingVisitor(ShouldTraversePostOrder::Yes), Code,
   R"txt(
 WalkUpFromStmt IntegerLiteral(1)
 WalkUpFromStmt IntegerLiteral(2)
 WalkUpFromStmt IntegerLiteral(3)
-WalkUpFromStmt BinaryOperator(+)
 WalkUpFromStmt DeclRefExpr(add)
 WalkUpFromStmt ImplicitCastExpr
 WalkUpFromStmt IntegerLiteral(4)
@@ -530,9 +511,6 @@
   WalkUpFromStmt IntegerLiteral(2)
 WalkUpFromExpr IntegerLiteral(3)
   WalkUpFromStmt IntegerLiteral(3)
-WalkUpFromBinaryOperator BinaryOperator(+)
-  WalkUpFromExpr BinaryOperator(+)
-WalkUpFromStmt BinaryOperator(+)
 WalkUpFromExpr DeclRefExpr(add)
   WalkUpFromStmt DeclRefExpr(add)
 WalkUpFromExpr ImplicitCastExpr
@@ -690,7 +668,6 @@
   WalkUpFromStmt IntegerLiteral(4)
   WalkUpFromStmt IntegerLiteral(5)
   WalkUpFromStmt CallExpr(add)
-WalkUpFromStmt CallExpr(add)
 WalkUpFromStmt CompoundStmt
 )txt"));
 }
@@ -784,9 +761,6 @@
   WalkUpFromCallExpr CallExpr(add)
 WalkUpFromExpr CallExpr(add)
   WalkUpFromStmt CallExpr(add)
-WalkUpFromCallExpr CallExpr(add)
-  WalkUpFromExpr CallExpr(add)
-WalkUpFromStmt CallExpr(add)
 WalkUpFromStmt CompoundStmt
 )txt"));
 }
Index: clang/lib/Tooling/Syntax/BuildTree.cpp
===
--- clang/lib/Tooling/Syntax/BuildTree.cpp
+++ clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -578,15 +578,19 @@
 // RAV traverses it as a statement, we produce invalid node kinds in that
 // case.
 // FIXME: should do this in RAV instead?
-if (S->getInit() && !TraverseStmt(S->getInit()))
-  return false;
-if (S->getLoopVariable() && !TraverseDecl(S->getLoopVariable()))
-  return false;
-if (S->getRangeInit() && !TraverseStmt(S->getRangeInit()))
-  return false;
-if (S->getBody() &&

[PATCH] D82486: RecursiveASTVisitor: don't call WalkUp unnecessarily in post-order traversal

2020-06-26 Thread Dmitri Gribenko via Phabricator via cfe-commits 
gribozavr2 marked an inline comment as done.
gribozavr2 added inline comments.



Comment at: clang/include/clang/AST/RecursiveASTVisitor.h:646
+ * receives a DataRecursionQueue, can't call WalkUpFrom after traversing   
\
+ * children because it only enqueues the children. and does not traverse   
\
+ * them. TraverseStmt traverses the enqueued children, and we call 
\

ymandel wrote:
> delete stray period after "children"
Done.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82486/new/

https://reviews.llvm.org/D82486



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D82486: RecursiveASTVisitor: don't call WalkUp unnecessarily in post-order traversal

2020-06-26 Thread Dmitri Gribenko via Phabricator via cfe-commits 
gribozavr updated this revision to Diff 273736.
gribozavr added a comment.

Removed a stray period in a comment.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82486/new/

https://reviews.llvm.org/D82486

Files:
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp

Index: clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp
===
--- clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp
+++ clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp
@@ -117,18 +117,13 @@
   Code,
   R"txt(
 TraverseIntegerLiteral IntegerLiteral(1)
-WalkUpFromStmt IntegerLiteral(1)
 TraverseIntegerLiteral IntegerLiteral(2)
-WalkUpFromStmt IntegerLiteral(2)
 TraverseIntegerLiteral IntegerLiteral(3)
-WalkUpFromStmt IntegerLiteral(3)
 WalkUpFromStmt BinaryOperator(+)
 WalkUpFromStmt DeclRefExpr(add)
 WalkUpFromStmt ImplicitCastExpr
 TraverseIntegerLiteral IntegerLiteral(4)
-WalkUpFromStmt IntegerLiteral(4)
 TraverseIntegerLiteral IntegerLiteral(5)
-WalkUpFromStmt IntegerLiteral(5)
 WalkUpFromStmt CallExpr(add)
 WalkUpFromStmt CompoundStmt
 )txt"));
@@ -189,18 +184,13 @@
   Code,
   R"txt(
 TraverseIntegerLiteral IntegerLiteral(1)
-WalkUpFromIntegerLiteral IntegerLiteral(1)
 TraverseIntegerLiteral IntegerLiteral(2)
-WalkUpFromIntegerLiteral IntegerLiteral(2)
 TraverseIntegerLiteral IntegerLiteral(3)
-WalkUpFromIntegerLiteral IntegerLiteral(3)
 WalkUpFromExpr BinaryOperator(+)
 WalkUpFromExpr DeclRefExpr(add)
 WalkUpFromExpr ImplicitCastExpr
 TraverseIntegerLiteral IntegerLiteral(4)
-WalkUpFromIntegerLiteral IntegerLiteral(4)
 TraverseIntegerLiteral IntegerLiteral(5)
-WalkUpFromIntegerLiteral IntegerLiteral(5)
 WalkUpFromExpr CallExpr(add)
 WalkUpFromStmt CompoundStmt
 )txt"));
@@ -315,7 +305,6 @@
 WalkUpFromStmt IntegerLiteral(1)
 WalkUpFromStmt IntegerLiteral(2)
 WalkUpFromStmt IntegerLiteral(3)
-WalkUpFromStmt BinaryOperator(+)
 WalkUpFromStmt DeclRefExpr(add)
 WalkUpFromStmt ImplicitCastExpr
 WalkUpFromStmt IntegerLiteral(4)
@@ -383,7 +372,6 @@
 WalkUpFromExpr IntegerLiteral(1)
 WalkUpFromExpr IntegerLiteral(2)
 WalkUpFromExpr IntegerLiteral(3)
-WalkUpFromBinaryOperator BinaryOperator(+)
 WalkUpFromExpr DeclRefExpr(add)
 WalkUpFromExpr ImplicitCastExpr
 WalkUpFromExpr IntegerLiteral(4)
@@ -500,7 +488,6 @@
 WalkUpFromStmt IntegerLiteral(3)
 WalkUpFromStmt BinaryOperator(+)
 TraverseCallExpr CallExpr(add)
-WalkUpFromStmt CallExpr(add)
 WalkUpFromStmt CompoundStmt
 )txt"));
 }
@@ -560,7 +547,6 @@
 WalkUpFromExpr IntegerLiteral(3)
 WalkUpFromExpr BinaryOperator(+)
 TraverseCallExpr CallExpr(add)
-WalkUpFromCallExpr CallExpr(add)
 WalkUpFromStmt CompoundStmt
 )txt"));
 }
Index: clang/lib/Tooling/Syntax/BuildTree.cpp
===
--- clang/lib/Tooling/Syntax/BuildTree.cpp
+++ clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -578,15 +578,19 @@
 // RAV traverses it as a statement, we produce invalid node kinds in that
 // case.
 // FIXME: should do this in RAV instead?
-if (S->getInit() && !TraverseStmt(S->getInit()))
-  return false;
-if (S->getLoopVariable() && !TraverseDecl(S->getLoopVariable()))
-  return false;
-if (S->getRangeInit() && !TraverseStmt(S->getRangeInit()))
-  return false;
-if (S->getBody() && !TraverseStmt(S->getBody()))
-  return false;
-return true;
+bool Result = [&, this]() {
+  if (S->getInit() && !TraverseStmt(S->getInit()))
+return false;
+  if (S->getLoopVariable() && !TraverseDecl(S->getLoopVariable()))
+return false;
+  if (S->getRangeInit() && !TraverseStmt(S->getRangeInit()))
+return false;
+  if (S->getBody() && !TraverseStmt(S->getBody()))
+return false;
+  return true;
+}();
+WalkUpFromCXXForRangeStmt(S);
+return Result;
   }
 
   bool TraverseStmt(Stmt *S) {
Index: clang/include/clang/AST/RecursiveASTVisitor.h
===
--- clang/include/clang/AST/RecursiveASTVisitor.h
+++ clang/include/clang/AST/RecursiveASTVisitor.h
@@ -331,6 +331,32 @@
   struct has_same_member_pointer_type
   : std::true_type {};
 
+  template  struct is_same_method_impl {
+template 
+static bool isSameMethod(FirstMethodPtrTy FirstMethodPtr,
+ SecondMethodPtrTy SecondMethodPtr) {
+  return false;
+}
+  };
+
+  template <> struct is_same_method_impl {
+template 
+static bool isSameMethod(FirstMethodPtrTy FirstMethodPtr,
+ SecondMethodPtrTy SecondMethodPtr) {
+  return FirstMethodPtr == SecondMethodPtr;
+}
+  };
+
+  /// Returns true if and only if

[PATCH] D82486: RecursiveASTVisitor: don't call WalkUp unnecessarily in post-order traversal

2020-06-26 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits 
ymandel accepted this revision.
ymandel added inline comments.



Comment at: clang/include/clang/AST/RecursiveASTVisitor.h:646
+ * receives a DataRecursionQueue, can't call WalkUpFrom after traversing   
\
+ * children because it only enqueues the children. and does not traverse   
\
+ * them. TraverseStmt traverses the enqueued children, and we call 
\

delete stray period after "children"


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82486/new/

https://reviews.llvm.org/D82486



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D82486: RecursiveASTVisitor: don't call WalkUp unnecessarily in post-order traversal

2020-06-26 Thread Dmitri Gribenko via Phabricator via cfe-commits 
gribozavr2 marked 2 inline comments as done.
gribozavr2 added inline comments.



Comment at: clang/include/clang/AST/RecursiveASTVisitor.h:335
+  template  struct is_same_method_impl {
+static bool isSameMethod(...) { return false; }
+  };

ymandel wrote:
> Why use var-args rather than spelling out the type arguments like you have on 
> lines 339-341 or, simpler, line 351?
Because all of those things are not relevant. However I do see your point, that 
in some sense it is a trick to reduce the number of characters, but somewhat 
hurting readability, so I changed the signature to be explicit.



Comment at: clang/include/clang/AST/RecursiveASTVisitor.h:339
+  template <> struct is_same_method_impl {
+template  Given that we've established them to be the same type, why two sets of 
> template arguments?
Indeed, simplified the code now!



Comment at: clang/include/clang/AST/RecursiveASTVisitor.h:637
   case Stmt::CLASS##Class: 
\
-TRY_TO(WalkUpFrom##CLASS(static_cast(S))); break;
+if (isSameMethod(::Traverse##CLASS,
\
+ ::Traverse##CLASS)) { 
\

ymandel wrote:
> Do you explain this logic somewhere? Or do you feel it will be obvious to the 
> reader? (I don't have a good intuition about this class to judge).
Added a comment. The RecursiveASTVisitor is full of tricky logic so no, none of 
this is obvious.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82486/new/

https://reviews.llvm.org/D82486



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D82486: RecursiveASTVisitor: don't call WalkUp unnecessarily in post-order traversal

2020-06-26 Thread Dmitri Gribenko via Phabricator via cfe-commits 
gribozavr updated this revision to Diff 273723.
gribozavr added a comment.

Addressed review comments, added more fixes that must be committed in the same
change, because splitting them would break tests.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82486/new/

https://reviews.llvm.org/D82486

Files:
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp

Index: clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp
===
--- clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp
+++ clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp
@@ -117,18 +117,13 @@
   Code,
   R"txt(
 TraverseIntegerLiteral IntegerLiteral(1)
-WalkUpFromStmt IntegerLiteral(1)
 TraverseIntegerLiteral IntegerLiteral(2)
-WalkUpFromStmt IntegerLiteral(2)
 TraverseIntegerLiteral IntegerLiteral(3)
-WalkUpFromStmt IntegerLiteral(3)
 WalkUpFromStmt BinaryOperator(+)
 WalkUpFromStmt DeclRefExpr(add)
 WalkUpFromStmt ImplicitCastExpr
 TraverseIntegerLiteral IntegerLiteral(4)
-WalkUpFromStmt IntegerLiteral(4)
 TraverseIntegerLiteral IntegerLiteral(5)
-WalkUpFromStmt IntegerLiteral(5)
 WalkUpFromStmt CallExpr(add)
 WalkUpFromStmt CompoundStmt
 )txt"));
@@ -189,18 +184,13 @@
   Code,
   R"txt(
 TraverseIntegerLiteral IntegerLiteral(1)
-WalkUpFromIntegerLiteral IntegerLiteral(1)
 TraverseIntegerLiteral IntegerLiteral(2)
-WalkUpFromIntegerLiteral IntegerLiteral(2)
 TraverseIntegerLiteral IntegerLiteral(3)
-WalkUpFromIntegerLiteral IntegerLiteral(3)
 WalkUpFromExpr BinaryOperator(+)
 WalkUpFromExpr DeclRefExpr(add)
 WalkUpFromExpr ImplicitCastExpr
 TraverseIntegerLiteral IntegerLiteral(4)
-WalkUpFromIntegerLiteral IntegerLiteral(4)
 TraverseIntegerLiteral IntegerLiteral(5)
-WalkUpFromIntegerLiteral IntegerLiteral(5)
 WalkUpFromExpr CallExpr(add)
 WalkUpFromStmt CompoundStmt
 )txt"));
@@ -315,7 +305,6 @@
 WalkUpFromStmt IntegerLiteral(1)
 WalkUpFromStmt IntegerLiteral(2)
 WalkUpFromStmt IntegerLiteral(3)
-WalkUpFromStmt BinaryOperator(+)
 WalkUpFromStmt DeclRefExpr(add)
 WalkUpFromStmt ImplicitCastExpr
 WalkUpFromStmt IntegerLiteral(4)
@@ -383,7 +372,6 @@
 WalkUpFromExpr IntegerLiteral(1)
 WalkUpFromExpr IntegerLiteral(2)
 WalkUpFromExpr IntegerLiteral(3)
-WalkUpFromBinaryOperator BinaryOperator(+)
 WalkUpFromExpr DeclRefExpr(add)
 WalkUpFromExpr ImplicitCastExpr
 WalkUpFromExpr IntegerLiteral(4)
@@ -500,7 +488,6 @@
 WalkUpFromStmt IntegerLiteral(3)
 WalkUpFromStmt BinaryOperator(+)
 TraverseCallExpr CallExpr(add)
-WalkUpFromStmt CallExpr(add)
 WalkUpFromStmt CompoundStmt
 )txt"));
 }
@@ -560,7 +547,6 @@
 WalkUpFromExpr IntegerLiteral(3)
 WalkUpFromExpr BinaryOperator(+)
 TraverseCallExpr CallExpr(add)
-WalkUpFromCallExpr CallExpr(add)
 WalkUpFromStmt CompoundStmt
 )txt"));
 }
Index: clang/lib/Tooling/Syntax/BuildTree.cpp
===
--- clang/lib/Tooling/Syntax/BuildTree.cpp
+++ clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -578,15 +578,19 @@
 // RAV traverses it as a statement, we produce invalid node kinds in that
 // case.
 // FIXME: should do this in RAV instead?
-if (S->getInit() && !TraverseStmt(S->getInit()))
-  return false;
-if (S->getLoopVariable() && !TraverseDecl(S->getLoopVariable()))
-  return false;
-if (S->getRangeInit() && !TraverseStmt(S->getRangeInit()))
-  return false;
-if (S->getBody() && !TraverseStmt(S->getBody()))
-  return false;
-return true;
+bool Result = [&, this]() {
+  if (S->getInit() && !TraverseStmt(S->getInit()))
+return false;
+  if (S->getLoopVariable() && !TraverseDecl(S->getLoopVariable()))
+return false;
+  if (S->getRangeInit() && !TraverseStmt(S->getRangeInit()))
+return false;
+  if (S->getBody() && !TraverseStmt(S->getBody()))
+return false;
+  return true;
+}();
+WalkUpFromCXXForRangeStmt(S);
+return Result;
   }
 
   bool TraverseStmt(Stmt *S) {
Index: clang/include/clang/AST/RecursiveASTVisitor.h
===
--- clang/include/clang/AST/RecursiveASTVisitor.h
+++ clang/include/clang/AST/RecursiveASTVisitor.h
@@ -331,6 +331,32 @@
   struct has_same_member_pointer_type
   : std::true_type {};
 
+  template  struct is_same_method_impl {
+template 
+static bool isSameMethod(FirstMethodPtrTy FirstMethodPtr,
+ SecondMethodPtrTy SecondMethodPtr) {
+  return false;
+}
+  };
+
+  template <> struct is_same_method_impl {
+template 
+static bool isSameMethod(FirstMethodPtrTy FirstMethodPtr,
+ SecondMethodPtrTy SecondMethodPtr) {
+

[PATCH] D82486: RecursiveASTVisitor: don't call WalkUp unnecessarily in post-order traversal

2020-06-25 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits 
ymandel added inline comments.



Comment at: clang/include/clang/AST/RecursiveASTVisitor.h:335
+  template  struct is_same_method_impl {
+static bool isSameMethod(...) { return false; }
+  };

Why use var-args rather than spelling out the type arguments like you have on 
lines 339-341 or, simpler, line 351?



Comment at: clang/include/clang/AST/RecursiveASTVisitor.h:339
+  template <> struct is_same_method_impl {
+template (S))); break;
+if (isSameMethod(::Traverse##CLASS,
\
+ ::Traverse##CLASS)) { 
\

Do you explain this logic somewhere? Or do you feel it will be obvious to the 
reader? (I don't have a good intuition about this class to judge).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82486/new/

https://reviews.llvm.org/D82486



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D82486: RecursiveASTVisitor: don't call WalkUp unnecessarily in post-order traversal

2020-06-24 Thread Dmitri Gribenko via Phabricator via cfe-commits 
gribozavr created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
gribozavr2 added reviewers: eduucaldas, ymandel.

How does RecursiveASTVisitor call the WalkUp callback for expressions?

- In pre-order traversal mode, RecursiveASTVisitor calls the WalkUp

callback from the default implementation of Traverse callbacks.

- In post-order traversal mode when we don't have a DataRecursionQueue,

RecursiveASTVisitor also calls the WalkUp callback from the default
implementation of Traverse callbacks.

- However, in post-order traversal mode when we have a DataRecursionQueue,

RecursiveASTVisitor calls the WalkUp callback from PostVisitStmt.

As a result, when the user overrides the Traverse callback, in pre-order
traversal mode they never get the corresponding WalkUp callback. However
in the post-order traversal mode the WalkUp callback is invoked or not
depending on whether the data recursion optimization could be applied.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D82486

Files:
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp

Index: clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp
===
--- clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp
+++ clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp
@@ -117,18 +117,13 @@
   Code,
   R"txt(
 TraverseIntegerLiteral IntegerLiteral(1)
-WalkUpFromStmt IntegerLiteral(1)
 TraverseIntegerLiteral IntegerLiteral(2)
-WalkUpFromStmt IntegerLiteral(2)
 TraverseIntegerLiteral IntegerLiteral(3)
-WalkUpFromStmt IntegerLiteral(3)
 WalkUpFromStmt BinaryOperator(+)
 WalkUpFromStmt DeclRefExpr(add)
 WalkUpFromStmt ImplicitCastExpr
 TraverseIntegerLiteral IntegerLiteral(4)
-WalkUpFromStmt IntegerLiteral(4)
 TraverseIntegerLiteral IntegerLiteral(5)
-WalkUpFromStmt IntegerLiteral(5)
 WalkUpFromStmt CallExpr(add)
 WalkUpFromStmt CompoundStmt
 )txt"));
@@ -189,18 +184,13 @@
   Code,
   R"txt(
 TraverseIntegerLiteral IntegerLiteral(1)
-WalkUpFromIntegerLiteral IntegerLiteral(1)
 TraverseIntegerLiteral IntegerLiteral(2)
-WalkUpFromIntegerLiteral IntegerLiteral(2)
 TraverseIntegerLiteral IntegerLiteral(3)
-WalkUpFromIntegerLiteral IntegerLiteral(3)
 WalkUpFromExpr BinaryOperator(+)
 WalkUpFromExpr DeclRefExpr(add)
 WalkUpFromExpr ImplicitCastExpr
 TraverseIntegerLiteral IntegerLiteral(4)
-WalkUpFromIntegerLiteral IntegerLiteral(4)
 TraverseIntegerLiteral IntegerLiteral(5)
-WalkUpFromIntegerLiteral IntegerLiteral(5)
 WalkUpFromExpr CallExpr(add)
 WalkUpFromStmt CompoundStmt
 )txt"));
@@ -315,7 +305,6 @@
 WalkUpFromStmt IntegerLiteral(1)
 WalkUpFromStmt IntegerLiteral(2)
 WalkUpFromStmt IntegerLiteral(3)
-WalkUpFromStmt BinaryOperator(+)
 WalkUpFromStmt DeclRefExpr(add)
 WalkUpFromStmt ImplicitCastExpr
 WalkUpFromStmt IntegerLiteral(4)
@@ -383,7 +372,6 @@
 WalkUpFromExpr IntegerLiteral(1)
 WalkUpFromExpr IntegerLiteral(2)
 WalkUpFromExpr IntegerLiteral(3)
-WalkUpFromBinaryOperator BinaryOperator(+)
 WalkUpFromExpr DeclRefExpr(add)
 WalkUpFromExpr ImplicitCastExpr
 WalkUpFromExpr IntegerLiteral(4)
@@ -500,7 +488,6 @@
 WalkUpFromStmt IntegerLiteral(3)
 WalkUpFromStmt BinaryOperator(+)
 TraverseCallExpr CallExpr(add)
-WalkUpFromStmt CallExpr(add)
 WalkUpFromStmt CompoundStmt
 )txt"));
 }
@@ -560,7 +547,6 @@
 WalkUpFromExpr IntegerLiteral(3)
 WalkUpFromExpr BinaryOperator(+)
 TraverseCallExpr CallExpr(add)
-WalkUpFromCallExpr CallExpr(add)
 WalkUpFromStmt CompoundStmt
 )txt"));
 }
Index: clang/include/clang/AST/RecursiveASTVisitor.h
===
--- clang/include/clang/AST/RecursiveASTVisitor.h
+++ clang/include/clang/AST/RecursiveASTVisitor.h
@@ -331,6 +331,31 @@
   struct has_same_member_pointer_type
   : std::true_type {};
 
+  template  struct is_same_method_impl {
+static bool isSameMethod(...) { return false; }
+  };
+
+  template <> struct is_same_method_impl {
+template 
+static bool
+isSameMethod(FirstResult (FirstTy::*FirstMethodPtr)(FirstParams...),
+ SecondResult (SecondTy::*SecondMethodPtr)(SecondParams...)) {
+  return FirstMethodPtr == SecondMethodPtr;
+}
+  };
+
+  /// Returns true if and only if \p FirstMethodPtr and \p SecondMethodPtr
+  /// are pointers to the same non-static member function.
+  template 
+  bool isSameMethod(FirstMethodPtrTy FirstMethodPtr,
+SecondMethodPtrTy SecondMethodPtr) {
+return is_same_method_impl<
+has_same_member_pointer_type::
+value>::isSameMethod(FirstMethodPtr, SecondMethodPtr);
+  }
+
   // Traverse the given statement. If the most-derived traverse function takes a
   // data recursion queue, pass it on; otherwise, discard it. Note that the