How to avoid unnecessary instances using rvalue references in C++

Question

I would like to create a custom container Container that stores data in individual arrays. However, to facilitate easy iterations over the container, I provide a 'view' on the container by overloading operator[] and return a single struct Value that holds all container variables as references to the actual container. This is what I got so far:

#include <iostream>
using namespace std;

struct Value {
  Value(int& data) : data_(data) { }
  int& data() { return data_; }
  int& data_;
};

struct Container {
  Value makeValue(int i) { return Value(data_[i]); } // EDIT 1
  Value&& operator[](int i) {
    // return std::forward<Value>(Value(data_[i]));
    return std::forward<Value>(makeValue(i)); // EDIT 1
  }

  int data_[5] = {1, 2, 3, 4, 5};
};

int main(int, char**)
{
  // Create and output temporary
  Container c;
  cout << c[2].data() << endl; // Output: 3 - OK!

  // Create, modify and output copy
  Value v = c[2];
  cout << v.data() << endl; // Output: 3 - OK!
  v.data() = 8;
  cout << v.data() << endl; // Output: 8 - OK!

  // Create and output reference
  Value&& vv = c[2];
  cout << vv.data() << endl; // Output: 8 - OK, but weird:
                             // shouldn't this be a dangling reference?
  cout << vv.data() << endl; // Output: 468319288 - Bad, but that's expected...
}

The code above is working as far as I can tell, but I'm wondering if I use the best approach here:

1. Is it correct to return the Value as an rvalue reference if I want to avoid unnecessary copying?
2. Is the use of std::forward correct? Should I use std::move (both will work in this example) or something else?
3. The output of the compiled program is stated in the comments. Is there any way I can avoid the dangling reference when I declare Value&& vv... (or even forbid it syntactically)?

EDIT 1

I made a small change to the source code so that the Value instance is not directly created in the operator[] method but in another helper function. Would that change anything? Should I use the makeValue(int i) method as shown or do I need to use std::move/std::forward in here?

Answer 1

Is it correct to return the Value as an rvalue reference if I want to avoid unnecessary copying?

No. Returning rvalue references from something that isn't a helper like std::move or std::forward is flat-out wrong. Rvalue references are still references. Returning a reference to a temporary or a local variable has always been wrong and it still is wrong. These are the same C++ rules of old.

Is the use of std::forward correct? Should I use std::move (both will work in this example) or something else?

The answer to the previous question kinda makes this one moot.

The output of the compiled program is stated in the comments. Is there any way I can avoid the dangling reference when I declare Value&& vv... (or even forbid it syntactically)?

It's not the Value&& vv = c[2]; part that creates a dangling reference. It's operator[] itself: see answer to the first question.

Rvalue references change pretty much nothing in this case. Just do things as you would have always done:

Value operator[](int i) {
    return Value(data_[i]);
}

Any compiler worth using will optimise this into a direct initialisation of the return value without any copies or moves or anything. With dumb/worthless/weird/experimental compilers it will at worst involve a move (but why would anyone use such a thing for serious stuff?).

So, the line Value v = c[2]; will initialise v directly. The line Value&& vv = c[2]; will initialise a temporary and bind it to the rvalue reference variable. These have the same property as const& used to, and they extend the lifetime of the temporary to the lifetime of the reference, so it wouldn't be dangling.

In sum, the same old C++ of always still works, and still gives results that are both correct and performant. Do not forget it.

Answer 2

Returning a reference to a temporary objects, even if it is an r-value reference, is always wrong! By the time you access the object it will be gone. In that case it also doesn't do what you want it to do, anyway: if you want to avoid unnecessary copies, have one return statement returning a temporary! Copy/move elision will take care of the object not being copied:

Value operator[](int i) {
    return Value(data_[i]);
}

Passing the temporary object through a function will inhibit copy/move elision and not copying/moving is even less work than moving.