[webkit-reviews] review denied: [Bug 131182] Fix JSC Debug Regressions on Windows : [Attachment 229374] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Apr 16 10:11:07 PDT 2014


Darin Adler <darin at apple.com> has denied peavo at outlook.com's request for
review:
Bug 131182: Fix JSC Debug Regressions on Windows
https://bugs.webkit.org/show_bug.cgi?id=131182

Attachment 229374: Patch
https://bugs.webkit.org/attachment.cgi?id=229374&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=229374&action=review


> Source/JavaScriptCore/runtime/JSCJSValueInlines.h:508
>      if (number != number)
>	   return false;
> +    if (!std::isfinite(number))
> +	   return false;

Adding this extra check is going to make the code slower. Do we need it on all
platforms, or just on Windows? Why isn’t this a problem on Mac? If we only need
it on some platforms perhaps it should be conditional.

The line before this is a check for NaN; not sure why it’s using "x != x"
rather than std::isnan, maybe performance reasons?

The std::isfinite also checks for NaN. We don’t need both. If we do need the
check for infinity we should either use std::isinf or remove the NaN check.


More information about the webkit-reviews mailing list