Created attachment 619836 [details] [diff] [review]
Currently, the single Parser object points to the innermost TreeContext object
and all TreeContext objects point back to the Parser object. This exposes lots
of data to code that doesn't need to see it. It also leads to stupid things
left = tc->parser->new_<BinaryNode>(kind, op, left, right);
in Parser methods, instead of the simpler:
left = new_<BinaryNode>(kind, op, left, right);
This patch changes things so that TreeContext no longer points to Parser.
This matches up nicely with the module structure that bug 750580.
- TreeContext::parser is gone.
- Lots of functions that used to take a TreeContext* now take a Parser*. No
great change there, but the flip-side is that functions that still take a
TreeContext* don't have access to a Parser.
- BytecodeEmitter, a sub-class of TreeContext, does still point to Parser, via
The patch is mostly boring. Some interesting bits that are worth more
- MakePlaceholder and NameNode::create() are passed both a Parser |parser| and
a TreeContext |tc|, because |parser->tc| != |tc| in LeaveFunction().
- ~TreeContext() makes use of a pointer that TreeContext has that points back
to the owning Parser's |tc| field.
- I refactored functionArguments() along the lines suggested by a FIXME
Comment on attachment 619836 [details] [diff] [review]
Review of attachment 619836 [details] [diff] [review]:
@@ +236,5 @@
> + TreeContext **tcToUpdateOnDeath; /* when this TreeContext dies, the TreeContext pointer
> + that this field points to is updated to point to this
> + TreeContext's parent */
This field's name is too operational, can you make this 'parserContext' or something and comment that it is a cursor to the parser's active tc and holds either this tc or one of its descendants?