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 like this: 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. Specific changes: - 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 BytecodeEmitter::parser. The patch is mostly boring. Some interesting bits that are worth more attention: - 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.
Attachment #619836 - Flags: review?(bhackett1024)
Comment on attachment 619836 [details] [diff] [review] patch Review of attachment 619836 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/frontend/TreeContext.h @@ +236,5 @@ > > private: > + 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?
Attachment #619836 - Flags: review?(bhackett1024) → review+
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla15
You need to log in before you can comment on or make changes to this bug.