Add default printers for enums

[obfuscated]

I've added an assertion for a function returning an enum and the enum value. When the test failed I've got pretty informative failure output {?} == {?}...

I don't intend to write a custom printer for my enum type, but still want to see readable values when a test fails. I've workaround the problem by casting to int, but I'd prefer if the library does this for me.

[onqtam]

I just tested this and it seems to work with C++98 enums but not with C++11 enum classes.

I could use std::is_enum and SFINAE to handle them but I'd rather not add any more templates (or expecially SFINAE) on that very important part of the assertions - it will start degrading the compile times.

I would advise to teach the framework how to stringify enum classes on your end like this:

#include <type_traits>
#include <ostream>

template <typename T>
typename std::enable_if<std::is_enum<T>::value, std::ostream&>::type
operator<<(std::ostream& stream, const T& in) {
    stream << typename std::underlying_type<T>::type(in);
    return stream;
}

Will link to this issue in the docs.

EDIT: see below for the actual working snippet - not based on operator<< but on StringMakerBase

[obfuscated]

But then I'll still get a degraded compile time performance...

[onqtam]

After looking a bit more into this issue I see no way of doing it without SFINAE - and even then I would need something from <type_traits> - Catch2 uses SFINAE for this.

[obfuscated]

So, users are left to fix this deficiency on their own?

[onqtam]

Well I could make yet another config option (off by default) that will force the inclusion of <type_traits> and add SFINAE for every assert but I'd rather not - this seems a simple enough extension for users to make on their own.

However if other people vote this is important (thumbs up/down for this issue) I will do it...

[onqtam]

you know what? it might be time to introduce a second header with extensions which users can optionally include..... like printers for types from the std namespace and stuff like in this issue. But it won't go into the main header with another config option........

[LossMentality]

Hi onqtam -

It seems as if the function template operator<< for writing enum class enumerants to ostreams (posted above in this thread) needs to be redefined in every namespace that has an enum class you want doctest to print. If it's not in the namespace, doctest prints {?} for enumerant integer values when an assert fails.

Interestingly, this is not true for anything else. For instance, if the operator<< function template is defined at global scope, it will allow the writing of all enum class enumerants to ostreams, no matter what namespace the enum is defined in, and no matter what namespace the ostream write happens in.

Is this what you expected - that one would need to redefine this operator in each enum class's namespace?

I've actually had an operator<< for enum classes defined in the global namespace for quite some time, and so don't want to have to duplicate it in each namespace just for doctest (thus I'll probably be overloading doctest::toString for enum class printing from doctest).

Anyway, based on all of what I've described here, I'd vote for what you discussed above - an additional and optional doctest header that would allow doctest to print enum class enumerants.

PS I think there is a missing "typename" in the operator<< posted earlier in this thread - it won't compile without it. Should be:
stream << typename std::underlying_type::type(in);

PPS I'm just evaulating doctest for the first time. I'm impressed - nice job.

[onqtam]

@LossMentality I think I initially proposed the function template to be in global scope - was working when I tested it - duplicating it in different namespaces wasn't intended.

One might make a proxy header that includes doctest and defines such extensions, and then source files should include the proxy header instead of the doctest header directly - like this:

https://github.com/onqtam/doctest/blob/master/examples/all_features/doctest_proxy.h

Let me know if it works!

glad you like the framework! version 2.1 is fresh out of the oven - an hour ago 🗡

[LossMentality]

Awesome, I'll have to check out the new release when I'm back at work on Monday.

Yeah, I was surprised that it wasn't working at global scope. I'll give the proxy header idea a try Monday, too.

I'm pretty sure at this point I'll be going forward with doctest, but once I know for sure, I'll to try to get my company to throw some bucks on your patreon.

[onqtam]

@LossMentality Actually I just tried it for an enum inside of some namespace and it indeed didn't work. I'll have to think about this.

[LossMentality]

@onqtam Okay, thanks.

[onqtam]

@LossMentality I found the only possible way to do this without modifying the framework and having to introduce SFINAE on the normal template path for asserts (or more compile time config options + complexity...) - there was 1 last point of customization that could be used - StringMakerBase<false> (not toString or StringMaker as advertised in the stringification docs...) - and it looks like this:

#include <type_traits>

namespace doctest { namespace detail {
    template <>
    struct StringMakerBase<false>
    {
        template <typename T> // if T is an enum
        static typename std::enable_if<std::is_enum<T>::value, String>::type
        convert(const T& in) {
            return toString(typename std::underlying_type<T>::type(in));
        }

        template <typename T> // if T is NOT an enum
        static typename std::enable_if<!std::is_enum<T>::value, String>::type
        convert(const T&) {
            return "{?}";
        }
    };
} } // namespace doctest, detail 

and works for the following example:

namespace ns {
    enum class enm {
        one,
        two
    };
}

TEST_CASE("") {
    CHECK(ns::enm::one == ns::enm::two);
}

and the output is:

test.cpp:34: ERROR: CHECK( ns::enm::one == ns::enm::two ) is NOT correct!
  values: CHECK( 0 == 1 )

This is a good enough solution for you, but I don't think it is good enough to enter the extensions header - I'll have to think if I'm okay with letting go of this last such customization point only for enums.

[LossMentality]

@onqtam Thanks very much, this works.

A couple of questions:

1. I expect that your comment about degrading compile times above is applicable here as well? Because the compiler will evaluate std::is_enum on every type in all asserts?

2. Regarding what you said about "the last customization point" - are you referring to the use of StringMakerBase? I see doctest.h already has a StringMakerBase, so I assume you're considering if using StringMakerBase just for stringification of enums is worth locking out the potential to use it for something else?

Assuming the answers to both of the above questions is yes, I think I may place this code in a separate header that can be included only in test code that is dealing with enums, or possibly fence it off with a macro we can define to include the enum stringification code only when needed. Then we won't degrade compile times unless we have to, and can future proof against a decision to use StringMakerBase for something else in a future version of doctest.

Anyway, thanks again.

[onqtam]

@LossMentality Regarding the compile times - the problem is not that much the evaluation of std::is_enum - but the whole SFINAE thing - see here: https://www.reddit.com/r/cpp/comments/6gur2x/the_rule_of_chiel_aka_compiletime_cost_of/

But even if there is SFINAE - asserts will not become more than 15-20% slower to compile - these compile time concerns are only for what enters the core library and matter only for HUGE test suites - I doubt that you have more than 1 million asserts like this project has: https://github.com/nlohmann/json
(see here: catchorg/Catch2#1398 - he mentions millions)

It should be fine for you to use this as a custom extension. I would strongly advise to just use this code everywhere (with the <type_traits> header included) - it shouldn't be that much of a difference (a few seconds at most for most reasonably-sized projects).

And regarding your second question - again the answer is Yes - I don't know what StringMakerBase could turn out to be useful for, but it should be fine for you to use it for the foreseeable future :)

[LossMentality]

@onqtam Interesting video! I had never thought about the wide range of costs across the various types of things that occur TMP.

And you're right - even if we someday approach a decent amount of test coverage in our codebase (it's a decade old with no tests, so we're refactoring and putting some in), I doubt we'll ever have anywhere near that many. So I'll go with your recommendation and just include the enum stringification code in the doctest configuration header I've set up so that it's always available.

[Trass3r]

This is awesome if you can use C++17: https://github.com/Neargye/magic_enum#magic-enum-c

[andreaplanet]

This is awesome if you can use C++17: https://github.com/Neargye/magic_enum#magic-enum-c

Mixing magic_enum and the sample code from onqtam we get a quick workaround

#include <magic_enum.hpp>
#include <type_traits>

namespace doctest { namespace detail {
    template <>
    struct StringMakerBase<false>
    {
        template <typename T> // if T is an enum
        static typename std::enable_if<std::is_enum<T>::value, String>::type
        convert(const T& in) {
            //return toString(typename std::underlying_type<T>::type(in)); <-- print enum value
            return toString(magic_enum::enum_name(in)); // <-- print enum name
        }

        template <typename T> // if T is NOT an enum
        static typename std::enable_if<!std::is_enum<T>::value, String>::type
        convert(const T&) {
            return "{?}";
        }
    };
} } // namespace doctest, detail

the output would be

test.cpp:34: ERROR: CHECK( ns::enm::one == ns::enm::two ) is NOT correct!
  values: CHECK( one == two )

[Trass3r]

Since we're talking about 17 anyway I'd put the enable ifs into the template part and use all the _t and _v aliases.

[onqtam]

Just merged this PR (thanks to @jkriegshauser ) which adds support for printing enums in the library by default (no need for extension headers or config options): #429
Will release a new version of the library soon.

[nlguillemot]

I had some enums where I defined an ostream<< operator and wanted to use it to print my enums rather than printing their underlying value like doctest seems to do by default. I came up with this hack:

namespace my {
  // A template that only matches insertable values.
  template <class T, class = std::enable_if_t<doctest::detail::has_insertion_operator<T>>>
  using match_insertable = T;
}

namespace doctest { namespace detail {
  template <class T> struct is_enum<my::match_insertable<T>> {
    constexpr static bool value = false;
  };
}}

This fools doctest into thinking that any value that is printable is not an enum, even if it's really an enum, so it will print the value with the normal logic rather than printing enums as their underlying type.

Maybe there's a better solution, or maybe there's problems with this solution, but this looks like it works for my limited usage so I thought I'd share.

[nlguillemot]

The hack above seems to only work on GCC, not clang. I think this is related: https://stackoverflow.com/questions/59756201/clang-gcc-inconsistency-in-class-specialization

I think the tweak above might have to be done in upstream doctest, for example by updating the existing is_enum template to ignore insertable values. Maybe doctest should use a template like is_uninsertable_enum rather than just is_enum

[Saalvage]

I don't really have the time to fully look into this atm, could you open a new issue?