alexfh added inline comments.
================
Comment at: clang-tidy/misc/DefinitionsInHeadersCheck.cpp:52
@@ +51,3 @@
+
+ // Internal linkage variable and function definitions are allowed:
+ // const int a = 1;
----------------
I'd say "ignored for now" instead of "allowed". We might want to flag them in
the future, but these are used quite frequently, so we don't warn on these now
to keep the number of warnings under control.
Maybe there should be an option for these cases.
================
Comment at: docs/clang-tidy/checks/misc-definitions-in-headers.rst:8
@@ +7,3 @@
+ // Foo.h
+ int a = 1; // error
+ static int b = 1; // ok
----------------
nit: It's a warning, not error. Also, please use proper capitalization and
punctuation:
```
int a = 1; // Warning.
```
or maybe even include the message:
```
int a = 1; // Warning: "variable definition is not allowed in header file".
```
Same holds for `// ok`: They should either be `// OK.` or maybe explain why
certain patterns don't generate a warning.
================
Comment at: test/clang-tidy/misc-definitions-in-headers.hpp:10
@@ +9,3 @@
+class CA {
+ void f1() {} // ok
+ void f2();
----------------
The `// ok` comments add no new information. Either explain why are these OK,
or just remove the comments.
================
Comment at: test/clang-tidy/misc-definitions-in-headers.hpp:27
@@ +26,3 @@
+void CA::f2() { }
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: function definition is not
allowed in header file [misc-definitions-in-headers]
+// CHECK-FIXES: inline void CA::f2() {
----------------
We usually leave each distinct warning message full once and truncate the other
CHECK-MESSAGES patterns to fit to the 80 characters limit (e.g. here I'd remove
the `in header file [misc-definitions-in-headers]` part).
================
Comment at: test/lit.cfg:46
@@ -45,2 +45,3 @@
# suffixes: A list of file extensions to treat as test files.
-config.suffixes = ['.c', '.cpp', '.m', '.mm', '.cu', '.ll', '.cl', '.s',
'.modularize', '.module-map-checker']
+config.suffixes = ['.c', '.cpp', '.m', '.mm', '.cu', '.ll', '.cl', '.s',
+ '.modularize', '.module-map-checker', '.hpp']
----------------
Please place the new extension after '.cpp' - it seems to be most relevant
there.
http://reviews.llvm.org/D15710
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
