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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to