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

Add switch to enable C++ 20 compilation #556

Open
Pheubel opened this issue Jan 31, 2023 · 0 comments
Open

Add switch to enable C++ 20 compilation #556

Pheubel opened this issue Jan 31, 2023 · 0 comments
Labels
proposal A proposal up for debate.

Comments

@Pheubel
Copy link
Contributor

Pheubel commented Jan 31, 2023

What is the current behaviour?

Right now, in order to take advantage of C++ 20, you have to edit the cmake files and change target_compile_features(Engine PUBLIC cxx_std_17) to target_compile_features(Engine PUBLIC cxx_std_20) in src/NovelRT/CMakeList.txt and src/NovelRT.Interop/CMakeList.txt by hand.

What is the expected behaviour/change?

An option should be added to switch between compiling NovelRT with C++ 17 and C++ 20. Additionally, the current option to use std::span should be locked behind activating this option with cmake_dependent_option.

What is the motivation / use case for changing the behavior?

It is cumbersome to change between C++ versions and it can be easy to miss a target. It will also make testing scenarios easier to set up to test between different versions of NovelRT. Additionally, it allows developers to take advantage of NOVELRT_USE_STD_SPAN without having to alter the cmake list.

Describe alternatives you've considered:

An alternative would be to force a minimum version of C++ 20 onto developers, but this solution is not ideal.

Are there any potential roadblocks or challenges facing this change?

All be that there are no plans for it currently, we might want to add support for C++23 and other versions in the distant future. which might require more thought that a simple on off switch.

Are there any downsides to implementing this change?

There should not be any directly visible downsides to adding this change. Users that already adopted NOVELRT_USE_STD_SPAN, will have to restore their cmake and enable C++ 20 compilation if the dependent option is accepted as well.

Additional context
N/A

@Pheubel Pheubel added the proposal A proposal up for debate. label Jan 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal A proposal up for debate.
Projects
None yet
Development

No branches or pull requests

1 participant