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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits