ARROW-13444: [C++] Remove usage of deprecated std::result_of

Also clean up some UBSAN nullptr arithmetic warnings Closes apache#10814 from bkietz/13444-C20-compatibility-by-upda Authored-by: Benjamin Kietzman <[email protected]> Signed-off-by: Benjamin Kietzman <[email protected]>
davidhao3300 · Jul 27, 2021 · 4c02c6d · 4c02c6d
1 parent 551c07c
commit 4c02c6d
Show file tree

Hide file tree

Showing 10 changed files with 30 additions and 26 deletions.
diff --git a/cpp/src/arrow/buffer_builder.h b/cpp/src/arrow/buffer_builder.h
@@ -45,8 +45,7 @@ class ARROW_EXPORT BufferBuilder {
   explicit BufferBuilder(MemoryPool* pool = default_memory_pool())
       : pool_(pool),
         data_(/*ensure never null to make ubsan happy and avoid check penalties below*/
-              &util::internal::non_null_filler),
-
+              util::MakeNonNull<uint8_t>()),
         capacity_(0),
         size_(0) {}
 

diff --git a/cpp/src/arrow/result.h b/cpp/src/arrow/result.h
@@ -385,7 +385,8 @@ class ARROW_MUST_USE_TYPE Result : public util::EqualityComparable<Result<T>> {
   /// Apply a function to the internally stored value to produce a new result or propagate
   /// the stored error.
   template <typename M>
-  typename EnsureResult<typename std::result_of<M && (T)>::type>::type Map(M&& m) && {
+  typename EnsureResult<decltype(std::declval<M&&>()(std::declval<T&&>()))>::type Map(
+      M&& m) && {
     if (!ok()) {
       return status();
     }
@@ -395,8 +396,8 @@ class ARROW_MUST_USE_TYPE Result : public util::EqualityComparable<Result<T>> {
   /// Apply a function to the internally stored value to produce a new result or propagate
   /// the stored error.
   template <typename M>
-  typename EnsureResult<typename std::result_of<M && (const T&)>::type>::type Map(
-      M&& m) const& {
+  typename EnsureResult<decltype(std::declval<M&&>()(std::declval<const T&>()))>::type
+  Map(M&& m) const& {
     if (!ok()) {
       return status();
     }

diff --git a/cpp/src/arrow/testing/util.h b/cpp/src/arrow/testing/util.h
@@ -123,7 +123,7 @@ struct VisitBuilderImpl {
   template <typename T, typename BuilderType = typename TypeTraits<T>::BuilderType,
             // need to let SFINAE drop this Visit when it would result in
             // [](NullBuilder*){}(double_builder)
-            typename E = typename std::result_of<Fn(BuilderType*)>::type>
+            typename = decltype(std::declval<Fn>()(std::declval<BuilderType*>()))>
   Status Visit(const T&) {
     fn_(internal::checked_cast<BuilderType*>(builder_));
     return Status::OK();

diff --git a/cpp/src/arrow/util/bit_block_counter.cc b/cpp/src/arrow/util/bit_block_counter.cc
@@ -37,19 +37,12 @@ BitBlockCount BitBlockCounter::GetBlockSlow(int64_t block_size) noexcept {
   return {run_length, popcount};
 }
 
-// Prevent pointer arithmetic on nullptr, which is undefined behavior even if the pointer
-// is never dereferenced.
-inline const uint8_t* EnsureNotNull(const uint8_t* ptr) {
-  static const uint8_t byte{};
-  return ptr == nullptr ? &byte : ptr;
-}
-
 OptionalBitBlockCounter::OptionalBitBlockCounter(const uint8_t* validity_bitmap,
                                                  int64_t offset, int64_t length)
     : has_bitmap_(validity_bitmap != nullptr),
       position_(0),
       length_(length),
-      counter_(EnsureNotNull(validity_bitmap), offset, length) {}
+      counter_(util::MakeNonNull(validity_bitmap), offset, length) {}
 
 OptionalBitBlockCounter::OptionalBitBlockCounter(
     const std::shared_ptr<Buffer>& validity_bitmap, int64_t offset, int64_t length)
@@ -64,10 +57,11 @@ OptionalBinaryBitBlockCounter::OptionalBinaryBitBlockCounter(const uint8_t* left
     : has_bitmap_(HasBitmapFromBitmaps(left_bitmap != nullptr, right_bitmap != nullptr)),
       position_(0),
       length_(length),
-      unary_counter_(EnsureNotNull(left_bitmap != nullptr ? left_bitmap : right_bitmap),
-                     left_bitmap != nullptr ? left_offset : right_offset, length),
-      binary_counter_(EnsureNotNull(left_bitmap), left_offset,
-                      EnsureNotNull(right_bitmap), right_offset, length) {}
+      unary_counter_(
+          util::MakeNonNull(left_bitmap != nullptr ? left_bitmap : right_bitmap),
+          left_bitmap != nullptr ? left_offset : right_offset, length),
+      binary_counter_(util::MakeNonNull(left_bitmap), left_offset,
+                      util::MakeNonNull(right_bitmap), right_offset, length) {}
 
 OptionalBinaryBitBlockCounter::OptionalBinaryBitBlockCounter(
     const std::shared_ptr<Buffer>& left_bitmap, int64_t left_offset,

diff --git a/cpp/src/arrow/util/bit_run_reader.h b/cpp/src/arrow/util/bit_run_reader.h
@@ -197,7 +197,7 @@ class BaseSetBitRunReader {
   /// \param[in] length number of bits to copy
   ARROW_NOINLINE
   BaseSetBitRunReader(const uint8_t* bitmap, int64_t start_offset, int64_t length)
-      : bitmap_(bitmap),
+      : bitmap_(util::MakeNonNull(bitmap)),
         length_(length),
         remaining_(length_),
         current_word_(0),

diff --git a/cpp/src/arrow/util/bitmap_generate.h b/cpp/src/arrow/util/bitmap_generate.h
@@ -62,7 +62,7 @@ void GenerateBits(uint8_t* bitmap, int64_t start_offset, int64_t length, Generat
 template <class Generator>
 void GenerateBitsUnrolled(uint8_t* bitmap, int64_t start_offset, int64_t length,
                           Generator&& g) {
-  static_assert(std::is_same<typename std::result_of<Generator && ()>::type, bool>::value,
+  static_assert(std::is_same<decltype(std::declval<Generator>()()), bool>::value,
                 "Functor passed to GenerateBitsUnrolled must return bool");
 
   if (length == 0) {

diff --git a/cpp/src/arrow/util/bitmap_reader.h b/cpp/src/arrow/util/bitmap_reader.h
@@ -76,7 +76,7 @@ class BitmapReader {
 class BitmapUInt64Reader {
  public:
   BitmapUInt64Reader(const uint8_t* bitmap, int64_t start_offset, int64_t length)
-      : bitmap_(bitmap + start_offset / 8),
+      : bitmap_(util::MakeNonNull(bitmap) + start_offset / 8),
         num_carry_bits_(8 - start_offset % 8),
         length_(length),
         remaining_length_(length_) {

diff --git a/cpp/src/arrow/util/functional.h b/cpp/src/arrow/util/functional.h
@@ -129,7 +129,7 @@ class FnOnce<R(A...)> {
 
   template <typename Fn,
             typename = typename std::enable_if<std::is_convertible<
-                typename std::result_of<Fn && (A...)>::type, R>::value>::type>
+                decltype(std::declval<Fn&&>()(std::declval<A>()...)), R>::value>::type>
   FnOnce(Fn fn) : impl_(new FnImpl<Fn>(std::move(fn))) {  // NOLINT runtime/explicit
   }
 

diff --git a/cpp/src/arrow/util/future.h b/cpp/src/arrow/util/future.h
@@ -28,6 +28,7 @@
 #include "arrow/result.h"
 #include "arrow/status.h"
 #include "arrow/type_fwd.h"
+#include "arrow/type_traits.h"
 #include "arrow/util/functional.h"
 #include "arrow/util/macros.h"
 #include "arrow/util/optional.h"
@@ -47,8 +48,17 @@ struct is_future : std::false_type {};
 template <typename T>
 struct is_future<Future<T>> : std::true_type {};
 
+template <typename Signature, typename Enable = void>
+struct result_of;
+
+template <typename Fn, typename... A>
+struct result_of<Fn(A...),
+                 internal::void_t<decltype(std::declval<Fn>()(std::declval<A>()...))>> {
+  using type = decltype(std::declval<Fn>()(std::declval<A>()...));
+};
+
 template <typename Signature>
-using result_of_t = typename std::result_of<Signature>::type;
+using result_of_t = typename result_of<Signature>::type;
 
 // Helper to find the synchronous counterpart for a Future
 template <typename T>

diff --git a/cpp/src/arrow/util/ubsan.h b/cpp/src/arrow/util/ubsan.h
@@ -30,7 +30,7 @@ namespace util {
 
 namespace internal {
 
-static uint8_t non_null_filler;
+constexpr uint8_t kNonNullFiller = 0;
 
 }  // namespace internal
 
@@ -44,12 +44,12 @@ static uint8_t non_null_filler;
 /// https://github.com/google/flatbuffers/pull/5355 is trying to resolve
 /// them.
 template <typename T>
-inline T* MakeNonNull(T* maybe_null) {
+inline T* MakeNonNull(T* maybe_null = NULLPTR) {
   if (ARROW_PREDICT_TRUE(maybe_null != NULLPTR)) {
     return maybe_null;
   }
 
-  return reinterpret_cast<T*>(&internal::non_null_filler);
+  return const_cast<T*>(reinterpret_cast<const T*>(&internal::kNonNullFiller));
 }
 
 template <typename T>