[Development] Move ctors for q_declare_shared types

Marc Mutz marc.mutz at kdab.com
Fri Jun 26 16:45:18 CEST 2015


On Friday 26 June 2015 12:34:31 Daniel Teske wrote:
> > Most of our implicitly shared types already had a move-assignment
> > operator, because it's just
> > 
> >    QFoo &operator=(QForr &&other) { swap(other); return *this; }
> 
> And that's indeed how QVector's move assignment is implemented.
> 
> It's also broken.

It isn't. Other is left in an unspecified, but valid state. It's not even 
partially formed, as many move operators leave their source.

> Just because other is a rvalue reference does not mean that it is a
> temporary and thus the is no guarantee that other's dtor gets called.
>
> And thus, this line:
> 
> QVector<Klass> vector = std::move(something);
> 
> does not gurantees that the dtors for the old contents of vector are
> called.

It does. It doesn't say when (when something is going out of scope), but 
saying that dtors aren't called is wrong.

The problem you have is that you want immediate resource release. If you want 
that, swap something with a new object:

   QVector<Klass>().swap(something);

(or call clear(), but that's Qt-specific). You're never holding more memory at 
any given time (if you do this), than with an immidiate-release move 
operation. According to the fundamental C++ principle "Don't pay for what you 
don't use", not clearing target is the correct thing to do. The standard 
should be fixed.

That's containers.

But the vast majority of implicitly-shared classes are pimpled (for BC, not 
for CoW reasons), so if you demand immediate resource release for moved-to 
objects, then the non-inline Qt classes cannot have move assignment (because 
it cannot be inline, and non-inline for BC reasons is also not allowed).
 
> For standard containers, this is specified in *container.requirements*
> To quote, with: a being a container and rv a non-const rvalue of the same
> type:
> 
> a = rv
> 
> "All existing elements of a are either move assigned to or destroyed,"
> 
> I do not see any reason why our containers should not give the same
> guarantee for move assignment and consider this a bug.

I guess then you also consider it a bug that Qt containers don't meet the std 
container complexity guarantees (b/c/o CoW) and don't meet the std container 
thread safety guarantees (b/c/o the omission of a non-sharable state when 
handing out references to internal state by way of op[] or mutable iterators), 
or that they don't have range-ctor or range-insert, or emplace, or allocator 
support.

Fix those first - or use a std vector if a std vector is what you want.

Thanks,
Marc

-- 
Marc Mutz <marc.mutz at kdab.com> | Senior Software Engineer
KDAB (Deutschland) GmbH & Co.KG, a KDAB Group Company
Tel: +49-30-521325470
KDAB - The Qt Experts



More information about the Development mailing list