Is the Practice of Returning a C++ Reference Variable Evil

Is the practice of returning a C++ reference variable evil?

In general, returning a reference is perfectly normal and happens all the time.

If you mean:

int& getInt() {
    int i;
    return i;  // DON'T DO THIS.
}

That is all sorts of evil. The stack-allocated i will go away and you are referring to nothing. This is also evil:

int& getInt() {
    int* i = new int;
    return *i;  // DON'T DO THIS.
}

Because now the client has to eventually do the strange:

int& myInt = getInt(); // note the &, we cannot lose this reference!
delete &myInt;         // must delete...totally weird and  evil

int oops = getInt(); 
delete &oops; // undefined behavior, we're wrongly deleting a copy, not the original

Note that rvalue references are still just references, so all the evil applications remain the same.

If you want to allocate something that lives beyond the scope of the function, use a smart pointer (or in general, a container):

std::unique_ptr<int> getInt() {
    return std::make_unique<int>(0);
}

And now the client stores a smart pointer:

std::unique_ptr<int> x = getInt();

References are also okay for accessing things where you know the lifetime is being kept open on a higher-level, e.g.:

struct immutableint {
    immutableint(int i) : i_(i) {}

    const int& get() const { return i_; }
private:
    int i_;
};

Here we know it's okay to return a reference to i_ because whatever is calling us manages the lifetime of the class instance, so i_ will live at least that long.

And of course, there's nothing wrong with just:

int getInt() {
   return 0;
}

If the lifetime should be left up to the caller, and you're just computing the value.

Summary: it's okay to return a reference if the lifetime of the object won't end after the call.

Why give a C++ compiler warning when returning an rvalue reference?

You're not moving your data. You're creating a local object, creating a reference to that local object, destroying that local object, and then still using that reference.

You should return by value, as you already found. But instead of copying, move the data. That's the safe way of ensuring you don't copy those massive amounts of data.

std::string Sampler::Serial() const {
    std::stringstream ss;
    .
    . [assemble a string value using data members]
    .
    return std::move(ss.str());
}

Note: the std::move is technically redundant here, as ss.str() already returns an rvalue and so would already be moved. I recommend leaving it in anyway. This way works in any situation, so you don't have to think about which form to use: if you want to move, write move.

---

As pointed out by T.C., in general, though not in your case, this can prevent RVO. In cases where RVO is possible and where the compiler would implicitly use a move anyway, there is no need to write move explicitly. For instance:

std::string f() {
  std::string x;
  ...
  return x; // not std::move(x)
}

Here, it should already be clear to the reader that x is a local variable. It's normal for C++ code to return local variables without writing move, because either the compiler will elide the x local variable entirely and construct the std::string object directly in the return slot (whatever that means for your platform), or the compiler will use the move constructor of std::string implicitly anyway.

Why is it bad practice to return a pointer to a local variable or parameter?

It's not so much a "bad practice" (implying that it might cause problems) as much as it is a practice that will absolutely cause undefined behavior. It's like dereferencing a null pointer: don't do it and expect your program to behave within the limits of logic.

Explanation:

When a local variable (including a parameter) is declared, it is given automatic storage, meaning that the compiler takes care of allocating memory for the variable and then deallocating that memory without any effort on the part of the programmer.

void foo(int bar)
{
    int baz;
} //baz and bar dissappear here

When the variables' lifetime ends (such as when the function returns), the compiler fulfills its promise and all automatic variables that were local to the function are destroyed. This means that any pointers to those variables now point to garbage memory that the program considers "free" to do whatever it wants with.

When returning a value, this isn't a problem: the program finds a new place to put the value.

int foo(int bar)
{
    int baz = 6;
    return baz + bar; //baz + bar copied to new memory location outside of foo
} //baz and bar disapear

When you return a pointer, the value of pointer is copied as normal. However, the pointer still points to the same location which is now garbage:

int* foo(int bar)
{
    int baz = 6;
    baz += bar;
    return &baz; //(&baz) copied to new memory location outside of foo
} //baz and bar disapear! &baz is now garbage memory!

Accessing this memory is undefined behavior, so your program will almost certainly misbehave in some way or another. For instance, I once fell victim to this exact problem, and while my program did not crash or terminate, my variables began degrading into garbage values as the compiler overwrote the "free" memory.

Does holding a C++ reference prevents variable of being destroyed?

Given that

MyShape::color is declared as a Color&

... the code is a bit problematic.

The thing to know about C++ references is that you can't re-seat them -- that is, unlike a C++ pointer, once a C++ reference is declared, it will always reference the object it was declared to reference, and cannot be re-directed to reference some other object instead. (Btw this is also the reason why you're not allowed to declare an unbound reference, e.g. MyColor & c; won't compile because it would be useless)

From that, it follows that what this code actually does:

void MyShape::setColor(const Color& newColor)
{
   this->color= newColor;
}

is overwrite the contents of (whatever Color object this->color is referencing) with the contents of newColor). It doesn't change the reference itself at all; rather it writes through the reference. (Remember that references are defined as aliases-to-something, so wherever you specify the reference, the program will behave as if you had specified the object-the-reference-is-referencing there instead)

To answer the titular question: holding a C++ reference does not guarantee that the object the reference is referencing will not be destroyed. It's quite possible to delete (or otherwise destroy) the referenced item, and end up with a "dangling reference" that will invoke Undefined Behavior if you try to use it.

As general advice, if you want to have a guard-value to represent "no-color", the best way to do it is to represent the guard-value as an actual color object (e.g. Color(0,0,0,0) could work, if your Color class has an alpha-channel, since any color with alpha=0 is transparent, hence no-color). That way there is no need to play games with pointers or references to try to represent that information. If that's not doable for some reason, have a look at std::optional as a way to represent "either-a-value-or-nothing".

As a final note, using pointers does not mean you have to use new and delete -- pointers work equally well (and you can shoot yourself in the foot with them equally badly) when they are pointing to stack-objects or member-objects. Which is to say, pointers are very powerful, and also very dangerous, and I wouldn't recommend using them for this purpose.

Is it alright to return a reference to a non-pointer member variable as a pointer?

Is this bad practice? It seems very hackish.

It is. If the class goes out of scope before the pointer does, the member variable will no longer exist, yet a pointer to it still exists. Any attempt to dereference that pointer post class destruction will result in undefined behaviour - this could result in a crash, or it could result in hard to find bugs where arbitrary memory is read and treated as a BigObject.

if he considered using some smart pointer

Using smart pointers, specifically std::shared_ptr<T> or the boost version, would technically work here and avoid the potential crash (if you allocate via the shared pointer constructor) - however, it also confuses who owns that pointer - the class, or the caller? Furthermore, I'm not sure you can just add a pointer to an object to a smart pointer.

Both of these two points deal with the technical issue of getting a pointer out of a class, but the real question should be "why?" as in "why are you returning a pointer from a class?" There are cases where this is the only way, but more often than not you don't need to return a pointer. For example, suppose that variable needs to be passed to a C API which takes a pointer to that type. In this case, you would probably be better encapsulating that C call in the class.

I'm Returning a Reference From a Lambda, why is a Copy Happening?

The return type of a lambda is auto ([expr.prim.lambda]/4), so a copy will be made unless you explicitly specify it with the trailing return type:

[](const A* pa) -> const auto& { return *pa; }(pa);

Returning references from a C++ methods

No. ref still refers to me which will be destroyed at the end of the call.

You should return a copy of your result (not prefixed by &).

MatrizEsparsa MatrizEsparsa::operator+(const MatrizEsparsa& outra) const {
    return MatrizEsparsa(outra.linhas(),outra.colunas());
}

I also added two const specifiers (to the parameter and to the method) since I doubt outra or the calling instance need to be modified in this case. (I could be wrong, but then your operator+ would have a weird semantic)

By doing what you did, you just made the code more complex. The compiler probably was confused and couldn't warn you about your possible mistake.

Usually, when you have to use clever tricks to do simple things, it means something is wrong.

Is returning uniform initialized reference valid?

This seems like an omission in the standard, where GCC is implementing exactly what the standard requires, and clang is going for what's probably intended.

From C++11 (emphasis mine):

5.2.3 Explicit type conversion (functional notation) [expr.type.conv]

1 A simple-type-specifier (7.1.6.2) or typename-specifier (14.6) followed by a parenthesized expression-list constructs a value of the specified type given the expression list. If the expression list is a single expression, the type conversion expression is equivalent (in definedness, and if defined in meaning) to the corresponding cast expression (5.4). [...]

[...]

3 Similarly, a simple-type-specifier or typename-specifier followed by a braced-init-list creates a temporary object of the specified type direct-list-initialized (8.5.4) with the specified braced-init-list, and its value is that temporary object as a prvalue.

For the braced-init-list case, the standard doesn't specify that that this works just like a C-style cast. And it doesn't:

typedef char *cp;
int main() {
  int i;
  (cp(&i)); // okay: C-style casts can be used as reinterpret_cast
  (cp{&i}); // error: no implicit conversion from int * to char *
}

Unfortunately, T(expr) being equivalent to (T)expr is also the one exception in which a functional cast doesn't necessarily produce a prvalue. The standard fails to specify a similar exception for a functional cast using a braced-init-list to a reference type. As a result, in your example, ref{x} constructs a temporary of type ref, direct-list-initialised from {x}. That temporary is then treated as a prvalue, because that's what the standard says the behaviour should be, and that prvalue cannot be used for binding to an lvalue reference.

I strongly suspect that if this were brought up to the ISO C++ committee, the standard would be changed to require clang's behaviour, but based on the current wording of the standard, I think it's GCC that's correct, at least for your specific example.

Instead of adding a variable, or switching to parentheses, you can omit ref (Tp) to avoid the problem:

template <typename Tp, typename... Us>
Tp bar(Us&&... us) {
  return {std::forward<Us>(us)...};
}

Returning reference to local temporary object

Since you're not in control of the return type, you must make sure you return a valid object and not just a temporary. One solution would be a function-local static variable:

virtual const core::matrix4& getViewMatrixAffector() const
{
  static const core::matrix4 val;
  return val;
};

If you find yourself doing this in many functions (with the same type of the variable), make val a (suitably renamed) static member of the class.

---

Related Topics