[PATCH] D37808: [clang-tidy] Add new hicpp-multiway-paths-covered check for missing branches

Roman Lebedev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sat Sep 16 05:33:38 PDT 2017


lebedev.ri added a comment.

In https://reviews.llvm.org/D37808#869810, @Eugene.Zelenko wrote:

> In https://reviews.llvm.org/D37808#869612, @JonasToth wrote:
>
> > In https://reviews.llvm.org/D37808#869602, @Eugene.Zelenko wrote:
> >
> > > I think will be good idea to extend -Wswitch diagnostics.
> >
> >
> > Ok. But it will introduce new warnings to llvm codebase itself. I prepare some example output i found right now.
>
>
> If number of them will not be huge, it'll be worth to fix before extended -Wswitch will be committed.


The other way around, all new warnings must to be fixed first, before committing the new diagnostic itself.

I might agree that the part of this check that is handling `switch()` might indeed be better off extending `-Wswitch` (as several new flags `-Wswitch-missing-default`, `-Wswitch-empty`, `-Wswitch-prefer-if`, or something like that)
But then a much greater thought shall be put into ensuring quality of the new diagnostic.

Oh, and i'd say the `WarnOnMissingElse` should *not* be moved to clang diagnostic. At least right now it will result in many false-positives.
E.g. what would it say about

  void maybefalse(int i) {
    if (i > 0) {
      return;
    } else if (i <= 0) {
      return;
    }


https://reviews.llvm.org/D37808





More information about the cfe-commits mailing list