aaron.ballman added a reviewer: aaron.ballman.
================
Comment at: docs/clang-tidy/checks/misc-definitions-in-headers.rst:18
@@ +17,3 @@
+ int c = 2; // ok
+ }
+
----------------
hokein wrote:
> 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
They do cause ODR violations. In addition to the CERT link above, see
http://stackoverflow.com/questions/23652156/how-would-use-of-unnamed-namespaces-in-headers-cause-odr-violations
for further information.
================
Comment at: unittests/clang-tidy/MiscModuleTest.cpp:38
@@ -36,1 +37,3 @@
+class DefinitionsInHeadersCheckTest : public ::testing::Test {
+protected:
----------------
hokein wrote:
> 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.
I was arguing for consistency as well -- clang-tidy tests are run via lit
instead of unit tests unless there's a reason to do otherwise. It sounds like
there's no reason for these to be unit tests.
http://reviews.llvm.org/D15710
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
