Bug 15917

Summary:

Reduce the insanity that is ArrayNode/ElementNode

Product:

WebKit

Reporter:

Eric Seidel (no email) <eric>

Component:

JavaScriptCore

Assignee:

Eric Seidel (no email) <eric>

Status:

RESOLVED INVALID

Severity:

Normal

Priority:

Version:

528+ (Nightly build)

Hardware:

Mac

OS:

OS X 10.4

Attachments:

Description	Flags
improved arrayNode	none
even better, just remove ElementNode	none

Eric Seidel (no email)

Reported 2007-11-09 02:02:00 PST

Reduce the insanity that is ArrayNode/ElementNode Bleh. This patch (partially) cleans up ArrayNode/ElementNode. In the end ElementNode doesn't really need to exist, elision information could be stored on the ArrayNode. Certainly ElementNode doesn't need to have its virtual functions (evaluate or otherwise). This patch does some small cleanup. Including renaming "elision" (meaningless to most of us in this context) to "m_extraTrailingCommas" and "m_extraLeadingCommas" better describing what it actually refers to. The unneeded "opt" special-case variable is also removed. A next-pass would remove ElementNode::evaluate entirely, and just let the ArrayNode walk through the list instead of making it recursive.

Attachments
improved arrayNode (8.43 KB, patch) 2007-11-09 02:02 PST, Eric Seidel (no email)	no flags	Details Formatted Diff Diff
even better, just remove ElementNode (7.86 KB, patch) 2007-11-09 03:47 PST, Eric Seidel (no email)	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Eric Seidel (no email)

Comment 1 2007-11-09 02:02:35 PST

Created attachment 17140 [details] improved arrayNode

Eric Seidel (no email)

Comment 2 2007-11-09 03:47:48 PST

Created attachment 17141 [details] even better, just remove ElementNode I went ahead and went further to the next step. :) This is my first multi-commit patch in git! I'm so proud. :)

Maciej Stachowiak

Comment 3 2007-11-09 07:37:15 PST

Comment on attachment 17141 [details] even better, just remove ElementNode Did you test performance effect of this? I tried to do something similar and it was mysteriously a regression.

Eric Seidel (no email)

Comment 4 2007-11-09 09:23:17 PST

Comment on attachment 17141 [details] even better, just remove ElementNode This one appears broken actually. I'll post another when I'm sure it works and is perf-friendly.

Eric Seidel (no email)

Comment 5 2008-01-13 14:53:22 PST

I've given up on this patch. Closing.

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