search

kokkos / mdspan

Reference implementation of mdspan targeting C++23
Other
401 stars 66 forks source link

configurable namespaces #256

Closed nmm0 closed 1 year ago

nmm0 commented 1 year ago

There are a ton of changes here, mostly because we use std::experimental extensively. This right now adds two macros, MDSPAN_IMPL_STANDARD_NAMESPACE, which defaults to md and MDSPAN_IMPL_PROPOSED_NAMESPACE which defaults to experimental and will be nested inside MDSPAN_IMPL_STANDARD_NAMESPACE.

These are of course, subject to change :)

I also added two headers for getting the "library"-style includes, mdspan.hpp and experimental/mdarray.hpp. I think eventually we probably want to remove the old std-style includes. I'm also considering moving these to a subfolder, e.g. #include <mdspan/mdspan.hpp> and #include <mdspan/experimental/mdarray.hpp>. Anyway, can adjust as needed.

Also, I avoided using CMake here so we can more easily copy the files over to Kokkos.

nmm0 commented 1 year ago

A couple of changes -- I put the main headers in <mdspan/mdspan.hpp> and <mdspan/experimental/mdarray.hpp>. I left all the other headers as is, but we can move them within mdspan/ (except the std headers of course). I figured that would probably make the diff a bit difficult to track though.

Finally, we have the "std-like" mdspan header as <experimental/mdspan>, but mdspan is not in namespace std anymore. Do we want to change that and move it to be just <mdspan>? This would of course conflict with any actual standard library implementation and we would probably want a cmake option to disable that from happening in installs.

nmm0 commented 1 year ago

I also had to add an extra define MDSPAN_IMPL_NAMESPACE_STD when we are in "std-like" mode because of conflicts with <span>

crtrott commented 1 year ago

Couple comments: I think I am ok to drop C++14 support, but I am also ok to NOT support nesting for things we consider mature. Also wouldn't it be a user thing, that if a user says I want everything in a nested namespace and they use C++17, they could define MDSPAN_IMPL_STANDARD_NAMESPACE as std::experimental, and then submdspan say would end up in std::experimental::proposed?

Also I have thought about defaults. I think md is too short and too potentially conflicted. I don't really see a reason why we shouldn't make the default "Kokkos" and "Experimental".

nmm0 commented 1 year ago

Couple comments: I think I am ok to drop C++14 support, but I am also ok to NOT support nesting for things we consider mature. Also wouldn't it be a user thing, that if a user says I want everything in a nested namespace and they use C++17, they could define MDSPAN_IMPL_STANDARD_NAMESPACE as std::experimental, and then submdspan say would end up in std::experimental::proposed?

Also I have thought about defaults. I think md is too short and too potentially conflicted. I don't really see a reason why we shouldn't make the default "Kokkos" and "Experimental".

@crtrott I changed the namespaces to Kokkos and Experimental