[Webkit-unassigned] [Bug 131707] Simple ES6 feature: Number constructor extras

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=131707


Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #238259|review?, commit-queue?      |review-, commit-queue-
               Flag|                            |




--- Comment #14 from Darin Adler <darin at apple.com>  2014-09-19 11:37:07 PST ---
(From update of attachment 238259)
View in context: https://bugs.webkit.org/attachment.cgi?id=238259&action=review

Much better. Getting really close. Still not quite right.

> Source/JavaScriptCore/runtime/NumberConstructor.cpp:157
> +    return JSValue::encode(jsNumber(-9007199254740991LL));

I believe this is the wrong value. I believe that -9007199254740992 is safe.

> Source/JavaScriptCore/runtime/NumberConstructor.cpp:204
> +        double integer = argument.toInteger(exec);

We really don't want to call all the way through toInteger, which can handle a result of any type, just to get the:

    std::isnan(d) ? 0.0 : trunc(d)

Code that is inside the toInteger function. We should rewrite this to not call toInteger. As long as we have sufficient test coverage that should be fine.

> Source/JavaScriptCore/runtime/NumberConstructor.cpp:205
> +        isInteger = std::isfinite(integer) && argument.asDouble() == integer;

I don’t think the isfinite check is needed in the current version of this. If we rewrite to not use toInteger above we may or may not need this.

> Source/JavaScriptCore/runtime/NumberConstructor.cpp:228
> +        double integer = argument.toInteger(exec);
> +        isInteger = argument.asDouble() == integer && std::abs(integer) <= 9007199254740991LL;

I believe this isn’t correct; it's going to incorrectly return false for the maximum negative integer.

But also, my comments above about not using toInteger apply here too.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.