6

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?

Improve this question
5
  • 2
    I don't see what that macro gives you over the bare if statement. Commented Sep 28, 2010 at 10:29
  • 2
    Forget about this example. You could have a complicated condition in that if that you don't want to repeat each time. Commented Sep 28, 2010 at 11:08
  • @JeremyP: there's not much utility in having a macro for this simple example, but as Nathan indicates it's possible that there might be more complexity in the debugging macro and/or that there are several variations of the macro based on build configuration (for example, a release version that always evaluates to false so the trace strings are stripped from the executable). Commented Sep 28, 2010 at 14:37
  • Well why not just have a macro for the conditional i.e. the boolean expression inside the if. Commented Sep 28, 2010 at 15:48
  • @JeremyP: there are many ways to get a similar result, and using a macro for the condition is one that I like (but you might be surprised how many people don't). Here's an article on my lame blog: blog.nth-element.com/?p=3. You're right that the macro in the question might not have a lot of reason to exist on its own in the simple form in the question. But, I've seen similar macros used in the wild, usually as one of a set of debug macros with different definitions depending on build config. I think that having a technique for dealing with the problem posed in the question has value. Commented Sep 28, 2010 at 16:27

4 Answers 4

Reset to default
11

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.

Improve this answer
Sign up to request clarification or add additional context in comments.

7 Comments

Trying and failing to come up with a way this could backfire. +1
Where is content of "if" block is passed?
in the else block; nice answer!
@dimba: The test has been inverted, so that the contents of the block are executed by the else.
@Michael Burr: there's not really a use case for this monstrosity in the first place. C has a perfectly good if control structure without creating new and broken ones.
|
2

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.

Improve this answer

5 Comments

The only thing I don't like in the solution is indentation of the block between stat and stop macros - your editor won't indent it.
is it not possible to simply use the braces, without do & while?
@ptmato - there's something in what @crypto says. do/while is used to enfore macro to look like normal function, by that that he's enforced to use ";" in the end. So ";" in the end of macro is redundant. Any way we don't ";" since the macro doesn't look like a function call
@dimba, this isn't meant to be used with a semicolon at the end. See edit.
@crypto, probably. I just thought this would be less likely to have a corner case where it didn't work.
1

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
Improve this answer

Comments

0

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} }
Improve this answer

5 Comments

This is not necessary printf(...). This could be any code that should be evaluated only when macro condition is true.
You should add a block around content.
@Ronny - this is what I want to enforce user, so if he forgets to do it, the compilation will fail
passing content to macro could be complicated and will influence user - we need be careful with commas in content not to be interpreted as macro argument separator, what if I want comment line to be part of comment.
@dimba it wouldn't fail this way

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.