[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 #238436|review?, commit-queue?      |review+, commit-queue-
               Flag|                            |




--- Comment #17 from Darin Adler <darin at apple.com>  2014-09-21 17:08:26 PST ---
(From update of attachment 238436)
View in context: https://bugs.webkit.org/attachment.cgi?id=238436&action=review

OK to land like this, but I think I spotted one more problem.

> Source/JavaScriptCore/runtime/NumberConstructor.cpp:115
> +    return JSValue::encode(jsNumber(std::numeric_limits<double>::epsilon()));

Should use jsDoubleNumber instead of jsNumber for better efficiency.

> Source/JavaScriptCore/runtime/NumberConstructor.cpp:127
>      return JSValue::encode(jsNumber(-std::numeric_limits<double>::infinity()));

Existing code, but should use jsDoubleNumber instead of jsNumber for better efficiency.

> Source/JavaScriptCore/runtime/NumberConstructor.cpp:133
>      return JSValue::encode(jsNumber(std::numeric_limits<double>::infinity()));

Existing code, but should use jsDoubleNumber instead of jsNumber for better efficiency.

> Source/JavaScriptCore/runtime/NumberConstructor.cpp:139
>      return JSValue::encode(jsNumber(1.7976931348623157E+308));

Existing code, but should use jsDoubleNumber instead of jsNumber for better efficiency.

> Source/JavaScriptCore/runtime/NumberConstructor.cpp:145
>      return JSValue::encode(jsNumber(5E-324));

Existing code, but should use jsDoubleNumber instead of jsNumber for better efficiency.

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

Should be jsDoubleNumber(9007199254740991.0) for better efficiency; avoids converting from long long to double and also some other checking.

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

Should be jsDoubleNumber(-9007199254740991.0) for better efficiency; avoids converting from long long to double and also some other checking.

> Source/JavaScriptCore/runtime/NumberConstructor.cpp:205
> +        isInteger = std::isfinite(number) && trunc(number) == number;

I think this might return true for -0 and I believe it should return false.

> Source/JavaScriptCore/runtime/NumberConstructor.cpp:228
> +        isInteger = trunc(number) == number && std::abs(number) <= 9007199254740991LL;

Should be 9007199254740991.0, not 9007199254740991LL.

I think this might return true for -0 and I believe it should return false.

> LayoutTests/js/script-tests/number-constructor.js:4
> +shouldBeTrue('Number.isFinite(0)');

Need to test negative zero.

> LayoutTests/js/script-tests/number-constructor.js:27
> +shouldBeTrue('Number.isInteger(0)');

Need to test negative zero.

> LayoutTests/js/script-tests/number-constructor.js:51
> +shouldBeFalse('Number.isNaN(0)');

Need to test negative zero.

> LayoutTests/js/script-tests/number-constructor.js:73
> +shouldBeTrue('Number.isSafeInteger(0)');

Need to test negative zero.

> LayoutTests/js/script-tests/number-constructor.js:100
> +shouldBe('Number.parseFloat("0")', '0');

Need to test negative zero.

> LayoutTests/js/script-tests/number-constructor.js:126
> +shouldBe('Number.parseInt("0")', '0');

Need to test negative zero.

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