Cpp14 support

[oliverlee]

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.

[mhoemmen]

Thanks for the contribution! There are some nice refactorings in there as well. @crtrott et al. will likely want to comment.

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.

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.

[oliverlee]

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.

[mhoemmen]

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.")

[oliverlee]

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.

[crtrott]

I will rebase this and merge my suggestions.

[crtrott]

@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.

[oliverlee]

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.

[crtrott]

They passed now. We might have changed Debug to Release build at some point and so the rebase made them fail.

[oliverlee]

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.

[oliverlee]

@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.

[crtrott]

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.

[crtrott]

Also I am waiting for a second review to finish (should happen latest by Monday I pinged appropriate people in my team).

[oliverlee]

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?

[crtrott]

It could make sense to separate them out. We had some platforms where the death tests never fully worked ...

[oliverlee]

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.

[crtrott]

Yeah let's do it. Do you have time to tackle that? If not I might get around to it some point this weekend.

[oliverlee]

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.

[crtrott]

Sounds good. Also I send you an invite to our slack channel in case you are interested.

[crtrott]

Note: I merged one other PR, but this one rebases cleanly on top of it.

[oliverlee]

Rebased + squashed some commits + separated out the death tests. Is this what you had in mind?

[crtrott]

Yeah this is great. Thanks so much!

[crtrott]

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?

[oliverlee]

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).

[crtrott]

Its now fixed.