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

Switch from C++14 to C++17 #650

Closed
s3rvac opened this issue Sep 17, 2019 · 3 comments
Closed

Switch from C++14 to C++17 #650

s3rvac opened this issue Sep 17, 2019 · 3 comments
Assignees
Milestone

Comments

@s3rvac
Copy link
Member

s3rvac commented Sep 17, 2019

We have discussed this and it seems that the time has come to switch from C++14 to C++17.

Reasons:

  • It is already 2019. All major compiler vendors and standard-library implementers support C++17 (at least the features that we would like to use, see this table and the one below).
  • We could replace e.g. Maybe with std::optional, FilesystemPath with std::filesystem, mpark_variant with std::variant.
  • The new version of yaramod (currently under development) will require C++17. As we use yaramod in RetDec, we will also need to switch to C++17.

The switch to C++17 would not mean that we will update all of our code to C++17 at once. The transition will be gradual, starting with what we need the most.

@s3rvac s3rvac added this to the RetDec v4 milestone Sep 17, 2019
@PeterMatula PeterMatula self-assigned this Sep 19, 2019
@ceeac
Copy link

ceeac commented Sep 20, 2019

One thing to note is that std::get for std::variant is not available for macOS 10.13 so you might have to bump the minimum required macOS version to 10.14 for full std::variant support.

@s3rvac
Copy link
Member Author

s3rvac commented Sep 20, 2019

Thank you for the note. Yes, we will have to figure out the minimal versions of operating systems, development environments, compilers, etc. and bump them.

@PeterMatula
Copy link
Collaborator

Our CMake now requires C++17. I also replaced some obvious stuff like mpark_variant or utils::Maybe with std::variant and std::optional. Also, I fixed some warnings, but there are many more on Windows - but there is another issue for that.

I also tried to use std::filesystem instead of our own implementation, but it turned out this is not the best idea at the moment:

  • On Linux, it requires GCC >= 8.
  • On Windows many regression tests failed. There are probably some minor differences, or we use it incorrectly.
  • Both <filesystem> and <external/filesystem> are missing in macOS < 10.15. Right now we don't have this version in TeamCity, nor Travis CI.

So I decided to throw away the new code and wait a little longer with this particular improvement until <filesystem> is widely available.

Also, C++17 allows many more improvements of our code, but these features will be used gradually in new/refactored code.

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

No branches or pull requests

3 participants