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

firolino added a comment. In https://reviews.llvm.org/D27621#1248849, @JonasToth wrote: > Hi @firolino, > I implemented a less general version of you check > https://reviews.llvm.org/D51949 as I wanted to achieve a slightly different > goal (and this revision seems to be abonded). My aim is

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

JonasToth added a comment. Hi @firolino, I implemented a less general version of you check https://reviews.llvm.org/D51949 as I wanted to achieve a slightly different goal (and this revision seems to be abonded). My aim is to have general transformation capabilities to separate declarations

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

firolino marked an inline comment as done. firolino added a comment. In https://reviews.llvm.org/D27621#1061562, @lebedev.ri wrote: > @firolino do you plan on finishing this? Sometimes the universe is really funny! I have just re-started working on it today. https://reviews.llvm.org/D27621

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

JonasToth added a comment. Still work on this one? https://reviews.llvm.org/D27621 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

lebedev.ri added a comment. @firolino do you plan on finishing this? https://reviews.llvm.org/D27621 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

alexfh added inline comments. Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:130 +if (isa(*(FirstVarIt + 1))) + return "typedef " + TypeString; + Should we suggest `using x = ...;` in C++11 code? Comment at:

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

aaron.ballman added inline comments. Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:37-38 + const auto *DeclStatement = Result.Nodes.getNodeAs("declstmt"); + if (!DeclStatement) +return; + Is there a case where this could happen? I would

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

firolino marked 7 inline comments as done. firolino added a comment. - nothing special, just went through some open comments and marked them as Done. Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:246 + +static std::string getCurrentLineIndent(SourceLocation

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

firolino added a comment. @aaron.ballman I am going to implement `CppCore`, `Cert` and `Everything` and test it on some projects and provide the results next week, to find out which one to set `Default`. https://reviews.llvm.org/D27621 ___

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

firolino added inline comments. Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.h:25 +class OneNamePerDeclarationCheck : public ClangTidyCheck { +private: + std::string getUserWrittenType(const DeclStmt *DeclStmt, SourceManager ); firolino wrote:

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

firolino added inline comments. Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:153 + const SourceRange FVLoc(DeclStmt->getLocStart(), Location); + std::string UserWrittenType = + Lexer::getSourceText(CharSourceRange::getCharRange(FVLoc), SM,

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

alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. A few more comments. Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:153 + const SourceRange FVLoc(DeclStmt->getLocStart(), Location); +

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

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( aaron.ballman wrote: >

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

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: >

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

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( aaron.ballman wrote: >

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

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: >

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

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( aaron.ballman wrote: >

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

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: >

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

firolino updated the summary for this revision. firolino updated this revision to Diff 82855. firolino marked an inline comment as done. firolino added a comment. - applied suggestions from Aaron - added support and test cases for TagDecl e.g. struct S {} S1, S2;

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

firolino marked 5 inline comments as done. 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(

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

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: >

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

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

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

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

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

firolino marked 3 inline comments as done. 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(

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

firolino marked 27 inline comments as done. 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(

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

aaron.ballman added inline comments. Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:22 + +const internal::VariadicDynCastAllOfMatcher tagDecl; + firolino wrote: > aaron.ballman wrote: > > We may want to consider adding this to

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

aaron.ballman added inline comments. Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:22 + +const internal::VariadicDynCastAllOfMatcher tagDecl; + We may want to consider adding this to ASTMatchers.h at some point (can be done in

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

firolino updated this revision to Diff 81964. firolino added a comment. Added support for new test cases: int S::*mdpa1[2] = {::a, ::a}, var1 = 0; // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: declaration statement can be split // CHECK-FIXES: {{^}}int S::*mdpa1[2] = {::a, ::a}; //

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

firolino updated this revision to Diff 81731. firolino added a comment. further cleanup in getUserWrittenType https://reviews.llvm.org/D27621 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/OneNamePerDeclarationCheck.cpp

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

firolino updated this revision to Diff 81730. firolino added a comment. little cleanup in getUserWrittenType https://reviews.llvm.org/D27621 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/OneNamePerDeclarationCheck.cpp

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

firolino updated this revision to Diff 81729. firolino added a comment. small improvement https://reviews.llvm.org/D27621 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/OneNamePerDeclarationCheck.cpp clang-tidy/readability/OneNamePerDeclarationCheck.h

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

firolino updated this revision to Diff 81605. firolino added a comment. Added fix and test case for `const int * const`. https://reviews.llvm.org/D27621 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/OneNamePerDeclarationCheck.cpp

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

firolino added a comment. Unfortunately, I am not able to make getUserWrittenType easier right now. TypeLoc does not give me the desired range. See also: - http://lists.llvm.org/pipermail/cfe-dev/2013-March/028467.html - http://lists.llvm.org/pipermail/cfe-dev/2010-April/008664.html -

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

firolino updated this revision to Diff 81591. firolino added a comment. - small improvements - re-added lost handling for: bool defPre = false, #ifdef IS_ENABLED defTest = false; #else defTest = true; #endif - added support for new test cases: int /* :: */ S::*pp2 =

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

Eugene.Zelenko added a comment. I think will be good idea to extend check for class members. Comment at: test/clang-tidy/readability-one-name-per-declaration-modern.cpp:13 + public: +vector() {} +vector(initializer_list init) {} Please use

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

firolino marked an inline comment as done. firolino added inline comments. Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:154 + if (const auto *MemberPointerT = Type->getAs()) { +auto Pos = UserWrittenType.find("::"); +if (Pos != std::string::npos) {

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

firolino added a comment. In https://reviews.llvm.org/D27621#620464, @rsmith wrote: > Please add a test to make sure this does not fire on C++17 decomposition > declarations: > > void f() { > struct X { int a, b, c; }; > auto [a, b, c] = X(); > } Done. Thanks!

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

malcolm.parsons added inline comments. Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:154 + if (const auto *MemberPointerT = Type->getAs()) { +auto Pos = UserWrittenType.find("::"); +if (Pos != std::string::npos) { // might be hidden behind typedef

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

rsmith added a comment. Please add a test to make sure this does not fire on C++17 decomposition declarations: void f() { struct X { int a, b, c; }; auto [a, b, c] = X(); } https://reviews.llvm.org/D27621 ___ cfe-commits mailing list

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

firolino updated this revision to Diff 81154. firolino marked an inline comment as done. firolino added a comment. Small improvments: - SourceRange -> SourceLocation (malcolms last comment) - added a few consts https://reviews.llvm.org/D27621 Files: clang-tidy/readability/CMakeLists.txt

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

firolino marked 2 inline comments as done. firolino added inline comments. Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:86 + + Diag << FixItHint::CreateReplacement(CommaRange, ";") + << FixItHint::CreateReplacement(AfterCommaToVarNameRange,

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

firolino marked an inline comment as done. firolino added inline comments. Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:87 + Diag << FixItHint::CreateReplacement(CommaRange, ";") + << FixItHint::CreateReplacement(AfterCommaToVarNameRange, +

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

malcolm.parsons added inline comments. Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:86 + + Diag << FixItHint::CreateReplacement(CommaRange, ";") + << FixItHint::CreateReplacement(AfterCommaToVarNameRange, A `SourceLocation`

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

firolino updated this revision to Diff 81101. firolino marked an inline comment as done. firolino added a comment. Simplified fix-it hint generation. https://reviews.llvm.org/D27621 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/OneNamePerDeclarationCheck.cpp

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

firolino marked 2 inline comments as done. firolino added inline comments. Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:133 +VariableLocation.setBegin(tidy::utils::lexer::findLocationAfterToken( +VariableLocation.getEnd(), Tokens,

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

arphaman added inline comments. Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:133 +VariableLocation.setBegin(tidy::utils::lexer::findLocationAfterToken( +VariableLocation.getEnd(), Tokens, *Result.Context)); + } If you use

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

firolino added a comment. In https://reviews.llvm.org/D27621#619649, @malcolm.parsons wrote: > The fixit construction looks overly complicated. Yes, you were right. I have changed a couple of things. It looks better now. Still working on a better getUserWrittenType.

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

firolino added a comment. In https://reviews.llvm.org/D27621#619649, @malcolm.parsons wrote: > The fixit construction looks overly complicated. > All you need to do is change a `,` to a `;` and insert a copy of the type: > > << FixItHint::CreateReplacement(CommaRange, ";") > <<

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

malcolm.parsons added a comment. The fixit construction looks overly complicated. All you need to do is change a `,` to a `;` and insert a copy of the type: << FixItHint::CreateReplacement(CommaRange, ";") << FixItHint::CreateInsertionFromRange(VarLocation, TypeRange) and insert some

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

firolino updated this revision to Diff 81039. firolino added a comment. A few namespace cleanups - Removed explicit llvm::'s, since the used names are already introduced into the clang namespace by llvm.h https://reviews.llvm.org/D27621 Files: clang-tidy/readability/CMakeLists.txt

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

firolino added inline comments. Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:246 + +static std::string getCurrentLineIndent(SourceLocation Loc, +const SourceManager ) { malcolm.parsons wrote: > Should

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

firolino updated this revision to Diff 81029. firolino marked an inline comment as done. firolino added a comment. Little typo fix. https://reviews.llvm.org/D27621 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/OneNamePerDeclarationCheck.cpp

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

firolino updated this revision to Diff 81023. firolino added a comment. typdef -> typedef https://reviews.llvm.org/D27621 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/OneNamePerDeclarationCheck.cpp clang-tidy/readability/OneNamePerDeclarationCheck.h

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

firolino updated this revision to Diff 81022. firolino marked 7 inline comments as done. firolino added a comment. Applied suggestions from malcolm. https://reviews.llvm.org/D27621 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/OneNamePerDeclarationCheck.cpp

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

malcolm.parsons added inline comments. Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:48 + // Single declarations and macros will be ignored + if (DeclStmt->isSingleDecl() || DeclStmt->getLocStart().isMacroID()) +return; Can you reject

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

firolino updated this revision to Diff 81001. firolino added a comment. Added test case with struct keyword: struct StructOne cs1, cs2( 42 ); https://reviews.llvm.org/D27621 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/OneNamePerDeclarationCheck.cpp

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

firolino added a comment. @Eugene.Zelenko No modernize-* check was reported. I applied //most// of the readability-* checks. For example, I did not put braces around: if (DeclStmt == nullptr) return; if (DeclStmt->isSingleDecl() || DeclStmt->getLocStart().isMacroID()) return;

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

firolino updated this revision to Diff 81000. firolino marked 9 inline comments as done. firolino added a comment. Applied readability-* checks. https://reviews.llvm.org/D27621 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/OneNamePerDeclarationCheck.cpp

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

firolino added a comment. In https://reviews.llvm.org/D27621#618379, @JDevlieghere wrote: > Some small stuff I noticed while reading through the code, I didn't check it > in much detail though. Thanks, fixed it. https://reviews.llvm.org/D27621

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

firolino updated this revision to Diff 80916. firolino added a comment. Addressed comment from JDevlieghere https://reviews.llvm.org/D27621 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/OneNamePerDeclarationCheck.cpp

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

Eugene.Zelenko added inline comments. Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:47 +// Single declarations and macros will be ignored +if (DeclStmt->isSingleDecl() == false && +DeclStmt->getLocStart().isMacroID() == false) {

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

firolino removed rL LLVM as the repository for this revision. firolino updated this revision to Diff 80915. firolino added a comment. Updated docs/ReleaseNotes.txt https://reviews.llvm.org/D27621 Files: clang-tidy/readability/CMakeLists.txt

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

JDevlieghere added a comment. Some small stuff I noticed while reading through the code, I didn't check it in much detail though. Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:43 +void OneNamePerDeclarationCheck::check(const MatchFinder::MatchResult ) { +

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

firolino added a comment. I have agreed with Kirill to continue this. See http://lists.llvm.org/pipermail/cfe-dev/2016-October/051270.html I would suggest to close https://reviews.llvm.org/D25024. Repository: rL LLVM https://reviews.llvm.org/D27621

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

Eugene.Zelenko added a comment. Please mention this check in docs/ReleaseNotes.rst (in alphabetical order). This is duplicate of https://reviews.llvm.org/D25024. Will be goo idea to combine code in one. Please note that there are multiple coding guidelines which suggest this style.

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

firolino added a comment. I have tested it on the whole llvm/clang/extra source code without any problems. https://reviews.llvm.org/D27621 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

firolino created this revision. firolino added reviewers: alexfh, aaron.ballman, klimek, malcolm.parsons. firolino added a subscriber: cfe-commits. Herald added subscribers: JDevlieghere, mgorny. This check can be used to find declarations, which declare more than one name. It helps improving