Skip to content

Commit 7ebe6e9

Browse files
GH-50236: Remove obsolete OpenSUSE 15.5 workarounds (#50258)
### Rationale for this change Apache Arrow no longer supports the OpenSUSE 15.5 build configuration that required explicit `shared_ptr` and `unique_ptr` constructions. This PR removes obsolete OpenSUSE 15.5-specific workarounds and simplifies the affected return statements by relying on standard implicit conversions. ### What changes are included in this PR? * Removed OpenSUSE 15.5-specific workaround comments. * Replaced explicit `std::shared_ptr<T>(std::move(...))` constructions with direct returns. * Replaced explicit `std::unique_ptr<T>(std::move(...))` constructions with direct returns. * Simplified code in the affected Arrow C++ source files without changing functionality. ### Are these changes tested? Yes. The changes were verified by building the affected components and running the relevant compute test suite: ```bash ninja arrow-compute-tests ctest -R "arrow-compute" --output-on-failure ``` All tests passed successfully. ### Are there any user-facing changes? No. This change is an internal code cleanup and does not affect public APIs, behavior, or user-facing functionality. * GitHub Issue: #50236 Authored-by: Shally Katariya <156352038+Shally-Katariya@users.noreply.github.com> Signed-off-by: Antoine Pitrou <antoine@python.org>
1 parent 4bedf49 commit 7ebe6e9

18 files changed

Lines changed: 53 additions & 78 deletions

cpp/src/arrow/array/concatenate.cc

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -636,8 +636,7 @@ class ConcatenateImpl {
636636
}
637637
out_data += data->length * index_width;
638638
}
639-
// R build with openSUSE155 requires an explicit shared_ptr construction
640-
return std::shared_ptr<Buffer>(std::move(out));
639+
return out;
641640
}
642641

643642
Status Visit(const DictionaryType& d) {

cpp/src/arrow/array/util.cc

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,8 +125,7 @@ class ArrayDataEndianSwapper {
125125
for (int64_t i = 0; i < length; i++) {
126126
out_data[i] = bit_util::ByteSwap(in_data[i]);
127127
}
128-
// R build with openSUSE155 requires an explicit shared_ptr construction
129-
return std::shared_ptr<Buffer>(std::move(out_buffer));
128+
return out_buffer;
130129
}
131130

132131
template <typename VALUE_TYPE>

cpp/src/arrow/buffer.cc

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,7 @@ Result<std::shared_ptr<Buffer>> Buffer::CopySlice(const int64_t start,
4141

4242
ARROW_ASSIGN_OR_RAISE(auto new_buffer, AllocateResizableBuffer(nbytes, pool));
4343
std::memcpy(new_buffer->mutable_data(), data() + start, static_cast<size_t>(nbytes));
44-
// R build with openSUSE155 requires an explicit shared_ptr construction
45-
return std::shared_ptr<Buffer>(std::move(new_buffer));
44+
return new_buffer;
4645
}
4746

4847
Buffer::Buffer() : Buffer(memory_pool::internal::kZeroSizeArea, 0) {}
@@ -186,8 +185,7 @@ Result<std::shared_ptr<Buffer>> AllocateBitmap(int64_t length, MemoryPool* pool)
186185
if (buf->size() > 0) {
187186
buf->mutable_data()[buf->size() - 1] = 0;
188187
}
189-
// R build with openSUSE155 requires an explicit shared_ptr construction
190-
return std::shared_ptr<Buffer>(std::move(buf));
188+
return buf;
191189
}
192190

193191
Result<std::shared_ptr<Buffer>> AllocateEmptyBitmap(int64_t length, MemoryPool* pool) {
@@ -199,8 +197,7 @@ Result<std::shared_ptr<Buffer>> AllocateEmptyBitmap(int64_t length, int64_t alig
199197
ARROW_ASSIGN_OR_RAISE(auto buf,
200198
AllocateBuffer(bit_util::BytesForBits(length), alignment, pool));
201199
memset(buf->mutable_data(), 0, static_cast<size_t>(buf->size()));
202-
// R build with openSUSE155 requires an explicit shared_ptr construction
203-
return std::shared_ptr<Buffer>(std::move(buf));
200+
return buf;
204201
}
205202

206203
Result<std::shared_ptr<Buffer>> ConcatenateBuffers(
@@ -218,8 +215,7 @@ Result<std::shared_ptr<Buffer>> ConcatenateBuffers(
218215
out_data += buffer->size();
219216
}
220217
}
221-
// R build with openSUSE155 requires an explicit shared_ptr construction
222-
return std::shared_ptr<Buffer>(std::move(out));
218+
return out;
223219
}
224220

225221
} // namespace arrow

cpp/src/arrow/compute/function_internal.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -706,8 +706,7 @@ const FunctionOptionsType* GetFunctionOptionsType(const Properties&... propertie
706706
auto options = std::make_unique<Options>();
707707
RETURN_NOT_OK(
708708
FromStructScalarImpl<Options>(options.get(), scalar, properties_).status_);
709-
// R build with openSUSE155 requires an explicit unique_ptr construction
710-
return std::unique_ptr<FunctionOptions>(std::move(options));
709+
return options;
711710
}
712711
std::unique_ptr<FunctionOptions> Copy(const FunctionOptions& options) const override {
713712
auto out = std::make_unique<Options>();

cpp/src/arrow/compute/kernels/aggregate_pivot.cc

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -159,10 +159,7 @@ Result<std::unique_ptr<KernelState>> PivotInit(KernelContext* ctx,
159159
const auto& options = checked_cast<const PivotWiderOptions&>(*args.options);
160160
auto state = std::make_unique<PivotImpl>();
161161
RETURN_NOT_OK(state->Init(options, args.inputs, ctx->exec_context()));
162-
// GH-45718: This can be simplified once we drop the R openSUSE155 crossbow
163-
// job
164-
// R build with openSUSE155 requires an explicit shared_ptr construction
165-
return std::unique_ptr<KernelState>(std::move(state));
162+
return state;
166163
}
167164

168165
Result<TypeHolder> ResolveOutputType(KernelContext* ctx, const std::vector<TypeHolder>&) {

cpp/src/arrow/compute/kernels/hash_aggregate_internal.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,7 @@ Result<std::unique_ptr<KernelState>> HashAggregateInit(KernelContext* ctx,
5555
const KernelInitArgs& args) {
5656
auto impl = std::make_unique<Impl>();
5757
RETURN_NOT_OK(impl->Init(ctx->exec_context(), args));
58-
// R build with openSUSE155 requires an explicit unique_ptr construction
59-
return std::unique_ptr<KernelState>(std::move(impl));
58+
return impl;
6059
}
6160

6261
inline Status HashAggregateResize(KernelContext* ctx, int64_t num_groups) {

cpp/src/arrow/compute/kernels/vector_hash.cc

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -533,8 +533,7 @@ Result<std::unique_ptr<KernelState>> HashInit(KernelContext* ctx,
533533
auto result = std::make_unique<HashKernel>(args.inputs[0].GetSharedPtr(), args.options,
534534
ctx->memory_pool());
535535
RETURN_NOT_OK(result->Reset());
536-
// R build with openSUSE155 requires an explicit unique_ptr construction
537-
return std::unique_ptr<KernelState>(std::move(result));
536+
return result;
538537
}
539538

540539
template <typename Action>

cpp/src/arrow/dataset/file_parquet.cc

Lines changed: 31 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -512,8 +512,7 @@ Result<std::shared_ptr<parquet::arrow::FileReader>> ParquetFileFormat::GetReader
512512
ARROW_ASSIGN_OR_RAISE(auto arrow_reader,
513513
parquet::arrow::FileReader::Make(options->pool, std::move(reader),
514514
std::move(arrow_properties)));
515-
// R build with openSUSE155 requires an explicit shared_ptr construction
516-
return std::shared_ptr<parquet::arrow::FileReader>(std::move(arrow_reader));
515+
return arrow_reader;
517516
}
518517

519518
Future<std::shared_ptr<parquet::arrow::FileReader>> ParquetFileFormat::GetReaderAsync(
@@ -532,37 +531,37 @@ Future<std::shared_ptr<parquet::arrow::FileReader>> ParquetFileFormat::GetReader
532531
source.filesystem(), options->pool);
533532
auto self = checked_pointer_cast<const ParquetFileFormat>(shared_from_this());
534533

535-
return source.OpenAsync().Then([self = self, properties = std::move(properties),
536-
source = source, options = options, metadata = metadata,
537-
parquet_scan_options = parquet_scan_options](
538-
const std::shared_ptr<io::RandomAccessFile>&
539-
input) mutable {
540-
return parquet::ParquetFileReader::OpenAsync(input, properties, metadata)
541-
.Then(
542-
[=](const std::unique_ptr<parquet::ParquetFileReader>& reader) mutable
543-
-> Result<std::shared_ptr<parquet::arrow::FileReader>> {
544-
auto arrow_properties = MakeArrowReaderProperties(
545-
*self, *reader->metadata(), *options, *parquet_scan_options);
546-
547-
ARROW_ASSIGN_OR_RAISE(
548-
auto arrow_reader,
549-
parquet::arrow::FileReader::Make(
550-
options->pool,
551-
// TODO(ARROW-12259): workaround since we have Future<(move-only
552-
// type)> It *wouldn't* be safe to const_cast reader except that
553-
// here we know there are no other waiters on the reader.
554-
std::move(const_cast<std::unique_ptr<parquet::ParquetFileReader>&>(
555-
reader)),
556-
arrow_properties));
557-
558-
// R build with openSUSE155 requires an explicit shared_ptr construction
559-
return std::shared_ptr<parquet::arrow::FileReader>(std::move(arrow_reader));
560-
},
561-
[path = source.path()](const Status& status)
534+
return source.OpenAsync().Then(
535+
[self = self, properties = std::move(properties), source = source,
536+
options = options, metadata = metadata,
537+
parquet_scan_options = parquet_scan_options](
538+
const std::shared_ptr<io::RandomAccessFile>& input) mutable {
539+
return parquet::ParquetFileReader::OpenAsync(input, properties, metadata)
540+
.Then(
541+
[=](const std::unique_ptr<parquet::ParquetFileReader>& reader) mutable
562542
-> Result<std::shared_ptr<parquet::arrow::FileReader>> {
563-
return WrapSourceError(status, path);
564-
});
565-
});
543+
auto arrow_properties = MakeArrowReaderProperties(
544+
*self, *reader->metadata(), *options, *parquet_scan_options);
545+
546+
ARROW_ASSIGN_OR_RAISE(
547+
auto arrow_reader,
548+
parquet::arrow::FileReader::Make(
549+
options->pool,
550+
// TODO(ARROW-12259): workaround since we have Future<(move-only
551+
// type)> It *wouldn't* be safe to const_cast reader except that
552+
// here we know there are no other waiters on the reader.
553+
std::move(
554+
const_cast<std::unique_ptr<parquet::ParquetFileReader>&>(
555+
reader)),
556+
arrow_properties));
557+
558+
return arrow_reader;
559+
},
560+
[path = source.path()](const Status& status)
561+
-> Result<std::shared_ptr<parquet::arrow::FileReader>> {
562+
return WrapSourceError(status, path);
563+
});
564+
});
566565
}
567566

568567
struct SlicingGenerator {

cpp/src/arrow/filesystem/s3fs.cc

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1605,8 +1605,7 @@ class ObjectInputFile final : public io::RandomAccessFile {
16051605
DCHECK_LE(bytes_read, nbytes);
16061606
RETURN_NOT_OK(buf->Resize(bytes_read));
16071607
}
1608-
// R build with openSUSE155 requires an explicit shared_ptr construction
1609-
return std::shared_ptr<Buffer>(std::move(buf));
1608+
return buf;
16101609
}
16111610

16121611
Result<int64_t> Read(int64_t nbytes, void* out) override {

cpp/src/arrow/io/buffered.cc

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -452,8 +452,7 @@ class BufferedInputStream::Impl : public BufferedBase {
452452
RETURN_NOT_OK(buffer->Resize(bytes_read, false /* shrink_to_fit */));
453453
buffer->ZeroPadding();
454454
}
455-
// R build with openSUSE155 requires an explicit shared_ptr construction
456-
return std::shared_ptr<Buffer>(std::move(buffer));
455+
return buffer;
457456
}
458457

459458
// For providing access to the raw file handles

0 commit comments

Comments
 (0)