You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Reviewing files that changed from the base of the PR and between e525bcf and 239dde8.
You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.
Use the checkbox below for a quick retry:
Trigger review
No actionable comments were generated in the recent review.
Summary
Summary by CodeRabbit
New Features
Added compile-time size information for ranges with statically known sizes, including built-in arrays, cuda::std::array, fixed-extent spans, and supported custom ranges. Ranges with dynamic or unavailable sizes are identified as having a dynamic extent.
Tests
Added compile-time coverage for supported range types, cv/ref variations, and dynamic or unavailable sizes, including ranges with non-default-constructible element types.
The change adds cuda::std::ranges::__static_size_v. It yields a compile-time size when ranges::size is a size_t constant expression, and otherwise yields dynamic_extent. Compile-time tests cover built-in and custom range types.
The tests check fixed-size and dynamic-size ranges, unsupported cases, cv/ref forms, and the variable’s result type.
Change: Feature
Merge Risk: Minimal · previous review e525b
This adds a new internal trait, ranges::__static_size_v, and compile-time tests for it. No existing behavior changes, and no merge-blocking risk was found.
Architecture risk: Low · up to 0bfc3
The change affects 1 system.
Changed systems:libcudacxx
Architecture concerns
No architecture-level concerns identified.
Review details
Systems and components
observed — libcudacxx (service) was modified; 2 changed files map to changed impact.
Before / after behavior
observed — Modified behavior in libcudacxx/test/libcudacxx/std/ranges/range.access/static_size.compile.pass.cpp: Adds headers and test types representing fixed-size and dynamic-size ranges, including member-size, ADL-size, runtime-size, value-dependent-size, sentinel-sized, and disabled-sized cases.
observed — Modified behavior in libcudacxx/test/libcudacxx/std/ranges/range.access/static_size.compile.pass.cpp: Adds test_cvref, which asserts the expected static size for mutable, const, volatile, and reference-qualified forms of a type, and verifies the result type is const cuda::std::size_t.
observed — Modified behavior in libcudacxx/test/libcudacxx/std/ranges/range.access/static_size.compile.pass.cpp: Adds a compile-time test covering expected sizes for arrays, cuda::std::array, fixed-extent spans, and custom fixed-size ranges; it checks dynamic_extent for dynamic or unsupported cases and runs the test from main.
observed — Modified behavior in libcudacxx/include/cuda/std/__ranges/size.h: The header adds the span forward declaration and includes integral_constant and void_t; the other listed includes remain in place.
- [ ] Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Reviewing files that changed from the base of the PR and between 0bfc31e and e525bcf.
Files selected for processing: 1
libcudacxx/include/cuda/std/__ranges/size.h
Files skipped from review as they are similar to previous changes: 1
libcudacxx/include/cuda/std/__ranges/size.h
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.
The reason will be displayed to describe this comment to others. Learn more.
Why extract the ::type here? void_t<integral_constant> is enough to exercise the SFINAE. Or I guess void_t<decltype(range::size(...))> would do it too.
The reason will be displayed to describe this comment to others. Learn more.
void_t<decltype(range::size(...))> wouldn't be enough, we need the value of the invocation, not the return type. We query ::type because we need to make sure that the range::size(...) is a constant expression and instantiates integral_constant
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR implements
ranges::__static_size_vtraits that holds the size of the range if it can be statically determined,dynamic_extentotherwise.