SameValue is too strict and used incorrectly for some tests

[littledan]

It looks like the patch at 9ccc663 replaced === in a number of tests with SameValue. While it's nice to tighten things up in general, I'm not sure if the changes are all valid.

For example, in the test built-ins/Array/prototype/indexOf/15.4.4.14-5-9 , -0 is given as the starting index, and 0 is expected as the output (when the answer is found at index 0). By my reading of the spec, -0 would be the right answer (though it's a little ambiguous since the font makes it look like we left float-world and we're in math-land). Would it make sense to change the SameValue test back to ===, or use -0 in this particular test?

[anba]

Great catch. This part should definitely be made more explicit in the specification, because the major implementations show different results for 1 / [true].indexOf(true, -0).

• V8/SpiderMonkey -> Return -Infinity
• Edge/JSC/Nashorn -> Return +Infinity

And I also agree on -Infinity being the correct return value per the current spec for Array.prototype.indexOf. Also 1 / [true].lastIndexOf(true, -0) should return +Infinity instead of -Infinity, because the call to min in Array.prototype.lastIndexOf step 8 converts -0 to +0 (per http://tc39.github.io/ecma262/#sec-algorithm-conventions).

[anba]

There is a similar issue in %TypedArray% ( buffer [ , byteOffset [ , length ] ] ), steps 7-8 and step 17:

7. Let offset be ToInteger(byteOffset).
8. ReturnIfAbrupt(offset).
9. [...]
17. Set O's [[ByteOffset]] internal slot to offset.

That means it's possible to store -0 in [[ByteOffset]]!

// Should return -Infinity per spec!
1 / new Int8Array(new ArrayBuffer(10), -0).byteOffset

[anba]

Created https://bugs.ecmascript.org/show_bug.cgi?id=4544 and https://bugs.ecmascript.org/show_bug.cgi?id=4545 to track these issues.

@bterlson Any recommendation for this issue? I'd prefer resolving bug 4545 instead of replacing SameValue with ===, because this is exactly the kind of cross-browser incompatibility test262 should help to identify.

[bterlson]

Fixing 4545 seems correct. IMO we should make indexOf return 0 not -0 because preserving -0s would require an implementation of the internal Math ops that preserve -0 and refactorings to use them in at least String#indexOf/lastIndexOf and Array#lastIndexOf. I might venture to say that last(?)IndexOf implementations deliberately convert -0 to 0 and Array#indexOf is simply bugged but maybe @allenwb could comment on his intent.

[littledan]

Well V8, for one, implements it per spec now, returning -0 and failing this test!

[bterlson]

@littledan V8/SM gets lastIndex wrong, though: 1 / [true].lastIndexOf(true, -0) ==> -Infinity. So it looks like we don't have anyone conforming completely here. I feel like converting -0 to 0 makes sense for operations returning indexes - do you disagree?

[littledan]

I agree that spec'ing it at 0 would make sense. (But, pedantically, I think, according to the current spec, -0 is a valid return value.)

[bterlson]

I agree that the current spec says -0 is correct for indexOf (not lastIndexOf, though).

[bterlson]

indexOf with a startIndex of -0 should return 0 now. We should get a test for this checked in :)

[leobalter]

The last commit on #485 is covering the byteOffset -0 conversion on TypedArray( buffer, ... ) constructors.