carlosgalvezp created this revision.
Herald added a reviewer: njames93.
Herald added a project: All.
carlosgalvezp requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
I will create a new check "readability-redundant-static" where this
functionality fits better, together with other cases where static
is redundant.
That way we can add automatic fixits there and keep this check
focused on doing only one thing.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D139197
Files:
clang-tools-extra/clang-tidy/misc/UseAnonymousNamespaceCheck.cpp
clang-tools-extra/clang-tidy/misc/UseAnonymousNamespaceCheck.h
clang-tools-extra/docs/clang-tidy/checks/misc/use-anonymous-namespace.rst
clang-tools-extra/test/clang-tidy/checkers/misc/use-anonymous-namespace.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/misc/use-anonymous-namespace.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/misc/use-anonymous-namespace.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/misc/use-anonymous-namespace.cpp
@@ -5,13 +5,6 @@
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
@@ -19,15 +12,6 @@
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: variable 'v3' declared 'static', move to anonymous namespace instead
}
-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;
Index: clang-tools-extra/docs/clang-tidy/checks/misc/use-anonymous-namespace.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/misc/use-anonymous-namespace.rst
+++ clang-tools-extra/docs/clang-tidy/checks/misc/use-anonymous-namespace.rst
@@ -4,9 +4,7 @@
============================
Finds instances of ``static`` functions or variables declared at global scope
-that could instead be moved into an anonymous namespace. It also detects
-instances moved to an anonymous namespace that still keep the redundant
-``static``.
+that could instead be moved into an anonymous namespace.
Anonymous namespaces are the "superior alternative" according to the C++
Standard. ``static`` was proposed for deprecation, but later un-deprecated to
@@ -27,18 +25,4 @@
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>`_
Index: clang-tools-extra/clang-tidy/misc/UseAnonymousNamespaceCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/misc/UseAnonymousNamespaceCheck.h
+++ clang-tools-extra/clang-tidy/misc/UseAnonymousNamespaceCheck.h
@@ -16,8 +16,7 @@
namespace misc {
/// Warns when using 'static' functions or variables at global scope, and
-/// suggests moving them to an anonymous namespace. It also suggests removing
-/// 'static' if they are already inside an anonymous namespace.
+/// suggests moving them to an anonymous namespace.
///
/// For the user-facing documentation see:
/// http://clang.llvm.org/extra/clang-tidy/checks/modernize/use-anonymous-namespace.html
Index: clang-tools-extra/clang-tidy/misc/UseAnonymousNamespaceCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/misc/UseAnonymousNamespaceCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/UseAnonymousNamespaceCheck.cpp
@@ -21,14 +21,10 @@
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 DeclContext *DC = Decl->getDeclContext();
+AST_POLYMORPHIC_MATCHER(isInAnonymousNamespace,
+ AST_POLYMORPHIC_SUPPORTED_TYPES(FunctionDecl,
+ VarDecl)) {
+ const DeclContext *DC = Node.getDeclContext();
if (DC && DC->isNamespace()) {
const auto *ND = llvm::cast<NamespaceDecl>(DC);
if (ND && ND->isAnonymousNamespace())
@@ -37,24 +33,29 @@
return false;
}
+AST_MATCHER(FunctionDecl, isMemberFunction) {
+ return llvm::isa<CXXMethodDecl>(&Node);
+}
+AST_MATCHER(VarDecl, isStaticDataMember) { return Node.isStaticDataMember(); }
+} // namespace
+
template <typename T>
void UseAnonymousNamespaceCheck::processMatch(const T *MatchedDecl) {
StringRef Type = llvm::isa<VarDecl>(MatchedDecl) ? "variable" : "function";
- if (isInAnonymousNamespace(MatchedDecl))
- diag(MatchedDecl->getLocation(), "%0 %1 declared 'static' in "
- "anonymous namespace, remove 'static'")
- << Type << MatchedDecl;
- else
- diag(MatchedDecl->getLocation(),
- "%0 %1 declared 'static', move to anonymous namespace instead")
- << Type << MatchedDecl;
+ diag(MatchedDecl->getLocation(),
+ "%0 %1 declared 'static', move to anonymous namespace instead")
+ << Type << MatchedDecl;
}
void UseAnonymousNamespaceCheck::registerMatchers(MatchFinder *Finder) {
Finder->addMatcher(
- functionDecl(isStatic(), unless(isMemberFunction())).bind("func"), this);
+ functionDecl(isStatic(),
+ unless(anyOf(isInAnonymousNamespace(), isMemberFunction())))
+ .bind("func"),
+ this);
Finder->addMatcher(
- varDecl(isStatic(), unless(anyOf(isStaticLocal(), isStaticDataMember())))
+ varDecl(isStatic(), unless(anyOf(isInAnonymousNamespace(),
+ isStaticLocal(), isStaticDataMember())))
.bind("var"),
this);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
