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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
