Export defines PUBLIC so that headers work properly
COMMON_TARGET_DEFINITIONS needs to be PUBLIC on the built libraries so that it is exported to the sleefConfig.cmake.
Otherwise, the sleef.h header does not work properly. SLEEF_STATIC_LIBS and ENABLEFLOAT128 are two examples that need to be exported. I am unsure if all defines need to be exported but currently the defines are not distinguished in any way.
Hi @xsacha , could you please explain what are you trying to fix here? You say that "sleef.h doesn't work properly". What do you mean by that? What is this patch allowing you to do with sleef.h that was not possible to do before?
What I mean is that if you include the header, it will not have the same defines that were used at compile time.
if you include the header, it will not have the same defines that were used at compile time
Hi , thank you for explaining, but this patch is still not cleat to me. Why would sleef.h need to have the same defines that are used at build time? Becasue from what you are saying, I understand that you are thinking of having those defines in sleef.h, did I get it right? If that's the case, could you provide and example when this is needed?
Apologies for asking you to explain more, but I am not a cmake expert and I really don't see why we should export those defines.
Any defines provided through the CMake are not sent when linking the project unless you use the correct CMake functions.
PUBLIC is INTERFACE + PRIVATE combined. By default these are only private (used as compile time but not for anything that links it). By making it PUBLIC, anyone who externally links to the project will also get the defines.
By making it PUBLIC, anyone who externally links to the project will also get the defines.
Does this mean that you need to gives visibility to those defines to other CMake projects that include SLEEF as a subproject? Is this what you mean by "externally link"? All these defines are doing is just selecting how to recompile the sources, why would an external project need to see them?
Because when I tried to use sleef headers, it threw undefined linker errors because variables such as avx and float128 were defined at compile time but not when I linked to it in CMake.
This commit fixed these errors.
@fpetrogalli How about this patch? I think this patch is safe to merge.