[cmake-developers] Please review CXXFeatures.cmake

[Rolf Eike Beer]

Stephen Kelly wrote:
> Rolf Eike Beer wrote:
> > Am Sonntag, 28. April 2013, 13:57:26 schrieb Rolf Eike Beer:
> >> One question I see increasingly often is "how do I test for C++11
> >> support" or for specific parts of that. For 2.8.12 I plan to include the
> >> check module I wrote for that a while back, and that I have reworked in
> >> the last weeks. You can find the current state in the "rework" branch of
> > 
> >> this repository:
> > Ok, I finally put it into the CMake repository:
> I have some feedback:
> 
> 1)
> 
> A recent commit in the topic does this:
> 
>  -g++ seems to support constexpr since 4.5 even if their support page says
> 4.6
> 
> It has been common for compilers to implement support for features like
> this, but have some fundamental brokenness in earlier versions. Your test
> which enables the use of constexpr might not exercise the feature enough to
> hit the fundamental brokenness, but that doesn't mean it's an edge-case. It
> would be better not to second-guess the version gcc says it supports
> features in.

I'm happy for all updated checks, so push a patch on that branch ;)

> 2)
> 
> Given that you're gathering the versions of each feature availability
> anyway, and given that boost.config and qcompilerdetection.h have the same
> information, there is no need for all users of the module to run all these
> try_compiles for all projects. Think of the energy waste :)!
> 
> I suggest you use CMAKE_CXX_COMPILER_ID and CMAKE_CXX_COMPILER_VERSION to
> hardcode the features. You could even do so for known compilers, and leave
> the try_compile stuff for not-known compilers if you really want to, but I
> don't think that's worthwhile maintenance.

We already found out that this is a bad idea for Apple, I still don't 
completely get it right for g++ and XL, and it isn't the way that CMake works 
for other things (I'm thinking of e.g. OpenMP). And qcompilerdetection.h is a 
good example of how I would not want it, last time I looked they just 
deactivated every feature on Clang. The idea I have about CMake is that it 
would check for stuff on any compiler I throw on it, so it would just report 
correct results for this that haven't been tested before (PGI? MipsPro? gcc 
2?).

> 3)
> 
> CXX11_COMPILER_FLAGS is not really the 'modern' way to specify compiler
> flags, as of recent CMake versions. See for example the reasoning here:
> 
>  http://thread.gmane.org/gmane.comp.programming.tools.cmake.devel/6876/focus
> =6890
> 
> regarding *_FLAGS, which is a whitespace delimited string, and
> COMPILE_OPTIONS, which is a regular ';' delimited CMake list.
> 
> The modern way to do something like that is a target property and a way to
> set the default.
> 
> See for example commit bd3496300262bd26073ce03e020731c592249148 (Refactor
> generation of shared library flags, 2012-05-30). The
> set(CMAKE_POSITION_INDEPENDENT_CODE ON) feature is enabled by
> 
>  this->SetPropertyDefault("POSITION_INDEPENDENT_CODE", 0);
> 
> in cmTarget.
> 
> For more/similar, see commit cd1fa537a03377974a4d0a27e592785f931a41e5 (Add a
> COMPILE_OPTION for a VISIBILITY_INLINES_HIDDEN target property.,
> 2013-05-23) and 0e9f4bc00c6b26f254e74063e4026ac33b786513 (Introduce target
> property <LANG>_VISIBILITY_PRESET, 2013-05-18).
> 
> I added a patch to my clone with the start of
> CMAKE_CXX_COMPILE_OPTIONS_STD_CXX11
> 
>  https://gitorious.org/~steveire/cmake/steveires-cmake/commits/std-cxx-targe
> t-property
> 
> but it didn't get anywhere yet:
> 
>  http://thread.gmane.org/gmane.comp.programming.tools.cmake.devel/4106

I'm not sure about this. I just return the flag that is needed. The user has 
to put this into the compiler flags on a global or per target base. I don't do 
anything with it.

> 4)
> 
> The COMPILE_OPTIONS for clang+apple might have to include -stdlib=libc++ for
> binary compatibility reasons if any of the dependencies use c++11 std
> library API in their interface and use libc++.
> 
> See what I wrote about that here:
> 
>  http://thread.gmane.org/gmane.comp.programming.tools.cmake.devel/5813

I don't see how this is different with and without C++11, so how does it 
affect this module in a way that would not affect the user anyway?

> 5)
> 
> This is not nice API:
> 
>  #if defined (CXXFEATURES_NULLPTR_FOUND)
>      void *nix = nullptr;
>  #else /* CXXFEATURES_NULLPTR_FOUND */
>      void *nix = 0;
>  #endif /* CXXFEATURES_NULLPTR_FOUND */
> 
> 
> Much better would be:
> 
>      void *nix = CXXFEATURES_NULLPTR;
> 
> where -DCXXFEATURES_NULLPTR=0 or -DCXXFEATURES_NULLPTR=nullptr.
> 
> See what Qt does for other similar API decisions on what should be defined
> to something (like nullptr, constexpr, final etc), and what should be a
> 'guard' define like above (eg lambdas, variadic templates etc).
> 
> Note also that by defining the CXXFEATURES_FINAL to something, you get to
> use the 'sealed' extension, which does the same thing, and works with
> VC2005. See qcompilerdetection.h.

The module returns just a list of CMake flags. How this is passed to the user 
(header, defines, whatever) is currently something the user must decide. I 
will not do anything fancy in the testcase for now.

> 6)
> 
> It is also unfortunate that because your solution is to define things on the
> command line, you can't easily define something for static_assert. See
> CMakeStaticAssert for example, which works for all compilers on the
> dashboard.
> 
> For that reason, I'll continue to recommend that anyone using Qt and CMake
> does not use your module, but instead uses the defines from Qt. Ditto for
> anyone using boost.
> 
> A missing piece that your module provides is determining the features at
> CMake time. However, if Qt is found, that can be done with a single
> try_compile of qglobal.h to get that information into a form usable to
> CMake. That's a feature I can add to Qt 5 soon, and something similar can be
> done to process the information from boost.config.

And if you have Clang you could use it's feature. And if you have neither you 
need to implement everything by hand anyway. For the moment I will not even 
more choices, but simply use the one method that works everywhere, at least 
eventually I hope. But I'm absolutely open for improvements, just send a 
patch.

Eike
-- 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://public.kitware.com/pipermail/cmake-developers/attachments/20130805/25f702fe/attachment.sig>