Skip to content

Commit

Permalink
Fix the index type in the indexing operator of the span types (#17971)
Browse files Browse the repository at this point in the history
Closes #17949
Closes #17960

Derived span classes use `size_type` for the index type in their
`operator[]` implementations. The intent was to use `base::size_type`,
but the type actually resolves to `cudf::size_type`, which is `int32_t`,
and does not allow access past `int32_t::max`.

This PR fixes the type used by explicitly using `typename
base::size_type`. Also added static_asserts to make sure the type has
the right size for element indexing.
  • Loading branch information
vuule authored Feb 11, 2025
1 parent d1a5558 commit 9ae04a7
Showing 1 changed file with 8 additions and 3 deletions.
11 changes: 8 additions & 3 deletions cpp/include/cudf/utilities/span.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,11 @@ struct host_span : public cudf::detail::span_base<T, Extent, host_span<T, Extent
* @param idx the index of the element to access
* @return A reference to the idx-th element of the sequence, i.e., `data()[idx]`
*/
constexpr typename base::reference operator[](size_type idx) const { return this->_data[idx]; }
constexpr typename base::reference operator[](typename base::size_type idx) const
{
static_assert(sizeof(idx) >= sizeof(size_t), "index type must not be smaller than size_t");
return this->_data[idx];
}

// not noexcept due to undefined behavior when size = 0
/**
Expand Down Expand Up @@ -402,8 +406,9 @@ struct device_span : public cudf::detail::span_base<T, Extent, device_span<T, Ex
* @param idx the index of the element to access
* @return A reference to the idx-th element of the sequence, i.e., `data()[idx]`
*/
__device__ constexpr typename base::reference operator[](size_type idx) const
__device__ constexpr typename base::reference operator[](typename base::size_type idx) const
{
static_assert(sizeof(idx) >= sizeof(size_t), "index type must not be smaller than size_t");
return this->_data[idx];
}

Expand Down Expand Up @@ -512,7 +517,7 @@ class base_2dspan {
* @param row the index of the element to access
* @return A reference to the row-th element of the sequence, i.e., `data()[row]`
*/
CUDF_HOST_DEVICE constexpr RowType<T, dynamic_extent> operator[](size_t row) const
CUDF_HOST_DEVICE constexpr RowType<T, dynamic_extent> operator[](std::size_t row) const
{
return _flat.subspan(row * _size.second, _size.second);
}
Expand Down

0 comments on commit 9ae04a7

Please sign in to comment.