3

With all the new features of C++ (C++11 suffices I think), what prevents from having a std::minmax function that returns a pair of references.

In this way, if one feeds two modifable references, they can be modified. Is this opening a can of worms?

#include<functional>
// maybe all this options can be simplified
template<class T1, class T2> struct common;
template<class T> struct common<T, T>{using type = T;};
template<class T> struct common<T const&, T&>{using type = T const&;};
template<class T> struct common<T&, T const&>{using type = T const&;};
template<class T> struct common<T, T&>{using type = T const&;};
template<class T> struct common<T&, T>{using type = T const&;};
template<class T> struct common<T const&, T>{using type = T const&;};
template<class T> struct common<T, T const&>{using type = T const&;};

template<class T1, class T2, class Compare = std::less<>, class Ret = typename common<T1, T2>::type> 
std::pair<Ret, Ret> minmax(T1&& a, T2&& b, Compare comp = {}){
    return comp(b, a) ? 
        std::pair<Ret, Ret>(std::forward<T2>(b), std::forward<T1>(a))
        : std::pair<Ret, Ret>(std::forward<T1>(a), std::forward<T2>(b));
}

Test:

#include<cassert>
int main(){
    {
    int a = 1;
    int b = 10;
    auto& small = minmax(a, b).first;
    assert(small == 1);
    small += 1;
    assert(a == 2);
    }{
    int const a = 1;
    int b = 10;
    auto& small = minmax(a, b).first;
    assert(small == 1);
//    small += 1; error small is const reference, because a was const
    }{
    int a = 1;
    int const b = 10;
    auto& small = minmax(a, b).first;
    assert(small == 1);
//    small += 1; error small is const reference, because a was const
    }{
    int const a = 1;
    int const b = 10;
    auto& small = minmax(a, b).first;
    assert(small == 1);
//    small += 1; error small is const reference, because a was const
    }{
    int b = 10;
    auto& small = minmax(int(1), b).first;
    assert(small == 1);
//   small += 1; error small is const reference, because first argument was const
    }{
    int a = 1;
    auto& small = minmax(a, int(10)).first;
    assert(small == 1);
//   small += 1; error small is const reference, because second argument was const
    }
    {
    int const a = 1;
    auto& small = minmax(a, int(10)).first;
    assert(small == 1);
//    small += 1; error small is const reference, because both arguments are const
    }
    {
//    auto& small = minmax(int(1), int(10)).first; // error, not clear why
    auto const& small = minmax(int(1), int(10)).first; // ok
//    auto small2 = minmax(int(1), int(10)).first; // also ok
    assert(small == 1);
//    small += 1; error small is const reference, because both arguments are const
    }
}
alfC
  • 14,261
  • 4
  • 67
  • 118
  • Did you intend to put this in a proposal for the C++ committee? Because I think that may be a better audience for such a proposal than stackoverflow. – dascandy Aug 16 '18 at 08:13

1 Answers1

1

There was a paper kind of along these lines a long time ago, by Howard Hinnant: N2199. Its very opening example demonstrates the precise problem you're trying to solve:

The function can not be used on the left hand side of an assignment:

int x = 1;
int y = 2;
std::min(x, y) = 3;  // x == 3 desired, currently compile time error

It goes on to list as examples the frequently dangling reference problem, mixing types, and being useful with move-only types, and goes on to propose new versions of min and max that address all of these problems - it includes a very thorough implementation at the bottom (which is too long to paste here). Implementing minmax() based on that should be pretty straightforward:

template <class T, class U,
    class R = typename min_max_return<T&&, U&&>::type>
inline
std::pair<R, R>    
minmax(T&& a, U&& b)
{
    if (b < a)
        return {std::forward<U>(b), std::forward<T>(a)};
    return {std::forward<T>(a), std::forward<U>(b)};
}

The paper was rejected at the time. It's possible it could come back though.

Being able to get back mutable references is nice, but being able to avoid dangling references is even nicer. Anonymously quoting from an example I saw recently:

template<typename T> T sign(T); 

template <typename T> 
inline auto frob(T x, T y) -> decltype(std::max(sign(x - y), T(0))) { 
    return std::max(sign(x - y), T(0)); 
} 

This function has undefined behaviour for all inputs (the narrowest contract possible?).

Note that your common implementation has this problem. These cases:

template<class T> struct common<T, T&>{using type = T const&;};
template<class T> struct common<T&, T>{using type = T const&;};
template<class T> struct common<T const&, T>{using type = T const&;};
template<class T> struct common<T, T const&>{using type = T const&;};

all dangle. What this means if I have:

int i = 4;
auto result = your_minmax(i, 5);

result here is a pair<int const&, int const&>, one of which is a reference to i and the other of which dangles. All of these cases have to do using type = T; in order to be safe.

Barry
  • 286,269
  • 29
  • 621
  • 977
  • Yes, writing the body of the function is not the challenge. The challenge is to write the `common` or `mix_max_result` metafunction and contemplate all the cases. I didn't even consider all the cases and it gets messy very quickly. Do you have an alternative `common` function that will not generate dangling references or is it impossible? – alfC Aug 16 '18 at 16:57
  • I think the problem in your last example is a combination of `auto`, which instead of deducing `pair const&` and that `pair` that doesn't let the temporary survive, this is a persistent problem with `const&` members. This last is a serious inconsistency of the language and has been there forever, ultimately leaking into problems like this one. – alfC Aug 16 '18 at 16:58
  • @alfC It doesn't matter if it was `pair const&` or `pair`, the problem is the reference members of the `pair`. There's no problem with `auto` here, and I don't know what "serious inconsistency" you're talking about. – Barry Aug 16 '18 at 17:00
  • @alfC Just doing `decltype(true ? declval() : declval())` and turning rvalue references into prvalues would get you a lot of the way. But really, Howard's implementation is the right approach. – Barry Aug 16 '18 at 17:01
  • the inconsistency in the language is that you can bind a temporary to a const reference and extend the lifetime `double const& d = f();` (`f ` returns a double) and this is good. But you can not do the same with `pair p = {f(), g()}`, this unfortunately produces a dangling reference. That is the inconsistency. – alfC Aug 16 '18 at 20:36