[Development] FileRe: Move ctors for q_declare_shared types

Daniel Teske Daniel.Teske at theqtcompany.com
Mon Jun 29 13:44:13 CEST 2015


> > > - QVector should give the same guarantee as the standard containers.
> > 
> > I disagree with that. QVector is not std::vector. At first, it is
> > implicitly shared, so that's already a big difference. And therefore we
> > can allow ourselfs many more differences.
> 
> But this point still stands.

Right, there's a argument possible that gratuitous differences should be 
avoided, after all it has the same name, so it should be semantically close, 
but well, I'm not going to bother with that.

Instead I will argue that:
This guarantee is important, valuable and good and Qt not giving this is bad.

So, in this example:

QSharedPointer<SomeClass> a = [...]
QSharedPointer<SomeClass> b = [...]
a  = std::move(b);

after this line, b holds a reference to whatever a contained prior to the 
move, since QSharedPointer is using swap to implement move assignment.

This is (mostly) fine if b is a temporary, since it only affects the order of 
destruction. 

If b though is a e.g. a member variable  of a class, and the user intends to 
move resources between objects, b might not be destructed until much later.

If the order of destruction is important, or that a resource destruction has a 
side effect this is bad.

Since Qt can not know whether the destructor of the type held in a shared 
pointer or a vector has a important side effect, Qt classes have to assume that 
there are such side effects. 

It is also surprising and counter intuitive that copy assignment and move 
assignment treat the old value differently, and none of the standard library 
types that hold user types behave in such a way. If the user intended this 
behavior, s/he could have called swap instead.

In support of this argument, let me point to the history of this guarantee  
for vector in the standard. (The text for shared_ptr always contained the 
guarantee, so the history there is not as interesting.)

First, in 2007!, the issue 675 added the relevant text:
"All existing elements of a are either move assigned or destructed." was added 
to container.requirements.

See Issue 675 at http://open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2456.html.

Do note, that the proposed resolution is at the bottom, and the discussion is 
best understood with the proposed resolution in mind.

This issue and proposed resolution was submitted by Howard E. Hinnant, who was 
the lead designer and author of the move semantics standardization papers, is 
the lead author of libc++ and was the chairman of the LWG from 2005 to 2010.

The relevant quote:

"Move semantics means not caring what happens to the source (v2 in this 
example). It doesn't mean not caring what happens to the target (v1). In the 
above example both [copy and move] assignments should have the same effect on 
v1. Any non-shared ostream's v1 owns before the assignment should be closed, 
whether v1 is undergoing copy assignment or move assignment.

This implies that the semantics of move assignment of a generic container 
should be clear, swap instead of just swap. An alternative which could achieve 
the same effect would be to move assign each element. In either case, the 
complexity of move assignment needs to be relaxed to O(v1.size()). "

The same argument of course applies to QSharedPointer too:

QSharedPointer<QFile> a;
QSharedPointer<QFile> b;

a = std::move(b) should close the old QFile held in a. 

And here:
http://stackoverflow.com/questions/6687388/why-do-some-people-use-swap-for-move-assignments

Howard E. Hinnant writes:
"In general a move assignment operator should:
- Destroy visible resources (though maybe save implementation detail 
resources).
- Move assign all bases and members.
- If the move assignment of bases and members didn't make the rhs resource-
less, then make it so."

daniel




More information about the Development mailing list