rsmith added inline comments.
================
Comment at: clang/lib/Sema/SemaCoroutine.cpp:1312
+
+ bool PromiseContainNew = [this, &PromiseType]() -> bool {
+ DeclarationName NewName =
----------------
Nit, should be `PromiseContainsNew`.
================
Comment at: clang/lib/Sema/SemaCoroutine.cpp:1320
+
+ return !R.empty() && !R.isAmbiguous();
+ }();
----------------
I don't see any tests covering the case where the lookup is ambiguous. Eg, you
find `operator new` in two different base classes of the promise type. Please
can you add one?
================
Comment at: clang/lib/Sema/SemaCoroutine.cpp:1337
+
+ LookupAllocationFunction();
----------------
This is no longer correct given [the resolution of
DR2585](https://wiki.edg.com/pub/Wg21telecons2022/Teleconference2022-06-03/cwg_active.html#2585):
the function arguments should only be passed to a class-scope allocation
function, not to a global one.
The logic should be:
1) See if there's any class-scope allocation functions. If so, form an argument
list based on the function's parameters and try looking for an allocation
function. If that succeeds, we're done.
2) Try looking for an allocation function in class scope (if there were any
class-scope allocation functions) or in global scope (if not), passing just the
size argument.
================
Comment at: clang/test/SemaCXX/coroutine-allocs.cpp:22
+
+resumable f1() { // expected-error {{'operator new' provided by
'std::coroutine_traits<resumable>::promise_type' (aka
'resumable::promise_type') is not usable}}
+ co_return;
----------------
Please can you include a complete error including the "with the function
signature of 'f1'" part?
I also wonder if there's some way we can get a candidate list included here.
================
Comment at: clang/test/SemaCXX/coroutine-allocs.cpp:40
+//
+// So the acctual type passed to resumable::promise_type::operator new is
lvalue
+// Allocator. It is allowed to convert a lvalue to a lvalue reference. So the
----------------
Typo 'actual'
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D125517/new/
https://reviews.llvm.org/D125517
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
