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