[clang-tools-extra] 4d4c0f9 - [clang-tidy] Add option to ignore capture default by reference in cppcoreguidelines-avoid-capture-default-when-capturing-this
Carlos Galvez via cfe-commits
cfe-commits at lists.llvm.org
Tue Mar 28 13:37:04 PDT 2023
Author: Carlos Galvez
Date: 2023-03-28T20:36:34Z
New Revision: 4d4c0f9734607bb0423593b060b8fa73c06fe3d3
URL: https://github.com/llvm/llvm-project/commit/4d4c0f9734607bb0423593b060b8fa73c06fe3d3
DIFF: https://github.com/llvm/llvm-project/commit/4d4c0f9734607bb0423593b060b8fa73c06fe3d3.diff
LOG: [clang-tidy] Add option to ignore capture default by reference in cppcoreguidelines-avoid-capture-default-when-capturing-this
The rule exists primarily for when using capture default
by copy "[=]", since member variables will be captured by
reference, which is against developer expectations.
However when the capture default is by reference, then there
is no doubt: everything will be captured by reference. Add
an option to allow just that.
Note: Release Notes do not need update since this check
has been introduced in the current WIP release.
A ticket has been opened at the C++ Core Guidelines repo
to consider updating the rule such that this behavior
is the default one:
https://github.com/isocpp/CppCoreGuidelines/issues/2060
Differential Revision: https://reviews.llvm.org/D147062
Added:
Modified:
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureDefaultWhenCapturingThisCheck.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureDefaultWhenCapturingThisCheck.h
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-capture-default-when-capturing-this.rst
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-default-when-capturing-this.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureDefaultWhenCapturingThisCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureDefaultWhenCapturingThisCheck.cpp
index 11ef35178765f..1489ca6c442b1 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureDefaultWhenCapturingThisCheck.cpp
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureDefaultWhenCapturingThisCheck.cpp
@@ -18,6 +18,19 @@ using namespace clang::ast_matchers;
namespace clang::tidy::cppcoreguidelines {
+AvoidCaptureDefaultWhenCapturingThisCheck::
+ AvoidCaptureDefaultWhenCapturingThisCheck(StringRef Name,
+ ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ IgnoreCaptureDefaultByReference(
+ Options.get("IgnoreCaptureDefaultByReference", false)) {}
+
+void AvoidCaptureDefaultWhenCapturingThisCheck::storeOptions(
+ ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "IgnoreCaptureDefaultByReference",
+ IgnoreCaptureDefaultByReference);
+}
+
void AvoidCaptureDefaultWhenCapturingThisCheck::registerMatchers(
MatchFinder *Finder) {
Finder->addMatcher(lambdaExpr(hasAnyCapture(capturesThis())).bind("lambda"),
@@ -74,24 +87,30 @@ static std::string createReplacementText(const LambdaExpr *Lambda) {
void AvoidCaptureDefaultWhenCapturingThisCheck::check(
const MatchFinder::MatchResult &Result) {
- if (const auto *Lambda = Result.Nodes.getNodeAs<LambdaExpr>("lambda")) {
- if (Lambda->getCaptureDefault() != LCD_None) {
- bool IsThisImplicitlyCaptured = std::any_of(
- Lambda->implicit_capture_begin(), Lambda->implicit_capture_end(),
- [](const LambdaCapture &Capture) { return Capture.capturesThis(); });
- auto Diag = diag(Lambda->getCaptureDefaultLoc(),
- "lambdas that %select{|implicitly }0capture 'this' "
- "should not specify a capture default")
- << IsThisImplicitlyCaptured;
-
- std::string ReplacementText = createReplacementText(Lambda);
- SourceLocation DefaultCaptureEnd =
- findDefaultCaptureEnd(Lambda, *Result.Context);
- Diag << FixItHint::CreateReplacement(
- CharSourceRange::getCharRange(Lambda->getCaptureDefaultLoc(),
- DefaultCaptureEnd),
- ReplacementText);
- }
+ const auto *Lambda = Result.Nodes.getNodeAs<LambdaExpr>("lambda");
+ if (!Lambda)
+ return;
+
+ if (IgnoreCaptureDefaultByReference &&
+ Lambda->getCaptureDefault() == LCD_ByRef)
+ return;
+
+ if (Lambda->getCaptureDefault() != LCD_None) {
+ bool IsThisImplicitlyCaptured = std::any_of(
+ Lambda->implicit_capture_begin(), Lambda->implicit_capture_end(),
+ [](const LambdaCapture &Capture) { return Capture.capturesThis(); });
+ auto Diag = diag(Lambda->getCaptureDefaultLoc(),
+ "lambdas that %select{|implicitly }0capture 'this' "
+ "should not specify a capture default")
+ << IsThisImplicitlyCaptured;
+
+ std::string ReplacementText = createReplacementText(Lambda);
+ SourceLocation DefaultCaptureEnd =
+ findDefaultCaptureEnd(Lambda, *Result.Context);
+ Diag << FixItHint::CreateReplacement(
+ CharSourceRange::getCharRange(Lambda->getCaptureDefaultLoc(),
+ DefaultCaptureEnd),
+ ReplacementText);
}
}
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureDefaultWhenCapturingThisCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureDefaultWhenCapturingThisCheck.h
index ae51fe5a10640..08bd3ebd3c3ec 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureDefaultWhenCapturingThisCheck.h
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureDefaultWhenCapturingThisCheck.h
@@ -27,13 +27,16 @@ namespace clang::tidy::cppcoreguidelines {
class AvoidCaptureDefaultWhenCapturingThisCheck : public ClangTidyCheck {
public:
AvoidCaptureDefaultWhenCapturingThisCheck(StringRef Name,
- ClangTidyContext *Context)
- : ClangTidyCheck(Name, Context) {}
+ ClangTidyContext *Context);
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
return LangOpts.CPlusPlus11;
}
+
+private:
+ bool IgnoreCaptureDefaultByReference;
};
} // namespace clang::tidy::cppcoreguidelines
diff --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-capture-default-when-capturing-this.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-capture-default-when-capturing-this.rst
index 9c9702a292938..d22f5b6f9343b 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-capture-default-when-capturing-this.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-capture-default-when-capturing-this.rst
@@ -41,3 +41,13 @@ Examples:
This check implements
`CppCoreGuideline F.54 <http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f54-if-you-capture-this-capture-all-variables-explicitly-no-default-capture>`_.
+
+
+Options
+-------
+
+.. option:: IgnoreCaptureDefaultByReference
+
+ Do not warn when using capture default by reference. In this case, there is no
+ confusion as to whether variables are captured by value or reference.
+ Defaults to `false`.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-default-when-capturing-this.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-default-when-capturing-this.cpp
index 8a06bd61607ab..3ae8cc47e0147 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-default-when-capturing-this.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-default-when-capturing-this.cpp
@@ -1,4 +1,7 @@
-// RUN: %check_clang_tidy -std=c++11-or-later %s cppcoreguidelines-avoid-capture-default-when-capturing-this %t
+// RUN: %check_clang_tidy -std=c++11-or-later %s cppcoreguidelines-avoid-capture-default-when-capturing-this %t \
+// RUN: -check-suffixes=,DEFAULT
+// RUN: %check_clang_tidy -std=c++11-or-later %s cppcoreguidelines-avoid-capture-default-when-capturing-this %t \
+// RUN: -config="{CheckOptions: [{key: cppcoreguidelines-avoid-capture-default-when-capturing-this.IgnoreCaptureDefaultByReference, value: true}]}"
struct Obj {
void lambdas_that_warn_default_capture_copy() {
@@ -55,24 +58,24 @@ struct Obj {
int local2{};
auto ref_explicit_this_capture = [&, this]() { };
- // CHECK-MESSAGES: :[[@LINE-1]]:39: warning: lambdas that capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-default-when-capturing-this]
- // CHECK-FIXES: auto ref_explicit_this_capture = [this]() { };
+ // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:39: warning: lambdas that capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-default-when-capturing-this]
+ // CHECK-FIXES-DEFAULT: auto ref_explicit_this_capture = [this]() { };
auto ref_explicit_this_capture_local = [&, this]() { return (local+x) > 10; };
- // CHECK-MESSAGES: :[[@LINE-1]]:45: warning: lambdas that capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-default-when-capturing-this]
- // CHECK-FIXES: auto ref_explicit_this_capture_local = [&local, this]() { return (local+x) > 10; };
+ // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:45: warning: lambdas that capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-default-when-capturing-this]
+ // CHECK-FIXES-DEFAULT: auto ref_explicit_this_capture_local = [&local, this]() { return (local+x) > 10; };
auto ref_implicit_this_capture = [&]() { return x > 10; };
- // CHECK-MESSAGES: :[[@LINE-1]]:39: warning: lambdas that implicitly capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-default-when-capturing-this]
- // CHECK-FIXES: auto ref_implicit_this_capture = [this]() { return x > 10; };
+ // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:39: warning: lambdas that implicitly capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-default-when-capturing-this]
+ // CHECK-FIXES-DEFAULT: auto ref_implicit_this_capture = [this]() { return x > 10; };
auto ref_implicit_this_capture_local = [&]() { return (local+x) > 10; };
- // CHECK-MESSAGES: :[[@LINE-1]]:45: warning: lambdas that implicitly capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-default-when-capturing-this]
- // CHECK-FIXES: auto ref_implicit_this_capture_local = [&local, this]() { return (local+x) > 10; };
+ // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:45: warning: lambdas that implicitly capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-default-when-capturing-this]
+ // CHECK-FIXES-DEFAULT: auto ref_implicit_this_capture_local = [&local, this]() { return (local+x) > 10; };
auto ref_implicit_this_capture_locals = [&]() { return (local+local2+x) > 10; };
- // CHECK-MESSAGES: :[[@LINE-1]]:46: warning: lambdas that implicitly capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-default-when-capturing-this]
- // CHECK-FIXES: auto ref_implicit_this_capture_locals = [&local, &local2, this]() { return (local+local2+x) > 10; };
+ // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:46: warning: lambdas that implicitly capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-default-when-capturing-this]
+ // CHECK-FIXES-DEFAULT: auto ref_implicit_this_capture_locals = [&local, &local2, this]() { return (local+local2+x) > 10; };
}
void lambdas_that_dont_warn() {
More information about the cfe-commits
mailing list