Hi
(apologies to Jason and Nathan for two copies - I sent from the wrong email and
omitted patches@)
This is a description of how the revised handling of coroutine proxy variables
is supposed to work, in the case of methods and lambdas. Given that this came
up again recently, perhaps I might put some version of this description as a
block comment before the rewrite code.
There is a bug-fix patch for PR96517, the long preamble is to put context (and,
hopefully, thus identify any more assumptions that I might have missed).
====
NOTE: It is the user’s responsibility to ensure that ‘this’ and any lambda
object persist for the duration of the coroutine.
IMO this is probably not the best design decision ever made by the committee
since it is very easy to make innocent-looking but broken coroutine code with
lambdas. [aside: I think a more user-friendly decision would have been to
extend the lifetime of temporaries in statements until *all* the code before
the semi-colon is executed (i.e. including deferred code in a coroutine)]
anyway…
consider this illustrative-only case (where ‘coroutine’ is a suitable
implementation of the customisation points).
I have elided some of the implementation variables, they are handled the same
way as the ones shown and this is already long enough.
==== code
struct test {
int a;
void foo() {
int c;
auto lam = [this, c](int b) -> coroutine {
co_return a + b + c;
};
}
};
Before the coroutine transformation process:
closure object:
{
int __c
test* __this
typedef closure_type
}
inline coroutine test::foo::…..::operator() (closure_type *__closure, int b)
{
BIND_EXPR
VARS {
const int c; // lambda proxy var => __closure->__c
test* const this; // lambda proxy var => __clousre->__this
}
BODY {
<STATEMENT_LIST>{
const int c;
test* const this;
co_return this->a + b + c;
}
}
}
Now because of the design decision mentioned above, we do not copy the closure
contents, only the pointer.
=== after transformation
our coroutine frame will look like this…
test::foo::……..Frame
{
void (*)test::foo::....Frame*)_Coro_resume_fn)(test::foo::…..Frame*)
void (*)test::foo::...Frame*)_Coro_destroy_fn)(est::foo::…...Frame*)
coroutine::promise_type _Coro_promise
...
closure_type* const __closure // parm copy
int b // parm copy, no need for mangling.
...
std::__n4861::suspend_always Is_1_1 // local var frame entries are mangled
with their scope info,
std::__n4861::suspend_never Fs_1_3
}
our rewritten lambda looks like this:
BIND_EXPR
VARS {
void (*_Coro_resume_fn)(test::foo::._anon_4::.....Frame*); // coroutine proxy
var for resume fn
void (*_Coro_destroy_fn)(test::foo::._anon_4::.....Frame*); // ...
coroutine::promise_type _Coro_promise; // for promise DVE =>
Frame->_Coro_promise
...
closure_type* const __closure; // coro proxy var for closure parm DVE =>
Frame->__closure
int b; // coro proxy var for b parm
….
}
BODY {
{
void (*_Coro_resume_fn)(test::foo::._anon_4::.....Frame*);
void (*_Coro_destroy_fn)(test::foo::._anon_4::.....Frame*);
coroutine::promise_type _Coro_promise;
...
const test::foo::._anon_4* const __closure;
int b;
...
try
{
co_await initial suspend expr.
BIND_EXPR (original lambda body)
VARS {
const int c; // lambda proxy var; DVE => __closure->__c
test* const this; // lambda proxy var DVE => __closure->__this
}
BODY {
{
const int c;
test* const this;
co_return this->a + b + c;
}
}
}
catch(...)
cleanup_stmt
…
}
c-finally
...
final.suspend:;
…
}
}
We have a predicate for is_capture_proxy() for lambdas
So .. in this expression:
co_return this->a + b + c;
If we ask “is_capture_proxy” for ’this’ or ‘c’ we should answer “yes” - these
vars are referred to via the closure object and, if the constraints of the
standard are met (lifetime of the closure object and the captured this), then
it should be just as if they were used in the body of a regular lambda.
The closure pointer itself is a coro proxy var (for the parm)
b is a coro proxy var (for the parm)
Note that the parm proxies are VAR decls not PARMs (we need to make a copy,
because the original parms are used in the ramp, and obviously they are *not*
parms of the helper function).
so when all the DECL_VALUE_EXPRs are expanded we would get:
Frame->__closure->__this->a + Frame->b + Frame->_closure->__c;
Whilst the initial intention of this was to make debug work (since GCC’s debug
generation already knows about DVEs) it turns out that it greatly simplifies
the code, since we are dealing with the original variables instead of replacing
them with a component ref.
PR 96517 fires because we answer “NO” for is_capture_proxy in the rewritten
code, because we do not (correctly) answer “yes” for LAMBDA_TYPE_P for the
rewritten function.
What we need to ask is “was the original function a lambda type?”.
This is what the patch does.
tested on x86_64-darwin,
OK for master and affected backports?
thanks
Iain
===== commit log
When we query is_capture_proxy(), and the scope of the var is one of
the two coroutine helpers, we need to look for the scope information
that pertains to the original function (represented by the ramp now).
We can look up the ramp function from either helper (in practice, the
only caller would be the actor) and if that lookup returns NULL, it
means that the coroutine component is the ramp already and handled by
the usual code path.
Signed-off-by: Iain Sandoe <[email protected]>
gcc/cp/ChangeLog:
PR c++/96517
* lambda.c (is_capture_proxy): When the scope of the var to
be tested is a coroutine helper, lookup the scope information
from the parent (ramp) function.
---
gcc/cp/lambda.c | 6 +++++-
diff --git a/gcc/cp/lambda.c b/gcc/cp/lambda.c
index 2e9d38bbe83..c1556480e22 100644
--- a/gcc/cp/lambda.c
+++ b/gcc/cp/lambda.c
@@ -244,7 +244,11 @@ is_capture_proxy (tree decl)
&& !(DECL_ARTIFICIAL (decl)
&& DECL_LANG_SPECIFIC (decl)
&& DECL_OMP_PRIVATIZED_MEMBER (decl))
- && LAMBDA_FUNCTION_P (DECL_CONTEXT (decl)));
+ && (LAMBDA_FUNCTION_P (DECL_CONTEXT (decl))
+ || (DECL_DECLARES_FUNCTION_P (DECL_CONTEXT (decl))
+ && DECL_COROUTINE_P (DECL_CONTEXT (decl))
+ && DECL_RAMP_FN (DECL_CONTEXT (decl))
+ && LAMBDA_FUNCTION_P (DECL_RAMP_FN (DECL_CONTEXT (decl))))));
}
/* Returns true iff DECL is a capture proxy for a normal capture
--
