[PATCH] D18180: [clang-tidy] Add a check to detect static definitions in anonymous namespace.

[Alexander Kornienko via cfe-commits]

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