Page MenuHomePhabricator

[libc++] Fix LWG3390: move_iterator now handles move-only iterators.
ClosedPublic

Authored by Quuxplusone on Jan 14 2022, 8:28 AM.

Details

Summary

This can't really be tested until C++20 move_iterator is completely implemented, but I'm working on that.

Depends on D117324.

Diff Detail

Unit TestsFailed

TimeTest
430 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/04aeedd4a736-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/04aeedd4a736-1/llvm-project/libcxx-ci/build/generic-cxx03/include/c++/v1 -isystem /home/libcxx-builder/.buildkite-agent/builds/04aeedd4a736-1/llvm-project/libcxx-ci/build/generic-cxx03/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/04aeedd4a736-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/04aeedd4a736-1/llvm-project/libcxx-ci/build/generic-cxx03/lib -Wl,-rpath,/home/libcxx-builder/.buildkite-agent/builds/04aeedd4a736-1/llvm-project/libcxx-ci/build/generic-cxx03/lib -lc++ -pthread -o /home/libcxx-builder/.buildkite-agent/builds/04aeedd4a736-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
750 mslibcxx CI C++20 > 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/62e9a7af32d2-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/62e9a7af32d2-1/llvm-project/libcxx-ci/build/generic-cxx20/include/c++/v1 -isystem /home/libcxx-builder/.buildkite-agent/builds/62e9a7af32d2-1/llvm-project/libcxx-ci/build/generic-cxx20/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/62e9a7af32d2-1/llvm-project/libcxx-ci/libcxx/test/support -std=c++20 -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/62e9a7af32d2-1/llvm-project/libcxx-ci/build/generic-cxx20/lib -Wl,-rpath,/home/libcxx-builder/.buildkite-agent/builds/62e9a7af32d2-1/llvm-project/libcxx-ci/build/generic-cxx20/lib -lc++ -pthread -o /home/libcxx-builder/.buildkite-agent/builds/62e9a7af32d2-1/llvm-project/libcxx-ci/build/generic-cxx20/std/iterators/predef.iterators/move.iterators/move.iter.ops/move.iter.op.const/Output/iter.pass.cpp.dir/t.tmp.exe
570 mslibcxx CI C++2b > 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/e27f39486111-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/e27f39486111-1/llvm-project/libcxx-ci/build/generic-cxx2b/include/c++/v1 -isystem /home/libcxx-builder/.buildkite-agent/builds/e27f39486111-1/llvm-project/libcxx-ci/build/generic-cxx2b/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/e27f39486111-1/llvm-project/libcxx-ci/libcxx/test/support -std=c++2b -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/e27f39486111-1/llvm-project/libcxx-ci/build/generic-cxx2b/lib -Wl,-rpath,/home/libcxx-builder/.buildkite-agent/builds/e27f39486111-1/llvm-project/libcxx-ci/build/generic-cxx2b/lib -lc++ -pthread -o /home/libcxx-builder/.buildkite-agent/builds/e27f39486111-1/llvm-project/libcxx-ci/build/generic-cxx2b/std/iterators/predef.iterators/move.iterators/move.iter.ops/move.iter.op.const/Output/iter.pass.cpp.dir/t.tmp.exe
460 mslibcxx CI GCC 11 / C++latest > llvm-libc++-shared-gcc-cfg-in.std/iterators/predef_iterators/move_iterators/move_iter_ops/move_iter_op_const::iter.pass.cpp
Script: -- : 'COMPILED WITH'; /usr/bin/g++-11 /home/libcxx-builder/.buildkite-agent/builds/f7c9c9c4e548-1/llvm-project/libcxx-ci/libcxx/test/std/iterators/predef.iterators/move.iterators/move.iter.ops/move.iter.op.const/iter.pass.cpp -nostdinc++ -isystem /home/libcxx-builder/.buildkite-agent/builds/f7c9c9c4e548-1/llvm-project/libcxx-ci/build/generic-gcc/include/c++/v1 -isystem /home/libcxx-builder/.buildkite-agent/builds/f7c9c9c4e548-1/llvm-project/libcxx-ci/build/generic-gcc/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/f7c9c9c4e548-1/llvm-project/libcxx-ci/libcxx/test/support -std=c++2b -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-aligned-allocation-unavailable -Wno-atomic-alignment -Wno-sized-deallocation -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-unused-local-typedef -D_LIBCPP_DISABLE_AVAILABILITY -Wno-macro-redefined -D_LIBCPP_HAS_THREAD_API_PTHREAD -Wno-macro-redefined -D_LIBCPP_ABI_VERSION=1 -lc++experimental -L /home/libcxx-builder/.buildkite-agent/builds/f7c9c9c4e548-1/llvm-project/libcxx-ci/build/generic-gcc/lib -Wl,-rpath,/home/libcxx-builder/.buildkite-agent/builds/f7c9c9c4e548-1/llvm-project/libcxx-ci/build/generic-gcc/lib -nodefaultlibs -lc++ -lm -lgcc_s -lgcc -lpthread -lc -lgcc_s -lgcc -latomic -o /home/libcxx-builder/.buildkite-agent/builds/f7c9c9c4e548-1/llvm-project/libcxx-ci/build/generic-gcc/std/iterators/predef.iterators/move.iterators/move.iter.ops/move.iter.op.const/Output/iter.pass.cpp.dir/t.tmp.exe

Event Timeline

Quuxplusone requested review of this revision.Jan 14 2022, 8:28 AM
Quuxplusone created this revision.
Herald added 1 blocking reviewer(s): Restricted Project. · View Herald TranscriptJan 14 2022, 8:28 AM

This can't really be tested until C++20 move_iterator is completely implemented, but I'm working on that.

Can't we add a simple test with a move_iterator constructed from a non-copyable iterator and ensure that works? What else is missing?

This LGTM but I'd like to see tests for this, so I'll wait to see the child revisions I assume you're about to upload.

@ldionne: I've uploaded my proposed test now, but as it turns out, it fails because move_it.base() returns by-value in C++17; actually pulling out the .base() of a move-only move_iterator requires the C++20 changes to make .base() return by-const-ref.

I'd still prefer to land this as-is (without that test for now), but I guess I could also abandon this. My original rationale for opening this individually was that this is a (minor) change to the pre-C++20 behavior — it now moves instead of copying. I think everything else I'm doing affects our behavior only in C++20-and-later.

(Okay, so technically this behavior is observable and therefore testable pre-C++20; I'd just have to make a local iterator type that is both copy-ctable and move-ctable, but its copy-ctor counts the number of times it's called, and verify that make_move_iterator(CountingItType()) never calls the copy-ctor. But that seems like overkill, IMVHO.)

ldionne accepted this revision.Jan 24 2022, 8:22 AM

Ahh I see. Fine with this as-is without the test. I like to have a single commit for this LWG issue. My understanding is that you will then test that (and more) in D117656.

This revision is now accepted and ready to land.Jan 24 2022, 8:22 AM
This revision was landed with ongoing or failed builds.Jan 26 2022, 8:50 PM
This revision was automatically updated to reflect the committed changes.