[libcu++] Implement P3060R3 Add std::views::indices(n) - #10700
Conversation
237e92b to
73ea3a2
Compare
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR adds Changesviews::indices
Assessment against linked issues
Suggested reviewers: Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
libcudacxx/include/cuda/std/__ranges/iota_view.h (1)
270-270: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winsuggestion: Declare
__sizeasconst _Tp __size. Line 270 does not modify this parameter. As per coding guidelines, “All variables that are not modified must be declaredconst.”Source: Coding guidelines
libcudacxx/test/libcudacxx/std/ranges/range.factories/range.iota.view/indices.pass.cpp (1)
27-57: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winsuggestion: Rename
HasIndices,HasIndicesFn,NotIntegerLike, andtest_SFINAEto snake_case. These are not template parameters or CUB public API symbols. As per coding guidelines, “Use snake_case for all other symbols.”Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: dd60016d-4852-4385-8a66-ec1cda5adf54
📒 Files selected for processing (3)
libcudacxx/include/cuda/std/__ranges/iota_view.hlibcudacxx/include/cuda/std/versionlibcudacxx/test/libcudacxx/std/ranges/range.factories/range.iota.view/indices.pass.cpp
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
libcudacxx/test/libcudacxx/std/ranges/range.factories/range.iota.view/indices.pass.cpp (1)
27-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winsuggestion: Rename
HasIndices,HasIndicesFn,NotIntegerLike, andtest_SFINAEto snake_case. KeepSizeTypeandTin PascalCase because they are template parameters. As per coding guidelines, “Use snake_case for all other symbols, except that the CUB public API uses PascalCase.”Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 9089676e-6bef-4dbb-8a8d-567e84e0a086
📒 Files selected for processing (3)
libcudacxx/include/cuda/std/__ranges/iota_view.hlibcudacxx/include/cuda/std/versionlibcudacxx/test/libcudacxx/std/ranges/range.factories/range.iota.view/indices.pass.cpp
🚧 Files skipped from review as they are similar to previous changes (2)
- libcudacxx/include/cuda/std/__ranges/iota_view.h
- libcudacxx/include/cuda/std/version
⏱️ CCCL compile-time benchmark comparison: Public headers compile-time benchResult: 0 regression row(s), 11 improvement row(s) above threshold.
Artifacts: reports and traces TU total compilation
🟢 TU total compilation — Improvements
Direct file processing
🟢 Direct file processing — Improvements
|
😬 CI Workflow Results🟥 Finished in 4h 27m: Pass: 88%/115 | Total: 4d 09h | Max: 4h 27m | Hits: 43%/1225052See results here. AI failure analysis1. Default-initialized SomeInt array breaks NVCC/MSVC constexpr evaluation · 13 jobsExplanation: The new `indices.pass.cpp` test fails during `static_assert(test())` in every NVCC/MSVC matrix entry. The compiler cannot treat the synthesized default-constructor closure for the test's uninitialized `cuda::std::array<SomeInt, 5>` as constexpr, preventing all 13 Windows builds from completing. Evidence: libcu++ nvcc MSVC / [CTK12.0 MSVC14.39 C++20] Build(amd64), step 4 Root cause: The PR declares `cuda::std::array<SomeInt, 5> v;` inside a function evaluated by `static_assert`. Under every tested NVCC/MSVC combination, that default initialization produces a synthesized `SomeInt::__dflt_ctor_closure` that the compiler cannot evaluate as constexpr; the neighboring integer array is explicitly value-initialized with `{}` and does not fail. Sources: libcudacxx/test/libcudacxx/std/ranges/range.factories/range.iota.view/indices.pass.cpp:90, libcudacxx/test/libcudacxx/std/ranges/range.factories/range.iota.view/indices.pass.cpp:110, libcudacxx/test/libcudacxx/std/ranges/range.factories/range.iota.view/types.h:20. Suggested next steps: Change the declaration at line 90 to `cuda::std::array<SomeInt, 5> v{};`, then run the focused `indices.pass.cpp` libcudacxx lit test with NVCC/MSVC in C++17 and C++20. Verify at least one oldest and one newest failing toolchain before rerunning the Windows matrix. Copy this prompt into a coding agentJobs:
|
Fixes #10356.