views:

156

answers:

5

Dear friends, i'm concerned if i'm making a bad use of references in C++ In the following method GCC complains warning "reference to local variable ‘me’ returned"

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

But, with the following changes the warning disappears:

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

Is the former method ( returning the 'ref' variable ) correct\acceptable ?

+2  A: 

No, you must return a value here, ideally a const value. See Effective C++, Item 21.

I suggest the following interface:

const MatrizEsparsa operator+(const MatrizEsparsa& left, const MatrizEsparsa& right);

Note that everything is either a const reference or a const value. Returning a const value is not as important as returning a value or declaring the parameters as const references, but the arguments of Scott Meyers have convinced me, although no one follows them.

Philipp
The constness of the result is another debate. Returning a const reference would btw still be incorrect.
ereOn
I'll buy and read Effective C++, thanks!
Lucas
@ereOn: Yes, the suggested solution in Effective C++ is a const value. Const references are (or, at least, were, before the advant of C++0x) usually used as return types for functions such as `std::min` where creating a new objects is not required.
Philipp
+2  A: 

It is not acceptable. It is actually the same problem: returning a non-const reference to an local object that will be destroyed after returning the method.

Cătălin Pitiș
Why the emphasis on the non-const? Returning a const reference would be just as problematic...
FredOverflow
Matthieu M.
+13  A: 

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.

ereOn
+4  A: 

I think you're mistaking your operators.

There are 2:

struct Foo
{
  Foo& operator+=(Foo const&);
  Foo operator+(Foo const&) const;
};

As you notice, the first returns a reference to itself, the second does not.

Also, in general, the second should be written as a free function.

Foo operator+(Foo const&, Foo const&);

This can be automated, because it's cumbersome, using Boost.Operators:

struct Foo: boost::addable<Foo>
{
  Foo& operator+=(Foo const& rhs)
  {
    // add
    return *this;
  }
};

The little boost::addable magic will automatically generate the + implementation based on Foo::operator+=.

Matthieu M.
+1: For the nice `boost::addable` trick.
ereOn
A: 

You can't return the reference since the object you are referencing will get destroyed outside of your control. Either put "me" as a member variable of MatrizEsparsa so that it will persist after execution of the function else return a pointer, or a boost smart_ptr, that points to the object.

Seeing as this is a + operator though, you probably want to return a value rather than a reference to a variable internal to the function.

Matt_JD