GH-50251: Add GetSpan() convenience method to ArrayData#50310
GH-50251: Add GetSpan() convenience method to ArrayData#50310VedantRalekar wants to merge 2 commits into
Conversation
|
|
| return std::span<const T>( | ||
| reinterpret_cast<const T*>(buffers[i]->data()) + offset, length); | ||
| } |
There was a problem hiding this comment.
Thanks for pr, @VedantRalekar!
These lines need clang-format fix per Dev / Lint job logs.
See also Styling and Dev Guidelines, you can run
pre-commit run --show-diff-on-failure --color=always --all-files cpp
One optional point to consider - perhaps add a test in array_test.cc to cover + offset and also the new null-buffer case?
There was a problem hiding this comment.
Thanks for the feedback! I'll apply the clang-format fixes and add the suggested tests for the offset and null-buffer cases. I'll update the PR once the changes are ready.
| /// \pre i > 0 | ||
| /// \pre length <= the length of the buffer (in number of values) that's expected for | ||
| /// this array type | ||
| /// \return A span<const T> of the requested length |
There was a problem hiding this comment.
Maybe add a mention of empty span to \return
There was a problem hiding this comment.
i have added a mention of empty span to \return, please review it.
2 participants
What changes were proposed in this pull request?
This PR adds a
GetSpan()convenience method toArrayData, similar to the existingGetSpan()implementation inArraySpan.Why are the changes needed?
ArraySpanalready provides a convenient way to access buffer data asstd::span, whileArrayDatadoes not. This change adds a similar API to improve consistency and make span-based buffer access easier.Fixes #50251.