kokkos / mdarray

Other
9 stars 9 forks source link

P1684: Specify requirements on Container #17

Open mhoemmen opened 2 years ago

mhoemmen commented 2 years ago

This comes from 1684R2 LEWG review on 2022/04/19.

We have no experience (do we?) with Container types other than vector and array. Should we restrict to those two types, or do the hard work of wording for other types like static_vector (fixed capacity that bounds the size)?

For example, the current wording only distinguishes between array and every other type. However, some other types, like static_vector, may default-construct nonempty, just like array. The question relates to whether the container can be constructed with a run-time size, vs. whether the container is "constructed nonempty" without a size.

crtrott commented 2 years ago

Right now the only distinguishing thing there is for the constructors which take arguments you can't directly allocate the underlying container from (say a mapping or multiple sizes). All the other constructors (from initializer list, from container, from container via move, ranges-to etc.) would work with any container which supports the corresponding constructors.

Unfortunately, there is nothing in containers which tells me how to generically know whether a containers default constructor constructs a thing with size while not having a constructor which takes a size. Hence we can't generically tell whether the mdarray<T,E,L,C>(N,M) constructor simply calling C() is valid if no C(size_t) constructor exists. You could still explicitly call mdarray<T,E,L,C>(std::move(C()),N,M) however for this generic case, assuming C is move constructable from C.

Note static_vector is constructible from size_type so mdarray<T,E,L,static_vector<T,N*M>>(N,M) should be valid.

crtrott commented 2 years ago

So I don't think there is anything more specific I want to require than what we require now. In the very worst case for a container which has just a move or copy constructor plus some constructor which takes a bazillion arguments, mdarray would only have the constructors which copy or move from the container, plust the arguments to construct the mapping.

mhoemmen commented 2 years ago

Minor design issue for container requirements: "contiguous container" just says that the iterators are contiguous iterators, not that the container has a .data() method.

nliber commented 2 years ago

I tried to fix that with P1768 Contiguous Containers Should Contain .data().

Do we need it, or can we state the requirement in terms of std::ranges::data(...)?

mhoemmen commented 2 years ago

Thank you @nliber ! : - ) I don't think we need .data() -- we just need some way to get the pointer out of the container. We could always rebase P1684 on top of that later.

@crtrott -- I think that's fine. The current wording already says that Container must be a contiguous container, and that gives us Container(size_t, ElementType) construction (except for array).