Fix assumption that mdspan components live in std::experimental namespace
mhoemmen opened this issue · 1 comments
PR #263 (merged today) required a hack to get use of submdspan
to build correctly. The issue is that this repository currently assumes that #include <experimental/mdspan>
puts all mdspan
components (including submdspan
) in namespace std::experimental
. In fact, this repository hard-codes the namespaces of mdspan
components as std::experimental
.
This assumption is bad for two reasons. First, eventually compilers will get complete mdspan
and submdspan
implementations, and they will live in the std
namespace. Second, user-space libraries are not allowed to put symbols in the std
namespace. An application or library that wants to use the reference mdspan implementation specifically should either assume the default Kokkos
, or should use the macros that the reference mdspan implementation already defines in order to control the default namespace.
<experimental/mdspan>
just does the following.
#pragma once
#ifndef MDSPAN_IMPL_STANDARD_NAMESPACE
#define MDSPAN_IMPL_STANDARD_NAMESPACE std
#endif
#ifndef MDSPAN_IMPL_PROPOSED_NAMESPACE
#define MDSPAN_IMPL_PROPOSED_NAMESPACE experimental
#endif
#include "../mdspan/mdspan.hpp"
// backward compatibility import into experimental
namespace MDSPAN_IMPL_STANDARD_NAMESPACE {
namespace MDSPAN_IMPL_PROPOSED_NAMESPACE {
using ::MDSPAN_IMPL_STANDARD_NAMESPACE::mdspan;
using ::MDSPAN_IMPL_STANDARD_NAMESPACE::extents;
using ::MDSPAN_IMPL_STANDARD_NAMESPACE::layout_left;
using ::MDSPAN_IMPL_STANDARD_NAMESPACE::layout_right;
using ::MDSPAN_IMPL_STANDARD_NAMESPACE::layout_stride;
using ::MDSPAN_IMPL_STANDARD_NAMESPACE::default_accessor;
}
}
The right thing for this repository to do would be to add a header for including mdspan
(and submdspan
), and import the relevant symbols via using
declarations.
#pragma once
#ifndef MDSPAN_IMPL_STANDARD_NAMESPACE
#define MDSPAN_IMPL_STANDARD_NAMESPACE Kokkos
#endif
#ifndef LINALG_IMPL_NAMESPACE
#define LINALG_IMPL_NAMESPACE linalg
#include "../mdspan/mdspan.hpp"
namespace LINALG_IMPL_NAMESPACE {
using ::MDSPAN_IMPL_STANDARD_NAMESPACE::extents;
using ::MDSPAN_IMPL_STANDARD_NAMESPACE::mdspan;
using ::MDSPAN_IMPL_STANDARD_NAMESPACE::submdspan;
using ::MDSPAN_IMPL_STANDARD_NAMESPACE::layout_left;
using ::MDSPAN_IMPL_STANDARD_NAMESPACE::layout_right;
using ::MDSPAN_IMPL_STANDARD_NAMESPACE::layout_stride;
using ::MDSPAN_IMPL_STANDARD_NAMESPACE::default_accessor;
// ... etc. ...
}
I've since fixed this in the tests.