[PATCH] D18180: [clang-tidy] Add a check to detect static definitions in anonymous namespace.
Alexander Kornienko via cfe-commits
cfe-commits at lists.llvm.org
Fri Apr 1 07:36:49 PDT 2016
alexfh added a comment.
Sorry for the delay. A few minor issues.
================
Comment at: clang-tidy/readability/StaticDefinitionInAnonymousNamespaceCheck.cpp:46
@@ +45,3 @@
+ auto Diag = diag(Def->getLocation(), "'%0' is a static definition in "
+ "anonymous namespace")
+ << Def->getName();
----------------
I'd expand the message a bit: "; static is redundant here".
================
Comment at: docs/clang-tidy/checks/readability-static-definition-in-anonymous-namespace.rst:8
@@ +7,3 @@
+
+Static is duplicated in this case because anonymous namespace is only visible in
+current transform unit.
----------------
nit: in _the_ current _translation_ unit.
================
Comment at: test/clang-tidy/readability-static-definition-in-anonymous-namespace.cpp:1
@@ +1,2 @@
+// RUN: %check_clang_tidy %s readability-static-definition-in-anonymous-namespace %t
+
----------------
Please add a couple of tests for different cases with macros.
================
Comment at: test/clang-tidy/readability-static-definition-in-anonymous-namespace.cpp:9
@@ +8,3 @@
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: 'c' is a static definition in anonymous namespace [readability-static-definition-in-anonymous-namespace]
+// CHECK-FIXES: int c = 1;
+static const int d = 1;
----------------
This check is not helpful, it will match `static int c = 1;` as well, since it's not anchored to the line start. Should be:
`CHECK-FIXES: {{^}}int c = 1;`. Same in other checks.
Repository:
rL LLVM
http://reviews.llvm.org/D18180
More information about the cfe-commits
mailing list