JonasToth marked 6 inline comments as done.
JonasToth 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`.
Woops. I mixed codeplaces (since bad duplication from my side). I did merge the
diagnostics better, making this a natural fit for 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 "
----------------
JDevlieghere wrote:
> 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.
I kinda rewrote the whole checking part.
Updated diff comes in a sec. I found that bitfields are not handled as they
should.
https://reviews.llvm.org/D37808
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
