[webkit-reviews] review granted: [Bug 33952] build failure on RVCT : [Attachment 47118] build fix

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Jan 21 11:40:28 PST 2010


Darin Adler <darin at apple.com> has granted Joe Mason <jmason at rim.com>'s request
for review:
Bug 33952: build failure on RVCT
https://bugs.webkit.org/show_bug.cgi?id=33952

Attachment 47118: build fix
https://bugs.webkit.org/attachment.cgi?id=47118&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
I'm unclear on what's ambiguous here and why RVCT is complaining. Where's the
ambiguity? In C++, the expression i / 255.0 is unambiguously a double, so I
can't see how adding a cast to that expression is helpful.

Further, pow(double, int) also does not seem ambiguous.

> -	   double val = 255.0 * (transferFunction.amplitude * pow((i / 255.0),
transferFunction.exponent) + transferFunction.offset);
> +	   double val = 255.0 * (transferFunction.amplitude *
pow((static_cast<double>(i) / 255.0),
static_cast<double>(transferFunction.exponent)) + transferFunction.offset);

The change is OK. It's better to keep casts to a minimum if possible, so an
approach that used local variables instead might be cleaner.

I'm going to say review+ because there's not a lot of downside here, but I
think this is probably not a great change given a lack of understanding of
what's ambiguous combined with the relatively ugly solution.


More information about the webkit-reviews mailing list