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 are 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)
?
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.
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)
).
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.
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.
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.
The braces make sure the elements are taken in the right order. You do not want the compiler to do:
(hr) >= 0
convert result of 1. to HRESULT
.
Instead you want to convert only the (hr)
expression to HRESULT
, then compare that with 0
.
It's to ensure that hr is converted to an HRESULT before being compared, instead of HRESULT (hr >= 0).
To make sure that the expression that expands from (hr) is casted to HRESULT, and THEN compared to 0.