coroutines-ts icon indicating copy to clipboard operation
coroutines-ts copied to clipboard

We should propose a compile time infrastructure to support detection of whether function/object is being co_await on

Open JCYang opened this issue 6 years ago • 4 comments

It's so easy to extend the state-of-art boost::asio(and in the future, the Network TS) to work with coroutine, I already have a private repository(will open its source when everything is done) that works elegantly. But there's one caution for the users, I decide that, by design the users must co_await coroutine_io::async_dosomething(...), call it plainly(without co_await) should be malformed(basically a no-op with current implementation), because I decide to delay all the operations to await_suspend() and always return false in await_ready(), which make the coroutine works elegantly without any sorts of synchronization internally. But now since there's no way to know at compile-time whether the user has co_await on the my wrapper function, I can't forbid this sort of invalid usage during compile time. I have considered to start the operation just inside the coroutine_io::async_dosomething(...) directly(make calling it directly no-longer a no-op) but then await_ready() require a mutex to sync the status, that's generally not acceptable for a coroutine wrapper that emphasize on efficiency without syncing. And that also does not meet users' expectation. So I think a compiler supported mechanism to detect whether a object is being co_await on is quite useful and possibly important infrastructure for correctness and robust coroutines library usage, at least for an asio based wrapper.

So I propose we should add an mechanism to help the compiler to detect all the invalid usage if the object is by design must be used with co_await. If the library author decide a function must be co_await for, maybe we should write something like,

awaitable_obj function_sth_ready() need(co_await) ; // as the signature.

If you think this is a correct approach, then I may write a proposal against the current TS.

JCYang avatar Aug 20 '17 04:08 JCYang

Have you tried applying the [[nodiscard]] attribute to the function or return-type? This attribute was intended for this sort of scenario. i.e. preventing the caller from accidentally ignoring/discarding the return-value.

There are a couple of benefits to waiting until the await_suspend() is called on the returned awaitable object before starting the async operation.

It means you can rely on the this pointer of the awaitable not changing once the operation starts which means you can pass the address of members into your underlying async methods and avoid allocating extra data on the heap to track the state of the operation. If you start the operation inside the initial call then the awaitable may be moved to a new location before it is awaited.

Another issue with starting the async operation before await_suspend() is called is that the awaiting coroutine has not yet suspended. This means that if the async operation can complete concurrently on another thread then you'll need to add some synchronisation (eg. using atomics) to decide the race between the awaiter suspending and the operation completing. If the awaiter suspends first then the async completion handler can resume the awaiting coroutine. If the operation completes before the awaiter can suspend then it can just continue without suspending the awaiter.

I find that doing the following has worked pretty well for me so far:

  • Make your method that returns awaitable_obj a simple factory function for constructing the 'operation' object that is then awaited. RVO generally means that it will be constructed in-place in the caller.
  • Decorate the method it with [[nodiscard]] to prevent callers from ignoring the result.
  • Put all of the logic for starting the operation in the await_suspend()

Then you can keep the no-synchronisation implementation while still ensuring that callers don't accidentally ignore the result.

Futher, if you limit the awaitable_obj interface to only have move assignment/construction and operator co_await() then the caller can't to much with it other than co_await it.

See https://github.com/lewissbaker/cppcoro/blob/8e728932be86a84120fc460da019707d4e787c69/include/cppcoro/readable_file.hpp#L47 for an example.

lewissbaker avatar Aug 21 '17 00:08 lewissbaker

Just as I said, I had already put all operations in await_suspend() in my initial design and the implementation, yes, I knew how to use the coroutine proposal correctly. I don't know [[nodiscard]] before, thanks for this suggestion. But after learning its usage and intention, I do not feel it's the correct solution. Think about this, auto result = async_do_sth(...);

[[nodiscard]] doesn't help here. And this is a invalid usage that compiler has no way to capture. We probably need more specific mechanism for coroutines.

JCYang avatar Aug 21 '17 03:08 JCYang

I see your point.

Another motivating example I came up with:

async_mutex mutex;

task<> correct()
{
  // co_await used, lock acquired correctly
  auto lock = co_await mutex.scoped_lock_async();
  //...
}

task<> buggy()
{
  // Whoops! No co_await, lock not acquired! [[nodiscard]] does not trigger warning here.
  auto lock = mutex.scoped_lock_async();
  //...
}

I'm wondering if you'd still want to allow deferring the co_await until later. ie. just check that the returned object is co_awaited at some point.

task<> example()
{
  auto lockOp = mutex.scoped_lock_async();

  // ... later

  // Result is still co_awaited at some point, should warning still be triggered?
  // Or is warning always triggered if the result is not immediately co_awaited?
  auto lock = co_await lockOp;
  //...
}

Is this something that should be picked up by the language? eg. via some [[must_await]] attribute Or just picked up by compiler static analysis / control-flow checkers? eg. detect never co_awaiting something that has an operator co_await()

lewissbaker avatar Aug 21 '17 06:08 lewissbaker

From the viewpoint of a user with good principle and who is well trained, IMHO one of the most important benefits that coroutines bring us is it makes writing async codes much more nature to read, to understand, and to reason about. So I think just "co_await-immediately-and-make-it-nature" is a principle that should be encouraged or even enforced throughout the usage of coroutines if possible.

I agree maybe sometimes/some usage cases should be left with freedom, this is c++ after all, so I guess compiler static heuristics are good place to emit warning when the lib author does allow this sort of usage, we don't need to enforce anything in the language. But for libraries designed to enforce good principle and help careless users to write good codes, we need something like [[must_await]].

JCYang avatar Aug 21 '17 08:08 JCYang