This is an archive of the discontinued LLVM Phabricator instance.

[libcxx][ranges] Add `contiguous_iterator`.

Authored by zoecarver on Apr 27 2021, 1:12 PM.


Diff Detail

Event Timeline

zoecarver requested review of this revision.Apr 27 2021, 1:12 PM
zoecarver created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptApr 27 2021, 1:12 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript

Most importantly, please check the test iterators from test_iterators.h — e.g.


Any test iterators that don't match the concepts will want to be fixed.

zoecarver added inline comments.Apr 27 2021, 1:55 PM

I have those a few lines above:

static_assert(!std::contiguous_iterator<input_iterator<int*> >);
static_assert(!std::contiguous_iterator<forward_iterator<int*> >);
static_assert(!std::contiguous_iterator<bidirectional_iterator<int*> >);
static_assert(!std::contiguous_iterator<random_access_iterator<int*> >);
static_assert(std::contiguous_iterator<contiguous_iterator<int*> >);
cjdb accepted this revision.Apr 27 2021, 3:01 PM

Missing conformance tests:

  • deque (negative test)
  • vector
  • span
  • string
  • reverse_iterator

LGTM, pending CI and added conformance tests.

zoecarver planned changes to this revision.May 6 2021, 11:33 AM
zoecarver updated this revision to Diff 344172.May 10 2021, 1:15 PM
  • Add conformance tests.
  • Fix review comments.
  • Detatch from contiguous_range.
zoecarver retitled this revision from [libcxx][ranges] Add `contiguous_{iterator,range}`. to [libcxx][ranges] Add `contiguous_iterator`..May 10 2021, 1:16 PM
zoecarver added inline comments.
201 ↗(On Diff #344172)

I'm going to make this into a separate PR with test.

zoecarver updated this revision to Diff 344175.May 10 2021, 1:19 PM
  • Grep/fix the last few conformance tests.
zoecarver updated this revision to Diff 344177.May 10 2021, 1:20 PM
  • Remove final remaining range test.

(Sorry for all the churn/emails!)

zoecarver updated this revision to Diff 344184.May 10 2021, 1:31 PM
  • Rebase on main to get the to_address fix.
zoecarver updated this revision to Diff 344185.May 10 2021, 1:34 PM
  • Reset changes in pointer_traits to fix the diff.

Fixing the build failures now. @Quuxplusone friendly ping :)

zoecarver updated this revision to Diff 345289.May 13 2021, 2:34 PM

Fix the two failing tests.


Just want to confirm this is correct: vector<bool> is not a contiguous container (while normal vector is).

cjdb added inline comments.May 13 2021, 2:40 PM


LGTM % comments


At some point: Alphabetize.


Please keep static_assert(std::random_access_iterator<reverse_iterator>); (just add the new negative asserts).


concept contiguous_iterator;


(A) this could be operator<=>?
(B) pass-by-const-value alert! I think you wanted pass-by-const-reference.
Ditto throughout.


Please keep the assert for


(just add the new negative assert). This also applies anywhere else in this PR that you removed a positive assert without replacing it with a stronger positive assert (if any such places exist).

zoecarver updated this revision to Diff 345462.May 14 2021, 9:22 AM

Apply Arthur's comments.

zoecarver updated this revision to Diff 345463.May 14 2021, 9:25 AM

Remove extra requires that's no longer needed now that to_address is SFINAE friendly.

Add back SFINAE fix.

This revision was not accepted when it landed; it landed in state Needs Review.May 14 2021, 3:28 PM
This revision was automatically updated to reflect the committed changes.