This is an automated email from the ASF dual-hosted git repository.
pitrou pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new 6219555bb1 GH-50250: [C++] Remove `call_traits::argument_type` in
favor of `<type_traits>` (#51328)
6219555bb1 is described below
commit 6219555bb133ea57d51fd38f56079f555bd7998f
Author: Alexander Taepper <[email protected]>
AuthorDate: Wed Sep 16 19:02:20 2026 +0200
GH-50250: [C++] Remove `call_traits::argument_type` in favor of
`<type_traits>` (#51328)
### Rationale for this change
This is the last change for #50250. The remaining `FnOnce` does not have
corresponding utilities in the C++20 standard library according to my knowledge.
### What changes are included in this PR?
This removes the custom meta-programming facility
`call_traits::argument_type`. We instead use the provided `type_traits` header
which allows us to check invocability (and thus indirectly the argument type)
with e.g. `std::is_invocable`.
It needs to be mentioned that none of `std::is_invocable_v`,
`std::invoke_result_t`, .. is a drop-in replacement for the removed
`argument_type`. I instead migrated all users of the old facility to the
standard constructs. Some call-sites needed changing by supplying template
parameters explicitly, but in my opinion all changes are defendable or even
improvements.
### Are these changes tested?
Yes, this refactoring commit still passes all test cases
### Are there any user-facing changes?
No
* GitHub Issue: #50250
Authored-by: Alexander Taepper <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
---
cpp/src/arrow/util/bit_util_benchmark.cc | 2 +-
cpp/src/arrow/util/bitmap.h | 20 ++++++++---------
cpp/src/arrow/util/cache_benchmark.cc | 5 +++--
cpp/src/arrow/util/cache_internal.h | 37 ++++++++++++++++----------------
cpp/src/arrow/util/cache_test.cc | 14 ++++++------
cpp/src/arrow/util/functional.h | 23 --------------------
cpp/src/arrow/util/future.h | 21 ++++++------------
cpp/src/arrow/util/future_test.cc | 12 +++++------
8 files changed, 49 insertions(+), 85 deletions(-)
diff --git a/cpp/src/arrow/util/bit_util_benchmark.cc
b/cpp/src/arrow/util/bit_util_benchmark.cc
index da624bec19..bc4cdad99c 100644
--- a/cpp/src/arrow/util/bit_util_benchmark.cc
+++ b/cpp/src/arrow/util/bit_util_benchmark.cc
@@ -172,7 +172,7 @@ static void BenchmarkBitmapVisitBitsetAnd(benchmark::State&
state) {
static void BenchmarkBitmapVisitUInt8And(benchmark::State& state) {
BenchmarkAndImpl(state, [](const internal::Bitmap(&bitmaps)[2],
internal::Bitmap* out) {
int64_t i = 0;
- internal::Bitmap::VisitWords(bitmaps, [&](std::array<uint8_t, 2> uint8s) {
+ internal::Bitmap::VisitWords<uint8_t>(bitmaps, [&](std::array<uint8_t, 2>
uint8s) {
reinterpret_cast<uint8_t*>(out->mutable_data())[i++] = uint8s[0] &
uint8s[1];
});
});
diff --git a/cpp/src/arrow/util/bitmap.h b/cpp/src/arrow/util/bitmap.h
index 49f319081f..4cba55a8ad 100644
--- a/cpp/src/arrow/util/bitmap.h
+++ b/cpp/src/arrow/util/bitmap.h
@@ -21,12 +21,14 @@
#include <array>
#include <bitset>
#include <cassert>
+#include <concepts>
#include <cstdint>
#include <cstring>
#include <memory>
#include <span>
#include <string>
#include <string_view>
+#include <type_traits>
#include <utility>
#include "arrow/buffer.h"
@@ -36,7 +38,6 @@
#include "arrow/util/bitmap_writer.h"
#include "arrow/util/compare.h"
#include "arrow/util/endian.h"
-#include "arrow/util/functional.h"
#include "arrow/util/string_util.h"
#include "arrow/util/visibility.h"
@@ -145,9 +146,8 @@ class ARROW_EXPORT Bitmap : public
util::ToStringOstreamable<Bitmap>,
// carefully in other cases.
// For 2 bitmaps or less, and/or smaller bitmaps, see also
VisitTwoBitBlocksVoid
// and BitmapUInt64Reader.
- template <size_t N, typename Visitor,
- typename Word = typename std::decay<
- internal::call_traits::argument_type<0,
Visitor&&>>::type::value_type>
+ template <typename Word = uint64_t, size_t N, typename Visitor>
+ requires std::same_as<void, std::invoke_result_t<Visitor, std::array<Word,
N>&>>
static int64_t VisitWords(const Bitmap (&bitmaps_arg)[N], Visitor&& visitor)
{
constexpr int64_t kBitWidth = sizeof(Word) * 8;
@@ -248,13 +248,12 @@ class ARROW_EXPORT Bitmap : public
util::ToStringOstreamable<Bitmap>,
return min_offset;
}
- template <size_t N, size_t M, typename ReaderT, typename WriterT, typename
Visitor,
- typename Word = typename std::decay<
- internal::call_traits::argument_type<0,
Visitor&&>>::type::value_type>
+ template <size_t N, size_t M, typename ReaderT, typename WriterT, typename
Visitor>
static void RunVisitWordsAndWriteLoop(int64_t bit_length,
std::array<ReaderT, N>& readers,
std::array<WriterT, M>& writers,
Visitor&& visitor) {
+ using Word = decltype(readers[0].NextWord());
constexpr int64_t kBitWidth = sizeof(Word) * 8;
std::array<Word, N> visited_words;
@@ -318,6 +317,7 @@ class ARROW_EXPORT Bitmap : public
util::ToStringOstreamable<Bitmap>,
/// may be offset within the first visited word, but words will otherwise
contain
/// densely packed bits loaded from the bitmap. That offset within the first
word is
/// returned.
+ ///
/// Visitor is expected to have the following signature
/// [](const std::array<Word, N>& in_words, std::array<Word, M>*
out_words){...}
///
@@ -326,9 +326,9 @@ class ARROW_EXPORT Bitmap : public
util::ToStringOstreamable<Bitmap>,
// carefully in other cases.
// For 2 bitmaps or less, and/or smaller bitmaps, see also
VisitTwoBitBlocksVoid
// and BitmapUInt64Reader.
- template <size_t N, size_t M, typename Visitor,
- typename Word = typename std::decay<
- internal::call_traits::argument_type<0,
Visitor&&>>::type::value_type>
+ template <typename Word = uint64_t, size_t N, size_t M, typename Visitor>
+ requires std::same_as<
+ void, std::invoke_result_t<Visitor, std::array<Word, N>&,
std::array<Word, M>*>>
static void VisitWordsAndWrite(const std::array<Bitmap, N>& bitmaps_arg,
std::array<Bitmap, M>* out_bitmaps_arg,
Visitor&& visitor) {
diff --git a/cpp/src/arrow/util/cache_benchmark.cc
b/cpp/src/arrow/util/cache_benchmark.cc
index 7439ee2f50..071367d8ec 100644
--- a/cpp/src/arrow/util/cache_benchmark.cc
+++ b/cpp/src/arrow/util/cache_benchmark.cc
@@ -126,7 +126,7 @@ static void BenchmarkMemoize(benchmark::State& state,
Memoized&& mem,
static void MemoizeLruCached(benchmark::State& state) {
const auto keys = MakeStrings(kCacheSize, state.range(0));
const auto values = MakeStrings(kCacheSize, state.range(1));
- auto mem = MemoizeLru(Callable(values), kCacheSize);
+ auto mem = MemoizeLru<std::string>(Callable(values), kCacheSize);
BenchmarkMemoize(state, mem, keys);
}
@@ -135,7 +135,8 @@ static void MemoizeLruCachedThreadUnsafe(benchmark::State&
state) {
const auto values = MakeStrings(kCacheSize, state.range(1));
// Emulate recommended usage of MemoizeLruCachedThreadUnsafe
// (the compiler is probably able to cache the TLS-looked up value, though)
- thread_local auto mem = MemoizeLruThreadUnsafe(Callable(values), kCacheSize);
+ thread_local auto mem =
+ MemoizeLruThreadUnsafe<std::string>(Callable(values), kCacheSize);
BenchmarkMemoize(state, mem, keys);
}
diff --git a/cpp/src/arrow/util/cache_internal.h
b/cpp/src/arrow/util/cache_internal.h
index fe10af8bb2..6c65712060 100644
--- a/cpp/src/arrow/util/cache_internal.h
+++ b/cpp/src/arrow/util/cache_internal.h
@@ -26,7 +26,6 @@
#include <utility>
#include <vector>
-#include "arrow/util/functional.h"
#include "arrow/util/logging.h"
#include "arrow/util/macros.h"
@@ -167,11 +166,12 @@ struct ThreadUnsafeMemoizer {
};
template <template <typename...> class Cache, template <typename...> class
MemoizerType,
- typename Func, typename Key =
std::decay_t<call_traits::argument_type<0, Func>>,
- typename Value = std::decay_t<std::invoke_result_t<Func, const
Key&>>,
- typename Memoizer = MemoizerType<Key, Value, Cache<Key, Value>,
Func>,
- typename RetType = typename Memoizer::RetType>
-static std::function<RetType(const Key&)> Memoize(Func&& func, int32_t
cache_capacity) {
+ typename Key, typename Func>
+static auto Memoize(Func&& func, int32_t cache_capacity) {
+ using Value = std::decay_t<std::invoke_result_t<Func, const Key&>>;
+ using Memoizer = MemoizerType<Key, Value, Cache<Key, Value>, Func>;
+ using RetType = typename Memoizer::RetType;
+
// std::function<> requires copy constructibility
struct {
RetType operator()(const Key& key) const { return (*memoized_)(key); }
@@ -179,30 +179,29 @@ static std::function<RetType(const Key&)> Memoize(Func&&
func, int32_t cache_cap
} shared_memoized = {
std::make_shared<Memoizer>(std::forward<Func>(func), cache_capacity)};
- return shared_memoized;
+ return std::function<RetType(const Key&)>(std::move(shared_memoized));
}
} // namespace detail
// Apply a LRU memoization cache to a callable.
-template <typename Func>
-static auto MemoizeLru(Func&& func, int32_t cache_capacity)
- -> decltype(detail::Memoize<LruCache, detail::ThreadSafeMemoizer>(
- std::forward<Func>(func), cache_capacity)) {
- return detail::Memoize<LruCache,
detail::ThreadSafeMemoizer>(std::forward<Func>(func),
- cache_capacity);
+// `Key` is the type of the callable's (single) argument.
+template <typename Key, typename Func>
+ requires std::is_invocable_v<Func, const Key&>
+static auto MemoizeLru(Func&& func, int32_t cache_capacity) {
+ return detail::Memoize<LruCache, detail::ThreadSafeMemoizer, Key>(
+ std::forward<Func>(func), cache_capacity);
}
// Like MemoizeLru, but not thread-safe. This version allows for much faster
// lookups (more than 2x faster), but you'll have to manage thread safety
yourself.
// A recommended usage is to declare per-thread caches using `thread_local`
// (see cache_benchmark.cc).
-template <typename Func>
-static auto MemoizeLruThreadUnsafe(Func&& func, int32_t cache_capacity)
- -> decltype(detail::Memoize<LruCache, detail::ThreadUnsafeMemoizer>(
- std::forward<Func>(func), cache_capacity)) {
- return detail::Memoize<LruCache,
detail::ThreadUnsafeMemoizer>(std::forward<Func>(func),
-
cache_capacity);
+template <typename Key, typename Func>
+ requires std::is_invocable_v<Func, const Key&>
+static auto MemoizeLruThreadUnsafe(Func&& func, int32_t cache_capacity) {
+ return detail::Memoize<LruCache, detail::ThreadUnsafeMemoizer, Key>(
+ std::forward<Func>(func), cache_capacity);
}
} // namespace internal
diff --git a/cpp/src/arrow/util/cache_test.cc b/cpp/src/arrow/util/cache_test.cc
index 264bfe68ec..d009806ab3 100644
--- a/cpp/src/arrow/util/cache_test.cc
+++ b/cpp/src/arrow/util/cache_test.cc
@@ -183,18 +183,16 @@ struct Callable {
};
struct MemoizeLruFactory {
- template <typename Func,
- typename RetType = decltype(MemoizeLru(std::declval<Func>(), 0))>
- RetType operator()(Func&& func, int32_t capacity) {
- return MemoizeLru(std::forward<Func>(func), capacity);
+ template <typename Func>
+ auto operator()(Func&& func, int32_t capacity) {
+ return MemoizeLru<std::string>(std::forward<Func>(func), capacity);
}
};
struct MemoizeLruThreadUnsafeFactory {
- template <typename Func,
- typename RetType =
decltype(MemoizeLruThreadUnsafe(std::declval<Func>(), 0))>
- RetType operator()(Func&& func, int32_t capacity) {
- return MemoizeLruThreadUnsafe(std::forward<Func>(func), capacity);
+ template <typename Func>
+ auto operator()(Func&& func, int32_t capacity) {
+ return MemoizeLruThreadUnsafe<std::string>(std::forward<Func>(func),
capacity);
}
};
diff --git a/cpp/src/arrow/util/functional.h b/cpp/src/arrow/util/functional.h
index 4444b747c8..1240f6c6ca 100644
--- a/cpp/src/arrow/util/functional.h
+++ b/cpp/src/arrow/util/functional.h
@@ -18,7 +18,6 @@
#pragma once
#include <memory>
-#include <tuple>
#include <type_traits>
#include "arrow/result.h"
@@ -36,28 +35,6 @@ struct Empty {
}
};
-/// Helper struct for examining lambdas and other callables.
-/// TODO(ARROW-12655) support function pointers
-struct call_traits {
- public:
- template <std::size_t I, typename F, typename R, typename... A>
- static typename std::tuple_element<I, std::tuple<A...>>::type
argument_type_impl(
- R (F::*)(A...));
-
- template <std::size_t I, typename F, typename R, typename... A>
- static typename std::tuple_element<I, std::tuple<A...>>::type
argument_type_impl(
- R (F::*)(A...) const);
-
- template <std::size_t I, typename F, typename R, typename... A>
- static typename std::tuple_element<I, std::tuple<A...>>::type
argument_type_impl(
- R (F::*)(A...) &&);
-
- /// If F is not overloaded, the argument types of its call operator can be
- /// extracted via call_traits::argument_type<Index, F>
- template <std::size_t I, typename F>
- using argument_type =
decltype(argument_type_impl<I>(&std::decay<F>::type::operator()));
-};
-
/// A type erased callable object which may only be invoked once.
/// It can be constructed from any lambda which matches the provided call
signature.
/// Invoking it results in destruction of the lambda, freeing any
state/references
diff --git a/cpp/src/arrow/util/future.h b/cpp/src/arrow/util/future.h
index fa45427a9d..1884f59697 100644
--- a/cpp/src/arrow/util/future.h
+++ b/cpp/src/arrow/util/future.h
@@ -61,10 +61,6 @@ struct SyncType<internal::Empty> {
using type = Status;
};
-template <typename Fn>
-using first_arg_is_status =
- std::is_same<std::decay_t<internal::call_traits::argument_type<0, Fn>>,
Status>;
-
template <typename Fn, typename Then, typename Else>
using if_has_no_args = std::conditional_t<std::is_invocable_v<Fn>, Then, Else>;
@@ -446,10 +442,12 @@ class [[nodiscard]] Future {
};
};
+ // conditional whether OnComplete is invokable with Status _and not with
Result_
template <typename OnComplete>
using WrapOnComplete = typename std::conditional<
- detail::first_arg_is_status<OnComplete>::value, WrapStatusyOnComplete,
- WrapResultOnComplete>::type::template Callback<OnComplete>;
+ std::is_invocable_v<OnComplete, const Status&> &&
+ !std::is_invocable_v<OnComplete, const Result<ValueType>&>,
+ WrapStatusyOnComplete, WrapResultOnComplete>::type::template
Callback<OnComplete>;
/// \brief Consumer API: Register a callback to run when this future
completes
///
@@ -514,15 +512,8 @@ class [[nodiscard]] Future {
ContinuedFuture>::value,
"OnSuccess and OnFailure must continue with the same future
type");
- struct DummyOnSuccess {
- void operator()(const T&);
- };
- using OnSuccessArg = typename
std::decay<internal::call_traits::argument_type<
- 0, detail::if_has_no_args<OnSuccess, DummyOnSuccess,
OnSuccess>>>::type;
-
- static_assert(
- !std::is_same<OnSuccessArg, typename
EnsureResult<OnSuccessArg>::type>::value,
- "OnSuccess' argument should not be a Result");
+ static_assert(!std::is_invocable_v<OnSuccess, const Result<T>&>,
+ "OnSuccess' argument should not be a Result");
void operator()(const Result<T>& result) && {
detail::ContinueFuture continue_future;
diff --git a/cpp/src/arrow/util/future_test.cc
b/cpp/src/arrow/util/future_test.cc
index 2ed2b69aed..1b55ddc6d8 100644
--- a/cpp/src/arrow/util/future_test.cc
+++ b/cpp/src/arrow/util/future_test.cc
@@ -1539,13 +1539,11 @@ TEST(FnOnceTest, MoveOnlyDataType) {
return *i0.data + *i1.data + (i0.moves * 1000) + (i1.moves * 100);
};
- using arg0 = call_traits::argument_type<0, decltype(fn)>;
- using arg1 = call_traits::argument_type<1, decltype(fn)>;
- using arg2 = call_traits::argument_type<2, decltype(fn)>;
- static_assert(std::is_same<arg0, const MoveOnlyDataType&>::value, "");
- static_assert(std::is_same<arg1, MoveOnlyDataType>::value, "");
- static_assert(std::is_same<arg2, std::string>::value,
- "should not add a && to the call type (demanding rvalue
unnecessarily)");
+ static_assert(std::is_invocable_r_v<int, decltype(fn), const
MoveOnlyDataType&,
+ MoveOnlyDataType, std::string&>);
+ // a move-only by-value argument must be moved in
+ static_assert(!std::is_invocable_v<decltype(fn), const MoveOnlyDataType&,
+ MoveOnlyDataType&, std::string&>);
MoveOnlyDataType i0{1}, i1{41};
std::string copyable = "";
