Skip to content

Commit

Permalink
GH-37891: [C++] Pass shared_ptr<DataType> by value in SliceBuffer-rel…
Browse files Browse the repository at this point in the history
…ated constructors (#45466)

### Rationale for this change

These changes facilitate the process of moving newly constructed shared_ptr without the need of adding a refcount and still support cases where a reference of a shared_ptr is passed from caller.

### What changes are included in this PR?

- Change `SliceBuffer`-related constructor to take `shared_ptr` by value instead of `const &`.
- Change `SliceBufferSafe`-related constructor to take `shared_ptr` by value instead of `const &`.
- Change `SliceMutableBufferSafe` -related constructors to take `shared_ptr` by value instead of `const &`.

### Are these changes tested?

Yes, by existing tests.

### Are there any user-facing changes?

No, the API changes should not break any compatibility.
* GitHub Issue: #37891

Lead-authored-by: chrisxu333 <[email protected]>
Co-authored-by: Antoine Pitrou <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
  • Loading branch information
chrisxu333 and pitrou authored Feb 24, 2025
1 parent 12cdaaa commit 31994b5
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 26 deletions.
24 changes: 12 additions & 12 deletions cpp/src/arrow/buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -63,28 +63,28 @@ Status CheckBufferSlice(const Buffer& buffer, int64_t offset) {

} // namespace

Result<std::shared_ptr<Buffer>> SliceBufferSafe(const std::shared_ptr<Buffer>& buffer,
Result<std::shared_ptr<Buffer>> SliceBufferSafe(std::shared_ptr<Buffer> buffer,
int64_t offset) {
RETURN_NOT_OK(CheckBufferSlice(*buffer, offset));
return SliceBuffer(buffer, offset);
return SliceBuffer(std::move(buffer), offset);
}

Result<std::shared_ptr<Buffer>> SliceBufferSafe(const std::shared_ptr<Buffer>& buffer,
Result<std::shared_ptr<Buffer>> SliceBufferSafe(std::shared_ptr<Buffer> buffer,
int64_t offset, int64_t length) {
RETURN_NOT_OK(CheckBufferSlice(*buffer, offset, length));
return SliceBuffer(buffer, offset, length);
return SliceBuffer(std::move(buffer), offset, length);
}

Result<std::shared_ptr<Buffer>> SliceMutableBufferSafe(
const std::shared_ptr<Buffer>& buffer, int64_t offset) {
Result<std::shared_ptr<Buffer>> SliceMutableBufferSafe(std::shared_ptr<Buffer> buffer,
int64_t offset) {
RETURN_NOT_OK(CheckBufferSlice(*buffer, offset));
return SliceMutableBuffer(buffer, offset);
return SliceMutableBuffer(std::move(buffer), offset);
}

Result<std::shared_ptr<Buffer>> SliceMutableBufferSafe(
const std::shared_ptr<Buffer>& buffer, int64_t offset, int64_t length) {
Result<std::shared_ptr<Buffer>> SliceMutableBufferSafe(std::shared_ptr<Buffer> buffer,
int64_t offset, int64_t length) {
RETURN_NOT_OK(CheckBufferSlice(*buffer, offset, length));
return SliceMutableBuffer(buffer, offset, length);
return SliceMutableBuffer(std::move(buffer), offset, length);
}

std::string Buffer::ToHexString() {
Expand Down Expand Up @@ -167,9 +167,9 @@ std::shared_ptr<Buffer> Buffer::FromString(std::string data) {
return std::make_shared<StlStringBuffer>(std::move(data));
}

std::shared_ptr<Buffer> SliceMutableBuffer(const std::shared_ptr<Buffer>& buffer,
std::shared_ptr<Buffer> SliceMutableBuffer(std::shared_ptr<Buffer> buffer,
const int64_t offset, const int64_t length) {
return std::make_shared<MutableBuffer>(buffer, offset, length);
return std::make_shared<MutableBuffer>(std::move(buffer), offset, length);
}

MutableBuffer::MutableBuffer(const std::shared_ptr<Buffer>& parent, const int64_t offset,
Expand Down
28 changes: 14 additions & 14 deletions cpp/src/arrow/buffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -396,66 +396,66 @@ class ARROW_EXPORT Buffer {
/// \brief Construct a view on a buffer at the given offset and length.
///
/// This function cannot fail and does not check for errors (except in debug builds)
static inline std::shared_ptr<Buffer> SliceBuffer(const std::shared_ptr<Buffer>& buffer,
static inline std::shared_ptr<Buffer> SliceBuffer(std::shared_ptr<Buffer> buffer,
const int64_t offset,
const int64_t length) {
return std::make_shared<Buffer>(buffer, offset, length);
return std::make_shared<Buffer>(std::move(buffer), offset, length);
}

/// \brief Construct a view on a buffer at the given offset, up to the buffer's end.
///
/// This function cannot fail and does not check for errors (except in debug builds)
static inline std::shared_ptr<Buffer> SliceBuffer(const std::shared_ptr<Buffer>& buffer,
static inline std::shared_ptr<Buffer> SliceBuffer(std::shared_ptr<Buffer> buffer,
const int64_t offset) {
int64_t length = buffer->size() - offset;
return SliceBuffer(buffer, offset, length);
return SliceBuffer(std::move(buffer), offset, length);
}

/// \brief Input-checking version of SliceBuffer
///
/// An Invalid Status is returned if the requested slice falls out of bounds.
ARROW_EXPORT
Result<std::shared_ptr<Buffer>> SliceBufferSafe(const std::shared_ptr<Buffer>& buffer,
Result<std::shared_ptr<Buffer>> SliceBufferSafe(std::shared_ptr<Buffer> buffer,
int64_t offset);
/// \brief Input-checking version of SliceBuffer
///
/// An Invalid Status is returned if the requested slice falls out of bounds.
/// Note that unlike SliceBuffer, `length` isn't clamped to the available buffer size.
ARROW_EXPORT
Result<std::shared_ptr<Buffer>> SliceBufferSafe(const std::shared_ptr<Buffer>& buffer,
Result<std::shared_ptr<Buffer>> SliceBufferSafe(std::shared_ptr<Buffer> buffer,
int64_t offset, int64_t length);

/// \brief Like SliceBuffer, but construct a mutable buffer slice.
///
/// If the parent buffer is not mutable, behavior is undefined (it may abort
/// in debug builds).
ARROW_EXPORT
std::shared_ptr<Buffer> SliceMutableBuffer(const std::shared_ptr<Buffer>& buffer,
std::shared_ptr<Buffer> SliceMutableBuffer(std::shared_ptr<Buffer> buffer,
const int64_t offset, const int64_t length);

/// \brief Like SliceBuffer, but construct a mutable buffer slice.
///
/// If the parent buffer is not mutable, behavior is undefined (it may abort
/// in debug builds).
static inline std::shared_ptr<Buffer> SliceMutableBuffer(
const std::shared_ptr<Buffer>& buffer, const int64_t offset) {
static inline std::shared_ptr<Buffer> SliceMutableBuffer(std::shared_ptr<Buffer> buffer,
const int64_t offset) {
int64_t length = buffer->size() - offset;
return SliceMutableBuffer(buffer, offset, length);
return SliceMutableBuffer(std::move(buffer), offset, length);
}

/// \brief Input-checking version of SliceMutableBuffer
///
/// An Invalid Status is returned if the requested slice falls out of bounds.
ARROW_EXPORT
Result<std::shared_ptr<Buffer>> SliceMutableBufferSafe(
const std::shared_ptr<Buffer>& buffer, int64_t offset);
Result<std::shared_ptr<Buffer>> SliceMutableBufferSafe(std::shared_ptr<Buffer> buffer,
int64_t offset);
/// \brief Input-checking version of SliceMutableBuffer
///
/// An Invalid Status is returned if the requested slice falls out of bounds.
/// Note that unlike SliceBuffer, `length` isn't clamped to the available buffer size.
ARROW_EXPORT
Result<std::shared_ptr<Buffer>> SliceMutableBufferSafe(
const std::shared_ptr<Buffer>& buffer, int64_t offset, int64_t length);
Result<std::shared_ptr<Buffer>> SliceMutableBufferSafe(std::shared_ptr<Buffer> buffer,
int64_t offset, int64_t length);

/// @}

Expand Down

0 comments on commit 31994b5

Please sign in to comment.