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

Why not simply use recursive if else? #61

Closed yuqian90 closed 5 years ago

yuqian90 commented 5 years ago

This is not an issue, it's a question. I want to ask, why does mpark::variant not adopt a simple implementation using recursive if else?

I hacked up something like this (no error checking, not carefully tested). It can be made to work with single-visitation. I think it's easy to extend to multi-visitation too. I did some benchmark on visit2() using the benchmark problem provided on the author's blog https://mpark.github.io/programming/2019/01/22/variant-visitation-v2/

I can't seem to tell any difference between mpark::visit() and visit2() in terms of performance (both compile time and runtime) when -O3 optimization is turned on. (i only tried with clang++6.0).

I'm asking because the switch case implementation in mpark::variant looks great but it is complicated. I wonder what's wrong with simple recursive if-else like this.

namespace mpark{

namespace detail{ namespace visitation{

    template<std::size_t N>
    struct visit2_impl
    {
        template<typename Visitor, typename V>
        static inline decltype(auto) visit(Visitor&& vi, V&& v)
        {
            constexpr std::size_t I = variant_size<lib::decay_t<V>>::value - 1 - N;
            if(v.index() == I)
                return lib::invoke(lib::forward<Visitor>(vi), get<I>(v));
            else
                return visit2_impl<N - 1>::visit(lib::forward<Visitor>(vi), lib::forward<V>(v));
        }
    };

    template<>
    struct visit2_impl<0>
    {
        template<typename Visitor, typename V>
        static inline decltype(auto) visit(Visitor&& vi, V&& v)
        {
            constexpr std::size_t I = variant_size<lib::decay_t<V>>::value - 1;
            return lib::invoke(lib::forward<Visitor>(vi), get<I>(v));
        }
    };

}}

template <typename Visitor, typename V>
inline constexpr decltype(auto) visit2(Visitor &&visitor, V&& v) {
        return  (detail::all({!v.valueless_by_exception()})
                    ? (void)0
                    : throw_bad_variant_access()),
                detail::visitation::visit2_impl<variant_size<lib::decay_t<V>>::value - 1>::visit(
                    lib::forward<Visitor>(visitor),
                    lib::forward<V>(v));
}

}
mpark commented 5 years ago

My observation is that GCC for example does not optimize as well as Clang does for the recursive if-else chain. I had observed this via the chain of ternary operators approach in #56, but here's a concrete example with your code: https://godbolt.org/z/SsbjYZ