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

[range.join.iterator, range.join.with.iterator] Add InnerBase and replace more OuterIter/InnerIter #5478

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

hewillk
Copy link
Contributor

@hewillk hewillk commented May 19, 2022

This is basically a follow-up to #5474, which mainly does three things

  1. Added type alias InnerBase for join_view::iterator to be consistent with join_with_view::iterator.
  2. Replaced more InnerIter/OuterIter for join_view::iterator.
  3. Replaced one missing OuterIter for join_with_view::iterator and corrected the italics for Base in using InnerBase = range_reference_t<Base>;.

Hope there are no errors.

@@ -5809,11 +5810,11 @@
\begin{itemize}
\item If \exposid{ref-is-glvalue} is \tcode{true},
\exposid{Base} models \libconcept{bidirectional_range}, and
\tcode{range_reference_t<\exposid{Base}>} models
both \libconcept{bidirectional_range} and \libconcept{common_range},
\tcode{\exposid{InnerBase}} models
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
\tcode{\exposid{InnerBase}} models
\exposid{InnerBase} models

then \tcode{iterator_concept} denotes \tcode{bidirectio\-nal_iterator_tag}.
\item Otherwise, if \exposid{ref-is-glvalue} is \tcode{true} and
\exposid{Base} and \tcode{range_reference_t<\exposid{Base}>}
\exposid{Base} and \tcode{\exposid{InnerBase}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
\exposid{Base} and \tcode{\exposid{InnerBase}}
\exposid{Base} and \exposid{InnerBase}

@@ -5741,11 +5741,12 @@
private:
using @\exposidnc{Parent}@ = @\exposidnc{maybe-const}@<Const, join_view>; // \expos
using @\exposidnc{Base}@ = @\exposidnc{maybe-const}@<Const, V>; // \expos
using @\exposidnc{InnerBase}@ = range_reference_t<@\exposid{Base}@>; // \expos
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
using @\exposidnc{InnerBase}@ = range_reference_t<@\exposid{Base}@>; // \expos
using @\exposidnc{InnerBase}@ = range_reference_t<@\exposidnc{Base}@>; // \expos

Not really necessary, but consistent, I guess.

\tcode{range_reference_t<\exposid{Base}>} models
both \libconcept{bidirectional_range} and \libconcept{common_range},
\tcode{\exposid{InnerBase}} models
both \libconcept{bidire\-ctional_range} and \libconcept{common_range},
Copy link
Contributor

Choose a reason for hiding this comment

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

It this the correct place to break? Shouldn't it be after the "c"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, the check will fail if after "c".

Copy link
Member

Choose a reason for hiding this comment

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

We don't violate English hyphenation rules. Try breaking even earlier.

@@ -5823,18 +5824,18 @@
The member \grammarterm{typedef-name} \tcode{iterator_category} is defined
if and only if \exposid{ref-is-glvalue} is \tcode{true},
\exposid{Base} models \libconcept{forward_range}, and
\tcode{range_reference_t<\exposid{Base}>} models \libconcept{forward_range}.
\tcode{\exposid{InnerBase}} models \libconcept{forward_range}.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
\tcode{\exposid{InnerBase}} models \libconcept{forward_range}.
\exposid{InnerBase} models \libconcept{forward_range}.

\item If
\placeholder{OUTERC} and \placeholder{INNERC} each model
\tcode{\libconcept{derived_from}<bidirectional_iterator_tag>} and
\tcode{range_reference_t<\exposid{Base}>} models \libconcept{common_range},
\tcode{\exposid{InnerBase}} models \libconcept{common_range},
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
\tcode{\exposid{InnerBase}} models \libconcept{common_range},
\exposid{InnerBase} models \libconcept{common_range},

@hewillk
Copy link
Contributor Author

hewillk commented May 20, 2022

It looks fine for me now, thanks for the review.

@wg21bot wg21bot added the needs rebase The pull request needs a git rebase to resolve merge conflicts. label Aug 19, 2022
@tkoeppe
Copy link
Contributor

tkoeppe commented Mar 13, 2023

@hewillk This PR needs some attention.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs rebase The pull request needs a git rebase to resolve merge conflicts.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants