Last Comment Bug 752758 - Separate TreeContext and BytecodeEmitter
: Separate TreeContext and BytecodeEmitter
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: 750606
Blocks: UntangleFrontEnd 752793
  Show dependency treegraph
Reported: 2012-05-07 17:48 PDT by Nicholas Nethercote [:njn]
Modified: 2012-05-15 06:33 PDT (History)
2 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch (227.37 KB, patch)
2012-05-07 17:48 PDT, Nicholas Nethercote [:njn]
no flags Details | Diff | Splinter Review
rebased patch (227.62 KB, patch)
2012-05-09 19:45 PDT, Nicholas Nethercote [:njn]
bhackett1024: review+
Details | Diff | Splinter Review

Description User image Nicholas Nethercote [:njn] 2012-05-07 17:48:07 PDT
Created attachment 621813 [details] [diff] [review]

BytecodeEmitter ("BCE") is a sub-class of TreeContext ("TC").  This is
because parsing and bytecode generation share some state.  But the amount of
shared state is much smaller than TC;  i.e. lots of Parser-only state is
unnecessarily exposed to BCE.

This patch pulls out this shared state into a new class called

So, when generating bytecode, at the top-level we used to have a single BCE
object, which was also a TC (due to the inheritance);  the Parser pointed to
the TC sub-object.  With this patch, at the top-level we have a TC object
that the Parser points to, a BCE object (which points to the Parser object
but not to the TC object), and a SharedContext object that both the TC object
and the BCE object point to.

SharedContext::flags still has an ugly mixture of state (e.g. some of it is
Parser-only;  some is shared between Parser and BCE);  I plan to split that
up in a follow-up.

Features of the patch:

- TreeContext.h has the key data structure changes.

- Much of it is boring;  lots of converting |tc| to |tc->sc| and |bce| to

- Some places where we used to pass a TC or a BCE we now just pass a
  SharedContext.  Information hiding FTW!

- One nice thing is that InitBehavior, which was a gross hack,
  is no longer necessary.

- Another nice thing is that the TC-to-BCE down-cast in parentBCE() is no
  longer necessary.

- This patch is on top of the patch in bug 750606 but should be pretty
  comprehensible in isolation.
Comment 1 User image Nicholas Nethercote [:njn] 2012-05-09 19:45:30 PDT
Created attachment 622608 [details] [diff] [review]
rebased patch

Rebased patch to account for changes in the precursor patches, all of which have now landed.

bhackett, I promise I won't ask you for a front-end refactoring review for quite some time after this :)
Comment 2 User image Nicholas Nethercote [:njn] 2012-05-14 16:23:11 PDT
Comment 3 User image Ed Morley [:emorley] 2012-05-15 06:33:35 PDT

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