Use the do {} while (false) idiom for BOOST_TEST_THROWS() and BOOST_TEST_NO_THROW()

[gennaroprota]

Reason: Common hygiene for multi-line macros. Makes things like

if (condition)
    BOOST_TEST_THROWS(...);
else
    ...

work correctly. Also, it doesn't generate an empty statement (which compilers may warn about) when the user adds a semicolon (which they usually do).

[pdimov]

This is technically a breaking change because the semicolon is now required, whereas it wasn't before. But it looks like all our uses have a semicolon.

The backslash at the last line shouldn't be present. We should remove it from BOOST_TEST_THROWS, instead of adding it to BOOST_TEST_NO_THROW.

[gennaroprota]

The backslash at the last line shouldn't be present. We should remove it from BOOST_TEST_THROWS, instead of adding it to BOOST_TEST_NO_THROW.

I thought the convention was to terminate with a // comment:

last_line    \
//

Didn't notice this hadn't been done for BOOST_TEST_NO_THROW().

[pdimov]

libs\core\test\lightweight_test_test.cpp(105) : warning C4127: conditional expression is constant

Yeah :-/

[gennaroprota]

Sigh. Which version of MSVC? I don't get that warning with MSVC 2022 and /Wall.

[pdimov]

Up to msvc-12.0 according to Appveyor.

[gennaroprota]

Hmm. We might try with do {} while (0), which is probably more common and might be special-cased, even in old versions of MSVC.

[Lastique]

Just add _Pragma("warning(disable: 4127)") to the macros. With the corresponding push and pop pragmas as well.

[pdimov]

BOOST_TEST_xxx is a public macro name. Please use BOOST_LWT_DETAIL_xxx for implementation details.

[Lastique]

One other alternative we could try is for (;;) { x; break; }.

[gennaroprota]

That doesn't play well with the semicolon (the one after the macro invocation).

[pdimov]

Everything fails, probably because you're missing a semicolon here BOOST_LWT_DETAIL_DO_WHILE_FALSE(static_cast<void>(0)).

[pdimov]

(void)0; is probably a more idiomatic form of a null statement, despite the C cast.

[gennaroprota]

Do you prefer that? (Why?)

[pdimov]

It's idiomatic because that's what assert typically uses.

Nowadays (void) is also emerging as the idiomatic way to silence nodiscard warnings.

[gennaroprota]

I don't have a strong opinion about that. I can change it, if you like. (About assert(), though, I think it's (void) because it's common to C.)

[pdimov]

Yes please, let's go with (void). I've always found static_cast<void> somewhat pretentious, and it does require parentheses around the expression which is also a bit jarring.

[pdimov]

@Lastique @glenfe Any objections to merging this?

[Lastique]

No, looks good to me.

[pdimov]

This breaks at least

https://github.com/boostorg/unordered/blob/4ec5cb59cd2acfd05186516f7ecede5d4440f2aa/test/unordered/transparent_tests.cpp#L1692

[pdimov]

Looks like this is the only such occurrence though.

[pdimov]

Nope, two more :-)

https://github.com/boostorg/variant2/blob/a63b03286e3347f9d570f2b75d480f129cec8ecc/test/variant_copy_assign_throw.cpp#L48
https://github.com/boostorg/variant2/blob/a63b03286e3347f9d570f2b75d480f129cec8ecc/test/variant_move_assign_throw.cpp#L48

[gennaroprota]

@Lastique: That changelog entry is a bit wrong: With the previous implementation (without do {} while (false)), this:

if (condition)
    BOOST_TEST_THROWS(...); // note the semicolon
else
    ...

was a syntax error. The warnings you hint at, instead, were for the do {} while (false) itself.

[Lastique]

The previous definition was a direct try/catch, so if I'm not mistaken, it wasn't an error under if without the semicolon, was it? BOOST_TEST_NO_THROW also wasn't an error.

BTW, given that the macros are now conditionally defined depending on BOOST_MSVC, does this mean that the motivating example with if/else and semicolon is still not portable and should not be used despite this PR?

Anyway, if you can provide a better text in the changelog, please create a PR.

[gennaroprota]

The previous definition was a direct try/catch, so if I'm not mistaken, it wasn't an error under if without the semicolon, was it? BOOST_TEST_NO_THROW also wasn't an error.

Yes.

BTW, given that the macros are now conditionally defined depending on BOOST_MSVC, does this mean that the motivating example with if/else and semicolon is still not portable and should not be used despite this PR?

Yes, it can be used only if MSVC < 2015 is of no concern :-(. That MSVC warning ruined everything. I wish I could withdraw this PR.

[pdimov]

Fixed in 7b2c714.