[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#701642, @jbcoe wrote:

> I've run the check on clang. It's noisy (690 warnings). The (few) cases I've manually inspected look like firm candidates for replacing with switches. Not my call though.
>
> F3145418: reduced-clang-tidy-switch-checks <https://reviews.llvm.org/F3145418>


The first thing I noticed is that this definitely emits false positives that would not be improved by a switch. Consider SemaExprCXX.cpp:3288:

  return ConditionResult(*this, ConditionVar, MakeFullExpr(E.get(), StmtLoc),
                         CK == ConditionKind::ConstexprIf);

or SemaLambda.cpp:1023:

  assert(C->InitKind == LambdaCaptureInitKind::NoInit &&
         "init capture has valid but null init?");

Out of the five random diagnostics I picked, three would be made worse by a switch (SemaExprCXX.cpp:3288, SemaLambda.cpp:1023, CGDebugInfo.cpp:4000), one would be made better (LLParser.cpp:6206), and one would be questionable (PDB.cpp:30), all in my opinion, of course. Given the number of diagnostics it produces, and the fact that my very small random sampling produced a 60% false-positive rate, that's a bit concerning. It could also be a total coincidence due to my small sample size, of course.

I think this check has utility and would be useful, but I think it needs some tuning (whether in the form of heuristics or options) to help reduce the noise out of the box. Taking a more thorough look at the results from running it over clang may identify some usage patterns that suggest what kind of heuristics or tuning would help.


Repository:
  rL LLVM

https://reviews.llvm.org/D30896