WillAyd commented on code in PR #48615:
URL: https://github.com/apache/arrow/pull/48615#discussion_r2643164604
##########
cpp/src/arrow/array/data.h:
##########
@@ -31,7 +31,7 @@
#include "arrow/type_fwd.h"
#include "arrow/util/bit_util.h"
#include "arrow/util/macros.h"
-#include "arrow/util/span.h"
+#include <span>
Review Comment:
Can you move the span include to be with the other stdlib headers, rather
than down here with the arrow headers?
##########
cpp/src/arrow/util/span_test.cc:
##########
@@ -22,61 +22,61 @@
#include "arrow/testing/gtest_util.h"
#include "arrow/testing/matchers.h"
-#include "arrow/util/span.h"
+#include <span>
using testing::ElementsAre;
using testing::ElementsAreArray;
using testing::PrintToString;
-namespace arrow::util {
+namespace arrow_util_span_test {
Review Comment:
Is this still required for other members?
##########
cpp/src/arrow/util/span_test.cc:
##########
@@ -22,61 +22,61 @@
#include "arrow/testing/gtest_util.h"
#include "arrow/testing/matchers.h"
-#include "arrow/util/span.h"
+#include <span>
using testing::ElementsAre;
using testing::ElementsAreArray;
using testing::PrintToString;
-namespace arrow::util {
+namespace arrow_util_span_test {
template <typename T>
-std::ostream& operator<<(std::ostream& os, const span<T>& span) {
+std::ostream& operator<<(std::ostream& os, const std::span<T>& span) {
// Inefficient but good enough for testing
os << PrintToString(std::vector(span.begin(), span.end()));
return os;
}
TEST(Span, Construction) {
// const spans may be constructed from mutable spans
- static_assert(std::is_constructible_v<span<const int>, span<int>>);
+ static_assert(std::is_constructible_v<std::span<const int>, std::span<int>>);
// ... but mutable spans may be constructed from const spans
- static_assert(!std::is_constructible_v<span<int>, span<const int>>);
+ static_assert(!std::is_constructible_v<std::span<int>, std::span<const
int>>);
int arr[] = {1, 2, 3};
constexpr int const_arr[] = {7, 8, 9};
- static_assert(std::is_constructible_v<span<int>, decltype(arr)&>);
- static_assert(!std::is_constructible_v<span<int>, decltype(const_arr)&>);
+ static_assert(std::is_constructible_v<std::span<int>, decltype(arr)&>);
+ static_assert(!std::is_constructible_v<std::span<int>,
decltype(const_arr)&>);
- static_assert(std::is_constructible_v<span<const int>, decltype(arr)&>);
- static_assert(std::is_constructible_v<span<const int>,
decltype(const_arr)&>);
+ static_assert(std::is_constructible_v<std::span<const int>, decltype(arr)&>);
+ static_assert(std::is_constructible_v<std::span<const int>,
decltype(const_arr)&>);
- static_assert(std::is_constructible_v<span<int>, std::vector<int>&>);
- static_assert(!std::is_constructible_v<span<int>, const std::vector<int>&>);
- static_assert(!std::is_constructible_v<span<int>, std::vector<int>&&>);
+ static_assert(std::is_constructible_v<std::span<int>, std::vector<int>&>);
+ static_assert(!std::is_constructible_v<std::span<int>, const
std::vector<int>&>);
+ static_assert(!std::is_constructible_v<std::span<int>, std::vector<int>&&>);
- static_assert(std::is_constructible_v<span<const int>, std::vector<int>&>);
- static_assert(std::is_constructible_v<span<const int>, const
std::vector<int>&>);
+ static_assert(std::is_constructible_v<std::span<const int>,
std::vector<int>&>);
+ static_assert(std::is_constructible_v<std::span<const int>, const
std::vector<int>&>);
// const spans may even be constructed from rvalue ranges
- static_assert(std::is_constructible_v<span<const int>, std::vector<int>&&>);
+ static_assert(std::is_constructible_v<std::span<const int>,
std::vector<int>&&>);
- EXPECT_THAT(span<const int>(const_arr), ElementsAreArray(const_arr));
- EXPECT_THAT(span<int>(arr), ElementsAreArray(arr));
+ EXPECT_THAT(std::span<const int>(const_arr), ElementsAreArray(const_arr));
+ EXPECT_THAT(std::span<int>(arr), ElementsAreArray(arr));
- static_assert(!std::is_constructible_v<span<const unsigned>,
decltype(const_arr)&>);
- static_assert(!std::is_constructible_v<span<const std::byte>,
decltype(const_arr)&>);
+ static_assert(!std::is_constructible_v<std::span<const unsigned>,
decltype(const_arr)&>);
+ static_assert(!std::is_constructible_v<std::span<const std::byte>,
decltype(const_arr)&>);
}
TEST(Span, TemplateArgumentDeduction) {
int arr[3];
const int const_arr[] = {1, 2, 3};
std::vector<int> vec;
const std::vector<int> const_vec;
- static_assert(std::is_same_v<decltype(span(arr)), span<int>>);
- static_assert(std::is_same_v<decltype(span(vec)), span<int>>);
- static_assert(std::is_same_v<decltype(span(const_arr)), span<const int>>);
- static_assert(std::is_same_v<decltype(span(const_vec)), span<const int>>);
+ static_assert(std::is_same_v<decltype(span(arr)), std::span<int>>);
Review Comment:
```suggestion
static_assert(std::is_same_v<decltype(std::span(arr)), std::span<int>>);
```
Should these have the namespace too?
--
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]
