Skip to content

P2494R2 Relaxing range adaptors to allow for move only types #5710

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Aug 17, 2022
Merged

P2494R2 Relaxing range adaptors to allow for move only types #5710

merged 3 commits into from
Aug 17, 2022

Conversation

jensmaurer
Copy link
Member

@jensmaurer jensmaurer added this to the post-2022-07 milestone Aug 5, 2022
@jensmaurer jensmaurer mentioned this pull request Aug 5, 2022
Merged
JohelEGP
JohelEGP suggested changes Aug 5, 2022
View reviewed changes
Copy link
Contributor

@JohelEGP JohelEGP left a comment

Choose a reason for hiding this comment

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

Mostly missing spaces to align // \expos.

@jensmaurer jensmaurer force-pushed the motions-2022-07-lwg-30 branch from 75f6712 to 8ca5607 Compare August 7, 2022 09:20
@tkoeppe tkoeppe force-pushed the motions-2022-07-lwg-30 branch from 8ca5607 to 8a7ee6c Compare August 17, 2022 20:02
@tkoeppe
Copy link
Contributor

tkoeppe commented Aug 17, 2022 •
edited
Loading

@jensmaurer, @JohelEGP: Several changes to the nested iterator types of the views don't seem to be in the paper, but I take it that's an error in the paper?

@JohelEGP
Copy link
Contributor

If you mean that an updated template-head of a range adaptor is not reflected in the template-head of the class synopsis of an inner class (template), then you're right.

JohelEGP
JohelEGP suggested changes Aug 17, 2022
View reviewed changes
@JohelEGP
Copy link
Contributor

If you mean that an updated template-head of a range adaptor is not reflected in the template-head of the class synopsis of an inner class (template), then you're right.

But the changes in this PR already takes account for those.

JohelEGP
JohelEGP suggested changes Aug 17, 2022
View reviewed changes
Copy link
Contributor

@JohelEGP JohelEGP left a comment

Choose a reason for hiding this comment

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

The paper fails to account for the use of copyable-box in [range.chunk.by.view]. I believe simply changing its use to movable-box, as done with filter_view, will suffice.

@tkoeppe
Copy link
Contributor

tkoeppe commented Aug 17, 2022

If you mean that an updated template-head of a range adaptor is not reflected in the template-head of the class synopsis of an inner class (template), then you're right.

But the changes in this PR already takes account for those.

Right, I saw that, and wanted to make sure that was deliberate. I'll mention this in the commit message. Thanks!

jensmaurer and others added 3 commits August 17, 2022 22:01
@jensmaurer @tkoeppe
P2494R2 Relaxing range adaptors to allow for move only types
b5898af 
Editorial changes:
* template-heads have also been updated in the synopses of member
  class templates.
@tkoeppe
[range.chunk.by.view] Change copyable-box to movable-box.
1df7957 
This change wasn't requested by P2494R2, but is necessary.
@jensmaurer @tkoeppe
[range.repeat.view] Apply modifications from P2494R2
d5aea28
@tkoeppe tkoeppe force-pushed the motions-2022-07-lwg-30 branch from 8a7ee6c to d5aea28 Compare August 17, 2022 21:01
@tkoeppe tkoeppe merged commit 0fc522c into main Aug 17, 2022
JohelEGP
JohelEGP reviewed Aug 17, 2022
View reviewed changes
source/ranges.tex
Comment on lines +3382 to +3383
constexpr explicit repeat_view(const W& value, Bound bound = Bound())
requires @\libconcept{copy_constructible}@<W>;
Copy link
Contributor

Choose a reason for hiding this comment

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

The corresponding change is missing in its detailed specifications.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, I'll fix it later

JohelEGP
JohelEGP reviewed Aug 17, 2022
View reviewed changes
source/ranges.tex
@@ -13302,7 +13302,7 @@
requires @\libconcept{view}@<V> && is_object_v<Pred>
class chunk_by_view : public view_interface<chunk_by_view<V, Pred>> {
V @\exposid{base_}@ = V(); // \expos
@\exposid{copyable-box}@<Pred> @\exposid{pred_}@ = Pred(); // \expos
@\exposid{movable-box}@<Pred> @\exposid{pred_}@ = Pred(); // \expos
Copy link
Contributor

Choose a reason for hiding this comment

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

A space is needed before the comment to keep them aligned.

Copy link
Contributor

Choose a reason for hiding this comment

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

Will fix!

@jensmaurer jensmaurer deleted the motions-2022-07-lwg-30 branch November 18, 2022 21:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tkoeppe tkoeppe tkoeppe left review comments

+1 more reviewer

@JohelEGP JohelEGP JohelEGP requested changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
post-2022-07
Development

Successfully merging this pull request may close these issues.

[2022-07 LWG Motion 30] P2494R2 Relaxing range adaptors to allow for move only types P2494 Relaxing range adaptors to allow for move only types
3 participants
@jensmaurer @tkoeppe @JohelEGP