Skip to content

Commit

Permalink
apacheGH-44898: [C++] Fix compilation error on GCC 8 (apache#44899)
Browse files Browse the repository at this point in the history
### Rationale for this change

The `span` implementation featured a complicated SFINAE construct to enable the `span(range...)` constructor, which failed compiling on gcc 8.x.

### What changes are included in this PR?

Replace this complicated SFINAE with a much simpler one that avoids the recursive `span` template dependency while keeping the intended semantics.

### Are these changes tested?

Yes, by existing tests.

### Are there any user-facing changes?

No.
* GitHub Issue: apache#44898

Authored-by: Antoine Pitrou <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
  • Loading branch information
pitrou authored Dec 6, 2024
1 parent fe32a7d commit 9907f37
Showing 1 changed file with 5 additions and 29 deletions.
34 changes: 5 additions & 29 deletions cpp/src/arrow/util/span.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,28 +28,6 @@ namespace arrow::util {
template <class T>
class span;

// This trait is used to check if a type R can be used to construct a span<T>.
// Specifically, it checks if std::data(R) and std::size(R) are valid expressions
// that may be passed to the span(T*, size_t) constructor. The reason this trait
// is needed rather than expressing this directly in the relevant span constructor
// is that this check requires instantiating span<T>, which would violate the
// C++ standard if written directly in the constructor's enable_if clause
// because span<T> is an incomplete type at that point. By defining this trait
// instead, we add an extra level of indirection that lets us delay the
// evaluation of the template until the first time the associated constructor
// is actually called, at which point span<T> is a complete type.
//
// Note that most compilers do support the noncompliant construct, but nvcc
// does not. See https://github.com/apache/arrow/issues/40252
template <class T, class R, class Enable = void>
struct ConstructibleFromDataAndSize : std::false_type {};

template <class T, class R>
struct ConstructibleFromDataAndSize<
span<T>, R,
std::void_t<decltype(span<T>{std::data(std::declval<R>()),
std::size(std::declval<R>())})>> : std::true_type {};

/// std::span polyfill.
///
/// Does not support static extents.
Expand Down Expand Up @@ -81,14 +59,12 @@ writing code which would break when it is replaced by std::span.)");
constexpr span(T* begin, T* end)
: data_{begin}, size_{static_cast<size_t>(end - begin)} {}

template <
typename R,
std::enable_if_t<ConstructibleFromDataAndSize<span<T>, R>::value, bool> = true,
typename DisableUnlessSimilarTypes = std::enable_if_t<std::is_same_v<
std::decay_t<std::remove_pointer_t<decltype(std::data(std::declval<R>()))>>,
std::decay_t<T>>>>
template <typename R, typename RD = decltype(std::data(std::declval<R>())),
typename RS = decltype(std::size(std::declval<R>())),
typename E = std::enable_if_t<std::is_constructible_v<T*, RD> &&
std::is_constructible_v<size_t, RS>>>
// NOLINTNEXTLINE runtime/explicit, non-const reference
constexpr span(R&& range) : span{std::data(range), std::size(range)} {}
constexpr span(R&& range) : data_{std::data(range)}, size_{std::size(range)} {}

constexpr T* begin() const { return data_; }
constexpr T* end() const { return data_ + size_; }
Expand Down

0 comments on commit 9907f37

Please sign in to comment.