[clang] [C++20][Coroutines] Lambda-coroutine with operator new in promise_type (PR #84193)

[Andreas Fertig via cfe-commits]

andreasfertig wrote:

Hello all,

I did more investigation and found another shortcoming. In some cases, my 
initial implementation picked the wrong `this`- type, for example, 
https://github.com/andreasfertig/llvm-project/blob/9c60fec6881cca7e7fed9dca5cf24a0bd1924eaa/clang/test/SemaCXX/coroutine-promise-ctor-lambda.cpp#L45.

I pushed a new branch 
(https://github.com/andreasfertig/llvm-project/tree/fixGH84064take2), which 
also fixes this behavior and requires fewer changes.

Regarding the crash, that was improper testing on my side. All tests let Clang 
crash, even the tests I checked in as part of the PR. The difference is that my 
tests used only `static_assert` to verify that the lambda's size did not 
increase, while the `async_simple::Generator` example _executes_ the code. This 
step leads to code generation, the `static_assert`, I assume, is handled only 
on the constexpr evaluator.

This is where I'm stuck. The call stack for all cases is:

``` 
 #5 0x000105534f38 
clang::CodeGen::CodeGenFunction::EmitCall(clang::CodeGen::CGFunctionInfo 
const&, clang::CodeGen::CGCallee const&, clang::CodeGen::ReturnValueSlot, 
clang::CodeGen::CallArgList const&, llvm::CallBase**, bool, 
clang::SourceLocation) (build/bin/clang-19+0x10262cf38)
 #6 0x0001055e10d8 
clang::CodeGen::CodeGenFunction::EmitCall(clang::QualType, 
clang::CodeGen::CGCallee const&, clang::CallExpr const*, 
clang::CodeGen::ReturnValueSlot, llvm::Value*) (build/bin/clang-19+0x1026d90d8)
 #7 0x0001055e01b4 
clang::CodeGen::CodeGenFunction::EmitCallExpr(clang::CallExpr const*, 
clang::CodeGen::ReturnValueSlot) (build/bin/clang-19+0x1026d81b4)
 #8 0x00010561e5a8 (anonymous 
namespace)::ScalarExprEmitter::VisitCallExpr(clang::CallExpr const*) 
(build/bin/clang-19+0x1027165a8)
 #9 0x00010560edec 
clang::CodeGen::CodeGenFunction::EmitScalarExpr(clang::Expr const*, bool) 
(build/bin/clang-19+0x102706dec)
#10 0x00010554e22c 
clang::CodeGen::CodeGenFunction::EmitCoroutineBody(clang::CoroutineBodyStmt 
const&) (build/bin/clang-19+0x10264622c)
```

The crash is within `CodeGenFunction::EmitCall` at 
https://github.com/llvm/llvm-project/blob/8bb944e0117ab61feecce9de339b11b924fc/clang/lib/CodeGen/CGCall.cpp#L5298

`I->getKnownRValue().getScalarVal()` is `nullptr`, which leads to the crash 
later when `V` is deferenced. I don't understand why `getKnownRValue` is empty. 
It looks to me like I had to mark `this` to be not optimized away. However, I 
could not see any coding doing this for the non-lambda case. Any thoughts?

https://github.com/llvm/llvm-project/pull/84193
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Coroutines] Lambda-coroutine with operator new in promise_type (PR #84193)

[Andreas Fertig via cfe-commits]

andreasfertig wrote:

Hello @ChuanqiXu9,

> Hi Andreas, thanks for looking into this. I am still confused about whether 
> or not your new branch can fix the crash or not.

It's my pleasure! The new branch doesn't fix the crash. If I understand why it 
is crashing, this branch is a better change compared to the previous merged 
code.


> For the question about the crash itself, I don't have any insight though, I 
> feel like this is a defect in the code generator. I didn't understand why 
> mark a variable as referenced or not by the frontend may affect the code 
> generation.

I'm also struggling with that. As far as I could see, the coroutine part does 
nothing special for classes. I suspect that some other coroutine-unrelated part 
does something with the lambda. I also checked whether `I` holds a 
`getKnownLValue`, but that's not the case either.

Do you know who I could ping to get assistance with the CodeGen part? 



https://github.com/llvm/llvm-project/pull/84193
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Coroutines] Lambda-coroutine with operator new in promise_type (PR #84193)

[Andreas Fertig via cfe-commits]

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 
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 &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, /*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 Exp

[clang] [C++20][Coroutines] Lambda-coroutine with operator new in promise_type (PR #84193)

[Andreas Fertig via cfe-commits]

@@ -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);

andreasfertig wrote:

Maybe `IsLambdasOwnThis`?

https://github.com/llvm/llvm-project/pull/84193
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Coroutines] Lambda-coroutine with operator new in promise_type (PR #84193)

[Andreas Fertig via cfe-commits]

@@ -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);

andreasfertig wrote:

It would require invert the logic in the code but state that the `this'- 
pointer that `ActOnCXXThis` handles is the lambda's own `this` and not the one 
from the lambda's surrounding scope. I don't know a more CWG wording term for 
the lambdas `this`- pointer.

https://github.com/llvm/llvm-project/pull/84193
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Coroutines] Lambda-coroutine with operator new in promise_type (PR #84193)

[Andreas Fertig via cfe-commits]

https://github.com/andreasfertig edited 
https://github.com/llvm/llvm-project/pull/84193
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Coroutines] Lambda-coroutine with operator new in promise_type (PR #84193)

[Andreas Fertig via cfe-commits]

https://github.com/andreasfertig edited 
https://github.com/llvm/llvm-project/pull/84193
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Coroutines] Lambda-coroutine with operator new in promise_type (PR #84193)

[Andreas Fertig via cfe-commits]

@@ -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);

andreasfertig wrote:

Sure, have a look at this code (https://cppinsights.io/s/3212c7be)

```
class Test {
  int a;

public:
  Test(int x)
  : a{x}
  {
int other{};
auto l1 = [=] { return a + 2 + other; };
  }
};
```

The lambda `l1` has _two_ things that we refer to as `this`-pointer. It's very 
own, as every non-static member function has. Used to access `other` like 
`this->other` inside the lambdas body. That `this`-pointer isn't captured. 
Hence, no check.

Then the captured `this`-pointer from the enclosing scope of `Test`. In C++ 
Insights called `__this`. While being used it looks like `this->__this->a`.

The check is in place for the latter. For the use case of calling `operator 
new` in the case of a coroutine, we are talking about the first one, which 
isn't captured.

While writing about this, I haven't checked whether a `promise_type` with a 
constructor worked before or does so now. I can check that maybe tomorrow.

https://github.com/llvm/llvm-project/pull/84193
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Coroutines] Lambda-coroutine with operator new in promise_type (PR #84193)

[Andreas Fertig via cfe-commits]

@@ -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
+static void*
+operator new(size_t size,
+ This&,
+ TheRest&&...) noexcept
+{
+return nullptr;
+}
+
+static void operator delete(void*, size_t)
+{
+}
+  };
+};
+
+int main()
+{
+  auto lamb = []() -> Generator {

andreasfertig wrote:

Sure. See my answer above, maybe that helps to understand that we aren't 
talking about a captured `this`, this time.

https://github.com/llvm/llvm-project/pull/84193
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Coroutines] Lambda-coroutine with operator new in promise_type (PR #84193)

[Andreas Fertig via cfe-commits]

@@ -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) {

andreasfertig wrote:

See above, because in this case nothing is captured.

https://github.com/llvm/llvm-project/pull/84193
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Coroutines] Lambda-coroutine with operator new in promise_type (PR #84193)

[Andreas Fertig via cfe-commits]

@@ -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);

andreasfertig wrote:

Sounds good! Thanks!

https://github.com/llvm/llvm-project/pull/84193
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Coroutines] Lambda-coroutine with operator new in promise_type (PR #84193)

[Andreas Fertig via cfe-commits]

https://github.com/andreasfertig edited 
https://github.com/llvm/llvm-project/pull/84193
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Coroutines] Lambda-coroutine with operator new in promise_type (PR #84193)

[Andreas Fertig via cfe-commits]

https://github.com/andreasfertig updated 
https://github.com/llvm/llvm-project/pull/84193

>From 680c99b2d832a5fad617f632df4a750f9c6f1cae Mon Sep 17 00:00:00 2001
From: Andreas Fertig 
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   | 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 592c7871a4a55d..8b4c18140a89e6 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

[clang] [C++20][Coroutines] Lambda-coroutine with operator new in promise_type (PR #84193)

[Andreas Fertig via cfe-commits]

@@ -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, /*SkipLambdaCaptureCheck*/ true);

andreasfertig wrote:

Updated and using `ThisRefersToClosureObject` suggested by @cor3ntin.

https://github.com/llvm/llvm-project/pull/84193
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Coroutines] Lambda-coroutine with operator new in promise_type (PR #84193)

[Andreas Fertig via cfe-commits]

@@ -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
+static void*
+operator new(size_t size,
+ This&,
+ TheRest&&...) noexcept
+{
+return nullptr;
+}
+
+static void operator delete(void*, size_t)
+{
+}
+  };
+};
+
+int main()
+{
+  auto lamb = []() -> Generator {

andreasfertig wrote:

I added tests that capture the `this` pointer. Please, let me know if you have 
new questions or if the previous ones are still open.

https://github.com/llvm/llvm-project/pull/84193
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Coroutines] Lambda-coroutine with operator new in promise_type (PR #84193)

[Andreas Fertig via cfe-commits]

@@ -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) {

andreasfertig wrote:

I assume your question is answered by my answer above: 
https://github.com/llvm/llvm-project/pull/84193#discussion_r1515631511.

https://github.com/llvm/llvm-project/pull/84193
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Coroutines] Lambda-coroutine with operator new in promise_type (PR #84193)

[Andreas Fertig via cfe-commits]

https://github.com/andreasfertig updated 
https://github.com/llvm/llvm-project/pull/84193

>From 9f8a741864b01e89ea17b5ea4c4296da27ef Mon Sep 17 00:00:00 2001
From: Andreas Fertig 
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   | 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 592c7871a4a55d..8b4c18140a89e6 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

[clang] [C++20][Coroutines] Lambda-coroutine with operator new in promise_type (PR #84193)

[Andreas Fertig via cfe-commits]

andreasfertig wrote:

The clang-format check fails due to a change in `clang/docs/ReleaseNotes. rst`, 
which isn't mine. What's the protocol? Shall I fix the format issue, or can my 
change be merged anyway?

https://github.com/llvm/llvm-project/pull/84193
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Coroutines] Lambda-coroutine with operator new in promise_type (PR #84193)

[Andreas Fertig via cfe-commits]

andreasfertig wrote:

> @andreasfertig Will you need us to merge that for you?

@cor3ntin: It looks like I need you, yes. I can only close the PR :-/

https://github.com/llvm/llvm-project/pull/84193
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Coroutines] lambda-coroutine with promise_type ctor. (PR #84519)

[Andreas Fertig via cfe-commits]

https://github.com/andreasfertig created 
https://github.com/llvm/llvm-project/pull/84519

This is a follow-up of #84064. It turned out that a coroutine-lambda with a 
`promise_type` and a user-defined constructor ignores the `this` pointer. Per 
http://eel.is/c++draft/dcl.fct.def.coroutine#4, in such a case, the first 
parameter to the constructor is an lvalue of `*this`.

>From ff5f858b64b3aca3ffd36d75b5f2c96ebf895f4d Mon Sep 17 00:00:00 2001
From: Andreas Fertig 
Date: Fri, 8 Mar 2024 17:49:15 +0100
Subject: [PATCH] [C++20][Coroutines] lambda-coroutine with promise_type ctor.

This is a follow-up of #84064. It turned out that a coroutine-lambda
with a `promise_type` and a user-defined constructor ignores the `this`
pointer. Per http://eel.is/c++draft/dcl.fct.def.coroutine#4, in such a
case, the first parameter to the constructor is an lvalue of `*this`.
---
 clang/lib/Sema/SemaCoroutine.cpp  | 17 -
 .../SemaCXX/coroutine-promise-ctor-lambda.cpp | 71 +++
 ...tine-promise-ctor-static-callop-lambda.cpp | 47 
 3 files changed, 133 insertions(+), 2 deletions(-)
 create mode 100644 clang/test/SemaCXX/coroutine-promise-ctor-lambda.cpp
 create mode 100644 
clang/test/SemaCXX/cxx23-coroutine-promise-ctor-static-callop-lambda.cpp

diff --git a/clang/lib/Sema/SemaCoroutine.cpp b/clang/lib/Sema/SemaCoroutine.cpp
index 301a5ff72a3b2a..79da92083a2be7 100644
--- a/clang/lib/Sema/SemaCoroutine.cpp
+++ b/clang/lib/Sema/SemaCoroutine.cpp
@@ -596,8 +596,21 @@ VarDecl *Sema::buildCoroutinePromise(SourceLocation Loc) {
 
   // Add implicit object parameter.
   if (auto *MD = dyn_cast(FD)) {
-if (MD->isImplicitObjectMemberFunction() && !isLambdaCallOperator(MD)) {
-  ExprResult ThisExpr = ActOnCXXThis(Loc);
+if (MD->isImplicitObjectMemberFunction()) {
+  ExprResult ThisExpr{};
+
+  if (isLambdaCallOperator(MD) && !MD->isStatic()) {
+Qualifiers ThisQuals = MD->getMethodQualifiers();
+CXXRecordDecl *Record = MD->getParent();
+
+Sema::CXXThisScopeRAII ThisScope(*this, Record, ThisQuals,
+ Record != nullptr);
+
+ThisExpr = ActOnCXXThis(Loc, /*ThisRefersToClosureObject=*/true);
+  } else {
+ThisExpr = ActOnCXXThis(Loc);
+  }
+
   if (ThisExpr.isInvalid())
 return nullptr;
   ThisExpr = CreateBuiltinUnaryOp(Loc, UO_Deref, ThisExpr.get());
diff --git a/clang/test/SemaCXX/coroutine-promise-ctor-lambda.cpp 
b/clang/test/SemaCXX/coroutine-promise-ctor-lambda.cpp
new file mode 100644
index 00..92e9a006c3a8d9
--- /dev/null
+++ b/clang/test/SemaCXX/coroutine-promise-ctor-lambda.cpp
@@ -0,0 +1,71 @@
+// 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
+promise_type(This&,
+ TheRest&&...)
+{
+}
+  };
+};
+
+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/cxx23-coroutine-promise-ctor-static-callop-lambda.cpp 
b/clang/test/SemaCXX/cxx23-coroutine-promise-ctor-static-callop-lambda.cpp
new file mode 100644
index 00..0e9e63bce86b87
--- /dev/null
+++ b/clang/test/SemaCXX/cxx23-coroutine-promise-ctor-static-callop-lambda.cpp
@@ -0,0 +1,47 @@
+// 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 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
+promise_type(TheRest&&...)
+{
+}
+  };
+};
+
+
+int main()
+{
+  auto lamb = []() static -> Generator {
+co_return;
+  };
+
+  static_assert(sizeof(decltype

[clang] [C++20][Coroutines] lambda-coroutine with promise_type ctor. (PR #84519)

[Andreas Fertig via cfe-commits]

andreasfertig wrote:

@ChuanqiXu9 here is a dedicated patch to support a `promise_type` with a 
user-provided constructor in a coroutine used with a lambda.

https://github.com/llvm/llvm-project/pull/84519
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Coroutines] lambda-coroutine with promise_type ctor. (PR #84519)

[Andreas Fertig via cfe-commits]

https://github.com/andreasfertig updated 
https://github.com/llvm/llvm-project/pull/84519

>From 3083833f9e87947c7e45c5ed5bd6a983294717c9 Mon Sep 17 00:00:00 2001
From: Andreas Fertig 
Date: Fri, 8 Mar 2024 17:49:15 +0100
Subject: [PATCH] [C++20][Coroutines] lambda-coroutine with promise_type ctor.

This is a follow-up of #84064. It turned out that a coroutine-lambda
with a `promise_type` and a user-defined constructor ignores the `this`
pointer. Per http://eel.is/c++draft/dcl.fct.def.coroutine#4, in such a
case, the first parameter to the constructor is an lvalue of `*this`.
---
 clang/lib/Sema/SemaCoroutine.cpp  | 19 -
 .../SemaCXX/coroutine-promise-ctor-lambda.cpp | 71 +++
 ...tine-promise-ctor-static-callop-lambda.cpp | 47 
 3 files changed, 134 insertions(+), 3 deletions(-)
 create mode 100644 clang/test/SemaCXX/coroutine-promise-ctor-lambda.cpp
 create mode 100644 
clang/test/SemaCXX/cxx23-coroutine-promise-ctor-static-callop-lambda.cpp

diff --git a/clang/lib/Sema/SemaCoroutine.cpp b/clang/lib/Sema/SemaCoroutine.cpp
index 301a5ff72a3b2a..fd8eb81ba55fa4 100644
--- a/clang/lib/Sema/SemaCoroutine.cpp
+++ b/clang/lib/Sema/SemaCoroutine.cpp
@@ -596,8 +596,21 @@ VarDecl *Sema::buildCoroutinePromise(SourceLocation Loc) {
 
   // Add implicit object parameter.
   if (auto *MD = dyn_cast(FD)) {
-if (MD->isImplicitObjectMemberFunction() && !isLambdaCallOperator(MD)) {
-  ExprResult ThisExpr = ActOnCXXThis(Loc);
+if (MD->isImplicitObjectMemberFunction()) {
+  ExprResult ThisExpr{};
+
+  if (isLambdaCallOperator(MD) && !MD->isStatic()) {
+Qualifiers ThisQuals = MD->getMethodQualifiers();
+CXXRecordDecl *Record = MD->getParent();
+
+Sema::CXXThisScopeRAII ThisScope(*this, Record, ThisQuals,
+ /*Enabled=*/Record != nullptr);
+
+ThisExpr = ActOnCXXThis(Loc, /*ThisRefersToClosureObject=*/true);
+  } else {
+ThisExpr = ActOnCXXThis(Loc);
+  }
+
   if (ThisExpr.isInvalid())
 return nullptr;
   ThisExpr = CreateBuiltinUnaryOp(Loc, UO_Deref, ThisExpr.get());
@@ -1387,7 +1400,7 @@ static bool collectPlacementArgs(Sema &S, FunctionDecl 
&FD, SourceLocation Loc,
 CXXRecordDecl *Record = MD->getParent();
 
 Sema::CXXThisScopeRAII ThisScope(S, Record, ThisQuals,
- Record != nullptr);
+ /*Enabled=*/Record != nullptr);
 
 ThisExpr = S.ActOnCXXThis(Loc, /*ThisRefersToClosureObject=*/true);
   } else {
diff --git a/clang/test/SemaCXX/coroutine-promise-ctor-lambda.cpp 
b/clang/test/SemaCXX/coroutine-promise-ctor-lambda.cpp
new file mode 100644
index 00..92e9a006c3a8d9
--- /dev/null
+++ b/clang/test/SemaCXX/coroutine-promise-ctor-lambda.cpp
@@ -0,0 +1,71 @@
+// 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
+promise_type(This&,
+ TheRest&&...)
+{
+}
+  };
+};
+
+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/cxx23-coroutine-promise-ctor-static-callop-lambda.cpp 
b/clang/test/SemaCXX/cxx23-coroutine-promise-ctor-static-callop-lambda.cpp
new file mode 100644
index 00..0e9e63bce86b87
--- /dev/null
+++ b/clang/test/SemaCXX/cxx23-coroutine-promise-ctor-static-callop-lambda.cpp
@@ -0,0 +1,47 @@
+// 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 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 {}
+
+templa

[clang] [C++20][Coroutines] lambda-coroutine with promise_type ctor. (PR #84519)

[Andreas Fertig via cfe-commits]

@@ -596,8 +596,21 @@ VarDecl *Sema::buildCoroutinePromise(SourceLocation Loc) {
 
   // Add implicit object parameter.
   if (auto *MD = dyn_cast(FD)) {
-if (MD->isImplicitObjectMemberFunction() && !isLambdaCallOperator(MD)) {
-  ExprResult ThisExpr = ActOnCXXThis(Loc);
+if (MD->isImplicitObjectMemberFunction()) {
+  ExprResult ThisExpr{};
+
+  if (isLambdaCallOperator(MD) && !MD->isStatic()) {

andreasfertig wrote:

Good catch, thanks!

https://github.com/llvm/llvm-project/pull/84519
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Coroutines] lambda-coroutine with promise_type ctor. (PR #84519)

[Andreas Fertig via cfe-commits]

https://github.com/andreasfertig updated 
https://github.com/llvm/llvm-project/pull/84519

>From cdeb381fb177a3d2991bdec8495cc952a5453712 Mon Sep 17 00:00:00 2001
From: Andreas Fertig 
Date: Fri, 8 Mar 2024 17:49:15 +0100
Subject: [PATCH] [C++20][Coroutines] lambda-coroutine with promise_type ctor.

This is a follow-up of #84064. It turned out that a coroutine-lambda
with a `promise_type` and a user-defined constructor ignores the `this`
pointer. Per http://eel.is/c++draft/dcl.fct.def.coroutine#4, in such a
case, the first parameter to the constructor is an lvalue of `*this`.
---
 clang/docs/ReleaseNotes.rst   |  4 ++
 clang/lib/Sema/SemaCoroutine.cpp  | 19 -
 .../SemaCXX/coroutine-promise-ctor-lambda.cpp | 71 +++
 ...tine-promise-ctor-static-callop-lambda.cpp | 47 
 4 files changed, 138 insertions(+), 3 deletions(-)
 create mode 100644 clang/test/SemaCXX/coroutine-promise-ctor-lambda.cpp
 create mode 100644 
clang/test/SemaCXX/cxx23-coroutine-promise-ctor-static-callop-lambda.cpp

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 690fc7ed271a3d..13c2de49196628 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -95,6 +95,10 @@ C++20 Feature Support
   templates (`P1814R0 `_).
   (#GH54051).
 
+- Clang now treats a lambda-coroutine with a `promise_type` with a constructor
+  or a user-defined `operator new` correctly, passing the lambdas
+  `this`-pointer as the first argument of the parameter list.
+
 C++23 Feature Support
 ^
 
diff --git a/clang/lib/Sema/SemaCoroutine.cpp b/clang/lib/Sema/SemaCoroutine.cpp
index 301a5ff72a3b2a..d9a523fca13d99 100644
--- a/clang/lib/Sema/SemaCoroutine.cpp
+++ b/clang/lib/Sema/SemaCoroutine.cpp
@@ -596,8 +596,21 @@ VarDecl *Sema::buildCoroutinePromise(SourceLocation Loc) {
 
   // Add implicit object parameter.
   if (auto *MD = dyn_cast(FD)) {
-if (MD->isImplicitObjectMemberFunction() && !isLambdaCallOperator(MD)) {
-  ExprResult ThisExpr = ActOnCXXThis(Loc);
+if (MD->isImplicitObjectMemberFunction()) {
+  ExprResult ThisExpr{};
+
+  if (isLambdaCallOperator(MD)) {
+Qualifiers ThisQuals = MD->getMethodQualifiers();
+CXXRecordDecl *Record = MD->getParent();
+
+Sema::CXXThisScopeRAII ThisScope(*this, Record, ThisQuals,
+ /*Enabled=*/Record != nullptr);
+
+ThisExpr = ActOnCXXThis(Loc, /*ThisRefersToClosureObject=*/true);
+  } else {
+ThisExpr = ActOnCXXThis(Loc);
+  }
+
   if (ThisExpr.isInvalid())
 return nullptr;
   ThisExpr = CreateBuiltinUnaryOp(Loc, UO_Deref, ThisExpr.get());
@@ -1387,7 +1400,7 @@ static bool collectPlacementArgs(Sema &S, FunctionDecl 
&FD, SourceLocation Loc,
 CXXRecordDecl *Record = MD->getParent();
 
 Sema::CXXThisScopeRAII ThisScope(S, Record, ThisQuals,
- Record != nullptr);
+ /*Enabled=*/Record != nullptr);
 
 ThisExpr = S.ActOnCXXThis(Loc, /*ThisRefersToClosureObject=*/true);
   } else {
diff --git a/clang/test/SemaCXX/coroutine-promise-ctor-lambda.cpp 
b/clang/test/SemaCXX/coroutine-promise-ctor-lambda.cpp
new file mode 100644
index 00..92e9a006c3a8d9
--- /dev/null
+++ b/clang/test/SemaCXX/coroutine-promise-ctor-lambda.cpp
@@ -0,0 +1,71 @@
+// 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
+promise_type(This&,
+ TheRest&&...)
+{
+}
+  };
+};
+
+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/cxx23-coroutine-promise-ctor-static-callop-lambda.cpp 
b/clang/test/SemaCXX/cxx23-coroutine-promise-ctor-static-callop-lambda.cpp
new file mode 100644
index 00..0e9e63bce86b87
--- /dev/null
+++ b/clang/test/SemaCXX/cxx23-coroutine-promise-ctor-static-callop-lambda.cp

[clang] [C++20][Coroutines] lambda-coroutine with promise_type ctor. (PR #84519)

[Andreas Fertig via cfe-commits]

andreasfertig wrote:

> Can we add a changelog entry for that?

Done!

https://github.com/llvm/llvm-project/pull/84519
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Coroutines] Lambda-coroutine with operator new in promise_type (PR #84193)

[Andreas Fertig via cfe-commits]

andreasfertig wrote:

Interesting! I'll see what I can do.

https://github.com/llvm/llvm-project/pull/84193
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Coroutines] lambda-coroutine with promise_type ctor. (PR #84519)

[Andreas Fertig via cfe-commits]

https://github.com/andreasfertig updated 
https://github.com/llvm/llvm-project/pull/84519

>From cdeb381fb177a3d2991bdec8495cc952a5453712 Mon Sep 17 00:00:00 2001
From: Andreas Fertig 
Date: Fri, 8 Mar 2024 17:49:15 +0100
Subject: [PATCH 1/2] [C++20][Coroutines] lambda-coroutine with promise_type
 ctor.

This is a follow-up of #84064. It turned out that a coroutine-lambda
with a `promise_type` and a user-defined constructor ignores the `this`
pointer. Per http://eel.is/c++draft/dcl.fct.def.coroutine#4, in such a
case, the first parameter to the constructor is an lvalue of `*this`.
---
 clang/docs/ReleaseNotes.rst   |  4 ++
 clang/lib/Sema/SemaCoroutine.cpp  | 19 -
 .../SemaCXX/coroutine-promise-ctor-lambda.cpp | 71 +++
 ...tine-promise-ctor-static-callop-lambda.cpp | 47 
 4 files changed, 138 insertions(+), 3 deletions(-)
 create mode 100644 clang/test/SemaCXX/coroutine-promise-ctor-lambda.cpp
 create mode 100644 
clang/test/SemaCXX/cxx23-coroutine-promise-ctor-static-callop-lambda.cpp

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 690fc7ed271a3d..13c2de49196628 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -95,6 +95,10 @@ C++20 Feature Support
   templates (`P1814R0 `_).
   (#GH54051).
 
+- Clang now treats a lambda-coroutine with a `promise_type` with a constructor
+  or a user-defined `operator new` correctly, passing the lambdas
+  `this`-pointer as the first argument of the parameter list.
+
 C++23 Feature Support
 ^
 
diff --git a/clang/lib/Sema/SemaCoroutine.cpp b/clang/lib/Sema/SemaCoroutine.cpp
index 301a5ff72a3b2a..d9a523fca13d99 100644
--- a/clang/lib/Sema/SemaCoroutine.cpp
+++ b/clang/lib/Sema/SemaCoroutine.cpp
@@ -596,8 +596,21 @@ VarDecl *Sema::buildCoroutinePromise(SourceLocation Loc) {
 
   // Add implicit object parameter.
   if (auto *MD = dyn_cast(FD)) {
-if (MD->isImplicitObjectMemberFunction() && !isLambdaCallOperator(MD)) {
-  ExprResult ThisExpr = ActOnCXXThis(Loc);
+if (MD->isImplicitObjectMemberFunction()) {
+  ExprResult ThisExpr{};
+
+  if (isLambdaCallOperator(MD)) {
+Qualifiers ThisQuals = MD->getMethodQualifiers();
+CXXRecordDecl *Record = MD->getParent();
+
+Sema::CXXThisScopeRAII ThisScope(*this, Record, ThisQuals,
+ /*Enabled=*/Record != nullptr);
+
+ThisExpr = ActOnCXXThis(Loc, /*ThisRefersToClosureObject=*/true);
+  } else {
+ThisExpr = ActOnCXXThis(Loc);
+  }
+
   if (ThisExpr.isInvalid())
 return nullptr;
   ThisExpr = CreateBuiltinUnaryOp(Loc, UO_Deref, ThisExpr.get());
@@ -1387,7 +1400,7 @@ static bool collectPlacementArgs(Sema &S, FunctionDecl 
&FD, SourceLocation Loc,
 CXXRecordDecl *Record = MD->getParent();
 
 Sema::CXXThisScopeRAII ThisScope(S, Record, ThisQuals,
- Record != nullptr);
+ /*Enabled=*/Record != nullptr);
 
 ThisExpr = S.ActOnCXXThis(Loc, /*ThisRefersToClosureObject=*/true);
   } else {
diff --git a/clang/test/SemaCXX/coroutine-promise-ctor-lambda.cpp 
b/clang/test/SemaCXX/coroutine-promise-ctor-lambda.cpp
new file mode 100644
index 00..92e9a006c3a8d9
--- /dev/null
+++ b/clang/test/SemaCXX/coroutine-promise-ctor-lambda.cpp
@@ -0,0 +1,71 @@
+// 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
+promise_type(This&,
+ TheRest&&...)
+{
+}
+  };
+};
+
+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/cxx23-coroutine-promise-ctor-static-callop-lambda.cpp 
b/clang/test/SemaCXX/cxx23-coroutine-promise-ctor-static-callop-lambda.cpp
new file mode 100644
index 00..0e9e63bce86b87
--- /dev/null
+++ b/clang/test/SemaCXX/cxx23-coroutine-promise-ctor-static-callop-lamb

[clang] [C++20][Coroutines] lambda-coroutine with promise_type ctor. (PR #84519)

[Andreas Fertig via cfe-commits]

https://github.com/andreasfertig converted_to_draft 
https://github.com/llvm/llvm-project/pull/84519
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits