aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
LGTM with a testing request.
================
Comment at: clang-tools-extra/clang-tidy/bugprone/NoEscapeCheck.cpp:34
+ const BlockDecl *EscapingBlockDecl = MatchedEscapingBlock->getBlockDecl();
+ for (const BlockDecl::Capture &CapturedVar : EscapingBlockDecl->captures()) {
+ const VarDecl *Var = CapturedVar.getVariable();
----------------
ellis wrote:
> aaron.ballman wrote:
> > This makes me think we should extend the `hasAnyCaptures()` AST matcher so
> > it works with blocks as well as lambdas. It would be nice to do all of this
> > from the matcher interface.
> Should I add a TODO for this?
Naw, I don't think it's that critical. I'm mostly just surprised we haven't
done that before now given how similar blocks and lambdas are.
================
Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-no-escape.m:1
+// RUN: %check_clang_tidy %s bugprone-no-escape %t
+
----------------
Can you add an additional RUN line so we get coverage of the blocks-enabled
behavior? Something like:
```
// RUN: %check_clang_tidy %s bugprone-no-escape %t -- -- -fblocks -x c
```
I'm not 100% certain I have that syntax right, but the idea is to run the test
as though it were a C compilation unit with blocks explicitly enabled.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D82904/new/
https://reviews.llvm.org/D82904
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
