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

[Olivier Goffart]

Hi,

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


But first, some reminders of the cost of copies.

In Qt, most of our objects are shared,  meaning a copy is cheap: atomically 
increment the reference count (which will be decremented later when the 
previous object is destroyed).

But in C++11, the concept of move has been introduced, and move is even 
cheaper, it is a simple swap of pointers for types that implement the move 
operators. This is faster than a copy, and generates much less code.

 ჻

Setters in Qt looks like this:

void Foo::setText(const QString &text) {
	d_ptr->text = text; // here a copy
}

foo->setText(tr("hello world")); // no copy here, since we pass by reference.


But wait... tr() returns a temporary so the move could be used somehow...

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 solution is actually much simpler:

void Foo::setText(QString text) {
	d_ptr->text = std::move(text);  // no copy
}

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); }


The problem is that this code might be slightly slower in some cases in C++98 
due to the additional copy if qMove is not supported by the compiler.
But the idea is that C++11 is going to be more widely adopted really soon. 
(And it is much faster according to my benchmark http://paste.kde.org/463334 )


It is a bit late now to change all the existing setters for Qt 5.0.
But shall we change the default convention for future setters (and in general 
any function that need to do a copy)?


-- 
Olivier