vyasr commented on code in PR #40253:
URL: https://github.com/apache/arrow/pull/40253#discussion_r1504877416
##########
cpp/src/arrow/util/span.h:
##########
@@ -59,12 +59,13 @@ writing code which would break when it is replaced by
std::span.)");
template <
typename R,
typename DisableUnlessConstructibleFromDataAndSize =
- decltype(span<T>(std::data(std::declval<R>()),
std::size(std::declval<R>()))),
+ std::enable_if_t<std::is_convertible_v<
std::remove_pointer_t<decltype(std::data(std::declval<R&>()))> (*)[],
+ T(*)[]>>,
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>>>>
// NOLINTNEXTLINE runtime/explicit, non-const reference
- constexpr span(R&& range) : span{std::data(range), std::size(range)} {}
+ constexpr span(R& range) : span{std::data(range), std::size(range)} {}
Review Comment:
I ran into a different issue with the universal ref where the template also
matched an rvalue input, which is forbidden by one of the tests. I just
reverted that change so that you can see it. I believe that the test is correct
since you wouldn't want to create a span pointing to a temporary that is
immediately discarded. While rvalues can extend the lifetime of objects (see
the "rvalue references" section of
https://en.cppreference.com/w/cpp/language/reference) that wouldn't make this
kind of usage safe since the span would persist after that expression.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
