-3

Apparently I have a fundamental misunderstanding of how macros work. I thought a macro just caused the preprocessor to replace the @defined macro with the replacement text. But apparently that's not always the case. My code follows: TstBasInc.h

#pragma once

#include <stdlib.h>
#include <string.h>
#include <ctype.h>
#include <math.h>
#include <stdint.h>
#include <stdbool.h>
#include <stdarg.h>
#include <time.h>

//  copy macro
#define Cpy(ToVar, FrmVar) do {                                                                                 \
                                errno = strncpy_s(ToVar, sizeof(ToVar), FrmVar, _TRUNCATE);                     \
                                    if (errno == STRUNCATE)                                                     \
                                        fprintf(stderr, "string '%s' was truncated to '%s'\n", FrmVar, ToVar);  \
                            } while(0);

//  clear numeric array macro
#define ClrNumArr(ArrNam, ArrCnt)           \
            for (s = 0; s < ArrCnt; s++)    \
                ArrNam[s] = 0;


uint32_t    s;          //  subscript

typedef struct {
    short   C;
    short   YY;
    short   MM;
    short   DD;
} SysDat;

TstMacCmpErr:

#include "stdafx.h"
#include "TstBasInc.h"      //  test basic include file

#define ARRCNT 3

int main()
{
    char    Cnd = 'E';      //  define to use 'else' path
    char    ToVar[7 + 1];   //  Cpy To-Variable
    int     IntArr[ARRCNT]; //  integer array

    Cpy(ToVar, "short")                     //  compiles with or without the semi-colon
    if (Cnd != 'E')
//      Cpy(ToVar, "short string");         //  won't compile:  illegal else without matching if
        Cpy(ToVar, "short string")          //  will compile
    else
        Cpy(ToVar, "extra long string");    //  compiles with or without the semi-colon
//  the following code shows how I thought the macro would expand to    
    {                                                                                   \
        errno = strncpy_s(ToVar, sizeof(ToVar), "short str", _TRUNCATE);                \
        if (errno == STRUNCATE)                                                         \
            fprintf(stderr, "string '%s' was truncated to '%s'\n", "short str", ToVar);;    \
    }

    if (Cnd == 'E') {
        ClrNumArr(IntArr, ARRCNT)               //  compiles with or without the semi-colon
            printf("intarr[0] = %d\n", IntArr[0]);
    }
    else
        printf("intarr[0] is garbage\n");

    return 0;
}

The results follow:

string 'extra long string' was truncated to 'extra l'
string 'short str' was truncated to 'short s'
intarr[0] = 0;

As the comments say, when I have a semi-colon after Cpy(ToVar, "short string"); it won't even compile because I get an "C2181 illegal else without matching if" error. As you see, I tried adding a do-while in the macro as suggested in this post, but it didn't make any difference. When the macro code is copied directly (even without the do-while) that code works okay. I would have thought just adding the braces in the macro would have fixed the problem but it didn't. It must have something to do with the Cpy ending in an if because the ClrNumArr macro compiles with or without the semi-colon. So can someone tell me why the Cpy macro doesn't just replace the text? I must be missing something simple.

I'm using VS 2015 Community Edition Update 1.

Edit: I documented the problem and isolated it to (I think) the if statement in the Cpy macro. Still nobody has explained why the macro didn't expand the way I thought it should. That should have been the title of the post, as that's my question more than the problem with the semi-colon, which I now have a solution for.

Community
  • 1
  • 1
J. Toran
  • 157
  • 2
  • 2
  • 14
  • 3
    Don't put semi colons in macro definitions. – Iharob Al Asimi Jan 18 '16 at 12:26
  • 4
    Don't use macros where a function would work. – Roger Lipscombe Jan 18 '16 at 12:28
  • @iharob: Thank you. In Visual Studio, I get `C2143 syntax error: Missing ';' before 'if'` and the same message on many other lines. – J. Toran Jan 18 '16 at 12:29
  • 1
    see https://gcc.gnu.org/onlinedocs/cpp/Swallowing-the-Semicolon.html note that semicolon is actually a null statement – tigris Jan 18 '16 at 12:30
  • @Roger Lipscombe: I thought about that, but I thought the overhead calling a function would slow things down. I could be calling this simple routine perhaps millions of times if in loops, is why I left it a macro. – J. Toran Jan 18 '16 at 12:34
  • @tigris: that's why I added a do-while in the macro, but it didn't' make any difference. – J. Toran Jan 18 '16 at 12:36
  • 2
    0) As stated, don't use macros where a functions also works. One reason just bit you. 1) Do not try premature optimisations. The compiler and CPU are possibly smarter than you are. 2) If your code really **proves** too slow, use `inline` functions and/or link time optimisation (LTO). 3) As a beginner don't even think about such things. – too honest for this site Jan 18 '16 at 12:41
  • @Olaf: All your points are probably valid. I did some profiling of calling an empty procedure, but I lost it. It maybe wasn't as significant as I thought. I would like to learn what an `inline` function is and LTO, but I think that's a little too advanced for me right now. – J. Toran Jan 18 '16 at 12:45
  • Hmm... simply adding `inline` specifier is too advanced, but macros are not? Strange priorities, I'd say. Note that as I wrote - the error would not have occured with either a normal or an `inline` function. About LTO, I basically agree; I just mentioned it for completeness. Until then, don't try replacing functions with macros. (Personal note: I personally think a student using such a macro should fail it's assignment/test/whatever. Regardless of the rest of the code. I strongly emphasise that in my lessons.) – too honest for this site Jan 18 '16 at 12:54
  • @Olaf: In my reading, I've never ran across `inline`. Granted I've only read K&R and 3 other books. I've seen `inline` on SO, but never grokked what it meant. As far as the macro itself, I don't see why it should cause me to fail a test. Seems pretty self-explanatory to me. – J. Toran Jan 18 '16 at 13:05
  • 1
    Don't use K&R. This does not even cover C99, not to mention standard C (which is C11 - there is no other valid C standard). You would not want to learn driving/using a Model-T car either, or a pre-digital mobile phone. – too honest for this site Jan 18 '16 at 13:08
  • I admit, I've heard that about K&R and there only being the C11 standard, but I had to start somewhere. :) – J. Toran Jan 18 '16 at 13:21
  • On further investigation, it's proved that the macro is only a few milliseconds faster than a function call, over 1,000,000 loops. So I will change it to a function. However, nobody has yet explained _why_ the macro didn't expand the way I thought it would. – J. Toran Jan 18 '16 at 14:06

3 Answers3

3

In case you put a semicolon,

    Cpy(ToVar, "short")                     //  compiles with or without the semi-colon
    if (Cnd != 'E')
//      Cpy(ToVar, "short string");         //  won't compile:  illegal else without matching if
        Cpy(ToVar, "short string")          //  will compile
    else
        Cpy(ToVar, "extra long string");    //  compiles with or without the semi-colon

this makes the syntax look like

if (...)
    ...;
    ;                // <-- The problem here
else
   ...;

which is illegal syntax, as the else will have no associated if in this syntax.

OTOH, in case you write

if (...) {
    ...;
    ;                // <-- no problem here, inside a block
 }
else
   ...;

the else is associated with the previous if. So, all fine.

Sourav Ghosh
  • 133,132
  • 16
  • 183
  • 261
  • Thank you. I had already found that if I enclosed the macro in braces, it would work fine. That's why I added the braces in the macro, so I wouldn't have to keep typing in the extra braces every time I wanted to use the macro inside an `if`. My question is why don't the braces in the macro expand properly. – J. Toran Jan 18 '16 at 12:54
  • 1
    @J.Toran I think you did not get the point of my post. The macro definition is one _statement_, and without the `{}` in the code, the ending `;` present the code will break the `if..else` coalition. It does not matter whether you a `{}` in the MACRO definition or not. Am I clear? – Sourav Ghosh Jan 18 '16 at 12:57
  • I think I see what you're saying. But why does the following code work: ` { \ errno = strncpy_s(ToVar, sizeof(ToVar), "short str", _TRUNCATE); \ if (errno == STRUNCATE) \ fprintf(stderr, "string '%s' was truncated to '%s'\n", "short str", ToVar);; \ }` and the macro doesn't expande to this code? – J. Toran Jan 18 '16 at 13:17
  • Thank you. I did not get the point of your post until I read the post of dbush. You and the others deserve credit for a correct answer but I couldn't do that. – J. Toran Jan 19 '16 at 12:27
3

The problem becomes obvious if you look at what happens at expansion. The macro already expans to a statement including trailing semicolon.

The problem becomes apparent if you're using it together with an if-else construct since if you append an extra semicolon it would expand into two statements.

For example if you do the following:

#define HELLO printf("Hello\n");

and then

if( some_condition )
    HELLO;
else
    something_else();

this would expand to

if( some_condition )
    printf("Hello\n");;
else
    something_else();

and you see that there's two statements between the if and else which means that the else becomes out of place.

The very reason of using the do - while(0) is to create something that would fit where one statement is to be fitted and then it's imperative that you leave out the trailing semicolon after the while(0). That is your define should look like this instead:

#define Cpy(ToVar, FrmVar) do   {                                                                                 \
                            errno = strncpy_s(ToVar, sizeof(ToVar), FrmVar, _TRUNCATE);                     \
                                if (errno == STRUNCATE)                                                     \
                                    fprintf(stderr, "string '%s' was truncated to '%s'\n", FrmVar, ToVar);  \
                        } while(0)
skyking
  • 13,817
  • 1
  • 35
  • 57
  • Thank you. But why don't the braces expand along with the other lines to `do { strncpy(etc) if (condition) printf(line) } while (0);;`? Isn't the last "`;`" just a no-op? Or not? – J. Toran Jan 18 '16 at 12:40
  • 1
    Yes, the last semicolon is what's called an empty statement. It count's as a statement in it's own right when parsing the source. – skyking Jan 18 '16 at 12:45
  • When I added a `;` at the end of the section where I copied the macro code, as in: `fprintf(stderr, "string '%s' was truncated to '%s'\n", "short str", ToVar);;` with 2 semi-colons, it compiled and ran okay. My question is why didn't the macro expand with the braces and the do-while? – J. Toran Jan 18 '16 at 12:59
  • Thank you for the addition to your answer. When I left the `;` off the while, and added it after the "call" to the macro, it compiled and ran okay. But I'm still not sure _why_ the macro didn't expand as I showed in my question. The code I copied directly from the macro worked okay. The extra section seems to be identical to what I thought the macro would expand to. (If you see what I mean.) – J. Toran Jan 18 '16 at 13:14
  • Thank you. I did not understand that the macro wasn't expanding like i thought it was until I read the post of dbush. You also deserve credit for helping me understand. – J. Toran Jan 19 '16 at 12:29
1

In a comment to Sourav's answer, you asked why this works as a macro body:

{ 
    errno = strncpy_s(ToVar, sizeof(ToVar), "short str", _TRUNCATE); 
    if (errno == STRUNCATE) 
        fprintf(stderr, "string '%s' was truncated to '%s'\n", "short str", ToVar);; 
}

That's because in your context, it evaluates to this:

if (Cnd != 'E')
{ 
    errno = strncpy_s(ToVar, sizeof(ToVar), "short str", _TRUNCATE); 
    if (errno == STRUNCATE) 
        fprintf(stderr, "string '%s' was truncated to '%s'\n", "short str", ToVar);; 
}
else
    Cpy(ToVar, "extra long string");    //  compiles with or without the semi-colon

The if part of the block, which previously was a single statement is now a block. So now the else is properly matched.

That's still not a good way to implement a macro, however. Then including the semicolon (which users of a macro would generally expect to work) would cause an error.

if (Cnd != 'E')
    Cpy(ToVar, "short string");     // invalid syntax
else
    Cpy(ToVar, "extra long string");

You keep saying "why didn't the macro expand the way I thought it would". It is expanding as you thought it would. Based on your original definition, this:

if (Cnd != 'E')
    Cpy(ToVar, "short string");
else
    Cpy(ToVar, "extra long string");    //  compiles with or without the semi-colon

Expands to this:

if (Cnd != 'E')
    do{
        errno = strncpy_s(ToVar, sizeof(ToVar), "short string", _TRUNCATE);
        if (errno == STRUNCATE)
            fprintf(stderr, "string '%s' was truncated to '%s'\n", "short string", ToVar);
    } while(0);;
else
    Cpy(ToVar, "extra long string");    //  compiles with or without the semi-colon

The first semicolon after the while(0) (which is part of the macro) ends the first part of the if statement. The second semicolon (not part of the macro) is another (empty) statement. This statement is considered to be outside of the if statement. Because of this, the else is misplaced.

You would get the same error if you had something like this:

if (Cnd != 'E')
    do{
        errno = strncpy_s(ToVar, sizeof(ToVar), "short string", _TRUNCATE);
        if (errno == STRUNCATE)
            fprintf(stderr, "string '%s' was truncated to '%s'\n", "short string", ToVar);
    } while(0);
fprintf(stderr, "this is outside of the if statement\n");
else
    Cpy(ToVar, "extra long string");    //  compiles with or without the semi-colon

Note that I indented the fprintf call to where is logically belongs. That's exactly where the empty statement (i.e. the extra semicolon) belongs as well.

dbush
  • 205,898
  • 23
  • 218
  • 273
  • Ah... Thank you. I see now where you expanded the macro to say `} while(0);;`. When I was expanding the macro in my head, I was putting the extra semi-colon after the `printf` instead of the `while`. This also explains why skyking's answer worked. – J. Toran Jan 18 '16 at 14:39