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

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added inline comments.


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-this-with-capture-default.cpp:9
+    auto explicit_this_capture = [=, this]() { };
+    // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: lambdas that capture this should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+    // CHECK-FIXES: [this]() { };
----------------
"default capture"?


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-this-with-capture-default.cpp:9
+    auto explicit_this_capture = [=, this]() { };
+    // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: lambdas that capture this should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+    // CHECK-FIXES: [this]() { };
----------------
carlosgalvezp wrote:
> "default capture"?
I find the check name a bit unintuitive. If you are up for a rename (you can use `rename_check.py`), I would consider renaming to something like `cppcoreguidelines-avoid-default-capture-when-capturing-this`

Like, what should be avoided is not "capturing this", it's using a default capture.

Would be good to get other reviewers opinion before spending time on renaming.


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-this-with-capture-default.cpp:9
+    auto explicit_this_capture = [=, this]() { };
+    // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: lambdas that capture this should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+    // CHECK-FIXES: [this]() { };
----------------
carlosgalvezp wrote:
> carlosgalvezp wrote:
> > "default capture"?
> I find the check name a bit unintuitive. If you are up for a rename (you can use `rename_check.py`), I would consider renaming to something like `cppcoreguidelines-avoid-default-capture-when-capturing-this`
> 
> Like, what should be avoided is not "capturing this", it's using a default capture.
> 
> Would be good to get other reviewers opinion before spending time on renaming.
Maybe put it within quotes so clarify it's a C++ keyword? Either backticks `this` or single quotes 'this' would work I think, unless we have some other convention.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141133