Are overloads for const and universal references necessary?

Question

Consider a 'contract' function used for checking values of arguments etc:

  template< class T >
  const T& AssertNotEmpty( const T& val )
  {
    //raise hell if val empty/0/...
    return val;
  }

Which for example can be used as follows:

void foo( const std::shared_ptr< int >& val )
{
  AssertNotEmpty( val );
  //use *val
}

class Bar98
{
public:
  Bar98( const std::shared_ptr< int >& val ) : myVal( AssertNotEmpty( val ) ) {}
private:
  std::shared_ptr< int > myVal;
};

std::shared_ptr< int > x;
//...
AssertNotEmpty( x ); //(1)

Now enter C++11 where we want Bar98 to take the constructor argument by value and move from it:

class Bar11
{
public:
  Bar11( std::shared_ptr< int > val ) :
    myVal( AssertNotEmpty( std::move( val ) ) )
  {}
private:
  std::shared_ptr< int > myVal;
};

For this to work AssertNotEmpty needs a rewrite which I rather naively thought would work by making use of a universal reference:

  template< class T >
  T&& AssertNotEmpty( T&& val )
  {
    //...
    return std::move( val );
  }

This seems fine for all cases except the last one (1), where VS gives warning C4239: nonstandard extension used : 'return' : conversion from 'std::shared_ptr<int>' to 'std::shared_ptr<int> &'. As far as I know this is because the compiler sees AssertNotEmpty( x ) which is AssertNotEmpty( T& && x ) which collapses into AssertNotEmpty( T& ) and you cannot move from T&, please correct me if I'm wrong.

To fix this I added the universal reference as an overload that is enabled only for non-lvalue references to force the compiler to select the const reference one also when it encounters a plain lvalue reference like in (1):

  template< class T >
  const T& AssertNotEmpty( const T& val )
  {
    //...
    return val;
  }

  template< class T >
  T&& AssertNotEmpty( T&& val, typename std::enable_if< !std::is_lvalue_reference< T >::value, int >::type* = 0 )
  {
    //...
    return std::move( val );
  }

Seems to work as intended and the compiler selects the correct one in all cases I tried, but is this the 'correct' C++11 way to solve this? Are there any possible pitfalls? Isn't there a solution not requiring duplication?

`AssertNotEmpty` has no deal modifying its value nor returning anything, at least not by that name... — K-ballo, Jan 09 '13 at 15:18
@LightnessRacesinOrbit just some reference-counting noisy class printing out when it's moved/copied/assigned etc, was using that at the time of testing instead of std::shared_ptr from the example — stijn, Jan 09 '13 at 15:36
@stijn: Let's have a minimal _complete_ testcase then please — Lightness Races in Orbit, Jan 09 '13 at 15:41

score 5 · Answer 1 · edited Jan 10 '13 at 01:27

5

I don't think you should be returning anything from that function. However, this might do what you intend.

template<class T>
auto AssertNotEmpty(T&& val) -> decltype(std::forward<T>(val))
{
    //...
    return std::forward<T>(val);
}

edited Jan 10 '13 at 01:27

ildjarn

62,044
9
127
211

answered Jan 09 '13 at 15:19

ronag

49,529
25
126
221

1

`decltype(std::forward(val))` is a rather verbose way of saying `T&&`. – R. Martinho Fernandes Jan 09 '13 at 15:20
@R.MartinhoFernandes: Wasn't sure whether it would always result in a `T&&`, so I took the safe alternative. – ronag Jan 09 '13 at 15:21
can you elaborate on why exactly forward is ok but move isn't? – stijn Jan 09 '13 at 15:41
3

`std::move` always returns an rvalue, whereas `std:forward` returns an rvalue when called with an rvalue and returns an lvalue when called with an lvalue. – Jonathan Wakely Jan 09 '13 at 16:16
@stijn: [Read this](http://stackoverflow.com/a/8527373/87234) and if you want more in-depth info read the linked duplicate. – GManNickG Jan 09 '13 at 17:37
thanks; while this perfectly answers the question I still went for K-ballo's answer as it provides a complete *solution* – stijn Jan 10 '13 at 08:18

score 3 · Accepted Answer · edited Jan 10 '13 at 01:30

3

This is not strictly an answer to your question, but I do not think that AssertNotEmpty should modify its argument nor return anything. You can still use it in constructors, thanks to the comma operator, like this:

template< class T >
void AssertNotEmpty( T const& val )
{
  /* assert( val not empty ) */
}

class Bar11
{
public:
  Bar11( std::shared_ptr< int > val ) :
    myVal( ( AssertNotEmpty( val ), std::move( val ) ) )
  {}
private:
  std::shared_ptr< int > myVal;
};

Note that the extra parenthesis are required, so the two expressions are evaluated and the result is that of the last expression.

Otherwise, you should rename your function. Something like AssertNotEmptyThenMove comes to mind...

edited Jan 10 '13 at 01:30

ildjarn

62,044
9
127
211

answered Jan 09 '13 at 15:27

K-ballo

80,396
20
159
169

Interesting use of the comma operator, was not aware you could do that. – ronag Jan 09 '13 at 15:28
you are right that the function should not modify nor return anything, the only reason it does is to be able to use one liners. However, your comma operator soluion is a pretty brilliant way to solve that! – stijn Jan 09 '13 at 15:38

Are overloads for const and universal references necessary?

2 Answers2