[PATCH] D82485: Add tests for sequences of callbacks that RecursiveASTVisitor produces

nemanjai added a comment. Hi @gribozavr do you think we can do something about this test? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82485/new/ https://reviews.llvm.org/D82485 ___ cfe-commits mailing

[PATCH] D82485: Add tests for sequences of callbacks that RecursiveASTVisitor produces

nemanjai added a comment. Is there a way this test case can somehow be broken up into multiple files? The test case takes a very long time to compile which causes intermittent but frequent failures on one of our bots that runs on a fairly small VM. Most of the failures listed here:

[PATCH] D82485: Add tests for sequences of callbacks that RecursiveASTVisitor produces

This revision was automatically updated to reflect the committed changes. Closed by commit rG8e5a56865f28: Add tests for sequences of callbacks that RecursiveASTVisitor produces (authored by gribozavr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D82485: Add tests for sequences of callbacks that RecursiveASTVisitor produces

gribozavr updated this revision to Diff 274010. gribozavr added a comment. Simplified capture lists in lambdas Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82485/new/ https://reviews.llvm.org/D82485 Files:

[PATCH] D82485: Add tests for sequences of callbacks that RecursiveASTVisitor produces

gribozavr2 marked an inline comment as done. gribozavr2 added inline comments. Comment at: clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp:117 + recordDefaultImplementation( + __func__, [&, this]() { RecordingVisitorBase::WalkUpFromStmt(S); }); +

[PATCH] D82485: Add tests for sequences of callbacks that RecursiveASTVisitor produces

eduucaldas accepted this revision. eduucaldas added inline comments. Comment at: clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp:117 + recordDefaultImplementation( + __func__, [&, this]() { RecordingVisitorBase::WalkUpFromStmt(S); }); + return

[PATCH] D82485: Add tests for sequences of callbacks that RecursiveASTVisitor produces

gribozavr updated this revision to Diff 273835. gribozavr added a comment. Unified recordCallback and recordDefaultImplementation into one call. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82485/new/ https://reviews.llvm.org/D82485 Files:

[PATCH] D82485: Add tests for sequences of callbacks that RecursiveASTVisitor produces

gribozavr2 added a comment. > Now, in all the test cases we are calling the default implementation. We are > not surfacing that WalkUpFrom can not walk up. I think we are. The log of callbacks called from the default implementation is indented to the right, so we clearly see what the default

[PATCH] D82485: Add tests for sequences of callbacks that RecursiveASTVisitor produces

gribozavr updated this revision to Diff 273834. gribozavr added a comment. Removed an unused argument from recordDefaultImplementation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82485/new/ https://reviews.llvm.org/D82485 Files:

[PATCH] D82485: Add tests for sequences of callbacks that RecursiveASTVisitor produces

eduucaldas added a comment. A more general feedback. From our conversation one of the issues was that the tests wre only surfacing overriden methods. For instance, whenever we recorded a WalkUpFromExpr, and thus the callback showed up in the test, we actually did **not** walk up. Now, in all

[PATCH] D82485: Add tests for sequences of callbacks that RecursiveASTVisitor produces

gribozavr updated this revision to Diff 273464. gribozavr added a comment. Added calls to default implementations. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82485/new/ https://reviews.llvm.org/D82485 Files:

[PATCH] D82485: Add tests for sequences of callbacks that RecursiveASTVisitor produces

gribozavr updated this revision to Diff 273302. gribozavr added a comment. Addressed code review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82485/new/ https://reviews.llvm.org/D82485 Files: clang/unittests/Tooling/CMakeLists.txt

[PATCH] D82485: Add tests for sequences of callbacks that RecursiveASTVisitor produces

gribozavr2 marked 5 inline comments as done. gribozavr2 added inline comments. Comment at: clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp:16 +template +class RecordingVisitorBase : public TestVisitor { + bool VisitPostOrder; ymandel wrote: >

[PATCH] D82485: Add tests for sequences of callbacks that RecursiveASTVisitor produces

eduucaldas marked 2 inline comments as done. eduucaldas added inline comments. Comment at: clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp:1 +//===--- clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp ---===// +// I find this name

[PATCH] D82485: Add tests for sequences of callbacks that RecursiveASTVisitor produces

ymandel accepted this revision. ymandel added inline comments. This revision is now accepted and ready to land. Comment at: clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp:16 +template +class RecordingVisitorBase : public TestVisitor { + bool VisitPostOrder;

[PATCH] D82485: Add tests for sequences of callbacks that RecursiveASTVisitor produces

gribozavr created this revision. Herald added subscribers: cfe-commits, mgorny. Herald added a project: clang. gribozavr2 added reviewers: ymandel, eduucaldas. These tests show a bug: post-order traversal introduces an extra call to WalkUp*, that is not present in pre-order traversal. I'm fixing