JonasToth added inline comments.

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


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

In the testcases there is one switch with all numbers explicitly written, 
meaning there is no need to add a default branch.

This would allow further 
```
else if (CaseCount == MaximumPossibleCases) { /* No warning */ }
```
path which is not modable with `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 "
----------------
JDevlieghere wrote:
> 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?
I try to get something shorter.
Maybe 
```
if(CaseCount == 1 && MatchedSwitch) {}
else if(CaseCount == 1 && MatchedElseIf) {}
```
?


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