-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
<mdspan>
: Product code review
#3971
Merged
StephanTLavavej
merged 12 commits into
microsoft:feature/mdspan2
from
StephanTLavavej:final-mdspan
Aug 15, 2023
Merged
<mdspan>
: Product code review
#3971
StephanTLavavej
merged 12 commits into
microsoft:feature/mdspan2
from
StephanTLavavej:final-mdspan
Aug 15, 2023
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
They return `mapping_type::is_always_MEOW()` which is required to be a constant expression of type `bool` (N4950 \[mdspan.mdspan.overview\]/3, \[mdspan.layout.policy.reqmts\]/1, \[mdspan.layout.reqmts\]/22,24,26). The Standard should simply be enhanced to say `noexcept` here. Also introduce `constexpr bool _Result` to make this extra clear and improve debug codegen.
…ype&` in this constraint.
…the order of N4950 \[mdspan.accessor.default.overview\].
…ted by N4950 \[mdspan.extents.overview\] and \[mdspan.extents.cons\]/12.
…l_index()` to match its function parameters; it's always called with template argument deduction.
…ot commented as strengthened because it's `_Ugly`. It's now identical to the real multidimensional subscript operator except for the name, the comment, and the intentional lack of constraints.
…es the `span` overload. This is what N4950 \[mdspan.mdspan.members\]/6 depicts. It was being implicitly skipped before (`const array&` has the same effect), but adding this will make the following unification clearer.
This unifies them by wrapping the `array` in a `span`.
…not commented, it's `_Ugly`), then strengthen `operator[]` for `span`/`array`. This brings the `span`/`array` subscript operators up to parity with the multidimensional subscript operator.
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.
mdspan::is_always_MEOW()
.mapping_type::is_always_MEOW()
which is required to be a constant expression of typebool
(N4950 [mdspan.mdspan.overview]/3, [mdspan.layout.policy.reqmts]/1, [mdspan.layout.reqmts]/22,24,26). The Standard should simply be enhanced to saynoexcept
here.constexpr bool _Result
to make this extra clear and improve debug codegen._Access_impl()
is_Ugly
.const _OtherIndexType&
in this constraint.default_accessor
, defineaccess()
beforeoffset()
to follow the order of N4950 [mdspan.accessor.default.overview].layout_stride::template mapping
is unnecessary;layout_stride
is not a template.explicit
to the deduction guideextents(_Integrals...)
, depicted by N4950 [mdspan.extents.overview] and [mdspan.extents.cons]/12._Contains_multidimensional_index()
to match its function parameters; it's always called with template argument deduction.operator[]
forspan
/array
. Fine-grained commits for clarity:_Multidimensional_access()
up.noexcept
to_Multidimensional_access()
, not commented as strengthened because it's_Ugly
.as_const()
to thearray
overload, so it exactly matches thespan
overload.const array&
has the same effect), but adding this will make the following unification clearer._Multidimensional_subscript()
.array
in aspan
.noexcept
to_Multidimensional_subscript
(not commented, it's_Ugly
), then strengthenoperator[]
forspan
/array
.span
/array
subscript operators up to parity with the multidimensional subscript operator.