653438 - Incorrect result with number.toExponential(undefined)

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jan de Mooij [:jandem]]

js> (123456789).toExponential()
"1.23456789e+8"
js> (123456789).toExponential(undefined)
"1e+8"

The second value is incorrect per ES5 15.7.4.6.

Comment 1

[Jan de Mooij [:jandem]]

I was going to explain why this is a [good first bug], but it's a one-line change so it's faster to just fix it myself. Patch tomorrow.

Comment 2

[Till Schneidereit [:till]]

Jandem, I'm pretty sure you won't know what that one-line fix is, anymore, but maybe you can find out, again?

Comment 3

[André Bargull [:anba]]

Simply change `if (args.length() == 0)` to `if (!args.hasDefined(0))` in num_toExponential_impl.

For bonus points improve overall spec compliance, e.g. `Infinity.toExponential(Infinity)` should return Infinity instead of throwing a RangeError. And remove dead code in num_toPrecision_impl, the `if (args.length() == 0){...}` branch will never be true.

Comment 4

[Till Schneidereit [:till]]

Thanks, that's very helpful!

André, don't you sometimes get an itch, you know, where you just *know* you want to write a patch? ;)

Comment 5

[Jan de Mooij [:jandem]]

Attachment: Patch

Not exactly "tomorrow", but here's a patch. This also removes the dead branch André noticed in num_toPrecision_impl.

> For bonus points improve overall spec compliance, e.g.
> `Infinity.toExponential(Infinity)` should return Infinity instead of
> throwing a RangeError.

That's not as trivial and I don't really have the time to fix this. But I can file a follow-up bug for it.

Comment 6

[Till Schneidereit [:till]]

Comment on attachment 8340004 [details] [diff] [review]
Patch

Review of attachment 8340004 [details] [diff] [review]:
-----------------------------------------------------------------

Nice.

Not that this particular test really contains anything copyrightable, but I *do* like the idea of having license headers in all test files. Ideally just Public Domain, IMO.

Comment 7

[Jan de Mooij [:jandem]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b58f32ae7646

(In reply to Till Schneidereit [:till] from comment #6)
> Not that this particular test really contains anything copyrightable, but I
> *do* like the idea of having license headers in all test files. Ideally just
> Public Domain, IMO.

We add a ton of jit-tests, a lot of them from fuzzers, and IMO it seems overkill to add license headers to all of them..

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b58f32ae7646