ARROW-8488: [R] Remove VALUE_OR_STOP and STOP_IF_NOT_OK macros

Prefer templated functions over macro to align with the C++ style.

Closes #6969 from fsaintjacques/ARROW-8488

Authored-by: François Saint-Jacques <fsaintjacques@gmail.com>
Signed-off-by: François Saint-Jacques <fsaintjacques@gmail.com>

r/src/array.cpp

@@ -143,7 +143,7 @@ bool Array__RangeEquals(const std::shared_ptr<arrow::Array>& self,
 // [[arrow::export]]
 std::shared_ptr<arrow::Array> Array__View(const std::shared_ptr<arrow::Array>& array,
                                           const std::shared_ptr<arrow::DataType>& type) {
-  return VALUE_OR_STOP(array->View(type));
+  return ValueOrStop(array->View(type));
 }
 
 // [[arrow::export]]
@@ -164,7 +164,7 @@ LogicalVector Array__Mask(const std::shared_ptr<arrow::Array>& array) {
 
 // [[arrow::export]]
 void Array__Validate(const std::shared_ptr<arrow::Array>& array) {
-  STOP_IF_NOT_OK(array->Validate());
+  StopIfNotOk(array->Validate());
 }
 
 // [[arrow::export]]
@@ -194,7 +194,7 @@ std::shared_ptr<arrow::Array> StructArray__GetFieldByName(
 // [[arrow::export]]
 arrow::ArrayVector StructArray__Flatten(
     const std::shared_ptr<arrow::StructArray>& array) {
-  return VALUE_OR_STOP(array->Flatten());
+  return ValueOrStop(array->Flatten());
 }
 
 // [[arrow::export]]

r/src/array_from_vector.cpp

@@ -203,13 +203,13 @@ struct VectorToArrayConverter {
 
   static std::shared_ptr<Array> Visit(SEXP x, const std::shared_ptr<DataType>& type) {
     std::unique_ptr<ArrayBuilder> builder;
-    STOP_IF_NOT_OK(MakeBuilder(arrow::default_memory_pool(), type, &builder));
+    StopIfNotOk(MakeBuilder(arrow::default_memory_pool(), type, &builder));
 
     VectorToArrayConverter converter{x, builder.get()};
-    STOP_IF_NOT_OK(arrow::VisitTypeInline(*type, &converter));
+    StopIfNotOk(arrow::VisitTypeInline(*type, &converter));
 
     std::shared_ptr<Array> result;
-    STOP_IF_NOT_OK(builder->Finish(&result));
+    StopIfNotOk(builder->Finish(&result));
     return result;
   }
 
@@ -228,7 +228,7 @@ std::shared_ptr<Array> MakeFactorArrayImpl(Rcpp::IntegerVector_ factor,
   auto n = factor.size();
 
   std::shared_ptr<Buffer> indices_buffer =
-      VALUE_OR_STOP(AllocateBuffer(n * sizeof(value_type)));
+      ValueOrStop(AllocateBuffer(n * sizeof(value_type)));
 
   std::vector<std::shared_ptr<Buffer>> buffers{nullptr, indices_buffer};
 
@@ -243,7 +243,7 @@ std::shared_ptr<Array> MakeFactorArrayImpl(Rcpp::IntegerVector_ factor,
 
   if (i < n) {
     // there are NA's so we need a null buffer
-    auto null_buffer = VALUE_OR_STOP(AllocateBuffer(BitUtil::BytesForBits(n)));
+    auto null_buffer = ValueOrStop(AllocateBuffer(BitUtil::BytesForBits(n)));
     internal::FirstTimeBitmapWriter null_bitmap_writer(null_buffer->mutable_data(), 0, n);
 
     // catch up
@@ -273,7 +273,7 @@ std::shared_ptr<Array> MakeFactorArrayImpl(Rcpp::IntegerVector_ factor,
   SEXP levels = Rf_getAttrib(factor, R_LevelsSymbol);
   auto dict = MakeStringArray(levels, utf8());
 
-  return VALUE_OR_STOP(DictionaryArray::FromArrays(type, array_indices, dict));
+  return ValueOrStop(DictionaryArray::FromArrays(type, array_indices, dict));
 }
 
 std::shared_ptr<Array> MakeFactorArray(Rcpp::IntegerVector_ factor,
@@ -1070,7 +1070,7 @@ std::shared_ptr<Array> MakeSimpleArray(SEXP x) {
 
   auto first_na = std::find_if(p_vec_start, p_vec_end, is_na<value_type>);
   if (first_na < p_vec_end) {
-    auto null_bitmap = VALUE_OR_STOP(AllocateBuffer(BitUtil::BytesForBits(n)));
+    auto null_bitmap = ValueOrStop(AllocateBuffer(BitUtil::BytesForBits(n)));
     internal::FirstTimeBitmapWriter bitmap_writer(null_bitmap->mutable_data(), 0, n);
 
     // first loop to clear all the bits before the first NA
@@ -1192,25 +1192,25 @@ std::shared_ptr<arrow::Array> Array__from_vector(
   // struct types
   if (type->id() == Type::STRUCT) {
     if (!type_inferred) {
-      STOP_IF_NOT_OK(arrow::r::CheckCompatibleStruct(x, type));
+      StopIfNotOk(arrow::r::CheckCompatibleStruct(x, type));
     }
 
     return arrow::r::MakeStructArray(x, type);
   }
 
   // general conversion with converter and builder
   std::unique_ptr<arrow::r::VectorConverter> converter;
-  STOP_IF_NOT_OK(arrow::r::GetConverter(type, &converter));
+  StopIfNotOk(arrow::r::GetConverter(type, &converter));
 
   // Create ArrayBuilder for type
   std::unique_ptr<arrow::ArrayBuilder> type_builder;
-  STOP_IF_NOT_OK(arrow::MakeBuilder(arrow::default_memory_pool(), type, &type_builder));
-  STOP_IF_NOT_OK(converter->Init(type_builder.get()));
+  StopIfNotOk(arrow::MakeBuilder(arrow::default_memory_pool(), type, &type_builder));
+  StopIfNotOk(converter->Init(type_builder.get()));
 
   // ingest R data and grab the result array
-  STOP_IF_NOT_OK(converter->Ingest(x));
+  StopIfNotOk(converter->Ingest(x));
   std::shared_ptr<arrow::Array> result;
-  STOP_IF_NOT_OK(converter->GetResult(&result));
+  StopIfNotOk(converter->GetResult(&result));
 
   return result;
 }
@@ -1261,8 +1261,8 @@ std::shared_ptr<arrow::ChunkedArray> ChunkedArray__from_list(Rcpp::List chunks,
   if (n == 0) {
     std::shared_ptr<arrow::Array> array;
     std::unique_ptr<arrow::ArrayBuilder> type_builder;
-    STOP_IF_NOT_OK(arrow::MakeBuilder(arrow::default_memory_pool(), type, &type_builder));
-    STOP_IF_NOT_OK(type_builder->Finish(&array));
+    StopIfNotOk(arrow::MakeBuilder(arrow::default_memory_pool(), type, &type_builder));
+    StopIfNotOk(type_builder->Finish(&array));
     vec.push_back(array);
   } else {
     // the first - might differ from the rest of the loop
@@ -1285,7 +1285,7 @@ std::shared_ptr<arrow::Array> DictionaryArray__FromArrays(
     const std::shared_ptr<arrow::DataType>& type,
     const std::shared_ptr<arrow::Array>& indices,
     const std::shared_ptr<arrow::Array>& dict) {
-  return VALUE_OR_STOP(arrow::DictionaryArray::FromArrays(type, indices, dict));
+  return ValueOrStop(arrow::DictionaryArray::FromArrays(type, indices, dict));
 }
 
 #endif

r/src/array_to_vector.cpp

@@ -129,7 +129,7 @@ Status SomeNull_Ingest(SEXP data, R_xlen_t start, R_xlen_t n,
 SEXP ArrayVector__as_vector(R_xlen_t n, const ArrayVector& arrays) {
   auto converter = Converter::Make(arrays);
   Shield<SEXP> data(converter->Allocate(n));
-  STOP_IF_NOT_OK(converter->IngestSerial(data));
+  StopIfNotOk(converter->IngestSerial(data));
   return data;
 }
 
@@ -387,7 +387,7 @@ class Converter_Struct : public Converter {
   Status Ingest_all_nulls(SEXP data, R_xlen_t start, R_xlen_t n) const {
     int nf = converters.size();
     for (int i = 0; i < nf; i++) {
-      STOP_IF_NOT_OK(converters[i]->Ingest_all_nulls(VECTOR_ELT(data, i), start, n));
+      StopIfNotOk(converters[i]->Ingest_all_nulls(VECTOR_ELT(data, i), start, n));
     }
     return Status::OK();
   }
@@ -397,9 +397,9 @@ class Converter_Struct : public Converter {
     auto struct_array = internal::checked_cast<arrow::StructArray*>(array.get());
     int nf = converters.size();
     // Flatten() deals with merging of nulls
-    auto arrays = VALUE_OR_STOP(struct_array->Flatten(default_memory_pool()));
+    auto arrays = ValueOrStop(struct_array->Flatten(default_memory_pool()));
     for (int i = 0; i < nf; i++) {
-      STOP_IF_NOT_OK(
+      StopIfNotOk(
           converters[i]->Ingest_some_nulls(VECTOR_ELT(data, i), arrays[i], start, n));
     }
 
@@ -760,7 +760,7 @@ Rcpp::List to_dataframe_serial(
 
   for (int i = 0; i < nc; i++) {
     SEXP column = tbl[i] = converters[i]->Allocate(nr);
-    STOP_IF_NOT_OK(converters[i]->IngestSerial(column));
+    StopIfNotOk(converters[i]->IngestSerial(column));
   }
   tbl.attr("names") = names;
   tbl.attr("class") = Rcpp::CharacterVector::create("tbl_df", "tbl", "data.frame");
@@ -801,7 +801,7 @@ Rcpp::List to_dataframe_parallel(
   // wait for the ingestion to be finished
   status &= tg->Finish();
 
-  STOP_IF_NOT_OK(status);
+  StopIfNotOk(status);
 
   tbl.attr("names") = names;
   tbl.attr("class") = Rcpp::CharacterVector::create("tbl_df", "tbl", "data.frame");

r/src/arrow_types.h

@@ -43,14 +43,6 @@ struct data {
 }  // namespace r
 }  // namespace arrow
 
-#define STOP_IF_NOT(TEST, MSG)    \
-  do {                            \
-    if (!(TEST)) Rcpp::stop(MSG); \
-  } while (0)
-
-#define STOP_IF_NOT_OK(status) StopIfNotOk(status)
-#define VALUE_OR_STOP(result) ValueOrStop(result)
-
 template <typename T>
 struct NoDelete {
   inline void operator()(T* ptr) {}
@@ -244,18 +236,18 @@ namespace fs = ::arrow::fs;
 
 namespace arrow {
 
-template <typename R>
-auto ValueOrStop(R&& result) -> decltype(std::forward<R>(result).ValueOrDie()) {
-  STOP_IF_NOT_OK(result.status());
-  return std::forward<R>(result).ValueOrDie();
-}
-
 static inline void StopIfNotOk(const Status& status) {
   if (!(status.ok())) {
     Rcpp::stop(status.ToString());
   }
 }
 
+template <typename R>
+auto ValueOrStop(R&& result) -> decltype(std::forward<R>(result).ValueOrDie()) {
+  StopIfNotOk(result.status());
+  return std::forward<R>(result).ValueOrDie();
+}
+
 namespace r {
 
 Status count_fields(SEXP lst, int* out);

r/src/chunkedarray.cpp

@@ -74,12 +74,12 @@ std::shared_ptr<arrow::ChunkedArray> ChunkedArray__Slice2(
 std::shared_ptr<arrow::ChunkedArray> ChunkedArray__View(
     const std::shared_ptr<arrow::ChunkedArray>& array,
     const std::shared_ptr<arrow::DataType>& type) {
-  return VALUE_OR_STOP(array->View(type));
+  return ValueOrStop(array->View(type));
 }
 
 // [[arrow::export]]
 void ChunkedArray__Validate(const std::shared_ptr<arrow::ChunkedArray>& chunked_array) {
-  STOP_IF_NOT_OK(chunked_array->Validate());
+  StopIfNotOk(chunked_array->Validate());
 }
 
 // [[arrow::export]]

r/src/compression.cpp

@@ -22,7 +22,7 @@
 // [[arrow::export]]
 std::unique_ptr<arrow::util::Codec> util___Codec__Create(arrow::Compression::type codec,
                                                          int compression_level) {
-  return VALUE_OR_STOP(arrow::util::Codec::Create(codec, compression_level));
+  return ValueOrStop(arrow::util::Codec::Create(codec, compression_level));
 }
 
 // [[arrow::export]]
@@ -39,14 +39,14 @@ bool util___Codec__IsAvailable(arrow::Compression::type codec) {
 std::shared_ptr<arrow::io::CompressedOutputStream> io___CompressedOutputStream__Make(
     const std::unique_ptr<arrow::util::Codec>& codec,
     const std::shared_ptr<arrow::io::OutputStream>& raw) {
-  return VALUE_OR_STOP(arrow::io::CompressedOutputStream::Make(codec.get(), raw));
+  return ValueOrStop(arrow::io::CompressedOutputStream::Make(codec.get(), raw));
 }
 
 // [[arrow::export]]
 std::shared_ptr<arrow::io::CompressedInputStream> io___CompressedInputStream__Make(
     const std::unique_ptr<arrow::util::Codec>& codec,
     const std::shared_ptr<arrow::io::InputStream>& raw) {
-  return VALUE_OR_STOP(arrow::io::CompressedInputStream::Make(codec.get(), raw));
+  return ValueOrStop(arrow::io::CompressedInputStream::Make(codec.get(), raw));
 }
 
 #endif

r/src/compute.cpp

@@ -36,7 +36,7 @@ std::shared_ptr<arrow::Array> Array__cast(
     const std::shared_ptr<arrow::compute::CastOptions>& options) {
   std::shared_ptr<arrow::Array> out;
   arrow::compute::FunctionContext context;
-  STOP_IF_NOT_OK(arrow::compute::Cast(&context, *array, target_type, *options, &out));
+  StopIfNotOk(arrow::compute::Cast(&context, *array, target_type, *options, &out));
   return out;
 }
 
@@ -48,7 +48,7 @@ std::shared_ptr<arrow::ChunkedArray> ChunkedArray__cast(
   arrow::compute::Datum value(chunked_array);
   arrow::compute::Datum out;
   arrow::compute::FunctionContext context;
-  STOP_IF_NOT_OK(arrow::compute::Cast(&context, value, target_type, *options, &out));
+  StopIfNotOk(arrow::compute::Cast(&context, value, target_type, *options, &out));
   return out.chunked_array();
 }
 
@@ -88,7 +88,7 @@ std::shared_ptr<arrow::Array> Array__Take(const std::shared_ptr<arrow::Array>& v
   std::shared_ptr<arrow::Array> out;
   arrow::compute::FunctionContext context;
   arrow::compute::TakeOptions options;
-  STOP_IF_NOT_OK(arrow::compute::Take(&context, *values, *indices, options, &out));
+  StopIfNotOk(arrow::compute::Take(&context, *values, *indices, options, &out));
   return out;
 }
 
@@ -100,7 +100,7 @@ std::shared_ptr<arrow::ChunkedArray> Array__TakeChunked(
   arrow::compute::FunctionContext context;
   arrow::compute::TakeOptions options;
 
-  STOP_IF_NOT_OK(arrow::compute::Take(&context, *values, *indices, options, &out));
+  StopIfNotOk(arrow::compute::Take(&context, *values, *indices, options, &out));
   return out;
 }
 
@@ -111,7 +111,7 @@ std::shared_ptr<arrow::RecordBatch> RecordBatch__Take(
   std::shared_ptr<arrow::RecordBatch> out;
   arrow::compute::FunctionContext context;
   arrow::compute::TakeOptions options;
-  STOP_IF_NOT_OK(arrow::compute::Take(&context, *batch, *indices, options, &out));
+  StopIfNotOk(arrow::compute::Take(&context, *batch, *indices, options, &out));
   return out;
 }
 
@@ -123,7 +123,7 @@ std::shared_ptr<arrow::ChunkedArray> ChunkedArray__Take(
   arrow::compute::FunctionContext context;
   arrow::compute::TakeOptions options;
 
-  STOP_IF_NOT_OK(arrow::compute::Take(&context, *values, *indices, options, &out));
+  StopIfNotOk(arrow::compute::Take(&context, *values, *indices, options, &out));
   return out;
 }
 
@@ -135,7 +135,7 @@ std::shared_ptr<arrow::ChunkedArray> ChunkedArray__TakeChunked(
   arrow::compute::FunctionContext context;
   arrow::compute::TakeOptions options;
 
-  STOP_IF_NOT_OK(arrow::compute::Take(&context, *values, *indices, options, &out));
+  StopIfNotOk(arrow::compute::Take(&context, *values, *indices, options, &out));
   return out;
 }
 
@@ -146,7 +146,7 @@ std::shared_ptr<arrow::Table> Table__Take(const std::shared_ptr<arrow::Table>& t
   arrow::compute::FunctionContext context;
   arrow::compute::TakeOptions options;
 
-  STOP_IF_NOT_OK(arrow::compute::Take(&context, *table, *indices, options, &out));
+  StopIfNotOk(arrow::compute::Take(&context, *table, *indices, options, &out));
   return out;
 }
 
@@ -158,7 +158,7 @@ std::shared_ptr<arrow::Table> Table__TakeChunked(
   arrow::compute::FunctionContext context;
   arrow::compute::TakeOptions options;
 
-  STOP_IF_NOT_OK(arrow::compute::Take(&context, *table, *indices, options, &out));
+  StopIfNotOk(arrow::compute::Take(&context, *table, *indices, options, &out));
   return out;
 }
 
@@ -173,7 +173,7 @@ std::shared_ptr<arrow::Array> Array__Filter(const std::shared_ptr<arrow::Array>&
   if (keep_na) {
     options.null_selection_behavior = arrow::compute::FilterOptions::EMIT_NULL;
   }
-  STOP_IF_NOT_OK(arrow::compute::Filter(&context, values, filter, {}, &out));
+  StopIfNotOk(arrow::compute::Filter(&context, values, filter, {}, &out));
   return out.make_array();
 }
 
@@ -188,7 +188,7 @@ std::shared_ptr<arrow::RecordBatch> RecordBatch__Filter(
   if (keep_na) {
     options.null_selection_behavior = arrow::compute::FilterOptions::EMIT_NULL;
   }
-  STOP_IF_NOT_OK(arrow::compute::Filter(&context, batch, filter, options, &out));
+  StopIfNotOk(arrow::compute::Filter(&context, batch, filter, options, &out));
   return out.record_batch();
 }
 
@@ -203,7 +203,7 @@ std::shared_ptr<arrow::ChunkedArray> ChunkedArray__Filter(
   if (keep_na) {
     options.null_selection_behavior = arrow::compute::FilterOptions::EMIT_NULL;
   }
-  STOP_IF_NOT_OK(arrow::compute::Filter(&context, values, filter, options, &out));
+  StopIfNotOk(arrow::compute::Filter(&context, values, filter, options, &out));
   return out.chunked_array();
 }
 
@@ -218,7 +218,7 @@ std::shared_ptr<arrow::ChunkedArray> ChunkedArray__FilterChunked(
   if (keep_na) {
     options.null_selection_behavior = arrow::compute::FilterOptions::EMIT_NULL;
   }
-  STOP_IF_NOT_OK(arrow::compute::Filter(&context, values, filter, options, &out));
+  StopIfNotOk(arrow::compute::Filter(&context, values, filter, options, &out));
   return out.chunked_array();
 }
 
@@ -233,7 +233,7 @@ std::shared_ptr<arrow::Table> Table__Filter(const std::shared_ptr<arrow::Table>&
   if (keep_na) {
     options.null_selection_behavior = arrow::compute::FilterOptions::EMIT_NULL;
   }
-  STOP_IF_NOT_OK(arrow::compute::Filter(&context, table, filter, options, &out));
+  StopIfNotOk(arrow::compute::Filter(&context, table, filter, options, &out));
   std::shared_ptr<arrow::Table> tab = out.table();
   if (tab->num_rows() == 0) {
     // Slight hack: if there are no rows in the result, instead do a 0-length
@@ -255,7 +255,7 @@ std::shared_ptr<arrow::Table> Table__FilterChunked(
   if (keep_na) {
     options.null_selection_behavior = arrow::compute::FilterOptions::EMIT_NULL;
   }
-  STOP_IF_NOT_OK(arrow::compute::Filter(&context, table, filter, options, &out));
+  StopIfNotOk(arrow::compute::Filter(&context, table, filter, options, &out));
   std::shared_ptr<arrow::Table> tab = out.table();
   if (tab->num_rows() == 0) {
     // Slight hack: if there are no rows in the result, instead do a 0-length