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

Upgrade required language level to C++17 #4000

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

adamkewley
Copy link
Contributor

@adamkewley adamkewley commented Jan 30, 2025

Related, the C++14 upgrade in Oct 2024: #3931

This PR upgrades the required language level from C++14 to C++17. The motivation for this is because I would like to implement caching mesh data between copies of OpenSim::Mesh, but doing so robustly requires filesystem queries like std::filesystem::last_write_time to handle edge-cases like "the file path didn't change but the timestamp did, so reload".

In terms of compatibility, C++17 is generally available in Ubuntu18+, and even (e.g.) Ubuntu16 with a backported libstdc++. MacOS (Apple Clang) and Windows (MSVC) have had very good feature coverage for C++17 for around 5 or 6 years (save a few library functions in Apple Clang's case). OpenSim Creator has been compiling for C++20 for the last year, but the required OS-level is higher (Ubuntu20/Mac Sonoma/Windows 10).

Additional benefits are the inclusion of std::optional<T> and std::string_view, which I imagine will slip into the codebase over time. This PR only addresses setting the flags, so that future PRs don't have to handle two things at one (e.g. "here's my new mesh caching routine, oh, also we need C++17").

Brief summary of changes

  • Relevant compiler flags updated

Testing I've completed

  • None required. CI (GitHub Actions) should be using representative operating systems for the target audience. If CI can't build C++17 as new in-source usages are added, then those usages will need to be fixed. This is independent of the compiler language level: telling a compiler to use C++17 doesn't imply 100 % language and library conformance - the only way to know that is to actually compile the code on each representative system (e.g. via CI).

Looking for feedback on...

CHANGELOG.md (choose one)

  • updated.

This change is Reviewable

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.

1 participant