[clang-tidy] Add more constexpr options to readability-identifier-naming

[MilkeeyCat]

I added new options as mentioned in the issue, do you think there should also be one more option for constexpr class member?

closes #54110

[llvmbot]

@llvm/pr-subscribers-clang-tools-extra

@llvm/pr-subscribers-clang-tidy

Author: Alex White (MilkeeyCat)

Changes

I added new options as mentioned in the issue, do you think there should also be one more option for constexpr class member?

closes #54110

---

Full diff: https://github.com/llvm/llvm-project/pull/162160.diff

1 Files Affected:

• (modified) clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp (+16-2)

diff --git a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
index 5178bee5c3374..1b6f7117ddc4d 100644
--- a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -83,14 +83,17 @@ namespace readability {
     m(Member) \
     m(ClassConstant) \
     m(ClassMember) \
+    m(ConstexprGlobalVariable) \
     m(GlobalConstant) \
     m(GlobalConstantPointer) \
     m(GlobalPointer) \
     m(GlobalVariable) \
+    m(ConstexprLocalVariable) \
     m(LocalConstant) \
     m(LocalConstantPointer) \
     m(LocalPointer) \
     m(LocalVariable) \
+    m(ConstexprStaticVariable) \
     m(StaticConstant) \
     m(StaticVariable) \
     m(Constant) \
@@ -1497,8 +1500,19 @@ StyleKind IdentifierNamingCheck::findStyleKindForField(
 StyleKind IdentifierNamingCheck::findStyleKindForVar(
     const VarDecl *Var, QualType Type,
     ArrayRef<std::optional<NamingStyle>> NamingStyles) const {
-  if (Var->isConstexpr() && NamingStyles[SK_ConstexprVariable])
-    return SK_ConstexprVariable;
+  if (Var->isConstexpr()) {
+    if (Var->isFileVarDecl() && NamingStyles[SK_ConstexprGlobalVariable])
+      return SK_ConstexprGlobalVariable;
+
+    if (Var->isStaticLocal() && NamingStyles[SK_ConstexprStaticVariable])
+      return SK_ConstexprStaticVariable;
+
+    if (Var->isLocalVarDecl() && NamingStyles[SK_ConstexprLocalVariable])
+      return SK_ConstexprLocalVariable;
+
+    if (NamingStyles[SK_ConstexprVariable])
+      return SK_ConstexprVariable;
+  }
 
   if (!Type.isNull() && Type.isConstQualified()) {
     if (Var->isStaticDataMember() && NamingStyles[SK_ClassConstant])

[MilkeeyCat]

@HerrCai0907 @PiotrZSL

[vbvictor]

Please add: release notes, tests.
See other PRs with clang-tidy tag for reference

[EugeneZelenko]

Please mention changes in Release Notes.

[MilkeeyCat]

I'll change Release Notes and add test cases soon, but I can't decide how should the options be called, is it GlobalConstexprCase because there's already GlobalConstantCase, or is it GlobalConstexprVariableCase because there's already ConstexprVariableCase or somehow else?

[HerrCai0907]

I prefer GlobalConstexprVariableCase since constexpr also works for function.

[ChillerDragon]

@HerrCai0907 a constexpr is const so it does not change. A variable means it can vary. Isn't it contradicting then to say a constant variable (ConstexprVariable) exists? The way I see it there are constants and there are variables.

[MilkeeyCat]

I made option descriptions using other options, but some sentences make no sense or are not clear/correct :\

llvm-project/clang-tools-extra/docs/clang-tidy/checks/readability/identifier-naming.rst

Lines 960 to 961 in 86ad98c

	When defined, the check will ensure global constant names will add the
	prefixed with the given value (regardless of casing).

llvm-project/clang-tools-extra/docs/clang-tidy/checks/readability/identifier-naming.rst

Lines 960 to 961 in 86ad98c

	When defined, the check will ensure global constant names will add the
	prefixed with the given value (regardless of casing).

Doesn't it check whether the name has the suffix?

[HerrCai0907]

@HerrCai0907 a constexpr is const so it does not change. A variable means it can vary. Isn't it contradicting then to say a constant variable (ConstexprVariable) exists? The way I see it there are constants and there are variables.

From correctness perspective, maybe GlobalConstexprConstantCase is better.
But there are already lots of ConstexprVariable in this check's options. I prefer to follow the existing names.

llvm-project/clang-tools-extra/docs/clang-tidy/checks/readability/identifier-naming.rst

Lines 801 to 804 in 86ad98c

	- ConstexprVariableCase of ``lower_case``
	- ConstexprVariablePrefix of ``pre_``
	- ConstexprVariableSuffix of ``_post``
	- ConstexprVariableHungarianPrefix of ``On``

[ewancg]

It is logically questionable but every C++ resource I know about refers to constants defined with constexpr as "constexpr variables". Here is a long list of references to it in the standard: https://github.com/search?q=repo%3Acplusplus%2Fdraft%20constexpr%20variable

[vbvictor]

GlobalConstexprVariableCase because there's already ConstexprVariableCase

I'm +1 on GlobalConstexprVariableCase to keep consistent, plus I don't see why constexpr declarations can't be variables. constexpr variable can be calculated at compile time by constexpr/constinit functions, so It doesn't have to hold some predefined literal value.

[ewancg]

This is ebbing into offtopic discussion but "variable" strongly implies it has the ability to be mutated (to vary) at runtime. The same can be said for constants defined via. const, they can hold values from any prior computations, but once they are initialized they are immutable (at least in concept).