[Development] How to do deprecation

[Edward Welbourne]

On 20 Jul 2022, at 16:50, Edward Welbourne wrote:
>> We noticed recently that some APIs had been wrapped in #if-ery on
>> QT_DEPRECATED_SINCE(major, minor) without having QT_DEPRECATED or one
>> of the QT_DEPRECATED_VERSION* macros prepended to the actual
>> declarations within that #if-ery.  This meant that when you build
>> with QT_DISABLE_DEPRECATED_BEFORE set to a recent enough version, the
>> declarations vanished but code that was still using it never got a
>> deprecation warning from the compiler, no matter how
>> QT_DEPRECATED_WARNINGS_SINCE was set.  So no-one had been warned to
>> fix their uses of deprecated API and the code that did so duly broke
>> the build.  Hopefully we can all agree that this is bad.

Lars Knoll (21 July 2022 09:31) replied:
> Agreed. I think part of the problem is that you currently need to do
> several things to properly deprecate a method.
>
> * You need to wrap the method in QT_DEPRECATED since #ifdefs
> * You need to add a QT_DEPRECATED_VERSION_X_maj_min macro before the declaration
> * You need to document the method as deprecated using the \deprecated tag
>
> It’s way too easy to make mistakes here. One solution could be to
> reduce this down to one way of deprecating:
>
> QT_DEPRECATED_METHOD(method, major, minor, reason)
>
> So instead of
>
> #if QT_DEPRECATED_SINCE(6, 4)
>     QT_DEPRECATED_VERSION_X_6_4("Use size() or length() instead.")
>     inline qsizetype count() const { return d.size; }
> #endif
>
> You’d have:
>
>     QT_DEPRECATED_METHOD(qsizetype count(), 6, 4, "Use size() or length() instead.”)
>
> It should be possible to make this work using some macro magic, so
> that it’ll expand to the right declaration (with or without warning or
> removed) depending on Qt version and the QT_DISABLE_DEPRECATED_BEFORE
> setting.

That's all well and good for the declaration - albeit you lost the
"const" after count() - but notice that the body just got lost; and
wherever the definition of the method now happens, it needs to be
wrapped in some #if-ery so that the compiler doesn't see it if it hasn't
seen the delcaration.  Likewise for any test code that remains to test
it as long as it's still there.

Also the machinery of QT_DEPRECATED_METHOD(method, major, minor, reason)
expanding to nothing for some settings, and possibly different things
for other settings, of QT_DEPRECATED_WARNINGS_SINCE and
QT_DISABLE_DEPRECATED_BEFORE entails (thanks to the limitations of the C
preprocessor) there being a family of macros (or possibly several
families) of form QT_DEPRECATED_METHOD_M_m(method, reason) analogous to
the existing QT_DEPRECATED_VERSION_M_m, its _X_ variant and the whole
new family of similar things that Ivan's in the process of introducing
[0].  These families are in danger of becoming a maintenance burden, so
adding more of them should be approached with care.  We can probably
automate the generation of those using some CMake magic, though,
although the configuration for that may end up somewhat runic.
See [1] for recent discussion related to this.

[0] https://codereview.qt-project.org/c/qt/qtbase/+/422121
[0] https://codereview.qt-project.org/c/qt/qtbase/+/422121/comment/f158a91c_42c24503/

And we'd need a similar macro for enum members, with matching #if-ery
round the case statements for them, and another for the (rarer) case of
deprecating a whole class.  Along with associated families of _M_m
variants to implement them.  Pretty it won't be.

All of which is as much as to say: it's complicated.  I can believe it
can be done - but show me working code and a few sample symbols (of each
kind) deprecated using it, then I'll be more confident.  In any case, in
the mean time, we need to be clear about how we use the existing
machinery.

> Oh, and qdoc should of course pick up those things automatically, so
> you don’t need to add the qdoc tag in the docs :)

That, at least, I can agree with - although I'd need to hear from Topi
whether it's feasible.  My guess is that it should be, since QDoc is now
based on clang, which surely does know about macros - unless what QDoc
sees is on the wrong end of the macros having been expanded; but then it
should be able to just look for the [[deprecated]] or whatever tag the
macro expanded to (and QDoc would need suitable settings of
QT_DEPRECATED_WARNINGS_SINCE and QT_DISABLE_DEPRECATED_BEFORE); but this
would lose the information about the version at which it was deprecated,
so my hope is that we can get the macro form.

	Eddy.