This bug clears some dead wood from FunctionBox.

This patch removes FunctionBox::inLoop, which is dead.

This patch removes FunctionBox::level, which is dead.

This patch removes FunctionBox::node. Its only use of note is in functionArguments(), but we can pass the relevant ParseNode directly as an argument instead of doing it indirectly via pc->sc->funbox->node. And once that's done, the field can be removed.

(In reply to Nicholas Nethercote [:njn] from comment #3) Hah, nice.

This patch removes FunctionBox::inAnyDynamicScope, which is dead. That allows FunctionBox::funHasExtensibleScope to be removed as well.

This patch just renames some args to make things clearer.

ParseContext::innermostWith is a ParseNode*, but it's only ever used in boolean contexts. This patch changes it to a boolean. I used the name "inWithPC" -- the "PC" part is meant to indicate that it tracks whether we're within a |with| in this ParseContext, but it ignores outer ParseContexts. For example, if we have function |f| within a |with|, |f|'s ParseContext does not set "inWithPC". This non-inheritance is the existing behaviour, but it might be nicer to just make it inherit, because it's (a) easier to think about, (b) it's what'll be needed for FunctionBox (see the follow-up patches) and (c) I think it would be ok for the only other use (in LeaveFunction()), because it would de-optimize more cases but who cares about that.

FunctionBox::inWith only indicates if the function is directly within a |with|, without any intervening functions. (Despite the fact that the field comment says "some enclosing scope is a with-statement or E4X filter-expression".) So in needsImplicitThis() we need to traverse all the ancestor FunctionBoxes as well. This patch changes it so that FunctionBox::inWith is true if it's within any |with| (making that comment more correct). This avoids the need for the FunctionBox::parent traversal.

FunctionBox::parent is now dead and can be removed.

With all those patches in place, FunctionBox::{kids,siblings} and ParseContext::functionList are only needed for the recursivelySetStrictMode() call in Parser::setStrictMode()... hmm.

Thinking some more, I think FunctionBox is being abused. One of its main reasons for existence is to serve as an intermediary -- so that a SharedContext created in the Parser can be re-created for in the BytecodeEmitter. So why does SharedContext::funbox_ exist? Hmm.

Comment on attachment 654503 [details] [diff] [review] (part 6) - Boolify ParseContext::innermostWith. Actually, I've got a better approach coming soon.

This patch renames some variables that are set during parsing/emitting, to clarify their temporary nature. This will help make the following patch clearer.

This patch boolifies |innermostWith|, changing it to |parsingWith|, because a bool is all that's needed. Furthermore, it changes its meaning -- |parsingWith| is now inherited from the parent context, which means its true if we're parsing within a with-statement in this ParseContext *chain*, not just this ParseContext. Consequences of this: - The meaning of FunctionBox::inWith is changed similarly -- it's now true if we're parsing within any with-statement (but in this case, it's true even if there is an intervening |eval| because of the scopeChain test in FunctionBox()). Happily, this change makes the comment for FunctionBox::inWith accurate. - As a result, BCE::needsImplicitThis() no longer needs to follow the whole FunctionBox chain (and this will allow FunctionBox::parent to be removed subsequently). - The DeoptimizeUsesWithin() case will happen more often in LeaveFunction(), but that shouldn't matter. Indeed, the comment on that case says "Make sure to deoptimize lexical dependencies that are polluted by ... any enclosing 'with'" -- the new behaviour matches that comment better than the old behaviour, AFAICT.

This patch removes FunctionBox::parent, which is dead.

Comment on attachment 654877 [details] [diff] [review] (part 8) - Remove FunctionBox::parent. nice

Thanks for the fast reviews, luke! jimb, you're up next.

Comment on attachment 654113 [details] [diff] [review] (part 1) - Remove FunctionBox::inLoop. Review of attachment 654113 [details] [diff] [review]: ----------------------------------------------------------------- Splendid, splendid.

Comment on attachment 654114 [details] [diff] [review] (part 2) - Remove FunctionBox::level. Review of attachment 654114 [details] [diff] [review]: ----------------------------------------------------------------- They didn't stand a chance.

Comment on attachment 654115 [details] [diff] [review] (part 3) - Remove FunctionBox::node. Review of attachment 654115 [details] [diff] [review]: ----------------------------------------------------------------- They're dropping like flies!

Comment on attachment 654465 [details] [diff] [review] (part 4) - Remove FunctionBox::inAnyDynamicScope. Review of attachment 654465 [details] [diff] [review]: ----------------------------------------------------------------- Hurrah.

Try server run of all eight patches looks green: https://tbpl.mozilla.org/?tree=Try&rev=6c8ccb64dfbc

I don't think I've ever written eight patches without having a single modification suggested from reviews. I must be working on things that are too easy :P https://hg.mozilla.org/integration/mozilla-inbound/rev/49c35b696124 https://hg.mozilla.org/integration/mozilla-inbound/rev/fdc5717ca867 https://hg.mozilla.org/integration/mozilla-inbound/rev/52cf0f9db2f9 https://hg.mozilla.org/integration/mozilla-inbound/rev/12cca7e45cce https://hg.mozilla.org/integration/mozilla-inbound/rev/45ac09b4bcb4 https://hg.mozilla.org/integration/mozilla-inbound/rev/878eb7f978b6 https://hg.mozilla.org/integration/mozilla-inbound/rev/660d18ddffcd https://hg.mozilla.org/integration/mozilla-inbound/rev/d178a49c979c

https://hg.mozilla.org/mozilla-central/rev/49c35b696124 https://hg.mozilla.org/mozilla-central/rev/fdc5717ca867 https://hg.mozilla.org/mozilla-central/rev/52cf0f9db2f9 https://hg.mozilla.org/mozilla-central/rev/12cca7e45cce https://hg.mozilla.org/mozilla-central/rev/45ac09b4bcb4 https://hg.mozilla.org/mozilla-central/rev/878eb7f978b6 https://hg.mozilla.org/mozilla-central/rev/660d18ddffcd https://hg.mozilla.org/mozilla-central/rev/d178a49c979c