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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D90180#2366418 <https://reviews.llvm.org/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 clang will fix
> switch () {} ; 
> it will also fix
> for () {
>
>   // tbd : add something here
>   ;
>
> }
> consensus needs to be found on as narrow a fix as possible.
> so there will be a specialized fixer for each problem consensus is reached on.

Interesting approach, thank you for sharing it.

> As for leaving whitespace fixing as an exercise for the user.
> No change will be accepted with a whitespace problem.
> When there are 100 fixes, there are 100 files to open, find the line, fix the line.
> This is slow and error prone.

It's also not really required. Apply the automated fixes, run the results through clang-format-diff.py (http://clang.llvm.org/docs/ClangFormat.html#script-for-patch-reformatting), and commit the results. If this is error-prone, we've got a pretty big issue (IMO).

> Another slow part is manually commit and submitting the fixes and doing a review cycle.
> But if your fixer can automagically do everything you can  hook it into a c/i and and
> it will keep the codebase free of its set of fixers while we all do something more 
> interesting.
>
> i am working on the kernel build to do this now.
> It would be helpful if this and future fixers could live in clang-tidy's linuxkernel/
> which is already part of the kernel build so i don't have to reinvent how to find them.

Given that we already have a module for Linux kernel-specific clang-tidy checks for this to live in, I can hold my nose on the fact that it's poorly replicating the frontend's work because other users won't be surprised by it (in theory). So I'll retract my disapproval; you have compelling rationale for why you want to do it this way.

Since the plan is to produce multiple distinct checks for each extraneous semicolon situation, then I think this check should probably be renamed to something more generic and given options to control which distinct scenarios to check for. This will reduce the amount of compilation overhead for the clang-tidy project over time by not needing to introduce a new check (with new boilerplate) for each scenario but should hopefully still allow you to do what you need (with config files perhaps) in your CI. WDYT?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90180/new/

https://reviews.llvm.org/D90180