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. There's no such thing
as binary compatibility between compilations using different flags anyway.
Indeed, most move constructors (excepting the few you implemented) cannot be
implemented inline (because they require the definition of ~FooPrivate), so
you already have an C++11/98 BiC issue once you've implemented all the shared
classes' move constructors (I did, does anyone else work on this, btw?).
Also, as you noted yourself, it's probably too late to change all setters.
However, an rvalue reference overload can be added in a BC way, in 5.1.
Taken together, this to me suggests to use rvalue overloads instead of
pass-by-value. The resulting code duplication can and should be factored in
the implementation, of course.
> The solution is actually much simpler:
>
> void Foo::setText(QString text) {
> d_ptr->text = std::move(text); // no copy
> }
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.
> 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
--
Marc Mutz <marc.m...@kdab.com> | Senior Software Engineer
KDAB (Deutschland) GmbH & Co.KG, a KDAB Group Company
www.kdab.com || Germany +49-30-521325470 || Sweden (HQ) +46-563-540090
KDAB - Qt Experts - Platform-Independent Software Solutions
_______________________________________________
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development
