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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
