[Development] Move ctors for q_declare_shared types

[Marc Mutz]

On Friday 26 June 2015 15:55:04 Daniel Teske wrote:
> On Friday 26 Jun 2015 16:45:18 Marc Mutz wrote:
> > 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.
> 
> It's nice that you follow up the denial by a statement that is entirely and
> completely irrelevant to the discussion.

Unspecified, but valid state is the wording in the standard. I was merely 
pointing out that it's acceptable, generally, according to the std, that a 
moved-from object is left in an unspecified state. It just has to be valid.

> > > 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.
> 
> The standard requires immediate release for a reason. If you don't want
> that, just use a swap instead. Implementing move assignment as a swap
> destroys a fundamental guarantee of assignment. The old value is
> destroyed.

That all resources of a are released after an assignment is not a fundamental 
guarantee. If you assign to a QRegExp, the state lives on in some internal 
cache.

The fundamental guarantee of assignment is that a == b after a = b (at least 
for Regular Types, and QVector is Regular).

And you keep that in all detectable cases, unless you use std::move to turn an 
lvalue into an xvalue. As soon as you do this, the (real) fundamental 
guarantee of assignment is already violated.

> > > "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.
> 
> Nope those are documented and there was a good reason for implementing them
> like that. Some of the omissions are surely only oversights.
>
> If there's a reason for implementing move assignment differently from the
> standard containers, then you need to provide it.

I did. Please re-read my mail.

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