ecorm / cppwamp

C++ client library for the WAMP protocol.
Boost Software License 1.0
35 stars 9 forks source link

make with boost 1.66.0 results in errors #114

Closed chenso closed 2 years ago

chenso commented 6 years ago

cppwamp/examples/timeclient/../../cppwamp/include/cppwamp/internal/corosession.ipp:273:40: error: no matching constructor for initialization of 'boost::asio::async_result' (aka 'async_result<coro_handler<boost::asio::executor_binder<void (*)(), boost::asio::executor>, wamp::AsyncResult > >') in 4 different locations.

ecorm commented 6 years ago

There were major changes in Boost.Asio in version 1.66.0.

From the release notes:

Implemented interface changes to reflect the Networking TS (N4656).

Sorry, but I don't know when I'll have time to investigate. It's possible that I might have to overhaul CppWAMP to be more in tune with the way Asio treats async handlers.

I don't like that CppWAMP has two sets of APIs (callback-based and coroutines) and would like to unify them the same way as is done within the Asio library. CppWAMP needs to treat yield as a special type of async handler.

katreniak commented 3 years ago

I don't like that CppWAMP has two sets of APIs (callback-based and coroutines) ...

As a workaround, we managed to use wrap the existing callback based api to work with c++20 coroutines and asio::awaitable with this wrapper:

template <class T, typename F, class CompletitionToken>
auto wampAsyncInvoke(F &&f, CompletitionToken &&token) {
  return asio::async_initiate<CompletitionToken, void(std::exception_ptr, T)>(
      [f = AIZO_FWD(f)](auto &&handler_) {
        // convert move only handler to copyable handler
        auto sharedHandler = std::make_shared<std::decay_t<decltype(handler_)>>(std::move(handler_));
        try {
          f([sharedHandler](wamp::AsyncResult<T> result) mutable {
            if (result.errorCode()) {
              (*sharedHandler)(std::make_exception_ptr(std::system_error(result.errorCode(), result.errorInfo())), T{});
            } else {
              (*sharedHandler)({}, std::move(result.get()));
            }
          });
        } catch (const std::exception &e) {
          (*sharedHandler)(std::current_exception(), T{});
        }
      },
      token);
}

Example usage:

  co_await wampAsyncInvoke<size_t>(
      [&](auto &&handler) { m_session->connect(std::forward<decltype(handler)>(handler)); },
      asio::use_awaitable);
ecorm commented 3 years ago

Thank you for your contribution, @katreniak ! I haven't yet had the chance to wrap my head around c++20 coroutines. I'm glad to see there's still interest in this library, so I'm going to try to put the effort into resolving this issue in the next few months.

I'm working on a revamped "cppwamp2" which will come with it's own serialization sub-library that supports JSON & CBOR and will be much more efficient when converting to C++ types (it will bypass the wamp::variant "DOM type" entirely). Seeing contributions like this from the community will help me to convince my employer to make cppwamp2 open-source. The benefits like bug fixes has to outweigh balance the cost in supporting users for me to make the case.

katreniak commented 3 years ago

For me, our workaround works fine. I just wanted to share back, it may be useful for others. Although I did not see a simple way how to integrate this into cppwamp code. So I just posted it here.

When it comes to json DOM and cppwamp2 ... did you consider to use boost::json? It is designed as a vocabulary json type.

ecorm commented 3 years ago

When it comes to json DOM and cppwamp2 ... did you consider to use boost::json? It is designed as a vocabulary json type.

I looked at it briefly, but I prefer the API of what I'm currently working on. When I have the chance, I'll look at it more closely to borrow some ideas concerning custom memory allocation.

ecorm commented 2 years ago

I'm now working full time on finally resolving this issue so that I'm no longer forced to work with old versions of Boost. It's possible that fixing this may involve breaking the API.

I'm first taking the opportunity to modernize the CMake build, which admittedly did not follow best practices. I'm currently looking at 3.12 as the minimum required CMake version.

ecorm commented 2 years ago

I finally have this fixed in my fix-coro development branch, using the latest Boost. I need to fix a regression in the unit tests. I'm aiming to deprecate CoroSession and having Session handle all possible completion token overloads. In short, CppWAMP will no longer care if the user passes callback functions, coroutine yield contexts, future completion tokens, etc. I will try to do this in a manner that is backward-compatible, where CoroSession is merely a stub that forwards everything to Session.

ecorm commented 2 years ago

Fixed in 487bd06485c40bb0426b8d5f198dcc06c0b271fe.

The deprecation of CoroSession will happen at a later release.

ecorm commented 2 years ago

@katreniak I'm currently working on C++20 coroutine support in the completion-tokens development branch. I've encountered problems with Boost's awaitable_handler not being able to be stored in a std::function due to its move-only nature. I now see why you're using shared_ptr in your wrapper code.

I want the type erasure offered by std::function because I want CppWAMP to be (optionally) compilable as a static library. Our parent project that uses CppWAMP is already slow to compile, so I'm not in favor of making CppWAMP purely header-only.

I've just found out there's a std::move_only_function proposed for C++23. I was hoping someone made a C++11 implementation of it, but can't seem to find such a thing.

I'm now looking to see if one of the "fast delegate" implementations support move-only functions.

ecorm commented 2 years ago

I've found this, but it requires C++14: https://github.com/Naios/function2

I'm now considering bumping CppWAMP's minimum C++ standard to C++14, which would provide the following benefits: