westonpace commented on a change in pull request #9095:
URL: https://github.com/apache/arrow/pull/9095#discussion_r566302012
##########
File path: cpp/src/arrow/util/iterator.h
##########
@@ -186,6 +194,109 @@ class Iterator : public
util::EqualityComparable<Iterator<T>> {
Result<T> (*next_)(void*) = NULLPTR;
};
+template <typename T>
+struct TransformFlow {
+ using YieldValueType = T;
+
+ TransformFlow(YieldValueType value, bool ready_for_next)
+ : finished_(false),
+ ready_for_next_(ready_for_next),
+ status_(),
+ yield_value_(std::move(value)) {}
+ TransformFlow(bool finished, bool ready_for_next)
+ : finished_(finished), ready_for_next_(ready_for_next), status_(),
yield_value_() {}
+ TransformFlow(Status s) // NOLINT runtime/explicit
+ : finished_(true), ready_for_next_(false), status_(s), yield_value_() {}
+
+ bool HasValue() const { return yield_value_.has_value(); }
+ bool Finished() const { return finished_; }
+ Status status() const { return status_; }
+ bool Ok() const { return status_.ok(); }
+ bool ReadyForNext() const { return ready_for_next_; }
+ T Value() const { return *yield_value_; }
+
+ bool finished_;
+ bool ready_for_next_;
+ Status status_;
+ util::optional<YieldValueType> yield_value_;
+};
+
+struct TransformFinish {
+ template <typename T>
+ operator TransformFlow<T>() && { // NOLINT explicit
+ return TransformFlow<T>(true, true);
+ }
+};
+
+struct TransformSkip {
+ template <typename T>
+ operator TransformFlow<T>() && { // NOLINT explicit
+ return TransformFlow<T>(false, true);
+ }
+};
+
+template <typename T>
+TransformFlow<T> TransformYield(T value = {}, bool ready_for_next = true) {
+ return TransformFlow<T>(std::move(value), ready_for_next);
+}
+
+template <typename T, typename V>
+using Transformer = std::function<TransformFlow<V>(T)>;
+
+template <typename T, typename V>
+class TransformIterator {
+ public:
+ explicit TransformIterator(Iterator<T> it, Transformer<T, V> transformer)
+ : it_(std::move(it)),
+ transformer_(std::move(transformer)),
+ last_value_(),
+ finished_() {}
+
+ util::optional<Result<V>> Pump() {
+ while (!finished_ && last_value_.has_value()) {
+ TransformFlow<V> next = transformer_(*last_value_);
+ if (next.ReadyForNext()) {
+ last_value_.reset();
+ }
+ if (next.Finished()) {
+ finished_ = true;
+ }
+ if (!next.Ok()) {
+ return next.status();
+ }
+ if (next.HasValue()) {
+ return next.Value();
+ }
+ }
+ if (finished_) {
+ return IterationTraits<V>::End();
+ }
+ return util::optional<V>();
+ }
+
+ Result<V> Next() {
+ while (!finished_) {
+ util::optional<Result<V>> next = Pump();
+ if (next.has_value()) {
+ return *next;
Review comment:
RVO is optional but I believe the the implicit move is not. If the
returned type is not a reference then it will first try and return it as an
rvalue and only if that fails will it consider copy. So any std::move will be
redundant at best and harmful (preventing RVO) at worst.
Otherwise we would have to determine ourselves when RVO applies and when it
does not so that we know whether to apply std::move or not.
I guess, what I am asking is...what makes this return different than the
dozens of other returns where I do not move the return value? Why is it needed
here and not elsewhere? What are the rules I use to decide when to move a
return value?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
