aaron.ballman 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: > > > 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`. > 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? I don't think that this should be diagnosed. For one, this is a relatively common pattern (arguably more common than `struct S { } S1, S2;`), but also, it names two very distinct entities (a type and a variable). > ``` > struct { > } nn1, nn2; >``` > Shall we ignore anonymous definitions? Yes, we basically have to. https://reviews.llvm.org/D27621 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits