Re: [PATCH] 2/2 List cleanup in SD

On Wednesday 11 of April 2012, Rafael Dominguez wrote:
> On Wed, Apr 11, 2012 at 2:53 PM, Caolán McNamara <caolanm@redhat.com> wrote:
> > On Tue, 2012-04-10 at 12:52 +0200, Lubos Lunak wrote:
> > >      List*                   GetSelectEntryList( sal_uInt16 nDepth );
> > > +    void                    GetSelectEntryList (sal_uInt16 nDepth,
> > > std::vector<rtl::OUString> &rEntries) const;
> > >
> > >  Why is that? Changing the return value to a reference argument makes
> > >  the API worse and it seems like an unnecessary change to me.
...
> Well the first reason is that you cant overload a function with a return
> value,

 Ok, this one didn't occur to me.

> second reason and the main one, is that copying a vector is "costly" 
> so to prevent that i passes it by reference avoids that, but i can change
> it later if needed.

 Both gcc and msvc can do named return value optimization ([1][2]), so the 
return object does not actually need to be copied. In a nutshell, if a 
function as the first thing creates a local variable of the same type as the 
return type and all return statements return this variable, then the compiler 
will optimize by placing the variable directly in the place of the return 
value, thus avoiding the copy. C++11's move semantics (will) make this moot 
completely.

 So, in code, a function like this does not create a copy of std::vector:

std::vector< A > foo( bool b )
{
std::vector< A > ret;
if( !b )
    return ret;  // return std::vector<A>() would prevent the optimization
... // do things with ret;
return ret;
}


 I see no good reason to delay your patches just because of this, but it would 
be nice if you could do a followup patch to change such functions to return 
values normally instead of the unneeded manual optimization (presumably with 
a short suffix on one of the variants as long as both are needed).


[1] http://en.wikipedia.org/wiki/Return_value_optimization
[2] http://msdn.microsoft.com/en-us/library/ms364057(v=vs.80).aspx

-- 
 Lubos Lunak
 l.lunak@suse.cz