[clang-tidy] Add misc-use-anonymous-namespace check

[carlosgalvezp]

Fix release notes.

[carlosgalvezp]

Add reference to undeprecation of static.

[Eugene.Zelenko]

clang-tools-extra/clang-tidy/modernize/UseAnonymousNamespaceCheck.cpp

31

Please don't use auto unless type is explicitly spelled in same statement or iterator.

[carlosgalvezp]

Address review comments.

[njames93]

I'm not entirely sure why this belongs in the modernize module given anonymous namespaces have been in c++ forever, maybe its more of a misc check? Also the modernize checks are meant to actually emit fixes(ignore the c arrays check :) ), Right now, this only warn, it doesn't appear to act

[carlosgalvezp]

In D137340#3906250, @njames93 wrote:

I'm not entirely sure why this belongs in the modernize module given anonymous namespaces have been in c++ forever, maybe its more of a misc check? Also the modernize checks are meant to actually emit fixes(ignore the c arrays check :) ), Right now, this only warn, it doesn't appear to act

Don't have a strong opinion, if you want I can move it there! I put in modernize as in "modernize from C-style into C++ style". But I know there's diverse opinions about this (LLVM not following this, for example) so perhaps it should be misc. Let me know which one you'd prefer so I can apply the change!

I didn't add fix because it would add quite some complexity, e.g. figuring out if there's is already an anon namespace to move into, or create a new one, users might want configurable behavior, etc.

[carlosgalvezp]

Move to misc

[carlosgalvezp]

Update commit message

[carlosgalvezp]

Remove some code duplication

[carlosgalvezp]

Fix typo in binding names

[carlosgalvezp]

Simplify code for printing the type of the declaration.

[carlosgalvezp]

@njames93 Friendly ping

[carlosgalvezp]

@njames93 Ping again. Alternatively, could someone else review? @aaron.ballman maybe?

[carlosgalvezp]

Another week, another ping @njames93 @aaron.ballman

I have addressed all comments since 3 weeks ago, and have not received any objections. I intend to land this patch next week (1st December) if I don't receive any further feedback.

[carlosgalvezp]

Replace "the" anonymous namespace with "an" anonymous namespace
Rebase.

[bigdavedev]

clang-tools-extra/docs/clang-tidy/checks/misc/use-anonymous-namespace.rst

7–9 ↗

(On Diff #478603)

Would we want this to be a fix as well? Removing the static keyword automatically would be nice.

[carlosgalvezp]

clang-tools-extra/docs/clang-tidy/checks/misc/use-anonymous-namespace.rst

7–9 ↗

(On Diff #478603)

Would make sense to fix, but unfortunately with my limited AST knowledge I don't know how to get the location of the static token to apply a removal. So unless someone wants to help out with that it will have to stay as is for now.

[bigdavedev]

Looks good to me. All comments/suggestions/concerns addressed.

clang-tools-extra/docs/clang-tidy/checks/misc/use-anonymous-namespace.rst

7–9 ↗

(On Diff #478603)

Ack.

clang-tools-extra/clang-tidy/modernize/CMakeLists.txt

add_clang_library(clangTidyModernizeModule
   RawStringLiteralCheck.cpp
   RedundantVoidArgCheck.cpp
   ReplaceAutoPtrCheck.cpp
   ReplaceDisallowCopyAndAssignMacroCheck.cpp
   ReplaceRandomShuffleCheck.cpp
   ReturnBracedInitListCheck.cpp
   ShrinkToFitCheck.cpp
   UnaryStaticAssertCheck.cpp
+  UseAnonymousNamespaceCheck.cpp
   UseAutoCheck.cpp
   UseBoolLiteralsCheck.cpp
   UseDefaultMemberInitCheck.cpp
   UseEmplaceCheck.cpp
   UseEqualsDefaultCheck.cpp
   UseEqualsDeleteCheck.cpp
   UseNodiscardCheck.cpp
   UseNoexceptCheck.cpp

clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp

 #include "RawStringLiteralCheck.h"
 #include "RedundantVoidArgCheck.h"
 #include "ReplaceAutoPtrCheck.h"
 #include "ReplaceDisallowCopyAndAssignMacroCheck.h"
 #include "ReplaceRandomShuffleCheck.h"
 #include "ReturnBracedInitListCheck.h"
 #include "ShrinkToFitCheck.h"
 #include "UnaryStaticAssertCheck.h"
+#include "UseAnonymousNamespaceCheck.h"
 #include "UseAutoCheck.h"
 #include "UseBoolLiteralsCheck.h"
 #include "UseDefaultMemberInitCheck.h"
 #include "UseEmplaceCheck.h"
 #include "UseEqualsDefaultCheck.h"
 #include "UseEqualsDeleteCheck.h"
 #include "UseNodiscardCheck.h"
 #include "UseNoexceptCheck.h"
CheckFactories.registerCheck<ReplaceDisallowCopyAndAssignMacroCheck>(
         "modernize-replace-disallow-copy-and-assign-macro");
     CheckFactories.registerCheck<ReplaceRandomShuffleCheck>(
         "modernize-replace-random-shuffle");
     CheckFactories.registerCheck<ReturnBracedInitListCheck>(
         "modernize-return-braced-init-list");
     CheckFactories.registerCheck<ShrinkToFitCheck>("modernize-shrink-to-fit");
     CheckFactories.registerCheck<UnaryStaticAssertCheck>(
         "modernize-unary-static-assert");
+    CheckFactories.registerCheck<UseAnonymousNamespaceCheck>(
+        "modernize-use-anonymous-namespace");
     CheckFactories.registerCheck<UseAutoCheck>("modernize-use-auto");
     CheckFactories.registerCheck<UseBoolLiteralsCheck>(
         "modernize-use-bool-literals");
     CheckFactories.registerCheck<UseDefaultMemberInitCheck>(
         "modernize-use-default-member-init");
     CheckFactories.registerCheck<UseEmplaceCheck>("modernize-use-emplace");
     CheckFactories.registerCheck<UseEqualsDefaultCheck>("modernize-use-equals-default");
     CheckFactories.registerCheck<UseEqualsDeleteCheck>(

clang-tools-extra/clang-tidy/modernize/UseAnonymousNamespaceCheck.h

+//===--- UseAnonymousNamespaceCheck.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_MODERNIZE_USEANONYMOUSNAMESPACECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USEANONYMOUSNAMESPACECHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+/// Warns when using 'static' functions or variables at global scope, and
+/// suggests moving them to the anonymous namespace. It also suggests removing
+/// 'static' if they are already inside the anonymous namespace.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/modernize/use-anonymous-namespace.html
+class UseAnonymousNamespaceCheck : public ClangTidyCheck {
+public:
+  UseAnonymousNamespaceCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    return LangOpts.CPlusPlus;
+  }
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USEANONYMOUSNAMESPACECHECK_H

clang-tools-extra/clang-tidy/modernize/UseAnonymousNamespaceCheck.cpp

+//===--- UseAnonymousNamespaceCheck.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 "UseAnonymousNamespaceCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+namespace {
+AST_POLYMORPHIC_MATCHER(isStatic, AST_POLYMORPHIC_SUPPORTED_TYPES(FunctionDecl,
+                                                                  VarDecl)) {
+  return Node.getStorageClass() == SC_Static;
+}
+
+AST_MATCHER(FunctionDecl, isMemberFunction) {
+  return llvm::isa<CXXMethodDecl>(&Node);
+}
+AST_MATCHER(VarDecl, isStaticDataMember) { return Node.isStaticDataMember(); }
+} // namespace
+
+static bool isInAnonymousNamespace(const Decl *Decl) {
+  const auto *DC = Decl->getDeclContext();
+  if (DC && DC->isNamespace()) {
+    const auto *ND = llvm::cast<NamespaceDecl>(DC);
+    if (ND && ND->isAnonymousNamespace())
+      return true;
+  }
+  return false;
+}
+
+void UseAnonymousNamespaceCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      functionDecl(isStatic(), unless(isMemberFunction())).bind("x"), this);
+  Finder->addMatcher(
+      varDecl(isStatic(), unless(anyOf(isStaticLocal(), isStaticDataMember())))
+          .bind("x"),
+      this);
+}
+
+void UseAnonymousNamespaceCheck::check(const MatchFinder::MatchResult &Result) {
+  if (const auto *MatchedDecl = Result.Nodes.getNodeAs<FunctionDecl>("x")) {
+    if (isInAnonymousNamespace(MatchedDecl))
+      diag(MatchedDecl->getLocation(), "function %0 declared 'static' in "
+                                       "anonymous namespace, remove 'static'")
+          << MatchedDecl;
+    else
+      diag(MatchedDecl->getLocation(),
+           "function %0 declared 'static', move to anonymous namespace instead")
+          << MatchedDecl;
+  }
+
+  if (const auto *MatchedDecl = Result.Nodes.getNodeAs<VarDecl>("x")) {
+    if (isInAnonymousNamespace(MatchedDecl))
+      diag(MatchedDecl->getLocation(), "variable %0 declared 'static' in "
+                                       "anonymous namespace, remove 'static'")
+          << MatchedDecl;
+    else
+      diag(MatchedDecl->getLocation(),
+           "variable %0 declared 'static', move to anonymous namespace instead")
+          << MatchedDecl;
+  }
+}
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang

clang-tools-extra/docs/ReleaseNotes.rst

- New :doc:`cppcoreguidelines-avoid-const-or-ref-data-members
 
   Warns when a struct or class uses const or reference (lvalue or rvalue) data members.
 
 - New :doc:`cppcoreguidelines-avoid-do-while
   <clang-tidy/checks/cppcoreguidelines/avoid-do-while>` check.
 
   Warns when using ``do-while`` loops.
 
+- New :doc:`modernize-use-anonymous-namespace
+  <clang-tidy/checks/modernize/use-anonymous-namespace>` check.
+
+  Warns when using ``static`` function or variables at global scope, and suggests
+  moving them to the anonymous namespace.
+
 New check aliases
 ^^^^^^^^^^^^^^^^^
 
 - New alias :doc:`cert-msc54-cpp
   <clang-tidy/checks/cert/msc54-cpp>` to
   :doc:`bugprone-signal-handler
   <clang-tidy/checks/bugprone/signal-handler>` was added.
 

clang-tools-extra/docs/clang-tidy/checks/list.rst

.. csv-table::
    `modernize-raw-string-literal <modernize/raw-string-literal.html>`_, "Yes"
    `modernize-redundant-void-arg <modernize/redundant-void-arg.html>`_, "Yes"
    `modernize-replace-auto-ptr <modernize/replace-auto-ptr.html>`_, "Yes"
    `modernize-replace-disallow-copy-and-assign-macro <modernize/replace-disallow-copy-and-assign-macro.html>`_, "Yes"
    `modernize-replace-random-shuffle <modernize/replace-random-shuffle.html>`_, "Yes"
    `modernize-return-braced-init-list <modernize/return-braced-init-list.html>`_, "Yes"
    `modernize-shrink-to-fit <modernize/shrink-to-fit.html>`_, "Yes"
    `modernize-unary-static-assert <modernize/unary-static-assert.html>`_, "Yes"
+   `modernize-use-anonymous-namespace <modernize/use-anonymous-namespace.html>`_,
    `modernize-use-auto <modernize/use-auto.html>`_, "Yes"
    `modernize-use-bool-literals <modernize/use-bool-literals.html>`_, "Yes"
    `modernize-use-default-member-init <modernize/use-default-member-init.html>`_, "Yes"
    `modernize-use-emplace <modernize/use-emplace.html>`_, "Yes"
    `modernize-use-equals-default <modernize/use-equals-default.html>`_, "Yes"
    `modernize-use-equals-delete <modernize/use-equals-delete.html>`_, "Yes"
    `modernize-use-nodiscard <modernize/use-nodiscard.html>`_, "Yes"
    `modernize-use-noexcept <modernize/use-noexcept.html>`_, "Yes"

clang-tools-extra/docs/clang-tidy/checks/modernize/use-anonymous-namespace.rst

+.. title:: clang-tidy - modernize-use-anonymous-namespace
+
+modernize-use-anonymous-namespace
+=================================
+
+Finds instances of ``static`` functions or variables declared at global scope
+that could instead be moved into the anonymous namespace. It also detects
+instances moved to the anonymous namespace that still keep the redundant
+``static``.
+
+Anonymous namespaces are the "superior alternative" according to the C++
+Standard. ``static`` was proposed for deprecation, but later un-deprecated to
+keep C compatibility [1]. ``static`` is an overloaded term with different meanings in
+different contexts, so it can create confusion.
+
+Examples:
+
+.. code-block:: c++
+
+  // Bad
+  static void foo();
+  static int x;
+
+  // Good
+  namespace {
+    void foo();
+    int x;
+  } // namespace
+
+.. code-block:: c++
+
+  // Bad
+  namespace {
+    static void foo();
+    static int x;
+  }
+
+  // Good
+  namespace {
+    void foo();
+    int x;
+  }  // namespace
+
+[1] `Undeprecating static <https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1012>`_

clang-tools-extra/test/clang-tidy/checkers/modernize/use-anonymous-namespace.cpp

+// RUN: %check_clang_tidy %s modernize-use-anonymous-namespace %t
+
+static void f1();
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function 'f1' declared 'static', move to anonymous namespace instead [modernize-use-anonymous-namespace]
+static int v1;
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: variable 'v1' declared 'static', move to anonymous namespace instead
+
+namespace {
+  static void f2();
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: function 'f2' declared 'static' in anonymous namespace, remove 'static'
+  static int v2;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: variable 'v2' declared 'static' in anonymous namespace, remove 'static'
+}
+
+namespace a {
+  static void f3();
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: function 'f3' declared 'static', move to anonymous namespace instead [modernize-use-anonymous-namespace]
+  static int v3;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: variable 'v3' declared 'static', move to anonymous namespace instead [modernize-use-anonymous-namespace]
+}
+
+namespace a {
+namespace {
+  static void f4();
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: function 'f4' declared 'static' in anonymous namespace, remove 'static'
+  static int v4;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: variable 'v4' declared 'static' in anonymous namespace, remove 'static'
+}
+}
+
+// OK
+void f5();
+int v5;
+
+// OK
+namespace {
+  void f6();
+  int v6;
+}
+
+// OK
+namespace a {
+namespace {
+  void f7();
+  int v7;
+}
+}
+
+// OK
+struct Foo {
+  static void f();
+  static int x;
+};
+
+// OK
+void foo()
+{
+  static int x;
+}