[Development] Making QScopedPointer scoped (again)

[Stephen Kelly]

On Tuesday, September 03, 2013 19:17:53 you wrote:
> Hi,
> 
> In modern C++, one should not handle pointer directly. 

Yes. A good goal. 

>  QGraphicesView::addItem(QScopedPointer<QGraphicsItem> item)
> 
> now you know that the the ownership is taken by the view.

Peter said what I was going to say. When the need to add such a method arises, 
you should write a QUniquePointer. You should not shoehorn moving into 
QScopedPointer thereby breaking the meaning of QScopedPointer. 

QUniquePointer could even share code with QScopedPointer, but that can be part 
of the implementation discussion. Currently we are discussing the features 
QScopedPointer should have, so we don't need to discuss the implementation 
details of QUniquePointer here.

The non-movability of QScopedPointer is a feature, and it is a feature 
QScopedPointer should have. 

If you are reading code written by someone smart (and based on Qt 5.1) and you 
see a QScopedPointer, you know that it will not escape its scope. 

Someone who is not smart might use q.reset(p.take()), but someone smart would 
not use QScopedPointer if they needed to do that.

Similarly, someone who is not smart might write 

 delete sharedPtr.data();

We should assume people are smart. Particularly if the smart pointer they are 
using has 'scoped' in the name, and has had the 'scoped' meaning since Qt 4.6 
when it was introduced.

>   QScopedPointer<QStyle> QStyleFactory::create(QString);

This looks really wrong. I'm surprised you do not see that. 

Obviously it should be 

 QUniquePointer<QStyle> QStyleFactory::create(QString);

> 
> Each C++ library come with its own set of smart pointer. Now that C++11 is
> there I think we should align to the same semantics, that's what C++
> developers should learn.

C++11 has no 'scoped' pointer. C++11 has a 'unique' pointer. If you want Qt to 
have a 'unique' pointer, it should not be called QScopedPointer (very very bad 
and non-Qt-like- class name for something which is supposed to be 'the Qt 
unique pointer'). 

It should be QUniquePointer. That should be very obvious. Making 
QScopedPointer movable was a mistake. That should be very obvious too.

> > Adding a move contructor to QScopedPointer makes no sense, because moving
> > means 'escaping the scope', which breaks the fundamental point of
> > QScopedPointer. QScopedPointer is different to std::unique_ptr and should
> > remain so.
> 
> I disagree.  The fact that you can exit the scope explicitly is perfectly
> fine.

Ok - you disagree. That doesn't change my mind.

> 
> I would even go further and add conversions operators for STL compatibility:
> 
> inline QScopePointer<T>::QScopePointer(std::unique_ptr<T> other)

This is somewhat reasonable. However, it is unrelated to whether 
QScopedPointer should be movable. I have comments on what you propose, but as 
it is off-topic for this thread, I'll keep them to myself for now.

> inline QScopePointer<T>::operator std::unique_ptr<T>()

This is not a good idea.

Thanks,

-- 
Join us in October at Qt Developer Days 2013 - https://devdays.kdab.com

Stephen Kelly <stephen.kelly at kdab.com> | 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.qt-project.org/pipermail/development/attachments/20130903/ce47aa0a/attachment.sig>