|Summary:||build failure on RVCT|
|Product:||WebKit||Reporter:||Joe Mason <joenotcharles>|
|Component:||WebCore Misc.||Assignee:||Nobody <webkit-unassigned>|
|Severity:||Normal||CC:||commit-queue, darin, hausmann, webkit.review.bot|
|Version:||528+ (Nightly build)|
Description Joe Mason 2010-01-21 07:46:05 PST
RVCT 4.0 fails to compile in FEComponentTransfer.cpp - "ambiguous overload in function pow" (provided args are (int,float), possible overloads include (int,int), (float,float) and (double,double). This patch casts the args to double to match the return value.
Comment 2 Darin Adler 2010-01-21 11:40:29 PST
Comment on attachment 47118 [details] build fix 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.
Comment 3 Darin Adler 2010-01-21 11:41:40 PST
I really don't see anything passing int, float here. Maybe double, float. Seems like yet another RVCT compiler bug. Is there any better way to support this flawed compiler?
Comment 4 Joe Mason 2010-01-21 11:46:03 PST
You're right, it's double,float, so only the second cast is needed. I'll update the patch.
Comment 6 WebKit Review Bot 2010-01-21 12:02:38 PST
Attachment 47134 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebCore/platform/graphics/filters/FEComponentTransfer.cpp:144: One space before end of line comments [whitespace/comments]  Total errors found: 1 If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 Joe Mason 2010-01-21 12:12:20 PST
Created attachment 47136 [details] style fix Grr. Knew I forgot something.
Comment 8 Darin Adler 2010-01-21 14:11:59 PST
Comment on attachment 47136 [details] style fix > + Copyright (C) Research In Motion Limited 2010. All rights reserved. I think there is a guideline about how big a chance is sufficient to assert copyright. This is probably small enough that it's not sufficient. Change looks great. r=me
Comment 9 WebKit Commit Bot 2010-01-21 21:23:18 PST
Comment on attachment 47136 [details] style fix Clearing flags on attachment: 47136 Committed r53674: <http://trac.webkit.org/changeset/53674>
Comment 10 WebKit Commit Bot 2010-01-21 21:23:23 PST
All reviewed patches have been landed. Closing bug.