views:

273

answers:

9

Windows SDK features SUCCEEDED macro:

#define SUCCEEDED(hr) (((HRESULT)(hr)) >= 0)
-----------------------^-------------^-----

clearly as with other macros there're parentheses to ensure right interpretation of the intent by compiler.

What I don't get is why there're parentheses around (HRESULT)(hr) (I marked them with ^ character). hr is parenthesized so that some complex construct can be there, HRESULT is parenthesized to form a C-style cast, then the whole >= construct is parenthesized as well, so why the extra pair of parentheses around (HRESULT)(hr)?

+2  A: 

The braces make sure the elements are taken in the right order. You do not want the compiler to do:

  1. (hr) >= 0

  2. convert result of 1. to HRESULT.

Instead you want to convert only the (hr) expression to HRESULT, then compare that with 0.

utnapistim
Sure, but that's what C and C++ operator precedence does anyway, and most people realize that.
David Thornley
+1  A: 

It's to ensure that hr is converted to an HRESULT before being compared, instead of HRESULT (hr >= 0).

DeadMG
The precedence rules ensure that; the parentheses just make it clear to the reader that that's the case.
Mike Seymour
A: 

To make sure that the expression that expands from (hr) is casted to HRESULT, and THEN compared to 0.

fingerprint211b
A: 

Because the author wasn't sure whether casting or relational operators had more precedence.

ninjalj
+3  A: 

The C standard puts the cast at a higher precedence than the comparison, so the parens are not required for the complier.

However, people read the macro definition, and putting them in makes the precedence explicit, so that it is obvious to people reading it that it is the result of comparing ((HRESULT)hr) with zero rather than casting the result of the comparison without having to think about the precedence.

Pete Kirkham
+5  A: 

The extra parentheses don't change the meaning, but they do make the precedence explicit. Without them, a reader who didn't know all the precedence rules would have to work out what the expression meant.

To clarify, I'm only referring to the parentheses around the cast expression, as marked in the question. The others are necessary (unless the macro were only intended for C++ and not C, in which case you could change the (HRESULT)(hr) to HRESULT(hr)).

Mike Seymour
-1, not true. See the example in my reply: The meaning may change depending on the arguments passed, as macros are jsut text replacements.
peterchen
@peterchen: in this case, the argument is enclosed in parentheses, so the meaning is the same for any valid expression, with or without any extra parentheses around the cast expression. Macros certainly can have pitfalls, but not this one. Unless you can provide an example of an argument which would break it.
Mike Seymour
You are right, mike, I ignored the "funny line" below his text. Can't revoke the -1 unless you edit, though.
peterchen
+3  A: 

Short answer: who knows what the MS developers were thinking. However, the parentheses around hr is obviously necessary since hr could be an expression consisting of more than a single operand. The parentheses around ((HRESULT)(hr)) is unnecessary as far as I can see. This was probably just done out of a cautionary habit: when working with the preprocessor, it's better to have too many parentheses than too few.

+4  A: 

This is to cope with macro shortcomings - they are just a text replacement!

imagine the following macro

#define DOUBLE(x) x*2

int v = DOUBLE(1+1); // == 1+1*2 == 3. Did you expect 4?

So the rules of thumb for macros are:

  • use them only if there's no other way to solve your problem
  • wrap every parameter in parantheses (to avoid above problem)
  • wrap the entire expression in parantheses (to avoid other, similar problems)
  • make every parameter only occur once (to avoid problems with side effects)

So, a better macro would be:

#define DOUBLE(x) ((x)*2)

You are almost there, the remaining parantheses in you example are due to the cast.


So we can criticise two points:

Q: why is it a macro, not an inline function?
A: Backward compatibility. C doesn't have inline functions (or at least didn't), using functions for the probably thousands of such declarations would have brought down most compilers of that time.

Q: Are the parantheses really required for this specific macro? A: I don't know. It would probably take me half an hour or more to formally proof (or disproof) there is no sensible parameter and "call" environment where this has unintended effects. I'd rather follow sensible guidelines as mentioned above, and go on coding.

peterchen
Your first example is missing the definition of `DOUBLE`…
Donal Fellows
This is good general advice, but not relevant to the question; the argument to SUCCEEDED is wrapped in parentheses as it should be, as is the entire expression. The question is about the superfluous parantheses around the cast expression.
Mike Seymour
@Donal: huh? it's the first line..
peterchen
@peter: Check the time when I commented and the times in the edit history; Edmund fixed it for you. :-)
Donal Fellows
@Donal - it was there I swear! - Thanks for pointing out.
peterchen
+2  A: 

The author was just stupid. All the extra parentheses (around the cast) do is make it harder to visually parse. Anyone who thinks comparison might possibly have a higher precedence than casting needs to learn the basics of the language before coding... not to mention just get some common sense.

R..