[PATCH] D27621: [clang-tidy] check to find declarations declaring more than one name

Firat Kasmis via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Dec 29 07:16:02 PST 2016


firolino added inline comments.


================
Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:34
+  // a tag declaration (e.g. struct, class etc.):
+  // class A { } Object1, Object2;  <-- won't be matched
+  Finder->addMatcher(
----------------
firolino wrote:
> firolino wrote:
> > firolino wrote:
> > > aaron.ballman wrote:
> > > > firolino wrote:
> > > > > aaron.ballman wrote:
> > > > > > Why do we not want to match this?
> > > > > If we decide, whether we transform 
> > > > > ```
> > > > > class A { 
> > > > > } Object1, Object2;
> > > > > ``` 
> > > > > to
> > > > > ```
> > > > > class A { 
> > > > > } Object1, 
> > > > > Object2;
> > > > > ``` 
> > > > > or
> > > > > ```
> > > > > class A { 
> > > > > } 
> > > > > Object1, 
> > > > > Object2;
> > > > > ``` 
> > > > > I might consider adding support for it. Moreover, this kind of definition is usually seen globally and I don't know how to handle globals yet. See http://lists.llvm.org/pipermail/cfe-dev/2015-November/046262.html
> > > > I think this should be handled. It can be handled in either of the forms you show, or by saying:
> > > > ```
> > > > A Object1;
> > > > A Object2;
> > > > ```
> > > > If all of these turn out to be a problem, we can still diagnose without providing a fixit.
> > > > 
> > > > As for globals in general, they can be handled in a separate patch once we figure out the declaration grouping.
> > > OK. I will try to split the object definition from the class definition, as you have suggested. Thus, I can kick out the tagDecl-matcher as well. If there is no easy way to do this, it will be reported anyway but without a fixit.
> > > 
> > > Note for me: Update documentation!
> > What about
> > ```
> > struct S {
> > } S1;
> > ```
> > I would like to report this too, since two names are being declared here. `S` and `S1`. What do you think?
> ```
> struct {
> } nn1, nn2;
> ```
> Shall we ignore anonymous definitions?
To be more precise: Warn and provide a fixit for `struct S {} S1`. Only warn for `struct {} nn1, nn2`.


https://reviews.llvm.org/D27621





More information about the cfe-commits mailing list