Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Cleave StmtInfo in twain

RESOLVED FIXED in mozilla16

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: njn, Assigned: njn)

Tracking

unspecified
mozilla16
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [js:t])

Attachments

(5 attachments)

(Assignee)

Description

5 years ago
StmtInfo is used in quite different ways by Parser and BytecodeEmitter.  We can improve things by splitting it in two.
(Assignee)

Comment 1

5 years ago
Created attachment 635565 [details] [diff] [review]
(part 1) - Don't export PopStatementBCE().

PopStatementBCE() doesn't need to be exported.
Attachment #635565 - Flags: review?(bhackett1024)
(Assignee)

Comment 2

5 years ago
Created attachment 635567 [details] [diff] [review]
(part 2) - Inline EnterFunction().

This patch inlines EnterFunction at its two call sites, in preparation for
the next patch.
Attachment #635567 - Flags: review?(bhackett1024)
(Assignee)

Comment 3

5 years ago
Created attachment 635568 [details] [diff] [review]
(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.
Attachment #635568 - Flags: review?(bhackett1024)
(Assignee)

Comment 4

5 years ago
Created attachment 635569 [details] [diff] [review]
(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().
Attachment #635569 - Flags: review?(bhackett1024)
Attachment #635565 - Flags: review?(bhackett1024) → review+
Attachment #635567 - Flags: review?(bhackett1024) → review+
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.
(Assignee)

Comment 6

5 years ago
> 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 :)
Attachment #635568 - Flags: review?(bhackett1024) → review+
Attachment #635569 - Flags: review?(bhackett1024) → review+
(Assignee)

Comment 7

5 years ago
Created attachment 637738 [details]
bundle for gavin
Attachment #637738 - Attachment mime type: text/plain → application/octet-stream
(Assignee)

Comment 8

5 years ago
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
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
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla16
You need to log in before you can comment on or make changes to this bug.