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

LGTM. Give it a few days in case @rjmccall have comments.



================
Comment at: clang/test/SemaCXX/coroutine-alloc-4.cpp:49
+    void return_value(int) {}
+    void *operator new(std::size_t, std::align_val_t) noexcept;
+    void *operator new(std::size_t) noexcept;
----------------
ChuanqiXu wrote:
> ychen wrote:
> > ChuanqiXu wrote:
> > > ychen wrote:
> > > > Like this test case, please add additional test cases to check the 
> > > > expected look-up order, one test for each consecutive pair should be 
> > > > good.
> > > > 
> > > > ```
> > > > void* T::operator new  ( std::size_t count, std::align_val_t al, 
> > > > user-defined-args... );
> > > > void* T::operator new  ( std::size_t count, std::align_val_t al);
> > > > void* T::operator new  ( std::size_t count, user-defined-args... );
> > > > void* T::operator new  ( std::size_t count);
> > > > void* operator new  ( std::size_t count, std::align_val_t al );
> > > > ```
> > > > 
> > > > 
> > > Yeah, I'm testing this in CodeGenCoroutines. (It is hard to test the 
> > > selection in Sema Test)
> > Thanks for adding the overload. I think the `noexcept` on operator new is 
> > not necessary. Strictly speaking, it is not a conforming API.
> The noexcept here is necessary. The specs says the allocation function should 
> have a noexcept specifier if get_return_object_on_allocation_failure presents.
I see. I didn't realize that, before P2014, the way class-specific operator 
new/delete are looked up is already different between coroutine and regular 
routine. Basically, with the normal operator new/delete lookup rules, the 
no-throw and potentially-throw allocation functions may overload, so the throw 
function type must have `std::throw_t` parameter; with the coroutine operator 
new/delete lookup rules, the no-throw and potentially-throw allocation 
functions do not overload since `get_return_object_on_allocation_failure` is 
either found or not found, thus the `std::throw_t` parameter is not necessary 
for the potentially-throw allocation function.



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

https://reviews.llvm.org/D133341

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

Reply via email to