search

kokkos / mdspan

Reference implementation of mdspan targeting C++23
Other
413 stars 69 forks source link

Add element access via `at()` to `std::mdspan` #302

Open stephanlachnit opened 12 months ago

stephanlachnit commented 12 months ago

Closes #300

Possible implementation for ::at element access to mdspan with boundary checks. The boundary check is implemented with a for loop, as I did not see any other way to achieve this. The error message is inspired by the std::span::at error message from libstdc++ (https://github.com/gcc-mirror/gcc/commit/1fa85dcf656e2f2c7e483c9ed3c2680bf7db6858).

Note that this does not implement ::at element access to mdarray.

stigrs commented 10 months ago

Given that SizeTypes can be signed, it should also check for indices[r] < 0.

stephanlachnit commented 3 months ago

Given that SizeTypes can be signed, it should also check for indices[r] < 0.

Thanks for this comment! Is this true for std::span as well? At least in libstdc++, they do not check this:

https://github.com/gcc-mirror/gcc/blob/fd7dabc116b9abc40ee6aa25bcc5d240b8cc516a/libstdc%2B%2B-v3/include/std/span#L290-L300

nmm0 commented 3 months ago

In span, I believe it's always just std::size_t, but mdspan has the size type as a template parameter for the extent and can be signed.

stephanlachnit commented 3 months ago

In span, I believe it's always just std::size_t, but mdspan has the size type as a template parameter for the extent and can be signed.

Ah right, thanks for pointing this out!

Edit: updated the pull request

stephanlachnit commented 4 days ago

if constexpr doesn't exist pre C++17, should I add a macro here? Or just go with if and assume the compiler optimizes for this anyway?

stephanlachnit commented 3 days ago

I don't quite get the build failure on MSVC, would need some help there...