Skip to content

[mdspan.layout.left.cons] Fix editorial issue in layout_left/right conversion constructors #6069

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

Merged
merged 1 commit into from
Mar 24, 2023

Conversation

crtrott
Copy link
Contributor

@crtrott crtrott commented Jan 31, 2023

The precondition was erroneously referring to the not yet constructed extents instead of other.extents().
Note that the extents of the to be constructed layout will be initialized with other.extents - i.e. after construction they will return the same value for the fwd-prod-of-extents etc.

Before:

template<class OtherExtents>
  constexpr explicit(extents_type::rank() > 0)
    mapping(const layout_stride::mapping<OtherExtents>& other);
  • Constraints: is_­constructible_­v<extents_­type, OtherExtents> is true.
  • Preconditions:
    • If extents_­type​::​rank() > 0 is true, then for all r in the range [0, extents_­type​::​rank()), other.stride(r) equals extents().fwd-prod-of-extents(r), and

After:

template<class OtherExtents>
  constexpr explicit(extents_type::rank() > 0)
    mapping(const layout_stride::mapping<OtherExtents>& other);
  • Constraints: is_­constructible_­v<extents_­type, OtherExtents> is true.
  • Preconditions:
    • If extents_­type​::​rank() > 0 is true, then for all r in the range [0, extents_­type​::​rank()), other.stride(r) equals other.extents().fwd-prod-of-extents(r), and

The precondition was erroneously referring to the not yet
constructed extents instead of other.extents().
Note that the extents of the to be constructed layout will be
initialized with other.extents - i.e. after construction they will
return the same value for the fwd-prod-of-extents etc.
Copy link
Member

@jensmaurer jensmaurer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@jwakely , what do you think?

@tkoeppe tkoeppe requested a review from jwakely March 19, 2023 00:42
@tkoeppe
Copy link
Contributor

tkoeppe commented Mar 23, 2023

@jwakely Any final thoughts?

@tkoeppe tkoeppe merged commit 2c7e87d into cplusplus:main Mar 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants