Closed Bug 750606 Opened 9 years ago Closed 9 years ago

Remove TreeContext::parser

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla15

People

(Reporter: n.nethercote, Assigned: n.nethercote)

References

Details

Attachments

(1 file)

Attached patch patchSplinter 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
  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+
Blocks: 752758
https://hg.mozilla.org/mozilla-central/rev/6ce438dfba3e
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla15
You need to log in before you can comment on or make changes to this bug.