skypjack / uvw

Header-only, event based, tiny and easy to use libuv wrapper in modern C++ - now available as also shared/static library!
MIT License
1.84k stars 209 forks source link

loop->walk example doesn't compile #239

Closed jagerman closed 3 years ago

jagerman commented 3 years ago

Attempting to do:

loop->walk([](auto &&h){ h.close(); });

as per the README fails to compile because the default case in Loop::walk invokes the callback with the uv_handle_t*, and so:

../external/uvw/src/uvw/loop.h:326:19:   required from ‘void uvw::Loop::walk(Func) [with Func = llarp::uv::Loop::stop()::<lambda(auto:44&&)>]’
../llarp/ev/ev_libuv.cpp:173:57:   required from here
../llarp/ev/ev_libuv.cpp:173:47: error: request for member ‘close’ in ‘handle’, which is of pointer type ‘uv_handle_s*’ (maybe you meant to use ‘->’ ?)
  173 |       m_Impl->walk([](auto&& handle) { handle.close(); });
      |                                        ~~~~~~~^~~~~

I can work around it with something like if constexpr (!std::is_pointer_v<std::remove_reference_t<decltype(h)>>) h.close();, but perhaps that fallback case should wrap in a generic uvw::Handle argument instead of passing the bare pointer?

skypjack commented 3 years ago

uvw::Handle is an intermediary class template, so we can't really use it here. However, the default case is only for when no uvw::*Handle class is attached to the underlying handle. In what case is it returning a uv_handle_t exactly? Is it something not managed by uvw itself?

jagerman commented 3 years ago

It isn't actually making it into that code path, but because of the default: case it needs to compile for it anyway.

skypjack commented 3 years ago

Aha got it. Well, probably we should get rid of it and only return handles that are managed by uvw. Actually, we should also check that handle->data is set, because it could be not for handles created by users.