Skip to content
This repository has been archived by the owner on Feb 7, 2023. It is now read-only.

Enable c++11 support through CMake functions. #464

Closed
wants to merge 1 commit into from

Conversation

hgaiser
Copy link
Contributor

@hgaiser hgaiser commented Apr 30, 2017

This PR enables C++11 support using CMake variables (CMAKE_CXX_STANDARD for choosing the standard and CMAKE_CXX_STANDARD_REQUIRED for disabling CMake's default functionality of "decaying" to c++98 if c++11 is not available).

These variables are added since CMake 3.1.13 so a bump of the minimum required cmake version was necessary.

@Yangqing
Copy link
Contributor

Yangqing commented May 1, 2017

Thanks so much for this!

A quick check: http://packages.ubuntu.com/trusty/cmake cmake on 14.04 is actually still 2.8, so do you think it is possible for us to provide a backward compatible path for folks who have an older cmake?

@facebook-github-bot
Copy link
Contributor

@hgaiser updated the pull request - view changes

@hgaiser
Copy link
Contributor Author

hgaiser commented May 2, 2017

Thanks so much for this!

A quick check: http://packages.ubuntu.com/trusty/cmake cmake on 14.04 is actually still 2.8, so do you think it is possible for us to provide a backward compatible path for folks who have an older cmake?

Ah .. 14.04 is a target platform for caffe2? I suppose it makes sense since it's a LTS, but it can be annoying in cases such as this. I suppose there are two options then, merge this PR and provide people a guide to upgrade cmake in Ubuntu 14.04 or don't merge this PR and keep it the way it was.

ps. I just updated this PR to also set the CMAKE_C_STANDARD variable.

@Yangqing
Copy link
Contributor

Yangqing commented Jun 1, 2017

@hgaiser I found that one can potentially use this to do cross version support. Would you like to give that a shot?

if(CMAKE_VERSION VERSION_LESS "3.1")
include(CheckCXXCompilerFlag)
CHECK_CXX_COMPILER_FLAG("-std=c++11" COMPILER_SUPPORTS_CXX11)
if(NOT COMPILER_SUPPORTS_CXX11)
MESSAGE(FATAL_ERROR "Your compiler does not support c++11")
endif()
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11")
else()
set(CMAKE_CXX_STANDARD 11)
endif()

@hgaiser
Copy link
Contributor Author

hgaiser commented Jun 1, 2017

@hgaiser I found that one can potentially use this to do cross version support. Would you like to give that a shot?

if(CMAKE_VERSION VERSION_LESS "3.1")
include(CheckCXXCompilerFlag)
CHECK_CXX_COMPILER_FLAG("-std=c++11" COMPILER_SUPPORTS_CXX11)
if(NOT COMPILER_SUPPORTS_CXX11)
MESSAGE(FATAL_ERROR "Your compiler does not support c++11")
endif()
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11")
else()
set(CMAKE_CXX_STANDARD 11)
endif()

Similar to the discussion here, having cross version support in my opinion defeats the purpose of this PR. This PR is intended to use a cleaner method of choosing CXX standard, but if both are still present it only adds unnecessary complexity.

My advise would be to drop official support for Ubuntu 14.04 (it's over 3 years old ;) ) and provide a guide for installing cmake >3.1 if necessary. The alternative is to ignore this PR in favor of support on Ubuntu 14.04.

@hgaiser
Copy link
Contributor Author

hgaiser commented Jun 26, 2017

@Yangqing what is the verdict?

@Yangqing
Copy link
Contributor

Yangqing commented Jul 5, 2017

@hgaiser I asked around a little bit and seems that Cmake <3.0 is still something preferred... so maybe let's keep the old way? It's definitely not very ideal and I in fact recommend putting the backward-compatible, but newer approach for 3.1 :)

williford added a commit to williford/caffe2 that referenced this pull request Aug 6, 2017
This PR replaces PR facebookarchive#464. It requires C+11 support using the
new CMake variables (CMAKE_CXX_STANDARD, CMAKE_CXX_STANDARD_REQUIRED,
etc.) when CMake is version 3.1 or above. Otherwise, if CMake is older
(e.g. Ubuntu 14.04) it falls back to using the -std=c++11 flag and
issues a warning.

This PR is based on the comment from @Yangqing:
facebookarchive#464 (comment)

The corresponding line in cmake/MiscCheck.cmake is removed in order to
reduce redundancy.
williford added a commit to williford/caffe2 that referenced this pull request Aug 6, 2017
This PR replaces PR facebookarchive#464. It requires C+11 support using the
new CMake variables (CMAKE_CXX_STANDARD, CMAKE_CXX_STANDARD_REQUIRED,
etc.) when CMake is version 3.1 or above. Otherwise, if CMake is older
(e.g. Ubuntu 14.04) it falls back to using the -std=c++11 flag and
issues a warning.

This PR is based on the comment from @Yangqing:
facebookarchive#464 (comment)

The corresponding line in cmake/MiscCheck.cmake is removed in order to
reduce redundancy.
williford added a commit to williford/caffe2 that referenced this pull request Aug 6, 2017
This PR replaces PR facebookarchive#464. It requires C+11 support using the
new CMake variables (CMAKE_CXX_STANDARD, CMAKE_CXX_STANDARD_REQUIRED,
etc.) when CMake is version 3.1 or above. Otherwise, if CMake is older
(e.g. Ubuntu 14.04) it falls back to using the -std=c++11 flag and
issues a warning.

This PR is based on the comment from @Yangqing:
facebookarchive#464 (comment)

The corresponding line in cmake/MiscCheck.cmake is removed in order to
reduce redundancy.
@williford
Copy link
Contributor

@Yangqing I implemented your suggestion in PR #1027. Would you take a look when you get the chance?

@Yangqing
Copy link
Contributor

Yangqing commented Aug 9, 2017

Closing in favor of #1027, thanks so much guys :)

@Yangqing Yangqing closed this Aug 9, 2017
facebook-github-bot pushed a commit that referenced this pull request Aug 9, 2017
Summary:
This PR replaces PR #464. It requires C+11 support using the
new CMake variables (`CMAKE_CXX_STANDARD`, `CMAKE_CXX_STANDARD_REQUIRED`,
etc.) when CMake is version 3.1 or above. Otherwise, if CMake is older
(e.g. Ubuntu 14.04) it falls back to using the -std=c++11 flag and
issues a warning.

This PR is based on the comment from Yangqing:
#464 (comment)

The corresponding line in cmake/MiscCheck.cmake is removed in order to
reduce redundancy. Another option would be to move the C++11 logic to MiscCheck.cmake.
Closes #1027

Differential Revision: D5590646

Pulled By: Yangqing

fbshipit-source-id: 11ac63fbeaab7a1da02115549e214f9c529f1873
ezyang pushed a commit to ezyang/ATen that referenced this pull request Apr 19, 2018
Summary:
This PR replaces PR #464. It requires C+11 support using the
new CMake variables (`CMAKE_CXX_STANDARD`, `CMAKE_CXX_STANDARD_REQUIRED`,
etc.) when CMake is version 3.1 or above. Otherwise, if CMake is older
(e.g. Ubuntu 14.04) it falls back to using the -std=c++11 flag and
issues a warning.

This PR is based on the comment from Yangqing:
facebookarchive/caffe2#464 (comment)

The corresponding line in cmake/MiscCheck.cmake is removed in order to
reduce redundancy. Another option would be to move the C++11 logic to MiscCheck.cmake.
Closes facebookarchive/caffe2#1027

Differential Revision: D5590646

Pulled By: Yangqing

fbshipit-source-id: 11ac63fbeaab7a1da02115549e214f9c529f1873
zdevito pushed a commit to zdevito/ATen that referenced this pull request Apr 19, 2018
Summary:
This PR replaces PR #464. It requires C+11 support using the
new CMake variables (`CMAKE_CXX_STANDARD`, `CMAKE_CXX_STANDARD_REQUIRED`,
etc.) when CMake is version 3.1 or above. Otherwise, if CMake is older
(e.g. Ubuntu 14.04) it falls back to using the -std=c++11 flag and
issues a warning.

This PR is based on the comment from Yangqing:
facebookarchive/caffe2#464 (comment)

The corresponding line in cmake/MiscCheck.cmake is removed in order to
reduce redundancy. Another option would be to move the C++11 logic to MiscCheck.cmake.
Closes facebookarchive/caffe2#1027

Differential Revision: D5590646

Pulled By: Yangqing

fbshipit-source-id: 11ac63fbeaab7a1da02115549e214f9c529f1873
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants