jfb added a comment. In D59523#1440263 <https://reviews.llvm.org/D59523#1440263>, @aaron.ballman wrote:
> In D59523#1440238 <https://reviews.llvm.org/D59523#1440238>, @jfb wrote: > > > In D59523#1440232 <https://reviews.llvm.org/D59523#1440232>, @aaronpuchert > > wrote: > > > > > > It's less about the regressions and more about the kind of regressions > > > > we're testing against. > > > > > > But the test also verifies that no diagnostics are omitted (`// > > > expected-no-diagnostics`), so it isn't just a "this doesn't crash" test. > > > Which is why I think it's a nice seed to build more systematic tests > > > around it. I'd generally like to test this more systematically, but that > > > isn't made easier if we're sprinkling the test cases over the code base. > > > > > > No, I wrote the test purely to check that the crash is gone. These tests > > *require* a diagnostic check, or `// expected-no-diagnostics`, so I put the > > later. > > > They only require `// expected-no-diagnostics` because you pass in `-verify` > on the RUN line. That isn't needed to test the crashing regression, which may > be part of what's causing confusion. Thanks for pointing this out! I've removed both. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59523/new/ https://reviews.llvm.org/D59523 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
