From 35d3b33ba5c9b90443ac985f2521b78f84b611fe Mon Sep 17 00:00:00 2001 From: Andreas Fertig Date: Fri, 8 Mar 2024 17:10:20 +0100 Subject: [PATCH] [C++20][Coroutines] Lambda-coroutine with operator new in promise_type (#84193) Fix #84064 According to http://eel.is/c++draft/dcl.fct.def.coroutine#9 the first parameter for overload resolution of `operator new` is `size_t` followed by the arguments of the coroutine function. http://eel.is/c++draft/dcl.fct.def.coroutine#4 states that the first argument is the lvalue of `*this` if the coroutine is a member function. Before this patch, Clang handled class types correctly but ignored lambdas. This patch adds support for lambda coroutines with a `promise_type` that implements a custom `operator new`. The patch does consider C++23 `static operator()`, which already worked as there is no `this` parameter. --- clang/include/clang/Sema/Sema.h | 12 ++++- clang/lib/Sema/SemaCoroutine.cpp | 18 +++++++- clang/lib/Sema/SemaExprCXX.cpp | 16 +++++-- clang/test/SemaCXX/gh84064-1.cpp | 79 ++++++++++++++++++++++++++++++++ clang/test/SemaCXX/gh84064-2.cpp | 53 +++++++++++++++++++++ 5 files changed, 169 insertions(+), 9 deletions(-) create mode 100644 clang/test/SemaCXX/gh84064-1.cpp create mode 100644 clang/test/SemaCXX/gh84064-2.cpp diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 5b4d67494f2a45..cfc1c3b3494788 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -6752,10 +6752,18 @@ class Sema final { SourceLocation RParenLoc); //// ActOnCXXThis - Parse 'this' pointer. - ExprResult ActOnCXXThis(SourceLocation loc); + /// + /// \param ThisRefersToClosureObject Whether to skip the 'this' check for a + /// lambda because 'this' refers to the closure object. + ExprResult ActOnCXXThis(SourceLocation loc, + bool ThisRefersToClosureObject = false); /// Build a CXXThisExpr and mark it referenced in the current context. - Expr *BuildCXXThisExpr(SourceLocation Loc, QualType Type, bool IsImplicit); + /// + /// \param ThisRefersToClosureObject Whether to skip the 'this' check for a + /// lambda because 'this' refers to the closure object. + Expr *BuildCXXThisExpr(SourceLocation Loc, QualType Type, bool IsImplicit, + bool ThisRefersToClosureObject = false); void MarkThisReferenced(CXXThisExpr *This); /// Try to retrieve the type of the 'this' pointer. diff --git a/clang/lib/Sema/SemaCoroutine.cpp b/clang/lib/Sema/SemaCoroutine.cpp index a969b9383563b2..301a5ff72a3b2a 100644 --- a/clang/lib/Sema/SemaCoroutine.cpp +++ b/clang/lib/Sema/SemaCoroutine.cpp @@ -25,6 +25,7 @@ #include "clang/Sema/Initialization.h" #include "clang/Sema/Overload.h" #include "clang/Sema/ScopeInfo.h" +#include "clang/Sema/Sema.h" #include "clang/Sema/SemaInternal.h" #include "llvm/ADT/SmallSet.h" @@ -1378,8 +1379,21 @@ bool CoroutineStmtBuilder::makeReturnOnAllocFailure() { static bool collectPlacementArgs(Sema &S, FunctionDecl &FD, SourceLocation Loc, SmallVectorImpl &PlacementArgs) { if (auto *MD = dyn_cast(&FD)) { - if (MD->isImplicitObjectMemberFunction() && !isLambdaCallOperator(MD)) { - ExprResult ThisExpr = S.ActOnCXXThis(Loc); + if (MD->isImplicitObjectMemberFunction()) { + ExprResult ThisExpr{}; + + if (isLambdaCallOperator(MD) && !MD->isStatic()) { + Qualifiers ThisQuals = MD->getMethodQualifiers(); + CXXRecordDecl *Record = MD->getParent(); + + Sema::CXXThisScopeRAII ThisScope(S, Record, ThisQuals, + Record != nullptr); + + ThisExpr = S.ActOnCXXThis(Loc, /*ThisRefersToClosureObject=*/true); + } else { + ThisExpr = S.ActOnCXXThis(Loc); + } + if (ThisExpr.isInvalid()) return false; ThisExpr = S.CreateBuiltinUnaryOp(Loc, UO_Deref, ThisExpr.get()); diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp index c34a40fa7c81ac..88e3d9ced044cb 100644 --- a/clang/lib/Sema/SemaExprCXX.cpp +++ b/clang/lib/Sema/SemaExprCXX.cpp @@ -1414,7 +1414,8 @@ bool Sema::CheckCXXThisCapture(SourceLocation Loc, const bool Explicit, return false; } -ExprResult Sema::ActOnCXXThis(SourceLocation Loc) { +ExprResult Sema::ActOnCXXThis(SourceLocation Loc, + bool ThisRefersToClosureObject) { /// C++ 9.3.2: In the body of a non-static member function, the keyword this /// is a non-lvalue expression whose value is the address of the object for /// which the function is called. @@ -1434,13 +1435,18 @@ ExprResult Sema::ActOnCXXThis(SourceLocation Loc) { return Diag(Loc, diag::err_invalid_this_use) << 0; } - return BuildCXXThisExpr(Loc, ThisTy, /*IsImplicit=*/false); + return BuildCXXThisExpr(Loc, ThisTy, /*IsImplicit=*/false, + ThisRefersToClosureObject); } -Expr *Sema::BuildCXXThisExpr(SourceLocation Loc, QualType Type, - bool IsImplicit) { +Expr *Sema::BuildCXXThisExpr(SourceLocation Loc, QualType Type, bool IsImplicit, + bool ThisRefersToClosureObject) { auto *This = CXXThisExpr::Create(Context, Loc, Type, IsImplicit); - MarkThisReferenced(This); + + if (!ThisRefersToClosureObject) { + MarkThisReferenced(This); + } + return This; } diff --git a/clang/test/SemaCXX/gh84064-1.cpp b/clang/test/SemaCXX/gh84064-1.cpp new file mode 100644 index 00000000000000..d9c2738a002b8d --- /dev/null +++ b/clang/test/SemaCXX/gh84064-1.cpp @@ -0,0 +1,79 @@ +// RUN: %clang_cc1 -fsyntax-only -verify -I%S/Inputs -std=c++20 %s + +// expected-no-diagnostics + +#include "std-coroutine.h" + +using size_t = decltype(sizeof(0)); + +struct Generator { + struct promise_type { + int _val{}; + + Generator get_return_object() noexcept + { + return {}; + } + + std::suspend_never initial_suspend() noexcept + { + return {}; + } + + std::suspend_always final_suspend() noexcept + { + return {}; + } + + void return_void() noexcept {} + void unhandled_exception() noexcept {} + + template + static void* + operator new(size_t size, + This&, + TheRest&&...) noexcept + { + return nullptr; + } + + static void operator delete(void*, size_t) + { + } + }; +}; + +struct CapturingThisTest +{ + int x{}; + + void AsPointer() + { + auto lamb = [=,this]() -> Generator { + int y = x; + co_return; + }; + + static_assert(sizeof(decltype(lamb)) == sizeof(void*)); + } + + void AsStarThis() + { + auto lamb = [*this]() -> Generator { + int y = x; + co_return; + }; + + static_assert(sizeof(decltype(lamb)) == sizeof(int)); + } +}; + +int main() +{ + auto lamb = []() -> Generator { + co_return; + }; + + static_assert(sizeof(decltype(lamb)) == 1); +} + diff --git a/clang/test/SemaCXX/gh84064-2.cpp b/clang/test/SemaCXX/gh84064-2.cpp new file mode 100644 index 00000000000000..457de43eab6d9e --- /dev/null +++ b/clang/test/SemaCXX/gh84064-2.cpp @@ -0,0 +1,53 @@ +// RUN: %clang_cc1 -fsyntax-only -verify -I%S/Inputs -std=c++23 %s + +// expected-no-diagnostics + +#include "std-coroutine.h" + +using size_t = decltype(sizeof(0)); + +struct GeneratorStatic { + struct promise_type { + int _val{}; + + GeneratorStatic get_return_object() noexcept + { + return {}; + } + + std::suspend_never initial_suspend() noexcept + { + return {}; + } + + std::suspend_always final_suspend() noexcept + { + return {}; + } + + void return_void() noexcept {} + void unhandled_exception() noexcept {} + + template + static void* + operator new(size_t size, + TheRest&&...) noexcept + { + return nullptr; + } + + static void operator delete(void*, size_t) + { + } + }; +}; + + +int main() +{ + auto lambCpp23 = []() static -> GeneratorStatic { + co_return; + }; + + static_assert(sizeof(decltype(lambCpp23)) == 1); +}