446382 - String.prototype.split broken

Status: RESOLVED FIXED

(Tamarin Graveyard :: Tracing Virtual Machine, defect)

Description

[Brent Baker]

Attachment: Testcase that shows the issue

An issue with skipping the delimiter has been fixed in tamarin-central and needs to be merged into tamarin-tracing

https://bugzilla.mozilla.org/show_bug.cgi?id=407156

trace("raaan".split("aa"));

traces: ["r","","n"]
expected: ["r","an"]

Comment 1

[Ben Newman (:bnewman) (:benjamn)]

Attachment: proposed patch

This patch is logically very similar to the patch for

https://bugzilla.mozilla.org/show_bug.cgi?id=407156

The main difference is that I've used a labeled continue statement to avoid the continue_loop1 flag.  If that's a stylistic sin, let me know.  I think it makes things cleaner.

At first I was tempted to get rid of the single-character delimiter special case, but the following test revealed the benefit of that optimization:

var i, n = 2000, arr = [];
for (i = 0; i < n; i++)
	arr.push('foo');

var str = arr.join(',');

for (i = 0; i < n; i++)
	str.split(',')

Without special case: 4.91s
With special case: 3.88s

Comment 2

[Ben Newman (:bnewman) (:benjamn)]

Attachment: proposed patch, with single-character delimiter optimization

Accidentally submitted the patch without the single-character optimization.  I guess now you can see both versions, for what that's worth :)

Comment 3

[Brent Baker]

Comment on attachment 330661 [details] [diff] [review]
proposed patch, with single-character delimiter optimization

I will only comment on the fact that I tried the patch in the sandbox (build #568) and it passed all of the tests including the testcase in this bug. I will leave it up to Lars to actually review the code changes.

Comment 4

[Dan Smith]

QE: does this work correctly in Tamarin Central?  Please confirm that a test case covers this in the test suite.

Comment 5

[Brent Baker]

This is working in tamarin-central/redux and the testcase (patch #1) is in the source tree @

http://hg.mozilla.org/tamarin-redux/file/tip/test/acceptance/ecma3/String/split_407156.as