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

Avoid enforcing std=c++11 when not necessary. #1222

Merged
merged 6 commits into from
Dec 21, 2016

Conversation

phcerdan
Copy link
Member

@phcerdan phcerdan commented Dec 8, 2016

PR Description

Avoid enforcing flag -std=c++11 when not necessary.
For example when the default flag of the compiler is
-std=gnu++11,c++14, etc... Or when the user explicitly set a std flag.

fix #1219

Checklist

Tested with gcc 6.2 that sets std=gnu++14 by default, and with gcc 5.4 that doesn't set any std flag.

For example when the default flag of the compiler is
std=gnu++11,c++14, etc...
@phcerdan
Copy link
Member Author

phcerdan commented Dec 12, 2016

@dcoeurjo
Copy link
Member

Thanks @phcerdan for the PR. Your edits look fine but I also think that using CheckCXXComplilerFlag could be a better solution than the adhoc try we have..

@phcerdan
Copy link
Member Author

It does seem that CheckCXXCompilerFlag, and CheckCXXSourceCompiles, are wraps around try_compile.

Also there is a new CXX_STANDARD variable, but it does not get passed to the CheckCXX... . See cmake bug: https://gitlab.kitware.com/cmake/cmake/issues/16456

So I think the existing try_compile is not that bad until cmake API stabilizes.

@dcoeurjo
Copy link
Member

Ok thanks

@dcoeurjo
Copy link
Member

Could you please add an entry to the change log ?

@dcoeurjo
Copy link
Member

Maybe we also should fix the DGtalConfig.cmake.in to re-discover cpp11

@phcerdan
Copy link
Member Author

phcerdan commented Dec 18, 2016

DGtalConfig.cmake modified.
Tested with gcc6.2 (-std=gnu++14 by default) and with gcc5.4 (no std flag) and it seems to work as expected, ie, avoid setting any std flag for the first case, and setting std=c++11 for the second.

phcerdan and others added 2 commits December 19, 2016 17:00
So the std flag is not override by find_package(DGtal)
This cover the case where:
The user installed DGtal with flag std less than c++11, so DGtal added
std=c++11.
Then in an application linking to DGtal, the user decided to use a
std flag greater than c++11.
With old setup this was not possible, std flag gets override by
find_package, even if the flag is more modern than c++11 as pointed in
[DGtal-team#1219].

fix DGtal-team#1219
@dcoeurjo
Copy link
Member

Excellent ! Thanks a lot.

@dcoeurjo dcoeurjo self-requested a review December 21, 2016 19:08
@dcoeurjo
Copy link
Member

All good. merging. thanks again

@dcoeurjo dcoeurjo closed this Dec 21, 2016
@dcoeurjo dcoeurjo reopened this Dec 21, 2016
@dcoeurjo dcoeurjo merged commit 0ce91b8 into DGtal-team:master Dec 21, 2016
@phcerdan phcerdan deleted the avoid_add_c11_flag branch January 28, 2017 03:56
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.

Issue with DGtalConfig.cmake
2 participants