[PATCH] D30896: [Clang-tidy] add check misc-prefer-switch-for-enums

[Aaron Ballman via Phabricator via cfe-commits]

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