Page MenuHomePhabricator

[libcxx][ranges][nfc] Factor out common logic in the copy algorithms.
Needs RevisionPublic

Authored by zoecarver on Jun 7 2021, 1:55 PM.

Details

Reviewers
ldionne
cjdb
EricWF
Quuxplusone
Group Reviewers
Restricted Project
Summary

Factors out common logic from std::copy, std::copy_backward, std::copy_if, and std::copy_n into __copy_*_impl. Also adds __in_out_result which is the return type for the impl function. This will make adding support for the ranges overloads super easy.

This is an example of how this will be done for all algorithms. Once this lands, I'll update all the algorithms, so let's get this review right.

Diff Detail

Unit TestsFailed

TimeTest
1,680 mslibcxx CI C++03 > libcxx-trunk-shared.libcxx::double_include.sh.cpp
Script: -- : 'RUN: at line 12'; /usr/bin/clang++ -c /home/libcxx-builder/.buildkite-agent/builds/54dee0c6df63-1/llvm-project/libcxx-ci/libcxx/test/libcxx/double_include.sh.cpp -o /home/libcxx-builder/.buildkite-agent/builds/54dee0c6df63-1/llvm-project/libcxx-ci/build/generic-cxx03/projects/libcxx/libcxx/Output/double_include.sh.cpp.dir/t.tmp.first.o --target=x86_64-unknown-linux-gnu -nostdinc++ -isystem /home/libcxx-builder/.buildkite-agent/builds/54dee0c6df63-1/llvm-project/libcxx-ci/build/generic-cxx03/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/54dee0c6df63-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 -fmodules-cache-path=/home/libcxx-builder/.buildkite-agent/builds/54dee0c6df63-1/llvm-project/libcxx-ci/build/generic-cxx03/projects/libcxx/libcxx/Output/double_include.sh.cpp.dir/t.tmp/ModuleCache -Wno-macro-redefined -D_LIBCPP_HAS_THREAD_API_PTHREAD -Wno-macro-redefined -D_LIBCPP_ABI_VERSION=1
1,680 mslibcxx CI C++03 > libcxx-trunk-shared.libcxx::double_include.sh.cpp
Script: -- : 'RUN: at line 12'; /usr/bin/clang++ -c /home/libcxx-builder/.buildkite-agent/builds/54dee0c6df63-1/llvm-project/libcxx-ci/libcxx/test/libcxx/double_include.sh.cpp -o /home/libcxx-builder/.buildkite-agent/builds/54dee0c6df63-1/llvm-project/libcxx-ci/build/generic-cxx03/projects/libcxx/libcxx/Output/double_include.sh.cpp.dir/t.tmp.first.o --target=x86_64-unknown-linux-gnu -nostdinc++ -isystem /home/libcxx-builder/.buildkite-agent/builds/54dee0c6df63-1/llvm-project/libcxx-ci/build/generic-cxx03/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/54dee0c6df63-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 -fmodules-cache-path=/home/libcxx-builder/.buildkite-agent/builds/54dee0c6df63-1/llvm-project/libcxx-ci/build/generic-cxx03/projects/libcxx/libcxx/Output/double_include.sh.cpp.dir/t.tmp/ModuleCache -Wno-macro-redefined -D_LIBCPP_HAS_THREAD_API_PTHREAD -Wno-macro-redefined -D_LIBCPP_ABI_VERSION=1
1,680 mslibcxx CI C++03 > libcxx-trunk-shared.libcxx::double_include.sh.cpp
Script: -- : 'RUN: at line 12'; /usr/bin/clang++ -c /home/libcxx-builder/.buildkite-agent/builds/54dee0c6df63-1/llvm-project/libcxx-ci/libcxx/test/libcxx/double_include.sh.cpp -o /home/libcxx-builder/.buildkite-agent/builds/54dee0c6df63-1/llvm-project/libcxx-ci/build/generic-cxx03/projects/libcxx/libcxx/Output/double_include.sh.cpp.dir/t.tmp.first.o --target=x86_64-unknown-linux-gnu -nostdinc++ -isystem /home/libcxx-builder/.buildkite-agent/builds/54dee0c6df63-1/llvm-project/libcxx-ci/build/generic-cxx03/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/54dee0c6df63-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 -fmodules-cache-path=/home/libcxx-builder/.buildkite-agent/builds/54dee0c6df63-1/llvm-project/libcxx-ci/build/generic-cxx03/projects/libcxx/libcxx/Output/double_include.sh.cpp.dir/t.tmp/ModuleCache -Wno-macro-redefined -D_LIBCPP_HAS_THREAD_API_PTHREAD -Wno-macro-redefined -D_LIBCPP_ABI_VERSION=1
1,610 mslibcxx CI C++03 > libcxx-trunk-shared.libcxx::min_max_macros.compile.pass.cpp
Script: -- : 'COMPILED WITH'; /usr/bin/clang++ /home/libcxx-builder/.buildkite-agent/builds/54dee0c6df63-1/llvm-project/libcxx-ci/libcxx/test/libcxx/min_max_macros.compile.pass.cpp --target=x86_64-unknown-linux-gnu -nostdinc++ -isystem /home/libcxx-builder/.buildkite-agent/builds/54dee0c6df63-1/llvm-project/libcxx-ci/build/generic-cxx03/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/54dee0c6df63-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 -fmodules-cache-path=/home/libcxx-builder/.buildkite-agent/builds/54dee0c6df63-1/llvm-project/libcxx-ci/build/generic-cxx03/projects/libcxx/libcxx/Output/min_max_macros.compile.pass.cpp.dir/t.tmp/ModuleCache -Wno-macro-redefined -D_LIBCPP_HAS_THREAD_API_PTHREAD -Wno-macro-redefined -D_LIBCPP_ABI_VERSION=1 -fsyntax-only
1,610 mslibcxx CI C++03 > libcxx-trunk-shared.libcxx::min_max_macros.compile.pass.cpp
Script: -- : 'COMPILED WITH'; /usr/bin/clang++ /home/libcxx-builder/.buildkite-agent/builds/54dee0c6df63-1/llvm-project/libcxx-ci/libcxx/test/libcxx/min_max_macros.compile.pass.cpp --target=x86_64-unknown-linux-gnu -nostdinc++ -isystem /home/libcxx-builder/.buildkite-agent/builds/54dee0c6df63-1/llvm-project/libcxx-ci/build/generic-cxx03/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/54dee0c6df63-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 -fmodules-cache-path=/home/libcxx-builder/.buildkite-agent/builds/54dee0c6df63-1/llvm-project/libcxx-ci/build/generic-cxx03/projects/libcxx/libcxx/Output/min_max_macros.compile.pass.cpp.dir/t.tmp/ModuleCache -Wno-macro-redefined -D_LIBCPP_HAS_THREAD_API_PTHREAD -Wno-macro-redefined -D_LIBCPP_ABI_VERSION=1 -fsyntax-only
View Full Test Results (6,816 Failed)

Event Timeline

zoecarver created this revision.Jun 7 2021, 1:55 PM
zoecarver requested review of this revision.Jun 7 2021, 1:55 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 7 2021, 1:55 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
cjdb added a comment.Jun 7 2021, 2:26 PM

We're going to need to hold off on this patch until we determine what's breaking Fuchsia.

cjdb requested changes to this revision.Jun 7 2021, 5:26 PM
This revision now requires changes to proceed.Jun 7 2021, 5:26 PM
Quuxplusone requested changes to this revision.Jun 8 2021, 5:27 AM
Quuxplusone added inline comments.
libcxx/include/__algorithm/copy.h
83

_VSTD:: — but also I worry a little bit about the codegen here. As you've written it so far, ranges::copy could just be

return __in_out_result{__last, _VSTD::copy(__first, __last, __result)};

and save a lot of modifications. Which is obviously not C++20-compliant. So for C++20, we're going to have to write a new ranges::copy algorithm that returns the computed iterator, generated by incrementing __first, which ended up comparing equal to _Sentinel __last. That's a much different-looking algorithm from what we've got today for std::copy, and might not even be able to share any code with it.

I strongly strongly strongly ask that you continue working on this PR until it succeeds at implementing the C++20 ranges::copy semantics, and then see if you still think this is a good direction. I strongly strongly strongly oppose merging this PR in its current state.

libcxx/test/std/algorithms/robust_against_adl.pass.cpp
15

This is testing that your library code is robust against ADL.
I assume it's failing at the moment because you didn't ADL-proof your call to __copy above.
Remember, every time you call a function in namespace _VSTD that would be subject to ADL, you need to qualify it. (The one common exception we make is for declval, because it takes no arguments.)