[libcu++] Implement tuple-like constructors for tuple#9059
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
5a777c9 to
fc262c0
Compare
|
/ok to test |
305b7ec to
c2fb71f
Compare
|
/ok to test |
This comment has been minimized.
This comment has been minimized.
c2fb71f to
93d612c
Compare
|
/ok to test |
|
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:
📝 WalkthroughWalkthroughRefactors CUDA libcudacxx tuple construction by introducing consteval-based constraint checkers and rebasing tuple-like trait detection, updates tuple/leaf/tuple_cat implementations, adds forward-declaration type traits, expands test coverage with pair/tuple conversion tests, and cleans up test directives. ChangesTuple construction and constraint machinery
Possibly related PRs
Suggested labels
Suggested reviewers
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 18663b2f-23bd-4106-9902-09d7a6c620d4
📒 Files selected for processing (16)
libcudacxx/include/cuda/std/__fwd/pair.hlibcudacxx/include/cuda/std/__fwd/subrange.hlibcudacxx/include/cuda/std/__fwd/tuple.hlibcudacxx/include/cuda/std/__tuple_dir/tuple.hlibcudacxx/include/cuda/std/__tuple_dir/tuple_cat.hlibcudacxx/include/cuda/std/__tuple_dir/tuple_constraints.hlibcudacxx/include/cuda/std/__tuple_dir/tuple_leaf.hlibcudacxx/include/cuda/std/__tuple_dir/tuple_like.hlibcudacxx/include/cuda/std/__tuple_dir/tuple_like_ext.hlibcudacxx/test/libcudacxx/std/utilities/tuple/tuple.tuple/tuple.cnstr/PR31384.pass.cpplibcudacxx/test/libcudacxx/std/utilities/tuple/tuple.tuple/tuple.cnstr/const_move_pair.pass.cpplibcudacxx/test/libcudacxx/std/utilities/tuple/tuple.tuple/tuple.cnstr/convert_const_move.pass.cpplibcudacxx/test/libcudacxx/std/utilities/tuple/tuple.tuple/tuple.cnstr/convert_non_const_copy.pass.cpplibcudacxx/test/libcudacxx/std/utilities/tuple/tuple.tuple/tuple.cnstr/non_const_pair.pass.cpplibcudacxx/test/libcudacxx/std/utilities/tuple/tuple.tuple/tuple.traits/trivially_copyable.pass.cpplibcudacxx/test/support/copy_move_types.h
💤 Files with no reviewable changes (1)
- libcudacxx/include/cuda/std/__tuple_dir/tuple_like_ext.h
93d612c to
f46bbc9
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
libcudacxx/include/cuda/std/__fwd/pair.h (1)
43-47: 💤 Low valuesuggestion: Consider renaming to
__is_cuda_std_pair_vfor consistency with__is_tuple_of_iterator_references_v(line 40 of__fwd/tuple.h). The codebase has mixed patterns (__tuple_like_extlacks_v, but iterator-references trait has it); standardizing on_vfor boolean variable-template detection traits would improve uniformity.libcudacxx/include/cuda/std/__fwd/tuple.h (1)
42-46: 💤 Low valuesuggestion: Consider renaming to
__is_cuda_std_tuple_vfor consistency with__is_tuple_of_iterator_references_vat line 40 in this same file. Both are boolean variable-template detection traits; matching naming conventions within the same header improves readability.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 31a7c568-a91d-424a-942f-65730dfe1952
📒 Files selected for processing (16)
libcudacxx/include/cuda/std/__fwd/pair.hlibcudacxx/include/cuda/std/__fwd/subrange.hlibcudacxx/include/cuda/std/__fwd/tuple.hlibcudacxx/include/cuda/std/__tuple_dir/tuple.hlibcudacxx/include/cuda/std/__tuple_dir/tuple_cat.hlibcudacxx/include/cuda/std/__tuple_dir/tuple_constraints.hlibcudacxx/include/cuda/std/__tuple_dir/tuple_leaf.hlibcudacxx/include/cuda/std/__tuple_dir/tuple_like.hlibcudacxx/include/cuda/std/__tuple_dir/tuple_like_ext.hlibcudacxx/test/libcudacxx/std/utilities/tuple/tuple.tuple/tuple.cnstr/PR31384.pass.cpplibcudacxx/test/libcudacxx/std/utilities/tuple/tuple.tuple/tuple.cnstr/const_move_pair.pass.cpplibcudacxx/test/libcudacxx/std/utilities/tuple/tuple.tuple/tuple.cnstr/convert_const_move.pass.cpplibcudacxx/test/libcudacxx/std/utilities/tuple/tuple.tuple/tuple.cnstr/convert_non_const_copy.pass.cpplibcudacxx/test/libcudacxx/std/utilities/tuple/tuple.tuple/tuple.cnstr/non_const_pair.pass.cpplibcudacxx/test/libcudacxx/std/utilities/tuple/tuple.tuple/tuple.traits/trivially_copyable.pass.cpplibcudacxx/test/support/copy_move_types.h
💤 Files with no reviewable changes (1)
- libcudacxx/include/cuda/std/__tuple_dir/tuple_like_ext.h
f46bbc9 to
21aa1e3
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 5671e078-4c87-47a1-9dcd-94832367d4ec
📒 Files selected for processing (16)
libcudacxx/include/cuda/std/__fwd/pair.hlibcudacxx/include/cuda/std/__fwd/subrange.hlibcudacxx/include/cuda/std/__fwd/tuple.hlibcudacxx/include/cuda/std/__tuple_dir/tuple.hlibcudacxx/include/cuda/std/__tuple_dir/tuple_cat.hlibcudacxx/include/cuda/std/__tuple_dir/tuple_constraints.hlibcudacxx/include/cuda/std/__tuple_dir/tuple_leaf.hlibcudacxx/include/cuda/std/__tuple_dir/tuple_like.hlibcudacxx/include/cuda/std/__tuple_dir/tuple_like_ext.hlibcudacxx/test/libcudacxx/std/utilities/tuple/tuple.tuple/tuple.cnstr/PR31384.pass.cpplibcudacxx/test/libcudacxx/std/utilities/tuple/tuple.tuple/tuple.cnstr/const_move_pair.pass.cpplibcudacxx/test/libcudacxx/std/utilities/tuple/tuple.tuple/tuple.cnstr/convert_const_move.pass.cpplibcudacxx/test/libcudacxx/std/utilities/tuple/tuple.tuple/tuple.cnstr/convert_non_const_copy.pass.cpplibcudacxx/test/libcudacxx/std/utilities/tuple/tuple.tuple/tuple.cnstr/non_const_pair.pass.cpplibcudacxx/test/libcudacxx/std/utilities/tuple/tuple.tuple/tuple.traits/trivially_copyable.pass.cpplibcudacxx/test/support/copy_move_types.h
💤 Files with no reviewable changes (1)
- libcudacxx/include/cuda/std/__tuple_dir/tuple_like_ext.h
✅ Files skipped from review due to trivial changes (1)
- libcudacxx/test/libcudacxx/std/utilities/tuple/tuple.tuple/tuple.cnstr/PR31384.pass.cpp
This comment has been minimized.
This comment has been minimized.
ce8a059 to
f423ab2
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
libcudacxx/test/libcudacxx/std/utilities/tuple/tuple.tuple/tuple.cnstr/convert_const_move.pass.cpp (1)
116-131: ⚡ Quick winsuggestion: Lines 116-131 disable these constraint cases for every compiler via
#if 0; gate them only for the compiler/version that actually segfaults so other toolchains keep this coverage.libcudacxx/include/cuda/std/__fwd/subrange.h (1)
56-56: 💤 Low valuesuggestion: Naming inconsistency with other detection traits.
__is_cuda_std_ranges_subrange_vhas a_vsuffix, but__is_cuda_std_pairand__is_cuda_std_tupledo not. For internal consistency, either add_vto all three or remove it from this one.Also applies to: 59-59
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: aff077ac-4ceb-498f-9dbe-7ce4a83c0012
📒 Files selected for processing (16)
libcudacxx/include/cuda/std/__fwd/pair.hlibcudacxx/include/cuda/std/__fwd/subrange.hlibcudacxx/include/cuda/std/__fwd/tuple.hlibcudacxx/include/cuda/std/__tuple_dir/tuple.hlibcudacxx/include/cuda/std/__tuple_dir/tuple_cat.hlibcudacxx/include/cuda/std/__tuple_dir/tuple_constraints.hlibcudacxx/include/cuda/std/__tuple_dir/tuple_leaf.hlibcudacxx/include/cuda/std/__tuple_dir/tuple_like.hlibcudacxx/include/cuda/std/__tuple_dir/tuple_like_ext.hlibcudacxx/test/libcudacxx/std/utilities/tuple/tuple.tuple/tuple.cnstr/PR31384.pass.cpplibcudacxx/test/libcudacxx/std/utilities/tuple/tuple.tuple/tuple.cnstr/const_move_pair.pass.cpplibcudacxx/test/libcudacxx/std/utilities/tuple/tuple.tuple/tuple.cnstr/convert_const_move.pass.cpplibcudacxx/test/libcudacxx/std/utilities/tuple/tuple.tuple/tuple.cnstr/convert_non_const_copy.pass.cpplibcudacxx/test/libcudacxx/std/utilities/tuple/tuple.tuple/tuple.cnstr/non_const_pair.pass.cpplibcudacxx/test/libcudacxx/std/utilities/tuple/tuple.tuple/tuple.traits/trivially_copyable.pass.cpplibcudacxx/test/support/copy_move_types.h
💤 Files with no reviewable changes (1)
- libcudacxx/include/cuda/std/__tuple_dir/tuple_like_ext.h
This comment has been minimized.
This comment has been minimized.
4ab569e to
57e3022
Compare
The test does fail with any other standard library, so just disable it
619692c to
40ce652
Compare
2f21ed9 to
5fd347c
Compare
5fd347c to
7a193ac
Compare
🥳 CI Workflow Results🟩 Finished in 3h 20m: Pass: 100%/116 | Total: 5d 04h | Max: 3h 20m | Hits: 38%/2098608See results here. |
This refactors the constructor constraints for
cuda::std::tupleand implements the new tuple-like constructors