[Development] The future of smart pointers in Qt API

[Giuseppe D'Angelo]

Il 03/02/20 23:55, André Pönitz ha scritto:
> On Mon, Feb 03, 2020 at 10:25:21PM +0100, Giuseppe D'Angelo wrote:
>> Il 03/02/20 20:38, André Pönitz ha scritto:
>>> Directly affected are for instance functions operating on full containers in
>>>
>>>     https://doc.qt.io/qt-5/qtalgorithms-obsolete.html
>>
>> Just to set the record straight, the main reason why qAlgorithm(begin, end)
>> as well as qAlgorithm(container) have been deprecated altogether, rather
>> than simply have their implementation replaced with std::algorithm() calls,
>> was the fact it would've been source incompatible.
> 
> I think I've never needed qAlgorithm(begin, end), but used quite a bit of
> qAlgorithm(container).
> 
>> For instance: qSort internally uses calls to qSwap and qLess.
> 
> So what?  It worked.  Now it doesn't anymore.

So changing qAlgorithm(container) implementation to call 
std::algorithm(container.begin(), container.end()) would have been a 
source incompatible change.

(Side note: throughout the entire Qt 5 lifetime qAlgorithm(container) 
still 100% works -- it's deprecated, not removed.)


In other words, in the Qt4->5 time, the choice was between one of these:

A) Keep QtAlgorithms as-is between Qt 4 and Qt 5, leave them fully 
supported.

B) Keep QtAlgorithms API as fully supported, but change the 
implementation under the hood to use std::algorithms.

C) Deprecate QtAlgorithms API, leave them as-is for Qt 4 compatibility. 
Tell the users that they have Qt 5.x entire lifetime to migrate away.

D) Something else: e.g. add the convenience wrappers back in some other 
namespace (after the port to std::algorithms, so with different 
semantics) etc.; no one has proposed or done so far.


Amongst these choices, A) was deemed a poor one (NIH + Qt only having a 
handful of algorithms, not all, and C++ getting more and more + 
QtAlgorithms implementation being much worse in feature set and 
implementation -- read, performance).

B) is a "hidden" source incompatible change (code would still compile 
but break at runtime).

<rant>

I cannot stress enough that the ultimate reason for the incompatibility 
is the decision of making the QtAlgorithms themselves non-Standard (I 
know that -no-stl was the ultimate reason behind this decision. Still, 
it's a fact.).

The argument of  diverging from upstream when reimplementing low-level 
facilities in Qt popped up a bunch of times in this thread: the 
algorithms prove the point that it's a bad idea, because it will bite 
you in the long run.

</rant>

So we went for C). Meaning that your code using qSort still works 100% 
fine (it's deprecated, not removed, and kept identical semantics). And 
also meaning that you'll have to spend "some" time to do a port. 
(Actually, coming Qt 6, we now have the qSwap name back, so we can do B) 
without worrying the SIC).

Is it bad and will cause pain and frustration to downstreams? 
Unfortunately yes -- and I'm not having any joy at being the bearer of 
bad news. D) would have helped with the impact and AFAICT no one would 
be opposing such a thing.

Was all of this done because "I don't need qAlgorithms' convenience so 
no one needs it"? Absolutely not!

Should we had gone for B) instead? I say no, QUIP-6 says no, other 
people are feel to say yes. (Side note: this *will* fuel arguments 
against using Qt facilities, namely, "Qt regularly breaks them".)


>> An user overloading or specializing them would have had their algorithms
>> broken by the mere replacement towards std::sort (which instead uses an
>> ADL-found swap() + std::swap, and std::less).
> 
> And most user code breaks by a mere replacement of 0 by 1.
> 
> What kind of argument is that?
> 
> "Break existing code, since other code might potentially break
> when uses replace some function by something else"?

No: it would break existing code without the user changing their code at 
all.

Qt 4: qSort(foo) (uses qLess)
Qt 5: qSort(foo) (uses std::less)

Users don't change their code; code compiles just as before; behavior 
and thus semantics of the call change, possibly breaking at runtime. 
That's a source incompatible change, of the worst kind.


>> So, for the record, if you want to point the finger against the
>> deprecation/removal of these algorithms, please point it against the
>> decision of making the Qt 4 algorithms _diverge_ from upstream, then
>> noticing that Qt cannot or shouldn't catch up with the significant
>> improvements happening upstream, then realizing that a direct port isn't
>> doable because of the diversion. I have already said that having different
>> behavior from from upstream is a terrible idea in this very thread, and the
>> algorithms example is an excellent one.
> 
> There is not upstream 'sort(Container)' that this has possibly diverged from.

Oh wait, in C++20 there is :-P

More seriously: is this arguing for having qSort(container) still around 
with the Qt 4 implementation?


>> Now, we may disagree on the extent of the incompatibility -- in the end, who
>> would override qSwap, specialize qLess, and so on? Can't we just bite the
>> bullet and break those rare usages rather than forcing everyone to port
>> away?
> 
> That 'porting away' basically consists of creating a layer of convenience
> ex-Qt functions on top of Qt. Same implementation initially, deviating
> now on a per-project base.

I, for one, would welcome centralization of these efforts. Anyone else?


>> As the one who did the porting work, then I get the privilege to say: no, we
>> can't; this is a gratuitous and very hidden source-incompatible change, and
>> the promise of Qt 4->5 was to keep them at a minimum. (And, I don't like
>> them.) So, keep using the deprecated qAlgorithms if you want, throughout the
>> entire Qt 5.x lifetime, with the same Qt 4.x semantics; they still work just
>> like before. _Port_ to the std:: equivalents at your earliest convenience
>> (no, you cannot do a mere s/qAlgorithm/std::algorithm/ in the general case;
>> it's a full port).
> 
> You don't need what I need, so you kill it, and I am forced to re-create
> it on my own time. Overall, this approach de-values Qt.

I never said that the convenience wrappers were unneeded (nor that *I* 
didn't need them so I killed them). I said that keeping them would've 
been a problem -- pick your poison, poor quality or source incompatibility.

If the Qt implementations were Standard-complaint, there would been no 
problem at all keeping qAlgorithm(container) undeprecated and just 
reimplemented on top of the corresponding std::algorithm; we would have 
had more features and performance for free, while keeping the convenience.


> And now we are back to the fallacy thing "I don't need it, noone else does",
> which was the actual point. No matter what the actual topic is, I consider
> it a fundamental necessity of any sensible discussion that both sides are
> allowed to use the same kind of arguments. So if "I don't need it, noone else
> does" is considered a valid argument for one side, the other side has to
> be allowed to use it as well. Alternatively, neither side should use it
> (which actually would be my prefered setup, but that seems out of question)

And I totally agree -- it *is* a fallacy, so it shouldn't be used!

Thanks,
-- 
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/20200204/5a378554/attachment-0001.bin>