ellis marked an inline comment as done.
ellis 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>(
----------------
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?


================
Comment at: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp:121
         "bugprone-narrowing-conversions");
+    CheckFactories.registerCheck<NoEscapeCheck>("bugprone-no-escape");
     CheckFactories.registerCheck<NotNullTerminatedResultCheck>(
----------------
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`.


================
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 "
----------------
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.


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

Reply via email to