malcolm.parsons 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]
----------------
aaron.ballman wrote:
> 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).
The CERT recommendation does care about declarator operators:
"Multiple, simple variable declarations can be declared on the same line given
that there are no initializations. A simple variable declaration is one that is
not a pointer or array."
|Declaration | CERT |CppCoreGuidelines| Me |
|int i; | GOOD | GOOD | GOOD |
|int i = 1; | GOOD | GOOD | GOOD |
|int *p; | GOOD | GOOD | GOOD |
|int *p, q; | BAD | BAD | BAD |
|int i, j; | GOOD | GOOD | GOOD |
|int i, j = 1; | BAD | GOOD | BAD |
|int i = 1, j = 1; | BAD | GOOD | GOOD |
|int i = 1, j; | BAD | GOOD | BAD |
|int *i, *j; | BAD | BAD | BAD |
|for (int i = 0, j = size; i != j; i++) | GOOD | GOOD | GOOD |
|for (int *p = a, *q = a+size; p != q; p++) | GOOD | BAD | GOOD |
I agree with CERT for most cases.
================
Comment at: test/clang-tidy/cppcoreguidelines-one-name-per-declaration.cpp:16
+ return 0;
+}
----------------
malcolm.parsons wrote:
> aaron.ballman wrote:
> > Please add tests that show the exceptions in the C++ Core Guidelines are
> > properly handled. Also, I'd like to see tests with other named
> > declarations, such as typedefs, template parameter lists, for loop
> > initializers, etc.
> and structured bindings (no warning).
and global variables.
https://reviews.llvm.org/D25024
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
