[PATCH] D24562: [libcxx] Recover no-exceptions XFAILs

Eric Fiselier via cfe-commits cfe-commits at lists.llvm.org
Wed Oct 5 15:32:21 PDT 2016


EricWF added a comment.

In https://reviews.llvm.org/D24562#562339, @rmaprath wrote:

> First batch of XFAIL fixes.
>
> I've changed some XFAILs to UNSUPPORTED where the test is all about exception handling. In other cases, I've used the test macro TEST_HAS_NO_EXCEPTIONS to conditionally exclude those parts that test
>  exception handling behaviour.


The changes look great! Thanks for the patch.

The only suggestion I have is using `TEST_THROW` to guard a single throw statement. `TEST_THROW` will abort if it's executed in a no-exceptions build, which is better than silently passing over code that should throw. See the inline comments for example usage.

> @EricWF: I can create a separate review if necessary, thought I'll re-use this review for the first batch, will be opening new reviews for the follow-ups.

Sounds good to me. This batch LGTM but I can't re-approve this review. Please commit w/ requested inline changes and then close this revision.

> / Asiri





> async.pass.cpp:76
>      std::this_thread::sleep_for(ms(200));
>      throw j;
> +#endif

To guard a single `throw x;` consider using `TEST_THROW(x)`. This way the test will abort ungracefully if it tries to evaluate a `throw` at runtime.

> move_ctor.pass.cpp:23
>  #include "test_allocator.h"
> +#include "test_macros.h"
>  

Nit: I prefer including `test_macros.h` as the first support header.

> set_value_const.pass.cpp:28
> +    A(const A&) {
> +#ifndef TEST_HAS_NO_EXCEPTIONS    
> +        throw 10;

Nit: Use `TEST_THROW`

> F.pass.cpp:36
>      if (throw_one == 0)
>          throw std::bad_alloc();
>      --throw_one;

Nit: `TEST_THROW`

https://reviews.llvm.org/D24562





More information about the cfe-commits mailing list