September 4th, 2020

Rough edges in the when_all coroutine, part 2: Overloaded comma operator

Raymond Chen
Raymond Chen

Last time, we looked at a problematic edge case in our when_all coroutine: The empty parameter list.

There’s another edge case that can cause trouble, and that’s the case where the comma operator itself has been overloaded.¹

struct S
{
  void Detonate();
  S operator,(S right) { Detonate(); return right; }
};

struct async_s : std::experimental::suspend_never
{
  S await_resume() { return {}; }
};

when_all(async_s(), async_s()); // kaboom

We start by defining a type S that has a comma operator. When you comma two S objects together, the first one explodes.

Next, we define an awaitable object async_s: When you co_await, an S comes out.

And then we pass two of these objects to when_all. The expectation is that the when_all awaits the two objects, throws away the results, and returns.

Instead, what happens is that the S object explodes.

What went wrong is that our fold expression expanded to

IAsyncAction when_all(async_s v1, async_s v2)
{
  (co_await v1, co_await v2);
  co_return;
}

The intent of the comma in the fold expression was to throw away the left-hand operand, leaving the last surviving operand to be thrown away by the statement-expression. But thanks to the custom comma operator, it actually causes the left-hand operand to explode.

To suppress any custom comma operators, we can cast the result of the co_await to void. Since you cannot overload the comma operator for void, this forces the use of the default comma operator, so we just comma-combine a bunch of voids, which is harmless.

template <typename... T>
Windows::Foundation::IAsyncAction when_all(T... async)
{
    (void(co_await async), ...);
    co_return;
}

Here is the PR that fixes the empty parameter list and comma operator issues, and a follow-up.

Bonus chatter: We could also have used a right fold:

    (co_await async, ..., void());

which expands to

    (co_await v1, (co_await v2, void()));

But I think casting away the value is simpler.

¹ Shame on you.

Category
Old New Thing
Topics
Code

Author

Raymond Chen

Raymond has been involved in the evolution of Windows for more than 30 years. In 2003, he began a Web site known as The Old New Thing which has grown in popularity far beyond his wildest imagination, a development which still gives him the heebie-jeebies. The Web site spawned a book, coincidentally also titled The Old New Thing (Addison Wesley 2007). He occasionally appears on the Windows Dev Docs Twitter account to tell stories which convey no useful information.

3 comments

Discussion is closed. Login to edit/delete existing comments.

Newest
Newest Popular Oldest
  • 紅樓鍮

    I think the when_all has another potentially undesirable behavior, namely an exception thrown by a task coming later in the argument list doesn’t fail the when_all until all earlier tasks have completed (or one of them itself throws an exception). It also doesn’t handle lazy tasks like cppcoro::task<> correctly.

    • Raymond ChenMicrosoft employee Author

      when_all doesn’t make any promises about which failure is reported if multiple operations fail. All it promises is that it completes when all the tasks have completed. (Or fails if any task fails.)

  • MGetz

    I wish there was a setting on github to automatically decline PRs that include overloads of operator,()

Read next