hokein marked 6 inline comments as done.

================
Comment at: docs/clang-tidy/checks/misc-definitions-in-headers.rst:18
@@ +17,3 @@
+     int c  = 2; // ok
+   }
+
----------------
Yeah, it's not a good practice to define an an unnamed namespace in header file 
(Google cpp code style also suggests that). 

But I think internal linkage variables are ok here, the variable only be 
visible in current translation unit which will not cause ODR violation. And 
It's common to define a `static const` constant in C++ header file. 

BTW, the sample code in the link can be compiled in g++ v4.8

================
Comment at: unittests/clang-tidy/MiscModuleTest.cpp:38
@@ -36,1 +37,3 @@
 
+class DefinitionsInHeadersCheckTest : public ::testing::Test {
+protected:
----------------
I'm not sure about this. The reason I put the test in unittest is that I see 
the other header checks(`GlobalNamesInHeadersCheck`) are also in the unittest. 
It would be better to keep the consistency.


http://reviews.llvm.org/D15710



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

Reply via email to