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() { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
