Diff Detail
- Repository
- rG LLVM Github Monorepo
Time | Test | |
---|---|---|
4,200 ms | libcxx CI C++2b > llvm-libc++-shared-cfg-in.libcxx/ranges/range_factories/range_repeat_view::ctor.value.bound.verify.cpp Script:
--
: 'COMPILED WITH'; /usr/bin/clang++-17 /home/libcxx-builder/.buildkite-agent/builds/50a70c54d0fa-1/llvm-project/libcxx-ci/libcxx/test/libcxx/ranges/range.factories/range.repeat.view/ctor.value.bound.verify.cpp --target=x86_64-unknown-linux-gnu -nostdinc++ -I /home/libcxx-builder/.buildkite-agent/builds/50a70c54d0fa-1/llvm-project/libcxx-ci/build/generic-cxx2b/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/50a70c54d0fa-1/llvm-project/libcxx-ci/build/generic-cxx2b/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/50a70c54d0fa-1/llvm-project/libcxx-ci/libcxx/test/support -std=c++2b -Werror -Wall -Wctad-maybe-unsupported -Wextra -Wshadow -Wundef -Wunused-template -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 -D_LIBCPP_ENABLE_ASSERTIONS=1 -fsyntax-only -Wno-error -Xclang -verify -Xclang -verify-ignore-unexpected=note -ferror-limit=0
| |
12,850 ms | libcxx CI Modular build > llvm-libc++-shared-cfg-in.libcxx/ranges/range_factories/range_repeat_view::ctor.value.bound.verify.cpp Script:
--
: 'COMPILED WITH'; /usr/bin/clang++-17 /home/libcxx-builder/.buildkite-agent/builds/c21d5090d5ad-1/llvm-project/libcxx-ci/libcxx/test/libcxx/ranges/range.factories/range.repeat.view/ctor.value.bound.verify.cpp --target=x86_64-unknown-linux-gnu -nostdinc++ -I /home/libcxx-builder/.buildkite-agent/builds/c21d5090d5ad-1/llvm-project/libcxx-ci/build/generic-modules/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/c21d5090d5ad-1/llvm-project/libcxx-ci/build/generic-modules/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/c21d5090d5ad-1/llvm-project/libcxx-ci/libcxx/test/support -std=c++2b -fmodules -fcxx-modules -Werror -Wall -Wctad-maybe-unsupported -Wextra -Wshadow -Wundef -Wunused-template -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 -D_LIBCPP_ENABLE_ASSERTIONS=1 -fsyntax-only -Wno-error -Xclang -verify -Xclang -verify-ignore-unexpected=note -ferror-limit=0
| |
580 ms | libcxx CI Modular build > llvm-libc++-shared-cfg-in.std/ranges/range_factories/range_repeat_view::ctad.compile.pass.cpp Script:
--
: 'COMPILED WITH'; /usr/bin/clang++-17 /home/libcxx-builder/.buildkite-agent/builds/c21d5090d5ad-1/llvm-project/libcxx-ci/libcxx/test/std/ranges/range.factories/range.repeat.view/ctad.compile.pass.cpp --target=x86_64-unknown-linux-gnu -nostdinc++ -I /home/libcxx-builder/.buildkite-agent/builds/c21d5090d5ad-1/llvm-project/libcxx-ci/build/generic-modules/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/c21d5090d5ad-1/llvm-project/libcxx-ci/build/generic-modules/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/c21d5090d5ad-1/llvm-project/libcxx-ci/libcxx/test/support -std=c++2b -fmodules -fcxx-modules -Werror -Wall -Wctad-maybe-unsupported -Wextra -Wshadow -Wundef -Wunused-template -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
| |
720 ms | libcxx CI Modular build > llvm-libc++-shared-cfg-in.std/ranges/range_factories/range_repeat_view::ctor.value.bound.pass.cpp Script:
--
: 'COMPILED WITH'; /usr/bin/clang++-17 /home/libcxx-builder/.buildkite-agent/builds/c21d5090d5ad-1/llvm-project/libcxx-ci/libcxx/test/std/ranges/range.factories/range.repeat.view/ctor.value.bound.pass.cpp --target=x86_64-unknown-linux-gnu -nostdinc++ -I /home/libcxx-builder/.buildkite-agent/builds/c21d5090d5ad-1/llvm-project/libcxx-ci/build/generic-modules/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/c21d5090d5ad-1/llvm-project/libcxx-ci/build/generic-modules/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/c21d5090d5ad-1/llvm-project/libcxx-ci/libcxx/test/support -std=c++2b -fmodules -fcxx-modules -Werror -Wall -Wctad-maybe-unsupported -Wextra -Wshadow -Wundef -Wunused-template -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/c21d5090d5ad-1/llvm-project/libcxx-ci/build/generic-modules/lib -Wl,-rpath,/home/libcxx-builder/.buildkite-agent/builds/c21d5090d5ad-1/llvm-project/libcxx-ci/build/generic-modules/lib -lc++ -pthread -o /home/libcxx-builder/.buildkite-agent/builds/c21d5090d5ad-1/llvm-project/libcxx-ci/build/generic-modules/test/std/ranges/range.factories/range.repeat.view/Output/ctor.value.bound.pass.cpp.dir/t.tmp.exe
| |
680 ms | libcxx CI Modular build > llvm-libc++-shared-cfg-in.std/ranges/range_factories/range_repeat_view::views_repeat.pass.cpp Script:
--
: 'COMPILED WITH'; /usr/bin/clang++-17 /home/libcxx-builder/.buildkite-agent/builds/c21d5090d5ad-1/llvm-project/libcxx-ci/libcxx/test/std/ranges/range.factories/range.repeat.view/views_repeat.pass.cpp --target=x86_64-unknown-linux-gnu -nostdinc++ -I /home/libcxx-builder/.buildkite-agent/builds/c21d5090d5ad-1/llvm-project/libcxx-ci/build/generic-modules/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/c21d5090d5ad-1/llvm-project/libcxx-ci/build/generic-modules/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/c21d5090d5ad-1/llvm-project/libcxx-ci/libcxx/test/support -std=c++2b -fmodules -fcxx-modules -Werror -Wall -Wctad-maybe-unsupported -Wextra -Wshadow -Wundef -Wunused-template -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/c21d5090d5ad-1/llvm-project/libcxx-ci/build/generic-modules/lib -Wl,-rpath,/home/libcxx-builder/.buildkite-agent/builds/c21d5090d5ad-1/llvm-project/libcxx-ci/build/generic-modules/lib -lc++ -pthread -o /home/libcxx-builder/.buildkite-agent/builds/c21d5090d5ad-1/llvm-project/libcxx-ci/build/generic-modules/test/std/ranges/range.factories/range.repeat.view/Output/views_repeat.pass.cpp.dir/t.tmp.exe
| |
View Full Test Results (12 Failed) |
Event Timeline
libcxx/include/__ranges/drop_view.h | ||
---|---|---|
281 | ||
libcxx/include/__ranges/repeat_view.h | ||
53 | Could you add a // TODO LLVM18: remove this workaround to make it easier to find? | |
101 | ||
107–108 | These aren't subject to ADL, so we don't have to qualify them. | |
111 | I think we should instead add new constructors to __copyable_box, so we can construct the type in-place. For example (not tested): template <class... _Args, size_t... _Indices> requires is_constructible_v<_Tp, _Args...> _LIBCPP_HIDE_FROM_ABI constexpr explicit __copyable_box( piecewise_construct_t, tuple<_Args...> __tuple, index_sequence<_Indices...>) : __copyable_box(in_place, std::get<_Indices>(std::forward<tuple<_Args...>>(__tuple))...) {} template <class... _Args> requires is_constructible_v<_Tp, _Args...> _LIBCPP_HIDE_FROM_ABI constexpr explicit __copyable_box(piecewise_construct_t, tuple<_Args...> __args) : __copyable_box(std::move(__args), std::make_index_sequence<sizeof...(_Args)>()) {} Then we can simply do __value_(piecewise_construct, std::move(__value_args)). | |
112 | Why not simply use std::make_from_tuple? | |
131 | ||
139 | Please add a regression test! | |
145 | ||
147 | ||
157 | ||
236–240 | IMO we should just inline the requires clauses. | |
268 | ||
libcxx/include/__ranges/take_view.h | ||
34 | Let's just forward-delcare repeat_view instead. | |
223–229 | Let's move this to __fwd/repeat_view.h, so we don't define this multiple times in the code base. | |
312 | ||
libcxx/test/std/ranges/range.factories/range.repeat.view/begin.pass.cpp | ||
11–12 | Both of these are already removed from the code base. Please make sure they are removed from all the new tests. | |
21 | Let's also add tests when we don't have unreachable_sentinel. | |
libcxx/test/std/ranges/range.factories/range.repeat.view/ctad.compile.pass.cpp | ||
22 | Same below. | |
40–44 | You can just put the static_asserts above at the global scope and remove this boilerplate. | |
libcxx/test/std/ranges/range.factories/range.repeat.view/ctor.default.pass.cpp | ||
17 | Please add a negative test for the constraint. | |
23 | This doesn't test the default constructor. | |
libcxx/test/std/ranges/range.factories/range.repeat.view/ctor.piecewise.pass.cpp | ||
22 | Please also add tests for the constraints. | |
23–26 | Let's add a constructor that only takes very specific arguments to make sure we actually std::forward the types. | |
34 | Let's also test that you can explicitly construct the unrechable_sentinel from a tuple{}. | |
libcxx/test/std/ranges/range.factories/range.repeat.view/ctor.value.bound.pass.cpp | ||
18 | Please add tests for the explicits and constraints. | |
20 | Please also add tests to make sure the _LIBCPP_ASSERTs actually work in libcxx/test/libcxx. | |
48 | To make sure it doesn't just take mutable references. | |
libcxx/test/std/ranges/range.factories/range.repeat.view/end.pass.cpp | ||
19 | Let's also add a test to make sure end() is noexcept for an unreachable_sentinel. | |
libcxx/test/std/ranges/range.factories/range.repeat.view/iterator/compare.pass.cpp | ||
23 | Let's not use CTAD here. | |
29 | Let's also check the return types. | |
libcxx/test/std/ranges/range.factories/range.repeat.view/iterator/ctor.default.pass.cpp | ||
15 | Please also make sure that repeat_view::iterator is_nothrow_default_constructible. | |
libcxx/test/std/ranges/range.factories/range.repeat.view/iterator/increment.pass.cpp | ||
31–34 | Let's just test for the specific type instead. | |
34 | Missing <concepts> include | |
libcxx/test/std/ranges/range.factories/range.repeat.view/iterator/member_typedefs.compile.pass.cpp | ||
24 | Let's also test with a user-defined type (especially a move-only type). | |
28–29 | ||
34 | This doesn't hurt, but also doesn't make a lot of sense if we test for a specific type. | |
58–62 | Let's just check sizeof(Iter::difference_type)) == sizeof(unsigned) to be more platform-agnostic. | |
libcxx/test/std/ranges/range.factories/range.repeat.view/iterator/plus.pass.cpp | ||
25 | This makes it a bit easier to verify at a glance. | |
31–32 | Let's check the exact type instead. | |
libcxx/test/std/ranges/range.factories/range.repeat.view/iterator/plus_eq.pass.cpp | ||
27 | Let's check the return type exactly, and make sure it's the same object as iter2 (i.e. compare the address). | |
libcxx/test/std/ranges/range.factories/range.repeat.view/iterator/star.pass.cpp | ||
22 | The returned type should always point to the same object IIUC. Let's test that too. | |
25 | This seems redundant given the static_assert below. | |
libcxx/test/std/ranges/range.factories/range.repeat.view/views_repeat.pass.cpp | ||
17 | Please add negative tests for the constraints of repeat_view. |
Thanks a lot for your review @philnik , and sorry for the late reply, I have address most comments, and improve test cases.
libcxx/include/__ranges/repeat_view.h | ||
---|---|---|
112 | __repeat_view_make_from_tuple still is a workaround for older version clang (https://godbolt.org/z/dreT87G4f). as far as I know, std::make_from_tuple initializing aggregates from a parenthesized list of values( https://github.com/llvm/llvm-project/blob/c0b4ca107a3b605f810bd60642907e6a77f7c6d3/libcxx/include/tuple#L1835 ), but this code requires P0960 | |
139 |
I think here is copy_constructible, not move_constructible ? template<copy_constructible W, semiregular Bound = unreachable_sentinel_t> requires is-integer-like<Bound> || same_as<Bound, unreachable_sentinel_t> class repeat_view<W, Bound>::iterator { private: ...... }; https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2022/p2474r2.html#wording-range.repeat.iterator |
Fix CI.
libcxx/include/__ranges/repeat_view.h | ||
---|---|---|
111 | I have tried to add new ctor to __copyable_box, but is_constructible_v<_Tp, _Args...> still need a workaround, so, can we add this new ctor and remove __repeat_view_constructible_from in LLVM 18? WDYH? |
libcxx/include/__ranges/repeat_view.h | ||
---|---|---|
112 | Looking at your example, this doesn't seem like something that's specific to repeat_view. I'd just remove the workarounds and XFAIL the tests that don't work. The extra amount of code to make this work really isn't worth the LLVM15 support, which almost nobody uses anyways. |
libcxx/include/__ranges/repeat_view.h | ||
---|---|---|
112 | Thanks, I will try to remove this workaround. |
The transitive_includes test case always fails locally on my laptop (ninja check-cxx)
*************** *** 162,171 **** --- 162,186 ---- expectedinitializer_list expectednew expectedversion + experimental/algorithmalgorithm + experimental/algorithmcstddef + experimental/algorithmtype_traits + experimental/coroutinecstddef + experimental/coroutinecstdint + experimental/coroutinecstring + experimental/coroutinelimits + experimental/coroutinenew + experimental/coroutinetype_traits experimental/dequedeque experimental/dequeexperimental/memory_resource experimental/forward_listexperimental/memory_resource experimental/forward_listforward_list + experimental/functionalarray + experimental/functionalcstddef + experimental/functionalfunctional + experimental/functionaltype_traits + experimental/functionalunordered_map + experimental/functionalvector experimental/iteratorcstddef experimental/iteratoriosfwd experimental/iteratoriterator error: command failed with exit status: 1
my host compiler is
Apple clang version 14.0.0 (clang-1400.0.29.202) Target: arm64-apple-darwin21.6.0 Thread model: posix InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin
You probably still have experimental/{algorithm, coroutine, functional} in your local install dir.
Remove fwd/repeat_view.h, and move is_repeat_specialization into __ranges/repeat_view.h
libcxx/docs/Status/Cxx2bPapers.csv | ||
---|---|---|
80 | Please add a release note. | |
libcxx/include/__ranges/repeat_view.h | ||
111 | This shouldn't be a problem anymore, since the workaround is removed, right? | |
112 | The XFAIL looks correct. The CI is currently failing earlier, so I can't see what the problem is exactly. Could you ping me again if you run into this again? | |
139 | The working draft uses move_constructible: http://eel.is/c++draft/range.repeat#iterator. Could you look into when this changed and apply the changes to your patch? | |
libcxx/include/__ranges/take_view.h | ||
34 | It seems you haven't don this? | |
libcxx/test/libcxx/ranges/range.factories/range.repeat.view/ctor.value.bound.verify.cpp | ||
20 | Why do you have an extra scope around this? | |
libcxx/test/std/ranges/range.factories/range.repeat.view/ctad.compile.pass.cpp | ||
29 | You don't need a main in a .compile.pass.cpp test. | |
libcxx/test/std/ranges/range.factories/range.repeat.view/ctor.value.bound.pass.cpp | ||
18 | You seem to be missing tests for the copy_constructible constraint. | |
libcxx/test/std/ranges/range.factories/range.repeat.view/iterator/increment.pass.cpp | ||
32–33 | This seems redundant, since you already check for the specific type below. | |
libcxx/test/std/ranges/range.factories/range.repeat.view/iterator/member_typedefs.compile.pass.cpp | ||
58–62 | This seems to not be done. | |
libcxx/utils/generate_feature_test_macro_components.py | ||
569 | Is something missing from the Paper? You claim it's complete in the status paper. |