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.

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

P2374R4 views::cartesian_product #5647

Merged
merged 2 commits into from Aug 17, 2022
Merged

P2374R4 views::cartesian_product #5647

merged 2 commits into from Aug 17, 2022

Conversation

jensmaurer
Copy link
Member

@jensmaurer jensmaurer added this to the post-2022-07 milestone Jul 25, 2022
JohelEGP
JohelEGP suggested changes Jul 25, 2022
View reviewed changes
source/ranges.tex Outdated Show resolved Hide resolved
source/ranges.tex Outdated Show resolved Hide resolved
source/ranges.tex Outdated Show resolved Hide resolved
source/ranges.tex Outdated Show resolved Hide resolved
source/ranges.tex Outdated Show resolved Hide resolved
source/ranges.tex Outdated Show resolved Hide resolved
source/ranges.tex
Comment on lines +13452 to +14190
$[\tcode{ranges::distance(*this, ranges::begin(*\exposid{parent_}))},$\newline
$\tcode{ranges::distance(*this, ranges::end(*\exposid{parent_}))}]$.
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't \crange work here?

Copy link
Member Author

Choose a reason for hiding this comment

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

  • \crange won't probably apply the \newline in the middle.
  • I'm still not convinced that \range and friends are the right tools for mathemetical (not: iterator) ranges, despite ample precedent.

Copy link
Contributor

Choose a reason for hiding this comment

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

  • \crange won't probably apply the \newline in the middle.

Maybe \brk{} could work:

regex.tex:\range{p}{p + char_traits<charT>::\brk{}length(p)}, and
strings.tex:@\range{str.data()}{\brk{}str.data() + str.size()}@
utilities.tex:In the function descriptions that follow, let $i$ be in the range \range{0}{sizeof...\brk{}(Types)}
  • I'm still not convinced that \range and friends are the right tools for mathemetical (not: iterator) ranges, despite ample precedent.

That's fair. In term of maintenance, it'd probably be easier to use \range and friends until some math range macros are made available if they're worth it. That way, you don't miss out updating manual math ranges.

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's open an issue about how to spell ranges then. It sounds like that's not entirely obvious.

source/ranges.tex Outdated Show resolved Hide resolved
source/ranges.tex Outdated Show resolved Hide resolved
source/ranges.tex Outdated Show resolved Hide resolved
@JohelEGP

This comment was marked as resolved.

Sign in to view
@JohelEGP
Copy link
Contributor

According to #5694 (comment), tuple-or-pair should be replaced by tuple.

@jensmaurer jensmaurer force-pushed the motions-2022-07-lwg-18 branch 2 times, most recently from 503c318 to 84ce17f Compare August 4, 2022 07:20
@JohelEGP JohelEGP mentioned this pull request Aug 7, 2022
Merged
@tkoeppe tkoeppe force-pushed the motions-2022-07-lwg-18 branch 3 times, most recently from e0f69c7 to 46c267c Compare August 16, 2022 23:38
JohelEGP
JohelEGP suggested changes Aug 16, 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.

There are uses of tuple-or-pair left.

@JohelEGP
Copy link
Contributor

Actually, those uses are its definition. #5694 also removes it. It's now an unused exposition-only thing, so I think this PR can also remove it.

@tkoeppe
Copy link
Contributor

tkoeppe commented Aug 16, 2022

Actually, those uses are its definition. #5694 also removes it. It's now an unused exposition-only thing, so I think this PR can also remove it.

Right. Is there an edit instruction anywhere to remove that? If not then let's remove it after the motions.

@tkoeppe
Copy link
Contributor

tkoeppe commented Aug 16, 2022 •
edited

@JohelEGP: Hm, tuple-transform still uses tuple-or-pair -- should that also be updated?

jensmaurer and others added 2 commits August 17, 2022 01:03
@jensmaurer @tkoeppe
P2374R4 views::cartesian_product
b77c526
@tkoeppe
[range.cartesian] Replace "tuple-or-pair" with "tuple".
d899065 
These changes are part of LWG-Motion 12 (P2165R4, "Compatibility
between tuple and tuple-like objects").
@JohelEGP
Copy link
Contributor

Yes. The actual instruction in https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2022/p2165r4.pdf is

[Editor’s note: Replace all usages of
tuple-or-pair by tuple in the range clause. This includes
[range.zip], [range.adjacent.iterator] as well as [range.cartesian] if P2374 is adopted]

And tuple-transform is in the "range clause".

@tkoeppe tkoeppe merged commit d6974c5 into main Aug 17, 2022
This was referenced Aug 17, 2022
Closed
Closed
Merged
Closed
@jensmaurer jensmaurer deleted the motions-2022-07-lwg-18 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

@JohelEGP JohelEGP JohelEGP requested changes

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 18] P2374R4 views::cartesian_product P2374 views::cartesian_product
3 participants
@jensmaurer @JohelEGP @tkoeppe