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

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:11110-11113
+def warn_always_inline_coroutine : Warning<
+  "A coroutine marked always_inline might not be inlined properly."
+  >,
+  InGroup<AlwaysInlineCoroutine>;
----------------
ChuanqiXu wrote:
> Quuxplusone wrote:
> > FWIW, this message isn't particularly helpful to the reader. My code "might" not be optimized "properly"? Does that mean it might be mis-optimized, improperly, and thus break in some way at runtime? Or is the compiler just saying that the attribute will be ignored? Or that it //might// be ignored, but it might not? How do I (the programmer) know whether the bad thing will happen?
> > 
> > I think as a programmer what I'd //like// to see here is just `"the '%0' attribute has no effect on coroutines"`. That's very clear, and easy to understand. Does that wording reflect what the compiler actually //does//, though?
> Thanks for suggestion. The actual behavior isn't easy to describe and understand. Since a coroutine would be splitted into pieces. And the original function would be reduced and we would call it 'ramp function' in compiler. And the call to other new functions would be indirect call. The compiler couldn't inline indirect call. But the compiler **might** convert the indirect call into direct call so that they could be inlined.
> 
> In summary, the actual behavior might be described as: "Only the ramp function are guaranteed to be inlined and the other new functions may or may not get inlined". But the term "ramp funciton" is used in compiler only (Some guys in LWG/LEWG know it too). And I believe the term shouldn't leak to other users. So I chose the current description. 
> 
> BTW, I thought the fact that coroutine would be splitted should be transparent to users too. This is the reason why I wrote previous message. But your words make sense. And I couldn't find methods to make it more clear and don't tell the user about coroutine splitting.
IIUC, `"this coroutine will be split into pieces; only the first piece will be inlined"` or simply `"the '%0' attribute applies to only the initial piece of this coroutine"`. Possible synonyms for "piece" include "section", "segment", "chunk". Is there any standardese for "the run of stuff in between two suspension points"?

However, I stand by my initial comment that this message is not helpful to the programmer. It's warning me that something bad will happen, right? Instead of having that bad thing happen, why don't you just make the compiler //ignore the attribute// in this situation?

If your answer is "Because always-inlining the initial piece isn't always bad; maybe the programmer thinks it's //good//, and //wants// it to happen," then this shouldn't be a `warning` at all; it should just be documented in the attribute's documentation. Warnings should be for bad/unintentional things, not for things someone might do on purpose.


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

https://reviews.llvm.org/D115867