Feature: check or return false

[UnePierre]

Description

I like the expressiveness of doctest checks and their usage as asserts -- especially their reporting on failure.

I'd like to use them to check untrusted, user-given input or otherwise implement defensive mechanisms:

• With nice reporting in a debug build, but
• still with active checks in release builds (defined DOCTEST_CONFIG_DISABLE).

Example (before):

bool check_positive_range(int a, int b)
{
    CHECK(a >= 0);
    if( !(a >= 0) ) return false;

    CHECK(b >= 0);
    if( !(b >= 0) ) return false;

    CHECK(a <= b);
    if( !(a <= b) ) return false;

    return true;
}

This looks like a bunch of code duplication!
And the expressions are evaluated twice.

Note on the side: of course, seeing all failed checks is also worth a try, but sometimes, you check a pointer against nullptr before dereferencing it, so the early returns should stay, where they are in this little example.

Example (after):

bool check_positive_range(int a, int b)
{
    CHECK_OR_RETURN(a >= 0, false);
    CHECK_OR_RETURN(b >= 0, false);
    CHECK_OR_RETURN(a <= b, false);
    return true;
}

Concise and readable.

Beware! In case DOCTEST_CONFIG_DISABLE is defined, this must fall back to:

bool check_positive_range(int a, int b)
{
    if( !(a >= 0) ) return false;
    if( !(b >= 0) ) return false;
    if( !(a <= b) ) return false;
    return true;
}

What do you think?
Would this help in cleaning / checking user input and writing defensive code, that is actually talkative in debug mode?

Second option

Have CHECK() return a bool.

But (big BUT) this is a contradiction to "everything testing-related is removed from the binary, when DOCTEST_CONFIG_DISABLE is defined".

The example would be written as follows and fall back just as in the above code snippet:

bool check_positive_range(int a, int b)
{
    if( !CHECK(a >= 0) ) return false;
    if( !CHECK(b >= 0) ) return false;
    if( !CHECK(a <= b) ) return false;
    return true;
}

[onqtam]

Hi @UnePierre , Sorry for the late reply.

Here is how the CHECK_OR_RETURN macro could be implemented:

#define CHECK_OR_RETURN_IMPL(assert_type, cond, ret)                                              \
    do {                                                                                          \
        DOCTEST_CLANG_SUPPRESS_WARNING_WITH_PUSH("-Woverloaded-shift-op-parentheses")             \
        doctest::detail::ResultBuilder _DOCTEST_RB(doctest::assertType::assert_type, __FILE__,    \
                                                __LINE__, #cond);                                 \
        DOCTEST_WRAP_IN_TRY(_DOCTEST_RB.setResult(                                                \
                doctest::detail::ExpressionDecomposer(doctest::assertType::assert_type) << cond)) \
        DOCTEST_ASSERT_LOG_AND_REACT(_DOCTEST_RB);                                                \
        if(_DOCTEST_RB.m_failed) return ret;                                                      \
        DOCTEST_CLANG_SUPPRESS_WARNING_POP                                                        \
    } while(false)

#define CHECK_OR_RETURN(cond, ret) CHECK_OR_RETURN_IMPL(DT_CHECK, cond, ret)

This is basically the implementation of the CHECK macro but with the addition of if(_DOCTEST_RB.m_failed) return ret; (and no longer accepting varargs).

However, I don't think the library should provide this macro as well - users might want exceptions instead of returns and there is already a way to use standard doctest asserts outside of a testing context (within the actual production codebase) with a custom assert handler: https://github.com/onqtam/doctest/blob/master/doc/markdown/assertions.md#using-asserts-out-of-a-testing-context

Let me know if this helps!

[onqtam]

Asserts now return the result of the expression and can be used in if statements ever since #496 was fixed in the dev branch so now you can use if(!CHECK(...)) return false; to achieve what you need.

I also just made it so that the comparison assertions also work properly when DOCTEST_CONFIG_DISABLE is defined but didn't do it for the exception macros (seems too niche and too much code) - again in the dev branch. Closing this as this should be enough for your needs!

[UnePierre]

Thanks for implementing the "Second option". I, too, consider this issue solved.

[onqtam]

FYI it's not enabled by default - you'll have to define DOCTEST_CONFIG_EVALUATE_ASSERTS_EVEN_WHEN_DISABLED because otherwise, it could lead to breaking changes for users with asserts in their production code that use DOCTEST_CONFIG_DISABLE - suddenly more functions & code might start to get evaluated. c6db85a