aaron.ballman added a comment.
In https://reviews.llvm.org/D30896#700324, @jbcoe wrote:
> @aaron.ballman
>
> re: "perhaps the check could be relaxed to only consider cases where you have
> multiple if/else if clauses".
>
> Sadly this does not satisfy my use case where most of the time a single enum
> value is used.
>
> enum class animal_kind { herbivore, carnivore };
> if(k == animal_kind:: carnivore) { ... }
>
>
> I now add `omnivore` and play 'hunt-the-if' knowing anything I miss will be a
> bug.
Hmm, okay, I can see that as a reasonable use case, but I still would like some
hard numbers as to how often this check triggers on a large code base to
understand how chatty this is in practice. Clang-tidy checks can be more chatty
than compiler diagnostics, but only up to a point.
>
>
> ---
>
> re: "There are definitely times when using == or != for a specific value is
> *way* cleaner than using a switch statement".
>
> I **totally** agree. That's why I want this check. People don't remember to
> use `switch` for enums because it's harder and looks uglier.
>
> Perhaps this check should not go in `misc` as it might be better to have it
> opt-in? I know I want it in one code-base I work on.
Misc might be a better place for it, but I'm definitely not keen on adding a
check that is so noisy that it needs to be opt-in unless we've exhausted the
ways to reduce chattiness with heuristics or reasonable options. If it's
opt-in, the user has to remember to opt in to it every time they add a new
enumerator to an enum, otherwise it provides them with no benefit anyway.
That's why it's good to run it over a large code base (I'd use LLVM and Clang
for this; we use a lot of enums which are frequently covered by switch
statements) -- the diagnostics you see may demonstrate some code patterns used
to set that heuristic or suggest good options.
Repository:
rL LLVM
https://reviews.llvm.org/D30896
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
