[C++] ConcatenateImpl Constructor re-uses moved argument "const ArrayDataVector & in"

[sjperkins]

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

In cpp/src/arrow/array/concatenate.cc , the const ArrayDataVector & in argument is referenced after being moved into the in_ within the member initializer list:

arrow/cpp/src/arrow/array/concatenate.cc

Lines 172 to 191 in 7ebc88c

	class ConcatenateImpl {
	public:
	ConcatenateImpl(const ArrayDataVector& in, MemoryPool* pool)
	: in_(std::move(in)), pool_(pool), out_(std::make_shared<ArrayData>()) {
	out_->type = in[0]->type;
	for (size_t i = 0; i < in_.size(); ++i) {
	out_->length = SafeSignedAdd(out_->length, in[i]->length);
	if (out_->null_count == kUnknownNullCount ||
	in[i]->null_count == kUnknownNullCount) {
	out_->null_count = kUnknownNullCount;
	continue;
	}
	out_->null_count = SafeSignedAdd(out_->null_count.load(), in[i]->null_count.load());
	}
	out_->buffers.resize(in[0]->buffers.size());
	out_->child_data.resize(in[0]->child_data.size());
	for (auto& data : out_->child_data) {
	data = std::make_shared<ArrayData>();
	}
	}

where using ArrayDataVector = std::vector<std::shared_ptr<ArrayData>>; in type_fwd.h.

I was surprised the code works and that this seems to result in a copy of in to in_ . But I suspect that either

1. The signature should be ConcatenateImpl(ArrayDataVector in, MemoryPool* pool) and in should not be re-used after the move, or:
2. The member initializer list should be : in_(in), pool_(pool), out_(std::make_shared<ArrayData>())

Component(s)

C++

[mapleFU]

Using std::move on a constant lvalue reference might not causing a bug, because it might just copy the vector. But it's worthing to fix it.

[sjperkins]

Using std::move on a constant lvalue reference might not causing a bug, because it might just copy the vector. But it's worthing to fix it.

Yes you're right, I'd forgotten that std::move instructs the compiler to try a move, but it's not guaranteed. This SO explains it well https://stackoverflow.com/a/28595207.