[PATCH] D90180: [clang-tidy] find/fix unneeded semicolon after switch

2020-11-07 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D90180#2379803 , @nickdesaulniers wrote: > In D90180#2375878 , @aaron.ballman > wrote: > >> In D90180#2374839 , >> @nickdesaulniers wrote

[PATCH] D90180: [clang-tidy] find/fix unneeded semicolon after switch

2020-11-06 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added a comment. In D90180#2375878 , @aaron.ballman wrote: > In D90180#2374839 , @nickdesaulniers > wrote: > >> In D90180#2357247 , @aaron.ballman >> wrote

[PATCH] D90180: [clang-tidy] find/fix unneeded semicolon after switch

2020-11-05 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D90180#2374839 , @nickdesaulniers wrote: > In D90180#2357247 , @aaron.ballman > wrote: > >> When you pass `-fix` to clang-tidy, it will apply fix-its from the compiler >> as well

[PATCH] D90180: [clang-tidy] find/fix unneeded semicolon after switch

2020-11-04 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added a comment. In D90180#2357247 , @aaron.ballman wrote: > When you pass `-fix` to clang-tidy, it will apply fix-its from the compiler > as well. See https://godbolt.org/z/7vzM4b and try adding a semicolon to the > end of the `switch`

[PATCH] D90180: [clang-tidy] find/fix unneeded semicolon after switch

2020-11-02 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D90180#2366418 , @trixirt wrote: > I am trying to address problems treewide so around 100 changes per fix. > Doing that requires consensus that the fix is generally ok for every > subsystem in the code base. > So while cl

[PATCH] D90180: [clang-tidy] find/fix unneeded semicolon after switch

2020-10-31 Thread Tom Rix via Phabricator via cfe-commits
trixirt marked 2 inline comments as done. trixirt added a comment. I am trying to address problems treewide so around 100 changes per fix. Doing that requires consensus that the fix is generally ok for every subsystem in the code base. So while clang will fix switch () {} ; it will also fix for

[PATCH] D90180: [clang-tidy] find/fix unneeded semicolon after switch

2020-10-27 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D90180#2357178 , @trixirt wrote: > Discussion of change on LKML > https://lkml.org/lkml/2020/10/27/2538 > > On why the existing clang fixit is not practical. > tl;dr > 10,000 changes to look for a treewide fix > The fixit

[PATCH] D90180: [clang-tidy] find/fix unneeded semicolon after switch

2020-10-27 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. In D90180#2357178 , @trixirt wrote: > Discussion of change on LKML > https://lkml.org/lkml/2020/10/27/2538 > > On why the existing clang fixit is not practical. > tl;dr > 10,000 changes to look for a treewide fix > The fixi

[PATCH] D90180: [clang-tidy] find/fix unneeded semicolon after switch

2020-10-27 Thread Tom Rix via Phabricator via cfe-commits
trixirt added a comment. Discussion of change on LKML https://lkml.org/lkml/2020/10/27/2538 On why the existing clang fixit is not practical. tl;dr 10,000 changes to look for a treewide fix The fixit has whitespace issue Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:

[PATCH] D90180: [clang-tidy] find/fix unneeded semicolon after switch

2020-10-27 Thread Tom Rix via Phabricator via cfe-commits
trixirt updated this revision to Diff 301059. trixirt added a comment. fix precheckin lint issues with auto vs const auto Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90180/new/ https://reviews.llvm.org/D90180 Files: clang-tools-extra/clang-tid

[PATCH] D90180: [clang-tidy] find/fix unneeded semicolon after switch

2020-10-27 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D90180#2354931 , @njames93 wrote: > Is this not already handled by `-Wextra-semi`. If it isn't I feel like it > should be handled there rather than in clang-tidy Strong +1 -- this is already handled by `-Wextra-semi-stmt

[PATCH] D90180: [clang-tidy] find/fix unneeded semicolon after switch

2020-10-27 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. I think CLang should do this job, since it has two related warnings already. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90180/new/ https://reviews.llvm.org/D90180 ___ c

[PATCH] D90180: [clang-tidy] find/fix unneeded semicolon after switch

2020-10-27 Thread Tom Rix via Phabricator via cfe-commits
trixirt added a comment. The 10,000 problems are for all the extra semicolon problems. Those after switch are a small subset, chosen because is ok to automagically fix them. The checker is in the linuxkernel/ because it is being used to fix the linux kernel now. And to enforce it stays fixed.

[PATCH] D90180: [clang-tidy] find/fix unneeded semicolon after switch

2020-10-27 Thread Tom Rix via Phabricator via cfe-commits
trixirt updated this revision to Diff 300965. trixirt added a comment. precheckin lint fixes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90180/new/ https://reviews.llvm.org/D90180 Files: clang-tools-extra/clang-tidy/linuxkernel/CMakeLists.txt

[PATCH] D90180: [clang-tidy] find/fix unneeded semicolon after switch

2020-10-26 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. Taking a step back, clang-tidy checks are supposed to enforce guidelines for the specific module they live in. If there are 10'000 occurrences of a semi directly after a switch closing brace in the linux kernel code base it could be argued that its a style guideline of

[PATCH] D90180: [clang-tidy] find/fix unneeded semicolon after switch

2020-10-26 Thread Tom Rix via Phabricator via cfe-commits
trixirt added a comment. Yes, this automates the fixit to something precise. There are about 10,000 problems in the kernel with -Wextra-semi, all of those fixes would be too many and diverse to practically submit. Also the -Wextra-semi fix removes just the semi and so it's fix will fail the kern

[PATCH] D90180: [clang-tidy] find/fix unneeded semicolon after switch

2020-10-26 Thread Tom Rix via Phabricator via cfe-commits
trixirt updated this revision to Diff 300830. trixirt added a comment. Comment on the matcher Comment explaining need to look for empty macro Add test for normal switch Add test for switch with empty macro Fix formatting Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://

[PATCH] D90180: [clang-tidy] find/fix unneeded semicolon after switch

2020-10-26 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added a comment. In D90180#2354931 , @njames93 wrote: > Is this not already handled by `-Wextra-semi`. If it isn't I feel like it > should be handled there rather than in clang-tidy That was my initial thought too, but I think the value o

[PATCH] D90180: [clang-tidy] find/fix unneeded semicolon after switch

2020-10-26 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. Is this not already handled by `-Wextra-semi`. If it isn't I feel like it should be handled there rather than in clang-tidy Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90180/new/ https://reviews.llvm.org/D90180 ___

[PATCH] D90180: [clang-tidy] find/fix unneeded semicolon after switch

2020-10-26 Thread Tom Roeder via Phabricator via cfe-commits
tmroeder added inline comments. Comment at: clang-tools-extra/clang-tidy/linuxkernel/LinuxKernelTidyModule.cpp:25 "linuxkernel-must-check-errs"); +CheckFactories.registerCheck( +"linuxkernel-switch-semi"); Please fix the linter error, here a

[PATCH] D90180: [clang-tidy] find/fix unneeded semicolon after switch

2020-10-26 Thread Tom Rix via Phabricator via cfe-commits
trixirt created this revision. trixirt added reviewers: alexfh, nickdesaulniers. trixirt added projects: clang, clang-tools-extra. Herald added subscribers: cfe-commits, xazax.hun, mgorny. trixirt requested review of this revision. Cleaning up -Wextra-semi-stmt in the linux kernel shows a high inc