C Macro Question -(x) vs (-x)

Question

I'm going through quiz answers from my professor and a question was:

the correct implementation of a function like macro for absolute value is:

#define abs(x) ((x)<0 ? (-x) : (x))
#define abs(x) ((x)<0 ? -(x) : (x))

Why is the second one correct vs the first one?

And why do you have to use all the (). Like what are the rules involved? Every variable needs a ()? Thanks.

Answer 1

Yes, every variable needs parenthesis around it directly.

The reason is because you can pass things into the macro that aren't "nice", like arithmetic expressions or really any expression that isn't a single variable. It should be easy to see that with abs(1+2), the expanded -(1 + 2) will give a different result than (-1 + 2). This is why -(x) is more correct.

Unfortunately neither macro is safe, you should use an inline function for things like this instead. Consider:

abs (x++); // expands to ((x++) < 0 ? - (x++) : (x++))

This is clearly wrong with the macro, but it would work correctly if an inline function was used instead.

There are other problems with using macros instead of functions as well. See this question.

Edit: Noting that the question is about C, inline functions may only be available in C99.

Answer 2

There are various related problems that the extra parentheses solve. I'll go through them one by one:

Try: int y = abs( a ) + 2

Let's assume you use:

#define abs(x)  (x<0)?-x:x
...
    int y = abs( a ) + 2

This expands to int y = (a<0)?-a:a+2. The +2 binds only to the false result. 2 is only added when a is positive, not when it is negative. So we need parenthesis around the whole thing:

#define abs(x)  ( (x<0) ? -x : x )

Try: int y = abs(a+b);

But then we might have int y = abs(a+b) which gets expanded to int y = ( (a+b<0) ? -a+b : a+b). If a + b is negative then b is not negated when they add for the result. So we need to put the x of -x in parentheses.

#define abs(x)  ( (x<0) ? -(x) : x )

Try: int y = abs(a=b);

This ought to be legal (though bad), but it expands to int y = ( (a=b<0)?-(a=b):a=b ); which tries to assign the final b to the ternary. This should not compile. (Note that it does in C++. I had to compile it with gcc instead of g++ to see it fail to compile with the "invalid lvalue in assignment" error.)

#define abs(x)  ( (x<0) ? -(x) : (x) )

Try: int y = abs((a<b)?a:b);

This expands to int y = ( ((a<b)?a:b<0) ? -((a<b)?a:b) : (a<b)?a:b ), which groups the <0 with the b, not the entire ternary as intended.

#define abs(x)  ( ( (x) < 0) ? -(x) : (x) )

In the end, each instance of x is prone to some grouping problem that parentheses are needed to solve.

Common problem: operator precedence

The common thread in all of these is operator precedence: if you put an operator in your abs(...) invocation that has lower precedence then something around where x is used in the macro, then it will bind incorrectly. For instance, abs(a=b) will expand to a=b<0 which is the same as a=(b<0)... that isn't what the caller meant.

The "Right Way" to Implement abs

Of course, this is the wrong way to implement abs anyways... if you don't want to use the built in functions (and you should, because they will be optimized for whatever hardware you port to), then it should be an inline template (if using C++) for the same reasons mentioned when Meyers, Sutter, et al discuss re-implementing the min and max functions. (Other answers have also mentioned it: what happens with abs(x++)?)

Off the top of my head, a reasonable implementation might be:

template<typename T> inline const T abs(T const & x)
{
    return ( x<0 ) ? -x : x;
}

Here it is okay to leave off the parentheses since we know that x is a single value, not some arbitrary expansion from a macro.

Better yet, as Chris Lutz pointed out in the comments below, you can use template specialization to call the optimized versions (abs, fabs, labs) and get all the benefits of type safety, support for non-builtin types, and performance.

Test Code

#if 0
gcc $0 -g -ansi -std=c99 -o exe && ./exe
exit
#endif




#include <stdio.h>

#define abs1(x)  (x<0)?-x:x
#define abs2(x)  ((x<0)?-x:x)
#define abs3(x)  ((x<0)?-(x):x)
#define abs4(x)  ((x<0)?-(x):(x))
#define abs5(x)  (((x)<0)?-(x):(x))


#define test(x)     printf("//%30s=%d\n", #x, x);
#define testt(t,x)  printf("//%15s%15s=%d\n", t, #x, x);

int main()
{
    test(abs1( 1)+2)
    test(abs1(-1)+2)
    //                    abs1( 1)+2=3
    //                    abs1(-1)+2=1

    test(abs2( 1+2))
    test(abs2(-1-2))
    //                    abs2( 1+2)=3
    //                    abs2(-1-2)=-1

    int a,b;
    //b =  1; testt("b= 1; ", abs3(a=b))
    //b = -1; testt("b=-1; ", abs3(a=b))
    // When compiled with -ansi -std=c99 options, this gives the errors:
    //./so1a.c: In function 'main':
    //./so1a.c:34: error: invalid lvalue in assignment
    //./so1a.c:35: error: invalid lvalue in assignment

    // Abs of the smaller of a and b. Should be one or two.
    a=1; b=2; testt("a=1; b=2; ", abs4((a<b)?a:b))
    a=2; b=1; testt("a=2; b=1; ", abs4((a<b)?a:b))
    //               abs4((a<b)?a:b)=-1
    //               abs4((a<b)?a:b)=1


    test(abs5( 1)+2)
    test(abs5(-1)+2)
    test(abs5( 1+2))
    test(abs5(-1-2))
    b =  1; testt("b= 1; ", abs5(a=b))
    b = -1; testt("b=-1; ", abs5(a=b))
    a=1; b=2; testt("a=1; b=2; ", abs5((a<b)?a:b))
    a=2; b=1; testt("a=2; b=1; ", abs5((a<b)?a:b))
}

Output

                    abs1( 1)+2=3
                    abs1(-1)+2=1
                    abs2( 1+2)=3
                    abs2(-1-2)=-1
     a=1; b=2; abs4((a<b)?a:b)=-1
     a=2; b=1; abs4((a<b)?a:b)=1
                    abs5( 1)+2=3
                    abs5(-1)+2=3
                    abs5( 1+2)=3
                    abs5(-1-2)=3
         b= 1;       abs5(a=b)=1
         b=-1;       abs5(a=b)=1
     a=1; b=2; abs5((a<b)?a:b)=1
     a=2; b=1; abs5((a<b)?a:b)=1