[PATCH] D82904: [clang-tidy] Warn pointer captured in async block
Aaron Ballman via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Jul 6 12:54:30 PDT 2020
aaron.ballman added inline comments.
================
Comment at: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp:82
"bugprone-bool-pointer-implicit-conversion");
- CheckFactories.registerCheck<BranchCloneCheck>(
- "bugprone-branch-clone");
+ CheckFactories.registerCheck<BranchCloneCheck>("bugprone-branch-clone");
CheckFactories.registerCheck<CopyConstructorInitCheck>(
----------------
It looks like unrelated formatting changes may have snuck in?
================
Comment at: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp:121
"bugprone-narrowing-conversions");
+ CheckFactories.registerCheck<NoEscapeCheck>("bugprone-no-escape");
CheckFactories.registerCheck<NotNullTerminatedResultCheck>(
----------------
Given that this is limited to Objective-C, why register this under `bugprone` as opposed to the `objc` module? Alternatively, why limit to Objective-C when blocks can be used in other modes like C or C++ with `-fblocks`?
================
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();
----------------
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.
================
Comment at: clang-tools-extra/clang-tidy/bugprone/NoEscapeCheck.cpp:37
+ if (Var && Var->hasAttr<NoEscapeAttr>()) {
+ diag(MatchedEscapingBlock->getBeginLoc(),
+ "pointer %0 with attribute 'noescape' is captured by an "
----------------
Given that the capture is the issue (not the block), why not point to the use of the captured variable?
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