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.

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

[C++] const qualifier on mutable_span_as prevents call to mutable_data_as without const qualifier #40366

Closed
sjperkins opened this issue Mar 5, 2024 · 1 comment
Assignees
Milestone

Comments

@sjperkins
Copy link
Contributor

Describe the bug, including details regarding any error messages, version, and platform.

#38027 added Buffer::span_as and Buffer::mutable_span_as

mutable_span_as has a const qualifier,

template <typename T>
T* mutable_data_as() {
return reinterpret_cast<T*>(mutable_data());
}
/// \brief Return the buffer's mutable data as a span
template <typename T>
util::span<T> mutable_span_as() const {
return util::span(mutable_data_as<T>(), static_cast<size_t>(size() / sizeof(T)));
}

but this prevents the call to mutable_data_as during compile:

...include/arrow/buffer.h:273:41: error: passing ‘const arrow::Buffer’ as ‘this’ argument discards qualifiers [-fpermissive]
  273 |     return util::span(mutable_data_as<T>(), static_cast<size_t>(size() / sizeof(T)));

By contrast, both span_as and data_as have const qualifiers.

/// \brief Return a pointer to the buffer's data cast to a specific type
///
/// The buffer has to be a CPU buffer (`is_cpu()` is true).
/// Otherwise, an assertion may be thrown or a null pointer may be returned.
template <typename T>
const T* data_as() const {
return reinterpret_cast<const T*>(data());
}
/// \brief Return the buffer's data as a span
template <typename T>
util::span<const T> span_as() const {
return util::span(data_as<T>(), static_cast<size_t>(size() / sizeof(T)));
}

Also, mutable_span_as should not have a const qualifier as it implies modification of arrow::Buffer.

Component(s)

C++

@sjperkins sjperkins changed the title const qualifier on mutable_span_as prevents call to mutable_data_as without const qualifier [C++] const qualifier on mutable_span_as prevents call to mutable_data_as without const qualifier Mar 5, 2024
kou pushed a commit that referenced this issue Mar 6, 2024
…40367)

### Rationale for this change

The const qualifier on `Buffer:mutable_span_as` prevents it from calling the non-const `Buffer::mutable_data_as`

### What changes are included in this PR?

See issue title

### Are these changes tested?

No, I believe this is a simple oversight 

### Are there any user-facing changes?

Yes, `Buffer::mutable_span_as` loses it's const qualifier

* GitHub Issue: #40366

Authored-by: Simon Perkins <simon.perkins@gmail.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
@kou kou added this to the 16.0.0 milestone Mar 6, 2024
@kou
Copy link
Member

kou commented Mar 6, 2024

Issue resolved by pull request 40367
#40367

@kou kou closed this as completed Mar 6, 2024
mapleFU pushed a commit to mapleFU/arrow that referenced this issue Mar 7, 2024
…n_as (apache#40367)

### Rationale for this change

The const qualifier on `Buffer:mutable_span_as` prevents it from calling the non-const `Buffer::mutable_data_as`

### What changes are included in this PR?

See issue title

### Are these changes tested?

No, I believe this is a simple oversight 

### Are there any user-facing changes?

Yes, `Buffer::mutable_span_as` loses it's const qualifier

* GitHub Issue: apache#40366

Authored-by: Simon Perkins <simon.perkins@gmail.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
thisisnic pushed a commit to thisisnic/arrow that referenced this issue Mar 8, 2024
…n_as (apache#40367)

### Rationale for this change

The const qualifier on `Buffer:mutable_span_as` prevents it from calling the non-const `Buffer::mutable_data_as`

### What changes are included in this PR?

See issue title

### Are these changes tested?

No, I believe this is a simple oversight 

### Are there any user-facing changes?

Yes, `Buffer::mutable_span_as` loses it's const qualifier

* GitHub Issue: apache#40366

Authored-by: Simon Perkins <simon.perkins@gmail.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants