From 764128a1f7bf84fa7713d1c80a1f6b647dfe8a1e Mon Sep 17 00:00:00 2001 From: Daniel Pfeifer Date: Fri, 28 Feb 2025 09:50:45 +0100 Subject: [PATCH 1/5] disable linting of generated files --- CMakeLists.txt | 1 + cmake/TargetCapnpSources.cmake | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index c41d1b4..b828368 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -51,6 +51,7 @@ configure_file(include/mp/config.h.in "${CMAKE_CURRENT_BINARY_DIR}/include/mp/co # Generated C++ Capn'Proto schema files capnp_generate_cpp(MP_PROXY_SRCS MP_PROXY_HDRS include/mp/proxy.capnp) +set_source_files_properties(${MP_PROXY_SRCS} PROPERTIES SKIP_LINTING ON) # util library add_library(mputil OBJECT src/mp/util.cpp) diff --git a/cmake/TargetCapnpSources.cmake b/cmake/TargetCapnpSources.cmake index 963d67d..3f9d32d 100644 --- a/cmake/TargetCapnpSources.cmake +++ b/cmake/TargetCapnpSources.cmake @@ -80,12 +80,14 @@ function(target_capnp_sources target include_prefix) DEPENDS ${capnp_file} VERBATIM ) - target_sources(${target} PRIVATE + set(generated_sources ${CMAKE_CURRENT_BINARY_DIR}/${capnp_file}.c++ ${CMAKE_CURRENT_BINARY_DIR}/${capnp_file}.proxy-client.c++ ${CMAKE_CURRENT_BINARY_DIR}/${capnp_file}.proxy-server.c++ ${CMAKE_CURRENT_BINARY_DIR}/${capnp_file}.proxy-types.c++ ) + set_source_files_properties(${generated_sources} PROPERTIES SKIP_LINTING ON) + target_sources(${target} PRIVATE ${generated_sources}) list(APPEND generated_headers ${capnp_file}.h) endforeach() From 8ed7b6364033e33088790f01251d3b715ae115c1 Mon Sep 17 00:00:00 2001 From: Daniel Pfeifer Date: Fri, 28 Feb 2025 09:51:47 +0100 Subject: [PATCH 2/5] replace SFINAE trick with `if constexpr` --- include/mp/proxy-types.h | 72 +++++++++++++++++++++++++--------------- 1 file changed, 46 insertions(+), 26 deletions(-) diff --git a/include/mp/proxy-types.h b/include/mp/proxy-types.h index 1a519ef..5657789 100644 --- a/include/mp/proxy-types.h +++ b/include/mp/proxy-types.h @@ -37,20 +37,45 @@ struct StructField } Struct& m_struct; - // clang-format off - template auto get() const -> decltype(A::get(this->m_struct)) { return A::get(this->m_struct); } - template auto has() const -> std::enable_if_t { return A::getHas(m_struct); } - template auto has() const -> std::enable_if_t { return A::has(m_struct); } - template auto has() const -> std::enable_if_t { return true; } - template auto want() const -> std::enable_if_t { return A::getWant(m_struct); } - template auto want() const -> std::enable_if_t { return true; } - template decltype(auto) set(Args&&... args) const { return A::set(this->m_struct, std::forward(args)...); } - template decltype(auto) init(Args&&... args) const { return A::init(this->m_struct, std::forward(args)...); } - template auto setHas() const -> std::enable_if_t { return A::setHas(m_struct); } - template auto setHas() const -> std::enable_if_t { } - template auto setWant() const -> std::enable_if_t { return A::setWant(m_struct); } - template auto setWant() const -> std::enable_if_t { } - // 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 decltype(auto) set(Args &&...args) const { + return Accessor::set(this->m_struct, std::forward(args)...); + } + + template decltype(auto) init(Args &&...args) const { + return Accessor::init(this->m_struct, std::forward(args)...); + } + + void setHas() const { + if constexpr (Accessor::optional) { + Accessor::setHas(m_struct); + } + } + + void setWant() const { + if constexpr (Accessor::requested) { + Accessor::setWant(m_struct); + } + } }; @@ -650,19 +675,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 -auto ReplaceVoid(Fn&& fn, Ret&& ret) -> - std::enable_if_t, decltype(ret())> +auto ReplaceVoid(Fn&& fn, Ret&& ret) { - fn(); - return ret(); -} - -//! Overload of above for non-void `fn()` case. -template -auto ReplaceVoid(Fn&& fn, Ret&& ret) -> - std::enable_if_t, decltype(fn())> -{ - return fn(); + if constexpr (std::is_same_v) { + fn(); + return ret(); + } else { + return fn(); + } } extern std::atomic server_reqs; From 36898dbfc72c9748f54bf79f18d1ad6485f48920 Mon Sep 17 00:00:00 2001 From: Daniel Pfeifer Date: Fri, 28 Feb 2025 09:52:49 +0100 Subject: [PATCH 3/5] replace custom tuple unpacking code with `std::apply` --- include/mp/proxy-types.h | 56 ++++++++++++---------------------------- 1 file changed, 17 insertions(+), 39 deletions(-) diff --git a/include/mp/proxy-types.h b/include/mp/proxy-types.h index 5657789..b35264d 100644 --- a/include/mp/proxy-types.h +++ b/include/mp/proxy-types.h @@ -389,30 +389,17 @@ struct ClientParam struct BuildParams : IterateFieldsHelper { - template - void handleField(Args&&... args) - { - callBuild<0>(std::forward(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 - auto callBuild(Args&&... args) -> std::enable_if_t<(I < sizeof...(Types))> - { - callBuild(std::forward(args)..., std::get(m_client_param->m_values)); - } - - template - auto callBuild(ClientInvokeContext& invoke_context, Params& params, ParamList, Values&&... values) -> - std::enable_if_t<(I == sizeof...(Types))> + template + void handleField(ClientInvokeContext& invoke_context, Params& params, ParamList) { - MaybeBuildField(std::integral_constant(), ParamList(), invoke_context, - Make(params), std::forward(values)...); - MaybeSetWant( - ParamList(), Priority<1>(), std::forward(values)..., Make(params)); + auto const fun = [&](Values&&... values) { + MaybeBuildField(std::integral_constant(), ParamList(), invoke_context, + Make(params), std::forward(values)...); + MaybeSetWant( + ParamList(), Priority<1>(), std::forward(values)..., Make(params)); + }; + + std::apply(fun, m_client_param->m_values); } BuildParams(ClientParam* client_param) : m_client_param(client_param) {} @@ -421,24 +408,15 @@ struct ClientParam struct ReadResults : IterateFieldsHelper { - template - void handleField(Args&&... args) - { - callRead<0>(std::forward(args)...); - } - - template - auto callRead(Args&&... args) -> std::enable_if_t<(I < sizeof...(Types))> + template + void handleField(ClientInvokeContext& invoke_context, Results& results, TypeList) { - callRead(std::forward(args)..., std::get(m_client_param->m_values)); - } + auto const fun = [&](Values&&... values) { + MaybeReadField(std::integral_constant(), TypeList...>(), invoke_context, + Make(results), ReadDestUpdate(values)...); + }; - template - auto callRead(ClientInvokeContext& invoke_context, Results& results, TypeList, Values&&... values) - -> std::enable_if_t - { - MaybeReadField(std::integral_constant(), TypeList...>(), invoke_context, - Make(results), ReadDestUpdate(values)...); + std::apply(fun, m_client_param->m_values); } ReadResults(ClientParam* client_param) : m_client_param(client_param) {} From 9a01a35d6517c5e105c4c44ac351b3c512f78e1c Mon Sep 17 00:00:00 2001 From: Daniel Pfeifer Date: Fri, 28 Feb 2025 09:53:29 +0100 Subject: [PATCH 4/5] make member function const --- include/mp/proxy-io.h | 2 +- src/mp/proxy.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/mp/proxy-io.h b/include/mp/proxy-io.h index 4eb27fa..be0d4f8 100644 --- a/include/mp/proxy-io.h +++ b/include/mp/proxy-io.h @@ -172,7 +172,7 @@ class EventLoop void addClient(std::unique_lock& lock); bool removeClient(std::unique_lock& lock); //! Check if loop should exit. - bool done(std::unique_lock& lock); + bool done(std::unique_lock& lock) const; Logger log() { diff --git a/src/mp/proxy.cpp b/src/mp/proxy.cpp index 194ded5..b4255b6 100644 --- a/src/mp/proxy.cpp +++ b/src/mp/proxy.cpp @@ -277,7 +277,7 @@ void EventLoop::startAsyncThread(std::unique_lock& lock) } } -bool EventLoop::done(std::unique_lock& lock) +bool EventLoop::done(std::unique_lock& lock) const { assert(m_num_clients >= 0); assert(lock.owns_lock()); From 049d93113f27aca50381028e082c5538c0b1c574 Mon Sep 17 00:00:00 2001 From: Daniel Pfeifer Date: Fri, 28 Feb 2025 09:54:01 +0100 Subject: [PATCH 5/5] use ranges transform --- src/mp/gen.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mp/gen.cpp b/src/mp/gen.cpp index 267c283..a50fdbd 100644 --- a/src/mp/gen.cpp +++ b/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';