Last Comment Bug 750606 - Remove TreeContext::parser
: Remove TreeContext::parser
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
-- normal (vote)
: mozilla15
Assigned To: Nicholas Nethercote [:njn]
: Jason Orendorff [:jorendorff]
Depends on: 750580
Blocks: UntangleFrontEnd 752758
  Show dependency treegraph
Reported: 2012-04-30 22:26 PDT by Nicholas Nethercote [:njn]
Modified: 2012-05-10 08:05 PDT (History)
2 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch (176.24 KB, patch)
2012-04-30 22:26 PDT, Nicholas Nethercote [:njn]
bhackett1024: review+
Details | Diff | Splinter Review

Description User image Nicholas Nethercote [:njn] 2012-04-30 22:26:47 PDT
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
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

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 1 User image Brian Hackett (:bhackett) 2012-05-03 10:04:17 PDT
Comment on attachment 619836 [details] [diff] [review]

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?
Comment 2 User image Nicholas Nethercote [:njn] 2012-05-09 19:16:30 PDT
Comment 3 User image Ed Morley [:emorley] 2012-05-10 08:05:41 PDT

Note You need to log in before you can comment on or make changes to this bug.