Re: why is binding non-const ref to return value bad?
On Mar 11, 10:31 am, PeteUK <newbar...@gmail.com> wrote:
On 10 Mar, 21:48, peter koch <peter.koch.lar...@gmail.com> wrote:
On 10 Mar., 18:08, PeteUK <newbar...@gmail.com> wrote:
I'm doing a code review for someone and picked them up on this:
BigClass AFunction() { /*returns a large object by value*/ }
Caller()
{
BigClass bc = AFunction();
/* use bc */
}
I saif they should do this in the caller to avoid copying
the temporary:
Caller()
{
const BigClass& bc = AFunction();
/* use bc */
}
They came back with the following code which compiled but
I asked them to change to const but I had trouble
justifying why it should be const:
Caller()
{
BigClass& bc = AFunction();
/* use bc */
}
Does the life of the temporary get extended using the
non-const reference as it does from using the const
reference? Was I right to pick him up on it not being
const? Please give me some rationale if I'm right!
Well, first of all I believe you should know C++ better to
do a code review.
I'm more versed in C++ than the other programmers in the
organisation I am part of. Please note that I am not making a
claim that I know it well. Far from it! It is part of the
job description of all programmers here to provide code
reviews. The guy whose code I'm reviewing also reviews my
code. That's the way the organisation works. Anyway, by asking
the question on here I am attempting to know C++ better.
There should be no requirement that all of the reviewers be
exceptionally gifted in C++. On the other hand, I don't think
reviewers should encourage less readable and more dangerous
coding styles. Unless there's a good reason for using a
reference, stick with the value.
First of all, you misunderstand the consequence of extending
the life of the temporary. By having a const& you
essentially have the code
BigClass __hidden = AFunction();
BigClass const& bc = __hidden;
Following on from that, would you then say that following two
forms are equivalent?:
const BigClass bc = AFunction();
const BigClass& bc = AFunction();
Because I thought the first form is *potentially* less
efficient than the second because in the first form the
compiler is given leway to construct bc via the copy
constructor.
The compiler always has leeway to throw in an extra copy. In
both cases. In practice, however: when a function returns a
class type, the compiler will pass a hidden pointer to the
function, pointing to where the returned object is to be
constructed. And it has to be constructed somewhere if it's
going to outlive the function. In the first case, the compiler
will pass the address of bc; in the second, the address of an
unnamed temporary. About the only difference is that the
compiler will then have to initialize the reference (almost
certainly a pointer) with the address of the hidden temporary.
Secondly you seem to be unaware of RVO. There will be no
copy of the big object in the code shown - at least not with
any compiler I know of when you optimise the code.
Read about RVO a long time ago. Probably from Meyers' book.
When I think of RVO and how best to enable it from the
programmer's POV, I've distilled it down to trying to return
an unnamed object - i.e. construct the object at return time.
As opposed to default constructing the object, calling
setters, then returning.
Note that this is *not* a question of RVO. RVO occurs within
the called function.
[...]
The point I'm talking about here is analogous (in my mind
anyway!) to how to accept an object as a parameter. If using a
builtin, then accept by value (copy), if an object then accept
by const ref.
Some would also consider that premature optimization:-). In
practice, however, it's pretty much standard practice. And if
the function is in a separate translation unit, it's almost
impossible for the compiler to optimize out the copy of pass by
value.
These are guidelines I stick to always and I figured my
guideline of attaching to a return value of a non builtin type
by const ref also had *potential* performance merit.
It makes no difference, and can be error prone.
--
James Kanze