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

[Jonas Devlieghere via Phabricator via cfe-commits]

JDevlieghere added a comment.

I very much like this check. I only have a few minor comments, but maybe this encourages others to have a look too!



================
Comment at: clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp:85
+    diag(ElseIfWithoutElse->getLocStart(),
+         "potential uncovered codepath found; add an ending else branch");
+    return;
----------------
I'm not a big fan of the 'found', can we just omit it? The same goes for the other diags. 


================
Comment at: clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp:96
+    // Only the default branch (we explicitly matched for default!) exists.
+    if (CaseCount == 1) {
+      diag(SwitchWithDefault->getLocStart(),
----------------
Why not a switch?


================
Comment at: clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp:152
+    // Should be written as an IfStmt.
+    if (CaseCount == 1) {
+      diag(SwitchWithoutDefault->getLocStart(), "switch stmt with only one "
----------------
I'm aware that the message and fixme are different, but since the structure is so similar to the handling of the other switch case, I wonder if there is any chance we could extract the common parts?


https://reviews.llvm.org/D37808