[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

This revision was automatically updated to reflect the committed changes. Closed by commit rL345735: [clang-tidy] new check readability-isolate-declaration (authored by JonasToth, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D51949

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

JonasToth updated this revision to Diff 171947. JonasToth added a comment. - Merge branch 'master' into experiment_isolate_decl - remove unused matcher Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51949 Files: clang-tidy/readability/CMakeLists.txt

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Comment at: docs/clang-tidy/checks/readability-isolate-declaration.rst:45 + +Global variables and member variables are excluded. +

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

JonasToth marked an inline comment as done. JonasToth added inline comments. Comment at: clang-tidy/readability/IsolateDeclarationCheck.cpp:119 + const LangOptions ) { + std::size_t DeclCount = std::distance(DS->decl_begin(), DS->decl_end()); + if (DeclCount < 2)

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

JonasToth added a comment. This is usually not done in the LLVM codebase, as its not a pointer/reference. Am 29.10.18 um 20:29 schrieb Alexander Zaitsev via Phabricator: > ZaMaZaN4iK added inline comments. > > > Comment at:

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

ZaMaZaN4iK added inline comments. Comment at: clang-tidy/readability/IsolateDeclarationCheck.cpp:119 + const LangOptions ) { + std::size_t DeclCount = std::distance(DS->decl_begin(), DS->decl_end()); + if (DeclCount < 2) Mark `DeclCount` as const

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

JonasToth updated this revision to Diff 171538. JonasToth added a comment. - Merge branch 'master' into experiment_isolate_decl - adjust doc slighty to comment Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51949 Files: clang-tidy/readability/CMakeLists.txt

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

Eugene.Zelenko added inline comments. Comment at: docs/ReleaseNotes.rst:142 + + Diagnoses local variable declarations declaring more than one variable and + tries to refactor the code to one statement per declaration. May be Finds or Detects like other checks?

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

JonasToth updated this revision to Diff 171361. JonasToth added a comment. - [Fix] wrong condition in matcher coming from incorrect code change Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51949 Files: clang-tidy/readability/CMakeLists.txt

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

JonasToth updated this revision to Diff 171346. JonasToth marked 16 inline comments as done. JonasToth added a comment. - address review comments Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51949 Files: clang-tidy/readability/CMakeLists.txt

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

JonasToth added inline comments. Comment at: clang-tidy/readability/IsolateDeclarationCheck.cpp:52 + const LangOptions ) { + assert(Indirections >= 0 && "Indirections must be non-negative"); + if (Indirections == 0)

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

aaron.ballman added a comment. Mostly minor nits that clean up wording and comments. Comment at: clang-tidy/readability/IsolateDeclarationCheck.cpp:52 + const LangOptions ) { + assert(Indirections >= 0 && "Indirections must be

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

lebedev.ri added inline comments. Comment at: test/clang-tidy/readability-isolate-declaration.cpp:233 + int member1, member2; + // TODO: Transform FieldDecl's as well +}; Comment is misleading. Transform == fixit, at least for me. But they are not even

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

JonasToth added a comment. ping :) Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51949 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

JonasToth updated this revision to Diff 169389. JonasToth added a comment. - fix headline in doc Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51949 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/IsolateDeclarationCheck.cpp

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

JonasToth added a comment. > Unfortunately, I won't be able to review the code in the upcoming few weeks > as I caught a cold and I'm trying to get better before the LLVM Meeting, so > if there is anyone else interested in reviewing proposed changes please feel > free to jump in. Thank you

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

kbobyrev added a comment. In https://reviews.llvm.org/D51949#1257430, @JonasToth wrote: > @kbobyrev is it ok for you if I stick with the `Optional<>` style in the > check? Yes, I think it's fine. Thank you very much for working on the patch, I am indeed very excited about this check.

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

JonasToth updated this revision to Diff 168598. JonasToth added a comment. - address review comments, simplifying code Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51949 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/IsolateDeclarationCheck.cpp

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

JonasToth marked 2 inline comments as done. JonasToth added a comment. @kbobyrev is it ok for you if I stick with the `Optional<>` style in the check? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51949 ___ cfe-commits mailing

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

kbobyrev added inline comments. Comment at: clang-tidy/readability/IsolateDeclarationCheck.cpp:24 +AST_MATCHER(DeclStmt, onlyDeclaresVariables) { + return std::all_of(Node.decl_begin(), Node.decl_end(), + [](Decl *D) { return isa(D); }); It

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

JonasToth updated this revision to Diff 168091. JonasToth marked 2 inline comments as done. JonasToth added a comment. - remove spurious local variable Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51949 Files: clang-tidy/readability/CMakeLists.txt

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

JonasToth updated this revision to Diff 167696. JonasToth added a comment. - remove spurious change in fixithintutils Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51949 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/IsolateDeclarationCheck.cpp

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

JonasToth updated this revision to Diff 167591. JonasToth added a comment. - remove changes from lexerutils refactoring in other checks Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51949 Files: clang-tidy/readability/CMakeLists.txt

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

JonasToth updated this revision to Diff 167589. JonasToth added a comment. - fix spurious changes Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51949 Files: clang-tidy/bugprone/ArgumentCommentCheck.cpp clang-tidy/bugprone/SuspiciousSemicolonCheck.cpp

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

JonasToth updated this revision to Diff 167588. JonasToth marked 2 inline comments as done. JonasToth added a comment. - address review comments Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51949 Files: clang-tidy/bugprone/ArgumentCommentCheck.cpp

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

JonasToth marked 15 inline comments as done. JonasToth added inline comments. Comment at: clang-tidy/readability/IsolateDeclarationCheck.cpp:52 + const LangOptions ) { + assert(Indirections >= 0 && "Indirections must be

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

JonasToth updated this revision to Diff 167523. JonasToth added a comment. - simplify matcher slighty, as initStmt in if/switch don't have a compundstmt as parent, add suggested tests Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51949 Files:

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

Eugene.Zelenko added a comment. How about creating CERT alias? Comment at: clang-tidy/readability/IsolateDeclarationCheck.cpp:58 + // number of transformations. + while (Indirections--) { +Start = findPreviousAnyTokenKind(Start, SM, LangOpts, tok::star, tok::amp);

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

aaron.ballman added a comment. Can you add some tests for cases like: // C code void (*signal(int sig, void (*func)(int)))(int); // One decl int i = sizeof(struct S { int i;}); // One decl int j = sizeof(struct T { int i;}), k; // Two decls void g(struct U { int i; } s); // One decl

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

JonasToth added inline comments. Comment at: docs/clang-tidy/checks/readability-isolate-declaration.rst:33 + for (int Begin = 0, End = 100; Begin < End; ++Begin); + it (int Begin = 42, Result = some_function(Begin); Begin == Result); + typo mention `if () int

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

JonasToth added a comment. llvm and clang look fine as well. No miscompilation after fixing, tests run as well. (i do exclude all targets except x86 for build-speed) F7327680: cfe_isolated_decl.patch F7327681: llvm_isolated_decl.patch

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

JonasToth updated this revision to Diff 167491. JonasToth added a comment. Herald added subscribers: jsji, kbarton, nemanjai. - fix paren issue, introduced non-local changes i will extract and post in a separate patch - remove debug output for paren search Repository: rCTE Clang Tools Extra

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

JonasToth added a comment. Blender looks better now, code compiles after full transformation F7327473: blender_isolated_decls.patch Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51949 ___

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

JonasToth added a comment. LLVM works, Blender doesn't, the offending piece is `float (((*f_ptr2)))[42], *f_ptr3, f_value2 = 42.f;` (parens around the pointer). I am trying to fix that. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51949

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

lebedev.ri added a comment. Thank you for working on this! Tried on a pet project , Two hits: $ ninja [16/376] Checking validity of cameras.xml /home/lebedevri/rawspeed/data/cameras.xml validates [100/376] Building CXX object

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

JonasToth updated this revision to Diff 167445. JonasToth added a comment. - add tests for auto and decltype - rename check to 'readability-isolate-declaration' Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51949 Files: clang-tidy/readability/CMakeLists.txt

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

JonasToth added inline comments. Comment at: test/clang-tidy/readability-isolate-decl.cpp:184 + + int member1, member2; + // TODO: Transform FieldDecl's as well @lebedev.ri :P Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51949

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

JonasToth added inline comments. Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:343 + auto Diag = + diag(WholeDecl->getBeginLoc(), "this statement declares %0 variables") + << static_cast( lebedev.ri wrote: > JonasToth wrote: > >

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

lebedev.ri added a comment. In https://reviews.llvm.org/D51949#1248861, @JonasToth wrote: > In https://reviews.llvm.org/D51949#1248850, @lebedev.ri wrote: > > > I have looked through tests, and it is possible that i just missed it, but > > does it test/handle the cases like: > > > > struct S

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

JonasToth added a comment. In https://reviews.llvm.org/D51949#1248850, @lebedev.ri wrote: > I have looked through tests, and it is possible that i just missed it, but > does it test/handle the cases like: > > struct S { > int X, Y, Z; // <- should be diagnosed. > } > Unfortunatly not

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

lebedev.ri added a comment. I have looked through tests, and it is possible that i just missed it, but does it test/handle the cases like: struct S { int X, Y, Z; // <- should be diagnosed. } Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:343 + auto Diag =