doctest: Regression between 2.4.6 and 2.4.7

Description

Following minimized code works perfectly on 2.4.6, but fails on 2.4.7

// #define DOCTEST_CONFIG_IMPLEMENT_WITH_MAIN shouldn't be defined in compilation unit to reproduce
#include <doctest/doctest.h>

#include <string>

namespace {

class B
{
public:
    const std::string& getName() const
    {
        return m_name;
    }

    std::string m_name = "B";
};

}  // namespace

TEST_CASE("Wtf?") {
    B b;

    CHECK(b.getName() == "B");
}

https://godbolt.org/z/Eq48fjsed (due to the godbolt limitations see “undefined reference to `main’” linker warnings as success)

Flags: “-std=c++11 -Wall -Wextra -Wpedantic -Werror”

It fails on GCC version 4.9 - 10.3 with -fpermissive warning. (GCC version 11.2 compiles it without a problem though 🤔)

doctest/doctest.h: In instantiation of 'void doctest::detail::fillstream(const T (&)[N]) [with T = char; long unsigned int N = 2]':
doctest/doctest.h:904:31:   required from 'static void doctest::detail::filldata<T [N]>::fill(const T (&)[N]) [with T = const char; long unsigned int N = 2]'
doctest/doctest.h:918:65:   required from 'void doctest::detail::filloss(const T (&)[N]) [with T = char; long unsigned int N = 2]'
doctest/doctest.h:933:20:   required from 'static doctest::String doctest::detail::StringMakerBase<true>::convert(const T&) [with T = char [2]]'
doctest/doctest.h:978:35:   required from 'doctest::String doctest::toString(const T&) [with T = char [2]; typename doctest::detail::enable_if<(! doctest::detail::is_enum<T>::value), bool>::type <anonymous> = true]'
doctest/doctest.h:1142:45:   required from 'doctest::String doctest::detail::stringifyBinaryExpr(const L&, const char*, const R&) [with L = std::__cxx11::basic_string<char>; R = char [2]]'
doctest/doctest.h:1320:9:   required from 'decltype (((void)((declval<L>() == declval<R>())), (doctest::detail::Result)(<brace-enclosed initializer list>()))) doctest::detail::Expression_lhs<L>::operator==(const R&) [with R = char [2]; typename doctest::detail::enable_if<(! doctest::detail::is_rvalue_reference<R>::value), void>::type* <anonymous> = 0; L = const std::__cxx11::basic_string<char>&; decltype (((void)((declval<L>() == declval<R>())), (doctest::detail::Result)(<brace-enclosed initializer list>()))) = doctest::detail::Result]'
<source>:24:5:   required from here
doctest/doctest.h:896:31: error: no match for 'operator<<' (operand types are 'std::ostream' {aka 'std::basic_ostream<char, std::char_traits<char> >'} and 'const char')
doctest/doctest.h:557:33: note: candidate: 'std::ostream& doctest::operator<<(std::ostream&, const doctest::String&)' (near match)
doctest/doctest.h:557:33: note:   conversion of argument 2 would be ill-formed:
doctest/doctest.h:896:36: error: invalid user-defined conversion from 'const char' to 'const doctest::String&' [-fpermissive]
doctest/doctest.h:519:5: note: candidate is: 'doctest::String::String(const char*)' (near match)
doctest/doctest.h:519:5: note:   conversion of argument 1 would be ill-formed:
doctest/doctest.h:896:36: error: invalid conversion from 'char' to 'const char*' [-fpermissive]
doctest/doctest.h:896:36: error: invalid conversion from 'char' to 'const char*' [-fpermissive]
doctest/doctest.h:519:24: note:   initializing argument 1 of 'doctest::String::String(const char*)'
In file included from /opt/compiler-explorer/gcc-10.3.0/include/c++/10.3.0/string:55,
                 from <source>:4:
/opt/compiler-explorer/gcc-10.3.0/include/c++/10.3.0/bits/basic_string.h:6468:5: note: candidate: 'template<class _CharT, class _Traits, class _Alloc> std::basic_ostream<_CharT, _Traits>& std::operator<<(std::basic_ostream<_CharT, _Traits>&, const std::__cxx11::basic_string<_CharT, _Traits, _Alloc>&)'
 6468 |     operator<<(basic_ostream<_CharT, _Traits>& __os,
      |     ^~~~~~~~
/opt/compiler-explorer/gcc-10.3.0/include/c++/10.3.0/bits/basic_string.h:6468:5: note:   template argument deduction/substitution failed:
doctest/doctest.h:896:31: note:   mismatched types 'const std::__cxx11::basic_string<_CharT, _Traits, _Alloc>' and 'const char'

It fails on any version of clang, but with an error instead of the warning. It fails on MSVC at least in version 19 that is accessible on Godbolt.

Using this code as a test, it’s possible to bisect a broken commit.

About this issue

Original URL
State: closed
Created 3 years ago
Reactions: 1
Comments: 28 (9 by maintainers)

Commits related to this issue

Solve #571 issue — committed to YarikTH/doctest by YarikTH 3 years ago
Solve #571 issue — committed to YarikTH/doctest by YarikTH 3 years ago
Solve #571 issue — committed to YarikTH/doctest by YarikTH 3 years ago

Most upvoted comments

I was about to write pretty much the same thing lol. I think for the sake of simplicity we should handle only the case where the last character is either: valid and printed or null and ignored. So we’d only have one printing function that uses stream.write and we pass either N (non-null) or N-1 (null).

If someone needs behavior that differs from this they can simply wrap their char array in a struct with a custom toString implementation.

Saalvage on Jan 12, 2022

If 3, then maybe

// Specialized since we don't want the terminating null byte!
template<unsigned long N>
struct filldata<const char[N]>
{
    static void fill(std::ostream* stream, const char(&in)[N]) {
        unsigned long len = N-1;
        if( in[N-1] != '\0' )
            *stream << String(in, N); // array is not null terminated, pass N to guarantee that we don't read past the buffer
        else
            *stream << String(in); // array null terminated. Pass it to String constructor, it determinates the size via strlen call by itself
    }
};

it should be this way

YarikTH on Jan 12, 2022

I don’t think it’s an issue in itself, but it would cause the String to report an incorrect length were it ever asked, therefore I think the cleaner approach is just checking it.

Saalvage on Jan 12, 2022

Look like it is. But such a ternary breaks my mind😵

// Specialized since we don't want the terminating null byte!
template<unsigned long N>
struct filldata<const char[N]>
{
    static void fill(std::ostream* stream, const char(&in)[N]) {
        unsigned long len = N-1;
        if( in[N-1] != '\0' ) // if array is not null terminated, use the whole size
            len = N;
        *stream << String(in, len);
    }
};

a little more verbose solution, but more clear might fit better. At least it keeps readers sanity.

YarikTH on Jan 12, 2022