error: conversion from ‘int’ to ‘const snapdev::timespec_ex’ is ambiguous

[AlexisWilke]

Describe the bug

I have a class named snapdev::timespec_ex with several constructors. The two we are concerned with here are:

timespec_ex(std::int64_t nsec);
timespec_ex(double sec);

which cause an issue when I try to do:

CATCH_REQUIRE(a == b);

because you have a macro which has this declaration (catch2/internal/catch_compare_traits.hpp):

[...]
struct is_##id##_0_comparable<T,                                          \
                              void_t<decltype( std::declval<T>() op 0 )>> \
    : std::true_type {};

I think that the value 0 should be taken as an std::int64_t, but in this case the compiler decides that double is also a viable choice. Here is the error:

[ 23%] Building CXX object tests/CMakeFiles/unittest.dir/catch_timespec_operations.cpp.o
In file included from /home/snapwebsites/snapcpp/BUILD/Debug/dist/include/catch2/internal/catch_decomposer.hpp:14:0,
                 from /home/snapwebsites/snapcpp/BUILD/Debug/dist/include/catch2/internal/catch_assertion_handler.hpp:12,
                 from /home/snapwebsites/snapcpp/BUILD/Debug/dist/include/catch2/internal/catch_test_macro_impl.hpp:12,
                 from /home/snapwebsites/snapcpp/BUILD/Debug/dist/include/catch2/catch_test_macros.hpp:11,
                 from /home/snapwebsites/snapcpp/BUILD/Debug/dist/include/catch2/snapcatch2.hpp:30,
                 from /home/snapwebsites/snapcpp/contrib/snapdev/tests/catch_main.h:28,
                 from /home/snapwebsites/snapcpp/contrib/snapdev/tests/catch_timespec_operations.cpp:29:
/home/snapwebsites/snapcpp/BUILD/Debug/dist/include/catch2/internal/catch_compare_traits.hpp: In instantiation of ‘struct Catch::Detail::is_eq_0_comparable<snapdev::timespec_ex&, void>’:
/home/snapwebsites/snapcpp/BUILD/Debug/dist/include/catch2/internal/catch_decomposer.hpp:222:9:   required from ‘class Catch::ExprLhs<snapdev::timespec_ex&>’
/home/snapwebsites/snapcpp/contrib/snapdev/tests/catch_timespec_operations.cpp:119:9:   required from here
/home/snapwebsites/snapcpp/BUILD/Debug/dist/include/catch2/internal/catch_compare_traits.hpp:52:44: error: conversion from ‘int’ to ‘const snapdev::timespec_ex’ is ambiguous
/home/snapwebsites/snapcpp/BUILD/Debug/dist/include/catch2/internal/catch_compare_traits.hpp:44:52:
                                   void_t<decltype( std::declval<T>() op 0 )>> \
                                                    ~~~~~~~~~~~~~~~~~~~~~~
/home/snapwebsites/snapcpp/BUILD/Debug/dist/include/catch2/internal/catch_compare_traits.hpp:51:44:
         CATCH_DEFINE_COMPARABLE_TRAIT( ge, >= )
/home/snapwebsites/snapcpp/BUILD/Debug/dist/include/catch2/internal/catch_compare_traits.hpp:44:70: note: in definition of macro ‘CATCH_DEFINE_COMPARABLE_TRAIT’
                                   void_t<decltype( std::declval<T>() op 0 )>> \
                                                                      ^~
In file included from /home/snapwebsites/snapcpp/contrib/snapdev/tests/catch_timespec_operations.cpp:27:0:
/home/snapwebsites/snapcpp/contrib/snapdev/snapdev/timespec_ex.h:148:5: note: candidate: snapdev::timespec_ex::timespec_ex(double)
     timespec_ex(double sec)
     ^~~~~~~~~~~
/home/snapwebsites/snapcpp/contrib/snapdev/snapdev/timespec_ex.h:135:5: note: candidate: snapdev::timespec_ex::timespec_ex(int64_t)
     timespec_ex(std::int64_t nsec)
     ^~~~~~~~~~~
/home/snapwebsites/snapcpp/contrib/snapdev/snapdev/timespec_ex.h:806:10: note:   initializing argument 1 of ‘bool snapdev::timespec_ex::operator>=(const snapdev::timespec_ex&) const’
     bool operator >= (timespec_ex const & t) const
          ^~~~~~~~

Expected behavior

I expect the code to compile as before.

Reproduction steps

Extract the test.cpp file, see the c++ command line at the top of the file. Just add a -I ... path if you do not have catch2 under /usr/include.

test.zip

Platform information:

• OS: Linux
• Compiler+version: g++ (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0
• Catch version: v3.2.0

Additional context

It was working fine with the previous version (3.1.1).

Temporary Fix

As shown in the attached test.cpp file, I can just add a new constructor accepting int and it compiles just fine. I would prefer not to have that operator as it is likely to cause very difficult to find bugs.

Reference

For reference, the original is found in the snapdev project. The file is named snapdev/timespec_ex.h. The corresponding test, tests/catch_timespec_operations.cpp is what generates the errors.

[horenmar]

And here I was, thinking that the decomposing rewrite has gone fine. 😃

I understand what the issue is, and I think I know how to fix it.

[horenmar]

I couldn't reproduce this locally, turns out that this only happens with older versions of GCC.

[AlexisWilke]

Okay, in that case it's certainly not too important. Ubuntu 18.04 will soon be out of date so it's probably not worth the trouble. Up to you if you want to just close this issue.

[horenmar]

Well, this turned out to be quite the journey. The workaround for old GCC worked fine pre C++20, which significantly changed the rules for comparison operators, in a way that was incompatible with the workaround (but worked without the workaround due to SFINAE rules).

The current devel supports C++20 and old GCCs, but it is still my professional opinion that you should not have easily-ambiguous implicit constructors on a type.

[AlexisWilke]

Excellent. That worked in my environment as well. So you fixed it! :-)