Bug 130286 – Fix build: using integer absolute value function 'abs' when argument is of floating point type

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[David Kilzer (:ddkilzer)]

Fix the following build failure when using trunk clang (r203900):

JavaScriptCore/assembler/MacroAssembler.h:992:17: error: using integer absolute value function 'abs' when argument is of floating point type [-Werror,-Wabsolute-value]
        value = abs(value);
                ^
JavaScriptCore/assembler/MacroAssembler.h:992:17: note: use function 'fabs' instead
        value = abs(value);
                ^~~
                fabs
1 error generated.

[David Kilzer (:ddkilzer)]

Created attachment 226808 [details]
Patch v1

[Filip Pizlo]

Comment on attachment 226808 [details]
Patch v1

View in context: https://bugs.webkit.org/attachment.cgi?id=226808&action=review

R=me to fix build.  Leaving cq? alone in case you want to land the fabs() change instead.  Calling abs() and casting to int means we miss a *bunch* of blinding opportunities.

> Source/JavaScriptCore/assembler/MacroAssembler.h:992
> -        value = abs(value);
> +        value = abs(static_cast<int>(value));

I strongly suspect that we meant to call fabs instead.

[David Kilzer (:ddkilzer)]

(In reply to comment #2)
> (From update of attachment 226808 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=226808&action=review
> 
> R=me to fix build.  Leaving cq? alone in case you want to land the fabs() change instead.

This is for a trunk version of clang we're not compiling with yet, so no need to rush a fix.

> Calling abs() and casting to int means we miss a *bunch* of blinding opportunities.
> 
> > Source/JavaScriptCore/assembler/MacroAssembler.h:992
> > -        value = abs(value);
> > +        value = abs(static_cast<int>(value));
> 
> I strongly suspect that we meant to call fabs instead.

That's why I posted a patch for review.  :)

I'll switch to using fabs() before I land the patch.  Thanks!

[David Kilzer (:ddkilzer)]

Committed r165683: <http://trac.webkit.org/changeset/165683>

[Darin Adler]

Comment on attachment 226808 [details]
Patch v1

View in context: https://bugs.webkit.org/attachment.cgi?id=226808&action=review

> Source/JavaScriptCore/ChangeLog:10
> +            JavaScriptCore/assembler/MacroAssembler.h:992:17: error: using integer absolute value function 'abs' when argument is of floating point type [-Werror,-Wabsolute-value]

I am *so* glad they added this warning. Use of abs instead of fabs is something I keep catching in patch review.