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 the test cases we are calling the default implementation. We are not surfacing that WalkUpFrom **can** not walk up.
================ Comment at: clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp:76 + template <typename CallDefault> + void recordDefaultImplementation(StringRef CallbackName, + CallDefault CallDefaultFn) { ---------------- We don't use CallbackName. ================ Comment at: clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp:107-110 + recordCallback(__func__, IL); + recordDefaultImplementation(__func__, [&, this]() { + RecordingVisitorBase::TraverseIntegerLiteral(IL); + }); ---------------- I think you meant to unify `recordCallback` and `recodDefaultImplentation` into one, and that's why you had added this `__func__` to `recordDefaultImplementation`. ================ Comment at: clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp:117 + recordDefaultImplementation( + __func__, [&, this]() { RecordingVisitorBase::WalkUpFromStmt(S); }); + return true; ---------------- Do we need this `this`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82485/new/ https://reviews.llvm.org/D82485 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits