llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tools-extra Author: Baranov Victor (vbvictor) <details> <summary>Changes</summary> New check to find variable declarations of expensive-to-construct classes that are constructed from only constant literals and so can be reused to avoid repeated construction costs on each function invocation. Closes https://github.com/llvm/llvm-project/issues/121276. --- Patch is 28.70 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/131455.diff 9 Files Affected: - (modified) clang-tools-extra/clang-tidy/performance/CMakeLists.txt (+1) - (added) clang-tools-extra/clang-tidy/performance/ConstructReusableObjectsOnceCheck.cpp (+103) - (added) clang-tools-extra/clang-tidy/performance/ConstructReusableObjectsOnceCheck.h (+43) - (modified) clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp (+3) - (modified) clang-tools-extra/docs/ReleaseNotes.rst (+6) - (modified) clang-tools-extra/docs/clang-tidy/checks/list.rst (+1) - (added) clang-tools-extra/docs/clang-tidy/checks/performance/construct-reusable-objects-once.rst (+91) - (added) clang-tools-extra/test/clang-tidy/checkers/performance/Inputs/construct-reusable-objects-once/regex.h (+45) - (added) clang-tools-extra/test/clang-tidy/checkers/performance/construct-reusable-objects-once.cpp (+366) ``````````diff diff --git a/clang-tools-extra/clang-tidy/performance/CMakeLists.txt b/clang-tools-extra/clang-tidy/performance/CMakeLists.txt index c6e547c5089fb..7e2684a481f2c 100644 --- a/clang-tools-extra/clang-tidy/performance/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/performance/CMakeLists.txt @@ -5,6 +5,7 @@ set(LLVM_LINK_COMPONENTS add_clang_library(clangTidyPerformanceModule STATIC AvoidEndlCheck.cpp + ConstructReusableObjectsOnceCheck.cpp EnumSizeCheck.cpp FasterStringFindCheck.cpp ForRangeCopyCheck.cpp diff --git a/clang-tools-extra/clang-tidy/performance/ConstructReusableObjectsOnceCheck.cpp b/clang-tools-extra/clang-tidy/performance/ConstructReusableObjectsOnceCheck.cpp new file mode 100644 index 0000000000000..0c6b54d942196 --- /dev/null +++ b/clang-tools-extra/clang-tidy/performance/ConstructReusableObjectsOnceCheck.cpp @@ -0,0 +1,103 @@ +//===--- ConstructReusableObjectsOnceCheck.cpp - clang-tidy -----------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "ConstructReusableObjectsOnceCheck.h" +#include "../utils/OptionsUtils.h" +#include "clang/AST/Decl.h" +#include "clang/AST/DeclCXX.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "llvm/ADT/StringRef.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::performance { + +namespace { + +const llvm::StringRef DefaultCheckedClasses = + "::std::basic_regex;::boost::basic_regex"; +const llvm::StringRef DefaultIgnoredFunctions = "::main"; + +} // namespace + +ConstructReusableObjectsOnceCheck::ConstructReusableObjectsOnceCheck( + StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + CheckedClasses(utils::options::parseStringList( + Options.get("CheckedClasses", DefaultCheckedClasses))), + IgnoredFunctions(utils::options::parseStringList( + Options.get("IgnoredFunctions", DefaultIgnoredFunctions))) {} + +void ConstructReusableObjectsOnceCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "CheckedClasses", DefaultCheckedClasses); + Options.store(Opts, "IgnoredFunctions", DefaultIgnoredFunctions); +} + +void ConstructReusableObjectsOnceCheck::registerMatchers(MatchFinder *Finder) { + const auto ConstStrLiteralDecl = + varDecl(unless(parmVarDecl()), hasType(constantArrayType()), + hasType(isConstQualified()), + hasInitializer(ignoringParenImpCasts(stringLiteral()))); + const auto ConstPtrStrLiteralDecl = varDecl( + unless(parmVarDecl()), + hasType(pointerType(pointee(isAnyCharacter(), isConstQualified()))), + hasInitializer(ignoringParenImpCasts(stringLiteral()))); + + const auto ConstNumberLiteralDecl = + varDecl(hasType(qualType(anyOf(isInteger(), realFloatingPointType()))), + hasType(isConstQualified()), + hasInitializer(ignoringParenImpCasts( + anyOf(integerLiteral(), floatLiteral()))), + unless(parmVarDecl())); + + const auto ConstEnumLiteralDecl = varDecl( + unless(parmVarDecl()), hasType(hasUnqualifiedDesugaredType(enumType())), + hasType(isConstQualified()), + hasInitializer(declRefExpr(to(enumConstantDecl())))); + + const auto ConstLiteralArg = expr(ignoringParenImpCasts( + anyOf(stringLiteral(), integerLiteral(), floatLiteral(), + declRefExpr(to(enumConstantDecl())), + declRefExpr(hasDeclaration( + anyOf(ConstNumberLiteralDecl, ConstPtrStrLiteralDecl, + ConstStrLiteralDecl, ConstEnumLiteralDecl)))))); + + const auto ConstructorCall = cxxConstructExpr( + hasDeclaration(cxxConstructorDecl( + ofClass(cxxRecordDecl(hasAnyName(CheckedClasses)).bind("class")))), + unless(hasAnyArgument(expr(unless(ConstLiteralArg))))); + + Finder->addMatcher( + varDecl(unless(hasGlobalStorage()), hasInitializer(ConstructorCall), + hasAncestor(functionDecl(unless(hasAnyName(IgnoredFunctions))) + .bind("function"))) + .bind("var"), + this); +} + +void ConstructReusableObjectsOnceCheck::check(const MatchFinder::MatchResult &Result) { + if (const auto *Var = Result.Nodes.getNodeAs<VarDecl>("var")) { + const auto *Class = Result.Nodes.getNodeAs<CXXRecordDecl>("class"); + assert(Class); + + const auto *Function = Result.Nodes.getNodeAs<FunctionDecl>("function"); + assert(Function); + + diag(Var->getLocation(), + "variable '%0' of type '%1' is constructed with only constant " + "literals on each invocation of '%2'; make this variable 'static', " + "declare as a global variable or move to a class member to avoid " + "repeated constructions") + << Var->getName() << Class->getQualifiedNameAsString() + << Function->getQualifiedNameAsString(); + } +} + +} // namespace clang::tidy::performance diff --git a/clang-tools-extra/clang-tidy/performance/ConstructReusableObjectsOnceCheck.h b/clang-tools-extra/clang-tidy/performance/ConstructReusableObjectsOnceCheck.h new file mode 100644 index 0000000000000..1af482354625f --- /dev/null +++ b/clang-tools-extra/clang-tidy/performance/ConstructReusableObjectsOnceCheck.h @@ -0,0 +1,43 @@ +//===--- ConstructReusableObjectsOnceCheck.h - clang-tidy -------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_CONSTRUCTREUSABLEOBJECTSONCECHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_CONSTRUCTREUSABLEOBJECTSONCECHECK_H + +#include "../ClangTidyCheck.h" +#include <optional> +#include <vector> + +namespace clang::tidy::performance { + +/// Finds variable declarations of expensive-to-construct classes that are +/// constructed from only constant literals and so can be reused. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/performance/construct-reusable-objects-once.html +class ConstructReusableObjectsOnceCheck : public ClangTidyCheck { +public: + ConstructReusableObjectsOnceCheck(StringRef Name, 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.CPlusPlus; + } + std::optional<TraversalKind> getCheckTraversalKind() const override { + return TK_IgnoreUnlessSpelledInSource; + } + +private: + std::vector<llvm::StringRef> CheckedClasses; + std::vector<llvm::StringRef> IgnoredFunctions; +}; + +} // namespace clang::tidy::performance + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_CONSTRUCTREUSABLEOBJECTSONCECHECK_H diff --git a/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp b/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp index 9e0fa6f88b36a..8c1ea1c22cff3 100644 --- a/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp @@ -10,6 +10,7 @@ #include "../ClangTidyModule.h" #include "../ClangTidyModuleRegistry.h" #include "AvoidEndlCheck.h" +#include "ConstructReusableObjectsOnceCheck.h" #include "EnumSizeCheck.h" #include "FasterStringFindCheck.h" #include "ForRangeCopyCheck.h" @@ -36,6 +37,8 @@ class PerformanceModule : public ClangTidyModule { public: void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { CheckFactories.registerCheck<AvoidEndlCheck>("performance-avoid-endl"); + CheckFactories.registerCheck<ConstructReusableObjectsOnceCheck>( + "performance-construct-reusable-objects-once"); CheckFactories.registerCheck<EnumSizeCheck>("performance-enum-size"); CheckFactories.registerCheck<FasterStringFindCheck>( "performance-faster-string-find"); diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 0ad52f83fad85..ac4b6b4726073 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -106,6 +106,12 @@ New checks Finds unintended character output from ``unsigned char`` and ``signed char`` to an ``ostream``. +- New :doc:`performance-construct-reusable-objects-once + <clang-tidy/checks/performance/construct-reusable-objects-once>` check. + + Finds variable declarations of expensive-to-construct classes that are + constructed from only constant literals and so can be reused. + - New :doc:`readability-ambiguous-smartptr-reset-call <clang-tidy/checks/readability/ambiguous-smartptr-reset-call>` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index c73bc8bff3539..48f99b54fcca7 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -331,6 +331,7 @@ Clang-Tidy Checks :doc:`openmp-exception-escape <openmp/exception-escape>`, :doc:`openmp-use-default-none <openmp/use-default-none>`, :doc:`performance-avoid-endl <performance/avoid-endl>`, "Yes" + :doc:`performance-construct-reusable-objects-once <performance/construct-reusable-objects-once>`, :doc:`performance-enum-size <performance/enum-size>`, :doc:`performance-faster-string-find <performance/faster-string-find>`, "Yes" :doc:`performance-for-range-copy <performance/for-range-copy>`, "Yes" diff --git a/clang-tools-extra/docs/clang-tidy/checks/performance/construct-reusable-objects-once.rst b/clang-tools-extra/docs/clang-tidy/checks/performance/construct-reusable-objects-once.rst new file mode 100644 index 0000000000000..9b02533cd5adb --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/performance/construct-reusable-objects-once.rst @@ -0,0 +1,91 @@ +.. title:: clang-tidy - performance-construct-reusable-objects-once + +performance-construct-reusable-objects-once +=========================================== + +Finds variable declarations of expensive-to-construct classes that are +constructed from only constant literals and so can be reused. These objects +should be made ``static``, declared as a global variable or moved to a class +member to avoid repeated construction costs on each function invocation. + +This check is particularly useful for identifying inefficiently constructed +regular expressions since their constructors are relatively expensive compared +to their usage, so creating them repeatedly with the same pattern can +significantly impact program performance. + +Example: + +.. code-block:: c++ + + void parse() { + std::regex r("pattern"); // warning + // ... + } + +The more efficient version could be any of the following: + +.. code-block:: c++ + + void parse() { + static std::regex r("pattern"); // static constructed only once + // ... + } + +.. code-block:: c++ + + std::regex r("pattern"); // global variable constructed only once + + void parse() { + // ... + } + +.. code-block:: c++ + + class Parser { + void parse() { + // ... + } + + std::regex r{"pattern"}; // class member constructed only once + } + + +Known Limitations +----------------- + +The check will not analyze variables that are template dependent. + +.. code-block:: c++ + + template <typename T> + void parse() { + std::basic_regex<T> r("pattern"); // no warning + } + +The check only warns on variables that are constructed from literals or const +variables that are directly constructed from literals. + +.. code-block:: c++ + + void parse() { + const int var = 1; + const int constructed_from_var = var; + std::regex r1("pattern", var); // warning + std::regex r2("pattern", constructed_from_var); // no warning + } + + +Options +------- + +.. option:: CheckedClasses + + Semicolon-separated list of fully qualified class names that are considered + expensive to construct and should be flagged by this check. Default is + `::std::basic_regex;::boost::basic_regex`. + +.. option:: IgnoredFunctions + + Semicolon-separated list of fully qualified function names that are expected + to be called once so they should not be flagged by this check. Default is + `::main`. diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/Inputs/construct-reusable-objects-once/regex.h b/clang-tools-extra/test/clang-tidy/checkers/performance/Inputs/construct-reusable-objects-once/regex.h new file mode 100644 index 0000000000000..4ccf93775d969 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/performance/Inputs/construct-reusable-objects-once/regex.h @@ -0,0 +1,45 @@ +#ifndef _REGEX_ +#define _REGEX_ + +namespace std { + +template <typename T> +class regex_traits {}; + +namespace regex_constants { + enum syntax_option_type : unsigned int { + _S_icase = 1 << 0, + _S_nosubs = 1 << 1, + _S_basic = 1 << 2, + _S_ECMAScript = 1 << 3, + }; +} // namespace regex_constants + +template<class CharT, class Traits = std::regex_traits<CharT>> +class basic_regex { +public: + typedef regex_constants::syntax_option_type flag_type; + + static constexpr flag_type icase = regex_constants::syntax_option_type::_S_icase; + static constexpr flag_type nosubs = regex_constants::syntax_option_type::_S_nosubs; + static constexpr flag_type basic = regex_constants::syntax_option_type::_S_basic; + static constexpr flag_type ECMAScript = regex_constants::syntax_option_type::_S_ECMAScript; + + basic_regex(); + explicit basic_regex(const CharT* s, flag_type f = ECMAScript); + basic_regex(const CharT* s, unsigned int count, std::regex_constants::syntax_option_type f = ECMAScript); + basic_regex(const basic_regex& other); + basic_regex(basic_regex&& other) noexcept; + template<class ForwardIt> + basic_regex(ForwardIt first, ForwardIt last, std::regex_constants::syntax_option_type f = ECMAScript); + + basic_regex& operator=(const basic_regex& other); + basic_regex& operator=(basic_regex&& other) noexcept; +}; + +typedef basic_regex<char> regex; +typedef basic_regex<wchar_t> wregex; + +} // namespace std + +#endif // _REGEX_ diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/construct-reusable-objects-once.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/construct-reusable-objects-once.cpp new file mode 100644 index 0000000000000..65bbd2bd3bda1 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/performance/construct-reusable-objects-once.cpp @@ -0,0 +1,366 @@ +// RUN: %check_clang_tidy %s performance-construct-reusable-objects-once %t -- \ +// RUN: -config='{CheckOptions: { \ +// RUN: performance-construct-reusable-objects-once.CheckedClasses: "::std::basic_regex;ReusableClass", \ +// RUN: performance-construct-reusable-objects-once.IgnoredFunctions: "::main;::global::init;C::C;D::~D;ns::MyClass::foo", \ +// RUN: }}' \ +// RUN: -- -I%S/Inputs/construct-reusable-objects-once + +#include "regex.h" + +void PositiveEmpty() { + std::regex r1; + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: variable 'r1' of type 'std::basic_regex' is constructed with only constant literals on each invocation of 'PositiveEmpty'; + std::wregex wr1; + // CHECK-MESSAGES: [[@LINE-1]]:15: warning: variable 'wr1' of type 'std::basic_regex' is constructed with only constant literals on each invocation of 'PositiveEmpty'; +} + +void PositiveSugared() { + using my_using_regex = std::regex; + my_using_regex r1; + // CHECK-MESSAGES: [[@LINE-1]]:18: warning: variable 'r1' of type 'std::basic_regex' is constructed with only constant literals on each invocation of 'PositiveSugared'; + + typedef std::wregex my_typedef_regex; + my_typedef_regex wr1; + // CHECK-MESSAGES: [[@LINE-1]]:20: warning: variable 'wr1' of type 'std::basic_regex' is constructed with only constant literals on each invocation of 'PositiveSugared'; + + using std::regex; + regex r2; + // CHECK-MESSAGES: [[@LINE-1]]:9: warning: variable 'r2' of type 'std::basic_regex' is constructed with only constant literals on each invocation of 'PositiveSugared'; +} + +void PositiveWithLiterals() { + std::regex r1("x"); + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: variable 'r1' of type 'std::basic_regex' is constructed with only constant literals on each invocation of 'PositiveWithLiterals'; + std::wregex wr1(L"x"); + // CHECK-MESSAGES: [[@LINE-1]]:15: warning: variable 'wr1' of type 'std::basic_regex' is constructed with only constant literals on each invocation of 'PositiveWithLiterals'; + std::regex r2("x", 1); + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: variable 'r2' of type 'std::basic_regex' is constructed with only constant literals on each invocation of 'PositiveWithLiterals'; + std::wregex wr2(L"x", 2); + // CHECK-MESSAGES: [[@LINE-1]]:15: warning: variable 'wr2' of type 'std::basic_regex' is constructed with only constant literals on each invocation of 'PositiveWithLiterals'; +} + +const char* const text = ""; +const char text2[] = ""; + +const wchar_t* const wtext = L""; +const wchar_t wtext2[] = L""; + +const int c_i = 1; +constexpr int ce_i = 1; + +void PositiveWithVariables() { + std::regex r1("x", c_i); + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: variable 'r1' of type 'std::basic_regex' is constructed with only constant literals on each invocation of 'PositiveWithVariables'; + std::regex r2("x", ce_i); + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: variable 'r2' of type 'std::basic_regex' is constructed with only constant literals on each invocation of 'PositiveWithVariables'; + + std::regex r3(text); + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: variable 'r3' of type 'std::basic_regex' is constructed with only constant literals on each invocation of 'PositiveWithVariables'; + std::wregex wr3(wtext); + // CHECK-MESSAGES: [[@LINE-1]]:15: warning: variable 'wr3' of type 'std::basic_regex' is constructed with only constant literals on each invocation of 'PositiveWithVariables'; + std::regex r4(text2); + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: variable 'r4' of type 'std::basic_regex' is constructed with only constant literals on each invocation of 'PositiveWithVariables'; + std::wregex wr4(wtext2); + // CHECK-MESSAGES: [[@LINE-1]]:15: warning: variable 'wr4' of type 'std::basic_regex' is constructed with only constant literals on each invocation of 'PositiveWithVariables'; + + std::regex r5(text, 1); + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: variable 'r5' of type 'std::basic_regex' is constructed with only constant literals on each invocation of 'PositiveWithVariables'; + std::wregex wr5(wtext, 2); + // CHECK-MESSAGES: [[@LINE-1]]:15: warning: variable 'wr5' of type 'std::basic_regex' is constructed with only constant literals on each invocation of 'PositiveWithVariables'; + std::regex r6(text2, 3); + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: variable 'r6' of type 'std::basic_regex' is constructed with only constant literals on each invocation of 'PositiveWithVariables'; + std::wregex wr6(wtext2, 4); + // CHECK-MESSAGES: [[@LINE-1]]:15: warning: variable 'wr6' of type 'std::basic_regex' is constructed with only constant literals on each invocation of 'PositiveWithVariables'; + + // Local variables constructed from literals + const char* const ltext = ""; + const wchar_t* const wltext = L""; + const int l_i = 1; + constexpr int l_ce_i = 1; + + std::regex r7(ltext, 7); + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: variable 'r7' of type 'std::basic_regex' is constructed with only constant literals on each invocation of 'PositiveWithVariables'; + std::wregex wr7(wltext, 8); + // CHECK-MESSAGES: [[@LINE-1]]:15: warning: variable 'wr7' of type 'std::basic_regex' is constructed with only constant literals on each invocation of 'PositiveWithVariables'; + + std::regex r8(text, 1, std::regex::icase); + // CHEC... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/131455 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
