[coroutines] Add std::experimental::task<T> type

[lewissbaker]

Summary

Adds the coroutine std::experimental::task<T> type described in proposal P1056R0.
See https://wg21.link/P1056R0.

This implementation allows customization of the allocator used to allocate the
coroutine frame by passing std::allocator_arg as the first argument, followed by
the allocator to use.

This supports co_awaiting the same task multiple times. The second and
subsequent times it returns a reference to the already-computed value.

This diff also adds some implementations of other utilities that have potential for
standardization as helpers within the test/... area:

• sync_wait(awaitable) - See P1171R0
• manual_reset_event

Move the definition of the aligned_allocation_size helper function
from <experimental/memory_resource> to <experimental/memory>
so it can be more widely used without pulling in memory_resource.

Outstanding work:

• Use C++14 keywords directly rather than macro versions eg. use noexcept instead of _NOEXCEPT).
• Add support for overaligned coroutine frames. This may need wording in the Coroutines TS to support passing the extra std::align_val_t.
• Eliminate use of if constexpr if we want it to compile under C++14.

[CaseyCarter]

include/experimental/task

57

Why qualify size_t here when you're in a subnamespace of std? (It's only necessary to qualify *functions* to avoid unintended ADL, not *types*.)

Why not _NOEXCEPT?

69

Ditto extraneous _VSTD. (I'll stop commenting these now.)

83

Is it an option to store struct foo : _Alloc { _DeallocFunc* bar; }; here so space isn't wasted for empty allocators? This would let you coalesce the allocator and non-allocator cases into a single case with a defaulted (empty) allocator.

95

#include<type_traits>

96

You probably want noexcept in the message instead of _NOEXCEPT.

98

This comment seems unnecessary in addition to the static_assert.

99

#include<utility>?

_Alloc __allocatorOnStack(_VSTD::move(__heapAllocator)); to avoid ADL on move, and because the Allocator requirements don't require *implicit* move construction. (I know, mega-nitpick.)

109

#include<new>

::new (static_cast<void*>(static_cast<char*>(__mem) + __paddedSize3)) _Alloc(__allocator); (to ensure True Placement New)

114

Allocator special member functions are forbidden to throw (http://eel.is/c++draft/allocator.requirements#tab:utilities.allocator.requirements)

118

deallocate is also forbidden to throw (same citation).

134

Ditto "static is implied but it's nice to be explicit"

_NOEXCEPT wouldn't hurt.

138

There's some "bonus" indentation on this line.

169

Style: reinterpret_cast<_Tp&>(__storage_).~_Tp(); saves a couple of valuable keystrokes. (And 172, 206, ... if you care)

186

::new (static_cast<void*>(&__storage_)) // ...

196

The parameters to is_convertible are the source expression's type-and-value-category, and the destination type - your arguments are reversed. Also, direct and copy-init can vary for class types in C++ so you should require is_constructible (I don't really care if it's "require instead" or "require as well"):

is_constructible<_Tp, _Value> && is_convertible<_Value, _Tp>

(The first argument of is_convertible and the non-first arguments to is_constructible go through declval, so X&& and X are equivalent.)

198

is_nothrow_constructible<_Tp, _Value> is equivalent and more concise.

199

Ditto ::new (static_cast<void*>(&__storage_) (I'll stop commenting these now.)

226

Too pretty; needs to be more __ugly (all three enumerators)

239

Weird indent.

252

Should be unsigned char so it can "provide storage" (http://eel.is/c++draft/intro.object#3)

alignas(_Tp) alignas(exception_ptr) would let you skip the __storageAlignment rigamarole

This class would be greatly simplified if this was an anonymous union: union { char __empty; _Tp __value; exception_ptr __exception; }; so the reinterpret_casting could go away.

263

Pure style comment: I'd use a default member initializer for __has_exception and not declare this constructor at all.

271

Ditto not a pointer to exception_ptr.

328

Ditto comments from the other specialization.

379

Why is this not simply:

template <>
class __task_promise<void> final : public __task_promise_base {
  using _Handle = coroutine_handle<__task_promise>;

public:
  _Handle get_return_object() _NOEXCEPT { return _Handle::from_promise(*this); }

  void return_void() _NOEXCEPT {}

  void unhandled_exception() _NOEXCEPT {
#ifndef _LIBCPP_NO_EXCEPTIONS
    __exception_ = current_exception();
#endif
  }

  void __lvalue_result() { __throw_if_exception(); }

  void __rvalue_result() { __throw_if_exception(); }

private:
  void __throw_if_exception() {
#ifndef _LIBCPP_NO_EXCEPTIONS
    if (__exception_) {
      rethrow_exception(__exception_);
    }
#endif
  }

#ifndef _LIBCPP_NO_EXCEPTIONS
  exception_ptr __exception_;
#endif
};

386

excess indent

415

#include<utility>?

431

Pure style comment: Let your destructor do the work:

task __old =  _VSTD::exchange(__coro_, _VSTD::exchange(__other.__coro_, {}));

531

Style: auto&& x is more concise.

Doesn't x dangle in this case? Be *really* explicit in the proposal about why it's a good idea if so.

559

@GorNishanov: Why isn't co_return foo; equivalent to co_return move(foo); for an automatic variable foo?

612

I suggest adding a test case with a stateful allocator.

[GorNishanov]

include/experimental/task

559

Not implemented yet in Clang and MSVC.
I filed a bug: https://bugs.llvm.org/show_bug.cgi?id=37265

[lewissbaker]

include/experimental/task

57

I meant to remove this _VSTD but missed it.
I went a bit overboard with _VSTD usage as I wasn't used to writing code within the std:: namespace ;)

I had missed that C++17 added the ability to add noexcept to the function type.
Nice! I'll add _NOEXCEPT in.

79

Should we also be adding overloads that take align_val_t for cases where the promise and/or coroutine frame is overaligned?

N4736 11.4.4(11) currently only mentions operator new overload with size_t argument.

@GorNishanov Are there plans to extend the allocator support for coroutines to use align_val_t overloads of operator new for over-aligned allocations?

81

I will run it through clang-format to fix up the formatting issues.

83

I don't think we can do that as we need to be able to lookup the _DeallocFunc* for a given coroutine frame regardless of the type of _Alloc, which is unknown in operator delete(). Placing the _DeallocFunc* member after the _Alloc would prevent that.

I could do something with if constexpr (is_empty_v<_Alloc>) to specialise operator new() for a stateless allocator.

Or maybe we can do something with the new-fangled [[no_unique_address]] attribute in C++20?

119

Do we need to use typename _Allocator::template rebind<char>::other as the allocator type rather than just using _Alloc directly or is it preferable to require the allocator to have a value_type of char (or should that be unsigned char)?

235

This approach will result in a different ABI for the task promise when compiled without exceptions.

@EricWF, @CaseyCarter: Does libc++ require standard library types to have the same ABI regardless of compiler flags?

379

This was done to avoid calling exception_ptr runtime functions in the common case where there is no exception created.

I thought I recalled this being done by someone from MS (yourself or perhaps Kenny Kerr?) in a generator<T> implementation to avoid the overhead of calls to runtime library functions for the exception_ptr default constructor, operator bool() and destructor as these are generally not inlinable. However, I currently can't find the reference code for this and it's possible I may be misremembering here.

A std::optional<exception_ptr> would achieve the same ends but I was conscious of incurring the extra dependency on the <optional> header.

I can simplify the implementation to what you suggest and re-apply the optimisation later if profiling determines it to be a performance problem.

384

This macro would seem to only enable nodiscard when building for c++2a or later.

Does libc++ not use [[nodiscard]] on any APIs when built against C++17?

396

The design I went for with cppcoro::task was to allow awaiting the value multiple times so that task<T> would act a bit like an asynchronous version of a lazy<T>.

If we only allowed co_await'ing the task once then this should:

• allow the coroutine frame to be destroyed earlier (eg. we could pass ownership of the coroutine frame to awaiter object which could destroy coroutine frame at end of statement rather than when ~task is run).
• eliminate the need for the conditional on await_ready().
• allow placing storage for the result in the awaiter object rather than the promise object - would this potentially provide more opportunity for copy elision?

431

I was considering implementing this by passing the parameter by value.

task& operator=(task __other) _NOEXCEPT {
  _VSTD::swap(__coro_, __other.__coro_);
}

436

I'm not sure either. The only place I've used ready() thus far has been in tests.

I'll remove them for now - we can always add them in later if we find a need.

451

Ok, I'll update this to an ASSERT.

531

This particular piece of code was just to check the expression's type was int&&.
The value would indeed dangle if this code was executed.
I did it this way as a workaround for the fact that co_await is not currently allowed inside a decltype() expression.

Also, auto&& has slightly different semantics to that of decltype(auto) when the initialising expression is a prvalue.

612

Will do.

[lewissbaker]

I have posted an updated version of the code here: https://gist.github.com/lewissbaker/38ba1d8a13e4fb0906559b7aa1c413d3

I'm still trying to figure out how to get the updated revision into this review.
I'm not familiar with Phabricator and it seems to want to create a new revision (review?) when I click the 'Update diff' button.
Does the updated diff need to be relative to the previous diff?
Any tips as to what I'm doing wrong are appreciated :)

[CaseyCarter]

In D46140#1083915, @lewissbaker wrote:

I have posted an updated version of the code here: https://gist.github.com/lewissbaker/38ba1d8a13e4fb0906559b7aa1c413d3

I'm still trying to figure out how to get the updated revision into this review.
I'm not familiar with Phabricator and it seems to want to create a new revision (review?) when I click the 'Update diff' button.
Does the updated diff need to be relative to the previous diff?
Any tips as to what I'm doing wrong are appreciated :)

Phabricator wants you to replace the entire differential when you update. Each diff should be relative to the original code, not to the previous diff.

[lewissbaker]

Addressed some feedback from initial version.

• factored out allocator padding calculations
• use anonymous unions instead of char-buffers and reinterpret_cast
• make co_await'ing an invalid task have undefined behaviour
• run clang-format over the file to fix some formatting

[GorNishanov]

include/experimental/task

373

Why I am concerned about

await_ready() => __coro.done()

is that check is inherently racy.

If the rule is, coroutine starts suspended and then you are allowed await on it once and that will start it. (Or you can use library helpers, spawn, when_all, etc. that will do similar thing. They will subscribe and start the coroutine).

Once the coroutine is started, you cannot touch it.

I struggle to see the use case.

Once coroutine is started, it has already has continuation hooked up and once finished it will resume the waiter that will lead to the destruction of the coroutine. So, nobody can check for done-ness.

If it has already been started, but has not finished, are we going to subscribe again? How would we distinguish the case that coroutine has not started, vs started but has not completed?

I see this await_ready() returning anything but false to be fraught with danger.

396

I played around some time ago and I think I was getting the best results when the result was in the promise.

We may get in the future a return_slot<T> type, that would allow to place result directly in the caller site (like RVO), if we get that, we would need the result in the promise, but, for now, it is fine to keep it there.

[lewissbaker]

include/experimental/task

132

Still need to qualify move() call with _VSTD::move() as per @CaseyCarter's earlier comment.

158

Add _NOEXCEPT.

373

I do not see this as being racy.

A task is not designed to be safe to access concurrently from multiple threads. I have tried to follow the general guideline of non-const methods cannot be called concurrently with any other access to the object, while const methods can generally be safely called concurrently with other const methods. The latest revision of task<T> does not have any const methods.

When you first call a task-returning coroutine it creates the coroutine initially suspended. When the task is first co_awaited this will call __coro.done() inside await_ready() (this is safe since the coroutine is suspended). The awaiting coroutine is then suspended and the task's coroutine is resumed. When the task's coroutine completes (ie. suspends at final_suspend point) it then becomes .done() and the awaiting coroutine is resumed.

If we treat the task as a normal non-thread-safe object then it is up to the caller to ensure the task is not accessed concurrently from another thread or coroutine. In this case, the awaiting coroutine should be the only coroutine that is accessing the task object for the duration of the co_await operation. As the awaiting coroutine will be suspended while the task's coroutine is executing, it will not have an opportunity to co_await the task again until the task completes, at which point the task's coroutine will be suspended at final_suspend-point and thus will be .done().

If a coroutine then subsequently co_awaits the task again after the first co_await operation has completed then it will see that the coroutine is alread .done() and so will continue without suspending.

How would we distinguish the case that coroutine has not started, vs started but has not completed?

A task object should only ever be able to observe the underlying coroutine being either suspended at initial_suspend (before the task is first co_awaited) or at final_suspend (after the first co_await has completed). The coroutine that has access to the task will be suspended while the task's coroutine is executing and so cannot observe the started-but-not-completed state.

Once coroutine is started, it has already has continuation hooked up and once finished it will resume the waiter that will lead to the destruction of the coroutine. So, nobody can check for done-ness.

The task's coroutine is not destroyed until ~task is executed, which may not be immediately after co_await expression resumes if the task object has been assigned to a local variable. In this case, the task's coroutine's .done() state would be observable.

I struggle to see the use case.

For one use-case, consider:

task<record> get_record(int id);

task<> example()
{
  // Want to only get the record once regardless of how many times
  // it's used below, but don't get the record if we don't need it.
  task<record> t = get_record(123);

  if (someCond)
  {
    record& r = co_await t;
    //...
  }

  //... later

  if (someOtherCond)
  {
    record& r = co_await t;
    //...
  }
}

373

As an aside, if we did want to allow multiple coroutines to concurrently co_await an asynchronous result then something like cppcoro::shared_task<T> would be more appropriate as it correctly handles synchronisation required to avoid the data races.

[CaseyCarter]

include/experimental/task

83

It occurred to me in the shower after making this comment that it would break because of the type erasure. I suspect the best choice would be to avoid storing an allocator when allocator_traits<_Alloc>::is_always_empty::value && is_default_constructible_v<_Alloc> is true, and just construct a new one in the deallocation function.

117

Is it intentional that this overload only takes lvalue parameters?

119

You need to go through allocator_traits for anything other than allocate, deallocate, and value_type. Your rebound allocator type would be typename allocator_traits<_Alloc>::template rebind_alloc<char>, for example.

I think I prefer always rebinding to the proper allocator type: it seems silly to make users specify exact allocator types when we can rebind them so easily.

132

_VSTD::move to avoid ADL hijacking (_Alloc is a program-defined type).

147

The allocator requirements mandate that the constructor does not throw; they don't mandate that it is declared noexcept.

151

::new to avoid broken garbage operator new overloads in _Alloc.

220

Ditto ::new.

379

That was Kenny's WinRT generator which I have since changed to use exception_ptr directly, and Microsoft's crappy implementation of exception_ptr. I have no idea about libc++'s exception_ptr. (I expect to fix ours when we can next break ABI: it's really terrible that everything isn't inlinable in the empty/fast case.)

393

Looking at these on my second readthrough I've realized you don't have a consistent SMF story for the promise hierarchy. Those classes are sometimes copy/movable depending on the parameter type and whether or not exceptions are enabled. Should they have explicitly deleted SMFs?

[GorNishanov]

include/experimental/task

373

With regard to the use case.

Let's imagine that we will get the return_slot<T> type in C++20.
Now, we don't have to store the result in the promise, return_value will directly constructed the result in the call site of the co_await.

Or, even without the return_slot<T> type, we can construct the result directly in the awaiter. (That is not what we are doing and it is fine, but, if it were the case, you would not be able to extract the result the second time.

It is similar to the std::future where you can only get the result once.

[GorNishanov]

include/experimental/task

403

Should this be

task& operator=(task &&__other) _NOEXCEPT {

[GorNishanov]

include/experimental/task

387

Do we really have a use case for implicit public constructor from coroutine_handle?
I think this should be a private constructor.

[GorNishanov]

include/experimental/task

412

since we have both

operator co_await () &
operator co_await() &&

is it the same as simply having a

operator co_await ()

You allow mutable l-values, R-values, but not const values.

Also, why operator co_await at all? as opposed to three await_xxx functions?

I am thinking specifying the wording that allow implementators flexibility to pick either approach.
Is there a reason to mandate one over the other?

[lewissbaker]

include/experimental/task

83

I assume you mean allocator_traits<_Alloc>::is_always_equal?

117

Yes, the wording from coroutines TS states that operator new is passed lvalue-references to each of the coroutine parameters.

11.4.4(7) ... If the lookup finds an allocation function in the scope of P, overload resolution is performed on a function call created by assembling an argument list. The first argument is the amount of space requested, and has type std::size_t. The lvalues p1 ... pn are the succeeding arguments. ...

373

It is similar to the std::future where you can only get the result once.

Yes, I guess that single-await or multi-await is really the major choice here.

If we limit it so that you can only co_await the task once then it does give more flexibility on implementation that could potentially lead to a more efficient implementation in some cases. This does prevent the use-case I mentioned above, however.

I'm open to putting this restriction in for now - it matches the behaviour of std::future::get().

If we do this then we might want to consider having a single operator co_await that returns a prvalue - although this would potentially incur an extra copy/move in some cases compared with returning a reference to the value stored in the promise.

Let's imagine that we will get the return_slot<T> type in C++20.

Can P0878R0 (sub-object copy elision) help here if we put the result object in the awaiter and try to return the sub-object from await_resume()? If this can be made to work we may not need return_slot<T> to get RVO for tasks.

387

Yes, it probably should be a private constructor.

It was added to allow the coroutine to implicitly construct the task from the coroutine_handle returned from get_return_object() - this simplifies the implementation but isn't required.

I'll change get_return_object() to return a task<T> instead and add some appropriate friend-declarations.

393

The promise types are not really part of the public API so I wasn't so concerned with locking down the interface.

I can add the explicit declaration of deleted SMFs if you like.

403

Yes, the parameter could be made task&&.
I'd just need to move-construct to a temporary inside the body instead.

412

The reason operator co_await() are added is to allow overloading so that when a task<T>& is awaited it returns a T& and when a task<T>&& is awaited it returns a T&&`.

We can't overload await_resume() & and await_resume() && since the co_await mechanics states that it always calls await_resume() on the awaiter lvalue.

If we change task<T> to only support awaiting once then we could get rid of operator co_await() and just add await_xxx() methods directly on task<T>.

Having said that, it does feel a little dirty having the await_xxx() methods popup in intellisense completion when auto-completing methods on a task<T> instance - these methods are not intended to be called directly by users of the type.

[lewissbaker]

• Explicitly delete __task_promise_base special member functions.
• Declare _task_promise_base::operator delete() as _NOEXCEPT.
• Explicitly scope _VSTD::move() calls for user-defined types.
• Explicitly call global-scope placement operator new for user-defined types.
• Tweak task::operator=() to remove ambiguity with deleted operator=(const task&).

[CaseyCarter]

include/experimental/task

83

Yes, I did. I was thinking "use is_always_equal instead of is_empty" and apparently my brain decided to type them both.

[GorNishanov]

include/experimental/task

403

Ah Ok. No need to change it then. I am starting to work on wording and trying to figure out which things are the artifacts of the implementation and which should be carried over into the wording.

412

I will try to specify the wording so that both await_xxx and a single operator co_await are acceptable implementations.

I do strongly feel that task<T> should have await once semantics. It has a potential to unlock RVO. It is (potentially) less prone to misuse (my subjective feeling).

await_resume() for task<T> should return T. It allows RVO and eliminates dangling references.

No need to change anything at the moment in the implementation. Let's see how specification works out and then alter implementation (if needed) to match. Sometimes specifying something precisely gives clarity to a problem :-).

[EricWF]

Jeez, this is weird. Having other STL maintainers review additions to libc++ :-)

[CaseyCarter]

In D46140#1086987, @EricWF wrote:

Jeez, this is weird. Having other STL maintainers review additions to libc++ :-)

Speaking of which, I don't know *your* policy but we std`-qualify pretty calls even when they can't be ADL hijacked like _VSTD::terminate(); or _VSTD::move(reference_to_internal_non_template_type). There are a few such in here that I didn't comment.

[EricWF]

My apologies to the reviewers with knowledge of libc++'s existing style, but I would like to modernize the style with new headers like this, which already use C++14 features, like auto return types. That is, I would rather use constexpr over _LIBCPP_CONSTEXPR, and noexcept over _NOEXCEPT, ect. At this point I think it's safe to no longer a need for C++11, and in this header C++14, features to be guarded using macros.

Does everyone agree it's reasonable to limit this header to C++14 and beyond? If so, let's #ifdef out the header contents in older dialects.
If we bump this up to C++17 we can use variant to handle the non-trivially destructible union.

For whitespace, feel free to just clang-format the header.

Also, tests! Let me know if you need any help building and setting up a testing environment. Take a look at the Building Libc++ and Testing Libc++ docs to get started.

include/experimental/task

58

Let's reuse __aligned_allocation_size from memory_resource. I would move it into __memory, drop the assert, and make it constexpr

326

We'll want a constant ABI here. The exception_ptr type should be complete even when exceptions are disabled, so lets just always enable this.

include/module.modulemap

507

By convention this should be it's own module.

[EricWF]

include/experimental/task

193

= default?

280

= default ?

[lewissbaker]

Many improvements:

• Remove move-assignment operator
• task<T>::operator co_await() now returns by value when awaiting task<T> rvalue.
• Explicitly scope calls to std-library functions that live in _VSTD.
• Make __task_promise ABI stable under exception/no-exception compilation modes.
• Moved __aligned_allocation_size to <experimental/__memory>
• Make task<T> have a default template parameter of void, allowing task<> to mean task<void>
• Allow co_return { ... } by adding overload of return_value() taking T&& and another taking initializer_list<U>.
• Implement optimisation that avoids storing the allocator in the coroutine frame if allocator_traits<A>::is_always_equal is true and the allocator is default constructible.
• Added unit-tests (currently failing some custom allocator tests under debug builds)

[lewissbaker]

• Make task<T> constructor from coroutine_handle private.
• Run clang-format over <experimental/task>.

[GorNishanov]

LGTM

With a few suggestions.

1. Remove swap. It is not part of the proposal at the moment.
2. If you feel like, add a lazy<T> alias to task<T>, that way we can taste both names and see how it feels.
3. Also, there is still an outstanding work to add handling of implicit 'this'. Probably would be good to add before committing,

My apologies for taking so long to review.

[lewissbaker]

test/std/experimental/task/sync_wait.hpp

37–38

The call to notify_all() needs to be inside the lock.
Otherwise, it's possible the waiting thread may see the write to __isSet_ inside wait() below and return, destroying the condition_variable before __cv_.notify_all() call completes.

[lewissbaker]

• Don't use __allocator as variable name (it is #defined to NASTY_MACRO).
• Remove workaround in test for https://bugs.llvm.org/show_bug.cgi?id=37265
• Add support for custom allocators on member functions.

[lewissbaker]

Fix race condition in __oneshot_event::set() method used by sync_wait().

[modocache]

Committed in rL357010. Apologies, I forgot to include the differential revision in the commit message so this diff wasn't closed automatically as a result. I'll comment on rL357010 with the missing information.

[lewissbaker]

Reopening as the commit for this diff was reverted due to it breaking the buildbot module builds.

[lewissbaker]

Added missing 'require coroutines' for experimental/task entry in module.modulemap
Added missing #include in experimental/task header that was failing module builds

[lewissbaker]

@EricWF This implementation will currently not work with MSVC as MSVC does not yet support the symmetric-transfer capability added to the coroutines specification in P0913R0.

Is MSVC a target that libc++ needs to support coroutines with?
Or can we just say the task<T> class is unsupported for compilers that don't support P0913?

I have added the 'requires coroutines' line to the task modulemap entry which should hopefully fix the module build failures this diff was seeing.
Are there any other potential gotchas that I should make sure I test?

test/std/experimental/task/task.basic/task_of_value.pass.cpp

26

@EricWF This line may fail to compile on older versions of clang without the coroutines bugfix for https://bugs.llvm.org/show_bug.cgi?id=37265.

Should I just change this back to std::move(p) here, since the intention is to test the library, not the compiler?
Alternatively I can conditionally compile this line as either co_return p; or co_return std::move(p); depending on the compiler version.

[CaseyCarter]

include/experimental/__memory

80

This is missing preconditions that __a is a power of 2, and that __s <= -__a.

include/experimental/task

29

"requires a compiler with support for coroutines" would be more informative.

142

The return value of allocate isn't necessarily convertible to void*, it could be a fancy pointer. We should either static_assert(is_same_v<typename allocator_traits<_CharAlloc>::void_pointer, void*>, "Piss off with your fancy pointers"); or use pointer_traits here and in __deallocFunc to unfancy and re-fancy the pointer.

211

Pure style comment: I recommend using the non-elaborated friend __task_promise_final_awaitable; whenever possible. friend struct foo declares or redeclares struct foo in the enclosing namespace, whereas friend foo uses name lookup to find foo and makes it a friend. The latter form makes it far easier to analyze compiler errors when you screw something up in maintenance. (And on 480)

221

This mem-initializer for __state_ is redundant with the default member initializer on 306.

228

I suggest moving the case label and break outside the #ifndef here so the compiler won't warn about this case being unhandled when _LIBCPP_NO_EXCEPTIONS.

245

Are you certain that unhandled_exception can't possibly be called after storing a value? If so, this would leak the value.

255

Style: you've been using the _v variable templates for traits elsewhere.

262

Unnecessary std:: qualifier. (Occurs repeatedly.)

264

Style: the use of trailing-return-type SFINAE here is inconsistent with the use of template parameter SFINAE on 254.

309

These __empty_ members seem extraneous.

310

Should we static_assert that _Tp is a destructible object type?

374

Do we care about task<cv-void>?

390

Should these __foovalue_result members be foo-qualified to make them harder to misuse?

408

Similar to above, should we static_assert that _Tp is void, an lvalue reference type, or a destructible non-array object type?

454

this-> is extraneous in these classes that derive from the concrete _AwaiterBase. (And on 470)

459

Is missing a subject. How about "co_await on an invalid task<T> has undefined behavior"? (And on 475)

test/std/experimental/task/awaitable_traits.hpp

18

It's super sketchy for test code to inject things into the production namespace and to use reserved names. It's also inappropriate to use libc++ internals in a test in the std tree. (Occurs repeatedly.)

test/std/experimental/task/counted.hpp

82

IIUC that the tests all require C++17, you could declare these inline and avoid the external definitions / DEFINE_COUNTED_VARIABLES macro.

91

Should nextId_ be initialized to 1, as it is in reset on 40?

test/std/experimental/task/manual_reset_event.hpp

19

"currently". Also, "was not currently" is weird.

24

Here's a great example of why elaborated friend can be weird. This doesn't make the _Awaiter class defined on 25 a friend - which needn't be a friend, it's nested so it has full access to manual_reset_event - it declares a class named _Awaiter in the enclosing namespace that is a friend.

26

Ditto "using reserved names in test code". (Occurs repeatedly.)

41

_LIBCPP_ASSERT is inappropriate in std test code; it's undefined when the library under test is not libc++. Use LIBCPP_ASSERT (with no leading underscore) from test/support/test_macros.h to mean "this assertion must hold only if the library under test is libc++" and assert if the assertion must hold regardless of the tested library. (Occurs repeatedly.)

(Ignore the misuses of _LIBCPP_ASSERT that @mclow.lists put in the span tests; I'll fix them when we implement span and enable the tests.)

EDIT: Actually, I'll fix them now to save myself the trouble of figuring out why the tests are broken later.

48

Why are we concerned about data races on __event_->__state_, but not __event_->__awaitingCoroutine_?

50

s/this means that//; s/and so we/so we/; s/suspend - this/suspend. This/ (Ok, I'm nitpicking, but I actually did have issues parsing this comment.)

87

thread

89

"awaits the" what? The suspense is killing me!

93

std::exchange

103

What's the antecedent of "it"?

test/std/experimental/task/sync_wait.hpp

15

There's no such header in the working draft or the Coroutines TS.

37–38

The write to __isSet_ must be under the lock, the call to notify_all need not be. (Not that I care either way, just clarifying.)

71

Ditto "doesn't do what you think it does and is unnecessary."

116

Ditto "not in test code". You should include "test_macros.h" and use TEST_NO_EXCEPTIONS. (Occurs repeatedly.)

136

#include<new> for the definition of True Placement New. (This may be missing above as well, I don't recall or have time to scroll back.)

140

Here's another _VSTD that should be std. Please audit all test code.

test/std/experimental/task/task.basic/task_custom_allocator.pass.cpp

19

This is not a Standard or Coroutines TS header.

27

static is extraneous in an anonymous namespace.

38

These are the defaults for size_type, difference_type, and is_always_equal; you should omit them to verify that the tested code properly uses allocator_traits.

55

This is almost certainly going to cause problems, since it leaves the moved-from allocator invalid. (And on 68.)

76

I suggest assert(allocator_instance_count > 0) before decrementing. We're testing code that explicitly destroys things, we should make sure it doesn't destroy more things than it constructs.

79

allocate needs to return T*. (Also, std::size_t.) (And on 88)

91

Here again, I suggest an assertion that we're not freeing more bytes than have been allocated.

222

In libc++ tests main is always declared int main(int, char**) (and explicitly returns 0) to support the freestanding work. (Occurs repeatedly.)

test/std/experimental/task/task.lifetime/task_parameter_lifetime.pass.cpp

47

Again, leaving me in suspense.

[lewissbaker]

Thanks very much for the detailed review @CaseyCarter! Very much appreciated :)

include/experimental/__memory

80

Is there a recommended way of documenting/implementing these preconditions on a constexpr function in libc++?

The previous version that lived in <experimental/memory_resource> was not marked constexpr and so was able to use _LIBCPP_ASSERT.

include/experimental/task

29

This messaging was copied from <experimental/coroutine>.

I'll update the message there as well.

142

I'm not quite sure how to go about using pointer_traits here when interacting with coroutines and allocators.

Under C++20 I guess I would do something like:

typename allocator_traits<_CharAlloc>::void_pointer __pointer = __charAllocator.allocate(...);
...
return _VSTD::to_address(__pointer);

I assume that operator new() still needs to return void* rather than the fancy pointer so we need the to_address call in there. Is there some fallback for to_address() for unfancying the pointer in earlier standard versions?

Maybe we should just go with the static_assert() for now?

221

I'll remove the default member initializer on 306 then.

We can't = default the constructor here due to the union member containing types with non-trivial constructors/destructors.

245

Yes, I think there is a case where this could happen.

If we execute a co_return someValue which calls promise.return_value() and constructs __value_.
Then while executing the goto final_suspend; if any of the destructors of in-scope variables throw an exception which then escapes the coroutine body we could end up calling unhandled_exception() with __value_ already having been constructed.

Similarly, the exception thrown from a destructor could be caught and then a new co_return someOtherValue; executed.
So we should probably be guarding against __value_ containing a value inside return_value() as well.

309

The __empty_ members were added at your request ;)

See https://reviews.llvm.org/D46140?id=144168#inline-403822

I can remove them if you think they're not necessary.

310

Sure, will do.

I wonder if we should make this a requirement in the wording of P1056?
What do you think @GorNishanov?

374

I don't feel strongly either way.

It's not something I've ever had a use for, but maybe it should be done for completeness?

390

I'm not sure if it makes it much safer.
We already have 'rvalue' in the method name which gives a hint to its move-ness.

It's currently only called in one place:

return this->__coro_.promise().__rvalue_result();

would become:

return std::move(this->__coro_.promise()).__rvalue_result();

408

Sure, can do.

Further, to support task<T>::operator co_await() && T needs to both be destructible and move-constructible.
For task<T>::operator co_await() & T only needs to be destructible.

454

I get compile-errors if I remove the this->.

libcxx/include/experimental/task:502:16: error: 'std::experimental::coroutines_v1::task<counted>::__coro_' is not a member of class '_Awaiter'
        return __coro_.promise().__rvalue_result();

Compiler bug?

test/std/experimental/task/awaitable_traits.hpp

18

These were intended to be eventually added to the <experimental/coroutine> header and under the std::experimental namespace which is why they were using these macros. See P1288R0.

Similarly for sync_wait() which is proposed in P1171R0.

Is there some common namespace that test utilities should be declared within?
If not I'll just move them to global scope and de-uglify them for now.

test/std/experimental/task/counted.hpp

91

The intent was that any test would always initialise these values by calling reset() before running.
I hadn't intended the static-initialization to "do the right thing" here but I can if you think that's preferable.

test/std/experimental/task/manual_reset_event.hpp

48

Only the awaiting thread writes to __awaitingCoroutine_ and publishes this value when it writes to __state_ by setting the state to __not_set_waiting_coroutine.

A thread that calls .set() will only read from __awaitingCoroutine_ if, when it updates the __state_ variable, sees that the previous state was __not_set_waiting_coroutine.

So the write the __awaitingCoroutine_ should strictly happens-before any read from __awaitingCoroutine_.

test/std/experimental/task/sync_wait.hpp

37–38

The problem here is that once we write true to __isSet_ and release the lock that the thread calling wait() can potentially return and the on to destroy the __oneshot_event object, leaving the current thread with a dangling reference to a destroyed std::condition_variable.

I ran into a similar problem in cppcoro.
See https://github.com/lewissbaker/cppcoro/issues/98#issuecomment-450695759

test/std/experimental/task/task.basic/task_custom_allocator.pass.cpp

19

I can't #include <memory_resource> as libc++ doesn't currently provide that header.

How should I go about writing a test that makes sure that std::experimental::task works with std::experimental::pmr::polymorphic_allocator (or std::pmr::polymorphic_allocator if available)?

55

Isn't a moved-from allocator left in a valid but unspecified state?
ie. you can't assume it's ok to use. but should be ok to destruct or assign to.

[lewissbaker]

This updated diff should address most of @CaseyCarter's review comments.

For detailed changelog you can find individual changes here https://github.com/lewissbaker/libcxx/commits/coro_task

There are a few outstanding issues that still need some clarification on how to proceed:

• Should the task tests be taking a dependency on <experimental/memory_resource>?
• How should I implement a precondition check for __aligned_allocation_size when it is declared constexpr?
• I've not yet implemented support for task<cv-void>. Is this something we should support?
• I've not yet handling allocators with fancy pointer types. If we are to support them is there a standard way to convert a fancy pointer to a void* to return from promise_type::operator new()? Does it even make sense to support them if the compiler is potentially going to be storing raw pointers to things inside the coroutine frame?

[lewissbaker]

include/experimental/task

160

This should use explicit move construction rather than implicit move construction.

[lewissbaker]

Gentle ping.

Is there anything else people would like to see changed?

include/CMakeLists.txt

   experimental/string
   experimental/string_view
   experimental/system_error
+  experimental/task
   experimental/tuple
   experimental/type_traits
   experimental/unordered_map

include/experimental/__memory

     >
 {};
 
+// Round __s up to next multiple of __a.
+inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR
+size_t __aligned_allocation_size(size_t __s, size_t __a) _NOEXCEPT
+{
+    return (__s + __a - 1) & ~(__a - 1);
+}
+
 template <class _Tp, class _Alloc, class ..._Args>
 inline _LIBCPP_INLINE_VISIBILITY
 void __lfts_user_alloc_construct(

include/experimental/coroutine

 
 #ifdef _LIBCPP_HAS_NO_COROUTINES
 # if defined(_LIBCPP_WARNING)
-    _LIBCPP_WARNING("<experimental/coroutine> cannot be used with this compiler")
+    _LIBCPP_WARNING("<experimental/coroutine> requires a compiler with support for coroutines")
 # else
-#   warning <experimental/coroutine> cannot be used with this compiler
+#   warning <experimental/coroutine> requires a compiler with support for coroutines
 # endif
 #endif
 

include/experimental/memory_resource

 
 _LIBCPP_BEGIN_NAMESPACE_LFTS_PMR
 
-// Round __s up to next multiple of __a.
-inline _LIBCPP_INLINE_VISIBILITY
-size_t __aligned_allocation_size(size_t __s, size_t __a) _NOEXCEPT
-{
-    _LIBCPP_ASSERT(__s + __a > __s, "aligned allocation size overflows");
-    return (__s + __a - 1) & ~(__a - 1);
-}
-
 // 8.5, memory.resource
 class _LIBCPP_TYPE_VIS memory_resource
 {

include/experimental/task

+// -*- C++ -*-
+//===------------------------------- task ---------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef _LIBCPP_EXPERIMENTAL_TASK
+#define _LIBCPP_EXPERIMENTAL_TASK
+
+#include <experimental/__config>
+#include <experimental/__memory>
+#include <experimental/coroutine>
+
+#include <exception>
+#include <memory>
+#include <type_traits>
+#include <utility>
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+#pragma GCC system_header
+#endif
+
+#ifdef _LIBCPP_HAS_NO_COROUTINES
+#if defined(_LIBCPP_WARNING)
+_LIBCPP_WARNING("<experimental/task> requires a compiler with support for coroutines")
+#else
+#warning <experimental/task> requires a compiler with support for coroutines
+#endif
+#endif
+
+_LIBCPP_BEGIN_NAMESPACE_EXPERIMENTAL_COROUTINES
+
+////// task<T>
+
+template <typename _Tp = void>
+class task;
+
+struct __task_promise_final_awaitable {
+  _LIBCPP_INLINE_VISIBILITY
+  _LIBCPP_CONSTEXPR bool await_ready() const _NOEXCEPT { return false; }
+
+  template <typename _TaskPromise>
+  _LIBCPP_INLINE_VISIBILITY coroutine_handle<>
+  await_suspend(coroutine_handle<_TaskPromise> __coro) const _NOEXCEPT {
+    _LIBCPP_ASSERT(
+        __coro.promise().__continuation_,
+        "Coroutine completed without a valid continuation attached.");
+    return __coro.promise().__continuation_;
+  }
+
+  _LIBCPP_INLINE_VISIBILITY
+  void await_resume() const _NOEXCEPT {}
+};
+
+class _LIBCPP_TYPE_VIS __task_promise_base {
+  using _DeallocFunc = void(void* __ptr, size_t __size) _NOEXCEPT;
+
+  template <typename _Alloc>
+  static constexpr bool __allocator_needs_to_be_stored =
+      !allocator_traits<_Alloc>::is_always_equal::value ||
+      !is_default_constructible_v<_Alloc>;
+
+  static _LIBCPP_CONSTEXPR size_t
+  __get_dealloc_func_offset(size_t __frameSize) _NOEXCEPT {
+    return _VSTD_LFTS::__aligned_allocation_size(__frameSize,
+                                                 alignof(_DeallocFunc*));
+  }
+
+  static _LIBCPP_CONSTEXPR size_t
+  __get_padded_frame_size(size_t __frameSize) _NOEXCEPT {
+    return __get_dealloc_func_offset(__frameSize) + sizeof(_DeallocFunc*);
+  }
+
+  template <typename _Alloc>
+  static _LIBCPP_CONSTEXPR size_t
+  __get_allocator_offset(size_t __frameSize) _NOEXCEPT {
+    return _VSTD_LFTS::__aligned_allocation_size(
+        __get_padded_frame_size(__frameSize), alignof(_Alloc));
+  }
+
+  template <typename _Alloc>
+  static _LIBCPP_CONSTEXPR size_t
+  __get_padded_frame_size_with_allocator(size_t __frameSize) _NOEXCEPT {
+    if constexpr (__allocator_needs_to_be_stored<_Alloc>) {
+      return __get_allocator_offset<_Alloc>(__frameSize) + sizeof(_Alloc);
+    } else {
+      return __get_padded_frame_size(__frameSize);
+    }
+  }
+
+  _LIBCPP_INLINE_VISIBILITY
+  static _DeallocFunc*& __get_dealloc_func(void* __frameStart,
+                                           size_t __frameSize) _NOEXCEPT {
+    return *reinterpret_cast<_DeallocFunc**>(
+        static_cast<char*>(__frameStart) +
+        __get_dealloc_func_offset(__frameSize));
+  }
+
+  template <typename _Alloc>
+  _LIBCPP_INLINE_VISIBILITY static _Alloc&
+  __get_allocator(void* __frameStart, size_t __frameSize) _NOEXCEPT {
+    return *reinterpret_cast<_Alloc*>(
+        static_cast<char*>(__frameStart) +
+        __get_allocator_offset<_Alloc>(__frameSize));
+  }
+
+public:
+  __task_promise_base() _NOEXCEPT = default;
+
+  // Explicitly disable special member functions.
+  __task_promise_base(const __task_promise_base&) = delete;
+  __task_promise_base(__task_promise_base&&) = delete;
+  __task_promise_base& operator=(const __task_promise_base&) = delete;
+  __task_promise_base& operator=(__task_promise_base&&) = delete;
+
+  static void* operator new(size_t __size) {
+    // Allocate space for an extra pointer immediately after __size that holds
+    // the type-erased deallocation function.
+    void* __pointer = ::operator new(__get_padded_frame_size(__size));
+
+    _DeallocFunc*& __deallocFunc = __get_dealloc_func(__pointer, __size);
+    __deallocFunc = [](void* __pointer, size_t __size) _NOEXCEPT {
+      ::operator delete(__pointer, __get_padded_frame_size(__size));
+    };
+
+    return __pointer;
+  }
+
+  template <typename _Alloc, typename... _Args>
+  static void* operator new(size_t __size, allocator_arg_t, _Alloc& __alloc,
+                            _Args&...) {
+    using _CharAlloc =
+        typename allocator_traits<_Alloc>::template rebind_alloc<char>;
+
+    _CharAlloc __charAllocator{__alloc};
+
+    void* __pointer = __charAllocator.allocate(
+        __get_padded_frame_size_with_allocator<_CharAlloc>(__size));
+
+    _DeallocFunc*& __deallocFunc = __get_dealloc_func(__pointer, __size);
+    __deallocFunc = [](void* __pointer, size_t __size) _NOEXCEPT {
+      // Allocators are required to not throw from their move constructors
+      // however they aren't required to be declared noexcept so we can't
+      // actually check this with a static_assert.
+      //
+      // static_assert(is_nothrow_move_constructible_v<_Alloc>,
+      //              "task<T> coroutine custom allocator requires a noexcept "
+      //              "move constructor");
+
+      size_t __paddedSize =
+          __get_padded_frame_size_with_allocator<_CharAlloc>(__size);
+
+      if constexpr (__allocator_needs_to_be_stored<_CharAlloc>) {
+        _CharAlloc& __allocatorInFrame =
+            __get_allocator<_CharAlloc>(__pointer, __size);
+        _CharAlloc __allocatorOnStack = _VSTD::move(__allocatorInFrame);
+        __allocatorInFrame.~_CharAlloc();
+        // Allocator requirements state that deallocate() must not throw.
+        // See [allocator.requirements] from C++ standard.
+        // We are relying on that here.
+        __allocatorOnStack.deallocate(static_cast<char*>(__pointer),
+                                      __paddedSize);
+      } else {
+        _CharAlloc __alloc;
+        __alloc.deallocate(static_cast<char*>(__pointer), __paddedSize);
+      }
+    };
+
+    // Copy the allocator into the heap frame (if required)
+    if constexpr (__allocator_needs_to_be_stored<_CharAlloc>) {
+      // task<T> coroutine custom allocation requires the copy constructor to
+      // not throw but we can't rely on it being declared noexcept.
+      // If it did throw we'd leak the allocation here.
+      ::new (static_cast<void*>(
+          _VSTD::addressof(__get_allocator<_CharAlloc>(__pointer, __size))))
+          _CharAlloc(_VSTD::move(__charAllocator));
+    }
+
+    return __pointer;
+  }
+
+  template <typename _This, typename _Alloc, typename... _Args>
+  static void* operator new(size_t __size, _This&, allocator_arg_t __allocArg, _Alloc& __alloc,
+                            _Args&...) {
+    return __task_promise_base::operator new(__size, __allocArg, __alloc);
+  }
+
+  _LIBCPP_INLINE_VISIBILITY
+  static void operator delete(void* __pointer, size_t __size)_NOEXCEPT {
+    __get_dealloc_func(__pointer, __size)(__pointer, __size);
+  }
+
+  _LIBCPP_INLINE_VISIBILITY
+  suspend_always initial_suspend() const _NOEXCEPT { return {}; }
+
+  _LIBCPP_INLINE_VISIBILITY
+  __task_promise_final_awaitable final_suspend() _NOEXCEPT { return {}; }
+
+  _LIBCPP_INLINE_VISIBILITY
+  void __set_continuation(coroutine_handle<> __continuation) {
+    _LIBCPP_ASSERT(!__continuation_, "task already has a continuation");
+    __continuation_ = __continuation;
+  }
+
+private:
+  friend __task_promise_final_awaitable;
+
+  coroutine_handle<> __continuation_;
+};
+
+template <typename _Tp>
+class _LIBCPP_TEMPLATE_VIS __task_promise final : public __task_promise_base {
+  using _Handle = coroutine_handle<__task_promise>;
+
+  static_assert(
+    std::is_destructible_v<_Tp>,
+    "task<T> requires that T is destructible");
+
+public:
+  __task_promise() _NOEXCEPT : __state_(_State::__no_value) {}
+
+  ~__task_promise() {
+    switch (__state_) {
+    case _State::__value:
+      __value_.~_Tp();
+      break;
+    case _State::__exception:
+#ifndef _LIBCPP_NO_EXCEPTIONS
+      __exception_.~exception_ptr();
+#endif
+      break;
+    case _State::__no_value:
+      break;
+    }
+  }
+
+  _LIBCPP_INLINE_VISIBILITY
+  task<_Tp> get_return_object() _NOEXCEPT;
+
+  void unhandled_exception() _NOEXCEPT {
+#ifndef _LIBCPP_NO_EXCEPTIONS
+    if (__state_ == _State::__value) {
+      __state_ = _State::__no_value;
+      __value_.~_Tp();
+    }
+    _LIBCPP_ASSERT(
+      __state_ == _State::__no_value,
+      "Storage for the result should not already hold a value.");
+    ::new (static_cast<void*>(&__exception_))
+        exception_ptr(current_exception());
+    __state_ = _State::__exception;
+#else
+    _LIBCPP_ASSERT(
+        false, "task<T> coroutine unexpectedly called unhandled_exception()");
+#endif
+  }
+
+  // Only enable return_value() overload if _Tp is implicitly constructible from
+  // _Value
+  template <typename _Value,
+            enable_if_t<is_convertible_v<_Value, _Tp>, int> = 0>
+  void return_value(_Value&& __value)
+      _NOEXCEPT_((is_nothrow_constructible_v<_Tp, _Value>)) {
+    __construct_value(static_cast<_Value&&>(__value));
+  }
+
+  template <typename _Value,
+            enable_if_t<is_constructible_v<_Tp, initializer_list<_Value>>, int> = 0>
+  void return_value(initializer_list<_Value> __initializer) _NOEXCEPT_(
+      (is_nothrow_constructible_v<_Tp, initializer_list<_Value>>)) {
+    __construct_value(_VSTD::move(__initializer));
+  }
+
+  auto return_value(_Tp&& __value)
+      _NOEXCEPT_((is_nothrow_move_constructible_v<_Tp>))
+          -> enable_if_t<is_move_constructible_v<_Tp>> {
+    __construct_value(static_cast<_Tp&&>(__value));
+  }
+
+  _Tp& __lvalue_result() {
+    __throw_if_exception();
+    return __value_;
+  }
+
+  _Tp __rvalue_result() {
+    __throw_if_exception();
+    return static_cast<_Tp&&>(__value_);
+  }
+
+private:
+  template <typename... _Args>
+  void __construct_value(_Args&&... __args) {
+    // It's possible that this could be called multiple times in the case that
+    // an exception is thrown from a destructor of an in-scope variable after
+    // executing co_return, that exception is then caught and a new co_return
+    // statement is executed. So make sure we destruct any prior constructed
+    // return value before constructing a new value in its place.
+    if (__state_ == _State::__value) {
+      __state_ = _State::__no_value;
+      __value_.~_Tp();
+    }
+    _LIBCPP_ASSERT(
+      __state_ == _State::__no_value,
+      "Storage for the return-value should not already hold a value.");
+    ::new (static_cast<void*>(_VSTD::addressof(__value_)))
+        _Tp(static_cast<_Args&&>(__args)...);
+
+    // Only set __state_ after successfully constructing the value.
+    // If constructor throws then exception will propagate to the
+    // coroutine and if uncaught may eventually end up calling
+    // unhandled_exception().
+    __state_ = _State::__value;
+  }
+
+  void __throw_if_exception() {
+#ifndef _LIBCPP_NO_EXCEPTIONS
+    if (__state_ == _State::__exception) {
+      rethrow_exception(__exception_);
+    }
+#endif
+  }
+
+  enum class _State { __no_value, __value, __exception };
+
+  _State __state_;
+  union {
+    char __empty_;
+    _Tp __value_;
+    exception_ptr __exception_;
+  };
+};
+
+template <typename _Tp>
+class __task_promise<_Tp&> final : public __task_promise_base {
+  using _Ptr = _Tp*;
+  using _Handle = coroutine_handle<__task_promise>;
+
+public:
+  __task_promise() _NOEXCEPT = default;
+
+  ~__task_promise() {
+#ifndef _LIBCPP_NO_EXCEPTIONS
+    if (__has_exception_) {
+      __exception_.~exception_ptr();
+    }
+#endif
+  }
+
+  _LIBCPP_INLINE_VISIBILITY
+  task<_Tp&> get_return_object() _NOEXCEPT;
+
+  void unhandled_exception() _NOEXCEPT {
+#ifndef _LIBCPP_NO_EXCEPTIONS
+    ::new (static_cast<void*>(&__exception_))
+        exception_ptr(current_exception());
+    __has_exception_ = true;
+#else
+    _LIBCPP_ASSERT(
+        false, "task<T> coroutine unexpectedly called unhandled_exception()");
+#endif
+  }
+
+  void return_value(_Tp& __value) _NOEXCEPT {
+    ::new (static_cast<void*>(&__pointer_)) _Ptr(_VSTD::addressof(__value));
+  }
+
+  _Tp& __lvalue_result() {
+    __throw_if_exception();
+    return *__pointer_;
+  }
+
+  _Tp& __rvalue_result() { return __lvalue_result(); }
+
+private:
+  void __throw_if_exception() {
+#ifndef _LIBCPP_NO_EXCEPTIONS
+    if (__has_exception_) {
+      rethrow_exception(__exception_);
+    }
+#endif
+  }
+
+  union {
+    char __empty_;
+    _Ptr __pointer_;
+    exception_ptr __exception_;
+  };
+  bool __has_exception_ = false;
+};
+
+template <>
+class __task_promise<void> final : public __task_promise_base {
+  using _Handle = coroutine_handle<__task_promise>;
+
+public:
+  task<void> get_return_object() _NOEXCEPT;
+
+  void return_void() _NOEXCEPT {}
+
+  void unhandled_exception() _NOEXCEPT {
+#ifndef _LIBCPP_NO_EXCEPTIONS
+    __exception_ = current_exception();
+#endif
+  }
+
+  void __lvalue_result() { __throw_if_exception(); }
+
+  void __rvalue_result() { __throw_if_exception(); }
+
+private:
+  void __throw_if_exception() {
+#ifndef _LIBCPP_NO_EXCEPTIONS
+    if (__exception_) {
+      rethrow_exception(__exception_);
+    }
+#endif
+  }
+
+  exception_ptr __exception_;
+};
+
+template <typename _Tp>
+class _LIBCPP_TEMPLATE_VIS _LIBCPP_NODISCARD_AFTER_CXX17 task {
+public:
+  using promise_type = __task_promise<_Tp>;
+
+  static_assert(
+    std::is_same_v<_Tp, void> ||
+    std::is_lvalue_reference_v<_Tp> ||
+    (std::is_destructible_v<_Tp> &&
+     std::is_object_v<_Tp> &&
+     !std::is_array_v<_Tp>),
+    "task<T> supports only 'void', lvalue-reference or "
+    "a destructible, non-array object result types.");
+
+private:
+  using _Handle = coroutine_handle<__task_promise<_Tp>>;
+
+  class _AwaiterBase {
+  public:
+    _AwaiterBase(_Handle __coro) _NOEXCEPT : __coro_(__coro) {}
+
+    _LIBCPP_INLINE_VISIBILITY
+    bool await_ready() const { return __coro_.done(); }
+
+    _LIBCPP_INLINE_VISIBILITY
+    _Handle await_suspend(coroutine_handle<> __continuation) const {
+      __coro_.promise().__set_continuation(__continuation);
+      return __coro_;
+    }
+
+  protected:
+    _Handle __coro_;
+  };
+
+public:
+  _LIBCPP_INLINE_VISIBILITY
+  task(task&& __other) _NOEXCEPT
+      : __coro_(_VSTD::exchange(__other.__coro_, {})) {}
+
+  task(const task&) = delete;
+  task& operator=(const task&) = delete;
+
+  _LIBCPP_INLINE_VISIBILITY
+  ~task() {
+    if (__coro_)
+      __coro_.destroy();
+  }
+
+  _LIBCPP_INLINE_VISIBILITY
+  void swap(task& __other) _NOEXCEPT { _VSTD::swap(__coro_, __other.__coro_); }
+
+  _LIBCPP_INLINE_VISIBILITY
+  auto operator co_await() & {
+    class _Awaiter : public _AwaiterBase {
+    public:
+      using _AwaiterBase::_AwaiterBase;
+
+      _LIBCPP_INLINE_VISIBILITY
+      decltype(auto) await_resume() {
+        return this->__coro_.promise().__lvalue_result();
+      }
+    };
+
+    _LIBCPP_ASSERT(__coro_,
+                   "Undefined behaviour to co_await an invalid task<T>");
+    return _Awaiter{__coro_};
+  }
+
+  _LIBCPP_INLINE_VISIBILITY
+  auto operator co_await() && {
+    class _Awaiter : public _AwaiterBase {
+    public:
+      using _AwaiterBase::_AwaiterBase;
+
+      _LIBCPP_INLINE_VISIBILITY
+      decltype(auto) await_resume() {
+        return this->__coro_.promise().__rvalue_result();
+      }
+    };
+
+    _LIBCPP_ASSERT(__coro_,
+                   "co_await on an invalid task<T> has undefined behaviour");
+    return _Awaiter{__coro_};
+  }
+
+private:
+  friend __task_promise<_Tp>;
+
+  _LIBCPP_INLINE_VISIBILITY
+  task(_Handle __coro) _NOEXCEPT : __coro_(__coro) {}
+
+  _Handle __coro_;
+};
+
+template <typename _Tp>
+task<_Tp> __task_promise<_Tp>::get_return_object() _NOEXCEPT {
+  return task<_Tp>{_Handle::from_promise(*this)};
+}
+
+template <typename _Tp>
+task<_Tp&> __task_promise<_Tp&>::get_return_object() _NOEXCEPT {
+  return task<_Tp&>{_Handle::from_promise(*this)};
+}
+
+task<void> __task_promise<void>::get_return_object() _NOEXCEPT {
+  return task<void>{_Handle::from_promise(*this)};
+}
+
+_LIBCPP_END_NAMESPACE_EXPERIMENTAL_COROUTINES
+
+#endif

include/module.modulemap

       header "experimental/string"
       export *
     }
+    module task {
+      requires coroutines
+      header "experimental/task"
+      export *
+    }
     module type_traits {
       header "experimental/type_traits"
       export *

test/std/experimental/task/awaitable_traits.hpp

+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef TEST_EXPERIMENTAL_TASK_AWAITABLE_TRAITS_HPP
+#define TEST_EXPERIMENTAL_TASK_AWAITABLE_TRAITS_HPP
+
+#include <type_traits>
+#include <experimental/coroutine>
+
+namespace test_detail {
+
+template<typename Tp>
+struct is_coroutine_handle : std::false_type {};
+
+template<typename Tp>
+struct is_coroutine_handle<std::experimental::coroutine_handle<Tp>> :
+  std::true_type
+{};
+
+template<typename Tp>
+struct is_valid_await_suspend_result :
+  std::disjunction<
+    std::is_void<Tp>,
+    std::is_same<Tp, bool>,
+    is_coroutine_handle<Tp>>
+{};
+
+} // namespace test_detail
+
+template<typename Tp, typename = void>
+struct is_awaiter : std::false_type {};
+
+template<typename Tp>
+struct is_awaiter<Tp, std::void_t<
+  decltype(std::declval<Tp&>().await_ready()),
+  decltype(std::declval<Tp&>().await_resume()),
+  decltype(std::declval<Tp&>().await_suspend(
+    std::declval<std::experimental::coroutine_handle<void>>()))>> :
+  std::conjunction<
+    std::is_same<decltype(std::declval<Tp&>().await_ready()), bool>,
+    test_detail::is_valid_await_suspend_result<decltype(
+      std::declval<Tp&>().await_suspend(
+        std::declval<std::experimental::coroutine_handle<void>>()))>>
+{};
+
+template<typename Tp>
+constexpr bool is_awaiter_v = is_awaiter<Tp>::value;
+
+namespace test_detail {
+
+template<typename Tp, typename = void>
+struct has_member_operator_co_await : std::false_type {};
+
+template<typename Tp>
+struct has_member_operator_co_await<Tp, std::void_t<decltype(std::declval<Tp>().operator co_await())>>
+: is_awaiter<decltype(std::declval<Tp>().operator co_await())>
+{};
+
+template<typename Tp, typename = void>
+struct has_non_member_operator_co_await : std::false_type {};
+
+template<typename Tp>
+struct has_non_member_operator_co_await<Tp, std::void_t<decltype(operator co_await(std::declval<Tp>()))>>
+: is_awaiter<decltype(operator co_await(std::declval<Tp>()))>
+{};
+
+} // namespace test_detail
+
+template<typename Tp>
+struct is_awaitable : std::disjunction<
+  is_awaiter<Tp>,
+  test_detail::has_member_operator_co_await<Tp>,
+  test_detail::has_non_member_operator_co_await<Tp>>
+{};
+
+template<typename Tp>
+constexpr bool is_awaitable_v = is_awaitable<Tp>::value;
+
+template<
+  typename Tp,
+  std::enable_if_t<is_awaitable_v<Tp>, int> = 0>
+decltype(auto) get_awaiter(Tp&& awaitable)
+{
+  if constexpr (test_detail::has_member_operator_co_await<Tp>::value)
+  {
+    return static_cast<Tp&&>(awaitable).operator co_await();
+  }
+  else if constexpr (test_detail::has_non_member_operator_co_await<Tp>::value)
+  {
+    return operator co_await(static_cast<Tp&&>(awaitable));
+  }
+  else
+  {
+    return static_cast<Tp&&>(awaitable);
+  }
+}
+
+template<typename Tp, typename = void>
+struct await_result
+{};
+
+template<typename Tp>
+struct await_result<Tp, std::enable_if_t<is_awaitable_v<Tp>>>
+{
+private:
+  using awaiter = decltype(get_awaiter(std::declval<Tp>()));
+public:
+  using type = decltype(std::declval<awaiter&>().await_resume());
+};
+
+template<typename Tp>
+using await_result_t = typename await_result<Tp>::type;
+
+#endif

test/std/experimental/task/counted.hpp

+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef TEST_EXPERIMENTAL_TASK_COUNTED_HPP
+#define TEST_EXPERIMENTAL_TASK_COUNTED_HPP
+
+class counted
+{
+public:
+
+  counted() : id_(nextId_++)
+  {
+    ++defaultConstructedCount_;
+  }
+
+  counted(const counted& other) : id_(other.id_)
+  {
+    ++copyConstructedCount_;
+  }
+
+  counted(counted&& other) : id_(std::exchange(other.id_, 0))
+  {
+    ++moveConstructedCount_;
+  }
+
+  ~counted()
+  {
+    ++destructedCount_;
+  }
+
+  static void reset()
+  {
+    nextId_ = 1;
+    defaultConstructedCount_ = 0;
+    copyConstructedCount_ = 0;
+    moveConstructedCount_ = 0;
+    destructedCount_ = 0;
+  }
+
+  static std::size_t active_instance_count()
+  {
+    return
+      defaultConstructedCount_ +
+      copyConstructedCount_ +
+      moveConstructedCount_ -
+      destructedCount_;
+  }
+
+  static std::size_t copy_constructor_count()
+  {
+    return copyConstructedCount_;
+  }
+
+  static std::size_t move_constructor_count()
+  {
+    return moveConstructedCount_;
+  }
+
+  static std::size_t default_constructor_count()
+  {
+    return defaultConstructedCount_;
+  }
+
+  static std::size_t destructor_count()
+  {
+    return destructedCount_;
+  }
+
+  std::size_t id() const { return id_; }
+
+private:
+  std::size_t id_;
+
+  inline static std::size_t nextId_;
+  inline static std::size_t defaultConstructedCount_;
+  inline static std::size_t copyConstructedCount_;
+  inline static std::size_t moveConstructedCount_;
+  inline static std::size_t destructedCount_;
+
+};
+
+#endif

test/std/experimental/task/lit.local.cfg

+# If the compiler doesn't support coroutines mark all of the tests under
+# this directory as unsupported. Otherwise add the required `-fcoroutines-ts`
+# flag.
+if 'fcoroutines-ts' not in config.available_features:
+  config.unsupported = True
+else:
+  import copy
+  config.test_format.cxx = copy.deepcopy(config.test_format.cxx)
+  config.test_format.cxx.compile_flags += ['-fcoroutines-ts']

test/std/experimental/task/manual_reset_event.hpp

+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef TEST_EXPERIMENTAL_TASK_MANUAL_RESET_EVENT_HPP
+#define TEST_EXPERIMENTAL_TASK_MANUAL_RESET_EVENT_HPP
+
+#include <experimental/coroutine>
+#include <atomic>
+#include <cassert>
+
+// manual_reset_event is a coroutine synchronisation tool that allows one
+// coroutine to await the event object. If the event is in the 'set' state
+// then it continues without suspending, otherwise the coroutine suspends
+// until some thread calls .set() on the event.
+class manual_reset_event
+{
+  class awaiter
+  {
+  public:
+
+    awaiter(const manual_reset_event* event) noexcept
+    : event_(event)
+    {}
+
+    bool await_ready() const noexcept
+    {
+      return event_->is_set();
+    }
+
+    bool await_suspend(std::experimental::coroutine_handle<> coro) noexcept
+    {
+      assert(
+        (event_->state_.load(std::memory_order_relaxed) !=
+        state_t::not_set_waiting_coroutine) &&
+        "This manual_reset_event already has another coroutine awaiting it. "
+        "Only one awaiting coroutine is supported.");
+
+      event_->awaitingCoroutine_ = coro;
+
+      // If the compare-exchange fails then the event was already 'set'
+      // so we should not suspend. This code path requires 'acquire'
+      // semantics so we have visibility of writes prior to the
+      // .set() operation that transitioned the event to the 'set' state.
+      // If the compare-exchange succeeds then this needs 'release' semantics
+      // so that a subsequent call to .set() has visibility of our writes
+      // to the coroutine frame and to event_->awaitingCoroutine_ after
+      // reading our write to event_->state_.
+      state_t oldState = state_t::not_set;
+      return event_->state_.compare_exchange_strong(
+        oldState,
+        state_t::not_set_waiting_coroutine,
+        std::memory_order_release,
+        std::memory_order_acquire);
+    }
+
+    void await_resume() const noexcept {}
+
+  private:
+    const manual_reset_event* event_;
+  };
+
+public:
+
+  manual_reset_event(bool initiallySet = false) noexcept
+  : state_(initiallySet ? state_t::set : state_t::not_set)
+  {}
+
+  bool is_set() const noexcept
+  {
+    return state_.load(std::memory_order_acquire) == state_t::set;
+  }
+
+  void set() noexcept
+  {
+    // Needs to be 'acquire' in case the old value was a waiting coroutine
+    // so that we have visibility of the writes to the coroutine frame in
+    // the current thread before we resume it.
+    // Also needs to be 'release' in case the old value was 'not-set' so that
+    // another thread that subsequently awaits the event and reads the 'set'
+    // value with 'acquire' semantics has visibility of the prior writes that
+    // this thread performed.
+    state_t oldState = state_.exchange(state_t::set, std::memory_order_acq_rel);
+    if (oldState == state_t::not_set_waiting_coroutine)
+    {
+      std::exchange(awaitingCoroutine_, {}).resume();
+    }
+  }
+
+  void reset() noexcept
+  {
+    assert(
+      (state_.load(std::memory_order_relaxed) != state_t::not_set_waiting_coroutine) &&
+      "Illegal to call reset() if a coroutine is currently awaiting the event.");
+
+    // Note, we use 'relaxed' memory order here since it is considered an
+    // API-race to call reset() concurrently either with operator co_await()
+    // or with set().
+    state_.store(state_t::not_set, std::memory_order_relaxed);
+  }
+
+  awaiter operator co_await() const noexcept
+  {
+    return awaiter{ this };
+  }
+
+private:
+
+  enum class state_t {
+    not_set,
+    not_set_waiting_coroutine,
+    set
+  };
+
+  // TODO: Can we combine these two members into a single std::atomic<void*>?
+  mutable std::atomic<state_t> state_;
+  mutable std::experimental::coroutine_handle<> awaitingCoroutine_;
+
+};
+
+#endif

test/std/experimental/task/sync_wait.hpp

+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef TEST_EXPERIMENTAL_TASK_SYNC_WAIT_HPP
+#define TEST_EXPERIMENTAL_TASK_SYNC_WAIT_HPP
+
+#include <experimental/coroutine>
+#include <type_traits>
+#include <mutex>
+#include <new>
+#include <condition_variable>
+#include <cassert>
+
+#include "awaitable_traits.hpp"
+#include "test_macros.h"
+
+namespace test_detail {
+
+// Thread-synchronisation helper that allows one thread to block in a call
+// to .wait() until another thread signals the thread by calling .set().
+class oneshot_event
+{
+public:
+  oneshot_event() : isSet_(false) {}
+
+  void set() noexcept
+  {
+    std::unique_lock<std::mutex> lock{ mutex_ };
+    isSet_ = true;
+    cv_.notify_all();
+  }
+
+  void wait() noexcept
+  {
+    std::unique_lock<std::mutex> lock{ mutex_ };
+    cv_.wait(lock, [this] { return isSet_; });
+  }
+
+private:
+  std::mutex mutex_;
+  std::condition_variable cv_;
+  bool isSet_;
+};
+
+template<typename Derived>
+class sync_wait_promise_base
+{
+public:
+
+  using handle_t = std::experimental::coroutine_handle<Derived>;
+
+private:
+
+  struct FinalAwaiter
+  {
+      bool await_ready() noexcept { return false; }
+      void await_suspend(handle_t coro) noexcept
+      {
+        sync_wait_promise_base& promise = coro.promise();
+        promise.event_.set();
+      }
+      void await_resume() noexcept {}
+  };
+
+public:
+
+  handle_t get_return_object() { return handle(); }
+  std::experimental::suspend_always initial_suspend() { return {}; }
+  FinalAwaiter final_suspend() { return {}; }
+
+private:
+
+  handle_t handle() noexcept
+  {
+    return handle_t::from_promise(static_cast<Derived&>(*this));
+  }
+
+protected:
+
+  // Start the coroutine and then block waiting for it to finish.
+  void run() noexcept
+  {
+    handle().resume();
+    event_.wait();
+  }
+
+private:
+
+  oneshot_event event_;
+
+};
+
+template<typename Tp>
+class sync_wait_promise final
+  : public sync_wait_promise_base<sync_wait_promise<Tp>>
+{
+public:
+
+  sync_wait_promise() : state_(state_t::empty) {}
+
+  ~sync_wait_promise()
+  {
+    switch (state_)
+    {
+      case state_t::empty:
+      case state_t::value:
+        break;
+      case state_t::exception:
+#ifndef TEST_HAS_NO_EXCEPTIONS
+        exception_.~exception_ptr();
+#endif
+        break;
+    }
+  }
+
+  void return_void() noexcept
+  {
+    // Should be unreachable since coroutine should always
+    // suspend at `co_yield` point where it will be destroyed
+    // or will fail with an exception and bypass return_void()
+    // and call unhandled_exception() instead.
+    std::abort();
+  }
+
+  void unhandled_exception() noexcept
+  {
+#ifndef TEST_HAS_NO_EXCEPTIONS
+    ::new (static_cast<void*>(&exception_)) std::exception_ptr(
+      std::current_exception());
+    state_ = state_t::exception;
+#else
+    std::abort();
+#endif
+  }
+
+  auto yield_value(Tp&& value) noexcept
+  {
+    valuePtr_ = std::addressof(value);
+    state_ = state_t::value;
+    return this->final_suspend();
+  }
+
+  Tp&& get()
+  {
+    this->run();
+
+#ifndef TEST_HAS_NO_EXCEPTIONS
+    if (state_ == state_t::exception)
+    {
+      std::rethrow_exception(std::move(exception_));
+    }
+#endif
+
+    assert(state_ == state_t::value);
+    return static_cast<Tp&&>(*valuePtr_);
+  }
+
+private:
+
+  enum class state_t {
+    empty,
+    value,
+    exception
+  };
+
+  state_t state_;
+  union {
+    std::add_pointer_t<Tp> valuePtr_;
+    std::exception_ptr exception_;
+  };
+
+};
+
+template<>
+struct sync_wait_promise<void> final
+  : public sync_wait_promise_base<sync_wait_promise<void>>
+{
+public:
+
+  void unhandled_exception() noexcept
+  {
+#ifndef TEST_HAS_NO_EXCEPTIONS
+    exception_ = std::current_exception();
+#endif
+  }
+
+  void return_void() noexcept {}
+
+  void get()
+  {
+    this->run();
+
+#ifndef TEST_HAS_NO_EXCEPTIONS
+    if (exception_)
+    {
+      std::rethrow_exception(std::move(exception_));
+    }
+#endif
+  }
+
+private:
+
+  std::exception_ptr exception_;
+
+};
+
+template<typename Tp>
+class sync_wait_task final
+{
+public:
+  using promise_type = sync_wait_promise<Tp>;
+
+private:
+  using handle_t = typename promise_type::handle_t;
+
+public:
+
+  sync_wait_task(handle_t coro) noexcept : coro_(coro) {}
+
+  ~sync_wait_task()
+  {
+    assert(coro_ && "Should always have a valid coroutine handle");
+    coro_.destroy();
+  }
+
+  decltype(auto) get()
+  {
+    return coro_.promise().get();
+  }
+private:
+  handle_t coro_;
+};
+
+template<typename Tp>
+struct remove_rvalue_reference
+{
+  using type = Tp;
+};
+
+template<typename Tp>
+struct remove_rvalue_reference<Tp&&>
+{
+  using type = Tp;
+};
+
+template<typename Tp>
+using remove_rvalue_reference_t =
+  typename remove_rvalue_reference<Tp>::type;
+
+template<
+  typename Awaitable,
+  typename AwaitResult = await_result_t<Awaitable>,
+  std::enable_if_t<std::is_void_v<AwaitResult>, int> = 0>
+sync_wait_task<AwaitResult> make_sync_wait_task(Awaitable&& awaitable)
+{
+  co_await static_cast<Awaitable&&>(awaitable);
+}
+
+template<
+  typename Awaitable,
+  typename AwaitResult = await_result_t<Awaitable>,
+  std::enable_if_t<!std::is_void_v<AwaitResult>, int> = 0>
+sync_wait_task<AwaitResult> make_sync_wait_task(Awaitable&& awaitable)
+{
+  co_yield co_await static_cast<Awaitable&&>(awaitable);
+}
+
+} // namespace test_detail
+
+template<typename Awaitable>
+auto sync_wait(Awaitable&& awaitable)
+  -> test_detail::remove_rvalue_reference_t<await_result_t<Awaitable>>
+{
+  return test_detail::make_sync_wait_task(
+    static_cast<Awaitable&&>(awaitable)).get();
+}
+
+#endif

test/std/experimental/task/task.basic/task_custom_allocator.pass.cpp

+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++98, c++03, c++11, c++14
+
+#include <experimental/task>
+#include <cstdlib>
+#include <cassert>
+#include <vector>
+#include <memory>
+#include <experimental/memory_resource>
+
+#include "../sync_wait.hpp"
+
+namespace coro = std::experimental::coroutines_v1;
+
+namespace
+{
+  std::size_t allocator_instance_count = 0;
+
+  // A custom allocator that tracks the number of allocator instances that
+  // have been constructed/destructed as well as the number of bytes that
+  // have been allocated/deallocated using the allocator.
+  template<typename T>
+  class my_allocator {
+  public:
+    using value_type = T;
+    // Omit size_type, difference_type, is_always_equal to check that task<T>
+    // implementation is using allocator_traits which provides defaults for these.
+
+    explicit my_allocator(
+      std::shared_ptr<std::size_t> totalAllocated) noexcept
+      : totalAllocated_(std::move(totalAllocated))
+    {
+      ++allocator_instance_count;
+      assert(totalAllocated_);
+    }
+
+    my_allocator(const my_allocator& other)
+    : totalAllocated_(other.totalAllocated_)
+    {
+      ++allocator_instance_count;
+    }
+
+    my_allocator(my_allocator&& other)
+    : totalAllocated_(std::move(other.totalAllocated_))
+    {
+      ++allocator_instance_count;
+    }
+
+    template<typename U>
+    my_allocator(const my_allocator<U>& other)
+    : totalAllocated_(other.totalAllocated_)
+    {
+      ++allocator_instance_count;
+    }
+
+    template<typename U>
+    my_allocator(my_allocator<U>&& other)
+    : totalAllocated_(std::move(other.totalAllocated_))
+    {
+      ++allocator_instance_count;
+    }
+
+    ~my_allocator()
+    {
+      assert(allocator_instance_count > 0);
+      --allocator_instance_count;
+    }
+
+    T* allocate(std::size_t n) {
+      assert(totalAllocated_);
+      const auto byteCount = n * sizeof(T);
+      void* p = std::malloc(byteCount);
+      if (!p) {
+        throw std::bad_alloc{};
+      }
+      *totalAllocated_ += byteCount;
+      return static_cast<T*>(p);
+    }
+
+    void deallocate(char* p, std::size_t n) {
+      const auto byteCount = n * sizeof(T);
+      assert(totalAllocated_);
+      assert(byteCount <= *totalAllocated_);
+      *totalAllocated_ -= byteCount;
+      std::free(p);
+    }
+  private:
+    template<typename U>
+    friend class my_allocator;
+
+    std::shared_ptr<std::size_t> totalAllocated_;
+  };
+}
+
+template<typename Allocator>
+coro::task<void> f(std::allocator_arg_t, [[maybe_unused]] Allocator alloc)
+{
+  co_return;
+}
+
+void test_custom_allocator_is_destructed()
+{
+  auto totalAllocated = std::make_shared<std::size_t>(0);
+
+  assert(allocator_instance_count == 0);
+
+  {
+    std::vector<coro::task<>> tasks;
+    tasks.push_back(
+      f(std::allocator_arg, my_allocator<char>{ totalAllocated }));
+    tasks.push_back(
+      f(std::allocator_arg, my_allocator<char>{ totalAllocated }));
+
+    assert(allocator_instance_count == 4);
+    assert(*totalAllocated > 0);
+  }
+
+  assert(allocator_instance_count == 0);
+  assert(*totalAllocated == 0);
+}
+
+void test_custom_allocator_type_rebinding()
+{
+  auto totalAllocated = std::make_shared<std::size_t>(0);
+  {
+    std::vector<coro::task<>> tasks;
+    tasks.emplace_back(
+      f(std::allocator_arg, my_allocator<int>{ totalAllocated }));
+    ::sync_wait(tasks[0]);
+  }
+  assert(*totalAllocated == 0);
+  assert(allocator_instance_count == 0);
+}
+
+void test_mixed_custom_allocator_type_erasure()
+{
+  assert(allocator_instance_count == 0);
+
+  // Show that different allocators can be used within a vector of tasks
+  // of the same type. ie. that the allocator is type-erased inside the
+  // coroutine.
+  std::vector<coro::task<>> tasks;
+  tasks.push_back(f(
+    std::allocator_arg, std::allocator<char>{}));
+  tasks.push_back(f(
+    std::allocator_arg,
+    std::experimental::pmr::polymorphic_allocator<char>{
+       std::experimental::pmr::new_delete_resource() }));
+  tasks.push_back(f(
+    std::allocator_arg,
+    my_allocator<char>{ std::make_shared<size_t>(0) }));
+
+  assert(allocator_instance_count > 0);
+
+  for (auto& t : tasks)
+  {
+    ::sync_wait(t);
+  }
+
+  tasks.clear();
+
+  assert(allocator_instance_count == 0);
+}
+
+template<typename Allocator>
+coro::task<int> add_async(std::allocator_arg_t, [[maybe_unused]] Allocator alloc, int a, int b)
+{
+  co_return a + b;
+}
+
+void test_task_custom_allocator_with_extra_args()
+{
+  std::vector<coro::task<int>> tasks;
+
+  for (int i = 0; i < 5; ++i) {
+    tasks.push_back(add_async(
+      std::allocator_arg,
+      std::allocator<char>{},
+      i, 2 * i));
+  }
+
+  for (int i = 0; i < 5; ++i)
+  {
+    assert(::sync_wait(std::move(tasks[i])) == 3 * i);
+  }
+}
+
+struct some_type {
+  template<typename Allocator>
+  coro::task<int> get_async(std::allocator_arg_t, [[maybe_unused]] Allocator alloc) {
+    co_return 42;
+  }
+
+  template<typename Allocator>
+  coro::task<int> add_async(std::allocator_arg_t, [[maybe_unused]] Allocator alloc, int a, int b) {
+    co_return a + b;
+  }
+};
+
+void test_task_custom_allocator_on_member_function()
+{
+  assert(allocator_instance_count == 0);
+
+  auto totalAllocated = std::make_shared<std::size_t>(0);
+  some_type obj;
+  assert(::sync_wait(obj.get_async(std::allocator_arg, std::allocator<char>{})) == 42);
+  assert(::sync_wait(obj.get_async(std::allocator_arg, my_allocator<char>{totalAllocated})) == 42);
+  assert(::sync_wait(obj.add_async(std::allocator_arg, std::allocator<char>{}, 2, 3)) == 5);
+  assert(::sync_wait(obj.add_async(std::allocator_arg, my_allocator<char>{totalAllocated}, 2, 3)) == 5);
+
+  assert(allocator_instance_count == 0);
+  assert(*totalAllocated == 0);
+}
+
+int main(int, char**)
+{
+  test_custom_allocator_is_destructed();
+  test_custom_allocator_type_rebinding();
+  test_mixed_custom_allocator_type_erasure();
+  test_task_custom_allocator_with_extra_args();
+  test_task_custom_allocator_on_member_function();
+
+  return 0;
+}

test/std/experimental/task/task.basic/task_of_value.pass.cpp

+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++98, c++03, c++11, c++14
+
+#include <experimental/task>
+#include <string>
+#include <vector>
+#include <memory>
+#include "../sync_wait.hpp"
+
+void test_returning_move_only_type()
+{
+  auto move_only_async =
+   [](bool x) -> std::experimental::task<std::unique_ptr<int>> {
+     if (x) {
+       auto p = std::make_unique<int>(123);
+       co_return p; // Should be implicit std::move(p) here.
+     }
+
+     co_return std::make_unique<int>(456);
+   };
+
+  assert(*::sync_wait(move_only_async(true)) == 123);
+  assert(*::sync_wait(move_only_async(false)) == 456);
+}
+
+void test_co_return_with_curly_braces()
+{
+  auto t = []() -> std::experimental::task<std::tuple<int, std::string>>
+  {
+    co_return { 123, "test" };
+  }();
+
+  auto result = ::sync_wait(std::move(t));
+
+  assert(std::get<0>(result) == 123);
+  assert(std::get<1>(result) == "test");
+}
+
+void test_co_return_by_initialiser_list()
+{
+  auto t = []() -> std::experimental::task<std::vector<int>>
+  {
+    co_return { 2, 10, -1 };
+  }();
+
+  auto result = ::sync_wait(std::move(t));
+
+  assert(result.size() == 3);
+  assert(result[0] == 2);
+  assert(result[1] == 10);
+  assert(result[2] == -1);
+}
+
+int main(int, char**)
+{
+  test_returning_move_only_type();
+  test_co_return_with_curly_braces();
+  test_co_return_by_initialiser_list();
+
+  return 0;
+}

test/std/experimental/task/task.basic/task_of_void.pass.cpp

+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++98, c++03, c++11, c++14
+
+#include <experimental/task>
+#include "../manual_reset_event.hpp"
+#include "../sync_wait.hpp"
+
+#include <optional>
+#include <thread>
+
+namespace coro = std::experimental::coroutines_v1;
+
+static bool has_f_executed = false;
+
+static coro::task<void> f()
+{
+  has_f_executed = true;
+  co_return;
+}
+
+static void test_coroutine_executes_lazily()
+{
+  coro::task<void> t = f();
+  assert(!has_f_executed);
+  ::sync_wait(t);
+  assert(has_f_executed);
+}
+
+static std::optional<int> last_value_passed_to_g;
+
+static coro::task<void> g(int a)
+{
+  last_value_passed_to_g = a;
+  co_return;
+}
+
+void test_coroutine_accepts_arguments()
+{
+  auto t = g(123);
+  assert(!last_value_passed_to_g);
+  ::sync_wait(t);
+  assert(last_value_passed_to_g);
+  assert(*last_value_passed_to_g == 123);
+}
+
+int shared_value = 0;
+int read_value = 0;
+
+coro::task<void> consume_async(manual_reset_event& event)
+{
+  co_await event;
+  read_value = shared_value;
+}
+
+void produce(manual_reset_event& event)
+{
+  shared_value = 101;
+  event.set();
+}
+
+void test_async_completion()
+{
+  manual_reset_event e;
+  std::thread t1{ [&e]
+  {
+    ::sync_wait(consume_async(e));
+  }};
+
+  assert(read_value == 0);
+
+  std::thread t2{ [&e] { produce(e); }};
+
+  t1.join();
+
+  assert(read_value == 101);
+
+  t2.join();
+}
+
+int main(int, char**)
+{
+  test_coroutine_executes_lazily();
+  test_coroutine_accepts_arguments();
+  test_async_completion();
+
+  return 0;
+}

test/std/experimental/task/task.lifetime/task_parameter_lifetime.pass.cpp

+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++98, c++03, c++11, c++14
+
+#include <experimental/task>
+#include <cassert>
+
+#include "../counted.hpp"
+#include "../sync_wait.hpp"
+
+void test_parameter_lifetime()
+{
+  counted::reset();
+
+  auto f = [](counted c) -> std::experimental::task<std::size_t>
+  {
+    co_return c.id();
+  };
+
+  {
+    auto t = f({});
+
+    assert(counted::active_instance_count() == 1);
+    assert(counted::copy_constructor_count() == 0);
+    assert(counted::move_constructor_count() <= 2); // Ideally <= 1
+
+    auto id = ::sync_wait(t);
+    assert(id == 1);
+
+    assert(counted::active_instance_count() == 1);
+    assert(counted::copy_constructor_count() == 0);
+
+    // We are relying on C++17 copy-elision when passing the temporary counter
+    // into f(). Then f() must move the parameter into the coroutine frame by
+    // calling the move-constructor. This move could also potentially be
+    // elided by the compiler if the parameter is not referenced after the
+    // first suspend-point.
+    assert(counted::move_constructor_count() <= 1);
+  }
+
+  assert(counted::active_instance_count() == 0);
+}
+
+int main(int, char**)
+{
+  test_parameter_lifetime();
+  return 0;
+}

test/std/experimental/task/task.lifetime/task_return_value_lifetime.pass.cpp

+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++98, c++03, c++11, c++14
+
+#include <experimental/task>
+#include <cassert>
+#include <iostream>
+
+#include "../counted.hpp"
+#include "../sync_wait.hpp"
+
+void test_return_value_lifetime()
+{
+  counted::reset();
+
+  auto f = [](bool x) -> std::experimental::task<counted>
+  {
+    if (x) {
+      counted c;
+      co_return std::move(c);
+    }
+    co_return {};
+  };
+
+  {
+    auto t = f(true);
+
+    assert(counted::active_instance_count() == 0);
+    assert(counted::copy_constructor_count() == 0);
+    assert(counted::move_constructor_count() == 0);
+
+    {
+      auto c = ::sync_wait(std::move(t));
+      assert(c.id() == 1);
+
+      assert(counted::active_instance_count() == 2);
+      assert(counted::copy_constructor_count() == 0);
+      assert(counted::move_constructor_count() > 0);
+      assert(counted::default_constructor_count() == 1);
+    }
+
+    // The result value in 't' is still alive until 't' destructs.
+    assert(counted::active_instance_count() == 1);
+  }
+
+  assert(counted::active_instance_count() == 0);
+
+  counted::reset();
+
+  {
+    auto t = f(false);
+
+    assert(counted::active_instance_count() == 0);
+    assert(counted::copy_constructor_count() == 0);
+    assert(counted::move_constructor_count() == 0);
+
+    {
+      auto c = ::sync_wait(std::move(t));
+      assert(c.id() == 1);
+
+      assert(counted::active_instance_count() == 2);
+      assert(counted::copy_constructor_count() == 0);
+      assert(counted::move_constructor_count() > 0);
+      assert(counted::default_constructor_count() == 1);
+    }
+
+    // The result value in 't' is still alive until 't' destructs.
+    assert(counted::active_instance_count() == 1);
+  }
+}
+
+struct my_error {};
+
+struct throws_on_destruction
+{
+  ~throws_on_destruction() noexcept(false)
+  {
+    throw my_error{};
+  }
+};
+
+void test_uncaught_exception_thrown_after_co_return()
+{
+  counted::reset();
+
+  assert(counted::active_instance_count() == 0);
+  assert(counted::copy_constructor_count() == 0);
+  assert(counted::move_constructor_count() == 0);
+
+  {
+    auto t = []() -> std::experimental::task<counted>
+    {
+      throws_on_destruction d;
+      co_return counted{};
+    }();
+
+    try {
+      (void)::sync_wait(std::move(t));
+      assert(false);
+    } catch (const my_error&) {
+    }
+
+    assert(counted::active_instance_count() == 0);
+    assert(counted::copy_constructor_count() == 0);
+    assert(counted::move_constructor_count() > 0);
+    assert(counted::default_constructor_count() == 1);
+  }
+
+  assert(counted::active_instance_count() == 0);
+}
+
+void test_exception_thrown_and_caught_after_co_return()
+{
+  counted::reset();
+
+  assert(counted::active_instance_count() == 0);
+  assert(counted::copy_constructor_count() == 0);
+  assert(counted::move_constructor_count() == 0);
+
+  {
+    auto t = []() -> std::experimental::task<counted>
+    {
+      try {
+        throws_on_destruction d;
+        co_return counted{};
+      } catch(const my_error&) {
+        co_return counted{};
+      }
+    }();
+
+    auto c = ::sync_wait(std::move(t));
+    assert(c.id() == 2);
+
+    assert(counted::active_instance_count() == 2);
+    assert(counted::copy_constructor_count() == 0);
+    assert(counted::move_constructor_count() > 0);
+    assert(counted::default_constructor_count() == 2);
+  }
+
+  assert(counted::active_instance_count() == 0);
+}
+
+int main(int, char**)
+{
+  test_return_value_lifetime();
+  test_uncaught_exception_thrown_after_co_return();
+  test_exception_thrown_and_caught_after_co_return();
+  return 0;
+}