https://github.com/andreasfertig created https://github.com/llvm/llvm-project/pull/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. >From 8f9336e6c534b4ba71e3e490ebe95527529dc398 Mon Sep 17 00:00:00 2001 From: Andreas Fertig <a...@cppinsights.io> Date: Wed, 6 Mar 2024 14:20:00 +0100 Subject: [PATCH] [C++20][Coroutines] Lambda-coroutine with operator new in promise_type 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 | 15 ++++++--- clang/test/SemaCXX/gh84064-1.cpp | 54 ++++++++++++++++++++++++++++++++ clang/test/SemaCXX/gh84064-2.cpp | 53 +++++++++++++++++++++++++++++++ 5 files changed, 143 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 25c4c58ad4ae43..f6d81a15487a3e 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -6898,10 +6898,18 @@ class Sema final { BinaryOperatorKind Operator); //// ActOnCXXThis - Parse 'this' pointer. - ExprResult ActOnCXXThis(SourceLocation loc); + /// + /// \param SkipLambdaCaptureCheck Whether to skip the 'this' check for a + /// lambda because 'this' is the lambda's 'this'-pointer. + ExprResult ActOnCXXThis(SourceLocation loc, + bool SkipLambdaCaptureCheck = false); /// Build a CXXThisExpr and mark it referenced in the current context. - Expr *BuildCXXThisExpr(SourceLocation Loc, QualType Type, bool IsImplicit); + /// + /// \param SkipLambdaCaptureCheck Whether to skip the 'this' check for a + /// lambda because 'this' is the lambda's 'this'-pointer. + Expr *BuildCXXThisExpr(SourceLocation Loc, QualType Type, bool IsImplicit, + bool SkipLambdaCaptureCheck = 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..d5655309d21f28 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<Expr *> &PlacementArgs) { if (auto *MD = dyn_cast<CXXMethodDecl>(&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, /*SkipLambdaCaptureCheck*/ 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..499c4943c23b01 100644 --- a/clang/lib/Sema/SemaExprCXX.cpp +++ b/clang/lib/Sema/SemaExprCXX.cpp @@ -1414,7 +1414,7 @@ bool Sema::CheckCXXThisCapture(SourceLocation Loc, const bool Explicit, return false; } -ExprResult Sema::ActOnCXXThis(SourceLocation Loc) { +ExprResult Sema::ActOnCXXThis(SourceLocation Loc, bool SkipLambdaCaptureCheck) { /// 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 +1434,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, + SkipLambdaCaptureCheck); } -Expr *Sema::BuildCXXThisExpr(SourceLocation Loc, QualType Type, - bool IsImplicit) { +Expr *Sema::BuildCXXThisExpr(SourceLocation Loc, QualType Type, bool IsImplicit, + bool SkipLambdaCaptureCheck) { auto *This = CXXThisExpr::Create(Context, Loc, Type, IsImplicit); - MarkThisReferenced(This); + + if (!SkipLambdaCaptureCheck) { + 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..dc7c475041094a --- /dev/null +++ b/clang/test/SemaCXX/gh84064-1.cpp @@ -0,0 +1,54 @@ +// 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<typename This, typename... TheRest> + static void* + operator new(size_t size, + This&, + TheRest&&...) noexcept + { + return nullptr; + } + + static void operator delete(void*, size_t) + { + } + }; +}; + +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<typename... TheRest> + 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); +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
