[Interest] The willy-nilly deletion of convenience methods (was: Mixing Commercial and Open...)

[Thiago Macieira]

On Wednesday, 24 March 2021 07:57:57 PDT Matthew Woehlke wrote:
> "Just ignore it" may not be an option; sometimes there are *policies*
> that preclude this.

If your policy is that you will port away from everything that got deprecated, 
then do so.

If your policy is that you shall have no warnings, then turn the deprecation 
warnings off for a particular version. Just define 
QT_DEPRECATED_WARNINGS_SINCE to version 5.13 or earlier. We provided this 
macro so you could port incrementally.

> ...And, again, even that would be less of an issue if it didn't require
> different code to support 5.14 and 5.15.

It doesn't.

The toSet() API was deprecated in 5.14. What changed between 5.14 and 5.15 was 
that you can disable the warning. In 5.14, it was:

    Q_DECL_DEPRECATED_X("Use QSet<T>(list.begin(), list.end()) instead.")
    QSet<T> toSet() const;

In 5.15, it is:

    QT_DEPRECATED_VERSION_X_5_14("Use QSet<T>(list.begin(), list.end()) 
instead.")
    QSet<T> toSet() const;

The 5.14 use was wrong (it should have used QT_DEPRECATED_X, which can be 
disabled centrally). In 5.15, you can disable the warning by setting 
QT_DEPRECATED_WARNINGS_SINCE to a value earlier than 5.14.

> Because history. If you want those, do them the "right" way. Meanwhile,
> don't "break" existing code.
> 
> Again, I think a lot of the reason folks are making such a big stink is
> because we had:
> 
>    5.14: toSet() is fully supported, no alternative is available
>    5.15: toSet() is deprecated, use ...
>    6.0: there is no toSet()

Let's assume you're off by 1 here because your history is inaccurate. What did 
happen was:

  5.13: toSet is fully supported, no alternative is available
  5.14: toSet is deprecated, warning can't be disabled
  5.15: toSet is deprecated, warning can be disabled
  6.0: toSet is not available

The warning in 5.14 was wrong. That's why it was changed in 5.15 to something 
you can disable.

> If things had *instead* looked like:
> 
>    5.9: toSet() is fully supported, "better" alternative is available
>    5.12: toSet() is deprecated, "better" alternative is available
>    5.15: same as 5.12
> 
> ...we might not be having this conversation.

There are other APIs that are like that. Some much earlier than 5.9.

There are also some APIs that did get deprecated in 5.15 and we still carried 
them over to 6.0. Explicitly because there had been no previous release with 
an alternative, so we decided we needed to provide at least one or two more 
releases where the deprecated API was available.

It didn't happen in this case. The deprecated API only got 2 releases of grace 
period.

We've noticed this before and this thread only reinforces so: deprecation 
warnings can't be given unless there has been an alternative for at least a 
couple of releases. So, right now, we have:

  Qt 6.1: no QLocale::territory()
  Qt 6.2: QLocale::territory() is available, no deprecation warnings
  Qt 6.6: QLocale::country() starts producing deprecation warning

And I don't think we'll be able to remove QLocale::country() for 7.0 either.

See https://codereview.qt-project.org/c/qt/qtbase/+/338066

> In any case, I'm not convinced that removing API is the solution to this
> problem. It seems more like something for clazy to complain about.

I can sympathise. And this particular case is definitely a sore thumb.

But that's not the vast majority of deprecations and changes done. I think 
we've done a good job for the majority. We screwed up on a few, this one 
included and even primarily.

Peppe, how about we bring these back still in 6.1 or 6.2, with a warning 
enabled by default that you can disable with a macro?

#define Q_ENABLE_INEFFICIENT_CONTAINER_CONVERSIONS

> 
> > * I need to keep the list, process it in order, but avoid processing
> > duplicate elements; 99% of the time the list is a handful of elements;
> > let me do
> > 
> >> QSet s; for (elem : list) { if (!s.contains(elem)) { s.insert(elem);
> >> process(elem); } }
> 
> And what, exactly, is the solution here?

QDuplicateTracker (not public API, but we can entertain that if there's need).

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel DPG Cloud Engineering