Page MenuHomePhabricator
Differential D117656

[libc++] [ranges] Implement move_sentinel and C++20 move_iterator
ClosedPublic
Actions

Authored by • Quuxplusone on Jan 19 2022, 3:54 AM.

Details

Reviewers
ldionne
philnik
var-const
Mordante
jloser
Group Reviewers
Restricted Project
Commits
rG2fb026ee4d1a: Implement move_sentinel and C++20 move_iterator.

Diff Detail

Unit TestsFailed
View All

TimeTest
330 mslibcxx CI C++03 > llvm-libc++-shared-cfg-in.libcxx/lint::lint_headers.sh.py
Script: -- : 'RUN: at line 1'; /usr/bin/python3.8 /home/libcxx-builder/.buildkite-agent/builds/3d9393666d0e-1/llvm-project/libcxx-ci/libcxx/test/libcxx/lint/lint_headers.sh.py
360 mslibcxx CI C++03 > llvm-libc++-shared-cfg-in.std/iterators/predef_iterators/move_iterators/move_iter_ops/move_iter_op_const::iter.pass.cpp
Script: -- : 'COMPILED WITH'; /usr/bin/c++ /home/libcxx-builder/.buildkite-agent/builds/3d9393666d0e-1/llvm-project/libcxx-ci/libcxx/test/std/iterators/predef.iterators/move.iterators/move.iter.ops/move.iter.op.const/iter.pass.cpp --target=x86_64-unknown-linux-gnu -nostdinc++ -isystem /home/libcxx-builder/.buildkite-agent/builds/3d9393666d0e-1/llvm-project/libcxx-ci/build/generic-cxx03/include/c++/v1 -isystem /home/libcxx-builder/.buildkite-agent/builds/3d9393666d0e-1/llvm-project/libcxx-ci/build/generic-cxx03/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/3d9393666d0e-1/llvm-project/libcxx-ci/libcxx/test/support -std=c++03 -Werror -Wall -Wextra -Wshadow -Wundef -Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move -Wno-c++11-extensions -Wno-user-defined-literals -Wno-noexcept-type -Wno-atomic-alignment -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-unused-local-typedef -D_LIBCPP_DISABLE_AVAILABILITY -fcoroutines-ts -Werror=thread-safety -Wuser-defined-warnings -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -Wno-macro-redefined -D_LIBCPP_HAS_THREAD_API_PTHREAD -Wno-macro-redefined -D_LIBCPP_ABI_VERSION=1 -lc++experimental -nostdlib++ -L /home/libcxx-builder/.buildkite-agent/builds/3d9393666d0e-1/llvm-project/libcxx-ci/build/generic-cxx03/lib -Wl,-rpath,/home/libcxx-builder/.buildkite-agent/builds/3d9393666d0e-1/llvm-project/libcxx-ci/build/generic-cxx03/lib -lc++ -pthread -o /home/libcxx-builder/.buildkite-agent/builds/3d9393666d0e-1/llvm-project/libcxx-ci/build/generic-cxx03/std/iterators/predef.iterators/move.iterators/move.iter.ops/move.iter.op.const/Output/iter.pass.cpp.dir/t.tmp.exe
330 mslibcxx CI C++11 > llvm-libc++-shared-cfg-in.libcxx/lint::lint_headers.sh.py
Script: -- : 'RUN: at line 1'; /usr/bin/python3.8 /home/libcxx-builder/.buildkite-agent/builds/37213b1d5629-1/llvm-project/libcxx-ci/libcxx/test/libcxx/lint/lint_headers.sh.py
440 mslibcxx CI C++14 > llvm-libc++-shared-cfg-in.libcxx/lint::lint_headers.sh.py
Script: -- : 'RUN: at line 1'; /usr/bin/python3.8 /home/libcxx-builder/.buildkite-agent/builds/04aeedd4a736-1/llvm-project/libcxx-ci/libcxx/test/libcxx/lint/lint_headers.sh.py
330 mslibcxx CI C++17 > llvm-libc++-shared-cfg-in.libcxx/lint::lint_headers.sh.py
Script: -- : 'RUN: at line 1'; /usr/bin/python3.8 /home/libcxx-builder/.buildkite-agent/builds/7d3f6681b420-1/llvm-project/libcxx-ci/libcxx/test/libcxx/lint/lint_headers.sh.py
View Full Test Results (8 Failed)

Event Timeline

• Quuxplusone created this revision.Jan 19 2022, 3:54 AM
Herald added a subscriber: mgorny. · View Herald TranscriptJan 19 2022, 3:54 AM
• Quuxplusone requested review of this revision.Jan 19 2022, 3:54 AM
Herald added 1 blocking reviewer(s): Restricted Project. · View Herald TranscriptJan 19 2022, 3:54 AM
Herald added a subscriber: libcxx-commits. · View Herald Transcript
• Quuxplusone planned changes to this revision.Jan 19 2022, 3:54 AM
• Quuxplusone added a parent revision: D117327: [libc++] Fix LWG3390: move_iterator now handles move-only iterators..
Harbormaster completed remote builds in B144243: Diff 401165.Jan 19 2022, 4:18 AM
ldionne mentioned this in D117327: [libc++] Fix LWG3390: move_iterator now handles move-only iterators..Jan 24 2022, 8:22 AM
• Quuxplusone updated this revision to Diff 403488.EditedJan 26 2022, 8:52 PM
• Quuxplusone removed a parent revision: D117327: [libc++] Fix LWG3390: move_iterator now handles move-only iterators..
• Quuxplusone retitled this revision from [libc++] [ranges] move_sentinel and C++20 move_iterator [WIP, needs tests] to [libc++] [ranges] move_sentinel and C++20 move_iterator [WIP, needs more tests].

Added tests for move_sentinel. Still missing: tests for the iterator_traits of move_iterator; tests for three-way comparison; tests for iter_move and iter_swap.

• Quuxplusone added inline comments.Jan 26 2022, 8:55 PM
libcxx/include/__iterator/move_iterator.h
191

I found that writing the naïve requires requires { __x.base() == __y.base(); } here did not work; Clang complains something about accessing into incomplete type move_iterator<X> when you go to instantiate it. Pulling the constraint out into a concept outside the body of the class seems to fix the issue.

Harbormaster completed remote builds in B145910: Diff 403488.Jan 26 2022, 9:18 PM
• Quuxplusone mentioned this in D120180: [libc++][ranges] Implement changes to reverse_iterator from One Ranges Proposal..Feb 19 2022, 8:07 AM
• Quuxplusone added a parent revision: D120417: [libc++] [ranges] Fix `decltype(auto) ranges::iter_move`.Mar 2 2022, 5:51 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 2 2022, 5:51 PM
• Quuxplusone updated this revision to Diff 412586.Mar 2 2022, 6:23 PM
• Quuxplusone retitled this revision from [libc++] [ranges] move_sentinel and C++20 move_iterator [WIP, needs more tests] to [libc++] [ranges] Implement move_sentinel and C++20 move_iterator.
• Quuxplusone edited the summary of this revision. (Show Details)
• Quuxplusone added reviewers: philnik, var-const, Mordante, jloser.
• Quuxplusone set the repository for this revision to rG LLVM Github Monorepo.

Rebase, add more tests for comparison operators.
Could still use more tests for

  • iterator_traits
  • iter_move
  • iter_swap

But I think this is pretty much ready to have people start hammering on it.

jloser added inline comments.Mar 2 2022, 7:45 PM
libcxx/include/__iterator/move_iterator.h
105

s/_LIBCPP_CONSTEXPR_AFTER_CXX14/constexpr here and below since we're already guarded with #if _LIBCPP_STD_VER > 17.

191

I'll buy that. I've also run into this several times in other codebases with requires requires expressions in member functions of a class template. It's not great, but the usual "pull it out to a named concept" works.

Harbormaster completed remote builds in B152281: Diff 412586.Mar 2 2022, 8:49 PM
• Quuxplusone updated this revision to Diff 413142.Mar 4 2022, 2:26 PM

Rebase, coalesce some repeated ifdefs, use unadorned constexpr in C++20-only codepaths

Harbormaster completed remote builds in B152677: Diff 413142.Mar 4 2022, 8:52 PM
• Quuxplusone mentioned this in D107500: [libc++][ranges] Implement `lazy_split_view`..Mar 8 2022, 10:29 AM
ldionne accepted this revision.Apr 12 2022, 3:58 PM

This LGTM -- I think we can add a few more tests post-commit, but this looks reasonable as-is too. I'll rebase and ship this.

This revision is now accepted and ready to land.Apr 12 2022, 3:58 PM
Closed by commit rG2fb026ee4d1a: Implement move_sentinel and C++20 move_iterator. (authored by • arthur.j.odwyer, committed by ldionne). · Explain WhyApr 13 2022, 6:51 AM
This revision was automatically updated to reflect the committed changes.
ldionne added a commit: rG2fb026ee4d1a: Implement move_sentinel and C++20 move_iterator..
ldionne mentioned this in rGdb6421ec58e1: [libc++] Post-commit adjustments after rebasing D117656.
var-const mentioned this in D126053: [libc++][ranges] Finish LWG issues directly related to the One Ranges Proposal..Jun 27 2022, 7:36 PM
var-const mentioned this in rG79a2b4ba98a1: [libc++][ranges] Finish LWG issues directly related to the One Ranges Proposal..Jun 28 2022, 12:01 PM
H-G-Hristov mentioned this in D145243: [libc++][spaceship][NFC] Marked `operator<=>` for `move_iterator` and `counted_iterator` as implemented.Mar 3 2023, 6:40 AM
philnik mentioned this in rGcc6de9f6a5bd: [libc++][spaceship][NFC] Marked `operator<=>` for `move_iterator` and….Mar 5 2023, 4:06 PM

Revision Contents
Changeset List

PathSize
libcxx/
docs/
Status/
2 lines
include/
1 line
__iterator/
166 lines
57 lines
1 line
1 line
test/
libcxx/
diagnostics/
detail.headers/
iterator/
15 lines
std/
iterators/
predef.iterators/
move.iterators/
move.iter.ops/
move.iter.op.const/
70 lines
move.iter.op.ref/
40 lines
move.iter.op=/
3 lines
move.iterator/
108 lines

Diff 401165

View Options

libcxx/docs/Status/Cxx20Issues.csv

Loading...
View Options

libcxx/include/CMakeLists.txt

Loading...
View Options

libcxx/include/__iterator/move_iterator.h

Loading...
View Options

libcxx/include/__iterator/move_sentinel.h

Loading...
View Options

libcxx/include/iterator

Loading...
View Options

libcxx/include/module.modulemap

Loading...
View Options

libcxx/test/libcxx/diagnostics/detail.headers/iterator/move_sentinel.module.verify.cpp

Loading...
View Options

libcxx/test/std/iterators/predef.iterators/move.iterators/move.iter.ops/move.iter.op.const/iter.pass.cpp

Loading...
View Options

libcxx/test/std/iterators/predef.iterators/move.iterators/move.iter.ops/move.iter.op.ref/op_arrow.pass.cpp

Loading...
View Options

libcxx/test/std/iterators/predef.iterators/move.iterators/move.iter.ops/move.iter.op=/move_iterator.pass.cpp

Loading...
View Options

libcxx/test/std/iterators/predef.iterators/move.iterators/move.iterator/iterator_concept_conformance.compile.pass.cpp

Loading...