You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by ko...@apache.org on 2019/03/28 02:23:31 UTC

[arrow] branch master updated: ARROW-5029: [C++] Fix compilation warnings in release mode

This is an automated email from the ASF dual-hosted git repository.

kou 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 97abab5  ARROW-5029: [C++] Fix compilation warnings in release mode
97abab5 is described below

commit 97abab50d5d09d55e050ea59b7751b1a5a42ed5b
Author: Antoine Pitrou <an...@python.org>
AuthorDate: Thu Mar 28 11:23:18 2019 +0900

    ARROW-5029: [C++] Fix compilation warnings in release mode
    
    Also fix a severe memory leak in CUDA (CUDA buffers were never deallocated).
    
    Author: Antoine Pitrou <an...@python.org>
    
    Closes #4049 from pitrou/ARROW-5029-fix-release-warnings and squashes the following commits:
    
    aa02a8db <Antoine Pitrou> ARROW-5029:  Fix compilation warnings in release mode
---
 cpp/src/arrow/adapters/orc/adapter.cc      |  2 +-
 cpp/src/arrow/array.cc                     |  3 +--
 cpp/src/arrow/dbi/hiveserver2/operation.cc |  2 +-
 cpp/src/arrow/gpu/cuda_memory.cc           |  6 +++---
 cpp/src/arrow/io/compressed.cc             |  4 ++--
 cpp/src/arrow/python/deserialize.cc        | 10 ++++++----
 cpp/src/arrow/util/compression_zlib.cc     |  2 +-
 cpp/src/arrow/util/decimal.cc              |  4 ++--
 cpp/src/arrow/util/logging.h               | 30 +++++++++++++++++++++++-------
 9 files changed, 40 insertions(+), 23 deletions(-)

diff --git a/cpp/src/arrow/adapters/orc/adapter.cc b/cpp/src/arrow/adapters/orc/adapter.cc
index 89ad597..a4311bb 100644
--- a/cpp/src/arrow/adapters/orc/adapter.cc
+++ b/cpp/src/arrow/adapters/orc/adapter.cc
@@ -377,7 +377,7 @@ class ORCFileReader::Impl {
     if (!include_indices.empty()) {
       RETURN_NOT_OK(SelectIndices(&opts, include_indices));
     }
-    StripeInformation stripe_info;
+    StripeInformation stripe_info({0, 0, 0, 0});
     RETURN_NOT_OK(SelectStripeWithRowNumber(&opts, current_row_, &stripe_info));
     std::shared_ptr<Schema> schema;
     RETURN_NOT_OK(ReadSchema(opts, &schema));
diff --git a/cpp/src/arrow/array.cc b/cpp/src/arrow/array.cc
index 5633a7c..bcf4342 100644
--- a/cpp/src/arrow/array.cc
+++ b/cpp/src/arrow/array.cc
@@ -928,8 +928,7 @@ class ArrayDataWrapper {
 std::shared_ptr<Array> MakeArray(const std::shared_ptr<ArrayData>& data) {
   std::shared_ptr<Array> out;
   internal::ArrayDataWrapper wrapper_visitor(data, &out);
-  Status s = VisitTypeInline(*data->type, &wrapper_visitor);
-  DCHECK(s.ok());
+  DCHECK_OK(VisitTypeInline(*data->type, &wrapper_visitor));
   DCHECK(out);
   return out;
 }
diff --git a/cpp/src/arrow/dbi/hiveserver2/operation.cc b/cpp/src/arrow/dbi/hiveserver2/operation.cc
index 09e6514..0af43f4 100644
--- a/cpp/src/arrow/dbi/hiveserver2/operation.cc
+++ b/cpp/src/arrow/dbi/hiveserver2/operation.cc
@@ -108,7 +108,7 @@ Status Operation::Fetch(int max_rows, FetchOrientation orientation,
     *has_more_rows = row_set_impl->resp.hasMoreRows;
   }
   Status status = TStatusToStatus(row_set_impl->resp.status);
-  DCHECK(status.ok());
+  RETURN_NOT_OK(status);
   results->reset(new ColumnarRowSet(row_set_impl.release()));
   return status;
 }
diff --git a/cpp/src/arrow/gpu/cuda_memory.cc b/cpp/src/arrow/gpu/cuda_memory.cc
index e91fef2..c777bf7 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(Close().ok()); }
+CudaBuffer::~CudaBuffer() { DCHECK_OK(Close()); }
 
 Status CudaBuffer::Close() {
   if (own_data_) {
@@ -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());
+  DCHECK_OK(CudaDeviceManager::GetInstance(&manager));
+  DCHECK_OK(manager->FreeHost(mutable_data_, size_));
 }
 
 // ----------------------------------------------------------------------
diff --git a/cpp/src/arrow/io/compressed.cc b/cpp/src/arrow/io/compressed.cc
index 1311dbc..bc04d49 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(Close().ok()); }
+  ~Impl() { DCHECK_OK(Close()); }
 
   Status Init() {
     RETURN_NOT_OK(codec_->MakeCompressor(&compressor_));
@@ -236,7 +236,7 @@ class CompressedInputStream::Impl {
     return Status::OK();
   }
 
-  ~Impl() { DCHECK(Close().ok()); }
+  ~Impl() { DCHECK_OK(Close()); }
 
   Status Close() {
     std::lock_guard<std::mutex> guard(lock_);
diff --git a/cpp/src/arrow/python/deserialize.cc b/cpp/src/arrow/python/deserialize.cc
index 8e7ba8a..e5091c4 100644
--- a/cpp/src/arrow/python/deserialize.cc
+++ b/cpp/src/arrow/python/deserialize.cc
@@ -107,10 +107,12 @@ Status DeserializeArray(int32_t index, PyObject* base, const SerializedPyObject&
   RETURN_NOT_OK(py::TensorToNdarray(blobs.ndarrays[index], base, out));
   // Mark the array as immutable
   OwnedRef flags(PyObject_GetAttrString(*out, "flags"));
-  DCHECK(flags.obj() != NULL) << "Could not mark Numpy array immutable";
-  Py_INCREF(Py_False);
-  int flag_set = PyObject_SetAttrString(flags.obj(), "writeable", Py_False);
-  DCHECK(flag_set == 0) << "Could not mark Numpy array immutable";
+  if (flags.obj() == NULL) {
+    return ConvertPyError();
+  }
+  if (PyObject_SetAttrString(flags.obj(), "writeable", Py_False) < 0) {
+    return ConvertPyError();
+  }
   return Status::OK();
 }
 
diff --git a/cpp/src/arrow/util/compression_zlib.cc b/cpp/src/arrow/util/compression_zlib.cc
index 736b0ab..202ef06 100644
--- a/cpp/src/arrow/util/compression_zlib.cc
+++ b/cpp/src/arrow/util/compression_zlib.cc
@@ -429,7 +429,7 @@ class GZipCodec::GZipCodecImpl {
     // Must be in compression mode
     if (!compressor_initialized_) {
       Status s = InitCompressor();
-      DCHECK(s.ok());
+      DCHECK_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 347a07d..4802862 100644
--- a/cpp/src/arrow/util/decimal.cc
+++ b/cpp/src/arrow/util/decimal.cc
@@ -39,8 +39,8 @@ using internal::SafeLeftShift;
 using internal::SafeSignedAdd;
 
 Decimal128::Decimal128(const std::string& str) : Decimal128() {
-  Status status(Decimal128::FromString(str, this));
-  DCHECK(status.ok()) << status.message();
+  Status status = Decimal128::FromString(str, this);
+  DCHECK_OK(status);
 }
 
 static const Decimal128 kTenTo36(static_cast<int64_t>(0xC097CE7BC90715),
diff --git a/cpp/src/arrow/util/logging.h b/cpp/src/arrow/util/logging.h
index c60742d..7b4ec65 100644
--- a/cpp/src/arrow/util/logging.h
+++ b/cpp/src/arrow/util/logging.h
@@ -81,22 +81,38 @@ enum class ArrowLogLevel : int {
 #ifdef NDEBUG
 #define ARROW_DFATAL ::arrow::util::ArrowLogLevel::ARROW_WARNING
 
-#define DCHECK(condition) \
+// CAUTION: DCHECK_OK() always evaluates its argument, but other DCHECK*() macros
+// only do so in debug mode.
+
+#define DCHECK(condition)                     \
+  while (false) ARROW_IGNORE_EXPR(condition); \
   while (false) ::arrow::util::detail::NullLog()
 #define DCHECK_OK(s)    \
   ARROW_IGNORE_EXPR(s); \
   while (false) ::arrow::util::detail::NullLog()
-#define DCHECK_EQ(val1, val2) \
+#define DCHECK_EQ(val1, val2)            \
+  while (false) ARROW_IGNORE_EXPR(val1); \
+  while (false) ARROW_IGNORE_EXPR(val2); \
   while (false) ::arrow::util::detail::NullLog()
-#define DCHECK_NE(val1, val2) \
+#define DCHECK_NE(val1, val2)            \
+  while (false) ARROW_IGNORE_EXPR(val1); \
+  while (false) ARROW_IGNORE_EXPR(val2); \
   while (false) ::arrow::util::detail::NullLog()
-#define DCHECK_LE(val1, val2) \
+#define DCHECK_LE(val1, val2)            \
+  while (false) ARROW_IGNORE_EXPR(val1); \
+  while (false) ARROW_IGNORE_EXPR(val2); \
   while (false) ::arrow::util::detail::NullLog()
-#define DCHECK_LT(val1, val2) \
+#define DCHECK_LT(val1, val2)            \
+  while (false) ARROW_IGNORE_EXPR(val1); \
+  while (false) ARROW_IGNORE_EXPR(val2); \
   while (false) ::arrow::util::detail::NullLog()
-#define DCHECK_GE(val1, val2) \
+#define DCHECK_GE(val1, val2)            \
+  while (false) ARROW_IGNORE_EXPR(val1); \
+  while (false) ARROW_IGNORE_EXPR(val2); \
   while (false) ::arrow::util::detail::NullLog()
-#define DCHECK_GT(val1, val2) \
+#define DCHECK_GT(val1, val2)            \
+  while (false) ARROW_IGNORE_EXPR(val1); \
+  while (false) ARROW_IGNORE_EXPR(val2); \
   while (false) ::arrow::util::detail::NullLog()
 
 #else