Allison Randal wrote:
include/parrot/interpreter.h:
Delete the 'struct' before the "PMC *" declaration in the interpreter
struct.
- struct Interp_Context ctx;
+ struct PMC *ctx; /* current Context */
This lingering bit of old code also appears in src/pmc/lexpad.pmc:
- SET_ATTR_ctx(INTERP, SELF, (struct Parrot_Context *)ctx);
+ SET_ATTR_ctx(INTERP, SELF, (struct PMC *)ctx);
Done.
Keep backward compatibility with the CONTEXT(interp) macro, by having it
return the data struct of the Context PMC. Introduce a new function
'Parrot_pcc_get_current_context' that returns the PMC (it's a clearer
name anyway), and we'll deprecate the old macro for 2.0. (General
principle: when you're working on refactoring old subsystems, think
about how to preserve the old interface until the next deprecation point.)
I tend to disagree with keeping CONTEXT macro backward compatible.
Parrot_Context structure changed anyway. But I can preserve it (and I
actually had it before).
This branch introduces two macros CONTEXT_FIELD and
CURRENT_CONTEXT_FIELD. They're an improvement over the old way of
directly accessing the context struct members, but they're still too
fragile. Since they use the struct field names directly, it ties us to
supporting those field names. Instead, create a series of simple API
functions for accessing context attributes. Once people are switched
over to those, we'll be free to change the internals contexts without
needing further API changes. (General principle: when adding a new
interface, it should be one we plan to keep.)
They are not part of API. Just quick way to keep branch compiling and
working. Almost all API accessors functions already implemented.
'Parrot_cx_get_caller_ctx' is a good idea, hiding those details behind
an interface. But the concurrency scheduler (which is what the 'cx'
prefix means) isn't where it belongs. Same for
'Parrot_cx_get_continuation', 'Parrot_cx_dec_recursion_depth',
'Parrot_cx_set_caller_ctx', 'Parrot_cx_get_namespace',
'Parrot_cx_get_pmc_constant', 'Parrot_cx_get_lex_pad'. Put them in
src/call/context.c and use the prefix 'pcc' instead of 'cx'. Also make
I misunderstood "cx" prefix. Done with renaming.
sure to use "current" in the name of the functions that are fetching or
setting the current namespace, context, or continuation, to make it
clear what the function is doing.
Technically they are not "current". They always belong to passed Context
PMC. I can add "current" version of functions which will fetch current
context from interpreter.
The function currently named 'Parrot_cx_get_context' takes a Context PMC
and returns the data struct for that PMC. We don't want that exposed as
an interface, so eliminate it. (CONTEXT(interp) will continue working
until after 2.0, so we have a migration strategy for anyone who was
relying on the old direct access to the struct. In src/gc/mark_sweep.c,
you can just use interp->ctx for the mark the same as all the other PMC
members of the interpreter struct.)
I actually think about implementing CONTEXT macro in terms of
Parrot_pcc_get_context function for simplifying future migration. It's
easier to put deprecation warning inside function than macro. OTOH may
be Parrot_pcc_get_context_struct cleaner.
Don't create a separate header file include/parrot/context.h, since it
isn't an independent subsystem. Just add the entries to
include/parrot/call.h.
Done.
src/pmc/context.pmc and src/context.c:
When you add a new file, just list the current year in the copyright,
not 2001-2009.
Done.
src/pmc/sub.pmc:
Parrot doesn't use the '//' style of C comments. Also, no generic 'XXX'
comments (in src/jit/i386/jit_defs.c too), either flag it with a ticket
number or don't flag it at all. See the coding standards for more
information.
They left on purpose to track places to fix before merging. Definitely
will be fixed before.
Be careful to review all changes when you get ready to merge the branch
back in. It looks like your branch reverts some trunk changes in
unrelated files (like src/pmc/addrregistry.pmc, src/pmc/class.pmc),
which is a sign that there may have been some glitches in your updates
of the branch from trunk. The cleanest approach is to create a fresh
branch and reapply your changes to it as a patch. That'd be a quick way
to filter out the CONTEXT_FIELD/CURRENT_CONTEXT_FIELD changes too and
all the changes where you have CONTEXT(interp) returning a PMC (faster
than manually undoing them).
I will fully review my changes before merging anyway. On positive note
branch is clean compiling and passing tests.
I recommend doing these changes in two passes, one that adds the Context
PMC, changes the 'ctx' member of the interpreter to a PMC, introduces
accessor functions for the context elements, and makes the minimal set
of changes to get the tests passing. Since it leaves the CONTEXT(interp)
macro still working, you can skip most of the changes currently in the
context_pmc3 branch, so it would be a lightweight branch that could be
merged quickly (you could have it ready and merged in a couple of days
if you start with a stripped down patch from the current branch).
Then go back and do a second branch for the weighter conversion of
direct struct member access to function calls across all the subsystems.
Actually branch contains both steps.
The logic in 'Parrot_alloc_context' should move into the PMC, but you
can save that for a third refactor. The context-related code in
src/sub.c should move into src/call/context.c, but save it for a third
or fourth refactor.
Further code reshuffling and function will be done in next branches. I'm
trying to stay closer to current trunk.
--
Bacek
_______________________________________________
http://lists.parrot.org/mailman/listinfo/parrot-dev
