pass rvalue reference to visitor when visiting an rvalue variant

[lightmare]

Fixes #142 A note about the admittedly awkward syntax std::move(var).match(...)

This branch includes a change to apply_visitor which I forgot to put in a separate commit. Here's the deal:

mapbox::util::variant<int> v{42};
auto f = mapbox::util::make_visitor(
    [](int&) { std::cout << "lvalue\n"; },
    [](int&&) { std::cout << "rvalue\n"; });

mapbox::util::apply_visitor(f, v); // should print "lvalue"
mapbox::util::apply_visitor(f, std::move(v)); // should print "rvalue"

v.match(f); // what should this print?

Now, if you wanted this to work:

v.match([](int&&) { ... });

... you'd need to either a) delegate to visit(std::move(*this), fn), which might have surprising behaviour (using the visitor from the above example, v.match(f) would print "rvalue"); or b) implement special logic for this case that'd only std::move(stored_alternative) if the visitor doesn't accept it by lvalue

a) is basically a design decision, and b) sounds non-trivial; so in order to keep it simple, I chose to do neither. Thence the required std::move(var).match(...) syntax.

[lightmare]

Noticed I created this PR with the wrong target branch, and some test failures caused by changed diagnostics messages. I've rebased the branch, moved changes to apply_visitor to a separate commit, and updated expected compilation failure messages.

There are two failing tests remaining:

• clang 3.7 with an obscure ICE in Catch, I have no idea what's going on
• gcc 4.7 which doesn't support ref-qualified member functions; I'd ditch it

[springmeyer]

👍 to ditching those versions @lightmare - as far as minimum compilers I think we should test on/maintain support for:

• gcc 4.9
• clang++ 5

[lightmare]

Ad clang 3.7 ICE: that error is unrelated to this PR, and to variant code altogether. I've found another, older PR build failing in exactly the same way when compiling test/unit.cpp which only includes catch.hpp, nothing else.

[artemp]

@lightmare https://github.com/mapbox/variant/pull/180#issuecomment-656568987

[artemp]

@lightmare - explicit return type in visitor functor is useful in some circumstances. So looks like we need some SFINAE logic deducting return type:

diff --git a/include/mapbox/variant.hpp b/include/mapbox/variant.hpp
index e92f00f..7aedbcd 100644
--- a/include/mapbox/variant.hpp
+++ b/include/mapbox/variant.hpp
@@ -195,11 +195,35 @@ struct copy_cvref<Src&&, Dest>
     using type = Dest&&;
 };

+template <typename T, typename R = void>
+struct enable_if_type
+{
+    using type = R;
+};
+
+template <typename F, typename Enable = void>
+struct return_type_fun
+{
+    using type = void;
+    static constexpr bool value = false;
+};
+
+template <typename F>
+struct return_type_fun<F, typename enable_if_type<typename F::result_type>::type>
+{
+    using type = typename F::result_type;
+    static constexpr bool value = true;
+};
+
 template <typename F, typename T>
-using result_of_unary_visit = decltype(std::declval<F>()(std::declval<T>()));
+using result_of_unary_visit = typename std::conditional<return_type_fun<F>::value,
+                                                        typename return_type_fun<F>::type,
+                                                        decltype(std::declval<F>()(std::declval<T>()))>::type;

 template <typename F, typename T>
-using result_of_binary_visit = decltype(std::declval<F>()(std::declval<T>(), std::declval<T>()));
+using result_of_binary_visit = typename std::conditional<return_type_fun<F>::value,
+                                                         typename return_type_fun<F>::type,
+                                                         decltype(std::declval<F>()(std::declval<T>(), std::declval<T>()))>::type;

 template <type_index_t arg1, type_index_t... others>
 struct static_max;

/cc @springmeyer

[artemp]

https://github.com/mapbox/variant/pull/180/commits/b1076bbee3857a325288dcf8658b66d2063ade66

[lightmare]

I agree that explicit result type may be useful. But I'm not entirely sure whether the above implementation with both explicit and deduced type in std::conditional doesn't defeat one of the benefits -- namely avoiding the guesswork required to deduce the return type. Maybe it doesn't touch the second type at all if the condition is true, I'm really not sure. Anyway, here's an alternative implementation that 100% doesn't try to deduce anything if there is result_type:

template <typename F, typename = void>
struct deduced_result_type
{};

template <typename F, typename... Args>
struct deduced_result_type<F(Args...), decltype((void)std::declval<F>()(std::declval<Args>()...))>
{
    using type = decltype(std::declval<F>()(std::declval<Args>()...));
};

template <typename F, typename = void>
struct visitor_result_type : deduced_result_type<F>
{};

// specialization for explicit result_type member in visitor class
template <typename F, typename... Args>
struct visitor_result_type<F(Args...), decltype((void)std::declval<typename std::decay<F>::type::result_type>())>
{
    using type = typename std::decay<F>::type::result_type;
};

template <typename F, typename T>
using result_of_unary_visit = typename visitor_result_type<F&&(T&&)>::type;

template <typename F, typename T>
using result_of_binary_visit = typename visitor_result_type<F&&(T&&, T&&)>::type;

Note: decltype((void)std::declval<typename std::decay<F>::type::result_type>()) could be contracted to void_t<std::decay_t<F>::result_type>, but I didn't want to C++11-mimic void_t just for that.

Here it is with a test, which was apparently missing: https://github.com/mapbox/variant/commit/0ebf09deaec67318c9fb2595dcdf12826197c245

[artemp]

^^ Cool, I'll check it out, thanks @lightmare !