omtcyfz added inline comments.

================
Comment at: clang-tidy/cppcoreguidelines/OneNamePerDeclarationCheck.cpp:38
+  diag(MultipleNameDeclaration->getStartLoc(),
+       "Do not declare multiple names per declaration");
+}
----------------
aaron.ballman wrote:
> Diagnostics do not start with a capital letter. Also, this diagnostic is not 
> really accurate. Consider `void f(int x, int y);` -- that's a single 
> declaration, but it declares multiple names. Perhaps: `do not declare more 
> than one variable per declaration` since this should really be focusing on 
> variable declarations rather than parameters, template parameter lists, etc
> Diagnostics do not start with a capital letter.

Clang SA diags do, actually. Though I can totally see the reason: consistency 
is important since it's clang-tidy check.

> Consider void f(int x, int y); -- that's a single declaration, but it 
> declares multiple names. Perhaps: do not declare more than one variable per 
> declaration since this should really be focusing on variable declarations 
> rather than parameters, template parameter lists, etc

Fixed, thank you for the note!


================
Comment at: test/clang-tidy/cppcoreguidelines-one-name-per-declaration.cpp:8
+  {
+    int x = 42, y = 43;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: Do not declare multiple names 
per declaration [cppcoreguidelines-one-name-per-declaration]
----------------
malcolm.parsons wrote:
> The guideline says "Flag non-function arguments with multiple declarators 
> involving declarator operators (e.g., int* p, q;)".
> 
> There are no declarator operators in this test, so there should be no warning.
The guideline says

> Reason: One-declaration-per line increases readability and avoids mistakes 
> related to the C/C++ grammar. It also leaves room for a more descriptive 
> end-of-line comment.

> Exception: a function declaration can contain several function argument 
> declarations.

I'm not sure why what you copied is written in "Enforcement" section, but I do 
not think that is how it should be handled. I am concerned not only about that 
specific case and I see no reason to cut off cases already presented in this 
test.


https://reviews.llvm.org/D25024



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

Reply via email to