Bug 74509 – Interpose CodeNode between ScopeNode and ProgramNode et al

• Status: UNCONFIRMED
• Component: JavaScriptCore

[Andy Wingo]

Currently in JS, variables are bound at functions, and in eval code, and in toplevel program code.  Harmony / ES6 introduces a new way to bind variables, in block scopes.  See bug 31813 for more.

The patch to be attached separates the concern of function binding with the other concerns of functions, eval code, and eval code (namely, holding a codeblock).  It does so by introducing a new class, CodeNode, between ScopeNode and ProgramNode.

A future patch will add BlockScopeNode whose superclass is ScopeNode.

[Andy Wingo]

Er, "separates the concern of variable binding".

[Andy Wingo]

Also, "eval code, and program code".  Sorry again for the noise.

[Andy Wingo]

Created attachment 119230 [details]
Patch

[Gavin Barraclough]

I think this patch may be going in the wrong direction.  If the purpose of this patch is to allow BlockScopeNodes that subclass ScopeNode, and the effect of this patch is to move the needsActivation & capture state out into a subclass, it appears that you are predicating this change on early tear-off onto a scope node? – but I think we may really want lazy tear-off using an activation.

You really need to address my comments in https://bugs.webkit.org/show_bug.cgi?id=74633 to further elucidate on your design before proceeding.

[Andy Wingo]

Thanks for the note, Gavin.  I have been thinking about that comment for a little while now, and will address it tomorrow.  Briefly now though, I'm not certain that this change (and a subsequent BlockScopeNode) necessitates "early tear-off".  This patch would seem to work equally well for both strategies.  But, more tomorrow.

Happy hacking :)

[Andy Wingo]

Created attachment 122957 [details]
Patch

[Andy Wingo]

Attaching a rebase on top of bug 79112

[Andy Wingo]

Created attachment 129266 [details]
Patch

[Andy Wingo]

Performance-neutral on master: 5.997s in my parse test[0] vs 5.983s for trunk.

[0] bug 79776 comment 12

I have some pending patches for lazy tear-off that haven't quite congealed yet.  This is a step in that direction, but if it's not clear to you, dear reviewer, then it's OK to hold off on reviewing this one.

[Andy Wingo]

Created attachment 130648 [details]
fix a loose ends from rebasing

[Filip Pizlo]

Comment on attachment 130648 [details]
fix a loose ends from rebasing

This probably needs a massive rebase.