Commit 6bfd3879 authored by Tom Jackson's avatar Tom Jackson Committed by Facebook GitHub Bot

Fix Guard to respect early termination

Summary:
`guard()` is ignoring the return from `handler()`, thereby breaking `take()`.

(Note: this ignores all push blocking failures!)

Reviewed By: yfeldblum

Differential Revision: D29416513

fbshipit-source-id: 1e84d3ba53cb68a1664a349e71398c782b1df751
parent 120926cd
...@@ -804,10 +804,8 @@ class Stride : public Operator<Stride> { ...@@ -804,10 +804,8 @@ class Stride : public Operator<Stride> {
size_t distance = stride_; size_t distance = stride_;
return source_.apply([&](Value value) -> bool { return source_.apply([&](Value value) -> bool {
if (++distance >= stride_) { if (++distance >= stride_) {
if (!handler(std::forward<Value>(value))) {
return false;
}
distance = 0; distance = 0;
return handler(std::forward<Value>(value));
} }
return true; return true;
}); });
...@@ -1730,28 +1728,28 @@ class RangeConcat : public Operator<RangeConcat> { ...@@ -1730,28 +1728,28 @@ class RangeConcat : public Operator<RangeConcat> {
**/ **/
template <class Exception, class ErrorHandler> template <class Exception, class ErrorHandler>
class GuardImpl : public Operator<GuardImpl<Exception, ErrorHandler>> { class GuardImpl : public Operator<GuardImpl<Exception, ErrorHandler>> {
ErrorHandler handler_; ErrorHandler exceptionHandler_;
public: public:
explicit GuardImpl(ErrorHandler handler) : handler_(std::move(handler)) {} explicit GuardImpl(ErrorHandler handler)
: exceptionHandler_(std::move(handler)) {}
template <class Value, class Source> template <class Value, class Source>
class Generator : public GenImpl<Value, Generator<Value, Source>> { class Generator : public GenImpl<Value, Generator<Value, Source>> {
Source source_; Source source_;
ErrorHandler handler_; ErrorHandler exceptionHandler_;
public: public:
explicit Generator(Source source, ErrorHandler handler) explicit Generator(Source source, ErrorHandler handler)
: source_(std::move(source)), handler_(std::move(handler)) {} : source_(std::move(source)), exceptionHandler_(std::move(handler)) {}
template <class Handler> template <class Handler>
bool apply(Handler&& handler) const { bool apply(Handler&& handler) const {
return source_.apply([&](Value value) -> bool { return source_.apply([&](Value value) -> bool {
try { try {
handler(std::forward<Value>(value)); return handler(std::forward<Value>(value));
return true;
} catch (Exception& e) { } catch (Exception& e) {
return handler_(e, std::forward<Value>(value)); return exceptionHandler_(e, std::forward<Value>(value));
} }
}); });
} }
...@@ -1762,12 +1760,12 @@ class GuardImpl : public Operator<GuardImpl<Exception, ErrorHandler>> { ...@@ -1762,12 +1760,12 @@ class GuardImpl : public Operator<GuardImpl<Exception, ErrorHandler>> {
template <class Value, class Source, class Gen = Generator<Value, Source>> template <class Value, class Source, class Gen = Generator<Value, Source>>
Gen compose(GenImpl<Value, Source>&& source) const { Gen compose(GenImpl<Value, Source>&& source) const {
return Gen(std::move(source.self()), handler_); return Gen(std::move(source.self()), exceptionHandler_);
} }
template <class Value, class Source, class Gen = Generator<Value, Source>> template <class Value, class Source, class Gen = Generator<Value, Source>>
Gen compose(const GenImpl<Value, Source>& source) const { Gen compose(const GenImpl<Value, Source>& source) const {
return Gen(source.self(), handler_); return Gen(source.self(), exceptionHandler_);
} }
}; };
...@@ -1805,10 +1803,7 @@ class Dereference : public Operator<Dereference> { ...@@ -1805,10 +1803,7 @@ class Dereference : public Operator<Dereference> {
template <class Handler> template <class Handler>
bool apply(Handler&& handler) const { bool apply(Handler&& handler) const {
return source_.apply([&](Value value) -> bool { return source_.apply([&](Value value) -> bool {
if (value) { return !value || handler(*std::forward<Value>(value));
return handler(*std::forward<Value>(value));
}
return true;
}); });
} }
......
...@@ -116,7 +116,7 @@ class Zip : public Operator<Zip<Container>> { ...@@ -116,7 +116,7 @@ class Zip : public Operator<Zip<Container>> {
template <class Handler> template <class Handler>
bool apply(Handler&& handler) const { bool apply(Handler&& handler) const {
auto iter = container_->begin(); auto iter = container_->begin();
return (source_.apply([&](Value value) -> bool { return source_.apply([&](Value value) -> bool {
if (iter == container_->end()) { if (iter == container_->end()) {
return false; return false;
} }
...@@ -126,7 +126,7 @@ class Zip : public Operator<Zip<Container>> { ...@@ -126,7 +126,7 @@ class Zip : public Operator<Zip<Container>> {
} }
++iter; ++iter;
return true; return true;
})); });
} }
}; };
......
...@@ -1171,6 +1171,15 @@ TEST(Gen, Guard) { ...@@ -1171,6 +1171,15 @@ TEST(Gen, Guard) {
}) })
| eachTo<int>() | eachTo<int>()
| sum); | sum);
EXPECT_EQ(
4,
from({"1", "a", "3", "99"})
| guard<runtime_error>([](runtime_error&, const char*) {
return true; // continue
})
| eachTo<int>()
| take(2) // Ensure take() is respected.
| sum);
EXPECT_EQ( EXPECT_EQ(
1, 1,
from({"1", "a", "3"}) from({"1", "a", "3"})
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment