Skip to content

Avoid duplicate C++ standard flags if CMAKE_CXX_STANDARD is set #999

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

Merged
merged 1 commit into from
Aug 17, 2017

Conversation

dean0x7d
Copy link
Member

@dean0x7d dean0x7d commented Aug 13, 2017

Fixes #998.

CMAKE_CXX_STANDARD is only available on CMake >= 3.1. If the flag is set, we avoid initializing PYBIND11_CPP_STANDARD.

I didn't add any tests because all the CI builds currently rely on PYBIND11_CPP_STANDARD (for older CMake support) and there's no need to complicate things there.

CMAKE_CXX_STANDARD is only available on CMake >= 3.1. If the flag is
set, we avoid initializing PYBIND11_CPP_STANDARD.
@jagerman
Copy link
Member

This is going to take care of the global, but what if the CXX_STANDARD property is set on a target?

@dean0x7d dean0x7d modified the milestone: v2.2 Aug 13, 2017
@dean0x7d
Copy link
Member Author

This is going to take care of the global, but what if the CXX_STANDARD property is set on a target?

We can't do anything about it as far as I know. It would be set by users after pybind11_add_module() has finished or the pybind11:: targets have been constructed.

We could conditionally, on CMake >= 3.1, set the per-target CXX_STANDARD and let CMake sort it out. But this starts to get really messy if we need to support both the old and new standard styles. I'm also not completely happy with the CXX_STANDARD implementation because it requires CMake to actually know about the target compiler and C++ standard. For example, older CMake versions may not know about c++17 or c++1z flags and refuse to set them or even the basic 11/14 flags on some not so common compilers (I've had issues with ICC on early CMake 3.x versions). Also, the way the CXX_STANDARD_REQUIRED property works (on/off) isn't really ideal (it would be nice if it was the minimum required standard, while CXX_STANDARD was the preferred if available standard).

All in all, I think it would be best to stick with our existing standard variable and just make this provision for the global CMAKE_CXX_STANDARD without too much messing around with the properties.

@dean0x7d dean0x7d merged commit 76e06c8 into pybind:master Aug 17, 2017
@dean0x7d dean0x7d deleted the cmake branch August 17, 2017 01:17
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

Successfully merging this pull request may close these issues.

2 participants