Bug 163869 – GCC warning in testb3.cpp testAbsArgWithEffectfulDoubleConversion

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Michael Catanzaro]

Since r207696 "Expand Trunc in B3 to support Double to Float":

[284/3418] Building CXX object Source/...akeFiles/testb3.dir/__/b3/testb3.cpp.o
../../Source/JavaScriptCore/b3/testb3.cpp: In function ‘void {anonymous}::testAbsArgWithEffectfulDoubleConversion(float)’:
../../Source/JavaScriptCore/b3/testb3.cpp:3832:38: warning: ISO C++ says that these are ambiguous, even though the worst conversion for the first is better than the worst conversion for the second:
     CHECK(isIdentical(effect, fabs(a)));
                                      ^
../../Source/JavaScriptCore/b3/testb3.cpp:100:16: note: in definition of macro ‘CHECK’
         if (!!(x))                                                      \
                ^
In file included from ../../Source/JavaScriptCore/b3/air/AirArg.h:31:0,
                 from ../../Source/JavaScriptCore/b3/air/AirCode.h:30,
                 from ../../Source/JavaScriptCore/b3/testb3.cpp:28:
../../Source/JavaScriptCore/b3/B3Common.h:67:13: note: candidate 1: bool JSC::B3::isIdentical(double, double)
 inline bool isIdentical(double left, double right)
             ^~~~~~~~~~~
../../Source/JavaScriptCore/b3/B3Common.h:72:13: note: candidate 2: bool JSC::B3::isIdentical(float, float)
 inline bool isIdentical(float left, float right)
             ^~~~~~~~~~~
../../Source/JavaScriptCore/b3/testb3.cpp: In function ‘void {anonymous}::testSqrtArgWithEffectfulDoubleConversion(float)’:
../../Source/JavaScriptCore/b3/testb3.cpp:4336:38: warning: ISO C++ says that these are ambiguous, even though the worst conversion for the first is better than the worst conversion for the second:
     CHECK(isIdentical(effect, sqrt(a)));
                                      ^
../../Source/JavaScriptCore/b3/testb3.cpp:100:16: note: in definition of macro ‘CHECK’
         if (!!(x))                                                      \
                ^
In file included from ../../Source/JavaScriptCore/b3/air/AirArg.h:31:0,
                 from ../../Source/JavaScriptCore/b3/air/AirCode.h:30,
                 from ../../Source/JavaScriptCore/b3/testb3.cpp:28:
../../Source/JavaScriptCore/b3/B3Common.h:67:13: note: candidate 1: bool JSC::B3::isIdentical(double, double)
 inline bool isIdentical(double left, double right)
             ^~~~~~~~~~~
../../Source/JavaScriptCore/b3/B3Common.h:72:13: note: candidate 2: bool JSC::B3::isIdentical(float, float)
 inline bool isIdentical(float left, float right)
             ^~~~~~~~~~~

[Michael Catanzaro]

Hm, I'm not sure how to fix this.

I tried turning all the different definitions of isIdentical into template specializations, but that just added a bunch of new ambiguous function call build failures.

[Michael Catanzaro]

Ah OK, the problem is the code is somehow accidentally using std::fabs and std::sqrt (not sure how) and getting a float result, but it looks like the test actually wants ::fabs and ::sqrt so that the return value will be a double.

Alternatively, the return values could just be cast to double to fix the warning, but not sure if that would be right for this test or not.

[Michael Catanzaro]

(In reply to comment #2)
> Ah OK, the problem is the code is somehow accidentally using std::fabs and
> std::sqrt (not sure how) and getting a float result, but it looks like the
> test actually wants ::fabs and ::sqrt so that the return value will be a
> double.

Hm, I'm wrong, it doesn't silence the warning....

> Alternatively, the return values could just be cast to double to fix the
> warning, but not sure if that would be right for this test or not.

I think this should be fine.

[Michael Catanzaro]

Created attachment 292546 [details]
Patch

[Michael Catanzaro]

Looks like Zan committed an identical patch today.