Skip to content

Commit 735522a

Browse files
authored
[orc-rt] Restore perfect forwarding to move_only_function init (#157784)
After the recent change to hoist std::decay_t (cd8f47b) we were forcing move-initialization of the callable type. This commit restores perfect forwarding so that we copy-initialize where expected.
1 parent 5590637 commit 735522a

File tree

2 files changed

+26
-5
lines changed

2 files changed

+26
-5
lines changed

orc-rt/include/orc-rt/move_only_function.h

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,9 @@ template <typename RetT, typename... ArgTs> class GenericCallable {
4141
template <typename CallableT, typename RetT, typename... ArgTs>
4242
class GenericCallableImpl : public GenericCallable<RetT, ArgTs...> {
4343
public:
44-
GenericCallableImpl(CallableT &&Callable) : Callable(std::move(Callable)) {}
44+
template <typename CallableInitT>
45+
GenericCallableImpl(CallableInitT &&Callable)
46+
: Callable(std::forward<CallableInitT>(Callable)) {}
4547
RetT call(ArgTs &&...Args) override {
4648
return Callable(std::forward<ArgTs>(Args)...);
4749
}
@@ -59,8 +61,9 @@ template <typename RetT, typename... ArgTs> class GenericConstCallable {
5961
template <typename CallableT, typename RetT, typename... ArgTs>
6062
class GenericConstCallableImpl : public GenericConstCallable<RetT, ArgTs...> {
6163
public:
62-
GenericConstCallableImpl(CallableT &&Callable)
63-
: Callable(std::move(Callable)) {}
64+
template <typename CallableInitT>
65+
GenericConstCallableImpl(CallableInitT &&Callable)
66+
: Callable(std::forward<CallableInitT>(Callable)) {}
6467
RetT call(ArgTs &&...Args) const override {
6568
return Callable(std::forward<ArgTs>(Args)...);
6669
}
@@ -93,7 +96,7 @@ class move_only_function<RetT(ArgTs...)> {
9396
template <typename CallableT>
9497
move_only_function(CallableT &&Callable)
9598
: C(std::make_unique<GenericCallableImpl<std::decay_t<CallableT>>>(
96-
std::move(Callable))) {}
99+
std::forward<CallableT>(Callable))) {}
97100

98101
RetT operator()(ArgTs... Params) const {
99102
return C->call(std::forward<ArgTs>(Params)...);
@@ -126,7 +129,7 @@ class move_only_function<RetT(ArgTs...) const> {
126129
template <typename CallableT>
127130
move_only_function(CallableT &&Callable)
128131
: C(std::make_unique<const GenericCallableImpl<std::decay_t<CallableT>>>(
129-
std::move(Callable))) {}
132+
std::forward<CallableT>(Callable))) {}
130133

131134
RetT operator()(ArgTs... Params) const {
132135
return C->call(std::forward<ArgTs>(Params)...);

orc-rt/unittests/move_only_function-test.cpp

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,3 +228,21 @@ TEST(MoveOnlyFunctionTest, Constness) {
228228
EXPECT_EQ(Const, 1U);
229229
}
230230
}
231+
232+
TEST(MoveOnlyFunctionTest, ShouldCopyInitialize) {
233+
// Check that we don't accidentally move-initialize move_only_functions.
234+
class ShouldCopy {
235+
public:
236+
ShouldCopy(bool &Moved) : Moved(Moved) {}
237+
ShouldCopy(const ShouldCopy &) = default;
238+
ShouldCopy(ShouldCopy &&Other) : Moved(Other.Moved) { Moved = true; }
239+
void operator()() {}
240+
241+
private:
242+
bool &Moved;
243+
};
244+
bool DidMove = false;
245+
ShouldCopy SC(DidMove);
246+
move_only_function<void()> F(SC);
247+
EXPECT_FALSE(DidMove);
248+
}

0 commit comments

Comments
 (0)