lidavidm commented on code in PR #14395:
URL: https://github.com/apache/arrow/pull/14395#discussion_r1015931928
##########
cpp/src/arrow/util/string_builder.h:
##########
@@ -44,6 +45,11 @@ class ARROW_EXPORT StringStreamWrapper {
} // namespace detail
+template <typename T>
+std::ostream& operator<<(std::ostream& os, std::optional<T> const& opt) {
Review Comment:
It's because `opts.stop` is used in `Status::Invalid` in
`scalar_nested.cc:109`, that's what this first part of the compiler error says:
<details>
```
In file included from
/home/lidavidm/Code/upstream/reviewing/arrow-17960/cpp/src/arrow/status.h:25,
from
/home/lidavidm/Code/upstream/reviewing/arrow-17960/cpp/src/arrow/buffer.h:29,
from
/home/lidavidm/Code/upstream/reviewing/arrow-17960/cpp/src/arrow/array/data.h:26,
from
/home/lidavidm/Code/upstream/reviewing/arrow-17960/cpp/src/arrow/array/array_base.h:26,
from
/home/lidavidm/Code/upstream/reviewing/arrow-17960/cpp/src/arrow/compute/kernels/scalar_nested.cc:20:
/home/lidavidm/Code/upstream/reviewing/arrow-17960/cpp/src/arrow/util/string_builder.h:
In instantiation of ‘void arrow::util::StringBuilderRecursive(std::ostream&,
Head&&) [with Head = const std::optional<long int>&; std::ostream =
std::basic_ostream<char>]’:
/home/lidavidm/Code/upstream/reviewing/arrow-17960/cpp/src/arrow/util/string_builder.h:55:25:
recursively required from ‘void
arrow::util::StringBuilderRecursive(std::ostream&, Head&&, Tail&& ...) [with
Head = const long int&; Tail = {const char (&)[52], const std::optional<long
int>&, const char (&)[2]}; std::ostream = std::basic_ostream<char>]’
/home/lidavidm/Code/upstream/reviewing/arrow-17960/cpp/src/arrow/util/string_builder.h:55:25:
required from ‘void arrow::util::StringBuilderRecursive(std::ostream&,
Head&&, Tail&& ...) [with Head = const char (&)[9]; Tail = {const long int&,
const char (&)[52], const std::optional<long int>&, const char (&)[2]};
std::ostream = std::basic_ostream<char>]’
/home/lidavidm/Code/upstream/reviewing/arrow-17960/cpp/src/arrow/util/string_builder.h:61:25:
required from ‘std::string arrow::util::StringBuilder(Args&& ...) [with Args
= {const char (&)[9], const long int&, const char (&)[52], const
std::optional<long int>&, const char (&)[2]}; std::string =
std::__cxx11::basic_string<char>]’
/home/lidavidm/Code/upstream/reviewing/arrow-17960/cpp/src/arrow/status.h:170:44:
required from ‘static arrow::Status
arrow::Status::FromArgs(arrow::StatusCode, Args&& ...) [with Args = {const char
(&)[9], const long int&, const char (&)[52], const std::optional<long int>&,
const char (&)[2]}]’
/home/lidavidm/Code/upstream/reviewing/arrow-17960/cpp/src/arrow/status.h:214:28:
required from ‘static arrow::Status arrow::Status::Invalid(Args&& ...) [with
Args = {const char (&)[9], const long int&, const char (&)[52], const
std::optional<long int>&, const char (&)[2]}]’
/home/lidavidm/Code/upstream/reviewing/arrow-17960/cpp/src/arrow/compute/kernels/scalar_nested.cc:109:29:
required from ‘static arrow::Status
arrow::compute::internal::{anonymous}::ListSlice<Type,
IndexType>::Exec(arrow::compute::KernelContext*, const
arrow::compute::ExecSpan&, arrow::compute::ExecResult*) [with Type =
arrow::ListType; IndexType = arrow::Int8Type]’
```
</details>
Something like this works for me:
<details>
```diff
diff --git a/cpp/src/arrow/compute/kernels/scalar_nested.cc
b/cpp/src/arrow/compute/kernels/scalar_nested.cc
index 333d98eb1..334173989 100644
--- a/cpp/src/arrow/compute/kernels/scalar_nested.cc
+++ b/cpp/src/arrow/compute/kernels/scalar_nested.cc
@@ -89,6 +89,10 @@ Status GetListElementIndex(const ExecValue& value, T*
out) {
return Status::OK();
}
+std::string ToString(const std::optional<int>& o) {
+ return o.has_value() ? std::to_string(*o) : "(nullopt)";
+}
+
template <typename Type, typename IndexType>
struct ListSlice {
using offset_type = typename Type::offset_type;
@@ -108,7 +112,7 @@ struct ListSlice {
// TODO: support start == stop which should give empty lists
return Status::Invalid("`start`(", opts.start,
") should be greater than 0 and smaller than
`stop`(",
- opts.stop, ")");
+ ToString(opts.stop), ")");
}
if (opts.step != 1) {
// TODO: support step in slicing
diff --git a/cpp/src/arrow/util/string_builder.h
b/cpp/src/arrow/util/string_builder.h
index 27e2bc710..7c05ccd51 100644
--- a/cpp/src/arrow/util/string_builder.h
+++ b/cpp/src/arrow/util/string_builder.h
@@ -18,7 +18,6 @@
#pragma once
#include <memory>
-#include <optional>
#include <ostream>
#include <string>
#include <utility>
@@ -45,11 +44,6 @@ class ARROW_EXPORT StringStreamWrapper {
} // namespace detail
-template <typename T>
-std::ostream& operator<<(std::ostream& os, std::optional<T> const& opt) {
- return opt ? os << opt.value() : os;
-}
-
template <typename Head>
void StringBuilderRecursive(std::ostream& stream, Head&& head) {
stream << head;
--
2.30.0
```
</details>
--
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]
