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

[Friedrich W. H. Kossebau]

kossebau updated this revision to Diff 67324.
kossebau added a comment.


  - extend unit tests to cover library group macro variants
  
  Sadly blows up by all combinations unit test time to >1 min on old machine

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23789?vs=67220&id=67324

BRANCH
  addgenerateexportheader

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

AFFECTED FILES
  docs/module/ECMGenerateExportHeader.rst
  modules/ECMGenerateExportHeader.cmake
  tests/CMakeLists.txt
  tests/ECMGenerateExportHeaderTest/CMakeLists.txt
  tests/ECMGenerateExportHeaderTest/consumer/CMakeLists.txt
  tests/ECMGenerateExportHeaderTest/consumer/main.cpp
  
tests/ECMGenerateExportHeaderTest/consumer/testAPI_DISABLE_DEPRECATED_BEFORE_AND_AT.cmake
  tests/ECMGenerateExportHeaderTest/consumer/testAPI_NO_DEPRECATED.cmake
  tests/ECMGenerateExportHeaderTest/format_version/CMakeLists.txt
  tests/ECMGenerateExportHeaderTest/format_version/main.cpp
  tests/ECMGenerateExportHeaderTest/library/CMakeLists.txt
  tests/ECMGenerateExportHeaderTest/library/library.cpp
  tests/ECMGenerateExportHeaderTest/library/library.hpp
  tests/ECMGenerateExportHeaderTest/library/main.cpp

To: kossebau
Cc: chehrlic, dfaure, cgiboudeaux, kde-frameworks-devel, kde-buildsystem, 
LeGast00n, GB_2, bencreasy, michaelh, ngraham, bruns

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

[Friedrich W. H. Kossebau]

kossebau added a comment.


  In D23789#540033 , @dfaure wrote:
  
  > I found confirmation in cmake's 
Tests/RunCMake/GenerateExportHeader/reference/
  
  
  Not sure what you exactly mean, can you please specify confirmation for what? 
And what this recommends us to do? :)

REPOSITORY
  R240 Extra CMake Modules

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

To: kossebau
Cc: chehrlic, dfaure, cgiboudeaux, kde-frameworks-devel, kde-buildsystem, 
LeGast00n, GB_2, bencreasy, michaelh, ngraham, bruns

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

[Friedrich W. H. Kossebau]

kossebau updated this revision to Diff 67220.
kossebau added a comment.


  - extend *_VERSION_DEPRECATED to expect third argument "text" first 
experimental consumption time support for "text" output with GCC compiler
  - make *_NO_DEPRECATED a proper shortcut for DISABLE_BEFORE_AND_AT=CURRENT
  - extend unit test to cover *_NO_DEPRECATED

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23789?vs=66670&id=67220

BRANCH
  addgenerateexportheader

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

AFFECTED FILES
  docs/module/ECMGenerateExportHeader.rst
  modules/ECMGenerateExportHeader.cmake
  tests/CMakeLists.txt
  tests/ECMGenerateExportHeaderTest/CMakeLists.txt
  tests/ECMGenerateExportHeaderTest/consumer-no-deprecated/CMakeLists.txt
  tests/ECMGenerateExportHeaderTest/consumer-no-deprecated/main.cpp
  tests/ECMGenerateExportHeaderTest/consumer/CMakeLists.txt
  tests/ECMGenerateExportHeaderTest/consumer/main.cpp
  tests/ECMGenerateExportHeaderTest/format_version/CMakeLists.txt
  tests/ECMGenerateExportHeaderTest/format_version/main.cpp
  tests/ECMGenerateExportHeaderTest/library/CMakeLists.txt
  tests/ECMGenerateExportHeaderTest/library/library.cpp
  tests/ECMGenerateExportHeaderTest/library/library.hpp
  tests/ECMGenerateExportHeaderTest/library/main.cpp

To: kossebau
Cc: chehrlic, dfaure, cgiboudeaux, kde-frameworks-devel, kde-buildsystem, 
LeGast00n, GB_2, bencreasy, michaelh, ngraham, bruns

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

[David Faure]

dfaure added a comment.


  I found confirmation in cmake's Tests/RunCMake/GenerateExportHeader/reference/
  
$ grep -r LIBSHARED_DEPRECATED\  . | grep -w define
./UNIX_DeprecatedOnly/libshared_export.h:#  define LIBSHARED_DEPRECATED 
__attribute__ ((__deprecated__))
./Win32-Clang/libshared_export.h:#  define LIBSHARED_DEPRECATED 
__attribute__ ((__deprecated__))
./Win32/libshared_export.h:#  define LIBSHARED_DEPRECATED 
__declspec(deprecated)
./UNIX/libshared_export.h:#  define LIBSHARED_DEPRECATED __attribute__ 
((__deprecated__))
./MinGW/libshared_export.h:#  define LIBSHARED_DEPRECATED __attribute__ 
((__deprecated__))

REPOSITORY
  R240 Extra CMake Modules

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

To: kossebau
Cc: chehrlic, dfaure, cgiboudeaux, kde-frameworks-devel, kde-buildsystem, 
LeGast00n, GB_2, bencreasy, michaelh, ngraham, bruns

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

[Friedrich W. H. Kossebau]

kossebau added a comment.


  In D23789#539940 , @dfaure wrote:
  
  > I see the theoretical problem, but how could this ever be a problem in 
practice?
  >  On Unix all compilers support the same syntax (`__attribute__ 
((__deprecated__))`), so you'd have to be on Windows, build a library with 
mingw, and then try to use it with MSVC, or vice-versa? I wonder if this even 
works. AFAIK it doesn't (hence the compiler-specific binary installers for Qt, 
for instance).
  
  
  No idea yet, I have no overview about which compilers support what, so far 
only tested with my local gnu compiler to have an test example how the 
respective values/macros need to be propagated :) Sounds promising then we 
could be able to leave complicated logic to cmake time

REPOSITORY
  R240 Extra CMake Modules

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

To: kossebau
Cc: chehrlic, dfaure, cgiboudeaux, kde-frameworks-devel, kde-buildsystem, 
LeGast00n, GB_2, bencreasy, michaelh, ngraham, bruns

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

[David Faure]

dfaure added a comment.


  > Which might be an issue for people who would like to use different compiler 
on the same system, both building against the same generated export header file.
  
  I see the theoretical problem, but how could this ever be a problem in 
practice?
  On Unix all compilers support the same syntax (`__attribute__ 
((__deprecated__))`), so you'd have to be on Windows, build a library with 
mingw, and then try to use it with MSVC, or vice-versa? I wonder if this even 
works. AFAIK it doesn't (hence the compiler-specific binary installers for Qt, 
for instance).
  
  Oh and if C++14 support is enabled, we could use `[[deprecated("use foo 
instead")]]` which is standard and portable :-)
  (requires C++17 for enums and namespaces)

REPOSITORY
  R240 Extra CMake Modules

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

To: kossebau
Cc: chehrlic, dfaure, cgiboudeaux, kde-frameworks-devel, kde-buildsystem, 
LeGast00n, GB_2, bencreasy, michaelh, ngraham, bruns

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

[Friedrich W. H. Kossebau]

kossebau added a comment.


  Quick Update (week 40):
  Locally have added experimental code to even set the proper attribute for GCC 
compiler, so we get e.g.:
  
/home/koder/Kode/kdegit/kf5/frameworks/kparts/tests/notepad.cpp: In 
constructor ‘NotepadPart::NotepadPart(QWidget*, QObject*, const QVariantList&)’:
/home/koder/Kode/kdegit/kf5/frameworks/kparts/tests/notepad.cpp:46:55: 
warning: ‘KAboutData& KAboutData::setProgramIconName(const QString&)’ is 
deprecated: Use QApplication::setWindowIcon [-Wdeprecated-declarations]
  
  Still need to clean up the code and update the patch.
  
  Open questions you might want to comment on already:
  a) Also plan to use the version info and prefix the message text with a 
"Since major.minor.", as this info also is interesting usually.
  b) A thing I am unsure about is: CMake's `generate_export_header` does the 
compiler detection in cmake code and then generates code for that very 
compiler. Which then ends up with the installed/deployed include files. Which 
might be an issue for people who would like to use different compiler on the 
same system, both building against the same generated export header file.
  Draft code currently moves compiler detection  for deprecated(text) support 
in generated code, as I have no clue whether one can expect all compilers to 
digest what the system compiler when it comes to this attribute?

REPOSITORY
  R240 Extra CMake Modules

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

To: kossebau
Cc: chehrlic, dfaure, cgiboudeaux, kde-frameworks-devel, kde-buildsystem, 
LeGast00n, GB_2, bencreasy, michaelh, ngraham, bruns

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

[Christian Ehrlicher]

chehrlic added a comment.


  In D23789#538985 , @kossebau wrote:
  
  > >> - why has all Qt code not yet been adapted to 
QT_DEPRECATED_VERSION/QT_DEPRECATED_VERSION_X, are there places where those 
macros should not be used, but the version-less ones?
  > > 
  > > Because noone wanted to do the work and it was added late in the Qt5 
lifetime -> A lot of stuff was deprecated for a long time already (in Qt4 
times) and there is was a replacement since Qt5.0.0 so the macro was not needed 
(even though a lot of people got very angry about it). I added it for some new 
signals which created a lot of discussion since the old ones are widely used.
  >
  > Can you point to those discussions? Would be curious to learn what people's 
thought are.
  
  
  https://lists.qt-project.org/pipermail/development/2019-March/035343.html
  
  It's most likely a discussion when a deprecated / replaced function should 
really create a warning (esp. when the replacement was just recently added) and 
when the function should be removed completely - should a function deprecated 
in Qt5 really be removed in Qt6 or wait for Qt7 or don't remove it at all.
  
  > 
  > 
  >>> - why did you go for both QT_DEPRECATED_VERSION & 
QT_DEPRECATED_VERSION_X, are there places where no message will be wanted?
  >> 
  >> Just for consistency - we've QT_DEPRECATED and QT_DEPRECATED_X but I think 
QT_DEPRECATED should be deprecated by itself. I hope no reviewer will accept a 
QT_DEPRECATED anymore.
  > 
  > So you would also think that message-less variants will not be needed I 
understand. Okay. From what I tested with experimentally deploying the macros 
on some KF modules I almost always could add a useful message, so running short 
of reasons for keeping a message-less variant :)
  
  I think it should be enough to have one macro nowadays :)

REPOSITORY
  R240 Extra CMake Modules

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

To: kossebau
Cc: chehrlic, dfaure, cgiboudeaux, kde-frameworks-devel, kde-buildsystem, 
LeGast00n, GB_2, bencreasy, michaelh, ngraham, bruns

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

[Friedrich W. H. Kossebau]

kossebau added a comment.


  Thanks for your reply, Christian :)
  
  In D23789#538876 , @chehrlic wrote:
  
  > In D23789#536338 , @kossebau 
wrote:
  >
  > > Actual questions I have:
  > >
  > > - why is QT_DEPRECATED_WARNINGS_SINCE not officially documented? like, 
any plans to change that macro to something else?
  >
  >
  > Just forgot it (and also the reviewers) I would guess. There is no plan to 
change it, at least none I'm aware of. I'm looking for an automatic generation 
of this macro.
  
  
  Okay. So will keep it documented as part of public API here then.
  
  > Since Qt6 switches to CMake I can maybe borrow some stuff from you ;)
  
  Sure :) As others commented and what I think as well, this should be even 
candidate for being added to upstream in some variant, once it has proven to 
work out.
  
  >> - why has all Qt code not yet been adapted to 
QT_DEPRECATED_VERSION/QT_DEPRECATED_VERSION_X, are there places where those 
macros should not be used, but the version-less ones?
  > 
  > Because noone wanted to do the work and it was added late in the Qt5 
lifetime -> A lot of stuff was deprecated for a long time already (in Qt4 
times) and there is was a replacement since Qt5.0.0 so the macro was not needed 
(even though a lot of people got very angry about it). I added it for some new 
signals which created a lot of discussion since the old ones are widely used.
  
  Can you point to those discussions? Would be curious to learn what people's 
thought are.
  
  >> - why did you go for both QT_DEPRECATED_VERSION & QT_DEPRECATED_VERSION_X, 
are there places where no message will be wanted?
  > 
  > Just for consistency - we've QT_DEPRECATED and QT_DEPRECATED_X but I think 
QT_DEPRECATED should be deprecated by itself. I hope no reviewer will accept a 
QT_DEPRECATED anymore.
  
  So you would also think that message-less variants will not be needed I 
understand. Okay. From what I tested with experimentally deploying the macros 
on some KF modules I almost always could add a useful message, so running short 
of reasons for keeping a message-less variant :)
  
  > The main problem with the macro is that you have to create a new one for 
each version so it will become lengthy. But I could not find a better solution 
somewhere - in the end it's only macro magic from the 80s... :(
  
  Also had no better idea, but then I am also nowhere a C++ macro magician, I 
operate by manual mostly :) Thankfully with the proposed cmake-generated 
approach here it's more a matter of adding another version to the 
`DEPRECATION_VERSIONS` list, and nobody looks into generated code anyway.

REPOSITORY
  R240 Extra CMake Modules

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

To: kossebau
Cc: chehrlic, dfaure, cgiboudeaux, kde-frameworks-devel, kde-buildsystem, 
LeGast00n, GB_2, bencreasy, michaelh, ngraham, bruns

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

[Christian Ehrlicher]

chehrlic added a comment.


  In D23789#536338 , @kossebau wrote:
  
  > Actual questions I have:
  >
  > - why is QT_DEPRECATED_WARNINGS_SINCE not officially documented? like, any 
plans to change that macro to something else?
  
  
  Just forgot it (and also the reviewers) I would guess. There is no plan to 
change it, at least none I'm aware of. I'm looking for an automatic generation 
of this macro. Since Qt6 switches to CMake I can maybe borrow some stuff from 
you ;)
  
  > - why has all Qt code not yet been adapted to 
QT_DEPRECATED_VERSION/QT_DEPRECATED_VERSION_X, are there places where those 
macros should not be used, but the version-less ones?
  
  Because noone wanted to do the work and it was added late in the Qt5 lifetime 
-> A lot of stuff was deprecated for a long time already (in Qt4 times) and 
there is was a replacement since Qt5.0.0 so the macro was not needed (even 
though a lot of people got very angry about it). I added it for some new 
signals which created a lot of discussion since the old ones are widely used.
  
  > - why did you go for both QT_DEPRECATED_VERSION & QT_DEPRECATED_VERSION_X, 
are there places where no message will be wanted?
  
  Just for consistency - we've QT_DEPRECATED and QT_DEPRECATED_X but I think 
QT_DEPRECATED should be deprecated by itself. I hope no reviewer will accept a 
QT_DEPRECATED anymore.
  
  The main problem with the macro is that you have to create a new one for each 
version so it will become lengthy. But I could not find a better solution 
somewhere - in the end it's only macro magic from the 80s... :(
  But for a library such a macro should be mandatory - your blog post explains 
the problem very good.

REPOSITORY
  R240 Extra CMake Modules

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

To: kossebau
Cc: chehrlic, dfaure, cgiboudeaux, kde-frameworks-devel, kde-buildsystem, 
LeGast00n, GB_2, bencreasy, michaelh, ngraham, bruns

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

[Friedrich W. H. Kossebau]

kossebau updated this revision to Diff 66670.
kossebau added a comment.


  add first set of unit tests covering the main functionality.
  Feedback on principal approach wanted!
  
  Also add some fixes to actual macro code found by the tests already :)

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23789?vs=65854&id=66670

BRANCH
  addgenerateexportheader

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

AFFECTED FILES
  docs/module/ECMGenerateExportHeader.rst
  modules/ECMGenerateExportHeader.cmake
  tests/CMakeLists.txt
  tests/ECMGenerateExportHeaderTest/CMakeLists.txt
  tests/ECMGenerateExportHeaderTest/consumer/CMakeLists.txt
  tests/ECMGenerateExportHeaderTest/consumer/main.cpp
  tests/ECMGenerateExportHeaderTest/format_version/CMakeLists.txt
  tests/ECMGenerateExportHeaderTest/format_version/main.cpp
  tests/ECMGenerateExportHeaderTest/library/CMakeLists.txt
  tests/ECMGenerateExportHeaderTest/library/library.cpp
  tests/ECMGenerateExportHeaderTest/library/library.hpp
  tests/ECMGenerateExportHeaderTest/library/main.cpp

To: kossebau
Cc: chehrlic, dfaure, cgiboudeaux, kde-frameworks-devel, kde-buildsystem, 
LeGast00n, GB_2, bencreasy, michaelh, ngraham, bruns

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

[Friedrich W. H. Kossebau]

kossebau added a subscriber: chehrlic.
kossebau added a comment.


  @chehrlic Hi. As I just discovered, you are the author of the macros for Qt 
(commit 
)
 which I took as inspiration/blue print when designing this here. So, curious 
what you think of your work now and if you can already point out things where 
you see potential for improvements. And also curious what you think of this 
approach here :)
  
  Actual questions I have:
  
  - why is QT_DEPRECATED_WARNINGS_SINCE not officially documented? like, any 
plans to change that macro to something else?
  - why has all Qt code not yet been adapted to 
QT_DEPRECATED_VERSION/QT_DEPRECATED_VERSION_X, are there places where those 
macros should not be used, but the version-less ones?
  - why did you go for both QT_DEPRECATED_VERSION & QT_DEPRECATED_VERSION_X, 
are there places where no message will be wanted?

REPOSITORY
  R240 Extra CMake Modules

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

To: kossebau
Cc: chehrlic, dfaure, cgiboudeaux, kde-frameworks-devel, kde-buildsystem, 
LeGast00n, GB_2, bencreasy, michaelh, ngraham, bruns

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

[Friedrich W. H. Kossebau]

kossebau added a comment.


  Quick Update (week 39):
  took me a bit longer to design and write tests, not trained to write such 
things in cmake code, all its magic & caches & scopes + own undiscovered typos 
make this not (yet?) fun, and I am not certain I have done things properly, so 
will be looking forward to feedback.
  Currently cleaning up and documenting the code, added tests should land 
tonight with an update here (and they already found an error in the so far 
prototype code :) ).
  
  (FTR: Meanwhile saw that Qt actually also introduced QT_DEPRECATED_VERSION_X 
next to QT_DEPRECATED_VERSION, in the very same commit. Not sure what version 
of qglobal.h file I had looked or why I came to claim something else)

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

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

[Friedrich W. H. Kossebau]

kossebau added a comment.


  Quick update:
  Currently still busy trying to get unit tests done, half-way through that. 
ETA begin of upcoming week.
  Next plan: see how only having the 3-arg-`FOO_DEPRECATED_VERSION(major. 
minor, message)` would work by using that in the experimental patches done for 
some KF repos.

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

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

[David Faure]

dfaure added a comment.


  I wouldn't offer both variants, but rather recommend always adding a hint. At 
worse that hint can be "See method documentation" or an empty string. I think 
we're just over-complicating things otherwise, for no actual gain, since 
providing a hint is good practice anyway.

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

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

[Friedrich W. H. Kossebau]

kossebau added inline comments.

INLINE COMMENTS

> dfaure wrote in ECMGenerateExportHeader.cmake:87
> 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...

I tried to reword things a bit, also changed all other places talking about 
build time to be explicit.

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

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

[Friedrich W. H. Kossebau]

kossebau updated this revision to Diff 65854.
kossebau marked 3 inline comments as done.
kossebau added a comment.


  - reword documentation based on dfaure's comments
  - bump since-version to 5.64.0, as currently targetted introduction version
  
  Hope is review can be finished at 5.63 tagging time, so this macro would be
  added right after that and all the ideally prepared patches for the KF
  modules, so there is a full KF release cycle of testing in git master before
  things get rolled out to users.

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23789?vs=65678&id=65854

BRANCH
  addgenerateexportheader

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

AFFECTED FILES
  docs/module/ECMGenerateExportHeader.rst
  modules/ECMGenerateExportHeader.cmake

To: kossebau
Cc: dfaure, cgiboudeaux, kde-frameworks-devel, kde-buildsystem, LeGast00n, 
GB_2, bencreasy, michaelh, ngraham, bruns

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

[Friedrich W. H. Kossebau]

kossebau added a comment.


  @dfaure Thanks for first in-detail feedback, good to get a feeling this is 
not totally insane over-engineered stuff to other people'e eyes :)
  
  In D23789#528994 , @dfaure wrote:
  
  > 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.
  
  
  It's basically trying to solve 2 problems at the same time, as they both 
would be solved by the same macro techniques:
   a) allow control over warnings and API visibility to people building against 
the library (incl. 2 level settings by additional library group level)
  b) allow building of the library itself with legacy code stripped
  
  If there are suggestions for better wording of variables/arguments, or 
improved ordering/threading of the documentation, all eager to hear. Myself 
still too much into thinking about details to be able to simulate a 
first-contact experience again, where ideas are taken from docs and term 
semantics :)
  
  > 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.
  
  As there might not be always a simple hint text, I would still like to also 
keep the just-version variant of that macro. So if we overload the macro name 
to support both 2 (major, minor) and 3 (major, minor, message) arguments,  any 
simple way to do default argument value with C++ preprocessor macros? Never 
wrote such code myself, all things I just found do complicated dances around 
__VA_ARGS__ of which I none could resolve to a simple application for our 
single-parameter default value yet.
  
  So, any C++ macro  magicians around to be able to tell how to support both
  
KIOCORE_DEPRECATED_VERSION(5, 0)
KIOCORE_DEPRECATED_VERSION(5, 0, "use uiDelegate()")
  
  while also making sure there is only one argument, and not many? (at least 
that would be my ambition here to catch this ourselves, instead of just taking 
__VA_ARGS__ and pass it on, to fail later if >1 args are used.

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

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 
> ``_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

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

[Friedrich W. H. Kossebau]

kossebau retitled this revision from "WIP: Add ECMGenerateExportHeaders, for 
improved handling of deprecated API" to "RFC: Add ECMGenerateExportHeaders, for 
improved handling of deprecated API".

REPOSITORY
  R240 Extra CMake Modules

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

To: kossebau
Cc: cgiboudeaux, kde-frameworks-devel, kde-buildsystem, LeGast00n, GB_2, 
bencreasy, michaelh, ngraham, bruns