ASIO integration

Thanks for the feedback! I will respond to your code review comments inline/push changes as needed.

As for the compatibility question

In the ideal case we could provide the non zero abstraction implementation for async_result for older boost asio versions and switch to the improved one when a suficient version number is detected (which can be done through ASIO_VERSION or BOOST_VERSION macros which should be exposed somehow by the implementation). Same gos for the MSVC bug you mentioned, is this possible?

That's a good idea so some points are probably worth clarifying.

Re: non-zero overhead version: This is doable and probably worth pursuing. However there are three possible tiers of integration. 1. Best case: ASIO >= 1.15, Boost >= 1.72, zero-overhead adaptation. We have the lazy form async_result::initiate plus return type deduction

1. Not bad: ASIO >= 1.14, Boost >= 1.70, adaptation with type erasure on the continuable. We still have the async_result::initiate which allows lazy initiation of async ops, but return_type must be specified purely based on signature. So we can do return_type = cti::continuable<_result_type_> which, if I understand right, uses your function2 type erasure. Still not bad!

2. Worst case: prior versions. Have to specify return_type, and only the async_result::get form exists, which requires eager initiation.

If I'm reading the invocation model docs correctly, case 3 is incompatible with the continuable invocation model, as it requires eager initiation. For example I am not doing this in my example code currently, but I could write auto ct = asio::async_meow(use_cti_t); And then nothing is actually initiated until I call ct.done() or ct is destructed, etc (possibly with some then/fail chained on beforehand). But strictly speaking this is illegal with the async_result::get model.

So if I'm understanding right, we want to provide support for both 1. and 2., but for 3. the recommendation remains to use promisify and/or follow the example of the existing example-asio.cpp? In this case we could even add macro checking to support/asio.hpp and say older versions are not supported.

Regarding the MSVC bug, this is indeed possible and trivial to do but I would argue not necessary. ASIO and Boost.ASIO both follow a release schedule with discrete releases, so I think we will say something like "here is ASIO support for ASIO >= 1.14 or Boost >= 1.70". But right now I am coding against some stray commits from the Boost 1.72/ASIO 1.15 beta, but I do expect the MSVC fix to make it into the actual 1.72/1.15. Again this is low effort to support, but arguably not really necessary as I imagine users tend to lock into release versions rather than random commits.

Those are my thoughts, but happy to go with whatever you see fit!

Yes, I think you are on a good way there.

You are right:

Worst case: prior versions. Have to specify return_type, and only the async_result::get form exists, which requires eager initiation.

This should not be supported by this library and is also out of scope for this PR since for that there is the promisify helper provided already. I'm looking forward to your changes.

I've addressed some of the changes you requested, others still in progress. I'll leave you to mark issues as resolved as you see fit.

In particular I wanted to get feedback on the new type deduction approach. I think this is kind of nice as we only need one non-trivial struct template specialization. This works as before on my machine with the previous examples, both in a vanilla configuration (ASIO version is 1.14, no deduced ret type) and with my manual check (I changed ASIO_VERSION to be 1.15, use deduced ret type).

I can also hopefully write a simple example of a novel composed operation to test the new signature with multiple args is handled correctly.

Let me know your thoughts on the CTI_DETAIL_ASIO_HAS macros related to ASIO integration, definitely open to naming convention changes.

Also you will note a bit of verbosity with the #if defined checks. An alternative is to do like ASIO here and define something like CTI_DETAIL_ASIO_INTEGRATION_DECL(...) which pastes its args iff we need explicit ret type. Thus we could use the macro to conditionally provide using return_type = ..., etc.

Similarly we could do CTI_DETAIL_ASIO_INTEGRATION_RET_TYPE(type) which evaluates to type if it's needed, else auto.

I think it may be a tossup--BOOST_ASIO_INITFN_AUTO_RESULT_TYPE appears many times throughout the codebase but we're only using these rules like 1-3 times.

The PR has made a huge step forward, good job so far!

Beside the comments I've made I've spotted two major points:

1) Conversion between `boost::error_code and cti::exception_t

Is it always given that for the 2x2 matrice of possible 4 combinations between asio/boost::asio and no/with exceptions the conversion between boost::error_code and cti::exception_t is correct and meaningful?

| | With exceptions | No exceptions | |-----------------|-----------------|---------------| | standalone asio | | | | boost asio | | |

2) Specialization against void signatures

The specialization of initiate_make_continuable and its usages can be shortened a lot as described above. Probably CTI_DETAIL_ASIO_INTEGRATION_RET_TYPE can be dropped completely too, in order to address your question from above.

All in all the fallback from previous boost versions looks great to me and blends in quite nicely!

Thanks for the pointers/encouragement!

Regarding point 1), this comment https://github.com/Naios/continuable/pull/28#discussion_r356044790 has lead me to an answer.

I think the answer is yes, and also that the case analysis may be simpler than I had initially expected. I was accounting for the possibility that a user had disabled exceptions and set boost::system::error_condition as the custom error type, which personally I would be very surprised to see in the wild.

So to address the matrix entries. In all cases assuming a handler call like h(ec, args...) 1.a) standalone ASIO with exceptions: we std::rethrow_exception(std::make_exception_ptr(exception_t(ec))) and catch(exception_t const&). This is meaningful and indeed standard practice, see e.g., the ASIO use_future_t or use_awaitable (and co_spawn).

1.b) standalone ASIO, no exceptions: we are converting an std::error_code to an std::error_condition (i.e., cti::exception_t with exceptions disabled) which is indeed a well-defined (but lossy, see below) conversion.

1. a) Boost.ASIO with exceptions: as in 1.a) but of course exception_t and error_code_t are their Boost counterparts; again standard practice.

2.b) Also the same, and due to the implicit conversions I discovered above, we may convert boost::error_code_t to cti::exception_t (i.e., std::error_condition) with no intermediate conversions.

Setting CONTINUABLE_WITH_CUSTOM_ERROR_TYPE to error_code_t also strikes me as a very probable use case for someone using ASIO with continuable, and is easy to support.

This may be better suited to a separate issue (please create a new one if you'd like to discuss it further), but I am curious about the rationale error_condition as the fallback error type for exceptions disabled. In abstract terms, error_code represents a particular exception type and error_condition represents a base class exception you'd derive from. For a concrete example, I might have error_codes like err::missing_colon or err::unmatched_paren, and those could be mapped to an error_condition like cond::parse_error, so I can do if (ec_val == errc::missing_colon) in the particular case or if (ec_val == cond::parse_error) for a more coarse check. So to answer your question above, the mapping is meaningful in all cases, but it is lossy in most cases too.

edit: looking more closely at the documentation of error_condition, I was wrong to call the conversion lossy as the original error_code can be recovered from an error_condition, but I think it is accurate still to say the goal of error_condition is more coarse error handling at the level of, say, family of errors, rather than a particular code.

Regarding point 2) I think you are right, you have given me some helpful leads to try out and I will report back accordingly!

I've addressed most of your comments at this point I think! Some questions:

1. Thoughts on the name cti::asio_token_t, and also if cti is an appropriate namespace--at a glance I did not see other non-detail entities which were in further nested namespaces.
2. Now that the token lives in namespace cti, maybe we can broach the topic of doxygen documentation comparable to that of cti::promisify? I may need some guidance on what modules or groups to put stuff in but I am happy to write the docs themselves

1

In my opinion the equivalent to boost::use_future_t would be cti::use_continuable_t and maybe we can also have the docs in the same section as promisify since it is meant for facilities which make other libraries compatible and easy to use with continuable.

2

std::rethrow_exception(std::make_exception_ptr(exception_t(ec)))

Is a std:.exception_ptr not std::exception which means the conversion should look like the following: std::rethrow_exception(std::make_exception_ptr(ec))

The other conversions look good to me.

any news?

have just been taking a bit of a break the last week or so, should be able to get back to it tomorrow :)

1. sounds good!

2. Not quite, an exception_ptr can point to (and even prolong the lifetime of) anything that was thrown with a throw statement, so it’s necessary to convert to an std::exception (sub) object if it’s to be rethrown and caught as such