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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to