ChuanqiXu added a subscriber: rsmith.
ChuanqiXu added a comment.

There are several topics now. Let's discuss them separately to make things 
clear:

(1) Should we add such warning?

> Does the use-case mentioned above make sense (allow C++20, disallow 
> coroutines for code using Clang 17)?

This looks like a coding guide to me. So I feel it may be better to follow what 
other coding guides does. As far as I know, it is rare to insert warnings to 
the compilers for a specific coding guide.

(2) Is it OK to claim coroutines as released?

The reason why we marked it is in the above link. And I think the key question 
here is that how stable coroutines is in production? And as far as I know, all 
the industry users (Facebook, Alibaba, Seastar, I am not sure if Google has 
used it in production) don't feel bad with the current status. Also there a lot 
of relatively smaller projects which have been using coroutines for a long 
time. So personally, I feel it is OK to mark it as completed.

Also my intention to mark it as released is primarily for smaller groups. Since 
they may lack the confidence to use it in serious situations. But I feel (and 
always tell) they can. Since we already tried it for a long time.

(3) What's the practical affects for these bugs?

https://github.com/llvm/llvm-project/issues/57638: it is about **the 
performance  at the O0 level**. So I feel the priority is relatively low. Since 
generally we don't care about the performance within O0. (I know there are some 
against opinions)

https://github.com/llvm/llvm-project/issues/63818: it looks like only related 
to statement expressions. Maybe it is a good idea to emit warnings for using 
coroutines in statement expressions. Maybe we can fix this in the frontend as 
@rsmith said.

https://github.com/llvm/llvm-project/issues/58459: while we need to look into 
the reasons, from the reproducer, it looks like it only appears if the return 
type of the `await_resume()` function is `std::nullptr_t`, which is rare. Also 
I feel this is a frontend issue.

https://github.com/llvm/llvm-project/issues/56301: this is the most severe 
issue among these reports.  This issue may only be possible with a special 
`await_suspend` implementation. And ecosystem of coroutines is that some people 
write coroutine library and other people use these coroutine libraries. Then it 
won't be a problem for the wide users if the library don't use a special 
`await_suspend` implementation. And this is why the existing users of 
coroutines don't meet the issue.

(4) How should we handle the issue?

I have an idea about https://github.com/llvm/llvm-project/issues/56301 and it 
requires us to implement a new alias analysis pass for coroutines.  And I can 
image there will be a lot of works and I don't think I can make it in the 
recent 2~3 months. (This may be the reason that I always delay it...). I can 
try to promote its priority but I can't promise when can I fix it...

For https://github.com/llvm/llvm-project/issues/57638, I feel it is hard to fix 
and the practical affects of it doesn't matter really. So I think we don't need 
to discuss it now.

I don't have insights about https://github.com/llvm/llvm-project/issues/63818 
and https://github.com/llvm/llvm-project/issues/58459. I feel 
https://github.com/llvm/llvm-project/issues/58459 may be an oversight somewhere 
but I didn't look into it.

Hope this helps.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156247/new/

https://reviews.llvm.org/D156247

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to