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

[Piotr Zegar via cfe-commits]

================
@@ -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);
----------------
PiotrZSL wrote:

Few things are missing here, I wrote same check already for private usage, so let me drop it here (for ideas):
```
  Finder->addMatcher(functionDecl(isExpansionInMainFile(),
                                  isDefinition(),
                                  hasExternalFormalLinkage(),
                                  unless(isMain()),
                                  unless(isExternC()),
                                  unless(isExplicitTemplateSpecialization()),
                                  unless(clang::ast_matchers::isTemplateInstantiation()),
                                  unless(cxxMethodDecl()),
                                  unless(hasAncestor(friendDecl())),
                                  unless(hasDeclarationOutsideMainFile())
                                 ).bind("fun"), this);
  Finder->addMatcher(varDecl(isExpansionInMainFile(),
                             isDefinition(),
                             hasGlobalStorage(),
                             unless(isStaticLocal()),
                             unless(isStaticStorageClass()),
                             hasExternalFormalLinkage(),
                             unless(isExternC()),
                             unless(hasDeclarationOutsideMainFile()),
                             unless(hasVarRedeclaration(anyOf(isStaticStorageClass(), isExternC())))
                            ).bind("var"), this);

```

```
AST_MATCHER(Decl, hasDeclarationOutsideMainFile)
{
    auto &SourceManager = Finder->getASTContext().getSourceManager();
    for(const auto* l_it : Node.redecls())
    {
        if (not SourceManager.isInMainFile(SourceManager.getExpansionLoc(l_it->getBeginLoc())))
        {
            return true;
        }
    }

    return not SourceManager.isInMainFile(SourceManager.getExpansionLoc(Node.getBeginLoc()));
}

AST_MATCHER_P(VarDecl, hasVarRedeclaration, ast_matchers::internal::Matcher<VarDecl>, InnerMatcher)
{
    for(const auto* l_it : Node.redecls())
    {
        if (InnerMatcher.matches(*l_it, Finder, Builder))
        {
            return true;
        }
    }

    return false;
}
```

You need also take into account unity builds, something that I didn't took into consideration, when function/variable is in .cpp file, but thats not a a main file. I think there is some API in Clang-tify for that.

But at same time, I were focusing more on external linkage:
`function %0 has external linkage but it's used only in this source file and has no public declaration, make it internal (anonymous namespace or static) or add public declaration in header file"`



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