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

Reply via email to