'else' without previous 'if' error when defining macro with arguments

Question

Consider the C code below.

#include <stdio.h>
#define foo(x) if (x>0) printf("Ouch\n");

int main()
{
    int a = 4;
    int b = -3;

    if(a>b)
        foo(b) ;
    else
        printf("Arrg!\n");
    printf("thanks\n");

    return 0;
}

When the program is run, I get an error saying error: ‘else’ without a previous ‘if’. When we replace foo(b) with if (b>0) printf("Ouch\n") according to the macro definition, shouldn't the program transfer into the following code when written with braces?

if(a>b){
    if(x>0){
        printf("Ouch\n");
    }
}
else{
    printf("Arrh!\n");
}
printf("thanks\n");    

I don't see why the compiler complains. What does the program actually get transferred to?

Thanks

Answer 1

No, it's turning into this code:

if (a > b)
        if (b > 0) printf("Ouch\n");;
    else
        printf("Arrg!\n");

Note that there are two semicolons after ("Ouch\n"). This is what breaks the code. The else follows that second semicolon which is a null statement, not the if statement before that. The cause of this is that you have a semicolon in your macro definition and another one where you call the macro.

I suggest putting the statement(s) of a function-like macro in its own block, like this answer suggests. But that's a generall suggestion, in this specific example it's best to not have a macro at all.

Answer 2

The problem is your macro expands to

if(a>b)
    if (b>0) printf("Ouch\n"); ; 
else 
    printf("Arrg!\n");
//...

and that won't work with the else because of the extra ;. (Also you could have parenthesesized the x argument (#define foo(x) if ((x)>0) printf("Ouch\n"))).

If you lost the ; from the macro, you'd get a different parse: the else would match up with the inner if like so:

if(a>b){ /*braces inserted to show the interpretation*/
    if (b>0) printf("Ouch\n");
    else printf("Arrg!\n");
}

and while you could solve the problem by making the macro with a dangling else

#define foo(x) if ((x)>0) printf("Ouch\n"); else
//a ; after the macro else would complete the `else` with an empty branch

using this, especially inside other if-else triggers warnings in compilers like gcc/clang when compiling -Wall and similar options, so the best way to deal with is with the idiomatic

#define macro() do{/*macro_body*/}while(0)

In your case

#define foo(x) do{ if ((x)>0) printf("Ouch\n"); }while(0)

Some people like to always use compound statements (surrounded by { }) with if/else statements, and while that would also solve the issue, I feel if you make function-like macros for other people to use, it's best not to force a style on them.

Answer 3

My advice is to ignore any suggestions on how to fix that macro, the real problem is that you're using the macro at all.

There is almost no reason to use macros (which are really just slightly-less-than-dumb text substitutions) in modern C for anything other than conditional compilation.

• Value-type macros should generally be replaced with enumerations because they better preserve type information.
• Function type macros should just be functions since modern compilers have very little trouble making them inline if they need it.

Using real functions also solves all those bizarre edge cases like:

#define calc(x) x * x
:
int a = 7;
int b = calc(a + 1);   // a + (1 * a) + 1, NOT (a + 1) * (a + 1)

If you want that to work reliably, you need the macro to be something like ((x) * (x)), but even that will fail with more complexity, such as b = calc(a++).

---

So, in short, what you should have in your code is:

void foo(int x) {
    if (x > 0)
        printf("Ouch\n");
}

---

If you do need to be able to use function macros anywhere (naked statement, statement within a braced if-block, statement within an unbraced if-block, braced and unbraced while statements and so on), you have to resort to bizarre macros such as (making sure to #include <stdbool.h> to get access to false, of course):

#define XYZZY(s) do {plugh(s);} while (false)

However, I would strongly suggest just making them functions in the first place.

Answer 4

Its always considered as safe and good practice to use { } after if and else block to avoid such issues.

Correct one is

if(a>b) { 
    foo(b) ; /* keep inside { } */
}
else {
    printf("Arrg!\n");
}

shouldn't the program transfer into the following code when written with braces?

No, compiler doesn't put manually curly braces. After Macro replacement gcc -E test.c looks like

int main()
{
    int a = 4;
    int b = -3;

    if(a>b)
        if (b>0) printf("Ouch\n"); ; /* extra semicolon causes the issue */
    else /* there is no if for this else block, previous one terminated by extra ; in above if */
        printf("Arrg!\n");
    printf("thanks\n");

    return 0;
}

sample code :

#include <stdio.h>
#define foo(x) if ((x)>0) printf("Ouch\n") /* if condition was wrong.. instead of x use (x) */
int main(void)
{
    int a = 4;
    int b = -3;
    if(a>b)
    { /* always keep curly braces even though there is only one statement after if */
        foo(b);
    }
    else
    {
        printf("Arrg!\n");
    }
    printf("thanks\n");
    return 0;
}

Though I would suggest to define MACRO like below

#define foo(x) \
    do {                   \
        if((x) > 0)         \
        printf("Ouch\n");  \
    } while(0)