[Webkit-unassigned] [Bug 104147] Math.{max, min}() must not return after first NaN value

[bugzilla-daemon at webkit.org]

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


Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #223974|review?                     |review-
               Flag|                            |




--- Comment #14 from Darin Adler <darin at apple.com>  2014-02-12 19:34:19 PST ---
(From update of attachment 223974)
The tests added look like fine minimal coverage of this change, but we also need to update the expected results in LayoutTests/js/math-expected.txt; please upload a new patch with that.

I also think the code change is incorrect. It looks like this code will continue to evaluate values even if ToNumber raises an exception. I don’t think that’s right. I think that once a ToNumber raises an exception, we should stop iterating through the rest of the arguments. The old code incorrectly had this behavior any time there is a NaN involved, but the new code incorrectly continues after an exception.

To test this we probably need test cases that involve side effects, not just exceptions. Here is one example (a bit clumsily written) that I think will show we have it wrong:

    sideEffect = 0;
    shouldThrow("Math.min({valueOf:function(){throw 'error1'}}, valueOf:function(){sideEffect = 1}})", "error1")
    shouldBe('sideEffect', '0');

I believe sideEffect will be 1.

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