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

Eduardo Caldas via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jun 29 01:02:21 PDT 2020


eduucaldas accepted this revision.
eduucaldas added inline comments.


================
Comment at: clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp:117
+      recordDefaultImplementation(
+          __func__, [&, this]() { RecordingVisitorBase::WalkUpFromStmt(S); });
+      return true;
----------------
gribozavr2 wrote:
> eduucaldas wrote:
> > Do we need this `this`?
> Yes, we're calling a method on `this` from the base class. `RecordingVisitorBase::WalkUpFromStmt(S);` is implicitly calling a member function, `this-> RecordingVisitorBase::WalkUpFromStmt(S);`.
True! The `&` default captures `this` already, but a better suggestion would be to use the capture: `[S, this]` or perhaps to just use `S` as an argument of the lambda. 
That is a nitpick though, feel free to push the patch :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82485





More information about the cfe-commits mailing list