Skip to content
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] require conversion results to be nonnegative #6561

Merged
merged 3 commits into from Sep 19, 2023

Conversation

CaseyCarter
Copy link
Contributor

"nonnegative" is meaningless for a value of some arbitrary type which we've only required to be convertible to the integral type index_type, so this wording clearly intends to constrain the result of the conversion.

…tive

"nonnegative" is meaningless for a value of some arbitrary type which we've only required to be convertible to the integral type `index_type`, so this wording clearly intends to constrain the result of the conversion.
@CaseyCarter
Copy link
Contributor Author

CaseyCarter commented Sep 11, 2023

"meow is representable as a value of type T" is also meaningless for an expression meow of arbitrary type, but I don't think that can be fixed editorially.

… to 0

`s[i]` is an lvalue of a type that we can only convert to `index_type`; clearly the wording intends that the result of the conversion should be `> 0`.
@CaseyCarter
Copy link
Contributor Author

I've pushed two more commits fixing the same issue in a couple more places, with the expectation it will be easiest to review these together. The commits are fine-grained for ease of review and each follow the editorial guidelines so it should be possible to merge them cleanly without squashing.

Shout if I made the wrong call and I should split these up into three PRs.

@CaseyCarter CaseyCarter changed the title [mdspan.extents.cons] require conversion results to be nonnegative [mdspan] require conversion results to be nonnegative Sep 12, 2023
source/containers.tex Outdated Show resolved Hide resolved
[mdspan.layout.stride.cons] uses either a span or array of a type which we can only convert to `index_type` as the second argument to the exposition-only `REQUIRED-SPAN-SIZE`. We must perform that conversion before doing math with the result.
@CaseyCarter
Copy link
Contributor Author

I should tag @jwakely for review. I know I'm near the boundary of what qualifies as editorial, and I think I'm on the right side of that boundary, but someone more objective should judge whether LWG needs to look at this.

Copy link
Member

@jwakely jwakely left a comment

Choose a reason for hiding this comment

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

LGTM. I don't think there's any doubt about what was intended, and this change expresses that more accurately.

@jensmaurer jensmaurer merged commit 24659bd into cplusplus:main Sep 19, 2023
2 checks passed
@CaseyCarter CaseyCarter deleted the mdspan1 branch September 19, 2023 21:02
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.

None yet

3 participants