Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

C++ different minmax implementation

Tags:

c++

c++11

stl

As You may (not) know using std::minmax with auto and temporary arguments may be dangerous. Following code for example is UB because std::minmax returns pair of references, not values:

auto fun(){
    auto res = std::minmax(3, 4);
    return res.first;
}    

I would like to ask if there is possibility to make std::minmax function act safely or at least safer without any overhead? I came up with a solution like this, but I am not completely sure if it is equivalent to current minmax as generated assembly is different for stl-like implementation and mine. So the question is: what are the possible problems/drawbacks of my implementation of minmax in relation to std-like one:

//below is std-like minmax
template< class T > 
constexpr std::pair<const T&,const T&> std_minmax( const T& a, const T& b ){
    return (b < a) ? std::pair<const T&, const T&>(b, a)
            : std::pair<const T&, const T&>(a, b);
}

//below is my minmax implementation
template< class T > 
constexpr std::pair<T, T> my_minmax( T&& a, T&& b ){
    return (b < a) ? std::pair<T, T>(std::forward<T>(b), std::forward<T>(a))
            : std::pair<T, T>(std::forward<T>(a), std::forward<T>(b));
}

Live demo at godbolt.org


As some of You claim it is unclear what I am asking, I would like to reword a bit what I want. I would like to write function which works exactly like std::minmax, but if given one temporary value - returns std::pair<T, T> instead of std::pair<const T &, const T &>. Secondly, while doing it I would like to avoid any unnecessary moving, copying of data and so forth.

like image 484
bartop Avatar asked Sep 26 '18 07:09

bartop


3 Answers

I am not exactly sure what are you trying to achieve. You wrote:

without any overhead

but your solution will copy lvalue arguments. Is it what you want?

Anyway, you cannot use two forwarding references with the same template parameter this way, since it will fail if both function arguments have different categories:

template <typename T> void f(T&& a, T&& b) { }

int main() {
  int a = 3;
  f(a, 1);  // error: template argument deduction/substitution failed
}

For the first function argument, T would be deduced as int&, and for second as int.


If you want to remove any copying, the only possibility is the member of the resulting pair to be:

  1. a (const) lvalue reference to the corresponding function argument in case it is an lvalue,

  2. a value moved from that argument if is it an rvalue.

I don't think this is possible to achieve. Consider:

std::string a("hello");
auto p = minmax(a, std::string("world"));

Here the resulting type would be std::pair<std::string&, std::string>. However, in case of

auto p = minmax(a, std::string("earth"));

the resulting type would be different, namely std::pair<std::string, std::string&>.

Therefore, the resulting type would depend on a runtime condition (which generally requires runtime polymorphism).


UPDATE

Out of curiosity, I just came up with a wrapper that can hold some object either by (const) pointer or by value:

template <typename T>
class val_or_ptr {
   std::variant<T, const T*> v_;
public:
   val_or_ptr(const T& arg) : v_(&arg) { }
   val_or_ptr(T&& arg) : v_(std::move(arg)) { }
   const T& get() const { return v_.index() ? *std::get<const T*>(v_) : std::get<T>(v_); }
};

With that, you can define minmax as:

template <typename T, typename U,
          typename V = std::enable_if_t<std::is_same_v<std::decay_t<T>, std::decay_t<U>>, std::decay_t<T>>>
std::pair<val_or_ptr<V>, val_or_ptr<V>> minmax(T&& a, U&& b) {
   if (b < a) return { std::forward<U>(b), std::forward<T>(a) };
   else return { std::forward<T>(a), std::forward<U>(b) };
}

Live demo is here: https://wandbox.org/permlink/N3kdI4hzllBGFWVH

This is very basic implementation, but it should prevent copying both from lvalue and rvalue arguments of minmax.

like image 81
Daniel Langr Avatar answered Nov 13 '22 05:11

Daniel Langr


One solution is when T is an r-value reference then copy it instead of returning an r-value reference:

#include <utility>

template<class T>
std::pair<T, T> minmax(T&& a, T&& b) {
    if(a < b)
        return {a, b};
    return {b, a};
}

When the argument is an r-value reference T is deduced as a non-reference type:

int main() {
    int a = 1;
    int const b = 2;
    minmax(1, 1); // std::pair<int, int>
    minmax(a, a); // std::pair<int&, int&>
    minmax(b, b); // std::pair<const int&, const int&>
}
like image 44
Maxim Egorushkin Avatar answered Nov 13 '22 06:11

Maxim Egorushkin


With C++17 it is possible to use constexpr if to tie lvalue args and copy everything else. With C++11 I would probably think twice before building an angle brackets moster with a scary look for such a simple use case.

godbolt, coliru

template <typename T>
decltype(auto) minmax(T&& x, T&& y)
{
    if constexpr(std::is_lvalue_reference_v<decltype(x)>)
        return std::minmax(std::forward<T>(x), std::forward<T>(y));
    else {
        auto const res = std::minmax(x, y);
        return std::make_pair(res.first, res.second);
    }
}

To support mixed l/r values you probably need two template params, 4 cases in the if/else, and std::cref(res.xxx) as an argument to std::make_pair for partial.

like image 1
bobah Avatar answered Nov 13 '22 05:11

bobah