[PATCH] D40737: [clang-tidy] Resubmit hicpp-multiway-paths-covered without breaking test

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Mar 2 11:34:31 PST 2018


aaron.ballman added a comment.

In https://reviews.llvm.org/D40737#1025777, @lebedev.ri wrote:

> In https://reviews.llvm.org/D40737#1024120, @JonasToth wrote:
>
> > After long inactivity (sorry!) i had a chance to look at it again:
> >
> >   switch(i) {
> >   case 0:;
> >   case 1:;
> >   case 2:;
> >   ...
> >   }
> >
> >
> > does *NOT* lead to the stack overflow. This is most likely an issue in the AST:
> >  https://godbolt.org/g/vZw2BD
> >
> > Empty case labels do nest, an empty statement prevents this. The nesting leads most likely to the deep recursion. I will file a bug for it.
>
>
> FWIW here are my 5 cent: this is a preexisting bug. Your testcase just happened to expose it.
>  I'd file the bug, and then simply adjust the testcases here not to trigger it and re-land this diff.
>
> I'm not sure what is to be gained by not doing that.
>  Of course, the bug is a bug, and should  be fixed, but it exists regardless of this differential...


Just because a particular check triggers an issue elsewhere doesn't mean we should gloss over the issue in the check. That just kicks the can farther down the road so that our users find the issue. In this case, the check is likely to push users *towards* discovering that AST bug rather than away from it which is why I'm giving the push-back.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40737





More information about the cfe-commits mailing list