[ 
https://issues.apache.org/jira/browse/ARROW-2393?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16446820#comment-16446820
 ] 

ASF GitHub Bot commented on ARROW-2393:
---------------------------------------

xhochy closed pull request #1904: ARROW-2393: [C++] Moving 
ARROW_CHECK_OK_[PREPEND] macros from status.h into util/logging.h since they 
use the logging infrastructure and shouldn't be in the public API.
URL: https://github.com/apache/arrow/pull/1904
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/c_glib/arrow-glib/input-stream.cpp 
b/c_glib/arrow-glib/input-stream.cpp
index c643ad2a7e..353d00a8b4 100644
--- a/c_glib/arrow-glib/input-stream.cpp
+++ b/c_glib/arrow-glib/input-stream.cpp
@@ -535,7 +535,7 @@ namespace garrow {
                        std::shared_ptr<arrow::Buffer> *out) override {
       arrow::MemoryPool *pool = arrow::default_memory_pool();
       std::shared_ptr<arrow::ResizableBuffer> buffer;
-      ARROW_RETURN_NOT_OK(AllocateResizableBuffer(pool, n_bytes, &buffer));
+      RETURN_NOT_OK(AllocateResizableBuffer(pool, n_bytes, &buffer));
 
       GError *error = NULL;
       auto n_read_bytes = g_input_stream_read(input_stream_,
@@ -549,7 +549,7 @@ namespace garrow {
                                       "[gio-input-stream][read][buffer]");
       } else {
         if (n_read_bytes < n_bytes) {
-          ARROW_RETURN_NOT_OK(buffer->Resize(n_read_bytes));
+          RETURN_NOT_OK(buffer->Resize(n_read_bytes));
         }
         *out = buffer;
         return arrow::Status::OK();
diff --git a/cpp/src/arrow/python/io.cc b/cpp/src/arrow/python/io.cc
index 155e86f828..4277ab377f 100644
--- a/cpp/src/arrow/python/io.cc
+++ b/cpp/src/arrow/python/io.cc
@@ -138,7 +138,7 @@ Status PyReadableFile::Read(int64_t nbytes, int64_t* 
bytes_read, void* out) {
   PyAcquireGIL lock;
 
   PyObject* bytes_obj = NULL;
-  ARROW_RETURN_NOT_OK(file_->Read(nbytes, &bytes_obj));
+  RETURN_NOT_OK(file_->Read(nbytes, &bytes_obj));
   DCHECK(bytes_obj != NULL);
 
   *bytes_read = PyBytes_GET_SIZE(bytes_obj);
@@ -152,7 +152,7 @@ Status PyReadableFile::Read(int64_t nbytes, 
std::shared_ptr<Buffer>* out) {
   PyAcquireGIL lock;
 
   OwnedRef bytes_obj;
-  ARROW_RETURN_NOT_OK(file_->Read(nbytes, bytes_obj.ref()));
+  RETURN_NOT_OK(file_->Read(nbytes, bytes_obj.ref()));
   DCHECK(bytes_obj.obj() != NULL);
 
   return PyBuffer::FromPyObject(bytes_obj.obj(), out);
@@ -177,15 +177,15 @@ Status PyReadableFile::GetSize(int64_t* size) {
 
   int64_t current_position = -1;
 
-  ARROW_RETURN_NOT_OK(file_->Tell(&current_position));
+  RETURN_NOT_OK(file_->Tell(&current_position));
 
-  ARROW_RETURN_NOT_OK(file_->Seek(0, 2));
+  RETURN_NOT_OK(file_->Seek(0, 2));
 
   int64_t file_size = -1;
-  ARROW_RETURN_NOT_OK(file_->Tell(&file_size));
+  RETURN_NOT_OK(file_->Tell(&file_size));
 
   // Restore previous file position
-  ARROW_RETURN_NOT_OK(file_->Seek(current_position, 0));
+  RETURN_NOT_OK(file_->Seek(current_position, 0));
 
   *size = file_size;
   return Status::OK();
diff --git a/cpp/src/arrow/status.h b/cpp/src/arrow/status.h
index 84f55e415c..ed524ae6c3 100644
--- a/cpp/src/arrow/status.h
+++ b/cpp/src/arrow/status.h
@@ -26,34 +26,11 @@
 #include "arrow/util/macros.h"
 #include "arrow/util/visibility.h"
 
-// Return the given status if it is not OK.
-#define ARROW_RETURN_NOT_OK(s)           \
-  do {                                   \
-    ::arrow::Status _s = (s);            \
-    if (ARROW_PREDICT_FALSE(!_s.ok())) { \
-      return _s;                         \
-    }                                    \
-  } while (false)
-
-// 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(); \
-  } 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")
-
-namespace arrow {
-
 #ifdef ARROW_EXTRA_ERROR_CONTEXT
 
 #define RETURN_NOT_OK(s)                                                       
     \
   do {                                                                         
     \
-    Status _s = (s);                                                           
     \
+    ::arrow::Status _s = (s);                                                  
     \
     if (ARROW_PREDICT_FALSE(!_s.ok())) {                                       
     \
       std::stringstream ss;                                                    
     \
       ss << __FILE__ << ":" << __LINE__ << " code: " << #s << "\n" << 
_s.message(); \
@@ -65,7 +42,7 @@ namespace arrow {
 
 #define RETURN_NOT_OK(s)                 \
   do {                                   \
-    Status _s = (s);                     \
+    ::arrow::Status _s = (s);            \
     if (ARROW_PREDICT_FALSE(!_s.ok())) { \
       return _s;                         \
     }                                    \
@@ -75,13 +52,21 @@ namespace arrow {
 
 #define RETURN_NOT_OK_ELSE(s, else_) \
   do {                               \
-    Status _s = (s);                 \
+    ::arrow::Status _s = (s);        \
     if (!_s.ok()) {                  \
       else_;                         \
       return _s;                     \
     }                                \
   } while (false)
 
+// This is used by other codebases. The macros above
+// should probably have that prefix, but there is a
+// lot of code that already uses that macro and changing
+// it would be of no benefit.
+#define ARROW_RETURN_NOT_OK(s) RETURN_NOT_OK(s)
+
+namespace arrow {
+
 enum class StatusCode : char {
   OK = 0,
   OutOfMemory = 1,
diff --git a/cpp/src/arrow/util/logging.h b/cpp/src/arrow/util/logging.h
index c823f06bdf..12defc0557 100644
--- a/cpp/src/arrow/util/logging.h
+++ b/cpp/src/arrow/util/logging.h
@@ -47,6 +47,18 @@ namespace arrow {
               : ::arrow::internal::FatalLog(ARROW_FATAL) \
                     << __FILE__ << ":" << __LINE__ << " 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(); \
+  } 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")
+
 #ifdef NDEBUG
 #define ARROW_DFATAL ARROW_WARNING
 


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


> [C++] arrow/status.h does not define ARROW_CHECK needed for ARROW_CHECK_OK
> --------------------------------------------------------------------------
>
>                 Key: ARROW-2393
>                 URL: https://issues.apache.org/jira/browse/ARROW-2393
>             Project: Apache Arrow
>          Issue Type: Bug
>          Components: C++
>    Affects Versions: 0.9.0
>            Reporter: dennis lucero
>            Priority: Trivial
>              Labels: pull-request-available
>             Fix For: 0.10.0
>
>
> test.cpp
> {code:c++}
> #include <arrow/api.h>
> int main(void) {
> arrow::Int64Builder i64builder;
> std::shared_ptr<arrow::Array> i64array;
> ARROW_CHECK_OK(i64builder.Finish(&i64array));
> return EXIT_SUCCESS;
> }
> {code}
> Attempt to build:
> {code:bash}
> $CXX test.cpp -std=c++11 -larrow
> {code}
> Error:
> {code}
> test.cpp:6:2: error: use of undeclared identifier 'ARROW_CHECK' 
> ARROW_CHECK_OK(i64builder.Finish(&i64array)); ^ 
> xxx/include/arrow/status.h:49:27: note: expanded from macro 'ARROW_CHECK_OK' 
> #define ARROW_CHECK_OK(s) ARROW_CHECK_OK_PREPEND(s, "Bad status") ^ 
> xxx/include/arrow/status.h:44:5: note: expanded from macro 
> 'ARROW_CHECK_OK_PREPEND' ARROW_CHECK(_s.ok()) << (msg) << ": " << 
> _s.ToString(); \ ^ 1 error generated.
> {code}
> I expect that ARROW_* macro are public API, and should work out of the box.
> A naive attempt to fix it
> {code}
> diff --git a/cpp/src/arrow/status.h b/cpp/src/arrow/status.h
> index 84f55e41..6da4a773 100644
> --- a/cpp/src/arrow/status.h
> +++ b/cpp/src/arrow/status.h
> @@ -25,6 +25,7 @@
>  #include "arrow/util/macros.h"
>  #include "arrow/util/visibility.h"
> +#include "arrow/util/logging.h"
>  // Return the given status if it is not OK.
>  #define ARROW_RETURN_NOT_OK(s)           \
> {code}
> fails with
> {code}
> public-api-test.cc:21:2: error: "DCHECK should not be visible from Arrow 
> public headers."
> {code}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to