[clang-tools-extra] [clang-tidy] new check readability-mark-static (PR #90830)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-tidy

Author: Congcong Cai (HerrCai0907)

<details>
<summary>Changes</summary>

Add new check readability-mark-static to dectect variable and function can be marked as static.


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


11 Files Affected:

- (modified) clang-tools-extra/clang-tidy/readability/CMakeLists.txt (+1) 
- (added) clang-tools-extra/clang-tidy/readability/MarkStaticCheck.cpp (+65) 
- (added) clang-tools-extra/clang-tidy/readability/MarkStaticCheck.h (+33) 
- (modified) clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp (+2) 
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+5) 
- (modified) clang-tools-extra/docs/clang-tidy/checks/list.rst (+1) 
- (added) clang-tools-extra/docs/clang-tidy/checks/readability/mark-static.rst (+26) 
- (added) clang-tools-extra/test/clang-tidy/checkers/readability/Inputs/mark-static-var/func.h (+3) 
- (added) clang-tools-extra/test/clang-tidy/checkers/readability/Inputs/mark-static-var/var.h (+3) 
- (added) clang-tools-extra/test/clang-tidy/checkers/readability/mark-static-func.cpp (+24) 
- (added) clang-tools-extra/test/clang-tidy/checkers/readability/mark-static-var.cpp (+34) 


``````````diff
diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
index 41065fc8e87859..3e0cbc4d943274 100644
--- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
@@ -23,6 +23,7 @@ add_clang_library(clangTidyReadabilityModule
   IdentifierLengthCheck.cpp
   IdentifierNamingCheck.cpp
   ImplicitBoolConversionCheck.cpp
+  MarkStaticCheck.cpp
   RedundantInlineSpecifierCheck.cpp
   InconsistentDeclarationParameterNameCheck.cpp
   IsolateDeclarationCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/readability/MarkStaticCheck.cpp b/clang-tools-extra/clang-tidy/readability/MarkStaticCheck.cpp
new file mode 100644
index 00000000000000..c8881963071de8
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/readability/MarkStaticCheck.cpp
@@ -0,0 +1,65 @@
+//===--- MarkStaticCheck.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 "MarkStaticCheck.h"
+#include "clang/AST/Decl.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/ASTMatchers/ASTMatchersMacros.h"
+#include "clang/Basic/Specifiers.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+namespace {
+
+AST_POLYMORPHIC_MATCHER(isFirstDecl,
+                        AST_POLYMORPHIC_SUPPORTED_TYPES(FunctionDecl,
+                                                        VarDecl)) {
+  return Node.isFirstDecl();
+}
+
+AST_MATCHER(Decl, isInMainFile) {
+  return Finder->getASTContext().getSourceManager().isInMainFile(
+      Node.getLocation());
+}
+
+AST_POLYMORPHIC_MATCHER(isExternStorageClass,
+                        AST_POLYMORPHIC_SUPPORTED_TYPES(FunctionDecl,
+                                                        VarDecl)) {
+  return Node.getStorageClass() == SC_Extern;
+}
+
+} // namespace
+
+void MarkStaticCheck::registerMatchers(MatchFinder *Finder) {
+  auto Common =
+      allOf(isFirstDecl(), isInMainFile(),
+            unless(anyOf(isStaticStorageClass(), isExternStorageClass(),
+                         isInAnonymousNamespace())));
+  Finder->addMatcher(
+      functionDecl(Common, unless(cxxMethodDecl()), unless(isMain()))
+          .bind("fn"),
+      this);
+  Finder->addMatcher(varDecl(Common, hasGlobalStorage()).bind("var"), this);
+}
+
+void MarkStaticCheck::check(const MatchFinder::MatchResult &Result) {
+  if (const auto *FD = Result.Nodes.getNodeAs<FunctionDecl>("fn")) {
+    diag(FD->getLocation(), "function %0 can be static") << FD;
+    return;
+  }
+  if (const auto *VD = Result.Nodes.getNodeAs<VarDecl>("var")) {
+    diag(VD->getLocation(), "variable %0 can be static") << VD;
+    return;
+  }
+  llvm_unreachable("");
+}
+
+} // namespace clang::tidy::readability
diff --git a/clang-tools-extra/clang-tidy/readability/MarkStaticCheck.h b/clang-tools-extra/clang-tidy/readability/MarkStaticCheck.h
new file mode 100644
index 00000000000000..0af6788d3e3d85
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/readability/MarkStaticCheck.h
@@ -0,0 +1,33 @@
+//===--- MarkStaticCheck.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_READABILITY_MARKSTATICCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_MARKSTATICCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::readability {
+
+/// Detects variable and function can be marked as static.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability/mark-static.html
+class MarkStaticCheck : public ClangTidyCheck {
+public:
+  MarkStaticCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  std::optional<TraversalKind> getCheckTraversalKind() const override {
+    return TK_IgnoreUnlessSpelledInSource;
+  }
+};
+
+} // namespace clang::tidy::readability
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_MARKSTATICCHECK_H
diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
index d61c0ba39658e5..5c04426818b5f7 100644
--- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -32,6 +32,7 @@
 #include "IsolateDeclarationCheck.h"
 #include "MagicNumbersCheck.h"
 #include "MakeMemberFunctionConstCheck.h"
+#include "MarkStaticCheck.h"
 #include "MathMissingParenthesesCheck.h"
 #include "MisleadingIndentationCheck.h"
 #include "MisplacedArrayIndexCheck.h"
@@ -106,6 +107,7 @@ class ReadabilityModule : public ClangTidyModule {
         "readability-identifier-naming");
     CheckFactories.registerCheck<ImplicitBoolConversionCheck>(
         "readability-implicit-bool-conversion");
+    CheckFactories.registerCheck<MarkStaticCheck>("readability-mark-static");
     CheckFactories.registerCheck<MathMissingParenthesesCheck>(
         "readability-math-missing-parentheses");
     CheckFactories.registerCheck<RedundantInlineSpecifierCheck>(
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 5956ccb925485c..35e4d7e722f1de 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -151,6 +151,11 @@ New checks
   Enforces consistent style for enumerators' initialization, covering three
   styles: none, first only, or all initialized explicitly.
 
+- New :doc:`readability-mark-static
+  <clang-tidy/checks/readability/mark-static>` check.
+
+  Detects variable and function can be marked as static.
+
 - New :doc:`readability-math-missing-parentheses
   <clang-tidy/checks/readability/math-missing-parentheses>` check.
 
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 5cdaf9e35b6acd..4f473e13eb9480 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -364,6 +364,7 @@ Clang-Tidy Checks
    :doc:`readability-isolate-declaration <readability/isolate-declaration>`, "Yes"
    :doc:`readability-magic-numbers <readability/magic-numbers>`,
    :doc:`readability-make-member-function-const <readability/make-member-function-const>`, "Yes"
+   :doc:`readability-mark-static <readability/mark-static>`,
    :doc:`readability-math-missing-parentheses <readability/math-missing-parentheses>`, "Yes"
    :doc:`readability-misleading-indentation <readability/misleading-indentation>`,
    :doc:`readability-misplaced-array-index <readability/misplaced-array-index>`, "Yes"
diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/mark-static.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/mark-static.rst
new file mode 100644
index 00000000000000..331e969ee061b9
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/readability/mark-static.rst
@@ -0,0 +1,26 @@
+.. title:: clang-tidy - readability-mark-static
+
+readability-mark-static
+=======================
+
+Detects variable and function can be marked as static.
+
+Static functions and variables are scoped to a single file. Marking functions
+and variables as static helps to better remove dead code. In addition, it gives
+the compiler more information and can help compiler make more aggressive
+optimizations.
+
+Example:
+
+.. code-block:: c++
+  int v1; // can be marked as static
+
+  void fn1(); // can be marked as static
+
+  namespace {
+    // no need since already in anonymous namespace
+    int v2;
+    void fn2();
+  }
+  // no need since already in anonymous namespace
+  extern int v2;
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/Inputs/mark-static-var/func.h b/clang-tools-extra/test/clang-tidy/checkers/readability/Inputs/mark-static-var/func.h
new file mode 100644
index 00000000000000..31c68e9ad0cd92
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/Inputs/mark-static-var/func.h
@@ -0,0 +1,3 @@
+#pragma once
+
+void func_header();
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/Inputs/mark-static-var/var.h b/clang-tools-extra/test/clang-tidy/checkers/readability/Inputs/mark-static-var/var.h
new file mode 100644
index 00000000000000..37e4cfbafff146
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/Inputs/mark-static-var/var.h
@@ -0,0 +1,3 @@
+#pragma once
+
+extern int gloabl_header;
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/mark-static-func.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/mark-static-func.cpp
new file mode 100644
index 00000000000000..c9adcc70b8c5cb
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/mark-static-func.cpp
@@ -0,0 +1,24 @@
+// RUN: %check_clang_tidy %s readability-mark-static %t -- -- -I%S/Inputs/mark-static-var
+
+#include "func.h"
+
+void func() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func'
+
+template<class T>
+void func_template() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func_template'
+
+struct S {
+  void method();
+};
+void S::method() {}
+
+void func_header();
+extern void func_extern();
+static void func_static();
+namespace {
+void func_anonymous_ns();
+} // namespace
+
+int main(int argc, const char*argv[]) {}
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/mark-static-var.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/mark-static-var.cpp
new file mode 100644
index 00000000000000..f0b0486dd38c63
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/mark-static-var.cpp
@@ -0,0 +1,34 @@
+// RUN: %check_clang_tidy %s readability-mark-static %t -- -- -I%S/Inputs/mark-static-var
+
+#include "var.h"
+
+int global;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: variable 'global'
+
+template<class T>
+T global_template;
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: variable 'global_template'
+
+int gloabl_header;
+
+extern int global_extern;
+
+static int global_static;
+
+namespace {
+static int global_anonymous_ns;
+namespace NS {
+static int global_anonymous_ns;
+}
+}
+
+static void f(int para) {
+  int local;
+  static int local_static;
+}
+
+struct S {
+  int m1;
+  static int m2;
+};
+int S::m2;

``````````

</details>


https://github.com/llvm/llvm-project/pull/90830