[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