Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[C++] APIs taking a const vector<T>& argument should instead take a span<const T> #44795

Closed
pitrou opened this issue Nov 20, 2024 · 1 comment
Closed

[C++] APIs taking a const vector<T>& argument should instead take a span<const T> #44795

pitrou opened this issue Nov 20, 2024 · 1 comment
Assignees
@raulcd
Labels
Component: C++ good-first-issue Type: enhancement
Milestone
19.0.0

Comments

@pitrou
Copy link
Member

pitrou commented Nov 20, 2024 •
edited
Loading

Describe the enhancement requested

arrow::util::span (a backport of C++20 std::span) is more generally applicable than std::vector, so any public API currently accepting a vector const-ref argument should instead accept a span argument.

This could be broken down into several subtasks or PRs if deemed convenient.

Component(s)

C++
raulcd added a commit to raulcd/arrow that referenced this issue Nov 20, 2024
@raulcd
apacheGH-44795: [C++] Use arrow::util::span on arrow::util::bitmap_bu…
34dd192 
…ilders utilities
@github-actions github-actions bot mentioned this issue Nov 20, 2024
Merged
raulcd added a commit to raulcd/arrow that referenced this issue Nov 21, 2024
@raulcd
apacheGH-44795: [C++] Use arrow::util::span on arrow::util::bitmap_bu…
6d5e649 
…ilders utilities
raulcd added a commit to raulcd/arrow that referenced this issue Nov 22, 2024
@raulcd
apacheGH-44795: [C++] Use arrow::util::span<T> on BooleanBuilder APIs…
3ec464b 
… instead of std::vector<T>&
@github-actions github-actions bot mentioned this issue Nov 22, 2024
Draft
pitrou pushed a commit that referenced this issue Dec 18, 2024
@raulcd
GH-44795: [C++] Use arrow::util::span on arrow::util::bitmap_builders…
7af2ad4 
…_utilities instead of std::vector (#44796)

### Rationale for this change

`arrow::util::span` (a backport of C++20 `std::span`) is more generally applicable than `std::vector`, so any public API currently accepting a vector const-ref argument should instead accept a span argument.

### What changes are included in this PR?

`arrow::util::BytesToBits` accepts `arrow::util::span`  instead of `std::vector`

### Are these changes tested?

Yes, existing C++ tests via CI

### Are there any user-facing changes?

Yes,  from `Result<std::shared_ptr<Buffer>> BytesToBits(const std::vector<uint8_t>&, MemoryPool* pool)` to `Result<std::shared_ptr<Buffer>> BytesToBits(util::span<const uint8_t> bytes, MemoryPool* pool)`
* GitHub Issue: #44795

Authored-by: Raúl Cumplido <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
@pitrou pitrou added this to the 19.0.0 milestone Dec 18, 2024
@pitrou
Copy link
Member Author

pitrou commented Dec 18, 2024

Issue resolved by pull request 44796
#44796

@pitrou pitrou closed this as completed Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@raulcd raulcd

Labels
Component: C++ good-first-issue Type: enhancement
Projects
None yet
Milestone
19.0.0
Development

No branches or pull requests
2 participants
@raulcd @pitrou