[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





--- Comment #3 from Filip Pizlo <fpizlo at apple.com>  2014-02-11 09:38:36 PST ---
View in context: https://bugs.webkit.org/attachment.cgi?id=223868&action=review

Your change needs a test.  Please include a test in your patch.

> Source/JavaScriptCore/runtime/MathObject.cpp:193
>          double val = exec->uncheckedArgument(k).toNumber(exec);
>          if (std::isnan(val)) {
>              result = QNaN;
> -            break;
>          }
>          if (val > result || (!val && !result && !std::signbit(val)))
>              result = val;

I wouldn't write this code this way.  You're relying on the comparison in the next 'if' statement falling through when result is NaN.  It's true that this will work, but it's subtle.  Subtle code just leads to more bugs later.

You could fix this with a comment above the second 'if' statement saying that it's written in a way that guarantees that it will fall through when result is NaN.

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