Page MenuHomeWildfire Games

Fix build on armhf with gcc 7: error: call of overloaded 'abs(unsigned int)' is ambiguous
ClosedPublic

Authored by LudovicRousseau on Dec 7 2017, 3:44 PM.

Details

Summary

Fix build on armhf:

...
../../../source/gui/CDropDown.cpp: In member function 'virtual InReaction CDropDown::ManuallyHandleEvent(const SDL_Event_*)':
../../../source/gui/CDropDown.cpp:355:94: error: call of overloaded 'abs(unsigned int)' is ambiguous
       diff = std::abs(pList->m_Items[i].GetRawString().LowerCase()[j] - (int)m_InputBuffer[j]);
                                                                                              ^
In file included from /usr/include/c++/7/cmath:47:0,
                 from /usr/include/c++/7/math.h:36,
                 from ../../../source/lib/posix/posix_types.h:48,
                 from ../../../source/lib/precompiled.h:64,
                 from ../../../source/pch/gui/precompiled.h:18:
/usr/include/c++/7/bits/std_abs.h:78:3: note: candidate: constexpr long double std::abs(long double)
   abs(long double __x)
   ^~~
/usr/include/c++/7/bits/std_abs.h:74:3: note: candidate: constexpr float std::abs(float)
   abs(float __x)
   ^~~
/usr/include/c++/7/bits/std_abs.h:70:3: note: candidate: constexpr double std::abs(double)
   abs(double __x)
   ^~~
/usr/include/c++/7/bits/std_abs.h:61:3: note: candidate: long long int std::abs(long long int)
   abs(long long __x) { return __builtin_llabs (__x); }
   ^~~
/usr/include/c++/7/bits/std_abs.h:56:3: note: candidate: long int std::abs(long int)
   abs(long __i) { return __builtin_labs(__i); }
   ^~~
In file included from /usr/include/c++/7/bits/std_abs.h:38:0,
                 from /usr/include/c++/7/cmath:47,
                 from /usr/include/c++/7/math.h:36,
                 from ../../../source/lib/posix/posix_types.h:48,
                 from ../../../source/lib/precompiled.h:64,
                 from ../../../source/pch/gui/precompiled.h:18:
/usr/include/stdlib.h:735:12: note: candidate: int abs(int)
 extern int abs (int __x) __THROW __attribute__ ((__const__)) __wur;
            ^~~
gui.make:198: recipe for target 'obj/gui_Release/CDropDown.o' failed
make[3]: *** [obj/gui_Release/CDropDown.o] Error 1

The root cause of this failure is that wchar_t is unsigned on armhf. Subtracting a signed 32-bit number from an unsigned one results in an unsigned result which std::abs (understandably) does not support.
Casting both arguments of the subtraction to int fixes the build failure

Author: Peter Michael Green <plugwash@debian.org>

The complete build log is available at https://buildd.debian.org/status/fetch.php?pkg=0ad&arch=armhf&ver=0.0.22-1&stamp=1508365643&raw=0

This was Debian bug #879071
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=879071

Test Plan

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

LudovicRousseau created this revision.Dec 7 2017, 3:44 PM
elexis accepted this revision.Jan 14 2018, 10:10 PM
elexis added a subscriber: elexis.

The patch is correct (We can't remove the abs because the character values of arbitrary letters can occur).
The patch is presumably complete. (There are 19 occurrences of abs (without the std). I didn't test all of them and I'll assume that you guys have confirmed that this is the only instance where a conversion is needed, since you have marked your ticket as fixed.)
Tested the thing and the dropdowns still work as expected.

Subtracting a signed 32-bit number from an unsigned one results in an unsigned

(Not sure why that would be reasonable, 3 - 5 clearly doesn't sound like something non-negative to me, but anyway).

Author: Peter Michael Green <plugwash@debian.org>

Indeed, I could see here that Peter Green has created the original patch (Debdiff):
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=879071#40
I will add him to our credits as well.

Thanks for the upload Ludovic!

This revision is now accepted and ready to land.Jan 14 2018, 10:10 PM
This revision was automatically updated to reflect the committed changes.
Owners added a subscriber: Restricted Owners Package.Jan 14 2018, 10:18 PM