[PATCH] D23130: [Clang-tidy] Add a check for definitions in the global namespace.

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

Could you run the check on LLVM and post a summary of results?


================
Comment at: clang-tidy/google/GlobalNamesCheck.cpp:90
@@ +89,3 @@
+    // extern "C" globals need to be in the global namespace.
+    if (VDecl->isExternC())
+      return;
----------------
Is this already filtered-out by the matcher?

================
Comment at: clang-tidy/google/GlobalNamesCheck.cpp:114
@@ +113,3 @@
+  // definitions into a global namespace or make them static with a FixIt.
+  diag(D->getLocation(), "%0 declared in the global namespace") << D;
+}
----------------
The warning explains, what's wrong, but doesn't tell why. We don't need the level of details of the documentation here, but maybe expand the message a bit to cover some of the possible effects (e.g. "this can cause ODR violations" or something like this)?

================
Comment at: docs/ReleaseNotes.rst:62
@@ -61,1 +61,3 @@
 
+- `google-global-names
+  <http://clang.llvm.org/extra/clang-tidy/checks/google-global-names.html>`_ check
----------------
Please mention the old check name as well.

================
Comment at: docs/clang-tidy/checks/google-global-names.rst:4
@@ -4,2 +3,3 @@
+google-global-names
 ==============================
 
----------------
Cut extra =s.


https://reviews.llvm.org/D23130