This simplifies the usage of __less by making the class not depend on the types compared, but instead the operator(). We can't remove the template completely because we explicitly instantiate std::__sort with __less<T>.
Diff Detail
- Repository
- rG LLVM Github Monorepo
Time | Test | |
---|---|---|
1,210 ms | libcxx CI C++03 > llvm-libc++-shared-cfg-in.libcxx/algorithms::robust_against_cpp20_hostile_iterators.compile.pass.cpp Script:
--
: 'COMPILED WITH'; /usr/bin/clang++-17 /home/libcxx-builder/.buildkite-agent/builds/ff401cdcac7e-1/llvm-project/libcxx-ci/libcxx/test/libcxx/algorithms/robust_against_cpp20_hostile_iterators.compile.pass.cpp --target=x86_64-unknown-linux-gnu -nostdinc++ -I /home/libcxx-builder/.buildkite-agent/builds/ff401cdcac7e-1/llvm-project/libcxx-ci/build/generic-cxx03/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/ff401cdcac7e-1/llvm-project/libcxx-ci/build/generic-cxx03/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/ff401cdcac7e-1/llvm-project/libcxx-ci/libcxx/test/support -std=c++03 -Werror -Wall -Wctad-maybe-unsupported -Wextra -Wshadow -Wundef -Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move -Wno-c++11-extensions -Wno-noexcept-type -Wno-atomic-alignment -Wno-user-defined-literals -Wno-tautological-compare -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-unused-local-typedef -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -D_LIBCPP_ENABLE_EXPERIMENTAL -D_LIBCPP_DISABLE_AVAILABILITY -Werror=thread-safety -Wuser-defined-warnings -Wno-ambiguous-reversed-operator -fsyntax-only
| |
1,730 ms | libcxx CI C++03 > llvm-libc++-shared-cfg-in.std/algorithms::robust_re_difference_type.compile.pass.cpp Script:
--
: 'COMPILED WITH'; /usr/bin/clang++-17 /home/libcxx-builder/.buildkite-agent/builds/ff401cdcac7e-1/llvm-project/libcxx-ci/libcxx/test/std/algorithms/robust_re_difference_type.compile.pass.cpp --target=x86_64-unknown-linux-gnu -nostdinc++ -I /home/libcxx-builder/.buildkite-agent/builds/ff401cdcac7e-1/llvm-project/libcxx-ci/build/generic-cxx03/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/ff401cdcac7e-1/llvm-project/libcxx-ci/build/generic-cxx03/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/ff401cdcac7e-1/llvm-project/libcxx-ci/libcxx/test/support -std=c++03 -Werror -Wall -Wctad-maybe-unsupported -Wextra -Wshadow -Wundef -Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move -Wno-c++11-extensions -Wno-noexcept-type -Wno-atomic-alignment -Wno-user-defined-literals -Wno-tautological-compare -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-unused-local-typedef -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -D_LIBCPP_ENABLE_EXPERIMENTAL -D_LIBCPP_DISABLE_AVAILABILITY -Werror=thread-safety -Wuser-defined-warnings -fsyntax-only
| |
1,170 ms | libcxx CI C++03 > llvm-libc++-shared-cfg-in.std/algorithms/alg_sorting/alg_binary_search/binary_search::binary_search.pass.cpp Script:
--
: 'COMPILED WITH'; /usr/bin/clang++-17 /home/libcxx-builder/.buildkite-agent/builds/ff401cdcac7e-1/llvm-project/libcxx-ci/libcxx/test/std/algorithms/alg.sorting/alg.binary.search/binary.search/binary_search.pass.cpp --target=x86_64-unknown-linux-gnu -nostdinc++ -I /home/libcxx-builder/.buildkite-agent/builds/ff401cdcac7e-1/llvm-project/libcxx-ci/build/generic-cxx03/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/ff401cdcac7e-1/llvm-project/libcxx-ci/build/generic-cxx03/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/ff401cdcac7e-1/llvm-project/libcxx-ci/libcxx/test/support -std=c++03 -Werror -Wall -Wctad-maybe-unsupported -Wextra -Wshadow -Wundef -Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move -Wno-c++11-extensions -Wno-noexcept-type -Wno-atomic-alignment -Wno-user-defined-literals -Wno-tautological-compare -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-unused-local-typedef -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -D_LIBCPP_ENABLE_EXPERIMENTAL -D_LIBCPP_DISABLE_AVAILABILITY -Werror=thread-safety -Wuser-defined-warnings -lc++experimental -nostdlib++ -L /home/libcxx-builder/.buildkite-agent/builds/ff401cdcac7e-1/llvm-project/libcxx-ci/build/generic-cxx03/lib -Wl,-rpath,/home/libcxx-builder/.buildkite-agent/builds/ff401cdcac7e-1/llvm-project/libcxx-ci/build/generic-cxx03/lib -lc++ -pthread -o /home/libcxx-builder/.buildkite-agent/builds/ff401cdcac7e-1/llvm-project/libcxx-ci/build/generic-cxx03/test/std/algorithms/alg.sorting/alg.binary.search/binary.search/Output/binary_search.pass.cpp.dir/t.tmp.exe
| |
1,220 ms | libcxx CI C++03 > llvm-libc++-shared-cfg-in.std/algorithms/alg_sorting/alg_binary_search/equal_range::equal_range.pass.cpp Script:
--
: 'COMPILED WITH'; /usr/bin/clang++-17 /home/libcxx-builder/.buildkite-agent/builds/ff401cdcac7e-1/llvm-project/libcxx-ci/libcxx/test/std/algorithms/alg.sorting/alg.binary.search/equal.range/equal_range.pass.cpp --target=x86_64-unknown-linux-gnu -nostdinc++ -I /home/libcxx-builder/.buildkite-agent/builds/ff401cdcac7e-1/llvm-project/libcxx-ci/build/generic-cxx03/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/ff401cdcac7e-1/llvm-project/libcxx-ci/build/generic-cxx03/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/ff401cdcac7e-1/llvm-project/libcxx-ci/libcxx/test/support -std=c++03 -Werror -Wall -Wctad-maybe-unsupported -Wextra -Wshadow -Wundef -Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move -Wno-c++11-extensions -Wno-noexcept-type -Wno-atomic-alignment -Wno-user-defined-literals -Wno-tautological-compare -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-unused-local-typedef -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -D_LIBCPP_ENABLE_EXPERIMENTAL -D_LIBCPP_DISABLE_AVAILABILITY -Werror=thread-safety -Wuser-defined-warnings -lc++experimental -nostdlib++ -L /home/libcxx-builder/.buildkite-agent/builds/ff401cdcac7e-1/llvm-project/libcxx-ci/build/generic-cxx03/lib -Wl,-rpath,/home/libcxx-builder/.buildkite-agent/builds/ff401cdcac7e-1/llvm-project/libcxx-ci/build/generic-cxx03/lib -lc++ -pthread -o /home/libcxx-builder/.buildkite-agent/builds/ff401cdcac7e-1/llvm-project/libcxx-ci/build/generic-cxx03/test/std/algorithms/alg.sorting/alg.binary.search/equal.range/Output/equal_range.pass.cpp.dir/t.tmp.exe
| |
1,450 ms | libcxx CI C++03 > llvm-libc++-shared-cfg-in.std/algorithms/alg_sorting/alg_binary_search/lower_bound::lower_bound.pass.cpp Script:
--
: 'COMPILED WITH'; /usr/bin/clang++-17 /home/libcxx-builder/.buildkite-agent/builds/ff401cdcac7e-1/llvm-project/libcxx-ci/libcxx/test/std/algorithms/alg.sorting/alg.binary.search/lower.bound/lower_bound.pass.cpp --target=x86_64-unknown-linux-gnu -nostdinc++ -I /home/libcxx-builder/.buildkite-agent/builds/ff401cdcac7e-1/llvm-project/libcxx-ci/build/generic-cxx03/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/ff401cdcac7e-1/llvm-project/libcxx-ci/build/generic-cxx03/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/ff401cdcac7e-1/llvm-project/libcxx-ci/libcxx/test/support -std=c++03 -Werror -Wall -Wctad-maybe-unsupported -Wextra -Wshadow -Wundef -Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move -Wno-c++11-extensions -Wno-noexcept-type -Wno-atomic-alignment -Wno-user-defined-literals -Wno-tautological-compare -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-unused-local-typedef -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -D_LIBCPP_ENABLE_EXPERIMENTAL -D_LIBCPP_DISABLE_AVAILABILITY -Werror=thread-safety -Wuser-defined-warnings -lc++experimental -nostdlib++ -L /home/libcxx-builder/.buildkite-agent/builds/ff401cdcac7e-1/llvm-project/libcxx-ci/build/generic-cxx03/lib -Wl,-rpath,/home/libcxx-builder/.buildkite-agent/builds/ff401cdcac7e-1/llvm-project/libcxx-ci/build/generic-cxx03/lib -lc++ -pthread -o /home/libcxx-builder/.buildkite-agent/builds/ff401cdcac7e-1/llvm-project/libcxx-ci/build/generic-cxx03/test/std/algorithms/alg.sorting/alg.binary.search/lower.bound/Output/lower_bound.pass.cpp.dir/t.tmp.exe
| |
View Full Test Results (127 Failed) |
Event Timeline
This cleanup seems incomplete. Why keep __less as a template at all given it's never actually used as one?
Also does the iterator's ::value_type always have to exactly match the type returned from dereferencing it?
libcxx/include/__algorithm/comp.h | ||
---|---|---|
37 | This doesn't at all seem correct to me. This takes away an explicit conversion | |
libcxx/include/__algorithm/includes.h | ||
64 | Is this correct? What if the iterator returns a type convertible to value_type? |
libcxx/include/__algorithm/comp.h | ||
---|---|---|
37 | *The implicit conversion. |
If you block other reviews on there not being a description, at least read it.
libcxx/include/__algorithm/includes.h | ||
---|---|---|
64 | AFAICT the standard it either very silent on this, or says one should use less{} as the comparator, which does what __less<>{} does after this change. So this change is either a conforming change, or arguably a bugfix. Although funnily enough std::less{} isn't actually well-formed, and it doesn't specifically use ranges::less{} for the classic algorithms. |
libcxx/include/__algorithm/comp.h | ||
---|---|---|
32 | So there's actually a behavior change in this patch (the removal of implicit conversions noticed by @EricWF). However, I think it is desirable -- right now I don't think we are conforming in C++20. For example, take lower_bound: http://eel.is/c++draft/lower.bound#1. It specifically says Let comp be less{} and proj be identity{} for overloads with no parameters by those names., which implies that we should be using the transparent comparator, but we are not (before this patch). I think this mandates a change everywhere along with tests. We should also confirm what's the expected behavior in prior Standards and make sure we conform in those as well. | |
33 | Please add a comment explaining why it's empty. |