[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

Nathan James via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 10 09:23:18 PST 2023


njames93 requested changes to this revision.
njames93 added a comment.
This revision now requires changes to proceed.

Can you run this through clang format to make sure the pre merge is happy and address those nits



================
Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureDefaultWhenCapturingThisCheck.cpp:81
+          Lambda->getCaptureDefaultLoc(),
+          "lambdas that capture 'this' should not specify a capture default");
+
----------------
Would be nice to show if `this` is implicitly captured.


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-capture-default-when-capturing-this.rst:6
+
+Warns when lambda specify a capture default and capture ``this``. The check also
+offers fix-its.
----------------
Not necessary to specify fix behaviour as that's provided in the check list


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-capture-default-when-capturing-this.rst:19
+      struct AClass {
+        int DataMember;
+        void misleadingLogic() {
----------------
Please fix these examples as this isn't actually captured because the names don't match.


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-this-with-capture-default.cpp:10
+    // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: lambdas that capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-default-when-capturing-this]
+    // CHECK-FIXES: [this]() { };
+
----------------
Please can you include the whole expected line in the check fix markers, same goes for all tests below 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141133/new/

https://reviews.llvm.org/D141133



More information about the cfe-commits mailing list