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

[Giuseppe D'Angelo]

Il 29/05/19 12:53, Mutz, Marc via Development ha scritto:
> 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'm assuming that they were made public to help users build their own 
CoW types. In that regard, deprecating/removing them would leave users 
in the cold (without a public replacement).

So either the proposal is to make the new one public as well, or I don't 
see an easy way out of this.


> == 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 don't think that it.remove() can be a motivation -- after all it's 
just as easy to produce quadratic behavior with it = vector.erase(it).

The real motivation should go along the lines of "is having them worth 
it"? Do they make life any easier in Modern C++? Do we still "target" 
Java developers?



> == 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).

The only missing bits would be the QScopedPointer deleter companion 
classes. Probably the only missing one is the "delete later" one, that 
would benefit from being renamed anyhow (one can use such a deleter in 
other smart pointers). The other ones can be refactored via search and 
replace -- e.g. QScopedPointerArrayDeleter<T> => 
std::default_delete<T[]> or something like that.



> == 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?

A problem to provide string/byte hashing and a hash combiner (for 
private use)? I don't think so.

But I'm not sure I understood the proposal: is it in practice suggesting 
to rewrite all qHash overloads for builtin/Qt types to forward to 
std::hash? (Basically just move the implementation and who-calls-whom)


> == 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.

This would be a lovely experiment to try. Apart from QPainter, how many 
other classes work in terms of a QPaintDevice*?


> == QRegExp ==
> 
> Is QRegularExpression good enough these days? :)

I think that thanks to Samuel Gaist it's also got wildcard matching. The 
only concerns are

1) the bigger footprint in general; that gives an argument for creating 
a build stripped of QRegularExpression, which in turn would break many 
low-level APIs (QDirIterator, for example)

2) should QRegExp stay in bootstrap? I have no idea of what's happening 
regarding to that in Qt 6.


> === QAtomic -> std::atomic ===
> 
> It already is just a thin wrapper around std::atomic, so there's not
> much point keeping it.

We're very lucky as the std atomics use even the same names that 
QAtomics are. There are probably a minor of points where tooling will 
help at porting, e.g. plain load/store having strictier semantics in 
std::atomics (seq_cst in std::, relaxed in Qt). (What I mean is that 
they're safe to port away, but the correct port would be towards a 
relaxed load!)


> === 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.

There are a few practical problems:

1) std::mutex is not constexpr+noexcept everywhere
2) std::mutex is still not futex-based everywhere => it's still 
significantly slower than QMutex

These block the upgrade path, at least for now.


> === 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, ...)

Yup.


> === QWaitCondition -> std::condition_variable(_any) ===
> 
> Plumbing that std::condition_variable can do better.

Does it actually do it better, though? For instance, is 
std::condition_variable still backed by a pthread_cond_t? That means a 
wait() on it is a POSIX cancellation point, which gives you extra costs.


MIA: QSemaphore, for which there's nothing yet in the stdlib (maybe in 
20, pending P1135).


> = 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).

While QStack can simply be deprecated in 6.0 and basically just stay 
around as-is, QQueue inheriting from QList is much more problematic if 
QList becomes QVector and loses the prepend optimization.



> == 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.

There's an extra feature that needs to be ported before deprecation is 
possible, namely the support for casting QVariants holding 
QSharedPointer<QObject> and similar plumbing. I've added 
qobject_pointer_cast in 5.14, and magic QObject handling has been 
deprecated since 5.0 already.


> = 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.

I don't think this is realistically possible. What I would suggest, to 
bring the maintenance burden down to a minimum, and increase our 
compatibility with stdlib to a maximum, is have QHash/QMap to be a CoW 
wrapper for std::(unordered_)map . The inner data structure should be 
exposed to the user, allowing for deep integration between stdlib and Qt.


> == 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.

QHash has the same problem with QMultiHash. And the consideration is 
just as above. For wrapping solution to be 100% API compatible with Qt 5 
QHash/QMap, it will therefore require:

1) (minor) deprecate insertMulti => solution, use a QMulti* container 
and plain insert

2) (minor) deprecate bidirectional iterators on QHash => 
std::unordered_map only has forward

3) (major) QMultiMap and std::multimap insert equivalent keys in 
opposite order. Breaking this behavior is another deep paper cut à la 
QList->QVector.


Thanks for starting this discussion,
-- 
Giuseppe D'Angelo | giuseppe.dangelo at kdab.com | Senior Software Engineer
KDAB (France) S.A.S., a KDAB Group company
Tel. France +33 (0)4 90 84 08 53, http://www.kdab.com
KDAB - The Qt, C++ and OpenGL Experts

-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 4329 bytes
Desc: Firma crittografica S/MIME
URL: <http://lists.qt-project.org/pipermail/development/attachments/20190529/18f506fd/attachment.bin>