[PATCH] D82904: [clang-tidy] Warn pointer captured in async block

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jul 7 07:33:31 PDT 2020


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





More information about the cfe-commits mailing list