Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DOCTEST_CHECK_THROWS_WITH_AS fails to work with dependant exception type #447

Closed
YarikTH opened this issue Dec 14, 2020 · 1 comment · Fixed by nlohmann/json#2538
Closed

Comments

@YarikTH
Copy link

YarikTH commented Dec 14, 2020

Description

I tried to write a templated test case. In this test case, I need to check the thrown exception. But compilation failed because my exception type depends on the tested type. Unfortunately deep inside doctest's macroses (namely in DOCTEST_ASSERT_THROWS_AS) code is not ready to dependant exception types.

Steps to reproduce

https://godbolt.org/z/f5M4on

#define DOCTEST_CONFIG_IMPLEMENT_WITH_MAIN
#include <https://raw.githubusercontent.com/onqtam/doctest/2.4.1/doctest/doctest.h>
#include <exception>

template<class T>
struct Json
{
    using out_of_range = std::out_of_range;

    T get()
    {
        throw out_of_range( "Out of range" );
        return T();
    }
};

TEST_CASE_TEMPLATE("My testcase", T,
                   Json<int>)
{
    T json;
    DOCTEST_CHECK_THROWS_WITH_AS(json.get(), "Out of range", typename T::out_of_range);
}

Example output

x86-64 gcc 7.3 (Editor #1, Compiler #1) C++
x86-64 gcc 7.3
-std=c++11 -O0
1
<Compilation failed>
x86-64 gcc 7.3 - 1737ms
#1 with x86-64 gcc 7.3
<source>: In function 'void _DOCTEST_ANON_TMP_8()':
<doctest/doctest.h>:2055:73: error: type/value mismatch at argument 1 in template parameter list for 'template<class T> struct doctest::detail::remove_const'
<doctest/doctest.h>:2099:58: note: in expansion of macro 'DOCTEST_ASSERT_THROWS_AS'
<source>:21:5: note: in expansion of macro 'DOCTEST_CHECK_THROWS_WITH_AS'
<doctest/doctest.h>:2055:73: note:   expected a type, got 'doctest::detail::remove_reference<typename T::out_of_range>::type'
<doctest/doctest.h>:2099:58: note: in expansion of macro 'DOCTEST_ASSERT_THROWS_AS'
<source>:21:5: note: in expansion of macro 'DOCTEST_CHECK_THROWS_WITH_AS'
<doctest/doctest.h>:2055:80: error: expected ')' before '&' token
<doctest/doctest.h>:2099:58: note: in expansion of macro 'DOCTEST_ASSERT_THROWS_AS'
<source>:21:5: note: in expansion of macro 'DOCTEST_CHECK_THROWS_WITH_AS'
<doctest/doctest.h>:2055:80: error: expected '{' before '&' token
<doctest/doctest.h>:2099:58: note: in expansion of macro 'DOCTEST_ASSERT_THROWS_AS'
<source>:21:5: note: in expansion of macro 'DOCTEST_CHECK_THROWS_WITH_AS'
<doctest/doctest.h>:2055:81: error: expected primary-expression before ')' token
<doctest/doctest.h>:2099:58: note: in expansion of macro 'DOCTEST_ASSERT_THROWS_AS'
<source>:21:5: note: in expansion of macro 'DOCTEST_CHECK_THROWS_WITH_AS'
<doctest/doctest.h>:2058:15: error: expected primary-expression before 'catch'
<doctest/doctest.h>:2099:58: note: in expansion of macro 'DOCTEST_ASSERT_THROWS_AS'
<source>:21:5: note: in expansion of macro 'DOCTEST_CHECK_THROWS_WITH_AS'

The workaround for this is quite straightforward:
https://godbolt.org/z/qWqxsr

#define DOCTEST_CONFIG_IMPLEMENT_WITH_MAIN
#include <https://raw.githubusercontent.com/onqtam/doctest/2.4.1/doctest/doctest.h>
#include <exception>

template<class T>
struct Json
{
    using out_of_range = std::out_of_range;

    T get()
    {
        throw out_of_range( "Out of range" );
        return T();
    }
};

#define DOCTEST_ASSERT_THROWS_AS_PATCHED(expr, assert_type, message, ...)                          \
    do {                                                                                           \
        if(!doctest::getContextOptions()->no_throw) {                                              \
            doctest::detail::ResultBuilder _DOCTEST_RB(doctest::assertType::assert_type, __FILE__, \
                                                       __LINE__, #expr, #__VA_ARGS__, message);    \
            try {                                                                                  \
                DOCTEST_CAST_TO_VOID(expr)                                                         \
            } catch(const typename doctest::detail::remove_const<                                  \
                    typename doctest::detail::remove_reference<__VA_ARGS__>::type>::type&) {       \
                _DOCTEST_RB.translateException();                                                  \
                _DOCTEST_RB.m_threw_as = true;                                                     \
            } catch(...) { _DOCTEST_RB.translateException(); }                                     \
            DOCTEST_ASSERT_LOG_AND_REACT(_DOCTEST_RB);                                             \
        }                                                                                          \
    } while(false)

#define DOCTEST_CHECK_THROWS_WITH_AS_PATCHED(expr, message, ...) DOCTEST_ASSERT_THROWS_AS_PATCHED(expr, DT_CHECK_THROWS_WITH_AS, message, __VA_ARGS__)


TEST_CASE_TEMPLATE("My testcase2", T,
                   Json<int>)
{
    T json;
    DOCTEST_CHECK_THROWS_WITH_AS_PATCHED(json.get(), "Out of range", typename T::out_of_range);
}

Need to add "typename" before both "doctest::detail::remove_const" and "doctest::detail::remove_reference". It seems not to harm even if the exception type is not dependant. But I'm not sure is it true for most of the compilers and c++ standards. At least such macro group could be added separately. Like DOCTEST_CHECK_THROWS_WITH_AS_DEPENDANT.

Extra information

  • doctest version: v2.4.1
  • Operating System: Godbolt
  • Compiler+version: any
@onqtam
Copy link
Member

onqtam commented Dec 14, 2020

adding typename doesn't seem to be a problem - pushing a fix for this in the dev branch - will release a new version by the end of December probably!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants