cmath::fabs() VS my macro. Different result.
Author
Ok so I wrote some code that deals with 3D bounding boxes which is not important.
It was all working, and I later adapted it to a new project. In this project I have been dealing with many situations where i needed the absolute value of floats.
So I wrote the following macro:
#define ABS(i) ( i > 0 ? i : ( i*-1 ) )
and called it doing something like:
ABS( fSomeFloat )
And life was good. So when I adapted this code i changes my calls for fabs() (which is the cmath float version of abs() which is of course, absolute value) to use my new macro ABS().
And things started going funny, so I ended up changing back to fabs() out of desperation and low and behold, it works perfectly again. WHY?!
Is my macro wrong? (it must be) what does fabs() do differently?!
Any ideas?
Author
Won't that give you the absolute value of 1 (1) and then increment i to 2?
And sorry, just realized I posted this in Game Programing, I meant to do General Programing.
And sorry, just realized I posted this in Game Programing, I meant to do General Programing.
int i = 1;
ABS( ++i );
equals to:
int i = 1;
( ++i > 0 ? ++i : ( ++i*-1 ) );
Do you see the problem now ?
It will return 3! and i will be 3 too.
ABS( ++i );
equals to:
int i = 1;
( ++i > 0 ? ++i : ( ++i*-1 ) );
Do you see the problem now ?
It will return 3! and i will be 3 too.
Author
Yes but Im simply sending in the value of i, not i++.
Unless there is something Im missing on how macros work?
That Macro should simply evaluate to what ever the ABS is, not affect the value of i at all correct?
Unless there is something Im missing on how macros work?
That Macro should simply evaluate to what ever the ABS is, not affect the value of i at all correct?
You probably have a function call that affects some value, and since macros are just text-replacements, it get's called a billion times.
Author
hhhmmm... you guys must be right.
I understand what you are saying,
my actual code looks like:
fabs((float)(curNode->min[0]) - pos.x)
or
ABS((float)(curNode->min[0]) - pos.x)
obviously.
I bet what is happening is it's taking the macro:
#define ABS(i) ( i > 0 ? i : ( i*-1 ) )
and in the false state it is doing:
( i*-1 ) = ( (float)(curNode->min[0]) - pos.x * -1 )
So order of operations, pos.x is being * -1 and THEN subtracted, thanks for the help guys! Seriously!
++rating for both ya ;)
I understand what you are saying,
my actual code looks like:
fabs((float)(curNode->min[0]) - pos.x)
or
ABS((float)(curNode->min[0]) - pos.x)
obviously.
I bet what is happening is it's taking the macro:
#define ABS(i) ( i > 0 ? i : ( i*-1 ) )
and in the false state it is doing:
( i*-1 ) = ( (float)(curNode->min[0]) - pos.x * -1 )
So order of operations, pos.x is being * -1 and THEN subtracted, thanks for the help guys! Seriously!
++rating for both ya ;)
This is reason #27 to not use a macro....
Seriously, just write an inlined function if you want to. It doesn't have to be ++i that you do in the call to ABS(), it could be any function, or pretty much anything. Besides, why don't you want to use fabs()? It's a good function!
Edit: Same time posting. What you want to do is wrap i inside of parenthesis. Order of operations means that it's only multiplying pos.x by -1. Like so:
(float)(curNode->min[0]) - pos.x*-1
Now, obviously, curNode->min[0] isn't being multiplied by -1! The horror!
Edit again: And it looks like you've figured it out, anyways.
Seriously, just write an inlined function if you want to. It doesn't have to be ++i that you do in the call to ABS(), it could be any function, or pretty much anything. Besides, why don't you want to use fabs()? It's a good function!
Edit: Same time posting. What you want to do is wrap i inside of parenthesis. Order of operations means that it's only multiplying pos.x by -1. Like so:
(float)(curNode->min[0]) - pos.x*-1
Now, obviously, curNode->min[0] isn't being multiplied by -1! The horror!
Edit again: And it looks like you've figured it out, anyways.
Quote:Original post by Anonymous Poster
int i = 1;
ABS( ++i );
equals to:
int i = 1;
( ++i > 0 ? ++i : ( ++i*-1 ) );
Do you see the problem now ?
It will return 3! and i will be 3 too.
No, there is no guarantee that will return 3. It is undefined behavior. It might return anything.
You are modifying the variable i twice between 2 sequence points.
More info on sequence points
This is why often macros are defined using a liberal sprinkling of parentheses anywhere the arguments are used, eg:
#define ABS(i) ( (i) > 0 ? (i) : ( (i) * -1 ) )
This won't stop the pre-/post-increment problem, but it solves most of the other side effects.
#define ABS(i) ( (i) > 0 ? (i) : ( (i) * -1 ) )
This won't stop the pre-/post-increment problem, but it solves most of the other side effects.
This topic is closed to new replies.
Advertisement
Popular Topics
Advertisement
