Rakudo RT#102994: Add flag indicating HLL init status on CodeRef (CodeRef edition) #806
Conversation
|
This is different from the PR I submitted yesterday. This PR sets the flags on the CodeRef associated with a closure, making it about 100% more useful. See Rakudo PR#1542 |
|
Did you consider storing a special marker pointer in the state var storage instead of allocating extra storage? |
|
I did consider it, but it was unclear to me where to store it. The lexicals are stored on the code ref and in the frame's env as a heap-allocated array of MVMRegister pointers. If can we assume that all the lexicals will be MVMObjects, then MVMCollectableFlags has a few free bits, but since it mostly deals with GC it didn't feel appropriate. Neither did anywhere on the STable. I'm also a little wary of messing with the structure of the memory on something as fundamental as MVMObject, given my lack of knowledge of GC, Spesh, etc, and the types of assumptions they would make. When native state variables are implemented, it would get trickier I imagine. This seemed like a simple place to put it, but also applicable across the board on all different REPR types. |
|
Is this an improper place to store the data or perhaps an inappropriate way to solve the Rakudo bug altogether? If so, please let me know what I can do to improve this so that we can squash this long-standing bug once and for all :) |
Add a value that indicates whether the statevar in the coderef has been assigned a value by the HLL (flag set via extop on HLL side). This change is being made to coincide with a Rakudo development regarding statevar initialization. See [RT#102994](https://rt.perl.org/Public/Bug/Display.html?id=102994)
jstuder-gh
force-pushed
the
state_init_2
branch
from
39655fd
to
4b39e3d
Compare
| @@ -568,6 +568,7 @@ void MVM_frame_invoke(MVMThreadContext *tc, MVMStaticFrame *static_frame, | |||
| MVMRegister *env = static_frame->body.static_env; | |||
| MVMuint8 *flags = static_frame->body.static_env_flags; | |||
| MVMint64 numlex = static_frame->body.num_lexicals; | |||
| MVMCode *cref = (MVMCode *)frame->code_ref; | |||
There was a problem hiding this comment.
Note that frame is rooted below, and that looking this up from the frame every time makes sure that we always get the correct value. If it's lifted out, then it must be also MVMROOTed below.
There was a problem hiding this comment.
Thanks for pointing that out. I had extracted that coderef because it seemed easier to read and write in it's shorter form, but didn't realize that side-effect at the time. I updated the branch to root that variable as well, but if you think it would be better put back as it was I can do so.
The 'cref' is not being rooted alongside the frame so when GC is invoked it may become stale. jnthn++ for pointing this out.
|
I'm closing this PR. I intend to resubmit in the future, but with a rebased and reworked version. |
Add a value that indicates whether the statevar in the coderef has been
assigned a value by the HLL (flag set via extop on HLL side).
This change is being made to coincide with a Rakudo development
regarding statevar initialization.
See RT#102994