[clang-tidy] Teach cppcoreguidelines-interfaces-global-init about constinit

[localspook]

This check already understands how constexpr makes initialization order problems impossible, and C++20's constinit provides the exact same guarantees.

[llvmbot]

@llvm/pr-subscribers-clang-tidy

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

Author: Victor Chernyakin (localspook)

Changes

This check already understands how constexpr makes initialization order problems impossible, and C++20's constinit provides the exact same guarantees while placing fewer restrictions on the user.

---

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

3 Files Affected:

• (modified) clang-tools-extra/clang-tidy/cppcoreguidelines/InterfacesGlobalInitCheck.cpp (+1-1)
• (modified) clang-tools-extra/docs/ReleaseNotes.rst (+4)
• (modified) clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/interfaces-global-init.cpp (+26-2)

diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/InterfacesGlobalInitCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/InterfacesGlobalInitCheck.cpp
index e8a2e7b8ef86d..e9f0bd98cad16 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/InterfacesGlobalInitCheck.cpp
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/InterfacesGlobalInitCheck.cpp
@@ -19,7 +19,7 @@ void InterfacesGlobalInitCheck::registerMatchers(MatchFinder *Finder) {
               hasDeclContext(anyOf(translationUnitDecl(), // Global scope.
                                    namespaceDecl(),       // Namespace scope.
                                    recordDecl())),        // Class scope.
-              unless(isConstexpr()));
+              unless(isConstexpr()), unless(isConstinit()));
 
   const auto ReferencesUndefinedGlobalVar = declRefExpr(hasDeclaration(
       varDecl(GlobalVarDecl, unless(isDefinition())).bind("referencee")));
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index ad869265a2db5..bff7cfa4fe525 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -212,6 +212,10 @@ Changes in existing checks
   <clang-tidy/checks/cppcoreguidelines/avoid-goto>` check by adding the option
   `IgnoreMacros` to ignore ``goto`` labels defined in macros.
 
+- Improved :doc:`cppcoreguidelines-interfaces-global-init
+  <clang-tidy/checks/cppcoreguidelines/interfaces-global-init>` check by
+  fixing false positives on uses of ``constinit`` variables.
+
 - Improved :doc:`cppcoreguidelines-missing-std-forward
   <clang-tidy/checks/cppcoreguidelines/missing-std-forward>` check by adding a
   flag to specify the function used for forwarding instead of ``std::forward``.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/interfaces-global-init.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/interfaces-global-init.cpp
index 51f79e522c0ca..38d02c6a7f186 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/interfaces-global-init.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/interfaces-global-init.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s cppcoreguidelines-interfaces-global-init %t
+// RUN: %check_clang_tidy -std=c++20 %s cppcoreguidelines-interfaces-global-init %t
 
 constexpr int makesInt() { return 3; }
 constexpr int takesInt(int i) { return i + 1; }
@@ -14,11 +14,19 @@ static int GlobalScopeBadInit3 = takesIntPtr(&ExternGlobal);
 static int GlobalScopeBadInit4 = 3 * (ExternGlobal + 2);
 // CHECK-MESSAGES: [[@LINE-1]]:12: warning: initializing non-local variable with non-const expression depending on uninitialized non-local variable 'ExternGlobal'
 
+extern constinit int ExternConstinitGlobal;
+static int GlobalScopeConstinit1 = ExternConstinitGlobal;
+static int GlobalScopeConstinit2 = takesInt(ExternConstinitGlobal);
+static int GlobalScopeConstinit3 = takesIntPtr(&ExternConstinitGlobal);
+static int GlobalScopeConstinit4 = 3 * (ExternConstinitGlobal + 2);
+
 namespace ns {
 static int NamespaceScope = makesInt();
 static int NamespaceScopeBadInit = takesInt(ExternGlobal);
 // CHECK-MESSAGES: [[@LINE-1]]:12: warning: initializing non-local variable with non-const expression depending on uninitialized non-local variable 'ExternGlobal'
 
+static int NamespaceScopeConstinit = takesInt(ExternConstinitGlobal);
+
 struct A {
   static int ClassScope;
   static int ClassScopeBadInit;
@@ -29,6 +37,15 @@ int A::ClassScopeBadInit = takesInt(ExternGlobal);
 
 static int FromClassBadInit = takesInt(A::ClassScope);
 // CHECK-MESSAGES: [[@LINE-1]]:12: warning: initializing non-local variable with non-const expression depending on uninitialized non-local variable 'ClassScope'
+
+struct B {
+  static constinit int ClassScopeConstinit;
+  static int ClassScopeFromConstinit;
+};
+
+int B::ClassScopeFromConstinit = takesInt(ExternConstinitGlobal);
+static int FromClassScopeConstinit = takesInt(B::ClassScopeConstinit);
+
 } // namespace ns
 
 // "const int B::I;" is fine, it just ODR-defines B::I. See [9.4.3] Static
@@ -42,6 +59,14 @@ const int B1::J;
 // CHECK-MESSAGES: [[@LINE-1]]:15: warning: initializing non-local variable with non-const expression depending on uninitialized non-local variable 'I'
 const int B1::I;
 
+class D {
+  static const constinit int I = 0;
+  static const int J = I;
+};
+
+const int D::J;
+const int D::I;
+
 void f() {
   // This is fine, it's executed after dynamic initialization occurs.
   static int G = takesInt(ExternGlobal);
@@ -81,4 +106,3 @@ class B2 {
 };
 const int B2::I;
 const int B2::J;
-

[vbvictor]

LGTM

[5chmidti]

LGTM