Clang error: lambda-coroutine with operator new in promise_type

[andreasfertig]

Hello,

the following code is accepted by g++ and MSVC, but rejected by Clang:

https://compiler-explorer.com/z/hG58nq7TE (lambda)
https://compiler-explorer.com/z/7WM7eP3h7 (struct and coroutine member function)

Without the This parameter for operator new and passing another parameter, the code is accepted by Clang but rejected by g++ and MSVC:

https://compiler-explorer.com/z/dnEhsj6hP

With my reading of http://eel.is/c++draft/dcl.fct.def.coroutine#4 and http://eel.is/c++draft/dcl.fct.def.coroutine#9.1 Clang is wrong in both cases. It should accept the first and second example but reject the third.

[llvmbot]

@llvm/issue-subscribers-coroutines

Author: Andreas Fertig (andreasfertig)

Hello,

the following code is accepted by g++ and MSVC, but rejected by Clang:

https://compiler-explorer.com/z/hG58nq7TE (lambda)
https://compiler-explorer.com/z/7WM7eP3h7 (struct and coroutine member function)

Without the This parameter for operator new and passing another parameter, the code is accepted by Clang but rejected by g++ and MSVC:

https://compiler-explorer.com/z/dnEhsj6hP

With my reading of http://eel.is/c++draft/dcl.fct.def.coroutine#4 and http://eel.is/c++draft/dcl.fct.def.coroutine#9.1 Clang is wrong in both cases. It should accept the first and second example but reject the third.

[llvmbot]

@llvm/issue-subscribers-clang-frontend

Author: Andreas Fertig (andreasfertig)

Hello,

the following code is accepted by g++ and MSVC, but rejected by Clang:

https://compiler-explorer.com/z/hG58nq7TE (lambda)
https://compiler-explorer.com/z/7WM7eP3h7 (struct and coroutine member function)

Without the This parameter for operator new and passing another parameter, the code is accepted by Clang but rejected by g++ and MSVC:

https://compiler-explorer.com/z/dnEhsj6hP

With my reading of http://eel.is/c++draft/dcl.fct.def.coroutine#4 and http://eel.is/c++draft/dcl.fct.def.coroutine#9.1 Clang is wrong in both cases. It should accept the first and second example but reject the third.

[yuxuanchen1997]

I don't see clang rejecting your example for "struct and coroutine member function"?

[yuxuanchen1997]

I also don't quite get why you expect the third to be rejected. The third has a int parameter so a lvalue of int being passed to a int &. It seems ok?

[zygoloid]

The *this of the lambda itself is supposed to be passed as the first argument, but apparently for lambdas we're treating the function as if it were not a non-static member function.

[ChuanqiXu9]

The *this of the lambda itself is supposed to be passed as the first argument, but apparently for lambdas we're treating the function as if it were not a non-static member function.

If it is the case, I feel like the root cause comes from how we treat lambdas.

CC: @cor3ntin

[cor3ntin]

The *this of the lambda itself is supposed to be passed as the first argument, but apparently for lambdas we're treating the function as if it were not a non-static member function.

If it is the case, I feel like the root cause comes from how we treat lambdas.

CC: @cor3ntin

in collectPlacementArgs (SemaCoroutine.cpp) we single out lambdas, which is probably incorrect.

[andreasfertig]

Hello,

I don't see clang rejecting your example for "struct and coroutine member function"?

Yes! Sorry, I was tired and seemed to have looked at something different. Strike the second example.

I also don't quite get why you expect the third to be rejected. The third has a int parameter so a lvalue of int being passed to a int &. It seems ok?

Yes, but according to http://eel.is/c++draft/dcl.fct.def.coroutine#4, in case of a member function, the first parameter is an lvalue of *this. With C++23s static operator()(), the code would be correct (and is accepted by Clang and g++).

The *this of the lambda itself is supposed to be passed as the first argument, but apparently for lambdas we're treating the function as if it were not a non-static member function.

If it is the case, I feel like the root cause comes from how we treat lambdas.
CC: @cor3ntin

in collectPlacementArgs (SemaCoroutine.cpp) we single out lambdas, which is probably incorrect.

I thought so, too! If I find the time, I'll try to look into it.

Andreas

[ChuanqiXu9]

The *this of the lambda itself is supposed to be passed as the first argument, but apparently for lambdas we're treating the function as if it were not a non-static member function.

If it is the case, I feel like the root cause comes from how we treat lambdas.
CC: @cor3ntin

in collectPlacementArgs (SemaCoroutine.cpp) we single out lambdas, which is probably incorrect.

It turns out that it introduced in 9860622, which is pretty early.

However, after I removed that limitation, I fall into the diagnostic at line 1434, which shows the type of this is Null

llvm-project/clang/lib/Sema/SemaExprCXX.cpp

Lines 1417 to 1438 in 6e27dd4

	ExprResult Sema::ActOnCXXThis(SourceLocation Loc) {
	/// 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.
	QualType ThisTy = getCurrentThisType();
	if (ThisTy.isNull()) {
	DeclContext *DC = getFunctionLevelDeclContext();
	if (const auto *Method = dyn_cast<CXXMethodDecl>(DC);
	Method && Method->isExplicitObjectMemberFunction()) {
	return Diag(Loc, diag::err_invalid_this_use) << 1;
	}
	if (isLambdaCallWithExplicitObjectParameter(CurContext))
	return Diag(Loc, diag::err_invalid_this_use) << 1;
	return Diag(Loc, diag::err_invalid_this_use) << 0;
	}
	return BuildCXXThisExpr(Loc, ThisTy, /*IsImplicit=*/false);
	}

I don't have an idea what happened.

[ChuanqiXu9]

I thought so, too! If I find the time, I'll try to look into it.

Great. If you'd like to take it, you can assign this to yourself. Then we can avoid repeating ourselves.

[cor3ntin]

The *this of the lambda itself is supposed to be passed as the first argument, but apparently for lambdas we're treating the function as if it were not a non-static member function.

If it is the case, I feel like the root cause comes from how we treat lambdas.
CC: @cor3ntin

in collectPlacementArgs (SemaCoroutine.cpp) we single out lambdas, which is probably incorrect.

It turns out that it introduced in 9860622, which is pretty early.

However, after I removed that limitation, I fall into the diagnostic at line 1434, which shows the type of this is Null

llvm-project/clang/lib/Sema/SemaExprCXX.cpp

Lines 1417 to 1438 in 6e27dd4

	ExprResult Sema::ActOnCXXThis(SourceLocation Loc) {
	/// 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.
	QualType ThisTy = getCurrentThisType();
	if (ThisTy.isNull()) {
	DeclContext *DC = getFunctionLevelDeclContext();
	if (const auto *Method = dyn_cast<CXXMethodDecl>(DC);
	Method && Method->isExplicitObjectMemberFunction()) {
	return Diag(Loc, diag::err_invalid_this_use) << 1;
	}
	if (isLambdaCallWithExplicitObjectParameter(CurContext))
	return Diag(Loc, diag::err_invalid_this_use) << 1;
	return Diag(Loc, diag::err_invalid_this_use) << 0;
	}
	return BuildCXXThisExpr(Loc, ThisTy, /*IsImplicit=*/false);
	}

I don't have an idea what happened.

We probably need a CXXThisScopeRAII so that this refers` to the lambda

[ChuanqiXu9]

The *this of the lambda itself is supposed to be passed as the first argument, but apparently for lambdas we're treating the function as if it were not a non-static member function.

If it is the case, I feel like the root cause comes from how we treat lambdas.
CC: @cor3ntin

in collectPlacementArgs (SemaCoroutine.cpp) we single out lambdas, which is probably incorrect.

It turns out that it introduced in 9860622, which is pretty early.
However, after I removed that limitation, I fall into the diagnostic at line 1434, which shows the type of this is Null

llvm-project/clang/lib/Sema/SemaExprCXX.cpp

Lines 1417 to 1438 in 6e27dd4

	ExprResult Sema::ActOnCXXThis(SourceLocation Loc) {
	/// 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.
	QualType ThisTy = getCurrentThisType();
	if (ThisTy.isNull()) {
	DeclContext *DC = getFunctionLevelDeclContext();
	if (const auto *Method = dyn_cast<CXXMethodDecl>(DC);
	Method && Method->isExplicitObjectMemberFunction()) {
	return Diag(Loc, diag::err_invalid_this_use) << 1;
	}
	if (isLambdaCallWithExplicitObjectParameter(CurContext))
	return Diag(Loc, diag::err_invalid_this_use) << 1;
	return Diag(Loc, diag::err_invalid_this_use) << 0;
	}
	return BuildCXXThisExpr(Loc, ThisTy, /*IsImplicit=*/false);
	}

I don't have an idea what happened.

We probably need a CXXThisScopeRAII so that this refers` to the lambda

hmm it doesn't work if I put that in front of ActOnCXXThis in collectPlacementArgs or in front of collectPlacementArgs.

Maybe I need to investigate more. Or if you have time, I guess you can look into this directly.

[andreasfertig]

Maybe I need to investigate more. Or if you have time, I guess you can look into this directly.

I don't know how to assign this issue to myself, but I'm on it. I have a version that works. I'm running the test suit on it to see whether it breaks other stuff.

Andreas