Bug 32675 – HTMLInputElement::stepMismatch() uses ambiguous overload of pow()

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Steve Block]

HTMLInputElement::stepMismatch() calls pow() with two integer arguments. This causes a compile error due to an ambiguous overload on Android.

This was added in http://trac.webkit.org/changeset/51159

[Steve Block]

Created attachment 45099 [details]
Patch 1 for Bug 32675

Cast first argument of pow to double to force 'double pow(double, int)'

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 45099 [details] without any errors.

[Eric Seidel (no email)]

Comment on attachment 45099 [details]
Patch 1 for Bug 32675

This looks OK to me.  It would be nice if you could post the error in the bug for posterity, or even in the ChangeLog.  It's not clear to me what other signature on Andriod would be causing this error.

[Eric Seidel (no email)]

(I CC'd darin adler just because he was one of the primary driving forces behind our MathExtras.h and similar WTF files which deal with x-platform issues similar to this one.  I figured he might like to see this go by.)

[Steve Block]

The build error is ...

external/webkit/WebCore/html/HTMLInputElement.cpp:343: error: call of overloaded 'pow(int, int)' is ambiguous
bionic/libm/include/math.h:222: note: candidates are: double pow(double, double)
external/webkit/WebKit/android/stlport/stl/_cmath.h:443: note:                 float pow(float, float)
external/webkit/WebKit/android/stlport/stl/_cmath.h:443: note:                 long double pow(long double, long double)
external/webkit/WebKit/android/stlport/stl/_cmath.h:448: note:                 float pow(float, int)
external/webkit/WebKit/android/stlport/stl/_cmath.h:455: note:                 double pow(double, int)
external/webkit/WebKit/android/stlport/stl/_cmath.h:468: note:                 long double pow(long double, int)

The problem is that pow(int, int) isn't defined, nor is it in the c standard or c98.

[Eric Seidel (no email)]

We have a pow() in MathExtras:
http://trac.webkit.org/browser/trunk/JavaScriptCore/wtf/MathExtras.h

We also seem to try hard to avoid use of cmath:
http://trac.webkit.org/changeset/13104
http://trac.webkit.org/changeset/11667
http://trac.webkit.org/changeset/12225

And sometimes add it?
http://trac.webkit.org/changeset/10511

I'm not convinced this is the right work-around.  It would be nice if someone who is more familiar with our historical header troubles could comment.

[Eric Seidel (no email)]

Actually, I take that back.  This change has nothing really to do with cmath.  I think it's still fine.

[WebKit Commit Bot]

Comment on attachment 45099 [details]
Patch 1 for Bug 32675

Clearing flags on attachment: 45099

Committed r52292: <http://trac.webkit.org/changeset/52292>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Darin Adler]

I would have preferred the use of the constant, 2.0, which is a double rather than writing it as static_cast<double>(2), which seems less elegant to me.

[Eric Seidel (no email)]

Bah!  Agreed.  Clearly i wans't thinking.  I'll fix it.

[Eric Seidel (no email)]

Created attachment 45126 [details]
constant fix as recommended by Darin

[Eric Seidel (no email)]

Going to have the commit-queue land this for me to make sure I didn't make any stupid typos.  This laptop is too slow to build on.

[Eric Seidel (no email)]

Comment on attachment 45126 [details]
constant fix as recommended by Darin

Make the commit-queue land this.  Since I already filled in the reviewer the commit-queue won't get confused by me marking this r+.

And since it’s C++, you could also use numeric_limits<double>::digits() instead of DBL_MANT_DIG

[WebKit Commit Bot]

Comment on attachment 45126 [details]
constant fix as recommended by Darin

Clearing flags on attachment: 45126

Committed r52302: <http://trac.webkit.org/changeset/52302>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Eric Seidel (no email)]

Nifty.  I don't actually know what either of those values do. :)  But that's OK.  That part of this function wasn't touched by either patch.  I'm not going to go back and do the limits conversion now, but if I'm ever in that code again I'd be happy to!