Brad King wrote:

> On 08/15/2013 08:37 AM, Stephen Kelly wrote:
>>  target_compiler_feature(<target> <PUBLIC|PRIVATE>
>>    REQUIRED <feature1> [<feature2> ...])
>>  target_compiler_feature(<target> <PUBLIC|PRIVATE>
>>    OPTIONAL <feature> DEFINE <define_name>)
> 
> Doesn't this require the language (CXX) to be specified somewhere?
> Perhaps the feature names should start with <lang>_ e.g. CXX_final.

I considered that, and I considered adding LANG <lang> to the command 
signatures, and I considered naming the command with the language 
(target_cxx_compiler_feature). 

I didn't pick one, partly because it's a relatively minor detail we can 
decide on after the larger issues below are resolved.


>> This is better in many noteworthy ways to the cxx11 topic in the stage.
>> 
>> Brad, do you have any response to any part of anything I wrote about that
>> topic?
> 
> The detection of features available for a given compiler and the
> usage requirements to require/enable them are orthogonal issues
> on the implementation side but may overlap in the CMake interface.

Yes. Even if we ignore the usage requirements angle, there are enough 
reasons to not accept the currently proposed interface in the topic though.

> I like your proposed target_compiler_feature feature for the latter.

Great.

> 
> As for detection, I like that Eike's solution will work without
> hard-coding knowledge for every compiler.  I also like hard-coding
> the answers for known compilers whose id/version can be reliably
> detected to avoid needless try_compile calls on every project.

Yes.

> Furthermore, your argument about partial implementations of features
> versus the documented later version introducing them is important.

Yes.

> It is also nice to have it builtin to the platform information
> modules for use with target_compiler_feature.  I'd like to see a
> solution that hard-codes the answers when known but also knows when
> it does not know and can run a detection step.  

Yes.

> This will be tricky
> because the platform files are too early to use try_compile so some
> kind of on-demand check may be needed.  Ideas?

That should be easy. Encode them into the platform files where known, and 
implement target_compiler_feature to run a detection if 
CMAKE_CXX_COMPILER_FEATURES is not set.

> I think Eike's topic will be a fantastic reference for the test
> case code 

I agree that Tests/Module/FindCXXFeatures/CMakeLists.txt is a useful 
reference. However, I don't think the interface in the topic should be 
merged into master. We agree that target_compiler_feature is a better 
interface, and hopefully Eike does too.

Eikes interface has some problems which I noted before here which make it 
useless for libraries (or at least too incomplete to be useful) because the 
result of the feature tests can't be used in public header files:

 
http://thread.gmane.org/gmane.comp.programming.tools.cmake.devel/6726/focus=7715

Additionally, there is a 'cross platform trap' in the topic as it is 
currently. Someone implementing a project on Windows using MSVC might write 
this (assuming they were creating an executable, not a library because of 
the above problem):

 find_package(CXXFeatures)
 if (CXXFeatures_class_override_final_FOUND)
   add_definitions(-DGrantlee_FINAL=final)
 endif()

It looks cross-platform, but it is not. 

When they port the code to GCC, they need to add the ${CXX11_COMPILER_FLAGS} 
somewhere (using the modern add_compile_options? No, using CMAKE_CXX_FLAGS 
because it is _FLAGS).

My proposed interface does not have that problem because adding the compile 
option for -std=c++11 (or -std=c++1y) is set internally.

As you said though, the detection code and the detection results encoded 
into the tests are orthogonal to the interface issues. I think those parts 
should be kept and refactored.

I don't think the topic should be merged as-is because of the problems with 
the interface we know about so far. If you merge it and we also merge the 
interface which we agree to be better, we'll have two competing interfaces.

> and the expected results and will be useful for you to
> write a more complete solution.

I'm sure Eike can do it too. Or at least get started with encoding the 
detection results into platform files and refactoring the detection code out 
of the find module so it can be used from the command.

> I like the target_compiler_feature
> syntax better than find_package(CXXFeatures), but something like the
> latter may be needed to make CMake-time decisions based on available
> features rather than waiting until preprocessing time.

Users could do the same as I suggest the target_compiler_feature does:

 if (CMAKE_CXX_COMPILER_FEATURES)
   list(FIND CMAKE_CXX_COMPILER_FEATURES final idx)
   if(idx EQUAL -1)
     set(HAVE_FINAL 0)
   else()
     set(HAVE_FINAL 1)
   endif()
 else()
   include(DetectCompilerFeature)
   detect_compiler_feature(final HAVE_FINAL)
 endif()

 if(HAVE_FINAL)
   # ...
 endif()


Or, even better, 


 include(DetectCompilerFeature)
 # Checks CMAKE_CXX_COMPILER_FEATURES first as above and try_compile 
 # only if needed.
 detect_compiler_feature(final HAVE_FINAL) 


I also still think there should be a way for CMake to write a header file 
instead of passing the results on the command line, as Eike designed it and 
as we've been discussing so far, and as I implemented with 
COMPILE_DEFINITIONS in the proof of concept I sent.

The header file would have the preprocessor tests and version checks for the 
known compilers. That is, the generated header file would be the same on all 
platforms, and would look something like qcompilerdetection.h, but with a 
customizable prefix for the macros. 

That way, a static_assert implementation and an enable_if implementation etc 
can also be generated.

 include(DetectCompilerFeature)
 write_compiler_detection_header(
   FILE ${CMAKE_CURRENT_BINARY_DIR}/grantlee_compiler_detection.h
   PREFIX Grantlee_
 )

Then in grantlee_global.h I include grantlee_compiler_detection.h and I can 
write code like this:

 struct A Grantlee_FINAL
 {
   int i;
 private:
   A(const A &other) Grantlee_EQUALS_DELETE;
   A& operator=(const A &other) Grantlee_EQUALS_DELETE;
   // Or write_compiler_detection_header could also create a 
   // Grantlee_DISABLE_COPY to use instead of the above.

   template<typename B>
   typename Grantlee_ENABLE_IF<
     Grantlee_IS_CONVERTIBLE<B, int>::value, B>::type getAs()
   {
     return i;
   }
 };

 Grantlee_STATIC_ASSERT(sizeof(A) == sizeof(int));
 
For is_convertible etc, there is a BSD licensed type_traits compatibility 
header here, which Qt 5 uses too and which could be distributed with CMake:

 http://sparsehash.googlecode.com/svn-history/r74/trunk/src/google/type_traits.h


This would be an interface that would be desirable, useful, complete and 
future-proof (c++1y). It's similar to the kinds of things provided by 
Boost.Config and by qcompilerdetection.h, but the use-cases of the CMake 
version are partly for cases where those are not used, and partly to get the 
information at cmake time.

All of the above is orthogonal and independent of the interface usage 
requirements, which is an added benefit of my proposed design.

Thanks,

Steve.


--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers

Reply via email to