[clang] 452fac9 - [Frontend] [Coroutines] Emit error when we found incompatible allocation
Chuanqi Xu via cfe-commits
cfe-commits at lists.llvm.org
Mon May 16 19:37:45 PDT 2022
Author: Chuanqi Xu
Date: 2022-05-17T10:36:21+08:00
New Revision: 452fac9534c00290e92819202d445810e33d0444
URL: https://github.com/llvm/llvm-project/commit/452fac9534c00290e92819202d445810e33d0444
DIFF: https://github.com/llvm/llvm-project/commit/452fac9534c00290e92819202d445810e33d0444.diff
LOG: [Frontend] [Coroutines] Emit error when we found incompatible allocation
function in promise_type
According to https://cplusplus.github.io/CWG/issues/2585.html, this
fixes https://github.com/llvm/llvm-project/issues/54881
Simply, the clang tried to found (do lookup and overload resolution. Is
there any better word to use than found?) allocation function in
promise_type and global scope. However, this is not consistent with the
standard. The standard behavior would be that the compiler shouldn't
lookup in global scope in case we lookup the allocation function name in
promise_type. In other words, the program is ill-formed if there is
incompatible allocation function in promise type.
Reviewed By: erichkeane
Differential Revision: https://reviews.llvm.org/D125517
Added:
clang/test/SemaCXX/coroutine-allocs.cpp
Modified:
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaCoroutine.cpp
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 59c42384c4ed7..8058df16bc4c3 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -149,6 +149,9 @@ Bug Fixes
because there is no way to fully qualify the enumerator name, so this
"extension" was unintentional and useless. This fixes
`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.
+ This fixes Issue `Issue 54881 <https://github.com/llvm/llvm-project/issues/54881>`_.
Improvements to Clang's diagnostics
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 6dcf2eafbf210..e03c583c63fe4 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -11245,6 +11245,9 @@ def warn_always_inline_coroutine : Warning<
"this coroutine may be split into pieces; not every piece is guaranteed to be inlined"
>,
InGroup<AlwaysInlineCoroutine>;
+def err_coroutine_unusable_new : Error<
+ "'operator new' provided by %0 is not usable with the function signature of %1"
+>;
} // end of coroutines issue category
let CategoryName = "Documentation Issue" in {
diff --git a/clang/lib/Sema/SemaCoroutine.cpp b/clang/lib/Sema/SemaCoroutine.cpp
index 44a7271910cf1..8709e7bed207a 100644
--- a/clang/lib/Sema/SemaCoroutine.cpp
+++ b/clang/lib/Sema/SemaCoroutine.cpp
@@ -1308,10 +1308,33 @@ bool CoroutineStmtBuilder::makeNewAndDeleteExpr() {
PlacementArgs.push_back(PDRefExpr.get());
}
- S.FindAllocationFunctions(Loc, SourceRange(), /*NewScope*/ Sema::AFS_Class,
- /*DeleteScope*/ Sema::AFS_Both, PromiseType,
- /*isArray*/ false, PassAlignment, PlacementArgs,
- OperatorNew, UnusedResult, /*Diagnose*/ false);
+
+ bool PromiseContainNew = [this, &PromiseType]() -> bool {
+ DeclarationName NewName =
+ S.getASTContext().DeclarationNames.getCXXOperatorName(OO_New);
+ LookupResult R(S, NewName, Loc, Sema::LookupOrdinaryName);
+
+ if (PromiseType->isRecordType())
+ S.LookupQualifiedName(R, PromiseType->getAsCXXRecordDecl());
+
+ return !R.empty() && !R.isAmbiguous();
+ }();
+
+ auto LookupAllocationFunction = [&]() {
+ // [dcl.fct.def.coroutine]p9
+ // The allocation function's name is looked up by searching for it in the
+ // scope of the promise type.
+ // - If any declarations are found, ...
+ // - Otherwise, a search is performed in the global scope.
+ Sema::AllocationFunctionScope NewScope = PromiseContainNew ? Sema::AFS_Class : Sema::AFS_Global;
+ S.FindAllocationFunctions(Loc, SourceRange(),
+ NewScope,
+ /*DeleteScope*/ Sema::AFS_Both, PromiseType,
+ /*isArray*/ false, PassAlignment, PlacementArgs,
+ OperatorNew, UnusedResult, /*Diagnose*/ false);
+ };
+
+ LookupAllocationFunction();
// [dcl.fct.def.coroutine]p9
// If no viable function is found ([over.match.viable]), overload resolution
@@ -1319,22 +1342,7 @@ bool CoroutineStmtBuilder::makeNewAndDeleteExpr() {
// space required as an argument of type std::size_t.
if (!OperatorNew && !PlacementArgs.empty()) {
PlacementArgs.clear();
- S.FindAllocationFunctions(Loc, SourceRange(), /*NewScope*/ Sema::AFS_Class,
- /*DeleteScope*/ Sema::AFS_Both, PromiseType,
- /*isArray*/ false, PassAlignment, PlacementArgs,
- OperatorNew, UnusedResult, /*Diagnose*/ false);
- }
-
- // [dcl.fct.def.coroutine]p9
- // The allocation function's name is looked up by searching for it in the
- // scope of the promise type.
- // - If any declarations are found, ...
- // - Otherwise, a search is performed in the global scope.
- if (!OperatorNew) {
- S.FindAllocationFunctions(Loc, SourceRange(), /*NewScope*/ Sema::AFS_Global,
- /*DeleteScope*/ Sema::AFS_Both, PromiseType,
- /*isArray*/ false, PassAlignment, PlacementArgs,
- OperatorNew, UnusedResult);
+ LookupAllocationFunction();
}
bool IsGlobalOverload =
@@ -1354,8 +1362,12 @@ bool CoroutineStmtBuilder::makeNewAndDeleteExpr() {
OperatorNew, UnusedResult);
}
- if (!OperatorNew)
+ if (!OperatorNew) {
+ if (PromiseContainNew)
+ S.Diag(Loc, diag::err_coroutine_unusable_new) << PromiseType << &FD;
+
return false;
+ }
if (RequiresNoThrowAlloc) {
const auto *FT = OperatorNew->getType()->castAs<FunctionProtoType>();
diff --git a/clang/test/SemaCXX/coroutine-allocs.cpp b/clang/test/SemaCXX/coroutine-allocs.cpp
new file mode 100644
index 0000000000000..4ffb080c9828f
--- /dev/null
+++ b/clang/test/SemaCXX/coroutine-allocs.cpp
@@ -0,0 +1,61 @@
+// RUN: %clang_cc1 %s -std=c++20 -fsyntax-only -verify
+#include "Inputs/std-coroutine.h"
+
+namespace std {
+typedef decltype(sizeof(int)) size_t;
+}
+
+struct Allocator {};
+
+struct resumable {
+ struct promise_type {
+ void *operator new(std::size_t sz, Allocator &);
+
+ resumable get_return_object() { return {}; }
+ auto initial_suspend() { return std::suspend_always(); }
+ auto final_suspend() noexcept { return std::suspend_always(); }
+ void unhandled_exception() {}
+ void return_void(){};
+ };
+};
+
+resumable f1() { // expected-error {{'operator new' provided by 'std::coroutine_traits<resumable>::promise_type' (aka 'resumable::promise_type') is not usable}}
+ co_return;
+}
+
+// NOTE: Although the argument here is a rvalue reference and the corresponding
+// allocation function in resumable::promise_type have lvalue references, it looks
+// the signature of f2 is invalid. But according to [dcl.fct.def.coroutine]p4:
+//
+// In the following, pi is an lvalue of type Pi, where p1 denotes the object
+// parameter and pi+1 denotes the ith non-object function parameter for a
+// non-static member function.
+//
+// And [dcl.fct.def.coroutine]p9.1
+//
+// overload resolution is performed on a function call created by assembling an argument list.
+// The first argument is the amount of space requested, and has type std::size_t.
+// The lvalues p1…pn are the succeeding arguments.
+//
+// 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
+// following one is valid.
+resumable f2(Allocator &&) {
+ co_return;
+}
+
+resumable f3(Allocator &) {
+ co_return;
+}
+
+resumable f4(Allocator) {
+ co_return;
+}
+
+resumable f5(const Allocator) { // expected-error {{operator new' provided by 'std::coroutine_traits<resumable, const Allocator>::promise_type' (aka 'resumable::promise_type') is not usable}}
+ co_return;
+}
+
+resumable f6(const Allocator &) { // expected-error {{operator new' provided by 'std::coroutine_traits<resumable, const Allocator &>::promise_type' (aka 'resumable::promise_type') is not usable}}
+ co_return;
+}
More information about the cfe-commits
mailing list