[concept] Swappable bug & concepts::valid_expr

[jamboree]

This code fails to compile: (tested clang 3.5 & g++ 4.9.1)

#include <range/v3/utility/concepts.hpp>

int main(int argc, char** argv) {
    static_assert(ranges::v3::Swappable<int&>(), "");  
    return 0;
}

In the implementation, concepts::valid_expr is used, but swap returns void, which cannot be an arg.
Why is concepts::valid_expr needed at the first place?

[pfultz2]

Yes the concepts::valid_expr is needed for substitution failure, but I believe, its a mistake in the code. Swappable should be defined something like this:

struct Swappable
{
    template<typename T>
    auto requires_(T && t) -> decltype(
        concepts::valid_expr(
            (swap((T&&)t, (T&&)t), 42)
        ));

    template<typename T, typename U>
    auto requires_(T && t, U && u) -> decltype(
        concepts::valid_expr(
            (swap((T&&)t, (U&&)u), 42),
            (swap((U&&)u, (T&&)t), 42)
        ));
};

This will prevent the void error.

BTW, because its easy to slip and forget to add the magic 42, in my library version of this, I actually made the whole decltype(valid_expr()) a macro define like this:

struct void_ {};
template<class T> T&& operator,(T&& x, void_);

template<typename T>
int valid_expr(T &&);

#define TICK_VALID(...) decltype(tick::detail::valid_expr((__VA_ARGS__, tick::detail::void_())))

I know its an extra macro, which people seem to not like, but I just found it too easy to forget the 42. A similar thing could be added to the ranges concepts as well.

[jamboree]

@pfultz2 , I don't see how valid_expr affects sfinae, could you explain?
It just works after removing valid_expr.

[pfultz2]

It doesn't apply in this context, but it is needed when using other constructs such as convertible_to.

However, after playing around with this more, I realized that the valid_expr may not be needed at all. It seems like there is something in the way the concept::models is implemented, that is causing substitution failure to be an error for the convertible_to constructs. I wonder if the concept::models can be written in a way so that a simple decltype can be used?

[ericniebler]

Decltype on which valid expression? I use valid_expr so that each valid expression can be a separate expression as a parameter to the function, without weird interference between expressions.

[ericniebler]

And yes, Swappable is buggy. I'll fix it. Thanks for the report.

[pfultz2]

Sorry, my bad, actually decltype can be used with concepts::models, ignore that part.

Decltype on which valid expression?

I think we mean that the concept, for example, EqualityComparable could be defined simply as:

struct EqualityComparable
{
    template<typename T>
    auto requires_(T && t) -> decltype(
            concepts::convertible_to<bool>(t == t),
            concepts::convertible_to<bool>(t != t)
        );
};

I use valid_expr so that each valid expression can be a separate expression as a parameter to the function, without weird interference between expressions.

Isn't the problem with functions returning void more common than problems with types that have a strange overloaded comma operator?

Furthermore, I have yet to run into an issue with commas(perhaps, it would be really difficult to figure out if I did), but I have run into problems quite a lot with using void, thats why I eventually created a macro to abstract it away. It would be awesome if I could just replace the macro with decltype.

[ericniebler]

Isn't the problem with functions returning void more common than problems with types that have a strange overloaded comma operator?

Yes, but to write a strictly correct concept, you'll need to guard against strange comma operators, even though they're vanishingly rare. And in guarding against it, you'll make your code verbose. The use of valid_expr eliminates that verbosity. I don't see its use as particularly onerous.

[pfultz2]

Yes, but to write a strictly correct concept, you'll need to guard against strange comma operators, even though they're vanishingly rare.

Do you have an example where this would fail? I am having trouble coming up with a test case where this would fail.

[pfultz2]

Nm, I found a test case. If the return type of the function is noncopyable, and the comma operator was overloaded by value, this would incorrectly cause a substitution failure.

[pfultz2]

Ok well as a partial solution you could start with a special object with the comma overloaded to prevent other comma overloads, something like this:

struct Swappable
{
    template<typename T>
    auto requires_(T && t) -> decltype(
        comma_guard(),
        swap((T&&)t, (T&&)t)
    );
};

So the comma_guard class will overload the comma operator using a member function, since member functions are preferred:

struct comma_guard
{
    template<class T>
    const comma_guard& operator,(T&&) const;
};

So this works on gcc, but doesn't on clang. Its an ambiguous overloads on clang, which shouldn't be since the member function should be preferred. Unless I am missing something.

[pfultz2]

Ok sorry to spam the list, but I solved it now. If the comma_guard is defined like this:

struct comma_guard
{
    struct any
    {
        template<class T>
        any(T&&);
    };
    const comma_guard& operator,(any) const;
};

It will work, because clang doesn't consider it a class member when its templated.

[jamboree]

comma_guard won't work in sequence like:

comma_guard, void, strange_comma, bang

[pfultz2]

comma_guard won't work in sequence like:

Thanks, you're right. I seemed so close to a solution.

So I guess there are two ways to implement concept predicates. The first way requires using void casts for stand alone expressions:

struct Swappable
{
    template<typename T>
    auto requires_(T && t) -> decltype(
            swap((T&&)t, (T&&)t)
        );

    template<typename T, typename U>
    auto requires_(T && t, U && u) -> decltype(
            (void)swap((T&&)t, (U&&)u),
            (void)swap((U&&)u, (T&&)t)
        );
};

If the void casts are forgotten, then if someone writes swap that returns a type that has a crazy comma overloading, then this concepts fails.

So then valid_expr can be used to workaround the comma problem, but now these 42 hacks have to be used just in case the function returns void:

struct Swappable
{
    template<typename T>
    auto requires_(T && t) -> decltype(
        concepts::valid_expr(
            (swap((T&&)t, (T&&)t), 42)
        ));

    template<typename T, typename U>
    auto requires_(T && t, U && u) -> decltype(
        concepts::valid_expr(
            (swap((T&&)t, (U&&)u), 42),
            (swap((U&&)u, (T&&)t), 42)
        ));
};

Fail from forgetting the void casts will almost never happen. The fail from forgetting to put 42 will happen quite often. Both have their advantages and disadvantages. It would be nice if there was a more robust solution to this. The only robust solution it seems would be to write some preprocessor DSL to take care of this, but most people hate macros.

[jamboree]

A more robust solution may look like this:

template<typename T, typename U>
auto requires_(T && t, U && u) -> list<
        decltype(swap((T&&)t, (U&&)u)),
        decltype(swap((U&&)u, (T&&)t))
    >;

but a bit verbose, you probably want a macro for that.

[pfultz2]

but a bit verbose, you probably want a macro for that.

That is an awesome idea. It really is not anymore verbose than using void casts or adding in 42. Additionally, the convertible_to constructs could be changed to a type instead of functions(so it actually would be a constructor). So the decltype wouldn't be needed for those constructs(which are more commonly used anyways). So it may be possible to still write this:

struct Convertible
{
    template<typename T, typename U>
    auto requires_(T &&t, U &&) -> valid<
            concepts::convertible_to<U>((T &&) t)
        >;
};

I don't if substitution failure still applies in this context. Perhaps, another decltype might be needed:

struct Convertible
{
    template<typename T, typename U>
    auto requires_(T &&t, U &&) -> decltype(valid<
            concepts::convertible_to<U>((T &&) t)
        >());
};

I will try to test this out later.

[pfultz2]

Nm, decltype would still be needed, they are still expressions and not types.

[jamboree]

@pfultz2 , I believe you mean it's needed for concepts::convertible_to<U>((T &&) t), not for list or valid you called, right? for clarity, the list may be defined as:

template<class... Ts>
using list = void;

[pfultz2]

@pfultz2 , I believe you mean it's needed for concepts::convertible_to((T &&) t), not for list or valid you called, right?

Yes. I was trying to find a way to avoid the extra decltype on the convertible_to constructs(but still used for simple expressions), but it doesn't seem possible. Also, a macro could possibly help a little, but parens will be needed for each expression:

struct EqualityComparable
{
    template<typename T>
    auto requires_(T && t) -> CONCEPT_VALID(
            (concepts::convertible_to<bool>(t == t))
            (concepts::convertible_to<bool>(t != t))
        );
};

It would be nice to find a way to avoid the extra parens for at least the convertible_to constructs.