diff --git a/include/mp/proxy-io.h b/include/mp/proxy-io.h index be0d4f8..dff8c2a 100644 --- a/include/mp/proxy-io.h +++ b/include/mp/proxy-io.h @@ -249,7 +249,7 @@ struct Waiter { const std::unique_lock lock(m_mutex); assert(!m_fn); - m_fn = std::move(fn); + m_fn = std::forward(fn); m_cv.notify_all(); } @@ -333,7 +333,7 @@ class Connection // to the EventLoop TaskSet to avoid "Promise callback destroyed itself" // error in cases where f deletes this Connection object. m_on_disconnect.add(m_network.onDisconnect().then( - [f = std::move(f), this]() mutable { m_loop.m_task_set->add(kj::evalLater(kj::mv(f))); })); + [f = std::forward(f), this]() mutable { m_loop.m_task_set->add(kj::evalLater(kj::mv(f))); })); } EventLoop& m_loop; @@ -634,7 +634,10 @@ void ListenConnections(EventLoop& loop, int fd, InitImpl& init) }); } -extern thread_local ThreadContext g_thread_context; +extern thread_local ThreadContext g_thread_context; // NOLINT(bitcoin-nontrivial-threadlocal) +// Silence nonstandard bitcoin tidy error "Variable with non-trivial destructor +// cannot be thread_local" which should not be a problem on modern platforms, and +// could lead to a small memory leak at worst on older ones. } // namespace mp diff --git a/include/mp/proxy-types.h b/include/mp/proxy-types.h index b35264d..a74c6de 100644 --- a/include/mp/proxy-types.h +++ b/include/mp/proxy-types.h @@ -385,7 +385,7 @@ struct ClientException template struct ClientParam { - ClientParam(Types&&... values) : m_values(values...) {} + ClientParam(Types&&... values) : m_values{std::forward(values)...} {} struct BuildParams : IterateFieldsHelper { @@ -399,7 +399,14 @@ struct ClientParam ParamList(), Priority<1>(), std::forward(values)..., Make(params)); }; - std::apply(fun, m_client_param->m_values); + // Note: The m_values tuple just consists of lvalue and rvalue + // references, so calling std::move doesn't change the tuple, it + // just causes std::apply to call the std::get overload that returns + // && instead of &, so rvalue references are preserved and not + // turned into lvalue references. This allows the BuildField call to + // move from the argument if it is an rvalue reference or was passed + // by value. + std::apply(fun, std::move(m_client_param->m_values)); } BuildParams(ClientParam* client_param) : m_client_param(client_param) {} @@ -577,7 +584,7 @@ void serverDestroy(Server& server) //! //! ProxyClient::M0::Result ProxyClient::methodName(M0::Param<0> arg0, M0::Param<1> arg1) { //! typename M0::Result result; -//! clientInvoke(*this, &InterfaceName::Client::methodNameRequest, MakeClientParam<...>(arg0), MakeClientParam<...>(arg1), MakeClientParam<...>(result)); +//! clientInvoke(*this, &InterfaceName::Client::methodNameRequest, MakeClientParam<...>(M0::Fwd<0>(arg0)), MakeClientParam<...>(M0::Fwd<1>(arg1)), MakeClientParam<...>(result)); //! return result; //! } //! diff --git a/include/mp/proxy.h b/include/mp/proxy.h index 76be099..e7faad9 100644 --- a/include/mp/proxy.h +++ b/include/mp/proxy.h @@ -181,7 +181,8 @@ struct ProxyServerCustom : public ProxyServerBase //! //! Params - TypeList of C++ ClassName::methodName parameter types //! Result - Return type of ClassName::method -//! Param - helper to access individual parameters by index number. +//! Param - helper to access individual parameter types by index number. +//! Fwd - helper to forward arguments by index number. //! Fields - helper alias that appends Result type to the Params typelist if //! it not void. template @@ -199,6 +200,16 @@ struct FunctionTraits<_Result (_Class::*const)(_Params...)> using Param = typename std::tuple_element>::type; using Fields = std::conditional_t, Params, TypeList<_Params..., _Result>>; + + //! Enable perfect forwarding for clientInvoke calls. If parameter is a + //! value type or rvalue reference type, pass it as an rvalue-reference to + //! MakeClientParam and BuildField calls so it can be moved from, and if it + //! is an lvalue reference, pass it an lvalue reference so it won't be moved + //! from. This method does the same thing as std::forward except it takes a + //! parameter number instead of a type as a template argument, so generated + //! code calling this can be less repetitive and verbose. + template + static decltype(auto) Fwd(Param& arg) { return static_cast&&>(arg); } }; //! Traits class for a proxy method, providing the same diff --git a/include/mp/type-interface.h b/include/mp/type-interface.h index 99adf2a..8a89ac2 100644 --- a/include/mp/type-interface.h +++ b/include/mp/type-interface.h @@ -44,7 +44,7 @@ void CustomBuildField(TypeList>, { if (value) { using Interface = typename decltype(output.get())::Calls; - output.set(CustomMakeProxyServer(invoke_context, std::move(value))); + output.set(CustomMakeProxyServer(invoke_context, std::forward(value))); } } diff --git a/include/mp/util.h b/include/mp/util.h index ebfc3b5..8b802ab 100644 --- a/include/mp/util.h +++ b/include/mp/util.h @@ -183,7 +183,7 @@ struct AsyncCallable template AsyncCallable> MakeAsyncCallable(Callable&& callable) { - return std::move(callable); + return std::forward(callable); } //! Format current thread name as "{exe_name}-{$pid}/{thread_name}-{$tid}". diff --git a/src/mp/gen.cpp b/src/mp/gen.cpp index a50fdbd..3d841a3 100644 --- a/src/mp/gen.cpp +++ b/src/mp/gen.cpp @@ -512,10 +512,20 @@ static void Generate(kj::StringPtr src_prefix, add_accessor(field_name); + std::ostringstream fwd_args; for (int i = 0; i < field.args; ++i) { if (argc > 0) client_args << ","; + + // Add to client method parameter list. client_args << "M" << method_ordinal << "::Param<" << argc << "> " << field_name; if (field.args > 1) client_args << i; + + // Add to MakeClientParam argument list using Fwd helper for perfect forwarding. + if (i > 0) fwd_args << ", "; + fwd_args << "M" << method_ordinal << "::Fwd<" << argc << ">(" << field_name; + if (field.args > 1) fwd_args << i; + fwd_args << ")"; + ++argc; } client_invoke << ", "; @@ -529,13 +539,10 @@ static void Generate(kj::StringPtr src_prefix, client_invoke << "Accessor<" << base_name << "_fields::" << Cap(field_name) << ", " << field_flags.str() << ">>("; - if (field.retval || field.args == 1) { + if (field.retval) { client_invoke << field_name; } else { - for (int i = 0; i < field.args; ++i) { - if (i > 0) client_invoke << ", "; - client_invoke << field_name << i; - } + client_invoke << fwd_args.str(); } client_invoke << ")";