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

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Mar 15 08:20:44 PDT 2017

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.



More information about the cfe-commits mailing list