Bug 74509

Summary:

Interpose CodeNode between ScopeNode and ProgramNode et al

Product:

WebKit

Reporter:

Andy Wingo <wingo>

Component:

JavaScriptCore

Assignee:

Andy Wingo <wingo>

Status:

UNCONFIRMED

Severity:

Normal

CC:

barraclough, ggaren, oliver

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

79112

Bug Blocks:

74725

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
fix a loose ends from rebasing	fpizlo: review-

Andy Wingo

Reported 2011-12-14 08:50:45 PST

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.

Attachments
Patch (20.37 KB, patch) 2011-12-14 09:06 PST, Andy Wingo	no flags	Details Formatted Diff Diff
Patch (20.33 KB, patch) 2012-01-18 10:28 PST, Andy Wingo	no flags	Details Formatted Diff Diff
Patch (16.33 KB, patch) 2012-02-28 08:45 PST, Andy Wingo	no flags	Details Formatted Diff Diff
fix a loose ends from rebasing (16.30 KB, patch) 2012-03-07 10:53 PST, Andy Wingo	fpizlo: review-	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Andy Wingo

Comment 1 2011-12-14 08:51:54 PST

Er, "separates the concern of variable binding".

Andy Wingo

Comment 2 2011-12-14 08:52:29 PST

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

Andy Wingo

Comment 3 2011-12-14 09:06:57 PST

Created attachment 119230 [details] Patch

Gavin Barraclough

Comment 4 2012-01-17 10:46:28 PST

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

Comment 5 2012-01-17 12:47:10 PST

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

Comment 6 2012-01-18 10:28:20 PST

Created attachment 122957 [details] Patch

Andy Wingo

Comment 7 2012-02-28 08:39:28 PST

Attaching a rebase on top of bug 79112

Andy Wingo

Comment 8 2012-02-28 08:45:10 PST

Created attachment 129266 [details] Patch

Andy Wingo

Comment 9 2012-03-07 10:33:08 PST

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

Comment 10 2012-03-07 10:53:16 PST

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

Filip Pizlo

Comment 11 2013-10-31 11:36:15 PDT

Comment on attachment 130648 [details] fix a loose ends from rebasing This probably needs a massive rebase.

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