mpark / variant

C++17 `std::variant` for C++11/14/17
https://mpark.github.io/variant
Boost Software License 1.0
659 stars 88 forks source link

Constructor broken with gcc-4.8.5 #75

Open rnburn opened 4 years ago

rnburn commented 4 years ago
#include <iostream>

#include "variant.hpp"

struct sv {
        sv(const char*) {}
};

int main() {
        mpark::variant<bool, sv> v{"abc"};
        std::cout << v.index() << "\n";
        return 0;
}

With gcc-4.8.5, this code incorrectly prints 0.

It works for later versions of gcc (tested 7.5.0) and prints 1.

I tested against master 3c7fc8266bb46046b42c2dc2663f9f505f0cec28

rnburn commented 4 years ago

It looks like the bool conversion logic in overload_leaf is disabled for gcc < 5.0 (I'm guessing because the narrowing conversion check doesn't work).

    template <typename Arg, std::size_t I, typename T>
    struct overload_leaf<
        Arg,
        I,
        T,
        true
#if defined(__clang__) || !defined(__GNUC__) || __GNUC__ >= 5
        ,
        lib::enable_if_t<
            std::is_same<lib::remove_cvref_t<T>, bool>::value
                ? std::is_same<lib::remove_cvref_t<Arg>, bool>::value
                : is_non_narrowing_convertible<Arg, T>::value>
#endif
        > {
      using impl = lib::size_constant<I> (*)(T);
      operator impl() const { return nullptr; };
    };

But could this be changed to

template <typename Arg, std::size_t I, typename T>
struct overload_leaf<Arg,
                     I,
                     T,
                     true,
                     enable_if_t<std::is_same<remove_cvref_t<T>, bool>::value
                                     ? std::is_same<remove_cvref_t<Arg>, bool>::value
                                     :
#if defined(__clang__) || !defined(__GNUC__) || __GNUC__ >= 5
                                     is_non_narrowing_convertible<Arg, T>::value
#else
                                     std::is_convertible<Arg, T>::value
#endif
                                 >>
{
  using impl = size_constant<I> (*)(T);
  operator impl() const { return nullptr; };
};

that would be closer to the correct std::variant behavior