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); });
+ return true;
----------------
eduucaldas wrote:
> 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 :)
> The & default captures this already
Oh indeed, thanks! I changed the capture list to `[&]` because I think it is
not interesting what this closure captures exactly. It is not stored anywhere
beyond the `recordCallback` call and invoked only once, so there are no
lifetime concerns for variables captured by reference.
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
