CODEWITHSUNDEEP
macrosc-preprocessor
dimba
dimba

Reputation: 27631

C preprocessor with if statement

I have the following macro:

#define IF_TRACE_ENABLED(level)  if (IsTraceEnabled(level))

The user code should look following:

IF_TRACE_ENABLED(LEVEL1)
{
    ... some very smart code
}

The emphasis here on curly brackets - I want to prevent "if" from macro to "eat" other code:

if (...)
   IF_TRACE_ENABLED(LEVEL1)
      printf(....);
else
   bla bla bla

In this example IF_TRACE_ENABLED "eats" else block.

Is there way to enforce user code not compile without curly brakes or there are other to define the macro to achieve the safety?

Upvotes: 6

Views: 4014

Answers (4)

Jeff Grills
Jeff Grills

Reputation: 61

If you just want to predicate the execution of a statement/block based on the evaluation of a macro, that dangling else is a risky latent bug that I've seen a lot of teams just live with.

The other solutions presented alter the structure of the code or are otherwise awkward in some fashion. The solution presented below isn't perfect - it has its own awkwardness, but hopefully it is more livable flavor of awkward.

One can generally transform a preprocessor macro's output from the simple but risky form of

if (expression)

to a for loop that will execute 0 or 1 times depending upon expression. As a loop, there's no risk of dangling else. This works like so:

for (bool _internal_once = true; _internal_once && (expression); _internal_once = false)

Note that expression could very well be a complex expression being text substituted, so we need the loop condition to be _internal_once && (expression) to both avoid operator precedence issues, and also guarantee expression is only evaluated once (due to C boolean short circuit logic).

The awkwardness of this solution is that is changes the break and continue behavior within predicated code, made worse because the loop is hidden and it is easy to forget. This has not yet been a problem in real world usage for me. We use these predicate macros to guard metrics, logging, telemetry, and other support code that doesn't often interact with an outer loop.

I have been bit by a real world problem in a previous incarnation of this loop. Never NEVER NEVER use a simple name for the loop control variable, I've seen it shadow another variable and cause the statement/block to see the wrong value.

This solution doesn't force the use of a block/braces on the client's code, but arguably since C and C++ don't, neither should you. It's the client's choice/policy.

One could argue you should generate a more unique name for the loop variable, concatenating ## the identifier with a suffix of __LINE__ or __COUNTER__ (for gnu). That feels excessive to me, the loop control variables are all local/auto and should shadow correctly.

You can even implement predicates like

  • DO_ONCE
    • Run only once
    • for (static bool _internal_once = true; _internal_once; _internal_once = false)
  • DO_EVERY_N(N)
    • Runs once per N iterations
  • DO_EVERY_MS(MS)
    • Won't run again until at least MS milliseconds has passed

Upvotes: 1

Michael Burr
Michael Burr

Reputation: 340436

This doesn't force the user of the macro to use braces, but it will prevent an else clause from being unintentionally eaten:

#define IF_TRACE_ENABLED(level)  if (!IsTraceEnabled(level)) {} else

A side note: braces around the printf() in the second example of the question wouldn't have fixed the problem - the else associated with bla bla bla would still be bound to the if statement in the macro.

Upvotes: 11

ptomato
ptomato

Reputation: 57920

You could try this:

#define IF_TRACE_ENABLED(level) do { if(IsTraceEnabled(level)) {
#define END_TRACE_ENABLED } } while(0);

I don't think there's any way to "enforce" good syntax from only the opening line of the macro. You will need to use two.

EDIT

I've added an extra pair of braces inside the macro to avoid all ambiguity.

In response to the comment, this macro is meant to be used like this:

IF_TRACE_ENABLED(LEVEL1)
    printf("Trace\n");
END_TRACE_ENABLED

Not as a statement. For the record, I think this is an abuse of the preprocessor and nobody should do this at all. What's wrong with just writing it out, bracketed with #ifdef DEBUG if necessary.

Upvotes: 2

user191776
user191776

Reputation:

This should work, but you'll have the pass the contents of the if block as an argument to the macro as well:

#define IF_TRACE_ENABLED(level,content)  { if (IsTraceEnabled(level)) {content} }

Upvotes: 0

Related Questions