This repository was archived by the owner on Sep 18, 2024. It is now read-only.
This repository was archived by the owner on Sep 18, 2024. It is now read-only.
Summary of ABQ comments on span
#46
Open
Description
I'm going through the comments on span
from Albuquerque to both understand where span
stands relative to incorporation into the IS (and as it relates to P0546, P0856, and P0860) and to make sure we haven't made the same mistakes in mdspan
. I'll enumerate summaries of the issues here (some with bulleted responses about how we've treated that in mdspan
)
Comments from a thread titled "Questions raised while trying to implement " on the lib-ext reflector:
- The comparison operators should work with differing extents, particularly if the extent of one of them is dynamic
mdspan
doesn't define comparison operators, not evenoperator==
(could this be a problem/inconsistency for some people? Probably it's okay to just tell people to convert to span first, especially with the potential for deduction guides on the conversion operators)
- The comparison operators should be able to compare
span
s of unequal extents, sincestd::equal
andstd::lexigraphical_compare
do this span<int>
andspan<long>
should be comparable- The comparison operators take their parameters by
const
reference rather than by value, even though the paper itself recommends passingspan
by value- We should change the free function
subspan
to take anmdspan
by value for the same reason
- We should change the free function
- Contiguous iterators and contiguous containers:
span
currently has (or had?) a constructor that takes aContiguousContainer
(i.e., aContainer
whose iterators meet the requirements of contiguous iterators [27.2.1]). This isn't implementable via SFINAE (or any other compile-time technique) since contiguous iterator is a runtime property.mdspan
doesn't have this constructor. It doesn't look like we're using the term "contiguous" in any contexts that would imply the need for SFINAE, but we should double-check this
- If the size of the span (in bytes) is larger than
numeric_limits<ptrdiff_t>::max()
(e.g., as could be the case ifsizeof(value_type)
is much larger than 1), thenas_bytes()
andas_writable_bytes()
could introduce undefined behavior.- This applies to us also. We should consider using
size_t
here instead. (Also, seems like a good motivation for theT[][][]
syntax, so that we don't have to use-1
for a magic extent value) - A later note says that LEWG has already discussed this and decided to stick with
ptrdiff_t
, so maybe we shouldn't change anything.
- This applies to us also. We should consider using
- No reason for the
nullptr_t
constructor, since it's the same as the default constructor- Not sure I agree with this one. Implicit convertibility from a
nullptr_t
is a reasonable trait to have. Either way, we omitted discussion/description of this constructor in the paper, even though it's in the wording.
- Not sure I agree with this one. Implicit convertibility from a
Other Pre-meeting comments
span<int, 42>.subspan<0>()
shouldn't return aspan<dynamic_extent>
- This is another artifact of the
dynamic_extent
magic number. It doesn't look like we have that problem because oursubspan
extents are always given as normal function arguments rather than template arguments (though this seems like a bit of a shortcoming since it means we can't have a subspan with static extents other than the entirety of the parent's extent for a given rank)
- This is another artifact of the
- all iterator functions shoudl be
constexpr
mdspan
is not iterable (?!?) so this doesn't apply.
- Shouldn't have both
length
andsize
Small Group discussion comments
- Lots of discussion about constructors taking
shared_ptr
/unique_ptr
(presumably the array partial specializations) - Something indecipherable about propagating
noexcept
(presumably relating to comparison operators?) - The static version should work with
std::get
, since it is likestd::array
(though then it would have to participate in structured binding?)- Probably not that applicable to
mdspan
. If you really need this, just convert to aspan
- Probably not that applicable to
Full group discussion
- Maybe considering switching back to
array_view
, now that the termview
does not imply immutable (presumably with some change tostring_view
?) - Poll to remove
unique_ptr
andshared_ptr
constructors: 6-8-0-0-0 - Poll to remove
nullptr_t
constructor: 4-5-3-0-0 - Poll to remove
span(pointer, pointer)
constructor: 0-0-2-10-0
Other notes (not from span
discussion, but encountered while working on it):
- On the conversion constructors and assignment operators:
template< typename UType , typename ... UProperties >
constexpr mdspan( mdspan< UType , UProperties ... > const & ) noexcept;
template< typename UType , typename ... UProperties >
mdspan & operator = ( mdspan< UType , UProperties ... > const & ) noexcept;
One of the requirements is that "V::static_extent(r) == U::static_extent(r)
or V::static_extent(r) == std::dynamic_extent
for 0 < r < V::rank()
". This should probably also include the possibility that U::static_extent(r) == std::dynamic_extent
.
mdspan
defines the constraints here in terms ofstd::is_assignable<U::pointer, V::pointer>
, butspan
defines it in terms of accessing aU::value_type
through aV::pointer
meeting the rules for well-defined object access defined in [basic.lval]/8. Are these definitions exactly equivalent?
Metadata
Metadata
Assignees
Labels
No labels