Commit 2dedc14b authored by Yedidya Feldblum's avatar Yedidya Feldblum Committed by Facebook Github Bot 2

gen::dereference should perfectly-forward unwrapped values

Summary:
[Folly] `gen::dereference` should perfectly-forward unwrapped values.

The problem comes in when the wrapped value is not actually a pointer, but is actually an rvalue-ref to some other kind of wrapper type with `Inner&& operator*() &&`. In such cases, the compiler emits a type mismatch error that it cannot cast `Inner` to `Inner&&`, with the errors originating in `Dereference::foreach` and `Dereference::apply`.

Fixes a couple other missing-forwarding and extra-forwarding bugs.

Reviewed By: ddrcoder

Differential Revision: D3776617

fbshipit-source-id: 6926fc18244a572846b22d428bd407d37fb20aa1
parent 214d5f7c
main v2022.02.14.00 v2022.02.07.00 v2022.01.31.00 v2022.01.24.00 v2022.01.17.00 v2022.01.10.00 v2022.01.03.00 v2021.12.27.00 v2021.12.20.00 v2021.12.13.00 v2021.12.06.00 v2021.11.29.00 v2021.11.15.00 v2021.11.08.00 v2021.11.01.00 v2021.10.25.00 v2021.10.18.00 v2021.10.11.00 v2021.10.04.00 v2021.09.27.00 v2021.09.20.00 v2021.09.13.00 v2021.09.06.00 v2021.08.30.00 v2021.08.23.00 v2021.08.02.00 v2021.07.22.00 v2021.07.20.01 v2021.07.20.00 v2021.06.28.00 v2021.06.14.00 v2021.06.07.00 v2021.05.31.00 v2021.05.24.00 v2021.05.17.00 v2021.05.10.00 v2021.05.03.00 v2021.04.26.00 v2021.04.19.00 v2021.04.12.00 v2021.04.05.00 v2021.03.29.00 v2021.03.22.00 v2021.03.15.00 v2021.03.08.00 v2021.03.01.00 v2021.02.22.00 v2021.02.15.00 v2021.02.08.00 v2021.02.01.00 v2021.01.25.00 v2021.01.18.01 v2021.01.18.00 v2021.01.11.00 v2021.01.04.00 v2020.12.28.00 v2020.12.21.00 v2020.12.14.00 v2020.12.07.00 v2020.11.30.00 v2020.11.23.00 v2020.11.16.00 v2020.11.09.00 v2020.11.02.00 v2020.10.26.00 v2020.10.19.00 v2020.10.12.00 v2020.10.05.00 v2020.09.28.00 v2020.09.21.00 v2020.09.14.00 v2020.09.07.00 v2020.08.31.00 v2020.08.24.00 v2020.08.17.00 v2020.08.10.00 v2020.08.03.00 v2020.07.27.00 v2020.07.20.00 v2020.07.13.00 v2020.07.06.00 v2020.06.29.00 v2020.06.15.00 v2020.06.08.00 v2020.06.01.00 v2020.05.25.00 v2020.05.18.00 v2020.05.11.00 v2020.05.04.00 v2020.04.27.00 v2020.04.20.00 v2020.04.13.00 v2020.04.06.00 v2020.03.30.00 v2020.03.23.00 v2020.03.16.00 v2020.03.09.00 v2020.03.02.00 v2020.02.24.00 v2020.02.17.00 v2020.02.10.00 v2020.02.03.00 v2020.01.27.00 v2020.01.20.00 v2020.01.13.00 v2020.01.06.00 v2019.12.30.00 v2019.12.23.00 v2019.12.16.00 v2019.12.09.00 v2019.12.06.00 v2019.12.02.00 v2019.11.11.00 v2019.11.04.00 v2019.10.28.00 v2019.10.21.00 v2019.10.14.00 v2019.10.07.00 v2019.09.30.00 v2019.09.23.00 v2019.09.16.00 v2019.09.09.00 v2019.09.02.00 v2019.08.26.00 v2019.08.19.00 v2019.08.12.00 v2019.08.05.00 v2019.07.29.00 v2019.07.22.00 v2019.06.17.00 v2019.06.10.00 v2019.06.03.00 v2019.05.27.00 v2019.05.20.00 v2019.05.13.00 v2019.05.06.00 v2019.04.29.00 v2019.04.22.00 v2019.04.15.00 v2019.04.08.00 v2019.04.01.00 v2019.03.25.00 v2019.03.18.00 v2019.03.04.00 v2019.02.25.00 v2019.02.18.00 v2019.02.11.00 v2019.02.04.00 v2019.01.28.00 v2019.01.21.00 v2019.01.14.00 v2019.01.07.00 v2018.12.31.00 v2018.12.24.00 v2018.12.17.00 v2018.12.10.00 v2018.12.03.00 v2018.11.26.00 v2018.11.19.00 v2018.11.12.00 v2018.11.05.00 v2018.10.29.00 v2018.10.22.00 v2018.10.15.00 v2018.10.08.00 v2018.10.01.00 v2018.09.24.00 v2018.09.17.00 v2018.09.10.01 v2018.09.10.00 v2018.09.03.01 v2018.09.03.00 v2018.08.27.00 v2018.08.20.00 v2018.08.13.00 v2018.08.09.00 v2018.08.06.00 v2018.07.30.00 v2018.07.23.00 v2018.07.16.00 v2018.07.09.00 v2018.07.02.00 v2018.06.25.00 v2018.06.18.00 v2018.06.11.00 v2018.06.04.00 v2018.05.28.00 v2018.05.21.00 v2018.05.14.00 v2018.05.07.00 v2018.04.30.00 v2018.04.23.00 v2018.04.16.00 v2018.04.09.00 v2018.04.02.00 v2018.03.26.00 v2018.03.19.00 v2018.03.12.00 v2018.03.05.00 v2018.02.26.00 v2018.02.19.00 v2018.02.12.00 v2018.02.05.00 v2018.01.29.00 v2018.01.22.00 v2018.01.15.00 v2018.01.08.00 v2018.01.01.00 v2017.12.25.00 v2017.12.18.00 v2017.12.11.00 v2017.12.04.00 v2017.11.27.00 v2017.11.20.00 v2017.11.13.00 v2017.11.06.00 v2017.10.30.00 v2017.10.23.00 v2017.10.16.00 v2017.10.09.00 v2017.10.02.00 v2017.09.25.00 v2017.09.18.00 v2017.09.11.00 v2017.09.04.00 v2017.08.28.00 v2017.08.21.00 v2017.08.14.00 v2017.08.07.00 v2017.07.31.00 v2017.07.24.00 v2017.07.17.01 v2017.07.17.00 v2017.07.10.00 v2017.07.03.00 v2017.06.26.01 v2017.06.26.00 v2017.06.19.00 v2017.06.12.00 v2017.06.05.00 v2017.05.29.00 v2017.05.22.00 v2017.05.15.00 v2017.05.08.00 v2017.05.01.00 v2017.04.24.00 v2017.04.17.00 v2017.04.10.00 v2017.04.03.00 v2017.03.27.00 v2017.03.20.00 v2017.03.13.00 v2017.03.06.00 v2016.12.19.00 v2016.12.12.00 v2016.12.05.00 v2016.11.28.00 v2016.11.21.00 v2016.11.14.00 v2016.11.07.00 v2016.10.31.00 v2016.10.24.00 v2016.10.17.00 v2016.10.10.00 v2016.10.03.00 v2016.09.26.00 v2016.09.19.00 v2016.09.12.01 v2016.09.12.00 v2016.09.05.00
No related merge requests found
...@@ -1567,7 +1567,7 @@ class Dereference : public Operator<Dereference> { ...@@ -1567,7 +1567,7 @@ class Dereference : public Operator<Dereference> {
void foreach(Body&& body) const { void foreach(Body&& body) const {
source_.foreach([&](Value value) { source_.foreach([&](Value value) {
if (value) { if (value) {
return body(*value); return body(*std::forward<Value>(value));
} }
}); });
} }
...@@ -1576,7 +1576,7 @@ class Dereference : public Operator<Dereference> { ...@@ -1576,7 +1576,7 @@ class Dereference : public Operator<Dereference> {
bool apply(Handler&& handler) const { bool apply(Handler&& handler) const {
return source_.apply([&](Value value) -> bool { return source_.apply([&](Value value) -> bool {
if (value) { if (value) {
return handler(*value); return handler(*std::forward<Value>(value));
} }
return true; return true;
}); });
...@@ -1627,14 +1627,14 @@ class Indirect : public Operator<Indirect> { ...@@ -1627,14 +1627,14 @@ class Indirect : public Operator<Indirect> {
template <class Body> template <class Body>
void foreach(Body&& body) const { void foreach(Body&& body) const {
source_.foreach([&](Value value) { source_.foreach([&](Value value) {
return body(&value); return body(&std::forward<Value>(value));
}); });
} }
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 {
return handler(&value); return handler(&std::forward<Value>(value));
}); });
} }
...@@ -1964,6 +1964,11 @@ class Min : public Operator<Min<Selector, Comparer>> { ...@@ -1964,6 +1964,11 @@ class Min : public Operator<Min<Selector, Comparer>> {
Selector selector_; Selector selector_;
Comparer comparer_; Comparer comparer_;
template <typename T>
const T& asConst(const T& t) const {
return t;
}
public: public:
Min() = default; Min() = default;
...@@ -1984,9 +1989,9 @@ class Min : public Operator<Min<Selector, Comparer>> { ...@@ -1984,9 +1989,9 @@ class Min : public Operator<Min<Selector, Comparer>> {
Optional<StorageType> min; Optional<StorageType> min;
Optional<Key> minKey; Optional<Key> minKey;
source | [&](Value v) { source | [&](Value v) {
Key key = selector_(std::forward<Value>(v)); Key key = selector_(asConst(v)); // so that selector_ cannot mutate v
if (!minKey.hasValue() || comparer_(key, minKey.value())) { if (!minKey.hasValue() || comparer_(key, minKey.value())) {
minKey = key; minKey = std::move(key);
min = std::forward<Value>(v); min = std::forward<Value>(v);
} }
}; };
......
...@@ -24,9 +24,10 @@ ...@@ -24,9 +24,10 @@
#include <folly/FBVector.h> #include <folly/FBVector.h>
#include <folly/MapUtil.h> #include <folly/MapUtil.h>
#include <folly/Memory.h> #include <folly/Memory.h>
#include <folly/String.h>
#include <folly/dynamic.h> #include <folly/dynamic.h>
#include <folly/gen/Base.h>
#include <folly/experimental/TestUtil.h> #include <folly/experimental/TestUtil.h>
#include <folly/gen/Base.h>
using namespace folly::gen; using namespace folly::gen;
using namespace folly; using namespace folly;
...@@ -1113,6 +1114,45 @@ TEST(Gen, Dereference) { ...@@ -1113,6 +1114,45 @@ TEST(Gen, Dereference) {
} }
} }
namespace {
struct DereferenceWrapper {
string data;
string& operator*() & {
return data;
}
string&& operator*() && {
return std::move(data);
}
explicit operator bool() {
return true;
}
};
bool operator==(const DereferenceWrapper& a, const DereferenceWrapper& b) {
return a.data == b.data;
}
void PrintTo(const DereferenceWrapper& a, std::ostream* o) {
*o << "Wrapper{\"" << cEscape<string>(a.data) << "\"}";
}
}
TEST(Gen, DereferenceWithLValueRef) {
auto original = vector<DereferenceWrapper>{{"foo"}, {"bar"}};
auto copy = original;
auto expected = vector<string>{"foo", "bar"};
auto actual = from(original) | dereference | as<vector>();
EXPECT_EQ(expected, actual);
EXPECT_EQ(copy, original);
}
TEST(Gen, DereferenceWithRValueRef) {
auto original = vector<DereferenceWrapper>{{"foo"}, {"bar"}};
auto empty = vector<DereferenceWrapper>{{}, {}};
auto expected = vector<string>{"foo", "bar"};
auto actual = from(original) | move | dereference | as<vector>();
EXPECT_EQ(expected, actual);
EXPECT_EQ(empty, original);
}
TEST(Gen, Indirect) { TEST(Gen, Indirect) {
vector<int> vs{1}; vector<int> vs{1};
EXPECT_EQ(&vs[0], from(vs) | indirect | first | unwrap); EXPECT_EQ(&vs[0], from(vs) | indirect | first | unwrap);
......
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