2

For this simple circular queue, I want to support C++11's move semantics so the push() doesnt incur a memory copy if it doesnt need to.

Circular<struct r, 1024> queue_;
queue_.push( { r0, r1 } );

However I'm concerned my implementation, specifically the second push() below, is unnecessary or too wordy.

template<typename T, size_t capacity>
class Circular
{
public:
  Circular()
    : have_(capacity)
    , w_(0)
    , r_(0)
  {}
  ~Circular() {}
  void push( const T& x ) { have_[ w_++ % capacity ] = x; }
  void push( T&& x )    { have_[ w_++ % capacity ] = x; }
...
protected:
  std::vector<T> have_;
  size_t w_;
  size_t r_;
};

Is there a better way to get these semantics? (Edit: are the intended semantics in fact implemented?)

Followup: Can we avoid repeating the body of the push() method while preserving the intended behavior?

FDS
  • 4,999
  • 2
  • 22
  • 13
  • 2
    Your code does nothing. You need `have_[foo] = std::move(x);` (E.g. [see here](http://stackoverflow.com/a/13219621/596781).) – Kerrek SB Jul 02 '15 at 10:55
  • 4
    To clarify what Kerrek means by "your code does nothing": without an explicit `std::move()` around `x` in `void push(T&& x)`, the assignment will still call the copy assignment operator even when `T` has a move assignment operator. Even though that function takes an rvalue reference, its parameter `x` is an lvalue because it has a name (the name `x`). – Jim Oldfield Jul 02 '15 at 11:19
  • Is there a way not to repeat the body of the push() twice. I.e. can I call one from the other while maintaining the desired behaviors? – FDS Jul 02 '15 at 12:48
  • Just note: performance of this buffer will be significantly different if `capacity` is power of 2 or not. You may want to specify capacity as exponent of 2. – Slava Jul 02 '15 at 16:27

1 Answers1

3

There is indeed a better way to get these semantics: in fact, there is a way to actually get these semantics where your code currently does not. :-)

Accepting an rvalue reference into push is one half of the equation, but within that function it's an lvalue expression again so you need to add std::move to "pass the rvalue reference along".

void push( T&& x )    { have_[ w_++ % capacity ] = std::move(x); }

Beyond that, I can't think of any particular improvements to make.

Lightness Races in Orbit
  • 378,754
  • 76
  • 643
  • 1,055
  • 1
    Not a forwarding (universal) reference. The template parameter is on the class, so the T is already fixed when the `push` member function is called. Forwarding references would require template argument deduction on the `T` to kick in. – ComicSansMS Jul 02 '15 at 12:02