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.

  rCTE Clang Tools Extra


cfe-commits mailing list

Reply via email to