Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Why not simply use recursive if else? #61

Closed
yuqian90 opened this issue Mar 13, 2019 · 1 comment
Closed

Why not simply use recursive if else? #61

yuqian90 opened this issue Mar 13, 2019 · 1 comment

Comments

@yuqian90
Copy link

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
Copy link
Owner

mpark commented Mar 14, 2019

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

@mpark mpark closed this as completed Mar 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants