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

Update CMake and documentation to use C++17 standard #189

Closed
pacmancoder opened this issue Jul 4, 2018 · 10 comments · Fixed by #292
Closed

Update CMake and documentation to use C++17 standard #189

pacmancoder opened this issue Jul 4, 2018 · 10 comments · Fixed by #292

Comments

@pacmancoder
Copy link

Most modern compilers are already support newest C++17 standard, so I think CMake files among with documentation should be updated to require compiller with C++17 support.

@arcuru
Copy link
Contributor

arcuru commented Aug 23, 2018

The issue with that is we are trying to be more lenient about the versions people have available to them. My thinking has been that we should try to support at least what's available on Ubuntu LTS, as it's the oldest reasonable version. See people asking in #195 for instance.

Not all the OS's and distributions have updated to C++17 in their default toolchain, and many people are on older releases from before C++17 was released anyways.

What do you think?

@pacmancoder
Copy link
Author

Well, that's makes sense. But I think we must at least clarify that students can submit their solutions using newer standard (e.g. make a note that student is allowed to change the standard in CMake) I guess it will be fine.

@arcuru
Copy link
Contributor

arcuru commented Aug 24, 2018

Oh, that's a good point. We need to support a relatively old version for the code we provide but not restrict the students from using a newer standard. I'd rather not force people to edit the CMake files just to code in C++14.

If we just remove the -std=c++11 flag will that do what we want?

@NobbZ
Copy link
Member

NobbZ commented Aug 24, 2018

Can't we use conditionals on the compiler version?

But to be honest, lets consider, I change that line to be C++17 using gcc 8.x, and I use some cool shiny features. Then a mentor using some gcc 5.x (and I think there are even some 4.x out there) will pick it up and download my submission.

In this download there will still be the original CMakeLists.txt, so his compiler will warn for invalid syntax or spit out other errors for first. Then he tries C++14, which still doesn't work, but when trying C++17 finally gcc might complain about the unknown flag/option.

How can we handle this?

@arcuru
Copy link
Contributor

arcuru commented Aug 24, 2018

To answer my own question, leaving off the flag just uses the compilers default std for compilation (currently the latest gcc default is C++14). If we use conditionals on the compiler version in CMake, we'd just be further complicating things.

The only way to get around these issues is actually just to lock to one particular C++ version, and push people to use that. A student can always edit the CMake file to get a later (or earlier, if they wanted) version to compile though, so it's not perfect.

We can look at moving the CMake setting up to C++14 if the compilers we want to support can use it.

@NobbZ - If we give students freedom to use the latest versions, there's not an easy way to handle that scenario other than just letting the mentors figure it out. I would expect the mentors to be able to get a reasonably up to date compiler on their system.

@KevinWMatthews
Copy link
Contributor

Are you willing/able to upgrade the version of CMake to 3.1.3? If so, there is a feature that allows one to both specify the default C++ standard and also to enable/disable C++ standard rollback.
Theoretically this more portable, too, since CMake can figure out what flags to use based on the current system.

Although it may provide value, I hesitate to suggest a change to such a basic tool. Will this work with Exercism's current build tools? Will this alienate students running software from 2013, i.e., CMake 2.8?

Something like this:

set_target_properties(<target> PROPERTIES
    CXX_STANDARD 14
    CXX_STANDARD_REQUIRED OFF
    CXX_EXTENSIONS OFF
)

Disabling CXX_EXTENSIONS prevents CMake from adding GNU extensions, which it enables by default.

CMake 3.1.3 only supports up to C++14; it looks like they added support for C++17 in version 3.8.

@arcuru
Copy link
Contributor

arcuru commented Aug 25, 2018

CMake 3.1.3 is definitely old enough to make a minimum requirement. Ubuntu 16.04 ships with CMake 3.5, and that's a a fairly reasonable lower bar for us to set our min requirements to.

I know that a 'modern' CMake file can be a lot easier/cleaner to write than what we have here. Modernizing the CMake files is on my list of things that may eventually be a good idea to do, but I don't have a lot of experience with writing CMake files so it would take some effort to do it myself. I would be very happy though if somebody took that up though :)

That feature looks like a much better way to specify the version, and I think setting that up to support C++14 is probably fine at this point. I'll probably still be a little careful to make sure that the test files we are providing will still compile for C++11 just in case though.

@KevinWMatthews
Copy link
Contributor

Sounds good. It didn't occur to me to reference a different issue - only takes a '#'. I'll comment over there.

@KevinWMatthews
Copy link
Contributor

Turns out that upgrading to C++14 actually breaks the Clang build. I've created a branch that updates only the C++ standard to illustrate this. You can see a diff and Travis CI failure.

At Stack Overflow's recommendation, I experimented with installing libc++-dev and adding the compiler flag -stdlib=libc++ to the Clang build. It changed the error message but still doesn't build.

Further experimentation required - I don't have much experience with Clang (yet).

@arcuru
Copy link
Contributor

arcuru commented Jun 15, 2019

I think we can probably push to update to C++14 at least. The latest Ubuntu LTS (18.04 bionic) has clang 6.0 and gcc 7.4, which both support C++14.

I expect that updating the requirements to C++14 will only break a very small number of people using very outdated development toolchains.

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 a pull request may close this issue.

4 participants