D23789: RFC: Add ECMGenerateExportHeaders, for improved handling of deprecated API

[David Faure]

dfaure added a comment.


  Great work. Not really easy to grasp at first sight (because it handles BC for no-compat builds of the lib itself, which we never did before) but this is certainly quite comprehensive.
  
  Since we don't yet have any source compat to worry about for these macros (unlike Qt, which needed _X variants for that), how about simply adding a third argument to FOO_DEPRECATED_VERSION right away, to allow specifying a hint in the compiler warning, provided the compiler supports C++14?
  
  kio's src/core/job_base.h:77 would become
  
    KIOCORE_DEPRECATED_VERSION(5, 0, "use uiDelegate()") KJobUiDelegate *ui() const;
  
  If your first version just ignores the argument, at least we'll start writing those hints and we can work on making cmake's GenerateExportHeaders support that, or fork it here to support it.

INLINE COMMENTS

> ECMGenerateExportHeader.cmake:75
> +# use to conditionally set a ``<prefix_name><uppercase_base_name>_DEPRECATED``
> +# macro for a class, struct or function (pther elements to be supported in
> +# future versions), depending on the visibility macro flags set (see below)

typo: pther -> other

> ECMGenerateExportHeader.cmake:85
> +# - evaluates to ``TRUE`` or ``FALSE`` depending on the value of
> +# ``EXCLUDE_DEPRECATED_BEFORE_AND_AT`. To be used mainly with ``#if`` &
> +# ``#endif`` to mark sections of implementation code for deprecated API, as

(minor) small inconsistency, here you use '&' as separator between #if and #endif, while on line 80 you use '/'.

> ECMGenerateExportHeader.cmake:87
> +# ``#endif`` to mark sections of implementation code for deprecated API, as
> +# well as declaration code of deprecated API only to disable at build time
> +# (e.g. virtual methods, see notes below).

I'm having trouble parsing the "only" in this sentence.

`deprecated API only`... that seems redundant, it's only API that gets deprecated :-)

Maybe you mean "to disable only at build time"?
Or "declaration-only code" ?

and it's always about build time... the choice is between building the library and building the users of the library, isn't it? It's all "build time" of someone...

> ECMGenerateExportHeader.cmake:107
> +# Note: The tricks applied here for hiding deprecated API to the compiler
> +# when building against a library does not work for all deprecated API:
> +#

"The tricks [....] does not work" is invalid grammar: either "The trick" (singular) or "do not work" (plural).

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D23789

To: kossebau
Cc: dfaure, cgiboudeaux, kde-frameworks-devel, kde-buildsystem, LeGast00n, GB_2, bencreasy, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-buildsystem/attachments/20190910/6b19a6f3/attachment.html>