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

[Mutz, Marc]

On 2019-06-03 11:26, Lars Knoll wrote:
[...]
>> == QSharedDataPointer / QExplicitlySharedDataPointer ==
[...]
> 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).

I'd still like to deprecate them. QSDP is issuing atomic operations on 
each mutable access hurting code-gen, and produces surprises such as 
https://codereview.qt-project.org/gitweb?p=qt%2Fqtbase.git;a=commit;h=96dc9a19ae11ca140d681f0e2605b5f4b953e581 
or 
https://codereview.qt-project.org/gitweb?p=qt%2Fqtbase.git;a=commit;h=a2cdfd23d61c8dd3a346982caaf1e846e30513b0 
or 
https://codereview.qt-project.org/gitweb?p=qt%2Fqtbase.git;a=commit;h=188eea0eb47c499f70a60f573948d529089d93b1.

QESDP has the bug that it doesn't propagate const. We could fix that, 
but it would be SiC, too. So, my idea was to keep them both, deprecate 
them, and use something much more cumbersome to use outside of Qt for 
Qt's own classes. I'd also be ok with keeping a fixed QESDP, but QSDP 
should go sooner rather than later.

That said, I'm ok with keeping them until Qt 7, but deprecated.

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

I think every macro removed is a good macro. There's really no point in 
having this, and it's easily emulated with

     DEFINES += Q_FOREACH=for(;;)

from any buildsystem. OTOH, such macros that don't do anything will 
become a headache when C++20 brings us modules.

>> == Java-style iteration 
>> (https://codereview.qt-project.org/c/qt/qtbase/+/262344) ==
[...]
> 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.

As I wrote in the other thread, I'm ok with keeping them. I'd like to 
deprecate them, though.

[...]
>> == QLinkedList -> std::list 
>> (https://codereview.qt-project.org/c/qt/qtbase/+/261421) ==
[...]
> 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.

I'd just let it sit in it's place, deprecated, untouched. It's not used 
in any Qt API, and the uses which I found in Qt are questionable to 
begin with.

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

That QThread is-a QObject has caused problems in the past. How to use 
QThread correctly has been the source of much discussion (e.g. 
https://blog.qt.io/blog/2010/06/17/youre-doing-it-wrong/), and any 
thread can have an event loop via QEventLoop. But, yes, QThread probably 
needs to stay around longer. That shouldn't mean it does so forever. I 
think std::thread is much easier to use and understand, and with 
QEventLoop supporting it already, I don't feel much attachment to 
QThread anymore.

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

IMHO, it's -perfectly_ acceptable, because architectures where a relaxed 
vs. a seq_cst order makes any difference are dying out. There's no diff 
on x86, AArch64, and, IIRC, newer POWER. Since we'd replacing a relaxed 
with a seq_cst load or store, correct code would stay correct.

The message I get here is that we should deprecate load() and store() 
ASAP and suggest to use explicit loadRelaxed()/storeRelaxed() instead.

[...]
>> == 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,

Open Addressing

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

I'm concerned about their use in APIs. I don't know any other library 
that is passing hash tables around through the API for small things like 
mapping role indexes to role names. That's an array of struct job.

And:

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

How do you intend to address the problem that QHash/QMap are all but 
unusable in C++11 ranged for loops, because of decltype(*it)? Like with 
QList-as-QVector, I reckon that the larger SC break would be to fix *it 
behaviour in-situ than to deprecate and replace them with something more 
sensible, like the wrapper around unordered_.

Thanks,
Marc