<mdspan>: Implement *Mandates* clauses
#3535
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Since the standard suggests `is-extents` name, I'm not going to add extra suffix http://eel.is/c++draft/views.multidim#mdspan.layout.stride.expo-3
Co-authored-by: Matt Stephanson <68978048+MattStephanson@users.noreply.github.com>
StephanTLavavej
approved these changes
Mar 4, 2023
|
|
||
| _NODISCARD static constexpr bool _Is_index_space_size_representable() { | ||
| if constexpr (rank_dynamic() == 0 && rank() > 0) { | ||
| return _STD in_range<index_type>((_Extents * ...)); |
There was a problem hiding this comment.
Do we have to worry about integer overflow during this monster multiplication?
There was a problem hiding this comment.
The only references I could find to index_type being able to represent the index space size are in Preconditions to the various layout mapping constructors. If we're going to upgrade it to a static_assert, then I think we do need to check for overflow, because it can happen and the compiler is supposed to diagnose UB in constant evaluation contexts. We also don't want any false positives, since the user won't have any way to bypass this check.
|
Thanks! My suggestions (if you agree with them) can be addressed in followup PRs. |
JMazurkiewicz
added a commit
to JMazurkiewicz/STL
that referenced
this pull request
Mar 25, 2023
See microsoft#3590 Also addresses microsoft#3535 (comment)
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
std::extentsnow accepts all signed and unsigned integer types (asTRANSITIONcomments said),Extentsare representable asIndexType,RenameStandard suggest name_Is_extentsto_Is_extents_vto indicate that this is a variable,is-extentswithout-vsuffix ([mdspan.layout.stride.expo]/3),mdspanclass,mdspanconstructor,extents::_Is_index_space_size_representablefunction (this name can be much better, so I'm open for suggestions),static_assertions for[mdspan.layout.left.overview]/2,[mdspan.layout.right.overview]/2, and[mdspan.layout.stride.overview]/2.e->_Ext(non-reserved identifier),OtherExtents->_OtherExtents(ditto),r->_Rank(ditto),_NODISCARD friend->_NODISCARD_FRIENDelement_typenested type,constuctible->constructible.