Is it the time to mark coroutine status as green in cxx_status page?

Clang Frontend
1

Hi,

I noticed that the status of coroutine in cxx_status page is partial previously: Clang - C++ Programming Language Status.
This looks weird to me since coroutine is used broadly and stably in practice and I want to mark it as green. So I tried
to move coroutine out of the experimental namespace before and implement the semantics of throwing unhandled_exception()
suggested by Lewis.
So I sent ⚙ D115692 [docs] Mark the support for coroutine as unreleased to mark coroutine as done. I want to ask if there is any blocking issue to do this?
I would love to try to fix any inconsistency.

Thanks,
Chuanqi

2

There hasn’t been a resolution for the issue where coroutine-lowering runs optimizations passes before splitting, which breaks thread_local and attribute((const)) functions (affecting e.g. pthread_self() and errno). That’s definitely a critical issue which must be resolved before coroutines can be reasonably called “done”.

See thread “[llvm-dev] [RFC] Coroutine and pthread_self” thread from a year ago, e.g. https://lists.llvm.org/pipermail/llvm-dev/2020-December/147325.html for the end/summary of the conversation. (Or, starting at https://lists.llvm.org/pipermail/llvm-dev/2020-November/146766.html and https://lists.llvm.org/pipermail/llvm-dev/2020-December/147012.html)

Looks like there’s a lot of open bugs about coroutines, https://github.com/llvm/llvm-project/issues?q=is%3Aissue+is%3Aopen+coroutine …might be worth going through all of them…
But, just cherry-picking a few about the above problem:
https://github.com/llvm/llvm-project/issues/47179 (with thread_local)

https://github.com/llvm/llvm-project/issues/47177 (with pthread_self)

https://github.com/llvm/llvm-project/issues/49257 (with errno)

3

There hasn't been a resolution for the issue where coroutine-lowering runs optimizations passes before splitting, which breaks thread_local and __attribute__((const)) functions (affecting e.g. pthread_self() and errno). That's definitely a critical issue which must be resolved before coroutines can be reasonably called "done".

See thread "[llvm-dev] [RFC] Coroutine and pthread_self" thread from a year ago, e.g. [llvm-dev] [RFC] Coroutine and pthread_self for the end/summary of the conversation. (Or, starting at [llvm-dev] [RFC] Coroutine and pthread_self and [llvm-dev] [RFC] Coroutine and pthread_self)

Looks like there's a lot of open bugs about coroutines, Issues · llvm/llvm-project · GitHub ...might be worth going through all of them...
But, just cherry-picking a few about the above problem:
[coroutines] Compiler incorrectly caches thread_local address across suspend-points · Issue #47179 · llvm/llvm-project · GitHub (with thread_local)
[coroutines] Clang incorrectly caches result of pthread_self() across coroutine suspend-points · Issue #47177 · llvm/llvm-project · GitHub (with pthread_self)
[coroutines] errno address is reused after context switch · Issue #49257 · llvm/llvm-project · GitHub (with errno)

Btw, when looking at "have we completed support for this proposal"
kind of questions and deciding we still have partial support, it would
be greatly appreciated if you could update the cxx_status (or
c_status) page to include rationale as to *why* we've marked something
as only partially supported. This is something we've only started
doing recently, but it helps users to know what level of support there
is (roughly) for a feature and it also helps maintainers understand
what's left to be fixed before claiming full support.

e.g. see P0857R0 or P1073R3 on Clang - C++ Programming Language Status

~Aaron

4

Hi James and Aaron,

Thanks for telling that. I knew the bug before. But the coroutine wouldn’t resume on other thread in our production so that I forgot it. My bad. It should be critical.
I plan changes for ⚙ D115692 [docs] Mark the support for coroutine as unreleased and sent ⚙ D115778 [docs] Give the reason why the support for coroutine is partial based on the suggestion of Aaron. Might you take a look if it satisfied?

Thanks,
Chuanqi

5

This feature is marked as partial, in part, because we don’t know what’s missing. We implemented against a moving target (the C++ Coroutines TS and evolving work towards the final standard feature), and there’s no guarantee we didn’t miss changes along the way. So, the major piece of work we’re missing here is: someone needs to go through the sections on coroutines in the C++ standard, and make sure that all the work listed there has been implemented and is tested. Someone should also go through all the open bugs on coroutines and see which ones are sufficiently important that we should not claim conformance until we address them. That’ll either result in a todo list of the remaining items, or a conclusion that we are in fact done.

Once we’re done, we’ll need to update our feature test macro to match; then we can mark it as done on the status page.

6

Hi Richard,

Thank you for telling the whole process to mark a language feature done. Although it seems hard, I know how to make it now.

And a bad news is that I defined the feature test macro __cpp_impl_coroutine previously when I move the coroutine components out of experimental namespace…
The intention why I introduce this is that I want to make libcxx known if the compiler is new enough to search coroutine components in std namespace.
Do you think we should undefine the macro in another patch?

Thanks,
Chuanqi

7

Hi Richard,

Thank you for telling the whole process to mark a language feature done. Although it seems hard, I know how to make it now.

And a bad news is that I defined the feature test macro __cpp_impl_coroutine previously when I move the coroutine components out of experimental namespace…
The intention why I introduce this is that I want to make libcxx known if the compiler is new enough to search coroutine components in std namespace.
Do you think we should undefine the macro in another patch?

If we’re not confident that we’re done with the implementation, then yes, we should urgently stop defining the macro to the value specified in the standard. If you want to define it as, say, 1 in the interim, to indicate that we have some support but incomplete support, that’s probably OK.

8

I think the current situation might be the latter one. Now the coroutine is used heavily and broadly in production.
And we didn’t see the question for the implementation of semantics. Most of the similar problems come from langugae side.
e.g., why a promise_type couldn’t define return_void and return_value and so on.

So I think it is sufficent to say “we have some support but incomplete support”.

Thanks,
Chuanqi