================ Comment at: test/Parser/cxx-concepts-value-function.cpp:11 @@ +10,3 @@ + +#ifdef DIAG + ---------------- rsmith wrote: > hubert.reinterpretcast wrote: > > rsmith wrote: > > > nwilson wrote: > > > > rsmith wrote: > > > > > hubert.reinterpretcast wrote: > > > > > > nwilson wrote: > > > > > > > hubert.reinterpretcast wrote: > > > > > > > > hubert.reinterpretcast wrote: > > > > > > > > > -DDIAG=0 would still trigger the #ifdef DIAG here (which > > > > > > > > > explains why the test file might pass with > > > > > > > > > expected-no-diagnostics). > > > > > > > > I meant //without// `expected-no-diagnostics` somewhere. > > > > > > > Hmm, yeah, I missed that and thought it was working. Is there a > > > > > > > better way to use/suppress the diagnostic cases? > > > > > > `#if DIAG` instead of `#ifdef` would work. I guess > > > > > > `expected-no-diagnostics` could be in the `#else` for that. > > > > > Just run the test once, and remove the `#ifdef`. > > > > I'll submit a patch for this shortly so you guys can see if this > > > > explanation doesn't make sense. But, how about if I use the directives > > > > likes this: > > > > > > > > #if DIAG == 0 > > > > // RUN: %clang_cc1 -std=c++14 -fconcepts-ts -x c++ -verify %s -DDIAG=0 > > > > // expected-no-diagnostics > > > > > > > > *** test cases *** > > > > > > > > #elif DIAG == 1 > > > > // RUN: %clang_cc1 -std=c++14 -fconcepts-ts -x c++ -verify %s -DDIAG=1 > > > > > > > > *** test cases *** > > > > > > > > #endif > > > > > > > > > > > What are you trying to achieve by running the test twice? > > The presence of errors in a file may prevent further processing. Such > > further processing is not guaranteed to apply only to code which occurs > > lexically after the location of the error. By avoiding diagnostic cases in > > one of the runs, we can be more confident that the cases which should not > > receive diagnostics would not do so (instead of merely having the > > unexpected diagnostics conveniently preempted). > Failure to recover properly from an error would be a bug -- the parse of > later unrelated code should not be affected by an earlier failure, except in > extreme cases. The same argument would also imply that we should not have two > "expected to fail" tests in a row in one test file, because the earlier > failure could conceivably prevent the later failure from happening (or the > later failure might only happen when the earlier failure does). > > The valid cases will be tested more thoroughly in higher layers (for > instance, testing that semantic analysis can use a concept to select an > overload and that IR generation can generate correct IR for use of a > concept), so I'm not at all worried about those later tests failing to catch > a bug where a trivial concept definition leads to an unexpected error. > > As a more general observation: there is an established pattern for these > kinds of test (and many other things) in Clang. If our established pattern is > wrong, we should deal with that by discussing the pattern and how to fix it, > then fixing it globally, not by deviating from that pattern in an individual > change (that will lead to a fragmented and harder-to-maintain codebase). Okay; thanks for the explanation.
http://reviews.llvm.org/D10528 EMAIL PREFERENCES http://reviews.llvm.org/settings/panel/emailpreferences/ _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
