GorNishanov / coroutines-ts

20 stars 2 forks source link

Editorial: coroutine_handle::from_address - consolidate duplicate definitions, add missing constexpr and replace address() with address in precondition wording #4

Open brycelelbach opened 7 years ago

brycelelbach commented 7 years ago

This PR contains editorial fixes relating to coroutine_handle's from_address method ([coroutine.handle.import.export]). It is related to #5.


Consolidate duplicate definitions of

There are currently two definitions of the from_address: one in [coroutine.handle.import.export] and one in [coroutine.handle.import]. In the class synopses in [coroutine.handle], the coroutine_handle<> specialization references [coroutine.handle.import.export] while primary definition for coroutine_handle references [corouinte.handle.import].

They are nearly identical in wording. although the definition in [coroutine.handle.import.export] is written as if it was out of line (e.g. coroutine_handle<>::from_address).

Even though the primary template of coroutine_handle inherits from coroutine_handle<> it is necessary to define from_address in the primary template, but I did not realize this at first. from_address returns coroutine_handle, which is a different type in the primary template than it is in coroutine_handle (we use coroutine_handle within the class definition so this may not be clear at first to consumers of the spec):

template <typename T>
struct A;

template <>
struct A<void> { static A make_A() { return A{}; } };

template <typename T>
struct A : A<void> { };

int main()
{
    A<void> a = A<void>::make_A(); // OK
    A<int> a = A<int>::make_A();   // ERROR: Conflicting declarations.
}

Otherwise, the two definitions are identical.

I think the best thing to do is to keep the declarations in both classes in the class synopsis and have one definition of from_address, but list both signatures as if they were out of line, e.g.:

constexpr static coroutine_handle<> coroutine_handle<>::from_address(void* addr);
constexpr static coroutine_handle<Promise> coroutine_handle<Promise>::from_address(void* addr);

Replace address() with address in precondition wording

from_address's Requires: paragraph in [coroutine.handle.import.export] states the pre-condition that "addr was obtained via a prior call to address()". It should be address, not address(), since address() is an expression not a method (e.g. read very pedantically, this says "obtained via a prior call to some void*" which doesn't make sense).


Add missing constexpr

from_address is declared constexpr in the class synopsis ([coroutine.handle], in both the primary template and the specialization for coroutine_handle<>) but is not constexpr in the definition. The design intent, I believe, is for from_address to be constexpr.


This pull request:

brycelelbach commented 6 years ago

Hey Gor whatever happened with this?

GorNishanov commented 6 years ago

Can you double check the current wording. I think it reflects your changes.