This is an automated email from the ASF dual-hosted git repository. wesm pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/master by this push: new f77c342 ARROW-5145: [C++] More input validation in release mode f77c342 is described below commit f77c3427ca801597b572fb197b92b0133269049b Author: Antoine Pitrou <anto...@python.org> AuthorDate: Thu Jun 27 11:17:55 2019 -0500 ARROW-5145: [C++] More input validation in release mode This will make user experience better when passing invalid arguments. Author: Antoine Pitrou <anto...@python.org> Closes #4720 from pitrou/ARROW-5145-release-mode-checks and squashes the following commits: daaeb220f <Antoine Pitrou> ARROW-5145: More input validation in release mode --- cpp/cmake_modules/SetupCxxFlags.cmake | 2 +- cpp/src/arrow/array-list-test.cc | 2 +- cpp/src/arrow/array.cc | 104 +++++++++++++------------ cpp/src/arrow/array/builder_adaptive.cc | 2 - cpp/src/arrow/array/builder_binary.cc | 6 +- cpp/src/arrow/array/builder_dict.cc | 4 +- cpp/src/arrow/array/builder_nested.cc | 9 ++- cpp/src/arrow/array/builder_primitive.cc | 2 +- cpp/src/arrow/buffer.cc | 4 +- cpp/src/arrow/compute/kernels/util-internal.cc | 6 +- cpp/src/arrow/extension_type.cc | 6 +- cpp/src/arrow/flight/client.cc | 4 +- cpp/src/arrow/flight/serialization-internal.cc | 4 +- cpp/src/arrow/gpu/cuda_memory.cc | 20 +++-- cpp/src/arrow/io/buffered.cc | 16 ++-- cpp/src/arrow/io/compressed.cc | 4 +- cpp/src/arrow/io/file.cc | 9 +-- cpp/src/arrow/io/memory.cc | 4 +- cpp/src/arrow/ipc/message.cc | 3 +- cpp/src/arrow/ipc/reader.cc | 20 +++-- cpp/src/arrow/python/arrow_to_pandas.cc | 2 +- cpp/src/arrow/python/common.cc | 4 +- cpp/src/arrow/python/decimal.cc | 8 +- cpp/src/arrow/python/helpers.cc | 9 +-- cpp/src/arrow/python/helpers.h | 2 +- cpp/src/arrow/python/inference.cc | 3 +- cpp/src/arrow/python/python-test.cc | 13 ++-- cpp/src/arrow/python/type_traits.h | 2 +- cpp/src/arrow/record_batch.cc | 4 +- cpp/src/arrow/scalar.cc | 13 ++-- cpp/src/arrow/sparse_tensor.cc | 10 +-- cpp/src/arrow/status.cc | 14 +++- cpp/src/arrow/status.h | 23 +++--- cpp/src/arrow/table.cc | 4 +- cpp/src/arrow/table_builder.cc | 2 +- cpp/src/arrow/tensor.cc | 6 +- cpp/src/arrow/type.cc | 14 ++-- cpp/src/arrow/util/compression_bz2.cc | 4 +- cpp/src/arrow/util/compression_zlib.cc | 8 +- cpp/src/arrow/util/decimal.cc | 4 +- cpp/src/arrow/util/io-util.cc | 7 +- cpp/src/arrow/util/key_value_metadata.cc | 4 +- cpp/src/arrow/util/logging.h | 45 ++++++----- cpp/src/arrow/util/thread-pool.cc | 2 +- 44 files changed, 245 insertions(+), 193 deletions(-) diff --git a/cpp/cmake_modules/SetupCxxFlags.cmake b/cpp/cmake_modules/SetupCxxFlags.cmake index 0c07a55..496904b 100644 --- a/cpp/cmake_modules/SetupCxxFlags.cmake +++ b/cpp/cmake_modules/SetupCxxFlags.cmake @@ -143,7 +143,7 @@ if("${BUILD_WARNING_LEVEL}" STREQUAL "CHECKIN") -Wno-comma -Wno-unused-macros -Wno-unused-parameter -Wno-unused-template -Wno-undef \ -Wno-shadow -Wno-switch-enum -Wno-exit-time-destructors \ -Wno-global-constructors -Wno-weak-template-vtables -Wno-undefined-reinterpret-cast \ --Wno-implicit-fallthrough -Wno-unreachable-code-return \ +-Wno-implicit-fallthrough -Wno-unreachable-code -Wno-unreachable-code-return \ -Wno-float-equal -Wno-missing-prototypes -Wno-documentation-unknown-command \ -Wno-old-style-cast -Wno-covered-switch-default \ -Wno-cast-align -Wno-vla-extension -Wno-shift-sign-overflow \ diff --git a/cpp/src/arrow/array-list-test.cc b/cpp/src/arrow/array-list-test.cc index 3847b9e..c3118e9 100644 --- a/cpp/src/arrow/array-list-test.cc +++ b/cpp/src/arrow/array-list-test.cc @@ -189,7 +189,7 @@ TEST_F(TestListArray, TestFromArrays) { ListArray::FromArrays(*offsets1->Slice(0, 0), *values, pool_, &tmp)); // Offsets not int32 - ASSERT_RAISES(Invalid, ListArray::FromArrays(*values, *offsets1, pool_, &tmp)); + ASSERT_RAISES(TypeError, ListArray::FromArrays(*values, *offsets1, pool_, &tmp)); } TEST_F(TestListArray, TestAppendNull) { diff --git a/cpp/src/arrow/array.cc b/cpp/src/arrow/array.cc index b41d6dd..95acc6b 100644 --- a/cpp/src/arrow/array.cc +++ b/cpp/src/arrow/array.cc @@ -78,7 +78,7 @@ std::shared_ptr<ArrayData> ArrayData::Make(const std::shared_ptr<DataType>& type } ArrayData ArrayData::Slice(int64_t off, int64_t len) const { - DCHECK_LE(off, length); + ARROW_CHECK_LE(off, length) << "Slice offset greater than array length"; len = std::min(length - off, len); off += offset; @@ -166,8 +166,7 @@ std::shared_ptr<Array> Array::Slice(int64_t offset) const { std::string Array::ToString() const { std::stringstream ss; - Status s = PrettyPrint(*this, 0, &ss); - DCHECK_OK(s); + ARROW_CHECK_OK(PrettyPrint(*this, 0, &ss)); return ss.str(); } @@ -190,7 +189,7 @@ PrimitiveArray::PrimitiveArray(const std::shared_ptr<DataType>& type, int64_t le BooleanArray::BooleanArray(const std::shared_ptr<ArrayData>& data) : PrimitiveArray(data) { - DCHECK_EQ(data->type->id(), Type::BOOL); + ARROW_CHECK_EQ(data->type->id(), Type::BOOL); } BooleanArray::BooleanArray(int64_t length, const std::shared_ptr<Buffer>& data, @@ -221,7 +220,7 @@ Status ListArray::FromArrays(const Array& offsets, const Array& values, MemoryPo } if (offsets.type_id() != Type::INT32) { - return Status::Invalid("List offsets must be signed int32"); + return Status::TypeError("List offsets must be signed int32"); } BufferVector buffers = {}; @@ -231,11 +230,15 @@ Status ListArray::FromArrays(const Array& offsets, const Array& values, MemoryPo const int64_t num_offsets = offsets.length(); if (offsets.null_count() > 0) { - std::shared_ptr<Buffer> clean_offsets, clean_valid_bits; + if (!offsets.IsValid(num_offsets - 1)) { + return Status::Invalid("Last list offset should be non-null"); + } + std::shared_ptr<Buffer> clean_offsets, clean_valid_bits; RETURN_NOT_OK(AllocateBuffer(pool, num_offsets * sizeof(int32_t), &clean_offsets)); // Copy valid bits, zero out the bit for the final offset + // XXX why? RETURN_NOT_OK(offsets.null_bitmap()->Copy(0, BitUtil::BytesForBits(num_offsets - 1), &clean_valid_bits)); BitUtil::ClearBit(clean_valid_bits->mutable_data(), num_offsets); @@ -245,7 +248,6 @@ Status ListArray::FromArrays(const Array& offsets, const Array& values, MemoryPo auto clean_raw_offsets = reinterpret_cast<int32_t*>(clean_offsets->mutable_data()); // Must work backwards so we can tell how many values were in the last non-null value - DCHECK(offsets.IsValid(num_offsets - 1)); int32_t current_offset = raw_offsets[num_offsets - 1]; for (int64_t i = num_offsets - 1; i >= 0; --i) { if (offsets.IsValid(i)) { @@ -271,8 +273,8 @@ Status ListArray::FromArrays(const Array& offsets, const Array& values, MemoryPo void ListArray::SetData(const std::shared_ptr<ArrayData>& data) { this->Array::SetData(data); - DCHECK_EQ(data->buffers.size(), 2); - DCHECK(data->type->id() == Type::LIST); + ARROW_CHECK_EQ(data->buffers.size(), 2); + ARROW_CHECK(data->type->id() == Type::LIST); list_type_ = checked_cast<const ListType*>(data->type.get()); auto value_offsets = data->buffers[1]; @@ -280,7 +282,9 @@ void ListArray::SetData(const std::shared_ptr<ArrayData>& data) { ? nullptr : reinterpret_cast<const int32_t*>(value_offsets->data()); - DCHECK_EQ(data_->child_data.size(), 1); + ARROW_CHECK_EQ(data_->child_data.size(), 1); + ARROW_CHECK_EQ(list_type_->value_type()->id(), data->child_data[0]->type->id()); + DCHECK(list_type_->value_type()->Equals(data->child_data[0]->type)); values_ = MakeArray(data_->child_data[0]); } @@ -309,12 +313,12 @@ MapArray::MapArray(const std::shared_ptr<DataType>& type, int64_t length, } void MapArray::SetData(const std::shared_ptr<ArrayData>& data) { - DCHECK_EQ(data->type->id(), Type::MAP); + ARROW_CHECK_EQ(data->type->id(), Type::MAP); auto pair_data = data->child_data[0]; - DCHECK_EQ(pair_data->type->id(), Type::STRUCT); - DCHECK_EQ(pair_data->null_count, 0); - DCHECK_EQ(pair_data->child_data.size(), 2); - DCHECK_EQ(pair_data->child_data[0]->null_count, 0); + ARROW_CHECK_EQ(pair_data->type->id(), Type::STRUCT); + ARROW_CHECK_EQ(pair_data->null_count, 0); + ARROW_CHECK_EQ(pair_data->child_data.size(), 2); + ARROW_CHECK_EQ(pair_data->child_data[0]->null_count, 0); auto pair_list_data = data->Copy(); pair_list_data->type = list(pair_data->type); @@ -343,13 +347,14 @@ FixedSizeListArray::FixedSizeListArray(const std::shared_ptr<DataType>& type, } void FixedSizeListArray::SetData(const std::shared_ptr<ArrayData>& data) { - DCHECK_EQ(data->type->id(), Type::FIXED_SIZE_LIST); + ARROW_CHECK_EQ(data->type->id(), Type::FIXED_SIZE_LIST); this->Array::SetData(data); + ARROW_CHECK_EQ(list_type()->value_type()->id(), data->child_data[0]->type->id()); DCHECK(list_type()->value_type()->Equals(data->child_data[0]->type)); list_size_ = list_type()->list_size(); - DCHECK_EQ(data_->child_data.size(), 1); + ARROW_CHECK_EQ(data_->child_data.size(), 1); values_ = MakeArray(data_->child_data[0]); } @@ -367,12 +372,12 @@ std::shared_ptr<Array> FixedSizeListArray::values() const { return values_; } // String and binary BinaryArray::BinaryArray(const std::shared_ptr<ArrayData>& data) { - DCHECK_EQ(data->type->id(), Type::BINARY); + ARROW_CHECK_EQ(data->type->id(), Type::BINARY); SetData(data); } void BinaryArray::SetData(const std::shared_ptr<ArrayData>& data) { - DCHECK_EQ(data->buffers.size(), 3); + ARROW_CHECK_EQ(data->buffers.size(), 3); auto value_offsets = data->buffers[1]; auto value_data = data->buffers[2]; this->Array::SetData(data); @@ -399,7 +404,7 @@ BinaryArray::BinaryArray(const std::shared_ptr<DataType>& type, int64_t length, } StringArray::StringArray(const std::shared_ptr<ArrayData>& data) { - DCHECK_EQ(data->type->id(), Type::STRING); + ARROW_CHECK_EQ(data->type->id(), Type::STRING); SetData(data); } @@ -453,7 +458,7 @@ DayTimeIntervalType::DayMilliseconds DayTimeIntervalArray::GetValue(int64_t i) c Decimal128Array::Decimal128Array(const std::shared_ptr<ArrayData>& data) : FixedSizeBinaryArray(data) { - DCHECK_EQ(data->type->id(), Type::DECIMAL); + ARROW_CHECK_EQ(data->type->id(), Type::DECIMAL); } std::string Decimal128Array::FormatValue(int64_t i) const { @@ -466,7 +471,7 @@ std::string Decimal128Array::FormatValue(int64_t i) const { // Struct StructArray::StructArray(const std::shared_ptr<ArrayData>& data) { - DCHECK_EQ(data->type->id(), Type::STRUCT); + ARROW_CHECK_EQ(data->type->id(), Type::STRUCT); SetData(data); boxed_fields_.resize(data->child_data.size()); } @@ -475,6 +480,7 @@ StructArray::StructArray(const std::shared_ptr<DataType>& type, int64_t length, const std::vector<std::shared_ptr<Array>>& children, std::shared_ptr<Buffer> null_bitmap, int64_t null_count, int64_t offset) { + ARROW_CHECK_EQ(type->id(), Type::STRUCT); SetData(ArrayData::Make(type, length, {null_bitmap}, null_count, offset)); for (const auto& child : children) { data_->child_data.push_back(child->data()); @@ -497,7 +503,6 @@ std::shared_ptr<Array> StructArray::field(int i) const { } boxed_fields_[i] = MakeArray(field_data); } - DCHECK(boxed_fields_[i]); return boxed_fields_[i]; } @@ -559,7 +564,8 @@ Status StructArray::Flatten(MemoryPool* pool, ArrayVector* out) const { void UnionArray::SetData(const std::shared_ptr<ArrayData>& data) { this->Array::SetData(data); - DCHECK_EQ(data->buffers.size(), 3); + ARROW_CHECK_EQ(data->type->id(), Type::UNION); + ARROW_CHECK_EQ(data->buffers.size(), 3); auto type_ids = data_->buffers[1]; auto value_offsets = data_->buffers[2]; @@ -571,10 +577,7 @@ void UnionArray::SetData(const std::shared_ptr<ArrayData>& data) { boxed_fields_.resize(data->child_data.size()); } -UnionArray::UnionArray(const std::shared_ptr<ArrayData>& data) { - DCHECK_EQ(data->type->id(), Type::UNION); - SetData(data); -} +UnionArray::UnionArray(const std::shared_ptr<ArrayData>& data) { SetData(data); } UnionArray::UnionArray(const std::shared_ptr<DataType>& type, int64_t length, const std::vector<std::shared_ptr<Array>>& children, @@ -600,11 +603,11 @@ Status UnionArray::MakeDense(const Array& type_ids, const Array& value_offsets, } if (value_offsets.type_id() != Type::INT32) { - return Status::Invalid("UnionArray offsets must be signed int32"); + return Status::TypeError("UnionArray offsets must be signed int32"); } if (type_ids.type_id() != Type::INT8) { - return Status::Invalid("UnionArray type_ids must be signed int8"); + return Status::TypeError("UnionArray type_ids must be signed int8"); } if (value_offsets.null_count() != 0) { @@ -640,7 +643,7 @@ Status UnionArray::MakeSparse(const Array& type_ids, const std::vector<uint8_t>& type_codes, std::shared_ptr<Array>* out) { if (type_ids.type_id() != Type::INT8) { - return Status::Invalid("UnionArray type_ids must be signed int8"); + return Status::TypeError("UnionArray type_ids must be signed int8"); } if (field_names.size() > 0 && field_names.size() != children.size()) { @@ -681,7 +684,6 @@ std::shared_ptr<Array> UnionArray::child(int i) const { } boxed_fields_[i] = MakeArray(child_data); } - DCHECK(boxed_fields_[i]); return boxed_fields_[i]; } @@ -689,7 +691,6 @@ const Array* UnionArray::UnsafeChild(int i) const { if (!boxed_fields_[i]) { boxed_fields_[i] = MakeArray(data_->child_data[i]); } - DCHECK(boxed_fields_[i]); return boxed_fields_[i].get(); } @@ -733,8 +734,8 @@ std::shared_ptr<Array> DictionaryArray::indices() const { return indices_; } DictionaryArray::DictionaryArray(const std::shared_ptr<ArrayData>& data) : dict_type_(checked_cast<const DictionaryType*>(data->type.get())) { - DCHECK_EQ(data->type->id(), Type::DICTIONARY); - DCHECK(data->dictionary); + ARROW_CHECK_EQ(data->type->id(), Type::DICTIONARY); + ARROW_CHECK_NE(data->dictionary, nullptr); SetData(data); } @@ -749,10 +750,10 @@ DictionaryArray::DictionaryArray(const std::shared_ptr<DataType>& type, const std::shared_ptr<Array>& indices, const std::shared_ptr<Array>& dictionary) : dict_type_(checked_cast<const DictionaryType*>(type.get())) { - DCHECK_EQ(type->id(), Type::DICTIONARY); + ARROW_CHECK_EQ(type->id(), Type::DICTIONARY); + ARROW_CHECK_EQ(indices->type_id(), dict_type_->index_type()->id()); + ARROW_CHECK_EQ(dict_type_->value_type()->id(), dictionary->type()->id()); DCHECK(dict_type_->value_type()->Equals(*dictionary->type())); - DCHECK_EQ(indices->type_id(), dict_type_->index_type()->id()); - DCHECK(dictionary); auto data = indices->data()->Copy(); data->type = type; data->dictionary = dictionary; @@ -765,9 +766,11 @@ Status DictionaryArray::FromArrays(const std::shared_ptr<DataType>& type, const std::shared_ptr<Array>& indices, const std::shared_ptr<Array>& dictionary, std::shared_ptr<Array>* out) { - DCHECK_EQ(type->id(), Type::DICTIONARY); + if (type->id() != Type::DICTIONARY) { + return Status::TypeError("Expected a dictionary type"); + } const auto& dict = checked_cast<const DictionaryType&>(*type); - DCHECK_EQ(indices->type_id(), dict.index_type()->id()); + ARROW_CHECK_EQ(indices->type_id(), dict.index_type()->id()); int64_t upper_bound = dictionary->length(); Status is_valid; @@ -786,16 +789,12 @@ Status DictionaryArray::FromArrays(const std::shared_ptr<DataType>& type, is_valid = ValidateDictionaryIndices<Int64Type>(indices, upper_bound); break; default: - return Status::NotImplemented("Categorical index type not supported: ", + return Status::NotImplemented("Dictionary index type not supported: ", indices->type()->ToString()); } - - if (!is_valid.ok()) { - return is_valid; - } - + RETURN_NOT_OK(is_valid); *out = std::make_shared<DictionaryArray>(type, indices, dictionary); - return is_valid; + return Status::OK(); } template <typename InType, typename OutType> @@ -816,7 +815,9 @@ Status DictionaryArray::Transpose(MemoryPool* pool, const std::shared_ptr<DataTy const std::shared_ptr<Array>& dictionary, const std::vector<int32_t>& transpose_map, std::shared_ptr<Array>* out) const { - DCHECK_EQ(type->id(), Type::DICTIONARY); + if (type->id() != Type::DICTIONARY) { + return Status::TypeError("Expected dictionary type"); + } const auto& out_dict_type = checked_cast<const DictionaryType&>(*type); const auto& out_index_type = @@ -1378,7 +1379,8 @@ class NullArrayFactory { : type_(*type), length_(length), buffer_length_(BitUtil::BytesForBits(length)) {} operator int64_t() && { - DCHECK_OK(VisitTypeInline(type_, this)); + // TODO this should implement proper error propagation + ARROW_CHECK_OK(VisitTypeInline(type_, this)); return buffer_length_; } @@ -1485,7 +1487,7 @@ class NullArrayFactory { Status Visit(const StructType& type) { for (int i = 0; i < type_->num_children(); ++i) { - DCHECK_OK(CreateChild(i, length_, &(*out_)->child_data[i])); + RETURN_NOT_OK(CreateChild(i, length_, &(*out_)->child_data[i])); } return Status::OK(); } @@ -1498,7 +1500,7 @@ class NullArrayFactory { } for (int i = 0; i < type_->num_children(); ++i) { - DCHECK_OK(CreateChild(i, length_, &(*out_)->child_data[i])); + RETURN_NOT_OK(CreateChild(i, length_, &(*out_)->child_data[i])); } return Status::OK(); } diff --git a/cpp/src/arrow/array/builder_adaptive.cc b/cpp/src/arrow/array/builder_adaptive.cc index e96c9a2..b88c9d3 100644 --- a/cpp/src/arrow/array/builder_adaptive.cc +++ b/cpp/src/arrow/array/builder_adaptive.cc @@ -86,7 +86,6 @@ Status AdaptiveIntBuilder::FinishInternal(std::shared_ptr<ArrayData>* out) { output_type = int64(); break; default: - DCHECK(false); return Status::NotImplemented("Only ints of size 1,2,4,8 are supported"); } @@ -272,7 +271,6 @@ Status AdaptiveUIntBuilder::FinishInternal(std::shared_ptr<ArrayData>* out) { output_type = uint64(); break; default: - DCHECK(false); return Status::NotImplemented("Only ints of size 1,2,4,8 are supported"); } diff --git a/cpp/src/arrow/array/builder_binary.cc b/cpp/src/arrow/array/builder_binary.cc index ccb79a1..818ad15 100644 --- a/cpp/src/arrow/array/builder_binary.cc +++ b/cpp/src/arrow/array/builder_binary.cc @@ -49,7 +49,11 @@ BinaryBuilder::BinaryBuilder(const std::shared_ptr<DataType>& type, MemoryPool* BinaryBuilder::BinaryBuilder(MemoryPool* pool) : BinaryBuilder(binary(), pool) {} Status BinaryBuilder::Resize(int64_t capacity) { - DCHECK_LE(capacity, kListMaximumElements); + if (capacity > kListMaximumElements) { + return Status::CapacityError( + "BinaryBuilder cannot reserve space for more then 2^31 - 1 child elements, got ", + capacity); + } RETURN_NOT_OK(CheckCapacity(capacity, capacity_)); // one more then requested for offsets diff --git a/cpp/src/arrow/array/builder_dict.cc b/cpp/src/arrow/array/builder_dict.cc index 648b6ff..8d5814a 100644 --- a/cpp/src/arrow/array/builder_dict.cc +++ b/cpp/src/arrow/array/builder_dict.cc @@ -107,7 +107,9 @@ Status DictionaryType::Unify(MemoryPool* pool, const std::vector<const DataType* return Status::Invalid("need at least one input type"); } - DCHECK_EQ(types.size(), dictionaries.size()); + if (types.size() != dictionaries.size()) { + return Status::Invalid("expecting the same number of types and dictionaries"); + } std::vector<const DictionaryType*> dict_types; dict_types.reserve(types.size()); diff --git a/cpp/src/arrow/array/builder_nested.cc b/cpp/src/arrow/array/builder_nested.cc index 309cd2a..0bf9171 100644 --- a/cpp/src/arrow/array/builder_nested.cc +++ b/cpp/src/arrow/array/builder_nested.cc @@ -89,7 +89,11 @@ Status ListBuilder::AppendNulls(int64_t length) { } Status ListBuilder::Resize(int64_t capacity) { - DCHECK_LE(capacity, kListMaximumElements); + if (capacity > kListMaximumElements) { + return Status::CapacityError( + "ListArray cannot reserve space for more then 2^31 - 1 child elements, got ", + capacity); + } RETURN_NOT_OK(CheckCapacity(capacity, capacity_)); // one more then requested for offsets @@ -169,7 +173,8 @@ void MapBuilder::Reset() { } Status MapBuilder::FinishInternal(std::shared_ptr<ArrayData>* out) { - DCHECK_EQ(item_builder_->length(), key_builder_->length()); + ARROW_CHECK_EQ(item_builder_->length(), key_builder_->length()) + << "keys and items builders don't have the same size in MapBuilder"; // finish list(keys) builder RETURN_NOT_OK(list_builder_->FinishInternal(out)); // finish values builder diff --git a/cpp/src/arrow/array/builder_primitive.cc b/cpp/src/arrow/array/builder_primitive.cc index 3c899c0..34d198e 100644 --- a/cpp/src/arrow/array/builder_primitive.cc +++ b/cpp/src/arrow/array/builder_primitive.cc @@ -49,7 +49,7 @@ BooleanBuilder::BooleanBuilder(MemoryPool* pool) BooleanBuilder::BooleanBuilder(const std::shared_ptr<DataType>& type, MemoryPool* pool) : BooleanBuilder(pool) { - DCHECK_EQ(Type::BOOL, type->id()); + ARROW_CHECK_EQ(Type::BOOL, type->id()); } void BooleanBuilder::Reset() { diff --git a/cpp/src/arrow/buffer.cc b/cpp/src/arrow/buffer.cc index 589d93d..510b9cf 100644 --- a/cpp/src/arrow/buffer.cc +++ b/cpp/src/arrow/buffer.cc @@ -32,8 +32,8 @@ namespace arrow { Status Buffer::Copy(const int64_t start, const int64_t nbytes, MemoryPool* pool, std::shared_ptr<Buffer>* out) const { // Sanity checks - DCHECK_LT(start, size_); - DCHECK_LE(nbytes, size_ - start); + ARROW_CHECK_LT(start, size_); + ARROW_CHECK_LE(nbytes, size_ - start); std::shared_ptr<ResizableBuffer> new_buffer; RETURN_NOT_OK(AllocateResizableBuffer(pool, nbytes, &new_buffer)); diff --git a/cpp/src/arrow/compute/kernels/util-internal.cc b/cpp/src/arrow/compute/kernels/util-internal.cc index f29badf..8b1f803 100644 --- a/cpp/src/arrow/compute/kernels/util-internal.cc +++ b/cpp/src/arrow/compute/kernels/util-internal.cc @@ -58,7 +58,7 @@ Status AllocateValueBuffer(FunctionContext* ctx, const DataType& type, int64_t l if (bit_width == 1) { buffer_size = BitUtil::BytesForBits(length); } else { - DCHECK_EQ(bit_width % 8, 0) + ARROW_CHECK_EQ(bit_width % 8, 0) << "Only bit widths with multiple of 8 are currently supported"; buffer_size = length * fw_type.bit_width() / 8; } @@ -182,7 +182,7 @@ Datum WrapArraysLike(const Datum& value, } else if (value.kind() == Datum::CHUNKED_ARRAY) { return Datum(std::make_shared<ChunkedArray>(arrays)); } else { - DCHECK(false) << "unhandled datum kind"; + ARROW_LOG(FATAL) << "unhandled datum kind"; return Datum(); } } @@ -200,7 +200,7 @@ Datum WrapDatumsLike(const Datum& value, const std::vector<Datum>& datums) { } return Datum(std::make_shared<ChunkedArray>(arrays)); } else { - DCHECK(false) << "unhandled datum kind"; + ARROW_LOG(FATAL) << "unhandled datum kind"; return Datum(); } } diff --git a/cpp/src/arrow/extension_type.cc b/cpp/src/arrow/extension_type.cc index 25945f3..b08f974 100644 --- a/cpp/src/arrow/extension_type.cc +++ b/cpp/src/arrow/extension_type.cc @@ -49,8 +49,8 @@ ExtensionArray::ExtensionArray(const std::shared_ptr<ArrayData>& data) { SetData ExtensionArray::ExtensionArray(const std::shared_ptr<DataType>& type, const std::shared_ptr<Array>& storage) { - DCHECK_EQ(type->id(), Type::EXTENSION); - DCHECK( + ARROW_CHECK_EQ(type->id(), Type::EXTENSION); + ARROW_CHECK( storage->type()->Equals(*checked_cast<const ExtensionType&>(*type).storage_type())); auto data = storage->data()->Copy(); // XXX This pointer is reverted below in SetData()... @@ -59,7 +59,7 @@ ExtensionArray::ExtensionArray(const std::shared_ptr<DataType>& type, } void ExtensionArray::SetData(const std::shared_ptr<ArrayData>& data) { - DCHECK_EQ(data->type->id(), Type::EXTENSION); + ARROW_CHECK_EQ(data->type->id(), Type::EXTENSION); this->Array::SetData(data); auto storage_data = data->Copy(); diff --git a/cpp/src/arrow/flight/client.cc b/cpp/src/arrow/flight/client.cc index 0e0d82c..c508dca 100644 --- a/cpp/src/arrow/flight/client.cc +++ b/cpp/src/arrow/flight/client.cc @@ -295,7 +295,9 @@ class DoPutPayloadWriter : public ipc::internal::IpcPayloadWriter { if (first_payload_) { // First Flight message needs to encore the Flight descriptor - DCHECK_EQ(ipc_payload.type, ipc::Message::SCHEMA); + if (ipc_payload.type != ipc::Message::SCHEMA) { + return Status::Invalid("First IPC message should be schema"); + } std::string str_descr; { pb::FlightDescriptor pb_descr; diff --git a/cpp/src/arrow/flight/serialization-internal.cc b/cpp/src/arrow/flight/serialization-internal.cc index 0ff5aba..74b1b20 100644 --- a/cpp/src/arrow/flight/serialization-internal.cc +++ b/cpp/src/arrow/flight/serialization-internal.cc @@ -150,7 +150,9 @@ grpc::Status FlightDataSerialize(const FlightPayload& msg, ByteBuffer* out, // Write the descriptor if present int32_t descriptor_size = 0; if (msg.descriptor != nullptr) { - DCHECK_LT(msg.descriptor->size(), kInt32Max); + if (msg.descriptor->size() > kInt32Max) { + return ToGrpcStatus(Status::CapacityError("Descriptor size overflow (>= 2**31)")); + } descriptor_size = static_cast<int32_t>(msg.descriptor->size()); header_size += 1 + WireFormatLite::LengthDelimitedSize(descriptor_size); } diff --git a/cpp/src/arrow/gpu/cuda_memory.cc b/cpp/src/arrow/gpu/cuda_memory.cc index c777bf7..522ebe2 100644 --- a/cpp/src/arrow/gpu/cuda_memory.cc +++ b/cpp/src/arrow/gpu/cuda_memory.cc @@ -99,7 +99,7 @@ CudaBuffer::CudaBuffer(uint8_t* data, int64_t size, mutable_data_ = data; } -CudaBuffer::~CudaBuffer() { DCHECK_OK(Close()); } +CudaBuffer::~CudaBuffer() { ARROW_CHECK_OK(Close()); } Status CudaBuffer::Close() { if (own_data_) { @@ -153,20 +153,26 @@ Status CudaBuffer::CopyToHost(const int64_t position, const int64_t nbytes, Status CudaBuffer::CopyFromHost(const int64_t position, const void* data, int64_t nbytes) { - DCHECK_LE(nbytes, size_ - position) << "Copy would overflow buffer"; + if (nbytes > size_ - position) { + return Status::Invalid("Copy would overflow buffer"); + } return context_->CopyHostToDevice(mutable_data_ + position, data, nbytes); } Status CudaBuffer::CopyFromDevice(const int64_t position, const void* data, int64_t nbytes) { - DCHECK_LE(nbytes, size_ - position) << "Copy would overflow buffer"; + if (nbytes > size_ - position) { + return Status::Invalid("Copy would overflow buffer"); + } return context_->CopyDeviceToDevice(mutable_data_ + position, data, nbytes); } Status CudaBuffer::CopyFromAnotherDevice(const std::shared_ptr<CudaContext>& src_ctx, const int64_t position, const void* data, int64_t nbytes) { - DCHECK_LE(nbytes, size_ - position) << "Copy would overflow buffer"; + if (nbytes > size_ - position) { + return Status::Invalid("Copy would overflow buffer"); + } return src_ctx->CopyDeviceToAnotherDevice(context_, mutable_data_ + position, data, nbytes); } @@ -182,8 +188,8 @@ Status CudaBuffer::ExportForIpc(std::shared_ptr<CudaIpcMemHandle>* handle) { CudaHostBuffer::~CudaHostBuffer() { CudaDeviceManager* manager = nullptr; - DCHECK_OK(CudaDeviceManager::GetInstance(&manager)); - DCHECK_OK(manager->FreeHost(mutable_data_, size_)); + ARROW_CHECK_OK(CudaDeviceManager::GetInstance(&manager)); + ARROW_CHECK_OK(manager->FreeHost(mutable_data_, size_)); } // ---------------------------------------------------------------------- @@ -225,7 +231,7 @@ class CudaBufferWriter::CudaBufferWriterImpl { buffer_size_(0), buffer_position_(0) { buffer_ = buffer; - DCHECK(buffer->is_mutable()) << "Must pass mutable buffer"; + ARROW_CHECK(buffer->is_mutable()) << "Must pass mutable buffer"; mutable_data_ = buffer->mutable_data(); size_ = buffer->size(); position_ = 0; diff --git a/cpp/src/arrow/io/buffered.cc b/cpp/src/arrow/io/buffered.cc index ed3cd4e..ad8260c 100644 --- a/cpp/src/arrow/io/buffered.cc +++ b/cpp/src/arrow/io/buffered.cc @@ -160,7 +160,9 @@ class BufferedOutputStream::Impl : public BufferedBase { Status SetBufferSize(int64_t new_buffer_size) { std::lock_guard<std::mutex> guard(lock_); - DCHECK_GT(new_buffer_size, 0); + if (new_buffer_size <= 0) { + return Status::Invalid("Buffer size should be positive"); + } if (buffer_pos_ >= new_buffer_size) { // If the buffer is shrinking, first flush to the raw OutputStream RETURN_NOT_OK(FlushUnlocked()); @@ -189,7 +191,7 @@ Status BufferedOutputStream::Create(int64_t buffer_size, MemoryPool* pool, return Status::OK(); } -BufferedOutputStream::~BufferedOutputStream() { DCHECK_OK(impl_->Close()); } +BufferedOutputStream::~BufferedOutputStream() { ARROW_CHECK_OK(impl_->Close()); } Status BufferedOutputStream::SetBufferSize(int64_t new_buffer_size) { return impl_->SetBufferSize(new_buffer_size); @@ -229,7 +231,7 @@ class BufferedInputStream::Impl : public BufferedBase { raw_read_bound_(raw_total_bytes_bound), bytes_buffered_(0) {} - ~Impl() { DCHECK_OK(Close()); } + ~Impl() { ARROW_CHECK_OK(Close()); } Status Close() { std::lock_guard<std::mutex> guard(lock_); @@ -253,7 +255,9 @@ class BufferedInputStream::Impl : public BufferedBase { Status SetBufferSize(int64_t new_buffer_size) { std::lock_guard<std::mutex> guard(lock_); - DCHECK_GT(new_buffer_size, 0); + if (new_buffer_size <= 0) { + return Status::Invalid("Buffer size should be positive"); + } if ((buffer_pos_ + bytes_buffered_) >= new_buffer_size) { return Status::Invalid("Cannot shrink read buffer if buffered data remains"); } @@ -339,7 +343,7 @@ class BufferedInputStream::Impl : public BufferedBase { Status Read(int64_t nbytes, int64_t* bytes_read, void* out) { std::lock_guard<std::mutex> guard(lock_); - DCHECK_GT(nbytes, 0); + ARROW_CHECK_GT(nbytes, 0); if (nbytes < buffer_size_) { // Pre-buffer for small reads @@ -408,7 +412,7 @@ BufferedInputStream::BufferedInputStream(std::shared_ptr<InputStream> raw, impl_.reset(new Impl(std::move(raw), pool, raw_total_bytes_bound)); } -BufferedInputStream::~BufferedInputStream() { DCHECK_OK(impl_->Close()); } +BufferedInputStream::~BufferedInputStream() { ARROW_CHECK_OK(impl_->Close()); } Status BufferedInputStream::Create(int64_t buffer_size, MemoryPool* pool, std::shared_ptr<InputStream> raw, diff --git a/cpp/src/arrow/io/compressed.cc b/cpp/src/arrow/io/compressed.cc index 301ebc3..43de132 100644 --- a/cpp/src/arrow/io/compressed.cc +++ b/cpp/src/arrow/io/compressed.cc @@ -46,7 +46,7 @@ class CompressedOutputStream::Impl { Impl(MemoryPool* pool, Codec* codec, const std::shared_ptr<OutputStream>& raw) : pool_(pool), raw_(raw), codec_(codec), is_open_(true), compressed_pos_(0) {} - ~Impl() { DCHECK_OK(Close()); } + ~Impl() { ARROW_CHECK_OK(Close()); } Status Init() { RETURN_NOT_OK(codec_->MakeCompressor(&compressor_)); @@ -241,7 +241,7 @@ class CompressedInputStream::Impl { return Status::OK(); } - ~Impl() { DCHECK_OK(Close()); } + ~Impl() { ARROW_CHECK_OK(Close()); } Status Close() { std::lock_guard<std::mutex> guard(lock_); diff --git a/cpp/src/arrow/io/file.cc b/cpp/src/arrow/io/file.cc index 2b7d1a5..d5a48df 100644 --- a/cpp/src/arrow/io/file.cc +++ b/cpp/src/arrow/io/file.cc @@ -264,7 +264,7 @@ class ReadableFile::ReadableFileImpl : public OSFile { ReadableFile::ReadableFile(MemoryPool* pool) { impl_.reset(new ReadableFileImpl(pool)); } -ReadableFile::~ReadableFile() { DCHECK_OK(impl_->Close()); } +ReadableFile::~ReadableFile() { ARROW_CHECK_OK(impl_->Close()); } Status ReadableFile::Open(const std::string& path, std::shared_ptr<ReadableFile>* file) { return Open(path, default_memory_pool(), file); @@ -337,7 +337,7 @@ FileOutputStream::FileOutputStream() { impl_.reset(new FileOutputStreamImpl()); FileOutputStream::~FileOutputStream() { // This can fail; better to explicitly call close - DCHECK_OK(impl_->Close()); + ARROW_CHECK_OK(impl_->Close()); } Status FileOutputStream::Open(const std::string& path, @@ -393,11 +393,10 @@ class MemoryMappedFile::MemoryMap : public MutableBuffer { MemoryMap() : MutableBuffer(nullptr, 0) {} ~MemoryMap() { - DCHECK_OK(Close()); + ARROW_CHECK_OK(Close()); if (mutable_data_ != nullptr) { int result = munmap(mutable_data_, static_cast<size_t>(size_)); - DCHECK_EQ(result, 0); - ARROW_UNUSED(result); + ARROW_CHECK_EQ(result, 0) << "munmap failed"; } } diff --git a/cpp/src/arrow/io/memory.cc b/cpp/src/arrow/io/memory.cc index 22a9f9b..e0013ea 100644 --- a/cpp/src/arrow/io/memory.cc +++ b/cpp/src/arrow/io/memory.cc @@ -65,7 +65,7 @@ Status BufferOutputStream::Reset(int64_t initial_capacity, MemoryPool* pool) { BufferOutputStream::~BufferOutputStream() { // This can fail, better to explicitly call close if (buffer_) { - DCHECK_OK(Close()); + ARROW_CHECK_OK(Close()); } } @@ -159,7 +159,7 @@ class FixedSizeBufferWriter::FixedSizeBufferWriterImpl { memcopy_blocksize_(kMemcopyDefaultBlocksize), memcopy_threshold_(kMemcopyDefaultThreshold) { buffer_ = buffer; - DCHECK(buffer->is_mutable()) << "Must pass mutable buffer"; + ARROW_CHECK(buffer->is_mutable()) << "Must pass mutable buffer"; mutable_data_ = buffer->mutable_data(); size_ = buffer->size(); position_ = 0; diff --git a/cpp/src/arrow/ipc/message.cc b/cpp/src/arrow/ipc/message.cc index 1de2a26..612b1a6 100644 --- a/cpp/src/arrow/ipc/message.cc +++ b/cpp/src/arrow/ipc/message.cc @@ -226,7 +226,8 @@ std::string FormatMessageType(Message::Type type) { Status ReadMessage(int64_t offset, int32_t metadata_length, io::RandomAccessFile* file, std::unique_ptr<Message>* message) { - DCHECK_GT(static_cast<size_t>(metadata_length), sizeof(int32_t)); + ARROW_CHECK_GT(static_cast<size_t>(metadata_length), sizeof(int32_t)) + << "metadata_length should be at least 4"; std::shared_ptr<Buffer> buffer; RETURN_NOT_OK(file->ReadAt(offset, metadata_length, &buffer)); diff --git a/cpp/src/arrow/ipc/reader.cc b/cpp/src/arrow/ipc/reader.cc index 992c4fc..002379e 100644 --- a/cpp/src/arrow/ipc/reader.cc +++ b/cpp/src/arrow/ipc/reader.cc @@ -104,9 +104,11 @@ class IpcComponentSource { *out = nullptr; return Status::OK(); } else { - DCHECK(BitUtil::IsMultipleOf8(buffer->offset())) - << "Buffer " << buffer_index - << " did not start on 8-byte aligned offset: " << buffer->offset(); + if (!BitUtil::IsMultipleOf8(buffer->offset())) { + return Status::Invalid( + "Buffer ", buffer_index, + " did not start on 8-byte aligned offset: ", buffer->offset()); + } return file_->ReadAt(buffer->offset(), buffer->length(), out); } } @@ -357,7 +359,9 @@ static Status LoadRecordBatchFromSource(const std::shared_ptr<Schema>& schema, for (int i = 0; i < schema->num_fields(); ++i) { auto arr = std::make_shared<ArrayData>(); RETURN_NOT_OK(LoadArray(*schema->field(i), &context, arr.get())); - DCHECK_EQ(num_rows, arr->length) << "Array length did not match record batch length"; + if (num_rows != arr->length) { + return Status::IOError("Array length did not match record batch length"); + } arrays[i] = std::move(arr); } @@ -893,9 +897,11 @@ Status ReadSparseTensor(const Buffer& metadata, io::RandomAccessFile* file, "Header-type of flatbuffer-encoded Message is not SparseTensor."); } const flatbuf::Buffer* buffer = sparse_tensor->data(); - DCHECK(BitUtil::IsMultipleOf8(buffer->offset())) - << "Buffer of sparse index data " - << "did not start on 8-byte aligned offset: " << buffer->offset(); + if (!BitUtil::IsMultipleOf8(buffer->offset())) { + return Status::Invalid( + "Buffer of sparse index data did not start on 8-byte aligned offset: ", + buffer->offset()); + } std::shared_ptr<Buffer> data; RETURN_NOT_OK(file->ReadAt(buffer->offset(), buffer->length(), &data)); diff --git a/cpp/src/arrow/python/arrow_to_pandas.cc b/cpp/src/arrow/python/arrow_to_pandas.cc index d992001..59bdb17 100644 --- a/cpp/src/arrow/python/arrow_to_pandas.cc +++ b/cpp/src/arrow/python/arrow_to_pandas.cc @@ -688,7 +688,7 @@ static Status ConvertDecimals(const PandasOptions& options, const ChunkedArray& OwnedRef decimal; OwnedRef Decimal; RETURN_NOT_OK(internal::ImportModule("decimal", &decimal)); - RETURN_NOT_OK(internal::ImportFromModule(decimal, "Decimal", &Decimal)); + RETURN_NOT_OK(internal::ImportFromModule(decimal.obj(), "Decimal", &Decimal)); PyObject* decimal_constructor = Decimal.obj(); for (int c = 0; c < data.num_chunks(); c++) { diff --git a/cpp/src/arrow/python/common.cc b/cpp/src/arrow/python/common.cc index 8f844e6..aa44ec0 100644 --- a/cpp/src/arrow/python/common.cc +++ b/cpp/src/arrow/python/common.cc @@ -55,7 +55,7 @@ PyBuffer::PyBuffer() : Buffer(nullptr, 0) {} Status PyBuffer::Init(PyObject* obj) { if (!PyObject_GetBuffer(obj, &py_buf_, PyBUF_ANY_CONTIGUOUS)) { data_ = reinterpret_cast<const uint8_t*>(py_buf_.buf); - DCHECK(data_ != nullptr); + ARROW_CHECK_NE(data_, nullptr) << "Null pointer in Py_buffer"; size_ = py_buf_.len; capacity_ = py_buf_.len; is_mutable_ = !py_buf_.readonly; @@ -94,7 +94,7 @@ Status ConvertPyError(StatusCode code) { PyErr_Fetch(&exc_type, &exc_value, &traceback); PyErr_NormalizeException(&exc_type, &exc_value, &traceback); - DCHECK_NE(exc_type, nullptr); + DCHECK_NE(exc_type, nullptr) << "ConvertPyError called without an exception set"; OwnedRef exc_type_ref(exc_type); OwnedRef exc_value_ref(exc_value); diff --git a/cpp/src/arrow/python/decimal.cc b/cpp/src/arrow/python/decimal.cc index 4b6932d..a1ba3d5 100644 --- a/cpp/src/arrow/python/decimal.cc +++ b/cpp/src/arrow/python/decimal.cc @@ -33,7 +33,7 @@ namespace internal { Status ImportDecimalType(OwnedRef* decimal_type) { OwnedRef decimal_module; RETURN_NOT_OK(ImportModule("decimal", &decimal_module)); - RETURN_NOT_OK(ImportFromModule(decimal_module, "Decimal", decimal_type)); + RETURN_NOT_OK(ImportFromModule(decimal_module.obj(), "Decimal", decimal_type)); return Status::OK(); } @@ -166,14 +166,13 @@ Status DecimalFromPyObject(PyObject* obj, const DecimalType& arrow_type, bool PyDecimal_Check(PyObject* obj) { static OwnedRef decimal_type; if (!decimal_type.obj()) { - Status status = ImportDecimalType(&decimal_type); - DCHECK_OK(status); + ARROW_CHECK_OK(ImportDecimalType(&decimal_type)); DCHECK(PyType_Check(decimal_type.obj())); } // PyObject_IsInstance() is slower as it has to check for virtual subclasses const int result = PyType_IsSubtype(Py_TYPE(obj), reinterpret_cast<PyTypeObject*>(decimal_type.obj())); - DCHECK_NE(result, -1) << " error during PyType_IsSubtype check"; + ARROW_CHECK_NE(result, -1) << " error during PyType_IsSubtype check"; return result == 1; } @@ -209,7 +208,6 @@ Status DecimalMetadata::Update(int32_t suggested_precision, int32_t suggested_sc Status DecimalMetadata::Update(PyObject* object) { bool is_decimal = PyDecimal_Check(object); - DCHECK(is_decimal) << "Object is not a Python Decimal"; if (ARROW_PREDICT_FALSE(!is_decimal || PyDecimal_ISNAN(object))) { return Status::OK(); diff --git a/cpp/src/arrow/python/helpers.cc b/cpp/src/arrow/python/helpers.cc index ab36036..e44878f 100644 --- a/cpp/src/arrow/python/helpers.cc +++ b/cpp/src/arrow/python/helpers.cc @@ -144,18 +144,13 @@ Status PyObject_StdStringStr(PyObject* obj, std::string* out) { Status ImportModule(const std::string& module_name, OwnedRef* ref) { PyObject* module = PyImport_ImportModule(module_name.c_str()); RETURN_IF_PYERROR(); - DCHECK_NE(module, nullptr) << "unable to import the " << module_name << " module"; ref->reset(module); return Status::OK(); } -Status ImportFromModule(const OwnedRef& module, const std::string& name, OwnedRef* ref) { - /// Assumes that ImportModule was called first - DCHECK_NE(module.obj(), nullptr) << "Cannot import from nullptr Python module"; - - PyObject* attr = PyObject_GetAttrString(module.obj(), name.c_str()); +Status ImportFromModule(PyObject* module, const std::string& name, OwnedRef* ref) { + PyObject* attr = PyObject_GetAttrString(module, name.c_str()); RETURN_IF_PYERROR(); - DCHECK_NE(attr, nullptr) << "unable to import the " << name << " object"; ref->reset(attr); return Status::OK(); } diff --git a/cpp/src/arrow/python/helpers.h b/cpp/src/arrow/python/helpers.h index 47b7810..4917f99 100644 --- a/cpp/src/arrow/python/helpers.h +++ b/cpp/src/arrow/python/helpers.h @@ -62,7 +62,7 @@ Status ImportModule(const std::string& module_name, OwnedRef* ref); // \param[out] ref The OwnedRef containing the \c name attribute of the Python module \c // module ARROW_PYTHON_EXPORT -Status ImportFromModule(const OwnedRef& module, const std::string& name, OwnedRef* ref); +Status ImportFromModule(PyObject* module, const std::string& name, OwnedRef* ref); // \brief Check whether obj is an integer, independent of Python versions. inline bool IsPyInteger(PyObject* obj) { diff --git a/cpp/src/arrow/python/inference.cc b/cpp/src/arrow/python/inference.cc index 9d2c35f..eddba0e 100644 --- a/cpp/src/arrow/python/inference.cc +++ b/cpp/src/arrow/python/inference.cc @@ -309,8 +309,7 @@ class TypeInferrer { max_decimal_metadata_(std::numeric_limits<int32_t>::min(), std::numeric_limits<int32_t>::min()), decimal_type_() { - Status status = internal::ImportDecimalType(&decimal_type_); - DCHECK_OK(status); + ARROW_CHECK_OK(internal::ImportDecimalType(&decimal_type_)); } /// \param[in] obj a Python object in the sequence diff --git a/cpp/src/arrow/python/python-test.cc b/cpp/src/arrow/python/python-test.cc index 03e4970..a8282a5 100644 --- a/cpp/src/arrow/python/python-test.cc +++ b/cpp/src/arrow/python/python-test.cc @@ -134,10 +134,11 @@ class DecimalTest : public ::testing::Test { OwnedRef decimal_module; Status status = internal::ImportModule("decimal", &decimal_module); - DCHECK_OK(status); + ARROW_CHECK_OK(status); - status = internal::ImportFromModule(decimal_module, "Decimal", &decimal_constructor_); - DCHECK_OK(status); + status = internal::ImportFromModule(decimal_module.obj(), "Decimal", + &decimal_constructor_); + ARROW_CHECK_OK(status); } OwnedRef CreatePythonDecimal(const std::string& string_value) { @@ -239,13 +240,15 @@ TEST(PandasConversionTest, TestObjectBlockWriteFails) { auto schema = ::arrow::schema(fields); auto table = Table::Make(schema, cols); - PyObject* out; + Status st; Py_BEGIN_ALLOW_THREADS; + PyObject* out; PandasOptions options; options.use_threads = true; MemoryPool* pool = default_memory_pool(); - ASSERT_RAISES(UnknownError, ConvertTableToPandas(options, table, pool, &out)); + st = ConvertTableToPandas(options, table, pool, &out); Py_END_ALLOW_THREADS; + ASSERT_RAISES(UnknownError, st); } TEST(BuiltinConversionTest, TestMixedTypeFails) { diff --git a/cpp/src/arrow/python/type_traits.h b/cpp/src/arrow/python/type_traits.h index bc71ec4..6153802 100644 --- a/cpp/src/arrow/python/type_traits.h +++ b/cpp/src/arrow/python/type_traits.h @@ -291,7 +291,7 @@ static inline int NumPyTypeSize(int npy_type) { case NPY_OBJECT: return sizeof(void*); default: - DCHECK(false) << "unhandled numpy type"; + ARROW_CHECK(false) << "unhandled numpy type"; break; } return -1; diff --git a/cpp/src/arrow/record_batch.cc b/cpp/src/arrow/record_batch.cc index 2bc8c22..1f266df 100644 --- a/cpp/src/arrow/record_batch.cc +++ b/cpp/src/arrow/record_batch.cc @@ -97,8 +97,8 @@ class SimpleRecordBatch : public RecordBatch { Status AddColumn(int i, const std::shared_ptr<Field>& field, const std::shared_ptr<Array>& column, std::shared_ptr<RecordBatch>* out) const override { - DCHECK(field != nullptr); - DCHECK(column != nullptr); + ARROW_CHECK(field != nullptr); + ARROW_CHECK(column != nullptr); if (!field->type()->Equals(column->type())) { return Status::Invalid("Column data type ", field->type()->name(), diff --git a/cpp/src/arrow/scalar.cc b/cpp/src/arrow/scalar.cc index 4bc9b92..2562862 100644 --- a/cpp/src/arrow/scalar.cc +++ b/cpp/src/arrow/scalar.cc @@ -36,26 +36,27 @@ bool Scalar::Equals(const Scalar& other) const { return ScalarEquals(*this, othe Time32Scalar::Time32Scalar(int32_t value, const std::shared_ptr<DataType>& type, bool is_valid) : internal::PrimitiveScalar{type, is_valid}, value(value) { - DCHECK_EQ(Type::TIME32, type->id()); + ARROW_CHECK_EQ(Type::TIME32, type->id()); } Time64Scalar::Time64Scalar(int64_t value, const std::shared_ptr<DataType>& type, bool is_valid) : internal::PrimitiveScalar{type, is_valid}, value(value) { - DCHECK_EQ(Type::TIME64, type->id()); + ARROW_CHECK_EQ(Type::TIME64, type->id()); } TimestampScalar::TimestampScalar(int64_t value, const std::shared_ptr<DataType>& type, bool is_valid) : internal::PrimitiveScalar{type, is_valid}, value(value) { - DCHECK_EQ(Type::TIMESTAMP, type->id()); + ARROW_CHECK_EQ(Type::TIMESTAMP, type->id()); } FixedSizeBinaryScalar::FixedSizeBinaryScalar(const std::shared_ptr<Buffer>& value, const std::shared_ptr<DataType>& type, bool is_valid) : BinaryScalar(value, type, is_valid) { - DCHECK_EQ(checked_cast<const FixedSizeBinaryType&>(*type).byte_width(), value->size()); + ARROW_CHECK_EQ(checked_cast<const FixedSizeBinaryType&>(*type).byte_width(), + value->size()); } Decimal128Scalar::Decimal128Scalar(const Decimal128& value, @@ -82,8 +83,8 @@ FixedSizeListScalar::FixedSizeListScalar(const std::shared_ptr<Array>& value, const std::shared_ptr<DataType>& type, bool is_valid) : Scalar{type, is_valid}, value(value) { - DCHECK_EQ(value->length(), - checked_cast<const FixedSizeListType*>(type.get())->list_size()); + ARROW_CHECK_EQ(value->length(), + checked_cast<const FixedSizeListType*>(type.get())->list_size()); } FixedSizeListScalar::FixedSizeListScalar(const std::shared_ptr<Array>& value, diff --git a/cpp/src/arrow/sparse_tensor.cc b/cpp/src/arrow/sparse_tensor.cc index fc2d538..d90392d 100644 --- a/cpp/src/arrow/sparse_tensor.cc +++ b/cpp/src/arrow/sparse_tensor.cc @@ -319,7 +319,7 @@ void MakeSparseTensorFromTensor(const Tensor& tensor, // Constructor with a column-major NumericTensor SparseCOOIndex::SparseCOOIndex(const std::shared_ptr<CoordsTensor>& coords) : SparseIndexBase(coords->shape()[0]), coords_(coords) { - DCHECK(coords_->is_column_major()); + ARROW_CHECK(coords_->is_column_major()); } std::string SparseCOOIndex::ToString() const { return std::string("SparseCOOIndex"); } @@ -331,8 +331,8 @@ std::string SparseCOOIndex::ToString() const { return std::string("SparseCOOInde SparseCSRIndex::SparseCSRIndex(const std::shared_ptr<IndexTensor>& indptr, const std::shared_ptr<IndexTensor>& indices) : SparseIndexBase(indices->shape()[0]), indptr_(indptr), indices_(indices) { - DCHECK_EQ(1, indptr_->ndim()); - DCHECK_EQ(1, indices_->ndim()); + ARROW_CHECK_EQ(1, indptr_->ndim()); + ARROW_CHECK_EQ(1, indices_->ndim()); } std::string SparseCSRIndex::ToString() const { return std::string("SparseCSRIndex"); } @@ -351,7 +351,7 @@ SparseTensor::SparseTensor(const std::shared_ptr<DataType>& type, shape_(shape), sparse_index_(sparse_index), dim_names_(dim_names) { - DCHECK(is_tensor_supported(type->id())); + ARROW_CHECK(is_tensor_supported(type->id())); } const std::string& SparseTensor::dim_name(int i) const { @@ -359,7 +359,7 @@ const std::string& SparseTensor::dim_name(int i) const { if (dim_names_.size() == 0) { return kEmpty; } else { - DCHECK_LT(i, static_cast<int>(dim_names_.size())); + ARROW_CHECK_LT(i, static_cast<int>(dim_names_.size())); return dim_names_[i]; } } diff --git a/cpp/src/arrow/status.cc b/cpp/src/arrow/status.cc index e97dc8c..cbb2911 100644 --- a/cpp/src/arrow/status.cc +++ b/cpp/src/arrow/status.cc @@ -15,11 +15,14 @@ #include <cassert> #include <cstdlib> #include <iostream> +#include <sstream> + +#include "arrow/util/logging.h" namespace arrow { Status::Status(StatusCode code, const std::string& msg) { - assert(code != StatusCode::OK); + ARROW_CHECK_NE(code, StatusCode::OK) << "Cannot construct ok status with message"; state_ = new State; state_->code = code; state_->msg = msg; @@ -126,4 +129,13 @@ void Status::Abort(const std::string& message) const { std::abort(); } +#ifdef ARROW_EXTRA_ERROR_CONTEXT +void Status::AddContextLine(const char* filename, int line, const char* expr) { + ARROW_CHECK(!ok()) << "Cannot add context line to ok status"; + std::stringstream ss; + ss << "\nIn " << filename << ", line " << line << ", code: " << expr; + state_->msg += ss.str(); +} +#endif + } // namespace arrow diff --git a/cpp/src/arrow/status.h b/cpp/src/arrow/status.h index 790d9b7..1ed0da6 100644 --- a/cpp/src/arrow/status.h +++ b/cpp/src/arrow/status.h @@ -20,10 +20,6 @@ #include <string> #include <utility> -#ifdef ARROW_EXTRA_ERROR_CONTEXT -#include <sstream> -#endif - #include "arrow/util/macros.h" #include "arrow/util/string_builder.h" #include "arrow/util/visibility.h" @@ -31,14 +27,13 @@ #ifdef ARROW_EXTRA_ERROR_CONTEXT /// \brief Return with given status if condition is met. -#define ARROW_RETURN_IF_(condition, status, expr) \ - do { \ - if (ARROW_PREDICT_FALSE(condition)) { \ - ::arrow::Status _s = (status); \ - std::stringstream ss; \ - ss << _s.message() << "\n" << __FILE__ << ":" << __LINE__ << " code: " << expr; \ - return ::arrow::Status(_s.code(), ss.str()); \ - } \ +#define ARROW_RETURN_IF_(condition, status, expr) \ + do { \ + if (ARROW_PREDICT_FALSE(condition)) { \ + ::arrow::Status _st = (status); \ + _st.AddContextLine(__FILE__, __LINE__, expr); \ + return _st; \ + } \ } while (0) #else @@ -338,6 +333,10 @@ class ARROW_EXPORT Status { [[noreturn]] void Abort() const; [[noreturn]] void Abort(const std::string& message) const; +#ifdef ARROW_EXTRA_ERROR_CONTEXT + void AddContextLine(const char* filename, int line, const char* expr); +#endif + private: struct State { StatusCode code; diff --git a/cpp/src/arrow/table.cc b/cpp/src/arrow/table.cc index addb255..5c58adc 100644 --- a/cpp/src/arrow/table.cc +++ b/cpp/src/arrow/table.cc @@ -61,7 +61,7 @@ Status CompactColumn(const std::shared_ptr<Column>& column, MemoryPool* pool, ChunkedArray::ChunkedArray(const ArrayVector& chunks) : chunks_(chunks) { length_ = 0; null_count_ = 0; - DCHECK_GT(chunks.size(), 0) + ARROW_CHECK_GT(chunks.size(), 0) << "cannot construct ChunkedArray from empty vector and omitted type"; type_ = chunks[0]->type(); for (const std::shared_ptr<Array>& chunk : chunks) { @@ -141,7 +141,7 @@ bool ChunkedArray::Equals(const std::shared_ptr<ChunkedArray>& other) const { } std::shared_ptr<ChunkedArray> ChunkedArray::Slice(int64_t offset, int64_t length) const { - DCHECK_LE(offset, length_); + ARROW_CHECK_LE(offset, length_) << "Slice offset greater than array length"; int curr_chunk = 0; while (curr_chunk < num_chunks() && offset >= chunk(curr_chunk)->length()) { diff --git a/cpp/src/arrow/table_builder.cc b/cpp/src/arrow/table_builder.cc index 21d158f..1ef8650 100644 --- a/cpp/src/arrow/table_builder.cc +++ b/cpp/src/arrow/table_builder.cc @@ -75,7 +75,7 @@ Status RecordBatchBuilder::Flush(std::shared_ptr<RecordBatch>* batch) { } void RecordBatchBuilder::SetInitialCapacity(int64_t capacity) { - DCHECK_GT(capacity, 0) << "Initial capacity must be positive"; + ARROW_CHECK_GT(capacity, 0) << "Initial capacity must be positive"; initial_capacity_ = capacity; } diff --git a/cpp/src/arrow/tensor.cc b/cpp/src/arrow/tensor.cc index 743a9bc..575f9a0 100644 --- a/cpp/src/arrow/tensor.cc +++ b/cpp/src/arrow/tensor.cc @@ -78,7 +78,7 @@ Tensor::Tensor(const std::shared_ptr<DataType>& type, const std::shared_ptr<Buff const std::vector<int64_t>& shape, const std::vector<int64_t>& strides, const std::vector<std::string>& dim_names) : type_(type), data_(data), shape_(shape), strides_(strides), dim_names_(dim_names) { - DCHECK(is_tensor_supported(type->id())); + ARROW_CHECK(is_tensor_supported(type->id())); if (shape.size() > 0 && strides.size() == 0) { ComputeRowMajorStrides(checked_cast<const FixedWidthType&>(*type_), shape, &strides_); } @@ -97,7 +97,7 @@ const std::string& Tensor::dim_name(int i) const { if (dim_names_.size() == 0) { return kEmpty; } else { - DCHECK_LT(i, static_cast<int>(dim_names_.size())); + ARROW_CHECK_LT(i, static_cast<int>(dim_names_.size())); return dim_names_[i]; } } @@ -172,7 +172,7 @@ struct NonZeroCounter { template <typename TYPE> typename std::enable_if<!is_number_type<TYPE>::value, Status>::type Visit( const TYPE& type) { - DCHECK(!is_tensor_supported(type.id())); + ARROW_CHECK(!is_tensor_supported(type.id())); return Status::NotImplemented("Tensor of ", type.ToString(), " is not implemented"); } diff --git a/cpp/src/arrow/type.cc b/cpp/src/arrow/type.cc index b3fcb0c..2dbd31a 100644 --- a/cpp/src/arrow/type.cc +++ b/cpp/src/arrow/type.cc @@ -208,7 +208,7 @@ TimeType::TimeType(Type::type type_id, TimeUnit::type unit) : TemporalType(type_id), unit_(unit) {} Time32Type::Time32Type(TimeUnit::type unit) : TimeType(Type::TIME32, unit) { - DCHECK(unit == TimeUnit::SECOND || unit == TimeUnit::MILLI) + ARROW_CHECK(unit == TimeUnit::SECOND || unit == TimeUnit::MILLI) << "Must be seconds or milliseconds"; } @@ -219,7 +219,7 @@ std::string Time32Type::ToString() const { } Time64Type::Time64Type(TimeUnit::type unit) : TimeType(Type::TIME64, unit) { - DCHECK(unit == TimeUnit::MICRO || unit == TimeUnit::NANO) + ARROW_CHECK(unit == TimeUnit::MICRO || unit == TimeUnit::NANO) << "Must be microseconds or nanoseconds"; } @@ -407,8 +407,8 @@ int StructType::GetChildIndex(const std::string& name) const { Decimal128Type::Decimal128Type(int32_t precision, int32_t scale) : DecimalType(16, precision, scale) { - DCHECK_GE(precision, 1); - DCHECK_LE(precision, 38); + ARROW_CHECK_GE(precision, 1); + ARROW_CHECK_LE(precision, 38); } // ---------------------------------------------------------------------- @@ -424,10 +424,10 @@ DictionaryType::DictionaryType(const std::shared_ptr<DataType>& index_type, index_type_(index_type), value_type_(value_type), ordered_(ordered) { -#ifndef NDEBUG + ARROW_CHECK(is_integer(index_type->id())) + << "dictionary index type should be signed integer"; const auto& int_type = checked_cast<const IntegerType&>(*index_type); - DCHECK_EQ(int_type.is_signed(), true) << "dictionary index type should be signed"; -#endif + ARROW_CHECK(int_type.is_signed()) << "dictionary index type should be signed integer"; } DataTypeLayout DictionaryType::layout() const { diff --git a/cpp/src/arrow/util/compression_bz2.cc b/cpp/src/arrow/util/compression_bz2.cc index 407fb82..a26bb3e 100644 --- a/cpp/src/arrow/util/compression_bz2.cc +++ b/cpp/src/arrow/util/compression_bz2.cc @@ -44,8 +44,8 @@ static constexpr auto kSizeLimit = static_cast<int64_t>(std::numeric_limits<unsigned int>::max()); Status BZ2Error(const char* prefix_msg, int bz_result) { - DCHECK(bz_result != BZ_OK && bz_result != BZ_RUN_OK && bz_result != BZ_FLUSH_OK && - bz_result != BZ_FINISH_OK && bz_result != BZ_STREAM_END); + ARROW_CHECK(bz_result != BZ_OK && bz_result != BZ_RUN_OK && bz_result != BZ_FLUSH_OK && + bz_result != BZ_FINISH_OK && bz_result != BZ_STREAM_END); StatusCode code; std::stringstream ss; ss << prefix_msg; diff --git a/cpp/src/arrow/util/compression_zlib.cc b/cpp/src/arrow/util/compression_zlib.cc index 5afd5e3..cd8b912 100644 --- a/cpp/src/arrow/util/compression_zlib.cc +++ b/cpp/src/arrow/util/compression_zlib.cc @@ -131,7 +131,7 @@ class GZipDecompressor : public Decompressor { *bytes_written = 0; *need_more_output = true; } else { - DCHECK(ret == Z_OK || ret == Z_STREAM_END); + ARROW_CHECK(ret == Z_OK || ret == Z_STREAM_END); // Some progress has been made *bytes_read = input_len - stream_.avail_in; *bytes_written = output_len - stream_.avail_out; @@ -224,7 +224,7 @@ Status GZipCompressor::Compress(int64_t input_len, const uint8_t* input, *bytes_written = output_len - stream_.avail_out; } else { // No progress was possible - DCHECK_EQ(ret, Z_BUF_ERROR); + ARROW_CHECK_EQ(ret, Z_BUF_ERROR); *bytes_read = 0; *bytes_written = 0; } @@ -250,7 +250,7 @@ Status GZipCompressor::Flush(int64_t output_len, uint8_t* output, int64_t* bytes if (ret == Z_OK) { *bytes_written = output_len - stream_.avail_out; } else { - DCHECK_EQ(ret, Z_BUF_ERROR); + ARROW_CHECK_EQ(ret, Z_BUF_ERROR); *bytes_written = 0; } // "If deflate returns with avail_out == 0, this function must be called @@ -428,7 +428,7 @@ class GZipCodec::GZipCodecImpl { // Must be in compression mode if (!compressor_initialized_) { Status s = InitCompressor(); - DCHECK_OK(s); + ARROW_CHECK_OK(s); } int64_t max_len = deflateBound(&stream_, static_cast<uLong>(input_length)); // ARROW-3514: return a more pessimistic estimate to account for bugs diff --git a/cpp/src/arrow/util/decimal.cc b/cpp/src/arrow/util/decimal.cc index ec659e3..46928b2 100644 --- a/cpp/src/arrow/util/decimal.cc +++ b/cpp/src/arrow/util/decimal.cc @@ -42,7 +42,7 @@ using internal::SafeSignedAdd; Decimal128::Decimal128(const std::string& str) : Decimal128() { Status status = Decimal128::FromString(str, this); - DCHECK_OK(status); + ARROW_CHECK_OK(status); } static const Decimal128 kTenTo36(static_cast<int64_t>(0xC097CE7BC90715), @@ -56,7 +56,7 @@ std::string Decimal128::ToIntegerString() const { // get anything above 10 ** 36 and print it Decimal128 top; - DCHECK_OK(Divide(kTenTo36, &top, &remainder)); + ARROW_CHECK_OK(Divide(kTenTo36, &top, &remainder)); if (top != 0) { buf << static_cast<int64_t>(top); diff --git a/cpp/src/arrow/util/io-util.cc b/cpp/src/arrow/util/io-util.cc index 1879110..87f64b7 100644 --- a/cpp/src/arrow/util/io-util.cc +++ b/cpp/src/arrow/util/io-util.cc @@ -866,7 +866,12 @@ Status DelEnvVar(const std::string& name) { return DelEnvVar(name.c_str()); } TemporaryDir::TemporaryDir(PlatformFilename&& path) : path_(std::move(path)) {} -TemporaryDir::~TemporaryDir() { DCHECK_OK(DeleteDirTree(path_)); } +TemporaryDir::~TemporaryDir() { + Status st = DeleteDirTree(path_); + if (!st.ok()) { + ARROW_LOG(WARNING) << "When trying to delete temporary directory: " << st; + } +} Status TemporaryDir::Make(const std::string& prefix, std::unique_ptr<TemporaryDir>* out) { bfs::path path; diff --git a/cpp/src/arrow/util/key_value_metadata.cc b/cpp/src/arrow/util/key_value_metadata.cc index 0b22403..1c73e30 100644 --- a/cpp/src/arrow/util/key_value_metadata.cc +++ b/cpp/src/arrow/util/key_value_metadata.cc @@ -57,13 +57,13 @@ KeyValueMetadata::KeyValueMetadata() : keys_(), values_() {} KeyValueMetadata::KeyValueMetadata( const std::unordered_map<std::string, std::string>& map) : keys_(UnorderedMapKeys(map)), values_(UnorderedMapValues(map)) { - DCHECK_EQ(keys_.size(), values_.size()); + ARROW_CHECK_EQ(keys_.size(), values_.size()); } KeyValueMetadata::KeyValueMetadata(const std::vector<std::string>& keys, const std::vector<std::string>& values) : keys_(keys), values_(values) { - DCHECK_EQ(keys.size(), values.size()); + ARROW_CHECK_EQ(keys.size(), values.size()); } void KeyValueMetadata::ToUnorderedMap( diff --git a/cpp/src/arrow/util/logging.h b/cpp/src/arrow/util/logging.h index 999aca6..a920db1 100644 --- a/cpp/src/arrow/util/logging.h +++ b/cpp/src/arrow/util/logging.h @@ -59,25 +59,34 @@ enum class ArrowLogLevel : int { #define ARROW_IGNORE_EXPR(expr) ((void)(expr)) -#define ARROW_CHECK(condition) \ - (condition) ? ARROW_IGNORE_EXPR(0) \ - : ::arrow::util::Voidify() & \ - ::arrow::util::ArrowLog(__FILE__, __LINE__, \ - ::arrow::util::ArrowLogLevel::ARROW_FATAL) \ - << " Check failed: " #condition " " +#define ARROW_CHECK(condition) \ + ARROW_PREDICT_TRUE(condition) \ + ? ARROW_IGNORE_EXPR(0) \ + : ::arrow::util::Voidify() & \ + ::arrow::util::ArrowLog(__FILE__, __LINE__, \ + ::arrow::util::ArrowLogLevel::ARROW_FATAL) \ + << " Check failed: " #condition " " // If 'to_call' returns a bad status, CHECK immediately with a logged message // of 'msg' followed by the status. -#define ARROW_CHECK_OK_PREPEND(to_call, msg) \ - do { \ - ::arrow::Status _s = (to_call); \ - ARROW_CHECK(_s.ok()) << (msg) << ": " << _s.ToString(); \ +#define ARROW_CHECK_OK_PREPEND(to_call, msg) \ + do { \ + ::arrow::Status _s = (to_call); \ + ARROW_CHECK(_s.ok()) << "Operation failed: " << ARROW_STRINGIFY(to_call) << "\n" \ + << (msg) << ": " << _s.ToString(); \ } while (false) // If the status is bad, CHECK immediately, appending the status to the // logged message. #define ARROW_CHECK_OK(s) ARROW_CHECK_OK_PREPEND(s, "Bad status") +#define ARROW_CHECK_EQ(val1, val2) ARROW_CHECK((val1) == (val2)) +#define ARROW_CHECK_NE(val1, val2) ARROW_CHECK((val1) != (val2)) +#define ARROW_CHECK_LE(val1, val2) ARROW_CHECK((val1) <= (val2)) +#define ARROW_CHECK_LT(val1, val2) ARROW_CHECK((val1) < (val2)) +#define ARROW_CHECK_GE(val1, val2) ARROW_CHECK((val1) >= (val2)) +#define ARROW_CHECK_GT(val1, val2) ARROW_CHECK((val1) > (val2)) + #ifdef NDEBUG #define ARROW_DFATAL ::arrow::util::ArrowLogLevel::ARROW_WARNING @@ -118,14 +127,14 @@ enum class ArrowLogLevel : int { #else #define ARROW_DFATAL ::arrow::util::ArrowLogLevel::ARROW_FATAL -#define DCHECK(condition) ARROW_CHECK(condition) -#define DCHECK_OK(status) ARROW_CHECK_OK(status) -#define DCHECK_EQ(val1, val2) ARROW_CHECK((val1) == (val2)) -#define DCHECK_NE(val1, val2) ARROW_CHECK((val1) != (val2)) -#define DCHECK_LE(val1, val2) ARROW_CHECK((val1) <= (val2)) -#define DCHECK_LT(val1, val2) ARROW_CHECK((val1) < (val2)) -#define DCHECK_GE(val1, val2) ARROW_CHECK((val1) >= (val2)) -#define DCHECK_GT(val1, val2) ARROW_CHECK((val1) > (val2)) +#define DCHECK ARROW_CHECK +#define DCHECK_OK ARROW_CHECK_OK +#define DCHECK_EQ ARROW_CHECK_EQ +#define DCHECK_NE ARROW_CHECK_NE +#define DCHECK_LE ARROW_CHECK_LE +#define DCHECK_LT ARROW_CHECK_LT +#define DCHECK_GE ARROW_CHECK_GE +#define DCHECK_GT ARROW_CHECK_GT #endif // NDEBUG diff --git a/cpp/src/arrow/util/thread-pool.cc b/cpp/src/arrow/util/thread-pool.cc index 6969f3f..81152bc 100644 --- a/cpp/src/arrow/util/thread-pool.cc +++ b/cpp/src/arrow/util/thread-pool.cc @@ -289,7 +289,7 @@ int ThreadPool::DefaultCapacity() { // Helper for the singleton pattern std::shared_ptr<ThreadPool> ThreadPool::MakeCpuThreadPool() { std::shared_ptr<ThreadPool> pool; - DCHECK_OK(ThreadPool::Make(ThreadPool::DefaultCapacity(), &pool)); + ARROW_CHECK_OK(ThreadPool::Make(ThreadPool::DefaultCapacity(), &pool)); // On Windows, the global ThreadPool destructor may be called after // non-main threads have been killed by the OS, and hang in a condition // variable.