felipecrv commented on code in PR #42067:
URL: https://github.com/apache/arrow/pull/42067#discussion_r1635546016
##########
cpp/src/arrow/compute/kernels/scalar_nested.cc:
##########
@@ -130,14 +131,56 @@ std::string ToString(const std::optional<T>& o) {
return o.has_value() ? ToChars(*o) : "(nullopt)";
}
-template <typename Type>
+/// \param stop User-provided stop or the length of the input list
+int64_t ListSliceLength(int64_t start, int64_t step, int64_t stop) {
+ DCHECK_GE(step, 1);
+ const auto size = std::max(stop - start, static_cast<int64_t>(0));
Review Comment:
Sure. I generally don't do it this way because I don't want both parameters
to be implicitly cast to the type I want as output.
It's not a problem here, but with `start` and `stop` being `uint64_t` this
would type check:
```cpp
int64_t ListSliceLength2(uint64_t start, int64_t step, uint64_t stop) {
const auto size = std::max<int64_t>(stop - start, 0);
return bit_util::CeilDiv(size, step);
}
```
where the intention would be something with the semantics of
```cpp
const auto size = static_cast<int64_t>(std::max(stop - start,
static_cast<uint64_t>(0)));
```
This even scarier when dealing with `std::min` though. You want to perform
`min` on a wider integer and cast only the output to the smaller integer type.
--
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]
