[PATCH] D126187: [C++20] [Coroutines] Conform the updates for CWG issue 2585

[Erich Keane via Phabricator via cfe-commits]

erichkeane accepted this revision.
erichkeane added a comment.
This revision is now accepted and ready to land.

I have a change to the release note that I'd like to see improved in SOME way, but I trust you to correct/reword as you wish.  I'm still not particularly happy with the mechanism of the test, but I cannot come up with a way to cause the Semantic Analyzer to cause this.

I think I have a preference to move it to CodeGenCXX anyway however, since we're actually testing the code-generated output (this is not novel, we DO often use CodeGen tests to make sure proper overloads/etc get called).



================
Comment at: clang/docs/ReleaseNotes.rst:152
   `Issue 42372 <https://github.com/llvm/llvm-project/issues/42372>`_.
 - Clang shouldn't lookup allocation function in global scope for coroutines
   in case it found the allocation function name in the promise_type body.
----------------
I realize it isn't part of this patch, but this release note reads awkwardly... How about:



> Clang will now find and emit a call to an allocation function in a promise_type body for coroutines.  Additionally, to implement CWG2585, a coroutine will no longer generate a call to a global allocation function with the signature (std::size_t, p0, ..., pn).
> This fixes Issue `Issue 54881 <https://github.com/llvm/llvm-project/issues/54881>`_.




================
Comment at: clang/lib/Sema/SemaCoroutine.cpp:1293
   // that just takes the requested size.
-
-  FunctionDecl *OperatorNew = nullptr;
-  FunctionDecl *OperatorDelete = nullptr;
-  FunctionDecl *UnusedResult = nullptr;
-  bool PassAlignment = false;
-  SmallVector<Expr *, 1> PlacementArgs;
-
+  //
   // [dcl.fct.def.coroutine]p9
----------------
ChuanqiXu wrote:
> erichkeane wrote:
> > Extra comment line.
> Oh, this is intended. I feel the format looks better with the blank line.
Ah, the deletes made it not clear that this continued into the comment on 1294, so I thought this was a blank before code.  Thanks for the clarification.


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

https://reviews.llvm.org/D126187