[PATCH] D25024: [clang-tidy] Add check for detecting declarations with multiple names

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.


================
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:
> omtcyfz wrote:
> > 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.
> "mistakes related to the C/C++ grammar" only occur when declarator operators are involved. e.g. in `int* p, q` a reader might incorrectly think that q was a pointer.
> 
> I see reasons not to warn about cases like
> `for (auto i = c.begin(), e = someExpensiveFn(); i != e; i++)`
> `for (int i = 0, j = someExpensiveFn(); i < j; i++);`
> because the alternatives increase variable scope, or for
> `int x = 42, y = 43;`
> because it's not difficult to read.
> 
> As we disagree on this, can it be made configurable?
We usually try to ensure that the check matches the behavior required by the normative wording of coding rules we follow. Based on that, the C++ core guideline rule only cares about declarator operators while the CERT recommendation kind of does not care about them. If you think the C++ core guideline enforcement is wrong, you can file a bug against that project to see if the editors agree, but I think the check should match the documented behavior from the guidelines. FWIW, I'm happy to work on the CERT semantics if you don't want to deal with those beyond what you've already done (or you can tackle the semantic differences if you want).


https://reviews.llvm.org/D25024