Add support for DEPRECATED_X("suggested replacement") in generated export headers
Open, Needs TriagePublic

Description

Qt has Q_DECL_DEPRECATED_X defined in src/corelib/global/qcompilerdetection.h as __attribute__ ((__deprecated__(text))) (on compilers that support it, it's a C++14 feature), which is quite convenient for people hitting a deprecation warning.

Our deprecated macros come from cmake's GenerateExportHeader.cmake which doesn't support adding such a suggestion text.

But while writing this, I just saw D23789 which forks that into ECM anyway....

dfaure created this task.Sep 10 2019, 11:18 AM
dfaure renamed this task from Add support for DEPRECATED_X("suggested replacement") in cmake's GenerateExportHeader.cmake to Add support for DEPRECATED_X("suggested replacement") in generated export headers.

ECMGenerateExportHeader does not fork it, but extend it, internally still the cmake generate_export_header one is called, to do the meat for getting the attribute strings as needed for the respective compiler.

Long term the addition of ECMGenerateExportHeader should be up-streamed as well, but first I would like to give the API some real world testing, to see what we want to have upstream provide.

Given DEPRECATED_X depends on compilers, I on purpose have left that out from what is proposed in ECMGenerateExportHeader. And would like to see this done directly in cmake first, so there is only one place doing ifdeffery for which compiler there is.

((Also personally not yet convinced having yet another place to copy text what to do instead for deprecated API is nice, as one also should put this in the normal API dox))

OK, so this task is about extending cmake's GenerateExportHeader.cmake, as I initially thought.

About your last comment: it's not a copy, it's a short summary / reminder. Just a few words to hint at the solution, as a reminder for those doing the porting so they don't have to look up for the Nth time the exact new name of the method (for simple renaming cases, for instance). I've been doing quite some porting-away-from-Qt-deprecated-methods the last two days and I find this very useful.

If easy to do without running into danger of mismatches with what cmake's logic does, surely we can also add logic to ECMGenerateExportHeader to provide such macros, I do not want to leave the impression I want to block this to be added there :)

For now just not eager to have yet another feature added to ECMGenerateExportHeader while under review which needs more care and mental power before people have started to comment on the current feature and code and things got blessed and might go in production.

Having it in ECMGenerateExportHeader, has the advantage KF can make use of it from the start, instead of only if some certain cmake version is min requirement which provides it or doing other not-so-nice hackery. So there is good reason not to wait for upstream first.

@dfaure So to plan ahead, could you add as comment to D23789 what DEPRECATED_X macro variants you might want like to see, so they can be prepared in the current design, and would just need the addition of compiler-dependent macro content generation?

When I looked at the Qt macros, I was a bit confused the _X variants seem to not integrate with the version-supporting macros, which we might want to do though.

dfaure added a comment.EditedSep 10 2019, 12:42 PM

Qt integrates the two needs (version and text) by doing

#if QT_DEPRECATED_SINCE(5, 15)
   QT_DEPRECATED_X("Use the last QWheelEvent constructor taking pixelDelta, angleDelta, phase, and inverted")
   QWheelEvent(const QPointF &pos, int delta, [...]);
 #endif

(taken from qevent.h)

kossebau added a comment.EditedSep 10 2019, 1:12 PM

That example though misses to support enabling/disabling of warnings based on version, based on QT_DEPRECATED_WARNINGS_SINCE . The #if QT_DEPRECATED_SINCE(5, 15) hides the complete signature only, but has no effect on whether warnings are emitted for this method or not.

Perhaps I am missing something, but when looking at the Qt macros, I would have expected to see a counterpart to QT_DEPRECATED_VERSION, i.e. some QT_DEPRECATED_X_VERSION.

At least I could see some sense to allow library consumers to set both FOO_DEPRECATED_WARNINGS_SINCE & FOO__DISABLE_DEPRECATED_BEFORE_AND_AT to different versions. E.g. if one has the latest Qt, but only wants to see warnings up to a certain version, because the deprecations in the latest/newest Qt version are not yet interesting for the project worked on.
That's why the current ECMGenerateExportHeader proposal makes the *_DEPRECATED_WARNINGS_SINCE a documented macro