Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better and more intuitive settings for C++11 and C++14 with different variants #10

Open
drodri opened this issue Apr 16, 2015 · 11 comments
Assignees

Comments

@drodri
Copy link
Contributor

drodri commented Apr 16, 2015

Currently, configuring some library/block to use C++11/14 is a bit complex. There are several options:

1-. Native use of CMake. It can require some conditionals for MultiOS

IF(UNIX)
    IF(CLANG)
        TARGET_COMPILE_OPTIONS(${BII_BLOCK_TARGET} INTERFACE -std=c++11)
....

2-. Use an existing CMake macro, possibly one existing in biicode and retrieved as dependency:

INCLUDE(biicode/cmake/tools)
ACTIVATE_CPP11(INTERFACE ${BII_BLOCK_TARGET})

This one requires one bii find or [requirements] to be written.

PROPOSAL:

From my point of view, it is still a bit complex, so I propose a high level definition, in biicode.conf, something like:

[cpp-std]
     c++11 PUBLIC   # or c++11 libstdc++...

That would be translated to a cmake variable, and biicode, checking that variable would automatically call something in the line of ACTIVATE_CPP11. Surely we have to check for CMake compile features as solution for this, instead of bare compile options.

Pros:

  • Much simpler approach
  • Easily deals with propagating usage to consumers
  • Might be able to deal with the problem of collisions
  • Totally optional, the user can still use the other approaches

Cons:

  • It wires the config to the internal ACTIVATE_CPP11 implementation, that might change with new versions of biicode, so changes, improvements, could break something.
@pfultz2
Copy link

pfultz2 commented Apr 16, 2015

I like the option of putting it in the biicode.conf. I don't see the need to put PUBLIC there. Would the user ever need to specify something other than PUBLIC? Also, it would also be nice to specify the latest:

[cpp-std]
     latest

And it would pick the latest possible. This is very common in the libraries I write. I usually have this in my CMake files:

set(ENABLE_CXXFLAGS_TO_CHECK 
    -std=gnu++1z 
    -std=c++1z
    -std=gnu++14 
    -std=c++14
    -std=gnu++1y 
    -std=c++1y
    -std=gnu++11 
    -std=c++11
    -std=gnu++0x 
    -std=c++0x)

foreach(flag ${ENABLE_CXXFLAGS_TO_CHECK})
    string(REPLACE "-std=" "_" flag_var ${flag})
    string(REPLACE "+" "x" flag_var ${flag_var})
    check_cxx_compiler_flag("${flag}" COMPILER_HAS_CXX_FLAG${flag_var})
    if(COMPILER_HAS_CXX_FLAG${flag_var})
        set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${flag}")
        break()
    endif()
endforeach()

It wires the config to the internal ACTIVATE_CPP11 implementation

Wouldn't this be just an implementation detail? So in newer versions biicode, if the user is requesting C++11 in biicode.conf it would just wire to whichever implementation would get the job done(Perhaps its ACTIVATE_CPP11 now, but in the future it could be ACTIVATE_MODERN_CPP or whatever).

Either way, the biicode.conf should support more than just C++11 and C++03.

@drodri
Copy link
Contributor Author

drodri commented Apr 16, 2015

Yes, I would say this is a nice approach, thanks! I like also the "latest", and including the check_compiler_flag. I thing that new CMake >3.1 compile_features can help with this, but have to check.

The PUBLIC option was intended for the use case when your interface for consumers is <C++11, but the internals of it are >=C++11, for example. Then you want to activate C++11 only in your current library, but your consumers are OK without it. I think this is possible, but not fully sure about the ABI, have to check. Maybe, assume the default is PUBLIC, no need to define it, and could implement optional PRIVATE just for this case if it is in fact possible.

@pfultz2
Copy link

pfultz2 commented Apr 16, 2015

I thing that new CMake >3.1 compile_features can help with this

Well I would prefer not to write out compiler features, although some users may want to do that. The user could list either compiler features:

[cpp-std]
     cxx_constexpr
     cxx_alias_templates

or standards(or latest):

[cpp-std]
     c++11

Of course, if a language standard is specified, it should be used to enable the language on compilers that allow for it to be optionally enabled. So if I specify C++11 or C++03, the library should still compile on MSVC even though it is not a C++03 nor a C++11 compiler.

@drodri
Copy link
Contributor Author

drodri commented Apr 16, 2015

Yes, I was exactly analyzing that just now, and thought about the same interface. But trying out CMake, there are two ways to do exactly that to which both options would translate nicely:

target_compile_features(${BII_main_TARGET} PUBLIC cxx_range_for)

or

set_property(TARGET ${BII_main_TARGET} PROPERTY CXX_STANDARD 11)

The first one can be defined PUBLIC/PRIVATE/INTERFACE, thus applied easily where you want. But does not account for general C++11. I dislike having to define individual features. Furthermore, it is failing in MinGW, tried both 4.8 and 4.9, it seems that still not has support for MinGW, Visual... http://stackoverflow.com/questions/28119552/no-known-features-for-cxx-compiler-when-compiling-with-msvc-2013

The second one is what we want, but has 2 problems: cannot be defined PUBLIC/PRIVATE and cannot be applied to INTERFACE targets, thus it is not suitable for header only libs.

I have posted the question in SO: http://stackoverflow.com/questions/29687331/how-to-define-transitive-cxx-standard-c11-in-cmake

@Manu343726
Copy link
Contributor

I still think that solving cmake UX issues via a custom DSL on top of it is not a good idea. Having a [cpp-std] entry might look nice, but that just covers this use case. Whatever problem you might find in the future will extend that DSL too if we follow with this policy. What about other compilation flags? What about optimization level? canaries? sanitizers?

Let's say a block with the following settings:

if(BIICODE)
    if((CMAKE_CXX_COMPILER_ID MATCHES "gcc") OR (CMAKE_CXX_COMPILER_ID MATCHES "clang"))
        target_compile_options(${BII_BLOCK_TARGET} INTERFACE 
                               -std=c++1y
                               $<$<CONFIG:Debug>:-g3 -O0 -fstack-protector>
                               $<$<CONFIG:Release>:-g0 -O2>)
    else()
        ...
    endif()
endif()

I don't see the point of having -std=c++1y outside the build settings. If enabling c++11 is a short of common option to be on the block settings, why optimizations settings are not? And those settings are not that easy.

@drodri
Copy link
Contributor Author

drodri commented Apr 17, 2015

Then, why Kitware and CMake is developing TARGET_COMPILE_FEATURES (new from cmake 3.1) and supporting the CXX_STANDARD variable?

@pfultz2
Copy link

pfultz2 commented Apr 17, 2015

I don't see the point of having -std=c++1y outside the build settings.

Because biicode can better propagate the setting. If library A uses -std=c++11 and library B uses -std=c++14, you can't use add both flags together if you use both libraries. Instead, biicode(or perhaps cmake) would need to resolve which flag can work for both library A and library B.

If enabling c++11 is a short of common option to be on the block settings, why optimizations settings are not?

First, those settings are already added by default to cmake. Secondly, optimizations settings are optional, and are not need to compile the program.

@drodri
Copy link
Contributor Author

drodri commented Apr 17, 2015

I agree with @pfultz2, optimizations and the standard requirement, they are completely different things, and setting C++ standards is still not a solved problem, thats why CMake is evolving to address this issue with compile_features, though still not reliable enough. So it makes sense to define a simple conf for it (in biicode.conf) and let the internal implementation do it. Now that implementation is open source, it is easier to fix in case of problems, and can more easily evolve and mature with the help of users.

Lets keep with it @franramirez688 , in any case it is totally a opt-in feature, lets do it and check if users like it, if there are too many problems, etc. Thanks!

@franramirez688
Copy link
Contributor

@pfultz2: I've implemented this feature (you can see part of the total code on this PR) and it is based on your system to get the most appropriate flag.

So, we've got doubts about the problem of how CMake performs transitive std flags and how these are overridden by the less dependent block. I mean:

My block is fenix/cpp14 and it depends on pfultz2/cpp11. So, pfultz2/cpp11 CMakeListst.txt has:

 target_compile_options(${BII_BLOCK_TARGET} INTERFACE -std=c++11)

And my own:

 target_compile_options(${BII_BLOCK_TARGET} INTERFACE -std=c++14)

Then, my std flag will be overridden by my dependent block pfultz2/cpp11. How should biicode solve it?

  • biicode should parse COMPILE_OPTIONS from the affected targets and delete the inherited std flags (only if those flags override any other one)
  • biicode should try to compile all the dependent blocks with the most modern flag allowed by user's compiler

Comes to your mind another way more straightforward? Thanks a lot in advance!

@pfultz2
Copy link

pfultz2 commented Apr 23, 2015

biicode should try to compile all the dependent blocks with the most modern flag allowed by user's compiler

Well biicode should compile all blocks(and dependents) using the most modern flag requested by all blocks(it will be the most modern flag allowed by the user's compiler if one of the blocks requested latest). So in your example:

  • pfultz2/cpp11 -> -std=c++11
  • fenix/cpp14 -> -std=c++14

Then pfultz2/cpp11 and fenix/cpp14 blocks will compile with -std=c++14. Now if later the user added the block pfultz2/cpp1z:

  • pfultz2/cpp11 -> -std=c++11
  • fenix/cpp14 -> -std=c++14
  • pfultz2/cpp1z -> -std=c++1z

Now pfultz2/cpp11, fenix/cpp14, and pfultz2/cpp1z blocks will compile with -std=c++1z.

Comes to your mind another way more straightforward?

Well Diego mentioned CMAKE_CXX_COMPILE_FEATURES, I don't know if that makes the flags transitive. Other than that, I don't really see a better way to do it since CMake has to compute the flags for the latest choice.

@franramirez688
Copy link
Contributor

Ok! I'll implement something like that ;)

For now, I'm using target_compile_options to make them transitive so I'll follow working with this feature until CMake improves the target_compile_feature one and biicode can integrate it without any problem.

Thanks a lot for reporting your opinion!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants