Hermes: An issue about String.prototype.split

[NWU-NISL]

The split() method divides a String into an ordered set of substrings, puts these substrings into an array, and returns the array. The division is done by searching for a pattern; where the pattern is provided as the first parameter in the method's call. In this test case, the search pattern is /$/, which is split from the end. When executing this test case on other engines, (like chakraCore, v8 and spiderMonkey) I get the expected output test, but the output of hermes is test,.

version

b6530ae

Test case 1

var NISLFuzzingFunc =function(){
    var a = 'test'.split(/$/); 
    print(a); 
    print(a.length);
};
NISLFuzzingFunc();

Command

./build/bin/hermes -w testcase.js

Output

test,
2

Expected behavior

test
1

contributor： @Wen Yi

[dulinriley]

Thanks for reporting this, we'll take a look and see if we can fix it

[dulinriley]

From some additional testing: it appears that in v8:

'test'.split(/test$/)
> ["", ""]

'test'.split(/abc/)
> ["test"]

So I think the difference is that Hermes thinks the line terminator is a part of the string "test",
and v8 doesn't. I'll review the spec to see what the right thing to do here is

[neildhar]

Looking at the spec, it does seem that Hermes' behaviour here is correct (or at least I can't find anything to the contrary). Specifically, step 20 states that we should add all the characters after the last match (in this case it's just the empty string) to the end of the returned array.

Both Hermes and v8 agree that there is a match at the end of the line:
v8:

d8> "test".match(/$/)
[""]
d8> "test".search(/$/)
4

Hermes:

>> "test".match(/$/)
[ "", [length]: 1, index: 4, input: "test" ]

In addition, if we change the pattern slightly to /t$/, then both agree that the empty string at the end should be added:
v8:

d8> "test".split(/t$/);
["tes", ""]

Hermes:

>> "test".split(/t$/);
[ "tes", "", [length]: 2 ]

Closing this for now since it doesn't seem like a bug in Hermes.

[ljharb]

If that's the case, can you please file a bug on https://github.com/tc39/ecma262 ? Web compatibility might mean the spec has to change to match what browsers do.

[tmikov]

Reopening.

[neildhar]

I apologise, I made a mistake in my reading of the spec, the other engines are in line with the spec here.

I had incorrectly focused my initial reading on step 24(f)(iii) of the spec for splitting (which is related to matches that don't match any characters). Indeed, it is true that if there were a match with the /$/ at the end of the string, we would expect the the split to produce ["test",""].

However, the issue is actually that according to the spec, /$/ should never match. This is because any match must start at a position _q_ that is less than the size of the string. /$/ could only possibly match at the end of the string and therefore is not a match we can split on.

This also explains why splitting on /t$/ does produce two elements, the match starts before the end of the input string and can therefore be used for splitting.

As an additional illustration, splitting "a,b,c," on /,/ produces ["a", "b", "c", ""] because the match on "," starts before the end of the input string. However, changing this to split on /(?<=,)/ produces ["a,", "b,", "c,"]. That is, we split on every comma except the last one because the last lookbehind could only match at the end of the string.

Thanks for reporting this bug! My apologies for the confusion and we'll get a fix up soon.

[neildhar]

Fixed in c1b5d0195a1b7008a8c74baee5d85a56ffcaec31