This is an archive of the discontinued LLVM Phabricator instance.

[libcxx][iterator] adds `std::ranges::prev`

Authored by cjdb on May 15 2021, 6:41 PM.



Implements part of P0896 'The One Ranges Proposal'.
Implements [range.iter.op.prev].

Depends on D102563.

Diff Detail

Event Timeline

cjdb created this revision.May 15 2021, 6:41 PM
cjdb requested review of this revision.May 15 2021, 6:41 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 15 2021, 6:41 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript

Do we need to mark anything down in the status?


FWIW I think it makes sense to have nodiscard here because it might be confusing to users that think next behaves like advance (advance takes a reference, this one doesn't).


Synopsis update.

SGTM, but I would like to know what our way forward will be with [[nodiscard]] before approving.

13 ↗(On Diff #345659)

next -> prev

13 ↗(On Diff #345659)

next -> prev

43 ↗(On Diff #345659)

@zoecarver wasn't expected-error@*:* an issue with older versions of Clang?

cjdb added a comment.May 18 2021, 1:42 PM

SGTM, but I would like to know what our way forward will be with [[nodiscard]] before approving.

I think this patch was put up for review after several days of no opposition to my proposal, but before your reply (which I need to reply to).

Quuxplusone added inline comments.

Yes, _LIBCPP_NODISCARD_EXT plz, being as this is an extension to the Standard. (The Standard doesn't have [[nodiscard]] here: )

@cjdb wrote: "my proposal" — FWIW, I haven't seen anything on the Discord, or the Slack, or libcxx-dev, related to nodiscard. (Maybe I'm not on libcxx-dev? I also sent two emails to libcxx-dev about how debug iterators are totally broken, and never received any replies.) How about you post on the Discord, and/or send an email to the-relevant-people-directly? I would think at least Louis/Zoe/myself/Mark/EricWF. I'll also go poke the Discord right now.

Mordante accepted this revision as: Mordante.May 20 2021, 1:10 PM

LGTM after updating the synopsis, the small comment changes and adjusting the [[nodiscard]] after discussing it on Discord.

cjdb updated this revision to Diff 347843.May 25 2021, 9:54 PM

Applies @ldionne's feedback (from D102563, since these two patches are siblings):

  • removes [[nodiscard]] from ranges::next (per offline discussion: things not explicitly marked [[nodiscard]] in the wording to be on hold till after P2351R0 and P2377R0 are discussed in Library Evolution)
  • removes [[nodiscard]] from tests
  • splits single next.pass.cpp into four files, one for each overload
ldionne accepted this revision.May 26 2021, 4:06 PM
This revision is now accepted and ready to land.May 26 2021, 4:06 PM
cjdb updated this revision to Diff 348143.May 26 2021, 7:05 PM

rebases to activate CI

This revision was automatically updated to reflect the committed changes.