kokkos / mdspan

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

mdarray<bool> with std::vector container #359

Open spraetor opened 2 months ago

spraetor commented 2 months ago

The specialization of std::vector<bool> always causes some trouble. I am trying to use the mdarray container adapter with element type bool and neither the current stable implementation in this repository nor to standard proposal document P1684R5 seem to address this issue.

There are many problems, I list just a few:

So, the issue clearly is that std::vector<bool> is not a SequenceContainer.

Should this be a supported type in std::mdarray? Is there a workaround, except for using a completely different container from outside the c++ standard library?

ghost commented 2 months ago

It's because the underlying storage of std::vector<bool> is not a bool* . The operator[] of std::vector<bool> return a proxy objects that uses bit-shifts to set/clear a single bit. It is still a sequence container, but not a sequence container that uses pointers as mdspan assumes. Have a look at this page: https://en.cppreference.com/w/cpp/container/vector_bool Maybe using a custom accessor that returns a std::vector<bool>::reference could do the trick ?

spraetor commented 2 months ago

Thanks for the feedback. Yes, you are right, it is not a problem of beeing a sequence container. The issue is more about the way type aliases are defined in std::mdarray (proposal P1684R5). One cannot even instantiate the mdarray with std::vector<bool> since the following typedefs would be illformed:

using pointer = decltype(to_address(declval<container_type>().begin()));
using const_pointer = decltype(to_address(declval<container_type>().cbegin()));

Also, the member function to_mdspan currently requires a .data() member of the container resulting in a compiler error.

I cannot specify a custom accessor for mdarray.

I am just wondering whether std::vector<bool> is simply out of scope of mdarray as container type, or whether a simple wrapper could be a solution, or whether maybe the specification needs some adjustments to allow for this. I have implemented for testing a class that inherits from std::vector and uses a custom bool-replacement type in case of element-type bool. This is not so very nice and easily 200-300 lines of code, just to get the bool case working.

spinicist commented 1 month ago

Two cents from someone not involved with mdspan - vector<bool> is plain weird and causes problems like this all over the place. Previously I've had to special case it just as you have. All the issues you mention are very much vector<bool>'s problem, and I wouldn't expect mdarray/mdspan to make any effort to support it!