You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
This repository was archived by the owner on Sep 18, 2024. It is now read-only.
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 even operator== (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 spans of unequal extents, since std::equal and std::lexigraphical_compare do this
span<int> and span<long> should be comparable
The comparison operators take their parameters by const reference rather than by value, even though the paper itself recommends passing span by value
We should change the free function subspan to take an mdspan by value for the same reason
Contiguous iterators and contiguous containers: span currently has (or had?) a constructor that takes a ContiguousContainer (i.e., a Container 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 if sizeof(value_type) is much larger than 1), then as_bytes() and as_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 the T[][][] 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.
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.
Other Pre-meeting comments
span<int, 42>.subspan<0>() shouldn't return a span<dynamic_extent>
This is another artifact of the dynamic_extent magic number. It doesn't look like we have that problem because our subspan 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)
all iterator functions shoudl be constexpr
mdspan is not iterable (?!?) so this doesn't apply.
Shouldn't have both length and size
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 like std::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 a span
Full group discussion
Maybe considering switching back to array_view, now that the term view does not imply immutable (presumably with some change to string_view?)
Poll to remove unique_ptr and shared_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:
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 of std::is_assignable<U::pointer, V::pointer>, but span defines it in terms of accessing a U::value_type through a V::pointer meeting the rules for well-defined object access defined in [basic.lval]/8. Are these definitions exactly equivalent?
I'm going through the comments on
spanfrom Albuquerque to both understand wherespanstands 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 inmdspan. I'll enumerate summaries of the issues here (some with bulleted responses about how we've treated that inmdspan)Comments from a thread titled "Questions raised while trying to implement " on the lib-ext reflector:
mdspandoesn'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)spans of unequal extents, sincestd::equalandstd::lexigraphical_comparedo thisspan<int>andspan<long>should be comparableconstreference rather than by value, even though the paper itself recommends passingspanby valuesubspanto take anmdspanby value for the same reasonspancurrently has (or had?) a constructor that takes aContiguousContainer(i.e., aContainerwhose 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.mdspandoesn'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 thisnumeric_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.size_there instead. (Also, seems like a good motivation for theT[][][]syntax, so that we don't have to use-1for a magic extent value)ptrdiff_t, so maybe we shouldn't change anything.nullptr_tconstructor, since it's the same as the default constructornullptr_tis a reasonable trait to have. Either way, we omitted discussion/description of this constructor in the paper, even though it's in the wording.Other Pre-meeting comments
span<int, 42>.subspan<0>()shouldn't return aspan<dynamic_extent>dynamic_extentmagic number. It doesn't look like we have that problem because oursubspanextents 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)constexprmdspanis not iterable (?!?) so this doesn't apply.lengthandsizeSmall Group discussion comments
shared_ptr/unique_ptr(presumably the array partial specializations)noexcept(presumably relating to comparison operators?)std::get, since it is likestd::array(though then it would have to participate in structured binding?)mdspan. If you really need this, just convert to aspanFull group discussion
array_view, now that the termviewdoes not imply immutable (presumably with some change tostring_view?)unique_ptrandshared_ptrconstructors: 6-8-0-0-0nullptr_tconstructor: 4-5-3-0-0span(pointer, pointer)constructor: 0-0-2-10-0Other notes (not from
spandiscussion, but encountered while working on it):One of the requirements is that "
V::static_extent(r) == U::static_extent(r)orV::static_extent(r) == std::dynamic_extentfor0 < r < V::rank()". This should probably also include the possibility thatU::static_extent(r) == std::dynamic_extent.mdspandefines the constraints here in terms ofstd::is_assignable<U::pointer, V::pointer>, butspandefines it in terms of accessing aU::value_typethrough aV::pointermeeting the rules for well-defined object access defined in [basic.lval]/8. Are these definitions exactly equivalent?