Skip to content

Commit

Permalink
remove side effects from DCHECK conditions
Browse files Browse the repository at this point in the history
  • Loading branch information
bkietz committed Feb 19, 2019
1 parent c693b34 commit c35b7ad
Show file tree
Hide file tree
Showing 20 changed files with 34 additions and 30 deletions.
3 changes: 1 addition & 2 deletions cpp/examples/parquet/low-level-api/reader-writer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.

#include <cassert>
#include <fstream>
#include <iostream>
Expand Down Expand Up @@ -166,7 +165,7 @@ int main(int argc, char** argv) {
file_writer->Close();

// Write the bytes to file
DCHECK(out_file->Close().ok());
DABORT_NOT_OK(out_file->Close());
} catch (const std::exception& e) {
std::cerr << "Parquet write error: " << e.what() << std::endl;
return -1;
Expand Down
2 changes: 1 addition & 1 deletion cpp/examples/parquet/low-level-api/reader-writer2.cc
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ int main(int argc, char** argv) {
file_writer->Close();

// Write the bytes to file
DCHECK(out_file->Close().ok());
DABORT_NOT_OK(out_file->Close());
} catch (const std::exception& e) {
std::cerr << "Parquet write error: " << e.what() << std::endl;
return -1;
Expand Down
2 changes: 1 addition & 1 deletion cpp/src/arrow/array.cc
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ std::shared_ptr<Array> Array::Slice(int64_t offset) const {

std::string Array::ToString() const {
std::stringstream ss;
DCHECK(PrettyPrint(*this, 0, &ss).ok());
DABORT_NOT_OK(PrettyPrint(*this, 0, &ss));
return ss.str();
}

Expand Down
6 changes: 3 additions & 3 deletions cpp/src/arrow/gpu/cuda_memory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ CudaBuffer::CudaBuffer(uint8_t* data, int64_t size,
mutable_data_ = data;
}

CudaBuffer::~CudaBuffer() { DCHECK(Close().ok()); }
CudaBuffer::~CudaBuffer() { DABORT_NOT_OK(Close()); }

Status CudaBuffer::Close() {
if (own_data_) {
Expand Down Expand Up @@ -182,8 +182,8 @@ Status CudaBuffer::ExportForIpc(std::shared_ptr<CudaIpcMemHandle>* handle) {

CudaHostBuffer::~CudaHostBuffer() {
CudaDeviceManager* manager = nullptr;
DCHECK(CudaDeviceManager::GetInstance(&manager).ok());
DCHECK(manager->FreeHost(mutable_data_, size_).ok());
DABORT_NOT_OK(CudaDeviceManager::GetInstance(&manager));
DABORT_NOT_OK(manager->FreeHost(mutable_data_, size_));
}

// ----------------------------------------------------------------------
Expand Down
6 changes: 3 additions & 3 deletions cpp/src/arrow/io/buffered.cc
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ Status BufferedOutputStream::Create(int64_t buffer_size, MemoryPool* pool,
return Status::OK();
}

BufferedOutputStream::~BufferedOutputStream() { DCHECK_OK(impl_->Close()); }
BufferedOutputStream::~BufferedOutputStream() { DABORT_NOT_OK(impl_->Close()); }

Status BufferedOutputStream::SetBufferSize(int64_t new_buffer_size) {
return impl_->SetBufferSize(new_buffer_size);
Expand Down Expand Up @@ -224,7 +224,7 @@ class BufferedInputStream::Impl : public BufferedBase {
Impl(std::shared_ptr<InputStream> raw, MemoryPool* pool)
: BufferedBase(pool), raw_(std::move(raw)), bytes_buffered_(0) {}

~Impl() { DCHECK_OK(Close()); }
~Impl() { DABORT_NOT_OK(Close()); }

Status Close() {
std::lock_guard<std::mutex> guard(lock_);
Expand Down Expand Up @@ -355,7 +355,7 @@ BufferedInputStream::BufferedInputStream(std::shared_ptr<InputStream> raw,
impl_.reset(new Impl(std::move(raw), pool));
}

BufferedInputStream::~BufferedInputStream() { DCHECK_OK(impl_->Close()); }
BufferedInputStream::~BufferedInputStream() { DABORT_NOT_OK(impl_->Close()); }

Status BufferedInputStream::Create(int64_t buffer_size, MemoryPool* pool,
std::shared_ptr<InputStream> raw,
Expand Down
4 changes: 2 additions & 2 deletions cpp/src/arrow/io/compressed.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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(Close().ok()); }
~Impl() { DABORT_NOT_OK(Close()); }

Status Init() {
RETURN_NOT_OK(codec_->MakeCompressor(&compressor_));
Expand Down Expand Up @@ -236,7 +236,7 @@ class CompressedInputStream::Impl {
return Status::OK();
}

~Impl() { DCHECK(Close().ok()); }
~Impl() { DABORT_NOT_OK(Close()); }

Status Close() {
std::lock_guard<std::mutex> guard(lock_);
Expand Down
6 changes: 3 additions & 3 deletions cpp/src/arrow/io/file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ class ReadableFile::ReadableFileImpl : public OSFile {

ReadableFile::ReadableFile(MemoryPool* pool) { impl_.reset(new ReadableFileImpl(pool)); }

ReadableFile::~ReadableFile() { DCHECK(impl_->Close().ok()); }
ReadableFile::~ReadableFile() { DABORT_NOT_OK(impl_->Close()); }

Status ReadableFile::Open(const std::string& path, std::shared_ptr<ReadableFile>* file) {
return Open(path, default_memory_pool(), file);
Expand Down Expand Up @@ -299,7 +299,7 @@ FileOutputStream::FileOutputStream() { impl_.reset(new FileOutputStreamImpl());

FileOutputStream::~FileOutputStream() {
// This can fail; better to explicitly call close
DCHECK(impl_->Close().ok());
DABORT_NOT_OK(impl_->Close());
}

Status FileOutputStream::Open(const std::string& path,
Expand Down Expand Up @@ -355,7 +355,7 @@ class MemoryMappedFile::MemoryMap : public MutableBuffer {
MemoryMap() : MutableBuffer(nullptr, 0) {}

~MemoryMap() {
DCHECK_OK(Close());
DABORT_NOT_OK(Close());
if (mutable_data_ != nullptr) {
DCHECK_EQ(munmap(mutable_data_, static_cast<size_t>(size_)), 0);
}
Expand Down
4 changes: 2 additions & 2 deletions cpp/src/arrow/io/hdfs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ HdfsReadableFile::HdfsReadableFile(MemoryPool* pool) {
impl_.reset(new HdfsReadableFileImpl(pool));
}

HdfsReadableFile::~HdfsReadableFile() { DCHECK_OK(impl_->Close()); }
HdfsReadableFile::~HdfsReadableFile() { DABORT_NOT_OK(impl_->Close()); }

Status HdfsReadableFile::Close() { return impl_->Close(); }

Expand Down Expand Up @@ -293,7 +293,7 @@ class HdfsOutputStream::HdfsOutputStreamImpl : public HdfsAnyFileImpl {

HdfsOutputStream::HdfsOutputStream() { impl_.reset(new HdfsOutputStreamImpl()); }

HdfsOutputStream::~HdfsOutputStream() { DCHECK_OK(impl_->Close()); }
HdfsOutputStream::~HdfsOutputStream() { DABORT_NOT_OK(impl_->Close()); }

Status HdfsOutputStream::Close() { return impl_->Close(); }

Expand Down
2 changes: 1 addition & 1 deletion cpp/src/arrow/io/memory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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(Close().ok());
DABORT_NOT_OK(Close());
}
}

Expand Down
4 changes: 2 additions & 2 deletions cpp/src/arrow/sparse_tensor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ void MakeSparseTensorFromTensor(const Tensor& tensor,
std::shared_ptr<Buffer>* data) {
NumericTensor<TYPE> numeric_tensor(tensor.data(), tensor.shape(), tensor.strides());
SparseTensorConverter<TYPE, SparseIndexType> converter(numeric_tensor);
DCHECK_OK(converter.Convert());
DABORT_NOT_OK(converter.Convert());
*sparse_index = converter.sparse_index;
*data = converter.data;
}
Expand Down Expand Up @@ -319,7 +319,7 @@ SparseTensorImpl<SparseIndexType>::SparseTensorImpl(const NumericTensor<TYPE>& t
: SparseTensorImpl(nullptr, tensor.type(), nullptr, tensor.shape(),
tensor.dim_names_) {
SparseTensorConverter<TYPE, SparseIndexType> converter(tensor);
DCHECK_OK(converter.Convert());
DABORT_NOT_OK(converter.Convert());
sparse_index_ = converter.sparse_index;
data_ = converter.data;
}
Expand Down
6 changes: 4 additions & 2 deletions cpp/src/arrow/util/basic_decimal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -639,7 +639,8 @@ void BasicDecimal128::GetWholeAndFraction(int scale, BasicDecimal128* whole,
DCHECK_LE(scale, 38);

BasicDecimal128 multiplier(ScaleMultipliers[scale]);
DCHECK_EQ(Divide(multiplier, whole, fraction), DecimalStatus::kSuccess);
auto status = Divide(multiplier, whole, fraction);
DCHECK_EQ(status, DecimalStatus::kSuccess);
}

const BasicDecimal128& BasicDecimal128::GetScaleMultiplier(int32_t scale) {
Expand All @@ -663,7 +664,8 @@ BasicDecimal128 BasicDecimal128::ReduceScaleBy(int32_t reduce_by, bool round) co
BasicDecimal128 divisor(ScaleMultipliers[reduce_by]);
BasicDecimal128 result;
BasicDecimal128 remainder;
DCHECK_EQ(Divide(divisor, &result, &remainder), DecimalStatus::kSuccess);
auto status = Divide(divisor, &result, &remainder);
DCHECK_EQ(status, DecimalStatus::kSuccess);
if (round) {
auto divisor_half = ScaleMultipliersHalf[reduce_by];
if (remainder.Abs() >= divisor_half) {
Expand Down
3 changes: 2 additions & 1 deletion cpp/src/arrow/util/bit-stream-utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,8 @@ inline bool BitReader::GetVlqInt(int32_t* v) {
if (!GetAligned<uint8_t>(1, &byte)) return false;
*v |= (byte & 0x7F) << shift;
shift += 7;
DCHECK_LE(++num_bytes, MAX_VLQ_BYTE_LEN);
++num_bytes;
DCHECK_LE(num_bytes, MAX_VLQ_BYTE_LEN);
} while ((byte & 0x80) != 0);
return true;
}
Expand Down
2 changes: 1 addition & 1 deletion cpp/src/arrow/util/decimal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ std::string Decimal128::ToIntegerString() const {

// get anything above 10 ** 36 and print it
Decimal128 top;
DCHECK_OK(Divide(kTenTo36, &top, &remainder));
DABORT_NOT_OK(Divide(kTenTo36, &top, &remainder));

if (top != 0) {
buf << static_cast<int64_t>(top);
Expand Down
2 changes: 2 additions & 0 deletions cpp/src/arrow/util/logging.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ enum class ArrowLogLevel : int {
while (false) ARROW_CHECK((val1) >= (val2))
#define DCHECK_GT(val1, val2) \
while (false) ARROW_CHECK((val1) > (val2))
#define DABORT_NOT_OK(expr) ARROW_IGNORE_EXPR(expr)

#else
#define ARROW_DFATAL ::arrow::util::ArrowLogLevel::ARROW_FATAL
Expand All @@ -108,6 +109,7 @@ enum class ArrowLogLevel : int {
#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 DABORT_NOT_OK(expr) DCHECK_OK(expr)

#endif // NDEBUG

Expand Down
2 changes: 1 addition & 1 deletion cpp/src/gandiva/filter_cache_key.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ class FilterCacheKey {
std::stringstream ss;
// indent, window, indent_size, null_rep and skip new lines.
arrow::PrettyPrintOptions options{0, 10, 2, "null", true};
DCHECK_OK(PrettyPrint(*schema_.get(), options, &ss));
DABORT_NOT_OK(PrettyPrint(*schema_.get(), options, &ss));

ss << "Condition: [" << expression_as_string_ << "]";
return ss.str();
Expand Down
2 changes: 1 addition & 1 deletion cpp/src/gandiva/precompiled/testing.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ namespace gandiva {

timestamp StringToTimestamp(const char* buf) {
int64_t out = 0;
DCHECK(internal::ParseTimestamp(buf, "%Y-%m-%d %H:%M:%S", false, &out));
DABORT_NOT_OK(internal::ParseTimestamp(buf, "%Y-%m-%d %H:%M:%S", false, &out));
return out * 1000;
}

Expand Down
2 changes: 1 addition & 1 deletion cpp/src/gandiva/projector_cache_key.h
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ class ProjectorCacheKey {
std::stringstream ss;
// indent, window, indent_size, null_rep and skip new lines.
arrow::PrettyPrintOptions options{0, 10, 2, "null", true};
DCHECK_OK(PrettyPrint(*schema_.get(), options, &ss));
DABORT_NOT_OK(PrettyPrint(*schema_.get(), options, &ss));

ss << "Expressions: [";
bool first = true;
Expand Down
2 changes: 1 addition & 1 deletion cpp/src/parquet/arrow/reader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ class PARQUET_NO_EXPORT PrimitiveImpl : public ColumnReader::ColumnReaderImpl {
PrimitiveImpl(MemoryPool* pool, std::unique_ptr<FileColumnIterator> input)
: pool_(pool), input_(std::move(input)), descr_(input_->descr()) {
record_reader_ = RecordReader::Make(descr_, pool_);
DCHECK(NodeToField(*input_->descr()->schema_node(), &field_).ok());
DABORT_NOT_OK(NodeToField(*input_->descr()->schema_node(), &field_));
NextRowGroup();
}

Expand Down
2 changes: 1 addition & 1 deletion cpp/src/plasma/client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ PlasmaBuffer::~PlasmaBuffer() { ARROW_UNUSED(client_->Release(object_id_)); }

PlasmaClient::Impl::Impl() : store_conn_(0), store_capacity_(0) {
#ifdef PLASMA_CUDA
DCHECK_OK(CudaDeviceManager::GetInstance(&manager_));
DABORT_NOT_OK(CudaDeviceManager::GetInstance(&manager_));
#endif
}

Expand Down
2 changes: 1 addition & 1 deletion cpp/src/plasma/store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ PlasmaStore::PlasmaStore(EventLoop* loop, std::string directory, bool hugepages_
store_info_.directory = directory;
store_info_.hugepages_enabled = hugepages_enabled;
#ifdef PLASMA_CUDA
DCHECK_OK(CudaDeviceManager::GetInstance(&manager_));
DABORT_NOT_OK(CudaDeviceManager::GetInstance(&manager_));
#endif
}

Expand Down

0 comments on commit c35b7ad

Please sign in to comment.