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 _______________________________________________ Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
