Merge bitcoin#32641: Update libmultiprocess subtree to fix clang-tidy errors

154af1e Squashed 'src/ipc/libmultiprocess/' changes from 35944ffd23fa..27c7e8e5a581 (Ryan Ofsky)

Pull request description:

  Includes:

  - bitcoin-core/libmultiprocess#165
  - bitcoin-core/libmultiprocess#173
  - bitcoin-core/libmultiprocess#172

  These changes are needed to fix CI errors in bitcoin#31802.

  The changes can be verified by running `test/lint/git-subtree-check.sh src/ipc/libmultiprocess` as described in [developer notes](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#subtrees) and [lint instructions](https://github.com/bitcoin/bitcoin/tree/master/test/lint#git-subtree-checksh)

ACKs for top commit:
  Sjors:
    ACK 9f65654
  TheCharlatan:
    ACK 9f65654

Tree-SHA512: 745789a6fba552aa066f9f69592b16a5277999dbf0413eaed7fe25cd440b78af57615edfce781592873659fda91463bef7c5dac202b80362bd86f6a90ab20d69

Author: fanquake

src/ipc/libmultiprocess/include/mp/proxy-io.h

@@ -172,7 +172,7 @@ class EventLoop
     void addClient(std::unique_lock<std::mutex>& lock);
     bool removeClient(std::unique_lock<std::mutex>& lock);
     //! Check if loop should exit.
-    bool done(std::unique_lock<std::mutex>& lock);
+    bool done(std::unique_lock<std::mutex>& lock) const;
 
     Logger log()
     {
@@ -249,7 +249,7 @@ struct Waiter
     {
         const std::unique_lock<std::mutex> lock(m_mutex);
         assert(!m_fn);
-        m_fn = std::move(fn);
+        m_fn = std::forward<Fn>(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>(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
 

src/ipc/libmultiprocess/include/mp/proxy-types.h

@@ -37,20 +37,45 @@ struct StructField
     }
     Struct& m_struct;
 
-    // clang-format off
-    template<typename A = Accessor> auto get() const -> decltype(A::get(this->m_struct)) { return A::get(this->m_struct); }
-    template<typename A = Accessor> auto has() const -> std::enable_if_t<A::optional, bool> { return A::getHas(m_struct); }
-    template<typename A = Accessor> auto has() const -> std::enable_if_t<!A::optional && A::boxed, bool> { return A::has(m_struct); }
-    template<typename A = Accessor> auto has() const -> std::enable_if_t<!A::optional && !A::boxed, bool> { return true; }
-    template<typename A = Accessor> auto want() const -> std::enable_if_t<A::requested, bool> { return A::getWant(m_struct); }
-    template<typename A = Accessor> auto want() const -> std::enable_if_t<!A::requested, bool> { return true; }
-    template<typename A = Accessor, typename... Args> decltype(auto) set(Args&&... args) const { return A::set(this->m_struct, std::forward<Args>(args)...); }
-    template<typename A = Accessor, typename... Args> decltype(auto) init(Args&&... args) const { return A::init(this->m_struct, std::forward<Args>(args)...); }
-    template<typename A = Accessor> auto setHas() const -> std::enable_if_t<A::optional> { return A::setHas(m_struct); }
-    template<typename A = Accessor> auto setHas() const -> std::enable_if_t<!A::optional> { }
-    template<typename A = Accessor> auto setWant() const -> std::enable_if_t<A::requested> { return A::setWant(m_struct); }
-    template<typename A = Accessor> auto setWant() const -> std::enable_if_t<!A::requested> { }
-    // clang-format on
+    decltype(auto) get() const { return Accessor::get(this->m_struct); }
+
+    bool has() const {
+      if constexpr (Accessor::optional) {
+        return Accessor::getHas(m_struct);
+      } else if constexpr (Accessor::boxed) {
+        return Accessor::has(m_struct);
+      } else {
+        return true;
+      }
+    }
+
+    bool want() const {
+      if constexpr (Accessor::requested) {
+        return Accessor::getWant(m_struct);
+      } else {
+        return true;
+      }
+    }
+
+    template <typename... Args> decltype(auto) set(Args &&...args) const {
+      return Accessor::set(this->m_struct, std::forward<Args>(args)...);
+    }
+
+    template <typename... Args> decltype(auto) init(Args &&...args) const {
+      return Accessor::init(this->m_struct, std::forward<Args>(args)...);
+    }
+
+    void setHas() const {
+      if constexpr (Accessor::optional) {
+        Accessor::setHas(m_struct);
+      }
+    }
+
+    void setWant() const {
+      if constexpr (Accessor::requested) {
+        Accessor::setWant(m_struct);
+      }
+    }
 };
 
 
@@ -360,34 +385,28 @@ struct ClientException
 template <typename Accessor, typename... Types>
 struct ClientParam
 {
-    ClientParam(Types&&... values) : m_values(values...) {}
+    ClientParam(Types&&... values) : m_values{std::forward<Types>(values)...} {}
 
     struct BuildParams : IterateFieldsHelper<BuildParams, sizeof...(Types)>
     {
-        template <typename... Args>
-        void handleField(Args&&... args)
-        {
-            callBuild<0>(std::forward<Args>(args)...);
-        }
-
-        // TODO Possible optimization to speed up compile time:
-        // https://stackoverflow.com/a/7858971 Using enable_if below to check
-        // position when unpacking tuple might be slower than pattern matching
-        // approach in the stack overflow solution
-        template <size_t I, typename... Args>
-        auto callBuild(Args&&... args) -> std::enable_if_t<(I < sizeof...(Types))>
-        {
-            callBuild<I + 1>(std::forward<Args>(args)..., std::get<I>(m_client_param->m_values));
-        }
-
-        template <size_t I, typename Params, typename ParamList, typename... Values>
-        auto callBuild(ClientInvokeContext& invoke_context, Params& params, ParamList, Values&&... values) ->
-            std::enable_if_t<(I == sizeof...(Types))>
+        template <typename Params, typename ParamList>
+        void handleField(ClientInvokeContext& invoke_context, Params& params, ParamList)
         {
-            MaybeBuildField(std::integral_constant<bool, Accessor::in>(), ParamList(), invoke_context,
-                Make<StructField, Accessor>(params), std::forward<Values>(values)...);
-            MaybeSetWant(
-                ParamList(), Priority<1>(), std::forward<Values>(values)..., Make<StructField, Accessor>(params));
+            auto const fun = [&]<typename... Values>(Values&&... values) {
+                MaybeBuildField(std::integral_constant<bool, Accessor::in>(), ParamList(), invoke_context,
+                    Make<StructField, Accessor>(params), std::forward<Values>(values)...);
+                MaybeSetWant(
+                    ParamList(), Priority<1>(), std::forward<Values>(values)..., Make<StructField, Accessor>(params));
+            };
+
+            // 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) {}
@@ -396,24 +415,15 @@ struct ClientParam
 
     struct ReadResults : IterateFieldsHelper<ReadResults, sizeof...(Types)>
     {
-        template <typename... Args>
-        void handleField(Args&&... args)
+        template <typename Results, typename... Params>
+        void handleField(ClientInvokeContext& invoke_context, Results& results, TypeList<Params...>)
         {
-            callRead<0>(std::forward<Args>(args)...);
-        }
+            auto const fun = [&]<typename... Values>(Values&&... values) {
+                MaybeReadField(std::integral_constant<bool, Accessor::out>(), TypeList<Decay<Params>...>(), invoke_context,
+                    Make<StructField, Accessor>(results), ReadDestUpdate(values)...);
+            };
 
-        template <int I, typename... Args>
-        auto callRead(Args&&... args) -> std::enable_if_t<(I < sizeof...(Types))>
-        {
-            callRead<I + 1>(std::forward<Args>(args)..., std::get<I>(m_client_param->m_values));
-        }
-
-        template <int I, typename Results, typename... Params, typename... Values>
-        auto callRead(ClientInvokeContext& invoke_context, Results& results, TypeList<Params...>, Values&&... values)
-            -> std::enable_if_t<I == sizeof...(Types)>
-        {
-            MaybeReadField(std::integral_constant<bool, Accessor::out>(), TypeList<Decay<Params>...>(), invoke_context,
-                Make<StructField, Accessor>(results), ReadDestUpdate(values)...);
+            std::apply(fun, m_client_param->m_values);
         }
 
         ReadResults(ClientParam* client_param) : m_client_param(client_param) {}
@@ -574,7 +584,7 @@ void serverDestroy(Server& server)
 //!
 //! ProxyClient<ClassName>::M0::Result ProxyClient<ClassName>::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;
 //! }
 //!
@@ -650,19 +660,14 @@ void clientInvoke(ProxyClient& proxy_client, const GetRequest& get_request, Fiel
 //! return value with value of `ret()`. This is useful for avoiding code
 //! duplication and branching in generic code that forwards calls to functions.
 template <typename Fn, typename Ret>
-auto ReplaceVoid(Fn&& fn, Ret&& ret) ->
-    std::enable_if_t<std::is_same_v<void, decltype(fn())>, decltype(ret())>
+auto ReplaceVoid(Fn&& fn, Ret&& ret)
 {
-    fn();
-    return ret();
-}
-
-//! Overload of above for non-void `fn()` case.
-template <typename Fn, typename Ret>
-auto ReplaceVoid(Fn&& fn, Ret&& ret) ->
-    std::enable_if_t<!std::is_same_v<void, decltype(fn())>, decltype(fn())>
-{
-    return fn();
+    if constexpr (std::is_same_v<decltype(fn()), void>) {
+        fn();
+        return ret();
+    } else {
+        return fn();
+    }
 }
 
 extern std::atomic<int> server_reqs;

src/ipc/libmultiprocess/include/mp/proxy.h

@@ -181,7 +181,8 @@ struct ProxyServerCustom : public ProxyServerBase<Interface, Impl>
 //!
 //! Params   - TypeList of C++ ClassName::methodName parameter types
 //! Result   - Return type of ClassName::method
-//! Param<N> - helper to access individual parameters by index number.
+//! Param<N> - helper to access individual parameter types by index number.
+//! Fwd<N>   - helper to forward arguments by index number.
 //! Fields   - helper alias that appends Result type to the Params typelist if
 //!            it not void.
 template <class Fn>
@@ -199,6 +200,16 @@ struct FunctionTraits<_Result (_Class::*const)(_Params...)>
     using Param = typename std::tuple_element<N, std::tuple<_Params...>>::type;
     using Fields =
         std::conditional_t<std::is_same_v<void, Result>, 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 <size_t N>
+    static decltype(auto) Fwd(Param<N>& arg) { return static_cast<Param<N>&&>(arg); }
 };
 
 //! Traits class for a proxy method, providing the same

src/ipc/libmultiprocess/include/mp/type-interface.h

@@ -44,7 +44,7 @@ void CustomBuildField(TypeList<std::shared_ptr<Impl>>,
 {
     if (value) {
         using Interface = typename decltype(output.get())::Calls;
-        output.set(CustomMakeProxyServer<Interface, Impl>(invoke_context, std::move(value)));
+        output.set(CustomMakeProxyServer<Interface, Impl>(invoke_context, std::forward<Value>(value)));
     }
 }
 

src/ipc/libmultiprocess/include/mp/util.h

@@ -183,7 +183,7 @@ struct AsyncCallable
 template <typename Callable>
 AsyncCallable<std::remove_reference_t<Callable>> MakeAsyncCallable(Callable&& callable)
 {
-    return std::move(callable);
+    return std::forward<Callable>(callable);
 }
 
 //! Format current thread name as "{exe_name}-{$pid}/{thread_name}-{$tid}".

src/ipc/libmultiprocess/src/mp/gen.cpp

@@ -215,7 +215,7 @@ static void Generate(kj::StringPtr src_prefix,
     cpp_types << "namespace mp {\n";
 
     std::string guard = output_path;
-    std::transform(guard.begin(), guard.end(), guard.begin(), [](unsigned char c) -> unsigned char {
+    std::ranges::transform(guard, guard.begin(), [](unsigned char c) -> unsigned char {
         if ('0' <= c && c <= '9') return c;
         if ('A' <= c && c <= 'Z') return c;
         if ('a' <= c && c <= 'z') return c - 'a' + 'A';
@@ -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 << ")";
 

src/ipc/libmultiprocess/src/mp/proxy.cpp

@@ -277,7 +277,7 @@ void EventLoop::startAsyncThread(std::unique_lock<std::mutex>& lock)
     }
 }
 
-bool EventLoop::done(std::unique_lock<std::mutex>& lock)
+bool EventLoop::done(std::unique_lock<std::mutex>& lock) const
 {
     assert(m_num_clients >= 0);
     assert(lock.owns_lock());

src/ipc/libmultiprocess/src/mp/util.cpp

@@ -137,7 +137,7 @@ void ExecProcess(const std::vector<std::string>& args)
     }
     argv.push_back(nullptr);
     if (execvp(argv[0], argv.data()) != 0) {
-        perror("execlp failed");
+        perror("execvp failed");
         _exit(1);
     }
 }