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>(
----------------
ellis wrote:
> aaron.ballman wrote:
> > It looks like unrelated formatting changes may have snuck in?
> The script `add_new_check.py` doesn't format the code that it generated so a
> few lines aren't linted. Would you like me to undo these extra formatted
> lines?
Yes, please. The way I usually do this, btw, is to run the patch through
clang-format-diff so that only the lines I've touched get formatted:
https://clang.llvm.org/docs/ClangFormat.html#script-for-patch-reformatting
================
Comment at: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp:121
"bugprone-narrowing-conversions");
+ CheckFactories.registerCheck<NoEscapeCheck>("bugprone-no-escape");
CheckFactories.registerCheck<NotNullTerminatedResultCheck>(
----------------
ellis wrote:
> njames93 wrote:
> > aaron.ballman wrote:
> > > 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`?
> > Thats a good point, maybe restrict this to `LangOptions::ObjC ||
> > LangOptions::Blocks` Then it can be left in bugprone.
> Ok, I'll include `LangOptions::Blocks`.
I think the only option you need is `LangOptions::Blocks` (it should be enabled
automatically in ObjC language mode).
================
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 "
----------------
ellis wrote:
> aaron.ballman wrote:
> > Given that the capture is the issue (not the block), why not point to the
> > use of the captured variable?
> I actually agree that pointing to the use of the captured variable would be
> easier to read, but honestly I couldn't figure out how to grab the location
> of that use. All I could get was the definition of the variable.
Ah, yeah, I just looked at the class and it looks like we don't track that
information yet and it would be somewhat involved to get it. Can you add a
FIXME comment above the call to `diag()` mentioning that we'd prefer to
diagnose the use of the capture rather than the block?
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
