Add clang tidy check performance constexpr non static in scope #147809
+165
−0
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
|
@llvm/pr-subscribers-clang-tools-extra Author: None (Spaarsh) ChangesCloses #146296 The PR merges the following into the main branch:
Full diff: https://github.com/llvm/llvm-project/pull/147809.diff 8 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/performance/CMakeLists.txt b/clang-tools-extra/clang-tidy/performance/CMakeLists.txt
index c6e547c5089fb..59a7bf9e1e08c 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
+ ConstexprNonStaticInScopeCheck.cpp
EnumSizeCheck.cpp
FasterStringFindCheck.cpp
ForRangeCopyCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/performance/ConstexprNonStaticInScopeCheck.cpp b/clang-tools-extra/clang-tidy/performance/ConstexprNonStaticInScopeCheck.cpp
new file mode 100644
index 0000000000000..fa4ac9de77b0c
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/performance/ConstexprNonStaticInScopeCheck.cpp
@@ -0,0 +1,45 @@
+//===--- ConstexprNonStaticInScopeCheck.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 "ConstexprNonStaticInScopeCheck.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::performance {
+
+void ConstexprNonStaticInScopeCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(varDecl(
+ hasLocalStorage(),
+ isConstexpr(),
+ unless(isStaticLocal())
+ ).bind("constexprVar"), this);
+}
+
+void ConstexprNonStaticInScopeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "WarnInConstexprFuncCpp23", WarnInConstexprFuncCpp23);
+}
+
+void ConstexprNonStaticInScopeCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *Var = Result.Nodes.getNodeAs<clang::VarDecl>("constexprVar");
+ if (!Var)
+ return;
+ const auto *EnclosingFunc = llvm::dyn_cast_or_null<clang::FunctionDecl>(Var->getDeclContext());
+
+ if (EnclosingFunc && EnclosingFunc->isConstexpr()) {
+ // If the function is constexpr, only warn in C++23 and above
+ if (!Result.Context->getLangOpts().CPlusPlus23 || !WarnInConstexprFuncCpp23)
+ return; // Don't warn unless in C++23+ AND the option is enabled
+ }
+
+ diag(Var->getLocation(),
+ "constexpr variable in function scope should be static to ensure static lifetime")
+ << FixItHint::CreateInsertion(Var->getSourceRange().getBegin(), "static ");
+}
+
+} // namespace clang::tidy::performance
diff --git a/clang-tools-extra/clang-tidy/performance/ConstexprNonStaticInScopeCheck.h b/clang-tools-extra/clang-tidy/performance/ConstexprNonStaticInScopeCheck.h
new file mode 100644
index 0000000000000..8bc353519a613
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/performance/ConstexprNonStaticInScopeCheck.h
@@ -0,0 +1,37 @@
+//===--- ConstexprNonStaticInScopeCheck.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_CONSTEXPRNONSTATICINSCOPECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_CONSTEXPRNONSTATICINSCOPECHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::performance {
+
+/// FIXME: Write a short description.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/performance/constexpr-non-static-in-scope.html
+class ConstexprNonStaticInScopeCheck : public ClangTidyCheck {
+public:
+ ConstexprNonStaticInScopeCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ WarnInConstexprFuncCpp23(Options.get("WarnInConstexprFuncCpp23", /*Default=*/true)) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+ return LangOpts.CPlusPlus;
+ }
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+private:
+ const bool WarnInConstexprFuncCpp23;
+};
+
+} // namespace clang::tidy::performance
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_CONSTEXPRNONSTATICINSCOPECHECK_H
diff --git a/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp b/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
index 10ad9ec6fef4c..b571b9b5dd158 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 "ConstexprNonStaticInScopeCheck.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<ConstexprNonStaticInScopeCheck>(
+ "performance-constexpr-non-static-in-scope");
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 e021d6350694e..a814597b130f0 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -148,6 +148,11 @@ New checks
Finds uses of ``std::lock_guard`` and suggests replacing them with C++17's
alternative ``std::scoped_lock``.
+- New :doc:`performance-constexpr-non-static-in-scope
+ <clang-tidy/checks/performance/constexpr-non-static-in-scope>` check.
+
+ FIXME: Write a short description.
+
- New :doc:`portability-avoid-pragma-once
<clang-tidy/checks/portability/avoid-pragma-once>` check.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 5098582d0c42b..2b2a9a316e6d9 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -335,6 +335,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-constexpr-non-static-in-scope <performance/constexpr-non-static-in-scope>`, "Yes"
: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/constexpr-non-static-in-scope.rst b/clang-tools-extra/docs/clang-tidy/checks/performance/constexpr-non-static-in-scope.rst
new file mode 100644
index 0000000000000..f76db268648da
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/performance/constexpr-non-static-in-scope.rst
@@ -0,0 +1,42 @@
+.. title:: clang-tidy - performance-constexpr-non-static-in-scope
+
+performance-constexpr-non-static-in-scope
+=========================================
+
+The check ``performance-constexpr-non-static-in-scope`` identifies ``constexpr`` variables declared in function (local) scope that are not marked ``static``. In most cases, such variables should be declared `static constexpr` to avoid creating a new instance on every function call.
+
+The check will always warn for non-static ``constexpr`` variables in non-constexpr functions. For ``constexpr`` functions, the check warns only in C++23 and newer (and this behavior can be controlled with the ``WarnInConstexprFuncCpp23`` option).
+
+For example:
+When a ``constexpr` ` is declared without ``static``:
+.. code-block:: c++
+ // BEFORE
+ void foo() {
+ constexpr int x = 42;
+ }
+
+ // AFTER
+ void foo() {
+ static constexpr int x = 42; // Corrected to static constexpr
+ }
+
+When a ``constexpr`` is declared in a ``constexpr`` function without ``static`` (only in C++23 and newer):
+.. code-block:: c++
+ // BEFORE
+ constexpr int bar() {
+ constexpr int y = 123;
+ return y;
+ }
+
+ // AFTER
+ constexpr int bar() {
+ static constexpr int y = 123; // Corrected to static constexpr
+ return y;
+ }
+
+Options
+-------
+
+.. option:: WarnInConstexprFuncCpp23
+
+ When true (default), warns on ``constexpr`` variables inside ``constexpr`` functions in C++23 and newer. Set to ``false`` to disable this warning in that scenario.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/constexpr-non-static-in-scope.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/constexpr-non-static-in-scope.cpp
new file mode 100644
index 0000000000000..4cc1d5efbad06
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance/constexpr-non-static-in-scope.cpp
@@ -0,0 +1,31 @@
+// RUN: %check_clang_tidy %s performance-constexpr-non-static-in-scope %t -- -- -std=c++23
+
+// This should trigger the check (constexpr local, not static).
+void f() {
+ constexpr int x = 42;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constexpr variable in function scope should be static to ensure static lifetime [performance-constexpr-non-static-in-scope]
+ // CHECK-FIXES: static constexpr int x = 42;
+}
+
+// This should trigger if WarnInConstexprFuncCpp23 is true and C++23 or newer.
+constexpr int g() {
+ constexpr int y = 123;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constexpr variable in function scope should be static to ensure static lifetime [performance-constexpr-non-static-in-scope]
+ // CHECK-FIXES: static constexpr int y = 123;
+ return y;
+}
+
+// This should NOT trigger the check (already static).
+void h() {
+ static constexpr int z = 99;
+}
+
+// This should NOT trigger the check (not constexpr).
+void i() {
+ int w = 100;
+}
+
+// This should NOT trigger the check (not declared inside a function)
+namespace ns {
+ inline constexpr int MAX_SIZE = 1024;
+}
\ No newline at end of file
|
|
@llvm/pr-subscribers-clang-tidy Author: None (Spaarsh) ChangesCloses #146296 The PR merges the following into the main branch:
Full diff: https://github.com/llvm/llvm-project/pull/147809.diff 8 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/performance/CMakeLists.txt b/clang-tools-extra/clang-tidy/performance/CMakeLists.txt
index c6e547c5089fb..59a7bf9e1e08c 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
+ ConstexprNonStaticInScopeCheck.cpp
EnumSizeCheck.cpp
FasterStringFindCheck.cpp
ForRangeCopyCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/performance/ConstexprNonStaticInScopeCheck.cpp b/clang-tools-extra/clang-tidy/performance/ConstexprNonStaticInScopeCheck.cpp
new file mode 100644
index 0000000000000..fa4ac9de77b0c
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/performance/ConstexprNonStaticInScopeCheck.cpp
@@ -0,0 +1,45 @@
+//===--- ConstexprNonStaticInScopeCheck.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 "ConstexprNonStaticInScopeCheck.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::performance {
+
+void ConstexprNonStaticInScopeCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(varDecl(
+ hasLocalStorage(),
+ isConstexpr(),
+ unless(isStaticLocal())
+ ).bind("constexprVar"), this);
+}
+
+void ConstexprNonStaticInScopeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "WarnInConstexprFuncCpp23", WarnInConstexprFuncCpp23);
+}
+
+void ConstexprNonStaticInScopeCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *Var = Result.Nodes.getNodeAs<clang::VarDecl>("constexprVar");
+ if (!Var)
+ return;
+ const auto *EnclosingFunc = llvm::dyn_cast_or_null<clang::FunctionDecl>(Var->getDeclContext());
+
+ if (EnclosingFunc && EnclosingFunc->isConstexpr()) {
+ // If the function is constexpr, only warn in C++23 and above
+ if (!Result.Context->getLangOpts().CPlusPlus23 || !WarnInConstexprFuncCpp23)
+ return; // Don't warn unless in C++23+ AND the option is enabled
+ }
+
+ diag(Var->getLocation(),
+ "constexpr variable in function scope should be static to ensure static lifetime")
+ << FixItHint::CreateInsertion(Var->getSourceRange().getBegin(), "static ");
+}
+
+} // namespace clang::tidy::performance
diff --git a/clang-tools-extra/clang-tidy/performance/ConstexprNonStaticInScopeCheck.h b/clang-tools-extra/clang-tidy/performance/ConstexprNonStaticInScopeCheck.h
new file mode 100644
index 0000000000000..8bc353519a613
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/performance/ConstexprNonStaticInScopeCheck.h
@@ -0,0 +1,37 @@
+//===--- ConstexprNonStaticInScopeCheck.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_CONSTEXPRNONSTATICINSCOPECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_CONSTEXPRNONSTATICINSCOPECHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::performance {
+
+/// FIXME: Write a short description.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/performance/constexpr-non-static-in-scope.html
+class ConstexprNonStaticInScopeCheck : public ClangTidyCheck {
+public:
+ ConstexprNonStaticInScopeCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ WarnInConstexprFuncCpp23(Options.get("WarnInConstexprFuncCpp23", /*Default=*/true)) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+ return LangOpts.CPlusPlus;
+ }
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+private:
+ const bool WarnInConstexprFuncCpp23;
+};
+
+} // namespace clang::tidy::performance
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_CONSTEXPRNONSTATICINSCOPECHECK_H
diff --git a/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp b/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
index 10ad9ec6fef4c..b571b9b5dd158 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 "ConstexprNonStaticInScopeCheck.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<ConstexprNonStaticInScopeCheck>(
+ "performance-constexpr-non-static-in-scope");
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 e021d6350694e..a814597b130f0 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -148,6 +148,11 @@ New checks
Finds uses of ``std::lock_guard`` and suggests replacing them with C++17's
alternative ``std::scoped_lock``.
+- New :doc:`performance-constexpr-non-static-in-scope
+ <clang-tidy/checks/performance/constexpr-non-static-in-scope>` check.
+
+ FIXME: Write a short description.
+
- New :doc:`portability-avoid-pragma-once
<clang-tidy/checks/portability/avoid-pragma-once>` check.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 5098582d0c42b..2b2a9a316e6d9 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -335,6 +335,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-constexpr-non-static-in-scope <performance/constexpr-non-static-in-scope>`, "Yes"
: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/constexpr-non-static-in-scope.rst b/clang-tools-extra/docs/clang-tidy/checks/performance/constexpr-non-static-in-scope.rst
new file mode 100644
index 0000000000000..f76db268648da
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/performance/constexpr-non-static-in-scope.rst
@@ -0,0 +1,42 @@
+.. title:: clang-tidy - performance-constexpr-non-static-in-scope
+
+performance-constexpr-non-static-in-scope
+=========================================
+
+The check ``performance-constexpr-non-static-in-scope`` identifies ``constexpr`` variables declared in function (local) scope that are not marked ``static``. In most cases, such variables should be declared `static constexpr` to avoid creating a new instance on every function call.
+
+The check will always warn for non-static ``constexpr`` variables in non-constexpr functions. For ``constexpr`` functions, the check warns only in C++23 and newer (and this behavior can be controlled with the ``WarnInConstexprFuncCpp23`` option).
+
+For example:
+When a ``constexpr` ` is declared without ``static``:
+.. code-block:: c++
+ // BEFORE
+ void foo() {
+ constexpr int x = 42;
+ }
+
+ // AFTER
+ void foo() {
+ static constexpr int x = 42; // Corrected to static constexpr
+ }
+
+When a ``constexpr`` is declared in a ``constexpr`` function without ``static`` (only in C++23 and newer):
+.. code-block:: c++
+ // BEFORE
+ constexpr int bar() {
+ constexpr int y = 123;
+ return y;
+ }
+
+ // AFTER
+ constexpr int bar() {
+ static constexpr int y = 123; // Corrected to static constexpr
+ return y;
+ }
+
+Options
+-------
+
+.. option:: WarnInConstexprFuncCpp23
+
+ When true (default), warns on ``constexpr`` variables inside ``constexpr`` functions in C++23 and newer. Set to ``false`` to disable this warning in that scenario.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/constexpr-non-static-in-scope.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/constexpr-non-static-in-scope.cpp
new file mode 100644
index 0000000000000..4cc1d5efbad06
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance/constexpr-non-static-in-scope.cpp
@@ -0,0 +1,31 @@
+// RUN: %check_clang_tidy %s performance-constexpr-non-static-in-scope %t -- -- -std=c++23
+
+// This should trigger the check (constexpr local, not static).
+void f() {
+ constexpr int x = 42;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constexpr variable in function scope should be static to ensure static lifetime [performance-constexpr-non-static-in-scope]
+ // CHECK-FIXES: static constexpr int x = 42;
+}
+
+// This should trigger if WarnInConstexprFuncCpp23 is true and C++23 or newer.
+constexpr int g() {
+ constexpr int y = 123;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constexpr variable in function scope should be static to ensure static lifetime [performance-constexpr-non-static-in-scope]
+ // CHECK-FIXES: static constexpr int y = 123;
+ return y;
+}
+
+// This should NOT trigger the check (already static).
+void h() {
+ static constexpr int z = 99;
+}
+
+// This should NOT trigger the check (not constexpr).
+void i() {
+ int w = 100;
+}
+
+// This should NOT trigger the check (not declared inside a function)
+namespace ns {
+ inline constexpr int MAX_SIZE = 1024;
+}
\ No newline at end of file
|
You can test this locally with the following command:git-clang-format --diff HEAD~1 HEAD --extensions cpp,h -- clang-tools-extra/clang-tidy/performance/ConstexprNonStaticInScopeCheck.cpp clang-tools-extra/clang-tidy/performance/ConstexprNonStaticInScopeCheck.h clang-tools-extra/test/clang-tidy/checkers/performance/constexpr-non-static-in-scope.cpp clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cppView the diff from clang-format here.diff --git a/clang-tools-extra/clang-tidy/performance/ConstexprNonStaticInScopeCheck.cpp b/clang-tools-extra/clang-tidy/performance/ConstexprNonStaticInScopeCheck.cpp
index fa4ac9de7..f785a7d86 100644
--- a/clang-tools-extra/clang-tidy/performance/ConstexprNonStaticInScopeCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/ConstexprNonStaticInScopeCheck.cpp
@@ -14,22 +14,24 @@ using namespace clang::ast_matchers;
namespace clang::tidy::performance {
void ConstexprNonStaticInScopeCheck::registerMatchers(MatchFinder *Finder) {
- Finder->addMatcher(varDecl(
- hasLocalStorage(),
- isConstexpr(),
- unless(isStaticLocal())
- ).bind("constexprVar"), this);
+ Finder->addMatcher(
+ varDecl(hasLocalStorage(), isConstexpr(), unless(isStaticLocal()))
+ .bind("constexprVar"),
+ this);
}
-void ConstexprNonStaticInScopeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+void ConstexprNonStaticInScopeCheck::storeOptions(
+ ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "WarnInConstexprFuncCpp23", WarnInConstexprFuncCpp23);
}
-void ConstexprNonStaticInScopeCheck::check(const MatchFinder::MatchResult &Result) {
+void ConstexprNonStaticInScopeCheck::check(
+ const MatchFinder::MatchResult &Result) {
const auto *Var = Result.Nodes.getNodeAs<clang::VarDecl>("constexprVar");
if (!Var)
return;
- const auto *EnclosingFunc = llvm::dyn_cast_or_null<clang::FunctionDecl>(Var->getDeclContext());
+ const auto *EnclosingFunc =
+ llvm::dyn_cast_or_null<clang::FunctionDecl>(Var->getDeclContext());
if (EnclosingFunc && EnclosingFunc->isConstexpr()) {
// If the function is constexpr, only warn in C++23 and above
@@ -37,9 +39,10 @@ void ConstexprNonStaticInScopeCheck::check(const MatchFinder::MatchResult &Resul
return; // Don't warn unless in C++23+ AND the option is enabled
}
- diag(Var->getLocation(),
- "constexpr variable in function scope should be static to ensure static lifetime")
- << FixItHint::CreateInsertion(Var->getSourceRange().getBegin(), "static ");
+ diag(Var->getLocation(), "constexpr variable in function scope should be "
+ "static to ensure static lifetime")
+ << FixItHint::CreateInsertion(Var->getSourceRange().getBegin(),
+ "static ");
}
} // namespace clang::tidy::performance
diff --git a/clang-tools-extra/clang-tidy/performance/ConstexprNonStaticInScopeCheck.h b/clang-tools-extra/clang-tidy/performance/ConstexprNonStaticInScopeCheck.h
index 8bc353519..ce6ec1a23 100644
--- a/clang-tools-extra/clang-tidy/performance/ConstexprNonStaticInScopeCheck.h
+++ b/clang-tools-extra/clang-tidy/performance/ConstexprNonStaticInScopeCheck.h
@@ -21,13 +21,15 @@ class ConstexprNonStaticInScopeCheck : public ClangTidyCheck {
public:
ConstexprNonStaticInScopeCheck(StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
- WarnInConstexprFuncCpp23(Options.get("WarnInConstexprFuncCpp23", /*Default=*/true)) {}
+ WarnInConstexprFuncCpp23(
+ Options.get("WarnInConstexprFuncCpp23", /*Default=*/true)) {}
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
return LangOpts.CPlusPlus;
}
void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+
private:
const bool WarnInConstexprFuncCpp23;
};
|
vbvictor
requested changes
Jul 9, 2025
| performance-constexpr-non-static-in-scope | ||
| ========================================= | ||
|
|
||
| The check ``performance-constexpr-non-static-in-scope`` identifies ``constexpr`` variables declared in function (local) scope that are not marked ``static``. In most cases, such variables should be declared `static constexpr` to avoid creating a new instance on every function call. |
There was a problem hiding this comment.
Please follow 80-character limit and dont use "the check" or its name.
Look at how other docs is written, e.g. https://clang.llvm.org/extra/clang-tidy/checks/modernize/use-scoped-lock.html
| // This should NOT trigger the check (not declared inside a function) | ||
| namespace ns { | ||
| inline constexpr int MAX_SIZE = 1024; | ||
| } |
| - New :doc:`performance-constexpr-non-static-in-scope | ||
| <clang-tidy/checks/performance/constexpr-non-static-in-scope>` check. | ||
|
|
||
| FIXME: Write a short description. |
There was a problem hiding this comment.
Please add description (should be same as first sentence in docs)
|
|
||
| namespace clang::tidy::performance { | ||
|
|
||
| /// FIXME: Write a short description. |
There was a problem hiding this comment.
Please put here description as first sentence in docs
| Options | ||
| ------- | ||
|
|
||
| .. option:: WarnInConstexprFuncCpp23 |
There was a problem hiding this comment.
I think we should have name WarnInConstexprFuncBeforeCpp23 or WarnInConstexprFuncPreCpp23, WDYT?
Comment on lines
+30
to
+31
| if (!Var) | ||
| return; |
There was a problem hiding this comment.
You should write assert(Var);, "constexprVar" should always be valid.
Comment on lines
+32
to
+38
| const auto *EnclosingFunc = llvm::dyn_cast_or_null<clang::FunctionDecl>(Var->getDeclContext()); | ||
|
|
||
| if (EnclosingFunc && EnclosingFunc->isConstexpr()) { | ||
| // If the function is constexpr, only warn in C++23 and above | ||
| if (!Result.Context->getLangOpts().CPlusPlus23 || !WarnInConstexprFuncCpp23) | ||
| return; // Don't warn unless in C++23+ AND the option is enabled | ||
| } |
There was a problem hiding this comment.
This logic should be put directly inside ASTMatchers, try writing something like:
// previous code
unless(isStaticLocal()),
(!WarnInConstexprFuncCpp23 && !getLangOpts().CPlusPlus23) ?
unless(hasParent(functionDecl(isConstexpr()))) :
anything() | } | ||
|
|
||
| diag(Var->getLocation(), | ||
| "constexpr variable in function scope should be static to ensure static lifetime") |
There was a problem hiding this comment.
"static to ensure static lifetime" seems repetitive and not the main reason why the check exists.
You should write a message about performance.
| @@ -0,0 +1,31 @@ | |||
| // RUN: %check_clang_tidy %s performance-constexpr-non-static-in-scope %t -- -- -std=c++23 | |||
There was a problem hiding this comment.
Write 2 RUN commands in this test file. One for default value of option, another with false.
| @@ -0,0 +1,31 @@ | |||
| // RUN: %check_clang_tidy %s performance-constexpr-non-static-in-scope %t -- -- -std=c++23 | |||
There was a problem hiding this comment.
Add tests with
- template functions
- consteval function/variables
- template classes with template methods
- variables that are type dependent on template arguments
- multiple variables declared in one line
constexpr int a = 10, b = 20;
vbvictor
requested review from
PiotrZSL,
carlosgalvezp,
EugeneZelenko,
5chmidti and
HerrCai0907
|
Please change PR description: use "[clang-tidy]" in front to tell what part of LLVM you changed, and write check name as it in clang-tidy "performance-constexpr-non-static-in-scope" |
PiotrZSL
requested changes
Jul 10, 2025
There was a problem hiding this comment.
I'm not sure if this check changes anything related to performance. Most of this stuff is optimized, local variables with builtin types will be optimized-out anyway and types that actually could harm performance cannot be constexpr. Additionally for such types you move them from stack to rdata/bss section, and that can introduce cache-miss.
For me check:
- Should ignore all builtin types (int, char, pointers, ...)
- Should have an option to ignore classes/structures bellow specific size
But still, I think that this check doesn't make sense, unless you can show example where it actually change something.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #146296
The PR merges the following into the main branch:
clang-tidycheckperformance-constexpr-non-static-in-scopeand its corresponding testsconstexprinside a function withoutstaticconstexprdeclared variable under the following conditions- the variable is not inside a function
- the variable has been declared inside a
constexprfunction with theWarnInConstexprFuncCpp23option set tofalse- the variable has been declared with
static