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

Visit + Lambda #44

Closed erakis closed 6 years ago

erakis commented 6 years ago

Hi,

I'm trying to use lambda instead of functor to visit a variant element. But is it possible to get compilation error when lambda do not handle all variant type ? Same behavior as a functor ?

Ex :

typedef mpark::variant<
    int8_t, uint8_t,
    double,
    std::string
> VariantValue;

VariantValue v{ (double)2.0 };

mpark::visit([](auto&& arg)
{
    using T = std::decay_t<decltype(arg)>;
    if (std::is_same_v<T, int8_t>)
        std::cout << "int8_t with value " << arg << '\n';
    else if (std::is_same_v<T, std::string>)
        std::cout << "std::string with value " << arg << '\n';
    else
        // static_assert(always_false<T>::value, "non-exhaustive visitor!");
        // static_assert(mpark::lib::is_invocable<T, arg>::value, "non-exhaustive visitor!");

        // ----------------------------------------------------------------------
        // I tried many thing but I'm always getting compilation error or
        // I can't achieve the behavior I'm look for.
}, v);

Important : I'm using Visual Studio 2015 Update 3 and GCC 4.9.1

Ref: http://en.cppreference.com/w/cpp/utility/variant/visit

mpark commented 6 years ago

In order for this technique to work, you need if constexpr, which you don't have prior to C++17.

erakis commented 6 years ago

@mpark , thank you for you time.

So without VS 2017 I'm stuck using functor ? But If I use lambda, it will be WITHOUT types validation at compile time ?

SaadAttieh commented 6 years ago

For C++14, You have a few options. If you want to pattern match directly on types, you can create a class that does the type dispatch (at compile time of course). So usage is like this:

mpark::visit(overloaded([](int a) { cout << a / 2 << endl; }, 
                        [](string& a) { cout << a + "\n"
                                             << endl; },
                        [](auto&) {
                            cout << “Catch all\n" << endl;
                        }), someVariant);

Simply remove the catch all case i.e. the [] (auto&) and you will get a compile time error if you don’t have a function for every type of the variant. A basic implementation of the overloaded class is below, it’s got some issues as I just hammered it out. For example it will only accept R-values meaning that you cannot pass in a variable that is assigned to the lambda, you must declare the lambda inline instead as shown above.

You can also make another class something like staticIf that would allow you to have more complex pattern matches than just type matching. This would allow conditions like std::is_base_of to be tested at compile time. I haven’t included it here but lmk me if you are interested. It would be something like staticIf(lambda) .otherwiseIf(lambda) .otherwise(lambda);

Here’s the overloaded class.

template struct OverLoaded;

template struct OverLoaded : F1 { using F1::operator();

OverLoaded(F1&& head) : F1(std::forward<F1>(head)) {}

};

template <class F1, class... Fs> struct OverLoaded<F1, Fs...> : F1, OverLoaded { using F1::operator(); using OverLoaded::operator();

OverLoaded(F1&& head, Fs&&... rest)
    : F1(std::forward<F1>(head)),
      OverLoaded<Fs...>(std::forward<Fs>(rest)...) {}

};

template OverLoaded overloaded(Fs&&... fs) { return OverLoaded(std::forward(fs)...); }

On 29 Mar 2018, at 13:10, Martin Beaudet notifications@github.com wrote:

@mpark https://github.com/mpark , thank you for you time.

So without VS 2017 I'm stuck using functor ? But If I use lambda, it will be WITHOUT types validation at compile time ?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/mpark/variant/issues/44#issuecomment-377215264, or mute the thread https://github.com/notifications/unsubscribe-auth/AFrgCpag4Pw_79QxLXBfQ-EGl0jkIpT5ks5tjM8mgaJpZM4S_WGi.

erakis commented 6 years ago

@SaadAttieh thank you.

I tried with

template <class... Fs>
struct OverLoaded;

template <class F1>
struct OverLoaded<F1> : F1 {
    using F1::operator();

    OverLoaded(F1&& head) : F1(std::forward<F1>(head)) {}
};

template <class F1, class... Fs>
struct OverLoaded<F1, Fs...> : F1, OverLoaded<Fs...> {
    using F1::operator();
    using OverLoaded<Fs...>::operator();

    OverLoaded(F1&& head, Fs&&... rest)
        : F1(std::forward<F1>(head)),
        OverLoaded<Fs...>(std::forward<Fs>(rest)...) {}
};

template <class... Fs>
OverLoaded<Fs...> overloaded(Fs&&... fs) {
    return OverLoaded<Fs...>(std::forward<Fs>(fs)...);
}
typedef mpark::variant<
    int8_t, uint8_t,
    double,
    std::string
> VariantValue;

VariantValue v((double)2.0);

Doing this compile fine and work great.

mpark::visit(overloaded(
    [](int a) { std::cout << a / 2 << std::endl; },
    [](std::string& a) { std::cout << a + "\n" << std::endl; }//,
    [](auto&) { std::cout << "Catch all\n" << std::endl; }
), v);

But this compile fine too but I'm getting a warning instead of a compilation error.

mpark::visit(overloaded(
    [](int a) { std::cout << a / 2 << std::endl; },
    [](std::string& a) { std::cout << a + "\n" << std::endl; }//,
    //[](auto&) { std::cout << "Catch all\n" << std::endl; }
), v);

The warning

1>d:\xxx\include\mpark\lib.hpp(237): warning C4244: 'argument': conversion from 'double' to 'int', possible loss of data
1>  d:\xxx\include\mpark\variant.hpp(684): note: see reference to function template instantiation 'void mpark::lib::cpp17::invoke<_Ty,T&>(F &&,T &)' being compiled
1>          with
1>          [
1>              _Ty=OverLoaded<main::<lambda_31e307beed92b69987871195c711937f>,main::<lambda_d72897b211ce01db93a67d525626e679>>,
1>              T=double,
1>              F=OverLoaded<main::<lambda_31e307beed92b69987871195c711937f>,main::<lambda_d72897b211ce01db93a67d525626e679>>
1>          ]
...
SaadAttieh commented 6 years ago

I believe this is the beautiful part of C++ that inherits a lot of C behaviour :). It annoyingly allows implicit conversions (all be it with a warning) from int to unsigned int.

If you are not aware doing something like int a; unsigned int b = a; tells the compiler to create a temporary unsigned int value, convert that int to the unsigned value and then store that temporary value in b.

In your specific case, the simplest fix is to make it take the int by reference. This is because an int reference cannot bind to a temporary value. Hence no implicit conversion.

mpark::visit(overloaded( [](int& a) { std::cout << a / 2 << std::endl; }, [](std::string& a) { std::cout << a + "\n" << std::endl; }//, { std::cout << "Catch all\n" << std::endl; } ), v);

now this should invoke the catch all case. And if you remove that catch all case you will get a compile time error. Just be careful though, if one day you try a const reference so [] (const int& a) then this unfortunately will still allow the implicit conversion as const reference is able to bind to a temporary.

This is a general issue with having multiple number types in a variant. As I said, the alternative is using the static if I previously demonstrated, it is closest to the if constexpr () mechanism described by @mpark. It’s just a bit messy so have chosen not to try producing an implementation. If you need it I will resurrect it from an old project. It’s functionality remember is something like

//method of removing references and const, etc from types

typename T > using BaseType =
    typename std::remove_cv<typename std::remove_reference<T>::type>::type;

mpark::visit([] (auto& a) {
    return staticIf < std::is_same < int,
           BaseType<decltype(a)>([](auto& a) { /* a is int*/; })
                   .otherwiseIf < std::is_same < double,
           BaseType<decltype(a)>([](auto& a) { /* a is double*/ })(a);
), variant);

On 29 Mar 2018, at 19:43, Martin Beaudet notifications@github.com wrote:

@SaadAttieh https://github.com/SaadAttieh thank you.

I tried with

template struct OverLoaded;

template struct OverLoaded : F1 { using F1::operator();

OverLoaded(F1&& head) : F1(std::forward<F1>(head)) {}

};

template <class F1, class... Fs> struct OverLoaded<F1, Fs...> : F1, OverLoaded { using F1::operator(); using OverLoaded::operator();

OverLoaded(F1&& head, Fs&&... rest)
    : F1(std::forward<F1>(head)),
    OverLoaded<Fs...>(std::forward<Fs>(rest)...) {}

};

template OverLoaded overloaded(Fs&&... fs) { return OverLoaded(std::forward(fs)...); } typedef mpark::variant< int8_t, uint8_t, double, std::string

VariantValue;

VariantValue v((double)2.0); Doing this compile fine and work great.

mpark::visit(overloaded( [](int a) { std::cout << a / 2 << std::endl; }, [](std::string& a) { std::cout << a + "\n" << std::endl; }//, { std::cout << "Catch all\n" << std::endl; } ), v); But this compile fine too but I'm getting a warning instead of a compilation error.

mpark::visit(overloaded( [](int a) { std::cout << a / 2 << std::endl; }, [](std::string& a) { std::cout << a + "\n" << std::endl; }//, // { std::cout << "Catch all\n" << std::endl; } ), v); The warning

1>d:\xxx\include\mpark\lib.hpp(237): warning C4244: 'argument': conversion from 'double' to 'int', possible loss of data 1> d:\xxx\include\mpark\variant.hpp(684): note: see reference to function template instantiation 'void mpark::lib::cpp17::invoke<_Ty,T&>(F &&,T &)' being compiled 1> with 1> [ 1> _Ty=OverLoaded<main::,main::>, 1> T=double, 1> F=OverLoaded<main::,main::> 1> ] ... — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mpark/variant/issues/44#issuecomment-377333877, or mute the thread https://github.com/notifications/unsubscribe-auth/AFrgCs5gTK56Bvv9YjNu4dQlooucSJB6ks5tjStogaJpZM4S_WGi.

SaadAttieh commented 6 years ago

p.s. in my first lines I meant unsigned int b = a. means create a temporary unsigned int value, convert the value of the signed int to unsigned…

On 30 Mar 2018, at 02:23, Saad Attieh saad_attieh@me.com wrote:

I believe this is the beautiful part of C++ that inherits a lot of C behaviour :). It annoyingly allows implicit conversions (all be it with a warning) from int to unsigned int.

If you are not aware doing something like int a; unsigned int b = a; tells the compiler to create a temporary unsigned int value, convert that int to the unsigned value and then store that temporary value in b.

In your specific case, the simplest fix is to make it take the int by reference. This is because an int reference cannot bind to a temporary value. Hence no implicit conversion.

mpark::visit(overloaded( [](int& a) { std::cout << a / 2 << std::endl; }, [](std::string& a) { std::cout << a + "\n" << std::endl; }//, { std::cout << "Catch all\n" << std::endl; } ), v);

now this should invoke the catch all case. And if you remove that catch all case you will get a compile time error. Just be careful though, if one day you try a const reference so [] (const int& a) then this unfortunately will still allow the implicit conversion as const reference is able to bind to a temporary.

This is a general issue with having multiple number types in a variant. As I said, the alternative is using the static if I previously demonstrated, it is closest to the if constexpr () mechanism described by @mpark. It’s just a bit messy so have chosen not to try producing an implementation. If you need it I will resurrect it from an old project. It’s functionality remember is something like

//method of removing references and const, etc from types

typename T > using BaseType =
    typename std::remove_cv<typename std::remove_reference<T>::type>::type;

mpark::visit([] (auto& a) {
    return staticIf < std::is_same < int,
           BaseType<decltype(a)>([](auto& a) { /* a is int*/; })
                   .otherwiseIf < std::is_same < double,
           BaseType<decltype(a)>([](auto& a) { /* a is double*/ })(a);
), variant);

On 29 Mar 2018, at 19:43, Martin Beaudet <notifications@github.com mailto:notifications@github.com> wrote:

@SaadAttieh https://github.com/SaadAttieh thank you.

I tried with

template struct OverLoaded;

template struct OverLoaded : F1 { using F1::operator();

OverLoaded(F1&& head) : F1(std::forward<F1>(head)) {}

};

template <class F1, class... Fs> struct OverLoaded<F1, Fs...> : F1, OverLoaded { using F1::operator(); using OverLoaded::operator();

OverLoaded(F1&& head, Fs&&... rest)
    : F1(std::forward<F1>(head)),
    OverLoaded<Fs...>(std::forward<Fs>(rest)...) {}

};

template OverLoaded overloaded(Fs&&... fs) { return OverLoaded(std::forward(fs)...); } typedef mpark::variant< int8_t, uint8_t, double, std::string

VariantValue;

VariantValue v((double)2.0); Doing this compile fine and work great.

mpark::visit(overloaded( [](int a) { std::cout << a / 2 << std::endl; }, [](std::string& a) { std::cout << a + "\n" << std::endl; }//, { std::cout << "Catch all\n" << std::endl; } ), v); But this compile fine too but I'm getting a warning instead of a compilation error.

mpark::visit(overloaded( [](int a) { std::cout << a / 2 << std::endl; }, [](std::string& a) { std::cout << a + "\n" << std::endl; }//, // { std::cout << "Catch all\n" << std::endl; } ), v); The warning

1>d:\xxx\include\mpark\lib.hpp(237): warning C4244: 'argument': conversion from 'double' to 'int', possible loss of data 1> d:\xxx\include\mpark\variant.hpp(684): note: see reference to function template instantiation 'void mpark::lib::cpp17::invoke<_Ty,T&>(F &&,T &)' being compiled 1> with 1> [ 1> _Ty=OverLoaded<main::,main::>, 1> T=double, 1> F=OverLoaded<main::,main::> 1> ] ... — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mpark/variant/issues/44#issuecomment-377333877, or mute the thread https://github.com/notifications/unsubscribe-auth/AFrgCs5gTK56Bvv9YjNu4dQlooucSJB6ks5tjStogaJpZM4S_WGi.

erakis commented 6 years ago

Initially the variant I'm using it's this one

typedef mpark::variant<
    bool,
    int8_t, uint8_t,
    int16_t, uint16_t,
    int32_t, uint32_t,
    int64_t, uint64_t,
    float, double,
    std::string
> VariantValue;

I ask my question without putting all types in the variant because I though the 3 initial one was enough to expose my problem.

If I'm using reference instead of value on Visual Studio 2015 Update 3, I'm still not getting compilation error when commenting the catch all instruction.


VariantValue v((double)2.0);

mpark::visit(overloaded(
    [](const uint8_t& a) { std::cout << a / 2 << std::endl; },
    [](const std::string& a) { std::cout << a + "\n" << std::endl; }//,
    //[](auto&) { std::cout << "Catch all\n" << std::endl; }
), v);

// I'm still getting the same warning about 
// Warning C4244    'argument': conversion from 'double' to 'const uint8_t', possible loss of data

I think I will have to go with the functor and forget about lambda until I get compliant C++17 compiler. As I'm using an old version of Yocto and I cannot upgrade to a newer version easily.

SaadAttieh commented 6 years ago

I’m sorry, not sure why, I have less experience with that platform. On clang at the moment I get g8afbcf97:~ SaadAttieh$ g++ -std=c++14 -Wextra -Wall test.cpp In file included from test.cpp:3: ./variant/include/mpark/variant.hpp:674:11: error: static_assert failed "mpark::visit requires the visitor to be exhaustive." My code:

template struct OverLoaded;

template struct OverLoaded : F1 { using F1::operator();

OverLoaded(F1&& head) : F1(std::forward<F1>(head)) {}

};

template <class F1, class... Fs> struct OverLoaded<F1, Fs...> : F1, OverLoaded { using F1::operator(); using OverLoaded::operator();

OverLoaded(F1&& head, Fs&&... rest)
    : F1(std::forward<F1>(head)),
      OverLoaded<Fs...>(std::forward<Fs>(rest)...) {}

};

template OverLoaded overloaded(Fs&&... fs) { return OverLoaded(std::forward(fs)...); }

typedef mpark::variant<bool, int8_t, uint8_t, int16_t, uint16_t, int32_t, uint32_t, int64_t, uint64_t, float, double, std::string> VariantValue;

int main() { VariantValue a = 1.0; mpark::visit(overloaded([](int& a) { std::cout << a / 2 << std::endl; }, [](std::string& a) { std::cout << (a + "\n") << std::endl; }), a); } and I get:

On 30 Mar 2018, at 12:59, Martin Beaudet notifications@github.com wrote:

Initially the variant I'm using it's this one

typedef mpark::variant< bool, int8_t, uint8_t, int16_t, uint16_t, int32_t, uint32_t, int64_t, uint64_t, float, double, std::string

VariantValue; I ask my question without putting all types in the variant because I though the 3 initial one was enough to expose my problem.

If I'm using reference instead of value on Visual Studio 2015 Update 3, I'm still not getting compilation error when commenting the catch all instruction.

VariantValue v((double)2.0);

mpark::visit(overloaded( [](const uint8_t& a) { std::cout << a / 2 << std::endl; }, [](const std::string& a) { std::cout << a + "\n" << std::endl; }//, // { std::cout << "Catch all\n" << std::endl; } ), v);

// I'm still getting the same warning about // Warning C4244 'argument': conversion from 'double' to 'const uint8_t', possible loss of data I think I will have to go with the functor and forget about lambda until I get compliant C++17 compiler. As I'm using an old version of Yocto https://www.yoctoproject.org/ and I cannot upgrade to a newer version easily.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mpark/variant/issues/44#issuecomment-377510002, or mute the thread https://github.com/notifications/unsubscribe-auth/AFrgCjRvj43yy4CZGCbLynzQ4EGSbl83ks5tjh4egaJpZM4S_WGi.

mpark commented 6 years ago

@SaadAttieh: Thanks for helping out with this!

@purell: @SaadAttieh's solution isn't working for you because you're taking const int& rather than int&. const int& will take a double whereas int& would not. But this solution only works if you're always visiting l-value variants, and you're forced to forgo const correctness.

If you really want to handle the exact types without implicit conversions, you probably want a different abstraction than visit. For example, you could introduce a type_switch like this:

  mpark::variant<int, double> v = 1.1, w = 101;

  // single visitation  
  type_switch(v)(
    [](type<int>, auto&& x) { std::cout << "int: " << x << '\n'; },
    [](type<double>, auto&& x) { std::cout << "double: " << x << '\n'; });

  // double visitation
  type_switch(v, w)(
    [](type<int, int>, auto&& x, auto&& y) {
      std::cout << "int, int: " << x << ' ' << y << '\n';
    },
    [](type<double, int>, auto&& x, auto&& y) {
      std::cout << "double, int: " << x << ' ' << y << '\n';
    },
    [](type<int, double>, auto&& x, auto&& y) {
      std::cout << "int, double: " << x << ' ' << y << '\n';
    },
    [](type<double, double>, auto&& x, auto&& y) {
      std::cout << "double, double: " << x << ' ' << y << '\n';
    });

Given template <typename... Ts> struct type {}; and an implementation of overload, type_switch can be implemented like this:

template <typename... Vs>
auto type_switch(Vs&&... vs) {  // take the variants
  return [&](auto&&... fs) {  // take the cases
    mpark::visit([&](auto&&... xs) {  // visit the variants, get the values
      return overload(std::forward<decltype(fs)>(fs)...)(  // overload the cases
        type<std::decay_t<decltype(xs)>...>{},  // perform tag-dispatching with `type`
        std::forward<decltype(xs)>(xs)...);  // forward the values
    }, std::forward<Vs>(vs)...);
  };
}

Example on Wandbox