[Development] Qt 5 types under consideration for deprecation / removal in Qt 6

[Lars Knoll]

Also here going back to the start of the thread (as I was offline for some days for a long weekend):

> On 29 May 2019, at 12:53, Mutz, Marc via Development <development at qt-project.org> wrote:
> 
> Hi,
> 
> Here's a list of stuff I consider has served it's purpose and is no longer needed, with respective replacements:
> 
> = Priority 1 =
> 
> == QSharedDataPointer / QExplicitlySharedDataPointer ==
> 
> These are basically Qt-internals, and should never have been public in the first place. It's _because_ they are public that we have two of them, and soon a third one (properly non-public): https://codereview.qt-project.org/c/qt/qtbase/+/115213 That commit's message also explains what's wrong with the QSDP and QESDP.
> 
I see no reason at all for removing those, quite to the contrary. They are very helpful for building refcounted classes (explicitly or implicitly shared).

> 
> == Q_FOREACH -> ranged for loops (https://codereview.qt-project.org/c/qt/qtbase/+/147363) ==
> 
> Q_FOREACH is less readable (because it allows mutating the container under iteration), and produces _a lot_ of code. It also is intrinsically tied to just a few Qt containers (excluding QVLA). Ranged for loops work on anything, incl. a C array. Also, as a macro, it will continue to cause problems down the road (e.g. with modules). Being a macro also means that it cannot deal with value_types that contain a comma.
> 
> === related: Q_FOREVER -> for(;;) ===
> 
> Suggested by Lars in ibid. Basically because it's a macro.

Right, I suggested this, mainly because I don’t think Q_FOREVER is nicer than for(;;) or while(true). But I don’t mind the macro and if it’s widely used we could simply keep it (it doesn’t cost us anything to have it).
> 
> 
> == Java-style iteration (https://codereview.qt-project.org/c/qt/qtbase/+/262344) ==
> 
> It's very easy to write quadratic loops with it.remove(), and a review of Qt code has shown that some users still use container.remove(), which is just as unsafe as with STL iterators. I also noted between 100b/loop and 5KiB for four loops of text size savings.

I’m a bit torn here. On code review I gave a +1 on deprecating them, but I see that this could lead to a lot of porting effort on user code that makes extensive use of them. And the problem is that the porting can be non-trivial in cases where the list gets modified while iterating. So I think we should most likely keep them for Qt 6.
> 
> 
> == QScopedPointer -> std::unique_ptr ==
> 
> Suggested by Thiago on https://codereview.qt-project.org/c/qt/qtbase/+/261553
> 
> I agree. We now have std::unique_ptr, and it's movable. QScopedPointer had the problem that it didn't know what it wanted to be: boost::scoped_ptr or std::unique_ptr. A real scoped pointer would not offer release(), a unique_ptr would need to provide move semantics. QScopedPointer has release(), but no moves (were proposed, but not accepted).


> 
> 
> == QLinkedList -> std::list (https://codereview.qt-project.org/c/qt/qtbase/+/261421) ==
> 
> Of the Qt containers, this one is the most-unused (in Qt code), as well as the one that's furthest behind it's STL counter-part. Whenever I use std::list, I use it for splice(), a function that just cannot be added to a CoW container. Qt is already almost completely QLinkedList-free. Let's bury QLinkedList.

It’s not used a whole lot, and I’m not against deprecating it. But do we need to remove it for 6.0? Or maybe go the route we thought about for other containers as well and have it wrap a std::list. ie. remove our implementation, keep our API.
> 
> 
> == qHash() -> std::hash ==
> 
> Suggested by Lars in https://codereview.qt-project.org/c/qt/qtbase/+/261819. To be precise, he's suggesting to specialise std::hash and have qHash() call std::hash.
> 
> Only problem I see so far is that std doesn't provide us with a tool to hash composites. E.g. there's no std::hash for std::tuple (which would mean we can std::tie the members and hash the result), and only C++17 adds some kind of raw bits hashing (via std::string_view). We'd need to provide these building blocks ourselves, which can be done, but it means we'll have at least _some_ qHash()-like functions we need for std::hash<> implementations.
> 
> Actual problem?

I think this depends on whether we use std::unordered_map to implement QHash or not.
> 
> 
> == QPaintDevice ==
> 
> I'd like this to become a static interface. In very shortened terms: everything that has a QPaintEngine *paintEngine() method is a paint device. QPainter's ctor would become a template and do the virtual dispatch internally, just like Sean Parent's document type in C++ Seasoning.
> 
> This would solve a lot of problems: QWidget would no longer need to use multiple inheritance, and QImage and QPixmap would become proper value types, without virtual functions that create problems with move semantics and swapping.

I like this idea. It wasn’t possible in older times, because we couldn’t use templated constructors.
> 
> 
> == QRegExp ==
> 
> Is QRegularExpression good enough these days? :)
> 
I’m in favour of deprecating and moving it into a Qt5Support library. As pointed out, qmake will need it.
> 
> == MT plumbing ==
> 
> Let's make use of the std facilities to enable checkers such as TSAN to work without hacks. It's not Qt's business to reimplement threading primitives.
> 
> Probably need to keep some stuff around, like QThread, because of the interaction with event processing, but before we add a lot of time making the following classes play nice with the std, let's perspectively remove them:

QThread has to stay. It’s a QObject, and provides the event loop for threads.
> 
> === QAtomic -> std::atomic ===
> 
> It already is just a thin wrapper around std::atomic, so there's not much point keeping it.

Except that a simple search and replace will change semantics of load(). And I think that’s not quite acceptable, given that atomics are in quite a few cases used for performance sensitive code paths.
> 
> === QMutex / QReadWriteLock -> std::*mutex* ===
> 
> It has too many responsibilities. Where the std knows many different mutex classes, Qt folds everything into just two.
> 
> We probably need to keep QRWL around a while longer, since C++ added shared_mutex only in C++17.

The only problem I see with QMutex is that it implements both a recursive and non recursive mutex in one class. I would consider splitting that up.
> 
> === QMutexLocker -> std::unique_lock ===
> 
> 1:1 replacement in the vast majority of cases. unique_lock has a lot more features (movable, adopting a locked mutex, not tied to any particular mutex class, ...)
> 
> === QWaitCondition -> std::condition_variable(_any) ===
> 
> Plumbing that std::condition_variable can do better.
> 
> 
> 
> 
> 
> = Priority 2 =
> 
> == QQueue / QStack -> std::queue, std::stack ==
> 
> These classes publicly inherit QList and QVector, resp., and are both very inflexible (due to the fixed underlying container), as well as too flexible (they offer non-queue, non-stack behaviour, such as iteration).
> 
> For QQueue, we have the additional problem that QList is going to be deprecated/removed in Qt 6 (see previous discussion).
> 
> 
> == QSharedPointer / QWeakPointer -> std::shared_ptr/weak_ptr ==
> 
> Once they are stripped of their magic QObject handling and QObject handling returned to QPointer proper, they don't do much other than std::shared_ptr, except being less flexible and largely untested for exception-safety.
> 
> 
> 
> 
> 
> = Priority 3 =
> 
> == QSet / QHash -> std::unordered_set/map ==
> 
> I'd really like to see these gone. Mainly to free up their names for OA hash containers, something that the STL doesn't, yet, have.

No idea what OA containers are, but the classes are widely used and have to stay or we break SC in a huge way. Implementing them as implicitly shared versions of std_unordered_set/map is something that we should investigate.
> 
> 
> == QMap -> std::map ==
> 
> These classes have taken some design decisions that make them very badly integrated into modern C++. One is that QMultiMap is-a QMap. The first and foremost is, though, that *it returns value_type, making ranged-for useless for maps. If we're going to change the return value of *it, though, we might as well kick the whole class out, too, since the code adjustments on the user's parts would be roughly the same.

Nope. Also here, this would break a huge amount of user code. We should enforce the split between QMap and QMultiMap (ie. get rid of insertMulti()) though. Same for QHash btw.

Cheers,
Lars

> 
> 
> ... to be continued.
> 
> Flame away :)
> 
> Thanks,
> Marc
> _______________________________________________
> Development mailing list
> Development at qt-project.org
> https://lists.qt-project.org/listinfo/development