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

[Jonas Devlieghere via Phabricator via cfe-commits]

JDevlieghere added inline comments.


================
Comment at: clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp:96
+    // Only the default branch (we explicitly matched for default!) exists.
+    if (CaseCount == 1) {
+      diag(SwitchWithDefault->getLocStart(),
----------------
JonasToth wrote:
> 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`.
Sounds good


================
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 "
----------------
JonasToth wrote:
> 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) {}
> ```
> ?
Wouldn't it be easier to have a function and pass as arguments the stuff that's different? Both are `SwitchStmt`s so if you pass that + the diagnostic message you should be able to share the other logic. 


https://reviews.llvm.org/D37808