JonasToth added inline comments.
================ Comment at: clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp:119 + if (!SwitchHasDefault && SwitchCaseCount == 0) { + diag(Switch->getLocStart(), "degenerated switch without labels"); + return; ---------------- aaron.ballman wrote: > JonasToth wrote: > > aaron.ballman wrote: > > > I think a better way to phrase this one is: "switch statement without > > > labels has no effect" and perhaps have a fix-it to replace the entire > > > switch construct with its predicate? > > The fixit would only aim for the sideeffects the predicate has, right? I > > would consider such a switch as a bug or are there reasonable things to > > accomplish? Given its most likely unintended code i dont think a fixit is > > good. > > > > Fixing the code removes the warning and might introduce a bug. > This code pattern comes up with machine-generated code with some frequency, > so I was thinking it would be nice to automatically fix that code. However, I > think you're right that "fixing" the code may introduce bugs because you > don't want to keep around non-side effecting operations and that's > complicated. e.g., > ``` > switch (i) { // Don't replace this with i; > } > > switch (some_function_call()) { // Maybe replace this with > some_function_call()? > } > > switch (i = 12) { // Should definitely be replaced with i = 12; > } > ``` > Perhaps only diagnosing is the best option. I will add another FIXME. The if-is-better pattern might be reasonable transformable and doing this allows addressing the issue again later. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40737 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits