Distribute std::tuple_(size|element) specializations to implementation files#9123
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughimportant: Remove centralized structured_bindings.h; distribute tuple_size/tuple_element specializations into core tuple trait headers and per-type headers, add std ↔ cuda::std interoperability, and update includes and tests. ChangesTuple Protocol Refactoring
suggested labels: suggested reviewers:
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
libcudacxx/include/cuda/std/__ranges/subrange.h (1)
496-504:⚠️ Potential issue | 🔴 Critical | ⚡ Quick wincritical: Same inheritance bug as the
cuda::stdversions—needstype_identitywrapper.template <::cuda::std::size_t _Idx, class _Ip, class _Sp, ::cuda::std::ranges::subrange_kind _Kp> struct tuple_element<_Idx, ::cuda::std::ranges::subrange<_Ip, _Sp, _Kp>> - : ::cuda::std::enable_if_t<(_Idx < 2), ::cuda::std::conditional_t<(_Idx == 0), _Ip, _Sp>> + : ::cuda::std::enable_if_t<(_Idx < 2), ::cuda::std::type_identity<::cuda::std::conditional_t<(_Idx == 0), _Ip, _Sp>>> {}; template <::cuda::std::size_t _Idx, class _Ip, class _Sp, ::cuda::std::ranges::subrange_kind _Kp> struct tuple_element<_Idx, const ::cuda::std::ranges::subrange<_Ip, _Sp, _Kp>> - : ::cuda::std::enable_if_t<(_Idx < 2), ::cuda::std::conditional_t<(_Idx == 0), _Ip, _Sp>> + : ::cuda::std::enable_if_t<(_Idx < 2), ::cuda::std::type_identity<::cuda::std::conditional_t<(_Idx == 0), _Ip, _Sp>>> {};
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 6eb35ad0-cca1-45fc-a35d-855ab6081716
📒 Files selected for processing (15)
cub/cub/detail/warpspeed/resource/smem_stage.cuhlibcudacxx/include/cuda/__complex/complex.hlibcudacxx/include/cuda/std/__complex/complex.hlibcudacxx/include/cuda/std/__complex/tuple.hlibcudacxx/include/cuda/std/__ranges/subrange.hlibcudacxx/include/cuda/std/__tuple_dir/structured_bindings.hlibcudacxx/include/cuda/std/__tuple_dir/tuple.hlibcudacxx/include/cuda/std/__tuple_dir/tuple_element.hlibcudacxx/include/cuda/std/__tuple_dir/tuple_size.hlibcudacxx/include/cuda/std/__tuple_dir/tuple_types.hlibcudacxx/include/cuda/std/__tuple_dir/vector_types.hlibcudacxx/include/cuda/std/__utility/pair.hlibcudacxx/include/cuda/std/arraylibcudacxx/include/cuda/std/tuplethrust/thrust/iterator/detail/tuple_of_iterator_references.h
💤 Files with no reviewable changes (4)
- libcudacxx/include/cuda/std/__tuple_dir/structured_bindings.h
- libcudacxx/include/cuda/std/__tuple_dir/vector_types.h
- libcudacxx/include/cuda/std/tuple
- libcudacxx/include/cuda/std/__complex/tuple.h
e29e5e9 to
41f3cca
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: af568c0e-a7ad-4440-aa0e-87981093659f
📒 Files selected for processing (18)
cub/cub/detail/warpspeed/resource/smem_stage.cuhlibcudacxx/include/cuda/__complex/complex.hlibcudacxx/include/cuda/std/__complex/complex.hlibcudacxx/include/cuda/std/__complex/tuple.hlibcudacxx/include/cuda/std/__ranges/subrange.hlibcudacxx/include/cuda/std/__tuple_dir/structured_bindings.hlibcudacxx/include/cuda/std/__tuple_dir/tuple.hlibcudacxx/include/cuda/std/__tuple_dir/tuple_element.hlibcudacxx/include/cuda/std/__tuple_dir/tuple_size.hlibcudacxx/include/cuda/std/__tuple_dir/tuple_types.hlibcudacxx/include/cuda/std/__tuple_dir/vector_types.hlibcudacxx/include/cuda/std/__utility/pair.hlibcudacxx/include/cuda/std/arraylibcudacxx/include/cuda/std/tuplelibcudacxx/test/libcudacxx/cuda/utilities/tuple/std_types_tuple_element.pass.cpplibcudacxx/test/libcudacxx/cuda/utilities/tuple/std_types_tuple_size.pass.cpplibcudacxx/test/libcudacxx/std/containers/sequences/array/array.tuple/tuple_element.fail.cppthrust/thrust/iterator/detail/tuple_of_iterator_references.h
💤 Files with no reviewable changes (4)
- libcudacxx/include/cuda/std/__tuple_dir/vector_types.h
- libcudacxx/include/cuda/std/tuple
- libcudacxx/include/cuda/std/__tuple_dir/structured_bindings.h
- libcudacxx/include/cuda/std/__complex/tuple.h
✅ Files skipped from review due to trivial changes (1)
- libcudacxx/test/libcudacxx/std/containers/sequences/array/array.tuple/tuple_element.fail.cpp
41f3cca to
91c17e9
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 5c25d0eb-118b-4488-be2d-4a6744c8b6b7
📒 Files selected for processing (18)
cub/cub/detail/warpspeed/resource/smem_stage.cuhlibcudacxx/include/cuda/__complex/complex.hlibcudacxx/include/cuda/std/__complex/complex.hlibcudacxx/include/cuda/std/__complex/tuple.hlibcudacxx/include/cuda/std/__ranges/subrange.hlibcudacxx/include/cuda/std/__tuple_dir/structured_bindings.hlibcudacxx/include/cuda/std/__tuple_dir/tuple.hlibcudacxx/include/cuda/std/__tuple_dir/tuple_element.hlibcudacxx/include/cuda/std/__tuple_dir/tuple_size.hlibcudacxx/include/cuda/std/__tuple_dir/tuple_types.hlibcudacxx/include/cuda/std/__tuple_dir/vector_types.hlibcudacxx/include/cuda/std/__utility/pair.hlibcudacxx/include/cuda/std/arraylibcudacxx/include/cuda/std/tuplelibcudacxx/test/libcudacxx/cuda/utilities/tuple/std_types_tuple_element.pass.cpplibcudacxx/test/libcudacxx/cuda/utilities/tuple/std_types_tuple_size.pass.cpplibcudacxx/test/libcudacxx/std/containers/sequences/array/array.tuple/tuple_element.fail.cppthrust/thrust/iterator/detail/tuple_of_iterator_references.h
💤 Files with no reviewable changes (4)
- libcudacxx/include/cuda/std/tuple
- libcudacxx/include/cuda/std/__tuple_dir/structured_bindings.h
- libcudacxx/include/cuda/std/__tuple_dir/vector_types.h
- libcudacxx/include/cuda/std/__complex/tuple.h
✅ Files skipped from review due to trivial changes (1)
- cub/cub/detail/warpspeed/resource/smem_stage.cuh
91c17e9 to
862ae56
Compare
This comment has been minimized.
This comment has been minimized.
🥳 CI Workflow Results🟩 Finished in 3h 10m: Pass: 100%/398 | Total: 19d 22h | Max: 3h 09m | Hits: 24%/4225373See results here. |
miscco
left a comment
There was a problem hiding this comment.
Looks good, I believe with the added forward declarations this is now the better approach
I want to implement tuple protocol for
integer_sequence, but there is a non-trivial circular dependency, because we specialize all relevantstd::traits in one file, which includesmake_index_sequence.So, I decided to decentralize the specializations and move them to the places where we specialize the
cuda::std::traits.Note that we already specialize for example
std::formatterinside the implementation files without any problems.