Overload rvalue and lvalue reference for template deduced type with return value and its implementation

Question

There are a lot of similar questions here on SO (e.g.: How to get different overloads for rvalue and lvalue references with a template-deduced type?), but not exactly this one. In particular, no questions are concered with value returning functions. Furthermore, I am not sure (euphemestically spoken) whether I understood the answers correctly.

---

I want to provide two versions of a function, which either return a new object or change the passed object (and return the changed object -- for consistency). For simplicity I present my code using a unique like algorithm.

User code

auto vec = std::vector<int>{1,3,5,2,4};
auto vec1 = tt::unique( vec );  
  // does not change vec and returns new vector
  // vec1 should be {1,2,3,4,5}
  // vec should be {1,3,5,2,4}

auto vec2 = tt::unique( std::move(vec) );  
  // does change vec and returns new vector vec2
  // vec2 should be {1,2,3,4,5}
  // vec should be empty

Implementation:

namespace tt {
    template< typename T > T unique( T && vec ) {
        std::sort( vec.begin(), vec.end() );
            vec.erase(
            std::unique( vec.begin(), vec.end()),
            vec.end()
        );
        return std::move( vec );
    }
    template< typename T > T unique( T & vec ) {
        return unique( T(vec) );
    }
}

To avoid code duplication the lvalue reference version calls the rvalue reference version.

My main questions:

• Is this correct?
• I think by calling the rvalue reference version from the lvalue reference version, I do one extra move.
• Apart from this extra move, do I make any other unnecessary constructions?
• Given the extra move, for very simple algorithms (oneliners), is it more efficient to duplicate the code? E.g. a sort above function I would implement as

  template< typename T > T sort( T & array ) {
      T array_ = array;
      std::sort( array_.begin(), array_.end() );
      return array_;
  }
  

  Is this efficient/correct code?

Extra questions:

• Why is this overload never used and why does is compile at all, given that c is const.

  template< typename T >
  T unique( const T && c ) {
      std::cout << "Rvalue reference unique wrapper!\n";
      return unique( T(c) );
  }
  

• Actually I would like to have const in the lvalue reference signature, i.e. something like template<typename T> T unique( const T & vec );. But, as I read, since the first is a forwarding reference, this does not work.

  Thus, what do I have to do to get const correctness here (My question is about C++14, but I am also interrested in C++17 answers).

Answer 1

Through the magic of reference collapsing...

 template<class T>
 [[nodiscard]] std::decay_t<T> unique(T&& t) {
   std::decay_t<T> vec=std::forward<T>(t);
   std::sort( vec.begin(), vec.end() );
   vec.erase(
       std::unique( vec.begin(), vec.end() ),
       vec.end()
   );
   return vec;
}

works for both cases. DRY.

Note that view types, like std span, have issues.

Answer 2

My main questions:

Is this correct?

Implementation is correct.

I think by calling the rvalue reference version from the lvalue reference version, I do one extra move.

Against

template <typename T>
[[nodiscard]] T unique(const T& orig)
{
    T vec{orig};
    std::sort(vec.begin(), vec.end());
    vec.erase(std::unique(vec.begin(), vec.end()), vec.end());
    return vec;
}

Your std::move(vec) disallows NRVO, so you have a forced move.

return vec; allows NRVO. if not applied, there is a (automatic) move (your version version as also one (explicit) move); if applied, there are no move.

So potentially one extra move indeed.

In C++20, you might get rid of std::move, as automatic move can be done from rvalue reference too.

Apart from this extra move, do I make any other unnecessary constructions?

In case of temporary, you have a forced-move (see above)

With (unique overload)

template <typename T>
[[nodiscard]] T unique(T vec)
{
    std::sort(vec.begin(), vec.end());
    vec.erase(std::unique(vec.begin(), vec.end()), vec.end());
    return vec;
}

NRVO can apply, and if so no extra move. (if not applied one move as your).

And for lvalue object, you also have one copy.

Given the extra move, for very simple algorithms (oneliners), is it more efficient to duplicate the code?

As shown above, calling rvalue version from lvalue version might have one extra move.

but the unique version doesn't have that overhead and avoid duplication of code.

Note: [[nodiscard]] is C++17, and seems really appropriate here (especially in your version with reference)

Answer 3

I would suggest to make the "non-move" version accepting const& as well. This allows the use the coping version for const objects.

Is there a need to force the caller to hand-off owner-ship of vec? I would propose to "just" use a T&. You can see similar implementations on the stream-operators.

std::ostream& operator<<(std::ostream&, foo)

I think by calling the rvalue reference version from the lvalue reference version, I do one extra move.

Yes, there is an extra move, but compared to the copy it is negligible mostly. It would be the the simplest/best way to avoid code duplication. But you can avoid id by using a reference. I'll show in my proposal.

You maybe kick-out copy-elision when you std::move. With separate code for the non-move version, you would set T on the stack and return at the end. The compiler would put the return value on the caller stack, so no move or copy would take place.

Apart from this extra move, do I make any other unnecessary constructions

Since your return type is T you force a copy on the return of unique (T& vec)

My suggested change:

namespace tt {
    template< typename T > T& unique( T& vec ) {
        std::sort( vec.begin(), vec.end() );
            vec.erase(
            std::unique( vec.begin(), vec.end()),
            vec.end()
        );
        return vec;
    }
    template< typename T > T unique( T const& vec ) {
        T r(vec);
        unique( r );
        return r; // support copy elision
    }
}

For "copy elision" there is a nice article on the cpp-standard: https://en.cppreference.com/w/cpp/language/copy_elision