Skip to content

Commit 7642538

Browse files
digantdesaifacebook-github-bot
authored andcommitted
Simplify runtime FunctionRef (#555)
Summary: Pull Request resolved: #555 FunctionRef simplified - removing lambdas in constructor initializer list, to ensure runtime portability for embedded systems w/ C++11. Reviewed By: dbort Differential Revision: D49798748 fbshipit-source-id: 4c9e46603ebbddeb71fc78f43de6b3e149285738
1 parent 869704e commit 7642538

File tree

3 files changed

+9
-58
lines changed

3 files changed

+9
-58
lines changed

runtime/core/function_ref.h

Lines changed: 5 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,7 @@ class FunctionRef;
5959

6060
template <typename Ret, typename... Params>
6161
class FunctionRef<Ret(Params...)> {
62-
Ret (*callback_)(const void* memory, Params... params) = nullptr;
6362
union Storage {
64-
void* callable;
6563
Ret (*function)(Params...);
6664
} storage_;
6765

@@ -70,57 +68,18 @@ class FunctionRef<Ret(Params...)> {
7068
explicit FunctionRef(std::nullptr_t) {}
7169

7270
/**
73-
* Case 1: A callable object passed by lvalue reference.
74-
* Taking rvalue reference is error prone because the object will be always
75-
* be destroyed immediately.
76-
*/
77-
template <
78-
typename Callable,
79-
// This is not the copy-constructor.
80-
typename std::enable_if<
81-
!std::is_same<remove_cvref_t<Callable>, FunctionRef>::value,
82-
int32_t>::type = 0,
83-
// Avoid lvalue reference to non-capturing lambda.
84-
typename std::enable_if<
85-
!std::is_convertible<Callable, Ret (*)(Params...)>::value,
86-
int32_t>::type = 0,
87-
// Functor must be callable and return a suitable type.
88-
// To make this container type safe, we need to ensure either:
89-
// 1. The return type is void.
90-
// 2. Or the resulting type from calling the callable is convertible to
91-
// the declared return type.
92-
typename std::enable_if<
93-
std::is_void<Ret>::value ||
94-
std::is_convertible<
95-
decltype(std::declval<Callable>()(std::declval<Params>()...)),
96-
Ret>::value,
97-
int32_t>::type = 0>
98-
explicit FunctionRef(Callable& callable)
99-
: callback_([](const void* memory, Params... params) {
100-
auto& storage = *static_cast<const Storage*>(memory);
101-
auto& callable = *static_cast<Callable*>(storage.callable);
102-
return static_cast<Ret>(callable(std::forward<Params>(params)...));
103-
}) {
104-
storage_.callable = &callable;
105-
}
106-
107-
/**
108-
* Case 2: A plain function pointer.
71+
* Case 1: A plain function pointer.
10972
* Instead of storing an opaque pointer to underlying callable object,
11073
* store a function pointer directly.
11174
* Note that in the future a variant which coerces compatible function
11275
* pointers could be implemented by erasing the storage type.
11376
*/
114-
/* implicit */ FunctionRef(Ret (*ptr)(Params...))
115-
: callback_([](const void* memory, Params... params) {
116-
auto& storage = *static_cast<const Storage*>(memory);
117-
return storage.function(std::forward<Params>(params)...);
118-
}) {
77+
/* implicit */ FunctionRef(Ret (*ptr)(Params...)) {
11978
storage_.function = ptr;
12079
}
12180

12281
/**
123-
* Case 3: Implicit conversion from lambda to FunctionRef.
82+
* Case 2: Implicit conversion from lambda to FunctionRef.
12483
* A common use pattern is like:
12584
* void foo(FunctionRef<...>) {...}
12685
* foo([](...){...})
@@ -144,11 +103,11 @@ class FunctionRef<Ret(Params...)> {
144103
: FunctionRef(static_cast<Ret (*)(Params...)>(function)) {}
145104

146105
Ret operator()(Params... params) const {
147-
return callback_(&storage_, std::forward<Params>(params)...);
106+
return storage_.function(std::forward<Params>(params)...);
148107
}
149108

150109
explicit operator bool() const {
151-
return callback_;
110+
return storage_.function;
152111
}
153112
};
154113

runtime/core/test/function_ref_test.cpp

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -37,17 +37,7 @@ void one(int32_t& i) {
3737

3838
} // namespace
3939

40-
TEST(FunctionRefTest, CapturingLambda) {
41-
auto one = 1;
42-
auto f = [&](int32_t& i) { i = one; };
43-
Item item(0, FunctionRef<void(int32_t&)>{f});
44-
EXPECT_EQ(item.get(), 1);
45-
// ERROR:
46-
// Item item1(0, f);
47-
// Item item2(0, [&](int32_t& i) { i = 2; });
48-
// FunctionRef<void(int32_t&)> ref([&](int32_t&){});
49-
}
50-
40+
// Only non-capturing lambdas can be used to initialize a function reference.
5141
TEST(FunctionRefTest, NonCapturingLambda) {
5242
int32_t val = 0;
5343
FunctionRef<void(int32_t&)> ref([](int32_t& i) { i = 1; });

runtime/kernel/operator_registry.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,9 @@ namespace executor {
4242

4343
class KernelRuntimeContext; // Forward declaration
4444
using RuntimeContext = KernelRuntimeContext; // TODO(T147221312): Remove
45-
using OpFunction = FunctionRef<void(KernelRuntimeContext&, EValue**)>;
45+
using OpFunction =
46+
FunctionRef<void(KernelRuntimeContext&, EValue**)>; // TODO(T165139545):
47+
// Remove FunctionRef
4648

4749
/**
4850
* Dtype and dim order metadata for a Tensor argument to an operator.

0 commit comments

Comments
 (0)