search

kokkos / mdspan

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

Single header version requires at least C++20 #191

Closed bernhardmgruber closed 2 years ago

bernhardmgruber commented 2 years ago

Hi! I am trying your mdspan implementation in alpaka, which is a C++17 codebase. I use your single-header version. Unfortunately I cannot get it compiling, because the single-header version just includes <span>, which is a C++20 header.

I guess this may be an artifact of how you produce the single-header version. The normal version from the stable branch does not include <span> in C++17 mode.

mhoemmen commented 2 years ago

Greetings! I wouldn't recommend the single-header version for use in a code base. It was really just intended for Compiler Explorer and other demos. Best practice is to download the repository and build it with the appropriate CMake options set.

crtrott commented 2 years ago

Hey,

I am not sure how you get that problem. With GCC and Clang it works perfectly fine on godbolt in the single header version https://godbolt.org/z/djbcvvYqT

There is a known issue with MSVC due to some issues in MSVC with the feature test macros (i.e. it seems the feature test macro for span is defined, but then it anyway complains about including span ...).

Christian

crtrott commented 2 years ago

Oh hm I see now, looks like clang and gcc just let you include it as long as you don't use it ... I hadn't actually looked at the single header version. Yeah I don't think I want to try and fix that, or promise to make the single header version usable like that.

bernhardmgruber commented 2 years ago

@mhoemmen thank you for the recommendation, I will use the stable branch then!

Oh hm I see now, looks like clang and gcc just let you include it as long as you don't use it ... I hadn't actually looked at the single header version.

Exactly, the header can be included but the content may be disabled.

Yeah I don't think I want to try and fix that, or promise to make the single header version usable like that.

Ok, good to know, thx!