0

With the code:

#define MACRO(A,B) foo(A); bar(B)

if(true) {
    MACRO(A,B);
}

Astyle will remove the brackets around the macro call

if(true)
    MACRO(A,B);

Fortunately I found a solution. If I place the ; inside the macro, Astyle will understand it.

#define MACRO(A,B) foo(A); bar(B);

if(true) {
    MACRO(A,B)
}

Is it a good solution, is it a bug with Astyle or is my misunderstanding?

nowox
  • 25,978
  • 39
  • 143
  • 293
  • 1
    That's very bad style for the macro to begin with. It should probably use the `do { ... } while(false)` pattern (without a final `;`). – unwind Oct 31 '14 at 12:41
  • It's also a dubious style to have the brackets removed... – DevSolar Oct 31 '14 at 12:50
  • @DevSolar: It's a dubious style to clutter things with excessive brackets, comparisons, newlines, ... – Deduplicator Oct 31 '14 at 12:51
  • @Deduplicator: I remember we already disagreed on this matter in a different comment thread. Just stay away from my source, OK? – DevSolar Oct 31 '14 at 12:52
  • @DevSolar: I never changed your code (that I remember). And anyway, whatever style an existing code-base already uses should be consistently followed for new code. But if you say that it's bad style / dubious, you are asking for contradiction. – Deduplicator Oct 31 '14 at 12:55

2 Answers2

4

It's a bad style.

If you absolutely must have multiple statements in macro, wrap them in do while loop (note the lack of semicolon at the end):

#define MACRO(A,B) do { foo(A); bar(B); } while(0) 
user694733
  • 15,208
  • 2
  • 42
  • 68
1

Function-like macros are always bad style, for multiple reasons. Poor type safety, hard to read, hard to debug/maintain, incredibly bug-prone, opens up the program to various poorly-defined behavior and so on.

Control or loop statements without braces is dangerous style, because it makes the program vulnerable to a number of incredibly common bugs. Some people will argue and say that "no braces makes the code more readable" (I was once in this camp myself), then they'll skip off to write bugs because of this. No braces always leads to bugs, sooner or later.

And if you always use braces, you needn't use various obscure tricks when writing macros.

Good style:

void function (type A, type B)
{
  foo(A);
  bar(b)
}

if(true) 
{
  function(A, B);
}
Lundin
  • 195,001
  • 40
  • 254
  • 396
  • You **really** want to start a style-war here? – Deduplicator Oct 31 '14 at 12:52
  • If you are targeting only GCC, I wouldn't say that function-like macros are bad. The `typeof` extension together with a statement expression allows you to write a type-generic `MAX` macro, for example. – Blagovest Buyukliev Oct 31 '14 at 12:56
  • 1
    @Deduplicator This isn't style, this is bad and dangerous practice. It is not my subjective personal opinion, programming authorities such as MISRA-C can be taken as reference. It is a document solely concerned with program safety with no restrictions on coding style. Yet it bans function-like macros and it bans control statements without braces, for _safety reasons_. This in turn is indirectly based on actual scientific research: population studies have been made on source code to determine common causes for software defects. See MISRA-C:2012 directive 4.9 and rule 15.6. – Lundin Oct 31 '14 at 13:03
  • It certainly **is** style, but it's good to see that they have statistics to back their choice of always using blocks (Which is not to say that their reasons are applicable for code-snippets for example, which should be short). (And certainly, the preprocessor is always to be avoided where possible, and if one writes function-style macros, they damn better should behave like proper functions (Which is their downfall)). – Deduplicator Oct 31 '14 at 13:10
  • Still, your last sentence "And if you always use braces, you needn't use various obscure tricks when writing macros." is really bad advice to follow: Think defense-in-depth, or your previous argument looses all credibility. – Deduplicator Oct 31 '14 at 13:13
  • @Deduplicator Sure, if you make a function-like macro that is to be called from some external code, you need to take pre-caution. What I meant was that if you have discipline in your own coding and stick to a coding standard, then you don't have to write all these tricks in your own encapsulated code, to save you from yourself. – Lundin Oct 31 '14 at 13:53
  • @Lundin: You know that that argument about not needing to make your macros as safe as possible because you are your only consumer, where you for some reason do not simply avoid them completely, can be far easier applied to only using `{}` where needed? – Deduplicator Oct 31 '14 at 14:04
  • The problem with your solution is a performance issue. In this specific case the compiler will use a "call". The only solution to avoid it is to use an inline function which is not easy to use (have to be placed into a .h and not a .c) – nowox Oct 31 '14 at 14:05
  • 1
    @coin First of all, the function call overhead is a non-issue in 99% of the cases. Mind you, I mostly program performance-critical real-time embedded systems and the function call overhead is rarely an issue in _those_. Anyway, what you are speaking of is pre-mature optimization as it was done in the early 90s. Modern compilers use function inlining and are typically able to do so without the inline keyword provided. In modern programming on modern compilers, you should never replace functions with macros for the sake of performance. – Lundin Oct 31 '14 at 14:14
  • @Lundin I also program performance-critical real-time embedded systems and always have to struggle with that kind of issues. Most of our code is in assembly though. With an internal RAM of 1MBits and a 300k lines program, you can't easily accept the overhead due to multiple function calls. – nowox Oct 31 '14 at 14:22
  • 1
    @coin Assembler can't get optimized so you can't really compare it with C. An assembler programmer has to do _all_ optimization manually. And 1 MB of RAM is an _ocean_ of memory :) – Lundin Oct 31 '14 at 14:52
  • You're right, that's why I would like to use C a bit more. 1Mb of ram is not 1MB, this is 32kB. Your ocean is pretty small then :) – nowox Oct 31 '14 at 15:30