How to prevent shadowing with iterator macros in C

Question

Here's an example of a macro that wraps iterator functions in C,

Macro definition:

/* helper macros for iterating over tree types */
#define NODE_TREE_TYPES_BEGIN(ntype) \
{ \
    GHashIterator *__node_tree_type_iter__ = ntreeTypeGetIterator(); \
    for (; !BLI_ghashIterator_done(__node_tree_type_iter__); BLI_ghashIterator_step(__node_tree_type_iter__)) { \
        bNodeTreeType *ntype = BLI_ghashIterator_getValue(__node_tree_type_iter__);
#define NODE_TREE_TYPES_END \
    } \
    BLI_ghashIterator_free(__node_tree_type_iter__); \
} (void)0

Example use:

NODE_TREE_TYPES_BEGIN(nt)
{
    if (nt->ext.free) {
        nt->ext.free(nt->ext.data);
    }
}
NODE_TREE_TYPES_END;

However nested use (while functional), causes shadowing (gcc's -Wshadow)

NODE_TREE_TYPES_BEGIN(nt_a)
{
    NODE_TREE_TYPES_BEGIN(nt_b)
    {
        /* do something */
    }
    NODE_TREE_TYPES_END;
}
NODE_TREE_TYPES_END;

The only way I can think of to avoid this is to pass a unique identifier to NODE_TREE_TYPES_BEGIN and NODE_TREE_TYPES_END. So my question is...

Is there there a way to prevent shadowing if variables declared within an iterator macro when its scope is nested?

This question boils down to creating a unique symbol in the pre-processor. Already answered here http://stackoverflow.com/questions/1132751/how-can-i-generate-unique-values-in-the-c-preprocessor — David-SkyMesh, Sep 13 '13 at 04:11
@David-SkyMesh, there is a difference - wrapping a loop means the unique symbol has to be referenced again by a terminating iterator at the same scope. — ideasman42, Sep 13 '13 at 04:12
@Aad, I dont see how an inline function would help, the macro still has to wrap a `for/while` loop and it needs a storage variable, so how would using an inline function solve this? — ideasman42, Sep 14 '13 at 04:14

Alex Celeste · Answer 1 · 2014-07-31T22:27:57.647

You don't need to insert the same unique identifier in two places, if you can restructure the block so that it never needs the second macro to close it - then you only have one macro invocation and can use simple solutions like __LINE__ or __COUNTER__.

You can restructure the block by taking further advantage of for, to insert operations intended to happen after the block, in a position textually before it:

#define NODE_TREE_TYPES(ntype) \
    for (GHashIterator *__node_tree_type_iter__ = ntreeTypeGetIterator(); \
         __node_tree_type_iter__; \
         (BLI_ghashIterator_free(__node_tree_type_iter__), __node_tree_type_iter__ = NULL)) \
        for (bNodeTreeType *ntype = NULL; \
             (ntype = BLI_ghashIterator_getValue(__node_tree_type_iter__), !BLI_ghashIterator_done(__node_tree_type_iter__)); \
             BLI_ghashIterator_step(__node_tree_type_iter__))

The outer level of your original macro pairs is a compound statement, containing exactly three things: a declaration+initialization, an enclosed for structure, and a single free operation after which the declared variable is not used again.

This makes it very easy to restructure as a for of its own instead of an explicit compound statement: the declaration+initialization goes in the first clause of the for (wouldn't be as easy if you'd had two variables, although it is still possible); the enclosed for can be placed after the end of the for header we're building, since it's a single statement; and the free operation is placed in the third clause. Since the variable is not used in any further statements, we can take advantage of it: combine the free with an explicit assignment of NULL, using the comma operator, and then make the middle clause a check that the variable is not NULL, ensuring the loop runs exactly once.

The nested for gets a similar but more minor modification. Its statement body contains a declaration and per-loop initialization, but we can still hoist this out; put the declaration in the unused first clause of the for (which will still put it in the new scope), and initialize it in the second clause so that it happens at the start of every iteration; combine that initialization with the actual test using the comma operator again. This removes all boilerplate from the statement block and therefore means you no longer have any braces, and thus no need for a second macro to close the braces.

Then you have a single macro invocation you can use like this:

NODE_TREE_TYPES (nt) {
    if (nt->ext.free) {
        nt->ext.free(nt->ext.data);
    }
}

(you can then apply the generation of a unique identifier to this to get rid of shadowing easily, using techniques shown in other questions)

Is this ugly? Does abusing the for statement and comma operator make the average C programmer's skin crawl? Oh lord yes. BUT, it's a bit cleaner, and it's the arguable "right" way to mess about if you really have to mess about.

Having a "close" macro that inserts compound-statement-breaks or hides close braces is a much worse idea, because not only does it give you problems with identifiers and matching scope, but it also hides the block structure of the program from the reader; abuse of the for statement at least means that the block structure of the program, and variable scope and so on, is not mutilated as well.

How to prevent shadowing with iterator macros in C

1 Answers1