Notes |
|
(0040708)
|
Andreas Schuh
|
2016-03-17 07:16
|
|
I attached a patch generated with "git format-patch" against the CMake master branch at commit 811e2974545579d166396ca1ed4f91b0ce7c7990.
This patch avoids the definition of DEFINE_NO_DEPRECATED within the export header altogether. Instead, the export header will have the lines
#if 1 /* DEFINE_NO_DEPRECATED */
# ifndef MyLibrary_NO_DEPRECATED
# define MyLibrary_NO_DEPRECATED
# endif
#endif
The name of the MyLibrary_NO_DEPRECATED macro can be chosen freely using the NO_DEPRECATED_MACRO_NAME option of the generate_export_header function. It can thus also be set to a common macro name for a set of libraries belonging to one project and the defines in the export headers, one for each library target, would not cause an "already defined" preprocessor error with this patch.
I kept the possibility to either use the DEFINE_NO_DEPRECATED option flag of generate_export_header to enable the define of the respective macro in the export header file, or set a CMake variable named DEFINE_NO_DEPRECATED before the call of the generate_export_header function. |
|
|
(0040710)
|
Brad King
|
2016-03-17 08:58
|
|
Thanks. The change looks good but it is not complete. The Module.GenerateExportHeader test in CMake's test suite fails after the change because it has some reference headers that need to be updated for comparison. Please update the patch to include fixes for the test. |
|
|
(0040711)
|
Andreas Schuh
|
2016-03-17 09:30
(edited on: 2016-03-17 09:32) |
|
Ok, I will have a look at the tests.
Before I resubmit a patch, maybe one question to clarify. The documentation of the GenerateExportHeader module specifies that the DEFINE_NO_DEPRECATED option flag of the generate_export_header would decide whether or not the macro is defined with an example on how to set it given a user option in the GUI. But the current module actually does not set DEFINE_NO_DEPRECATED explicitly to FALSE when the option flag was not given. I am wondering if that is deliberate? As noted before, with the previous patch I still consider also a DEFINE_NO_DEPRECATED CMake variable set beforehand.
My question, should the module be rather changed to
if(_GHE_DEFINE_NO_DEPRECATED)
set(DEFINE_NO_DEPRECATED 1)
else()
set(DEFINE_NO_DEPRECATED 0)
endif()
or should the module documentation also mention the alternatively considered CMake variable
or should there be a DEFINE_NO_DEPRECATED target property similar to DEFINE_SYMBOL?
This could of course be another issue to be dealt with separately...
|
|
|
(0040713)
|
Brad King
|
2016-03-17 09:51
|
|
DEFINE_NO_DEPRECATED is an argument to the function, not a variable to be checked or set by the project code. The function sets the variable as an implementation detail for configuring the header file. It should not escape the function's scope on the CMake language side. Therefore one should use just
if(_GHE_DEFINE_NO_DEPRECATED)
set(DEFINE_NO_DEPRECATED 1)
else()
set(DEFINE_NO_DEPRECATED 0)
endif() |
|
|
(0040714)
|
Andreas Schuh
|
2016-03-17 10:17
|
|
Ok, I also fixed this. I thought already it was not intended before. With the new patch, the Module.GenerateExportHeader test passes. |
|
|
(0040715)
|
Brad King
|
2016-03-17 10:42
|
|
|
|
(0040716)
|
Andreas Schuh
|
2016-03-17 10:44
|
|
Thanks! Glad there was a quick resolution for this. |
|
|
(0041222)
|
Kitware Robot
|
2016-06-10 14:21
|
|
This issue tracker is no longer used. Further discussion of this issue may take place in the current CMake Issues page linked in the banner at the top of this page. |
|