767202 - Cleave StmtInfo in twain

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Nicholas Nethercote [inactive]]

StmtInfo is used in quite different ways by Parser and BytecodeEmitter.  We can improve things by splitting it in two.

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: (part 1) - Don't export PopStatementBCE().

PopStatementBCE() doesn't need to be exported.

Comment 2

[Nicholas Nethercote [inactive]]

Attachment: (part 2) - Inline EnterFunction().

This patch inlines EnterFunction at its two call sites, in preparation for
the next patch.

Comment 3

[Nicholas Nethercote [inactive]]

Attachment: (part 3) - Split StmtInfo into two.

StmtInfo is used in Parser and BytecodeEmitter.  But Parser never uses the
|update|, |breaks| and |continues| fields, and BytecodeEmitter never uses
the the |blockid| field.

This patch splits StmtInfo into StmtInfoTC and StmtInfoBCE.  StmtInfoBase
holds the common parts.  (I recommend reading the changes to TreeContext.h
first.)  

It uses templates for several functions that operate on both types, e.g.
PushStatement().  (I had to move the definitions of these functions from
TreeContext.cpp to TreeContext-inl.h for the templates to work.)

The patch moves SharedContext::{topStmt,topScopeStmt} into both TreeContext
and BytecodeEmitter, because those fields now have different types in the
two cases.  It also moves SharedContext::blockChain because it's closely
associated with the {topStmt,topScopeStmt} fields;  this change isn't
strictly necessary.  For all three fields this works because they weren't
actually shared by Parser and BytecodeEmitter, even though they were present
in SharedContext.  (I have a long-term goal of making SharedContext as small
as possible.)

Finally, the patch also moves SharedContext::{bodyid,blockidGen} into
TreeContext;  this was necessary because StmtInfoBCE doesn't have the
|blockid| field.

Comment 4

[Nicholas Nethercote [inactive]]

Attachment: (part 4) - Initialize bodyid in TreeContext's constructor.

This patch initializes TreeContext::bodyid in TreeContext's constructor, and
then increments TreeContext::blockidGen in TreeContext::init().  This lets
us avoid numerous explicit calls to GenerateBlockId().

Comment 5

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 635567 [details] [diff] [review]
(part 2) - Inline EnterFunction().

Review of attachment 635567 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/frontend/Parser.cpp
@@ +5453,5 @@
>              return NULL;
>  
> +        gensc.blockidGen = outertc->sc->blockidGen;
> +        if (!GenerateBlockId(&gensc, gensc.bodyid))
> +            return false;

I don't think false is what you want.

Comment 6

[Nicholas Nethercote [inactive]]

> I don't think false is what you want.

Thanks.  It's surprising that GCC doesn't warn about that.  I've fixed several false/NULL mismatches recently, hopefully more than I've introduced :)

Comment 7

[Nicholas Nethercote [inactive]]

Attachment: bundle for gavin

Comment 8

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6c5cfab6e241
https://hg.mozilla.org/integration/mozilla-inbound/rev/74150db1e5bb
https://hg.mozilla.org/integration/mozilla-inbound/rev/51a436ea06a8
https://hg.mozilla.org/integration/mozilla-inbound/rev/33bee15f56d0

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6c5cfab6e241
https://hg.mozilla.org/mozilla-central/rev/74150db1e5bb
https://hg.mozilla.org/mozilla-central/rev/51a436ea06a8
https://hg.mozilla.org/mozilla-central/rev/33bee15f56d0