On Tuesday 26 December 2006 14:35, Bob Rogers wrote: > AFAICS, the old code failed because Parrot_switch_to_cs (which is what > normally sets ctx->contants in a sub call) is not given a chance to run > in this situation. Parrot_loadbc sets interp->code without doing any of > the other Parrot_switch_to_cs bookeeping. Sub:invoke subsequently finds > that sub->seg is already installed properly in interp->code, so it skips > calling Parrot_switch_to_cs, leaving ctx->contants uninitialized.
That makes sense. However, I'm curious whether Parrot_loadbc() does the right thing. > Dropping the interp->code assignment in Parrot_loadbc sorta works > (after telling RetContinuation:invoke not to panic if no returning code > segment), but causes other failures. Replacing the assignment with a > Parrot_switch_to_cs call fails immediately; this might be made to work, > but it would be setting the constants in the old context, just as > Nikolay's patch (and Parrot_call_sub) do; this seems wrong. I don't understand why; would you consider writing up some rough documentation on how contexts and interpreters and code segments currently work? Having to dig in the implementation and guess by reading the code isn't getting me very far. > The attached patch changes Sub:invoke to call Parrot_switch_to_cs > unconditionally. This seems to work without breaking anything else, and > also allows all these other extracurricular assignments to > ctx->constants to go away. But there may be something cleaner still > (and the code works now), so I too will wait to hear from @other (I > suspect they are mostly on vacation -- ;-). It's shorter than my attempts (which broke some threading tests, which seems to indicate what you suggested about old contexts). -- c
