How to fix this void_t issue with templated class?

Question

I have code like this:

#include <type_traits>

struct S1{};

struct S2{
    void rbegin(){}
    void rend(){}
};

template<typename S>
struct S3{
    void rbegin(){
        S().rbegin();
    }

    void rend(){
        S().rend();
    }
};

template<typename, typename = void>
constexpr bool is_reverse_iterable = false;
 
template<typename T>
constexpr bool is_reverse_iterable<
                T,
                std::void_t<
                    decltype(std::declval<T>().rbegin()),
                    decltype(std::declval<T>().rend())
                >
> = true;

#include <iostream>

int main(){
    std::cout << is_reverse_iterable<S1> << '\n'; // print 0, correct
    std::cout << is_reverse_iterable<S2> << '\n'; // print 1, correct

    std::cout << is_reverse_iterable<S3<S1> > << '\n'; // print 1, wrong !!!
    std::cout << is_reverse_iterable<S3<S2> > << '\n'; // print 1, correct

//  S3<S1>().rbegin(); // not compiles
    S3<S2>().rbegin();
}

Example is copy / paste from cppreference.com website, however, is_reverse_iterable<S3<S1> > is set to true and this is wrong.

How can I fix this?

Godbolt link: https://gcc.godbolt.org/z/qjG46EWsq

Answer 1

The methods of S3 are not SFINAE-friendly. (During SFINAE, the compiler won't look inside the function body for substitution errors. It'll either ignore them if the function body isn't needed yet, or fail with a hard error on them if the body is needed, e.g. if the return type is auto.)

In C++20 you would fix them like this:

template <typename S>
struct S3
{
    void rbegin() requires requires{S().rbegin();}
    // or: void rbegin() requires is_reverse_iterable<S>
    {
        S().rbegin();
    }

    void rend() requires requires{S().rend();}
    {
        S().rend();
    }
};

In C++17 you need to do something like this:

template <typename S, typename = void>
struct S3_Base {};

template <typename S>
struct S3_Base<S, std::enable_if_t<is_reverse_iterable<S>>>
{
    void rbegin()
    {
        S().rbegin();
    }

    void rend()
    {
        S().rend();
    }
};

template <typename S>
struct S3 : S3_Base<S>
{};

Another C++17 option:

template <typename S>
struct S3
{
    template <typename SS = S, std::enable_if_t<std::is_same_v<S, SS>, decltype(void(SS().rbegin()), nullptr)> = nullptr>
    // or template <typename SS = S, std::enable_if_t<std::is_same_v<S, SS> && is_reverse_iterable<SS>, std::nullptr_t> = nullptr>
    void rbegin()
    {
        S().rbegin();
    }

    template <typename SS = S, std::enable_if_t<std::is_same_v<S, SS>, decltype(void(SS().rend()), nullptr)> = nullptr>
    void rend()
    {
        S().rend();
    }
};

Answer 2

It's not wrong, you declare the functions uncondionally. The fact the their definition will be ill-formed doesn't change the fact their declarations always exist. SFINAE (what void_t works with) only deals with the existence of declarations.

If you want S3 to report correctly, you need to make it SFINAE friendly. For example:

template<typename S>
struct S3{
    template<typename T = S, decltype(std::declval<T>().rbegin(), 0) = 0>
    void rbegin(){
        S().rbegin();
    }

    template<typename T = S, decltype(std::declval<T>().rend(), 0) = 0>
    void rend(){
        S().rend();
    }
};

Now both members only participate in overload resolution if the wrapped type can make the call. This is of course only one way to (ab)use additional template definitions to make class definitions SFINAE friendly.