kokkos
Cpp14 support
I've updated the P0009 headers to compile without C++17 extensions.
For the last commit, I've dumped a few common utilities in the extents header. Please feel free to edit as desired.
Thanks for the contribution! There are some nice refactorings in there as well. @crtrott et al. will likely want to comment.
-
Do mdspan's tests and examples all build with C++14, or do these changes only just make your application build? We're generally only interested in the former.
-
Would you consider adding C++14 testing to https://github.com/kokkos/mdspan/blob/stable/.github/workflows/cmake.yml as a part of this PR? Current PR automatic testing will not exercise C++14.
1. Do mdspan's tests and examples all build with C++14, or do these changes only just make your application build? We're generally only interested in the former.
I've only tested with the tests defined in CMake along with C++14 so far. All tests pass before and after this PR - however after this PR now builds without warnings.
2. Would you consider adding C++14 testing to https://github.com/kokkos/mdspan/blob/stable/.github/workflows/cmake.yml as a part of this PR? Current PR automatic testing will not exercise C++14.
Sure.
Thanks for making the changes! Building with C++14 results in the following CMake error message.
CMake Error at CMakeLists.txt:200 (MESSAGE):
Benchmarks are not available in C++14 or C++23 mode. Turn
MDSPAN_ENABLE_BENCHMARKS OFF or use C++17 or C++20.
I think you could just change line 200 of CMakeLists.txt ( https://github.com/kokkos/mdspan/blob/0e6a69dfe045acbb623003588a4aff844ea4b276/CMakeLists.txt#L200 ) from what it is currently,
MESSAGE(FATAL_ERROR "Benchmarks are not available in C++14 or C++23 mode. Turn MDSPAN_ENABLE_BENCHMARKS OFF or use C++17 or C++20.")
to something like
set(MDSPAN_ENABLE_BENCHMARKS OFF)
MESSAGE(STATUS "Benchmarks are not available in C++14 or C++23 mode. Turn MDSPAN_ENABLE_BENCHMARKS OFF or use C++17 or C++20.")
Thanks for making the changes! Building with C++14 results in the following CMake error message.
CMake Error at CMakeLists.txt:200 (MESSAGE): Benchmarks are not available in C++14 or C++23 mode. Turn MDSPAN_ENABLE_BENCHMARKS OFF or use C++17 or C++20.I think you could just change line 200 of CMakeLists.txt (
https://github.com/kokkos/mdspan/blob/0e6a69dfe045acbb623003588a4aff844ea4b276/CMakeLists.txt#L200 ) from what it is currently,
MESSAGE(FATAL_ERROR "Benchmarks are not available in C++14 or C++23 mode. Turn MDSPAN_ENABLE_BENCHMARKS OFF or use C++17 or C++20.")to something like
set(MDSPAN_ENABLE_BENCHMARKS OFF) MESSAGE(STATUS "Benchmarks are not available in C++14 or C++23 mode. Turn MDSPAN_ENABLE_BENCHMARKS OFF or use C++17 or C++20.")
I disabled benchmarking for C++14 using the same approach for C++23 (i.e. via matrix settings in cmake.yml) since I thought it would be simpler to maintain. Let me know if you still prefer a CMake option based approach.
@oliverlee FYI: I rebased and force pushed. I'd like to get this in before I work on more of the features for C++26.
Not sure why the death tests are failing now - they were fine a few months back.
I'll probably have time to look into it tomorrow.
They passed now. We might have changed Debug to Release build at some point and so the rebase made them fail.
It should be possible to enable precondition checks in release mode with a preprocessor macro - I imagine this is enabled only for tests and disabled for benchmarks.
It shouldn't be too much trouble to do in a separate PR if you want to merge this PR and use it as a base for c++26 stuff.
@crtrott It looks like the macro in test/CMakeLists.txt was missed in the rename commit.
I've created two commits - please squash or do whatever is normal for this repo.
Thanks! I had overlooked that. That said I am not sure it's the best way to go about this. Now the tests never take the code path where the preconditions are skipped. Maybe instead we should have a mix of debug and non-debug test configs.
Also I am waiting for a second review to finish (should happen latest by Monday I pinged appropriate people in my team).
That said I am not sure it's the best way to go about this.
Same - whatever makes the most sense to you in terms of maintenance.
I've seen precondition tests defined in a separate test file. That could remove the need to conditionally define test cases with ASSERT_DEATH and simplify running tests with debug and non-debug configs. Would that make sense here?
It could make sense to separate them out. We had some platforms where the death tests never fully worked ...
It could make sense to separate them out. We had some platforms where the death tests never fully worked ...
There are only a few DEATH_TESTS now.
It makes sense to do it earlier rather than later if this is approach you want to pursue.
Yeah let's do it. Do you have time to tackle that? If not I might get around to it some point this weekend.
Yeah let's do it. Do you have time to tackle that? If not I might get around to it some point this weekend.
Maybe this afternoon? If not, also this weekend.
Sounds good. Also I send you an invite to our slack channel in case you are interested.
Rebased + squashed some commits + separated out the death tests. Is this what you had in mind?
I just noticed that this requires CXX Extensions for GCC. I.e. -std=gnu++14 - it doesn't work with -std=c++14. Do you consider that acceptable?
I just noticed that this requires CXX Extensions for GCC. I.e. -std=gnu++14 - it doesn't work with -std=c++14. Do you consider that acceptable?
Do you know what requires extensions? I didn't intend for that and I believe I am only using -std=c++14 in projects using mdspan (however, those don't use CMake).