[Development] Setters: Passing by value or const reference?

[Olivier Goffart]

On Wednesday 25 April 2012 15:07:24 Marc Mutz wrote:
> Hi Olivier,
> 
> On Wednesday April 25 2012, Olivier Goffart wrote:
> > In Qt, we traditionally uses const reference as parameter for all our
> > setters. This is indeed probably the most efficient thing to do for
> > non-POD
> > in C++98 since it avoids copy.  But the situation is different in C++11
> 
> [...]
> 
> > We could implement a move setter (Foo::setText(QString&&)).  But that
> > would
> > mean  duplication, and it could not be inline (access to d_ptr) so binary
> > incompatibility between Qt compiled with or without C++11.
> 
> The rvalue overloads could only be used by a C++11 compiler, and it would be
> natural for a C++11 app to require a C++11-compiled Qt.

Not really...  
On linux, if the dirstribution provides a qt compiled without C++11, i still 
want to be able to use C++11 it on my project.
C++11 is still (unfortunatelly) not the default to build Qt on linux.

> There's no such thing as binary compatibility between compilations using
> different flags anyway.

Yes there is!
On linux, Qt debug is binary compatible with Qt release
Most of the flags like -fno-exceptions  ... are still binary compatible.

> Indeed, most move constructors (excepting the few you implemented)
> cannot be implemented inline (because they require the definition of
> ~FooPrivate),

It is possible if you don't use smart pointer.
This is really unfortunate.  Why is it the case? Is there a way to avoid it?

[...] 
> Most setters look not like the one you quoted, but like this:
> 
> void Foo::setText(const QString &text) {
>     if ( text == d->text ) return;
>     d->text = text; // copy only if needed
>     // do something expensive, like repaint, or emit a signal
> }
> 
> Here, a copy is taken only if you don't early out. When you overload
> setText() for const-reference and rvalue-reference instead, you only incur
> a copy iff it's needed. Not sure that's a problem. Also not sure if slicing
> could become a problem in a pass-by-value world, but const-& and && are
> both real references, so they don't slice.

This is right.

> > foo->setText(tr("hello world")); // no copy here, this is a move.
> > 
> > Now you don't have any copy: you saved two atomic operations (increment,
> > decrement) and generated less code.
> > If you pass something that is not a temporary, you still have only one
> > copy, but on the caller.
> > 
> >  ჻
> > 
> > You notice the use of std::move,  which we can use only in C++11.
> > Hence the introduction of a qMove macro[1]
> > https://codereview.qt-project.org/24444
> > Which would lead to that pattern:
> > 
> > void Foo::setText(QString text) {	d_ptr->text = qMove(text); }
> 
> You should use d_ptr->text.swap(text) instead, that's just as fast (maybe
> even faster) and requires no C++11 :)
> 
> Talking of swap: I have a half-finished patch that makes Q_DECLARE_SHARED
> implement the two op='s with (copy)-swap (and adds swap() to classes that
> would benefit from it, but don't have it yet).
> 
> Foo &Foo::operator=(const Foo &other)
> { if ( this != &other ) { Foo copy(other); swap(other); } return *this; }
> Foo &Foo::operator=(Foo &&other)
> { other.swap(*this); return *this; }
> 
> I'll try to get it into a reviewable state.
> 
> Thanks,
> Marc