Address clang-tidy-19 warnings
#144
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,13 +5,15 @@ | |
| #include <calculator.h> | ||
| #include <fstream> | ||
| #include <init.capnp.h> | ||
| #include <init.capnp.proxy.h> // NOLINT(misc-include-cleaner) | ||
|
There was a problem hiding this comment. In commit "clang-tidy: Fix Is the include-cleaner check not compatible with IWYU? When I ran IWYU it seemed to correctly add Same applies to other nolint below. There was a problem hiding this comment.
They seem incompatible.
The It seem both linters fail to parse a convoluted template code properly. Additionally, IWYU suggests: for this line:https://github.com/chaincodelabs/libmultiprocess/blob/477405eda34d923bd2ba6b3abc4c4d31db84c3ea/example/calculator.cpp#L48 |
||
| #include <init.h> | ||
| #include <iostream> | ||
| #include <memory> | ||
| #include <mp/proxy-io.h> | ||
| #include <printer.h> | ||
| #include <stdexcept> | ||
| #include <string> | ||
| #include <utility> | ||
|
|
||
| class CalculatorImpl : public Calculator | ||
| { | ||
|
|
@@ -30,7 +32,7 @@ class InitImpl : public Init | |
| } | ||
| }; | ||
|
|
||
| static void LogPrint(bool raise, const std::string& message) | ||
| { | ||
| if (raise) throw std::runtime_error(message); | ||
| std::ofstream("debug.log", std::ios_base::app) << message << std::endl; | ||
|
|
@@ -43,7 +45,7 @@ int main(int argc, char** argv) | |
| return 1; | ||
| } | ||
| mp::EventLoop loop("mpcalculator", LogPrint); | ||
| const int fd = std::stoi(argv[1]); | ||
| std::unique_ptr<Init> init = std::make_unique<InitImpl>(); | ||
| mp::ServeStream<InitInterface>(loop, fd, *init); | ||
| loop.loop(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -63,7 +63,7 @@ struct ProxyClient<Thread> : public ProxyClientBase<Thread, ::capnp::Void> | |
| ProxyClient(const ProxyClient&) = delete; | ||
| ~ProxyClient(); | ||
|
|
||
| void setCleanup(const std::function<void()>& fn); | ||
|
|
||
| //! Cleanup function to run when the connection is closed. If the Connection | ||
| //! gets destroyed before this ProxyClient<Thread> object, this cleanup | ||
|
|
@@ -247,7 +247,7 @@ struct Waiter | |
| template <typename Fn> | ||
| void post(Fn&& fn) | ||
| { | ||
| std::unique_lock<std::mutex> lock(m_mutex); // NOLINT(misc-const-correctness) | ||
|
There was a problem hiding this comment. In commit "clang-tidy: Fix Maybe just declare these const? It seems more stable than adding NOLINT. Especially because if any an unlock call is made to any of these locks later the lint errors would go away but nolint comments would still be there. There was a problem hiding this comment. re: #144 (comment) This is done in #152 |
||
| assert(!m_fn); | ||
| m_fn = std::move(fn); | ||
| m_cv.notify_all(); | ||
|
|
@@ -269,7 +269,7 @@ struct Waiter | |
| fn(); | ||
| lock.lock(); | ||
| } | ||
| const bool done = pred(); | ||
| return done; | ||
| }); | ||
| } | ||
|
|
@@ -488,7 +488,7 @@ ProxyServerBase<Interface, Impl>::~ProxyServerBase() | |
| CleanupRun(fns); | ||
| }); | ||
| } | ||
| assert(m_context.cleanup_fns.empty()); | ||
| std::unique_lock<std::mutex> lock(m_context.connection->m_loop.m_mutex); | ||
| m_context.connection->m_loop.removeClient(lock); | ||
| } | ||
|
|
@@ -523,7 +523,7 @@ using ConnThread = ConnThreads::iterator; | |
| // Retrieve ProxyClient<Thread> object associated with this connection from a | ||
| // map, or create a new one and insert it into the map. Return map iterator and | ||
| // inserted bool. | ||
| std::tuple<ConnThread, bool> SetThread(ConnThreads& threads, std::mutex& mutex, Connection* connection, const std::function<Thread::Client()>& make_thread); | ||
|
|
||
| struct ThreadContext | ||
| { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,17 +39,17 @@ struct StructField | |
|
|
||
| // 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 | ||
| }; | ||
|
|
||
|
|
@@ -314,6 +314,9 @@ struct IterateFieldsHelper | |
| { | ||
| static_cast<Derived*>(this)->handleField(std::forward<Arg1>(arg1), std::forward<Arg2>(arg2), ParamList()); | ||
| } | ||
| private: | ||
| IterateFieldsHelper() = default; | ||
| friend Derived; | ||
|
Comment on lines
+317
to
+319
There was a problem hiding this comment. Reverting commit 1cc0bf5 clang-tidy: Fix does not reveal any warnings, as if that commit is not needed? There was a problem hiding this comment. With the reverted commit on Ubuntu 24.10: Here are Clang details: There was a problem hiding this comment. Thanks for posting the exact commands (I should have posted mine in the first place). It turns out that if the build directory is outside of the source directory, then and observe no warnings from This resolves the problem: --- i/CMakeLists.txt
+++ w/CMakeLists.txt
@@ -18,13 +18,13 @@ find_package(Threads REQUIRED)
option(Libmultiprocess_ENABLE_CLANG_TIDY "Run clang-tidy with the compiler." OFF)
if(Libmultiprocess_ENABLE_CLANG_TIDY)
find_program(CLANG_TIDY_EXECUTABLE NAMES clang-tidy)
if(NOT CLANG_TIDY_EXECUTABLE)
message(FATAL_ERROR "Libmultiprocess_ENABLE_CLANG_TIDY is ON but clang-tidy is not found.")
endif()
- set(CMAKE_CXX_CLANG_TIDY "${CLANG_TIDY_EXECUTABLE}")
+ set(CMAKE_CXX_CLANG_TIDY "${CLANG_TIDY_EXECUTABLE}" --config-file=${CMAKE_SOURCE_DIR}/.clang-tidy)
endif()
include("cmake/compat_config.cmake")
include("cmake/pthread_checks.cmake")
include(GNUInstallDirs)
if cmake -B /tmp/build -DCMAKE_CXX_COMPILER=clang++19 -DCMAKE_CXX_CLANG_TIDY="clang-tidy19;--config-file=/path_to_source/libmultiprocess/.clang-tidy""config file not found when out of source" is out of the scope of this PR, so I guess this issue can be marked as resolved. I will be using a subdirectory of the source to test this PR. |
||
| }; | ||
|
|
||
| struct IterateFields : IterateFieldsHelper<IterateFields, 0> | ||
|
|
@@ -372,14 +375,14 @@ struct ClientParam | |
| // 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))> | ||
| { | ||
| MaybeBuildField(std::integral_constant<bool, Accessor::in>(), ParamList(), invoke_context, | ||
| Make<StructField, Accessor>(params), std::forward<Values>(values)...); | ||
|
|
@@ -400,14 +403,14 @@ struct ClientParam | |
| } | ||
|
|
||
| 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)...); | ||
|
|
@@ -623,15 +626,15 @@ void clientInvoke(ProxyClient& proxy_client, const GetRequest& get_request, Fiel | |
| } catch (...) { | ||
| exception = std::current_exception(); | ||
| } | ||
| std::unique_lock<std::mutex> lock(invoke_context.thread_context.waiter->m_mutex); // NOLINT(misc-const-correctness) | ||
| done = true; | ||
| invoke_context.thread_context.waiter->m_cv.notify_all(); | ||
| }, | ||
| [&](const ::kj::Exception& e) { | ||
| kj_exception = kj::str("kj::Exception: ", e).cStr(); | ||
| proxy_client.m_context.connection->m_loop.logPlain() | ||
| << "{" << invoke_context.thread_context.thread_name << "} IPC client exception " << kj_exception; | ||
| std::unique_lock<std::mutex> lock(invoke_context.thread_context.waiter->m_mutex); // NOLINT(misc-const-correctness) | ||
| done = true; | ||
| invoke_context.thread_context.waiter->m_cv.notify_all(); | ||
| })); | ||
|
|
@@ -648,7 +651,7 @@ void clientInvoke(ProxyClient& proxy_client, const GetRequest& get_request, Fiel | |
| //! 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())> | ||
| { | ||
| fn(); | ||
| return ret(); | ||
|
|
@@ -657,7 +660,7 @@ auto ReplaceVoid(Fn&& fn, Ret&& 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(); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -43,9 +43,9 @@ struct TypeList | |
| //! Example: | ||
| //! Make<std::pair>(5, true) // Constructs std::pair<int, bool>(5, true); | ||
| template <template <typename...> class Class, typename... Types, typename... Args> | ||
| Class<Types..., std::remove_reference_t<Args>...> Make(Args&&... args) | ||
| { | ||
| return Class<Types..., std::remove_reference_t<Args>...>{std::forward<Args>(args)...}; | ||
| } | ||
|
|
||
| //! Type helper splitting a TypeList into two halves at position index. | ||
|
|
@@ -83,7 +83,7 @@ using RemoveCvRef = std::remove_cv_t<std::remove_reference_t<T>>; | |
|
|
||
| //! Type helper abbreviating std::decay. | ||
| template <typename T> | ||
| using Decay = std::decay_t<T>; | ||
|
|
||
| //! SFINAE helper, see using Require below. | ||
| template <typename SfinaeExpr, typename Result_> | ||
|
|
@@ -142,7 +142,7 @@ struct UnlockGuard | |
| template <typename Lock, typename Callback> | ||
| void Unlock(Lock& lock, Callback&& callback) | ||
| { | ||
| UnlockGuard<Lock> unlock(lock); // NOLINT(misc-const-correctness) | ||
| callback(); | ||
| } | ||
|
|
||
|
|
@@ -157,7 +157,7 @@ struct DestructorCatcher | |
| { | ||
| } | ||
| ~DestructorCatcher() noexcept try { | ||
| } catch (const kj::Exception& e) { // NOLINT(bugprone-empty-catch) | ||
| } | ||
|
Comment on lines
-160
to
161
There was a problem hiding this comment. From https://clang.llvm.org/extra/clang-tidy/checks/bugprone/empty-catch.html
This warning looks legit. I wonder if it can be resolved instead of suppressed. I am unable to reproduce it, so I am probably doing something wrong. But this should resolve it, suggested from capnproto/capnproto#553 (comment): --- i/include/mp/util.h
+++ w/include/mp/util.h
@@ -6,12 +6,13 @@
#define MP_UTIL_H
#include <capnp/schema.h>
#include <cstddef>
#include <functional>
#include <future>
+#include <kj/debug.h>
#include <kj/common.h>
#include <kj/exception.h>
#include <kj/string-tree.h>
#include <memory>
#include <string.h>
#include <string>
@@ -154,13 +155,14 @@ struct DestructorCatcher
T value;
template <typename... Params>
DestructorCatcher(Params&&... params) : value(kj::fwd<Params>(params)...)
{
}
~DestructorCatcher() noexcept try {
- } catch (const kj::Exception& e) { // NOLINT(bugprone-empty-catch)
+ } catch (const kj::Exception& e) {
+ KJ_LOG(ERROR, e);
}
};
//! Wrapper around callback function for compatibility with std::async.
//!
//! std::async requires callbacks to be copyable and requires noexceptFurther, unrelated to this PR, the class A
{
public:
~A()
{
throw std::runtime_error{"aaa"};
}
};
template <typename T>
struct DestructorCatcher
{
T value;
template <typename... Params>
DestructorCatcher(Params&&... params) : value(std::forward<Params>(params)...)
{
}
~DestructorCatcher() noexcept try {
} catch (const std::runtime_error& e) {
// Does not work with or without a return.
//return;
}
};
int main(int, char**)
{
try {
std::shared_ptr<DestructorCatcher<A>> p{std::make_shared<DestructorCatcher<A>>()};
} catch (const std::runtime_error& e) {
std::cout << "in main: " << e.what() << "\n";
}
return 0;
}There was a problem hiding this comment. re: #144 (comment) Good catch. The workaround suggested capnproto/capnproto#553 (comment) is not implemented correctly here. The derived constructor is trying to catch exceptions from a base constructor which will not even run until after it exits. To be implemented correctly it would have to add T as a member instead of deriving from it, and either set T to null or make it a std::optional member to make sure its destruct wont throw after DestructorCatcher exits. But I think a better fix for this issue is to change EventLoop::post method to use There was a problem hiding this comment. Now that I can reproduce, I confirm that the above patch which adds @ryanofsky, I am confused. Do you mean "destructor" instead of "constructor"? Also, there is no inheritance here, so there is no derived constructor. I tried to change ~DestructorCatcher() noexcept
{
try {
delete value;
} catch (const std::runtime_error& e) {
std::cout << e.what() << "\n";
}
}but it keeps crashing.
👍 There was a problem hiding this comment.
Quick attempt at doing that, I am not sure how to resolve the compilation error in [patch] Remove DestructorCatcher and AsyncCallablediff --git i/include/mp/proxy-io.h w/include/mp/proxy-io.h
index 4430a42..14d5906 100644
--- i/include/mp/proxy-io.h
+++ w/include/mp/proxy-io.h
@@ -8,12 +8,13 @@
#include <mp/proxy.h>
#include <mp/util.h>
#include <mp/proxy.capnp.h>
#include <capnp/rpc-twoparty.h>
+#include <kj/function.h>
#include <assert.h>
#include <functional>
#include <optional>
#include <map>
#include <memory>
@@ -141,20 +142,22 @@ public:
//! called once from the m_thread_id thread. This will block until
//! the m_num_clients reference count is 0.
void loop();
//! Run function on event loop thread. Does not return until function completes.
//! Must be called while the loop() function is active.
- void post(const std::function<void()>& fn);
+ void post(kj::Function<void()>& fn);
//! Wrapper around EventLoop::post that takes advantage of the
//! fact that callable will not go out of scope to avoid requirement that it
//! be copyable.
template <typename Callable>
void sync(Callable&& callable)
{
+ // XXX
+ // error: no viable conversion from 'reference_wrapper<(lambda at libmultiprocess/include/mp/proxy-io.h:427:43)>' to 'kj::Function<void ()>'
return post(std::ref(callable));
}
//! Start asynchronous worker thread if necessary. This is only done if
//! there are ProxyServerBase::m_impl objects that need to be destroyed
//! asynchronously, without tying up the event loop thread. This can happen
@@ -192,13 +195,13 @@ public:
//! Handle of an async worker thread. Joined on destruction. Unset if async
//! method has not been called.
std::thread m_async_thread;
//! Callback function to run on event loop thread during post() or sync() call.
- const std::function<void()>* m_post_fn = nullptr;
+ kj::Function<void()>* m_post_fn = nullptr;
//! Callback functions to run on async thread.
CleanupList m_async_fns;
//! Pipe read handle used to wake up the event loop thread.
int m_wait_fd = -1;
diff --git i/include/mp/type-context.h w/include/mp/type-context.h
index 7c12afe..40d7cc1 100644
--- i/include/mp/type-context.h
+++ w/include/mp/type-context.h
@@ -61,13 +61,13 @@ auto PassField(Priority<1>, TypeList<>, ServerContext& server_context, const Fn&
{
const auto& params = server_context.call_context.getParams();
Context::Reader context_arg = Accessor::get(params);
auto future = kj::newPromiseAndFulfiller<typename ServerContext::CallContext>();
auto& server = server_context.proxy_server;
int req = server_context.req;
- auto invoke = MakeAsyncCallable(
+ auto invoke =
[fulfiller = kj::mv(future.fulfiller),
call_context = kj::mv(server_context.call_context), &server, req, fn, args...]() mutable {
const auto& params = call_context.getParams();
Context::Reader context_arg = Accessor::get(params);
ServerContext server_context{server, call_context, req};
bool disconnected{false};
@@ -140,13 +140,13 @@ auto PassField(Priority<1>, TypeList<>, ServerContext& server_context, const Fn&
{
server.m_context.connection->m_loop.sync([&]() {
auto fulfiller_dispose = kj::mv(fulfiller);
fulfiller_dispose->reject(kj::mv(*exception));
});
}
- });
+ };
// Lookup Thread object specified by the client. The specified thread should
// be a local Thread::Server object, but it needs to be looked up
// asynchronously with getLocalServer().
auto thread_client = context_arg.getThread();
return server.m_context.connection->m_threads.getLocalServer(thread_client)
diff --git i/include/mp/util.h w/include/mp/util.h
index 0569c44..4fdfd4e 100644
--- i/include/mp/util.h
+++ w/include/mp/util.h
@@ -6,12 +6,13 @@
#define MP_UTIL_H
#include <capnp/schema.h>
#include <cstddef>
#include <functional>
#include <future>
+#include <kj/debug.h>
#include <kj/common.h>
#include <kj/exception.h>
#include <kj/string-tree.h>
#include <memory>
#include <string.h>
#include <string>
@@ -143,52 +144,12 @@ template <typename Lock, typename Callback>
void Unlock(Lock& lock, Callback&& callback)
{
UnlockGuard<Lock> unlock(lock); // NOLINT(misc-const-correctness)
callback();
}
-//! Needed for libc++/macOS compatibility. Lets code work with shared_ptr nothrow declaration
-//! https://github.com/capnproto/capnproto/issues/553#issuecomment-328554603
-template <typename T>
-struct DestructorCatcher
-{
- T value;
- template <typename... Params>
- DestructorCatcher(Params&&... params) : value(kj::fwd<Params>(params)...)
- {
- }
- ~DestructorCatcher() noexcept try {
- } catch (const kj::Exception& e) { // NOLINT(bugprone-empty-catch)
- }
-};
-
-//! Wrapper around callback function for compatibility with std::async.
-//!
-//! std::async requires callbacks to be copyable and requires noexcept
-//! destructors, but this doesn't work well with kj types which are generally
-//! move-only and not noexcept.
-template <typename Callable>
-struct AsyncCallable
-{
- AsyncCallable(Callable&& callable) : m_callable(std::make_shared<DestructorCatcher<Callable>>(std::move(callable)))
- {
- }
- AsyncCallable(const AsyncCallable&) = default;
- AsyncCallable(AsyncCallable&&) = default;
- ~AsyncCallable() noexcept = default;
- ResultOf<Callable> operator()() const { return (m_callable->value)(); }
- mutable std::shared_ptr<DestructorCatcher<Callable>> m_callable;
-};
-
-//! Construct AsyncCallable object.
-template <typename Callable>
-AsyncCallable<std::remove_reference_t<Callable>> MakeAsyncCallable(Callable&& callable)
-{
- return std::move(callable);
-}
-
//! Format current thread name as "{exe_name}-{$pid}/{thread_name}-{$tid}".
std::string ThreadName(const char* exe_name);
//! Escape binary string for use in log so it doesn't trigger unicode decode
//! errors in python unit tests.
std::string LogEscape(const kj::StringTree& string);
diff --git i/src/mp/proxy.cpp w/src/mp/proxy.cpp
index ca094e3..5b2fdf6 100644
--- i/src/mp/proxy.cpp
+++ w/src/mp/proxy.cpp
@@ -19,12 +19,13 @@
#include <future>
#include <kj/async-io.h>
#include <kj/async.h>
#include <kj/common.h>
#include <kj/debug.h>
#include <kj/exception.h>
+#include <kj/function.h>
#include <kj/memory.h>
#include <map>
#include <memory>
#include <mutex>
#include <stddef.h>
#include <stdexcept>
@@ -215,13 +216,13 @@ void EventLoop::loop()
KJ_SYSCALL(::close(post_fd));
std::unique_lock<std::mutex> lock(m_mutex); // NOLINT(misc-const-correctness)
m_wait_fd = -1;
m_post_fd = -1;
}
-void EventLoop::post(const std::function<void()>& fn)
+void EventLoop::post(kj::Function<void()>& fn)
{
if (std::this_thread::get_id() == m_thread_id) {
fn();
return;
}
std::unique_lock<std::mutex> lock(m_mutex);There was a problem hiding this comment. Thanks for doing ahead with this!
This is just what comes to mind but you might be able to replace std::ref with kj::mv. Intent of std::ref is just to avoid a copy. There was a problem hiding this comment.
I'd slightly prefer to explicitly suppress the error so we can know to revisit and fix this later than to add KJ_LOG in code we know is broken and will never run.
Ooops, I'm not sure what I was looking at when I saw inheritance. And I did mean destructor. I'm not sure about what was causing crashes you were seeing, but I if we wanted to implement suggested workaround correctly, easiest way would probably be to make member There was a problem hiding this comment.
That does not work. Probably for the same reason Try it yourself: gotbolt There was a problem hiding this comment. Interesting. In that case could maybe an appealing alternative could be declare T in inside an anonymous union and expliict placement-new to call constructor, and explicit call to its destructor in a try/catch. An approach like that might be able to work if can't get rid of asynccallable class, though I'm still hoping we could get rid of it using kj::Function There was a problem hiding this comment. re: #144 (comment)
This patch is now part of #160 |
||
| }; | ||
|
|
||
|
|
@@ -181,7 +181,7 @@ struct AsyncCallable | |
|
|
||
| //! Construct AsyncCallable object. | ||
| template <typename Callable> | ||
| AsyncCallable<std::remove_reference_t<Callable>> MakeAsyncCallable(Callable&& callable) | ||
| { | ||
| return std::move(callable); | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In commit "cmake: Skip linting for generated sources" (a692038)
I think my preference would be to not skip linting for generated files, and I implemented changes locally to actually fix the lint errors, so I think it would be slightly better to drop this commit to avoid having to undo this later. This commit also conflicts with #142 so that could be another reason to drop it. Ok to keep though if you prefer. Just pointing out that this may change later.