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

[reverse.iterators] Dissolve single-item subclauses. #1832

Merged
merged 1 commit into from Mar 31, 2018

Conversation

jensmaurer
Copy link
Member

Partially addresses #1429.

@jensmaurer
Copy link
Member Author

@zygoloid : This merge went the wrong way: You merged "master" into my branch, it seems, rather than the other way round.

@jensmaurer
Copy link
Member Author

Rebased.

@zygoloid
Copy link
Member

@jensmaurer This is github's "rebase someone else's PR" feature. Sadly it appears to be broken right now (it fails to actually re-check whether the PR is mergeable afterwards or something).


\indexlibrarymember{operator+}{reverse_iterator}%
\begin{itemdecl}
constexpr reverse_iterator operator+(difference_type n) const;
Copy link
Member

Choose a reason for hiding this comment

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

I find it a bit strange to call this an "observer", since it does not of itself provide any mechanism to actually observe the state of the iterator. (It seems a little odd to call operator*, operator->, and operator[] observers, rather than something like "element access", too).

I might be tempted to instead lump operator++, operator--, operator+, operator+=, operator-, operator-= under something like "reverse_iterator navigation", put these three under "reverse_iterator element access", and keep base as a single-item subclause (which I think makes sense, as it's the one and only part of the reverse_iterator interface that is not part of some Iterator concept).

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds reasonable. Done.

@AlisdairM
Copy link
Contributor

This is worth calling out to @mclow as it will affect a small number of issues in the LWG issues list. I think the proposed change looks cleaner though.

@mclow
Copy link
Contributor

mclow commented Jan 8, 2018

Thanks for the callout; I can fix the issues.
Anytime you change a stable name, I'd appreciate a shout-out.

@zygoloid zygoloid added the needs rebase The pull request needs a git rebase to resolve merge conflicts. label Feb 12, 2018
@zygoloid
Copy link
Member

Postponed to post-meeting mailing due to changes to stable names.

@jensmaurer jensmaurer removed the needs rebase The pull request needs a git rebase to resolve merge conflicts. label Feb 12, 2018
@jensmaurer
Copy link
Member Author

Rebased.

@jensmaurer jensmaurer added the decision-required A decision of the editorial group (or the Project Editor) is required. label Feb 12, 2018
@zygoloid zygoloid added after-motions Pull request is to be applied after the pending edits from WG21 straw polls have been applied. and removed decision-required A decision of the editorial group (or the Project Editor) is required. labels Mar 16, 2018
@zygoloid
Copy link
Member

To be merged after Jacksonville motions.

@tkoeppe tkoeppe merged commit e747966 into cplusplus:master Mar 31, 2018
@jensmaurer jensmaurer deleted the b19 branch March 31, 2018 18:01
@@ -1315,8 +1313,6 @@
\tcode{u.current}.
\end{itemdescr}

\rSec4[reverse.iter.op=]{\tcode{reverse_iterator::operator=}}
Copy link
Member

Choose a reason for hiding this comment

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

We're missing an xrefdelta entry for this.

\movedxref{reverse.iter.op+=}{reverse.iter.nav}
\movedxref{reverse.iter.op\dcr}{reverse.iter.nav}
\movedxref{reverse.iter.op-=}{reverse.iter.nav}
\movedxref{reverse.iter.op++}{reverse.iter.nav}
Copy link
Member

Choose a reason for hiding this comment

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

We already did this one.

\movedxref{reverse.iter.op>=}{reverse.iter.cmp}
\movedxref{reverse.iter.op<=}{reverse.iter.cmp}

\movedxref{reverse.iter.star}{reverse.iter.elem}
Copy link
Member

Choose a reason for hiding this comment

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

This was called reverse.iter.op.star, not reverse.iter.star, for some reason.

@jensmaurer
Copy link
Member Author

@zygoloid, your comments are addressed in pull request #2015.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
after-motions Pull request is to be applied after the pending edits from WG21 straw polls have been applied.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants