4

My team is writting code to be compiled for both Windows (using VS2015) and Android (using GCC 4.9 invoked by QtCreator).

We figured out that Android binaries had a problem with abs function.

double a = 1.0;
double b = 0.5;
std::cout << abs( a - b ) << std::endl;
std::cout << std::abs( a - b ) << std::endl;

Displays:

1
0.5

This is a known issue, found this topic (among others): Strange bug in usage of abs() I encountered recently

There are lots of places where we use abs, I'll replace them all by std::abs. Fine. But how can I prevent abs to be used again in the future?

Found this topic: Avoiding compiler issues with abs(), but it did not help.

I can't enable treating all warnings as errors (-Werror -Wall) because g++ is much less permissive than MSVC. Even if we make the effort to compile with 0 warning on MSVC, we still get tons of them with g++ (among them there could be one about abs being used badly) and we historically ignore them. Fixing them all would take us too much effort.

Community
  • 1
  • 1
jpo38
  • 20,821
  • 10
  • 70
  • 151
  • By treating warnings as errors? – Humam Helfawi Jul 21 '16 at 08:42
  • 7
    I don't think you'll like this, but you really need to invest in compiling with -Werror/Wall. Otherwise you're flying blind - for this issue and many more. If you're getting "tons of errors with G++" then it's likely that at least some of them are calling out bugs in your code. – Oliver Charlesworth Jul 21 '16 at 08:43
  • implicitly converting from double to int will raise warning, won't it? – Humam Helfawi Jul 21 '16 at 08:45
  • @jpi38 May you please compile the code in your question with the highest level of warning catching and tell us the result – Humam Helfawi Jul 21 '16 at 08:56
  • @HumamHelfawi: I definitely get the conversion warning, among all other ones. – jpo38 Jul 21 '16 at 08:58
  • @jpo38 Great.. depedning on your IDE, it may have an option for treat a specfic warning as an error. You may do that , then every developer will got an error which is enough to make it clear that this abs should not be used. However, you should treat all our warnings as errors.. warnings are the heaven gift for coder – Humam Helfawi Jul 21 '16 at 09:02
  • @HumamHelfawi: I don't think g++ lets you treat a specific warning as error – jpo38 Jul 21 '16 at 09:05
  • you can use `grep -rInv "std::abs" . | grep "abs"` to find most of the non-`std::abs`. Unless both `std::abs` and `abs` appear on the same line. – Morris Franken Jul 21 '16 at 09:08
  • @iamai: That won't prevent developers from adding a call to `abs`in the future. – jpo38 Jul 21 '16 at 09:14
  • This is more of a workflow issue than a bug in code. So if you are asking for a possible solution then it is more so dependant on your workflow. In which case we need a bit more information on your workflow... e.g. do you use cmake, git, an ide? etc. – silvergasp Jul 21 '16 at 09:25
  • @bruffalobill: Using Cmake to generate VS projects. And I'm manualy generating .pro files for QtCreator from my CMake scripts. Then QtCreator compiles using Android NDK. – jpo38 Jul 21 '16 at 09:29

3 Answers3

3

Do you have the possibility to set mandatory Compiler flags at a central Location (common cmake includes or whatever other build system you use)? If so, you can create a header file as indicated by John and add a compile flag that will include the file at the beginning of any compilation unit:

MSVC: /FI <path_to_file>

GCC: -include <path_to_file>

Clang: Same as GCC

Michael Schmidt
  • 31
  • 3
2

Do you have a header file that is basically included by everything? Some kind of holder-of-all-fundamentals? If so, you can put this in there:

extern void NeverDefined();

inline int abs(int a) {
    NeverDefined();
    return a;
} // abs(int)

If you then get a linker error on NeverDefined, you'll know there's a problem!

Normally I simply wouldn't define the function in question, but since it's a library function I had to have this second level.

Edit

In fact: don't bother with a header file (the standard definition will suffice). Write your own abs.cc with the above code (no inline) and it will supersede the library definition.

Of course, this won't work if the original file marks the function inline - in which case you could always edit the original definition...

John Burger
  • 3,662
  • 1
  • 13
  • 23
  • Imagine spending just half hour compiling a project and then got linker error at the end.. – Humam Helfawi Jul 21 '16 at 08:54
  • Nice proposal, but unfortunately, I don't have a header file included by all other ones... – jpo38 Jul 21 '16 at 08:55
  • @HumamHelfawi Imagine not getting any notification at all - or a runtime error as with an `assert`. This catches the problem before the program can be run. If you're taking 30 minutes for a compile, your developers aren't compiling often enough. You're supposed to make a small change (minimal compile), then note the error. If it takes 30 minutes, you've made a huge change. But then you fix the one or two places with the mistake, and recompile in twenty seconds. – John Burger Jul 21 '16 at 09:05
  • @jpo38 Maybe now's the time to start? I've found them useful in the past - but you need to control its use to prevent it from becoming a misused convenience area... But they're a Heaven-sent area for cross-compiler "fixes". – John Burger Jul 21 '16 at 09:06
  • @JohnBurger: Makes sense, but I have thousands of files. And it won't prevent developer from using `abs`....because he may still forget to include to header file in his file using `abs` function... – jpo38 Jul 21 '16 at 09:13
  • Next option: Define my version in a `.o` file (without `inline`) and add it to the link line before the libraries. That will supersede the library definition. See my edit... – John Burger Jul 21 '16 at 09:21
  • @JohnBurger: Header file usage does not work. g++ complains the same function is defined twice `stdlib.h:86:23: error: redefinition of 'int abs(int)' static __inline__ int abs(int __n) {`. Can you elaborate the abs.cc approach? – jpo38 Jul 21 '16 at 09:27
  • Sadly no. I was trying to add an edit "unless the original header file has it `inline`..." (which is your issue) Unless you want to edit `stdlib.h`? – John Burger Jul 21 '16 at 09:29
  • @JohnBurger: Defining `inline int abs(int a, int unused = 0)` works better, then compiler allows redefinition as it's different and reports error for ambiguous call upon compilation. But unfortunately you aslo get a compiler error when standard files are compiled. For instance vc\include\xlocnum included by cmath fails to compile because it uses `abs(int)` which is now ambiguous....so this solution does not work in the end. – jpo38 Jul 21 '16 at 12:48
1

As suggested in the same linked answer from your question:

  1. <math.h> is responsible for abs(int)
  2. <cmath> is responsible for std::abs(double)

If you have a top-most header file then simply put below method:

template<typename T> void abs (T);

Hence, wherever there is an abs found, a compiler error will be reported for its void as return value. Additionally this function is unimplemented to give a linker error at least.
This compiler error can be used for replacing such abs() with std::abs(). For harmless abs(int), this will expectedly not generate any error as it's available in <math.h>.

Suppose, you don't have any header file included in all the source files then as a one time exercise, you may want to replace every text of #include<math.h> with #include<cmath>. However, this won't be full proof in all cases. For example:

double my_abs () {
  using std::abs;
  return abs(x - y);
}

Hence, it's good to have a common utility header file in all the source file as a practice. Let it be empty.

iammilind
  • 68,093
  • 33
  • 169
  • 336
  • Declaring `template void abs (T);` does not lead to any error in msvc. Anyway, this is not an acceptable solution, because then, you aslo get compiler error from standard files. For instance `vc\include\xlocnum` included by cmath fails to compile because it uses `abs(int)`! – jpo38 Jul 21 '16 at 12:30
  • @jpo38, in g++ the template version leads to error if not used properly. See [the demo](http://ideone.com/I6n8Ha). I don't see any reason why MSVC shouldn't follow this simple standard as well. Is that something you are observing wrong? Also not sure, why `xlocnum` fails to compile. If you can provide a minimal verifiable example, then probably I can help you in solving the problem. On side note, I see that `int abs(int) is part of `` in my system. – iammilind Jul 21 '16 at 12:49
  • 1
    With MSVC, I confirm `abs` can still be used with no compiler error if setting `template void abs (T);`, they apparently don't follow this simple standard. However, we don't really care as this `abs` bug only appears with g++ (msvc does a double abs operation, as expected). So a header file with your function definition combined with Michael Schmidt's solution to automatically include the file for all sources does the trick (avoids `abs` from being used with g++)! – jpo38 Jul 21 '16 at 14:13