[PATCH] D115867: [C++20] [Coroutines] Warning for always_inline coroutine

Tue Jan 25 20:09:21 PST 2022

ChuanqiXu marked 2 inline comments as done.
ChuanqiXu added a comment.

In D115867#3270137 <https://reviews.llvm.org/D115867#3270137>, @Quuxplusone wrote:

> LGTM now, modulo my suggested edits (and the necessary corresponding edits in the test case).
> I don't think I'm really qualified to accept, but as nobody else is looking, and my name is the one with the red next to it, I'll at least change mine to green. I //recommend// getting someone else to look at this before landing it.

Thanks for helping to reword! This idea is inspired by @rjmccall so I guess this one should be good to him. Given the current situation, it might not be easy to find someone else to look at it. I would wait for 1~2 days before landing.

================
Comment at: clang/include/clang/Basic/AttrDocs.td:6140-6142
+Note that a coroutine function wouldn't get inlined in O0 if it is marked with "always_inline".
+In other optimization levels, only the first part of the coroutine - "ramp function"<https://llvm.org/docs/Coroutines.html>`_
+can be guaranteed to be inlined.
----------------
Quuxplusone wrote:
> Why is the `-O0` behavior different for coroutines, versus regular functions? My understanding from this documentation is that `-O0 + always_inline` //will// attempt inlining for regular functions, but will //not// attempt inlining (not even of the ramp) for coroutines. That feels asymmetric and weird.
> So my default assumption is that maybe this documentation is wrong?
> ...ah, I see you mention below that this is a known deficiency. So I think this documentation is OK after you take my suggested edit. (I don't know if it would be appropriate to mention the bug number in this documentation as well, so I'll err on the side of not mentioning it.)
I have made a longer explanation at: https://github.com/llvm/llvm-project/issues/53413 (and I cite this in following comment). I feel it is not good to write something so long in this document. 

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

https://reviews.llvm.org/D115867