Bug 33952 – build failure on RVCT

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Joe Mason]

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.

[Joe Mason]

Created attachment 47118 [details]
build fix

[Darin Adler]

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.

[Darin Adler]

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?

[Joe Mason]

You're right, it's double,float, so only the second cast is needed.  I'll update the patch.

[Joe Mason]

Created attachment 47134 [details]
updated patch

[WebKit Review Bot]

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] [5]
Total errors found: 1


If any of these errors are false positives, please file a bug against check-webkit-style.

[Joe Mason]

Created attachment 47136 [details]
style fix

Grr.  Knew I forgot something.

[Darin Adler]

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

[WebKit Commit Bot]

Comment on attachment 47136 [details]
style fix

Clearing flags on attachment: 47136

Committed r53674: <http://trac.webkit.org/changeset/53674>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.