Would VS2008 c++ compiler optimize the following if statement?

Question

if (false == x) { ...}

as opposed to:

if (!x) { ... }

and

if (false == f1()) { ...}

as opposed to:

if (!f1()) { ... }

I think the if(false == ... version is more readable. Do you agree, or have another trick you can propose? Will it be just as fast? Thanks.

This is why I do not like !x:

if (25 == a->function1(12345, 6789) &&
    45 == b->function1(12345, 6789) &&
    !c->someOtherFunction(123)) { ... }

The following seems better:

if (25 == a->function1(12345, 6789) &&
    45 == b->function1(12345, 6789) &&
    false == c->someOtherFunction(123)) { ... }

Ugh, why don't you just look at the output of the compiler? It's not going to matter anyway in 99.9999% of the cases out there. — Ed S., Dec 16 '09 at 01:03
My code reviews fail due to speed waste, however insignificant. Don't ask to elaborate. This may sound dumb, but how do I see the output? The file may or may not compile with C++ CLI ... — Hamish Grubijan, Dec 16 '09 at 01:05
If you fail due to speed issues, either the speed requirement is unrealistic, or the problem has to be solved another way in 99% of cases. For example, `std::binary_search` is going to beat `std::find` by many many times. Even if this resulted in different code, it'd still not make a measurable difference in runtimes. — Billy ONeal, Dec 16 '09 at 01:10
@lpthnc - since you mention code reviews, you're working with a team. I would recommend using same style everyone else uses. Even if right now you find the style less readable, you will need to deal with it when reading code of your other team members. Furthermore, once you use a style with regularity, it becomes quite readable. — R Samuel Klatchko, Dec 16 '09 at 01:11
IIRC use the /FA option to see the generated assembly code. Don't forget do enable optimizations! — Bastien Léonard, Dec 16 '09 at 01:11
If you want to know what *your* compiler does, you need to study the compiler's output, rather than asking on SO. SO is useful for getting information from other programmers only. — jalf, Dec 16 '09 at 01:12
Just compile it with the C++ compiler and then run it under windbg and look at the generated assembly code. — Franci Penov, Dec 16 '09 at 01:13
Btw, if your code reviews are being failed due to speed waste, you should a) ask the code reviewer to substantiate his claims your code suffers from perf problems; and b) ensure your team has clear perf scenarios and corresponding tests to measure the bottlenecks instead of micro-optimizing random spots during code review. — Franci Penov, Dec 16 '09 at 01:16
Ew ew ew, why is your constant on the left? :P You don't say "If false is this", you say "If this is false". — GManNickG, Dec 16 '09 at 01:34
@Gman, that's an old trick to avoid accidentally using = instead of ==. I don't use it myself but I can see how it's useful. "int c = 0; if (c = 1) printf ("WTF!\n);" would be caught if you used "if (1 = c)". — paxdiablo, Dec 16 '09 at 01:43
I know :P I don't think the unreadability of it is better than the very rare typo that a compiler will warn you about anyway. — GManNickG, Dec 16 '09 at 01:51

Billy ONeal · Answer 1 · 2009-12-16T01:04:56.227

7

I personally find the form if (!x) { ... } most readable, but nowadays they're all going to result in the same code.

EDIT: In your example, I'd do the following:

if (  a->function1(12345, 6789) == 25 &&
      b->function1(12345, 6789) == 45 &&
    !(c->someOtherFunction(123))) { ... }

But it's really just a personal preference thing. Do what's most readable to you. It's not going to make any difference.

edited Dec 16 '09 at 01:04

answered Dec 16 '09 at 00:54

Billy ONeal

104,103
58
317
552

8

+1. The "if (n == true)" paradigm is a hideous monstrosity :-) – paxdiablo Dec 16 '09 at 00:57
1

these days a compiler will take care of all this, there are far better things to do than worry about the speed of a line of code based purely on its syntax – Tom J Nowell Dec 16 '09 at 01:00
Thanks. Please check the revised question in regards to readability. – Hamish Grubijan Dec 16 '09 at 01:02

score 7 · Answer 2 · answered Dec 16 '09 at 01:16

I think the if(false == ... version is more readable. Do you agree, or have another trick you can propose?

You're doing it wrong.

false == x returns a bool, which obviously has to be compared to true!

if (true == (false == x)) { ...} would be better. But that again returns a bool, so just to be on the safe side, and make your code more readable, we'd better do this:

if (true == (true == (false == x))) { ...}. And so on. Once you start comparing booleans to booleans, where do you stop? The result will always be another boolean, and to be consistent, you have to compare that to a boolean.

Or you could learn to understand the language you're working in, and use if (!x) { ...}, which expresses exactly what you wanted.

Which do you really think is more readable?

if (false == x) { ...} translates to "If it is true that false is equal to x".
if (!x) { ...} translates to "if x is not true".

Can you honestly, seriously, really say that the first form is "more readable"? Would you ever say that in a sentence? "If it is true that false is equal to x"?

It is not more readable, it just shouts to any programmer reading your code that "I don't understand if statements or boolean values".

+1 for the reductio ad absurdum (crikey, I hope I spelled that correctly), especially the logical endpoint: if it is true that it is true that it is true ... that it is true that x is false. — paxdiablo, Dec 16 '09 at 01:41
Cute argument. +1 It is all about the ! character which is barely visible. Just as ' is bad character to use as a comment, ! is a bad character to use for "not". In Python I can write: if not k in dict. I wish C++ was just as readable. — Hamish Grubijan, Dec 16 '09 at 03:56
I've never found it unnoticeable. You can separate it with spaces if you like (`if ( ! x)`). Or use the `not` keyword instead of `!`. It's not commonly used (so it might confuse some programmers reading your code), but it's a part of standard C++. — jalf, Dec 16 '09 at 04:07

score 4 · Answer 3 · answered Dec 16 '09 at 01:49

4

C++ reserves the following keywords on the left as alternatives to the operators on the right:

and  and_eq  bitand    &&  &=  &
or   or_eq   bitor     ||  |=  |
     xor_eq  xor           ^=  ^
not  not_eq  compl     !   !=  ~

If you're worried about the ! getting lost, not stands out more.

if (    a->function1(12345, 6789) == 25 and
        b->function1(12345, 6789) == 45 and
        not c->someOtherFunction(123)) {
    ...
}

answered Dec 16 '09 at 01:49

ephemient

198,619
38
280
391

1

Read the C++ language specification, or a good C++ book: for example, http://books.google.com/books?id=Q4iP1mkfdtsC&pg=PT325&ei=12YoS9_UB4GGzgTcyvyWCw – ephemient Dec 16 '09 at 04:53

score 3 · Accepted Answer · answered Dec 16 '09 at 01:09

3

A good compiler should generate the same code for both code blocks.

However, instead of worrying about false == f1() vs. !f1(), you should be way more worried about the short-circuit evaluation in this example:

if (25 == a->function1(12345, 6789) &&
    45 == b->function1(12345, 6789) &&
    !c->someOtherFunction(123)) { ... }

Certain compilers will generate code that will skip the execution of b->function1() and c->someOtherFunction(), if a->function1() call happens to evaluate to something different than 25 - the reason being, the compiler already knows the outcome of the whole if () statement at that point, so it can jump at the right place.

If your code depends on a state being modified by any of the skipped functions, you might get nasty surprises.

answered Dec 16 '09 at 01:09

Franci Penov

74,861
18
132
169

5

Actually, the C++ standard mandates that the && operator short circuit. – Billy ONeal Dec 16 '09 at 01:11
Thanks. About short-circuiting ... I have not met any paid programmers who do not have this knowledge ingrained. Have you? – Hamish Grubijan Dec 16 '09 at 01:16
1

Billy: +1; however, only if a built-in `&&` is used, if you've overloaded it for a UDT then it MUST NOT short circuit. (Yes, it's almost always 100% evil to do that, for exactly this reason.) – Dec 16 '09 at 01:19
Lol, so why do you have that sample code in there if you know you shouldn't be doing this? – Franci Penov Dec 16 '09 at 01:29
Should not be doing what? I consider short-circuiting a relatively safe code. Creating functions which return an int and modify something - that is BAD. – Hamish Grubijan Dec 16 '09 at 03:59
1

@Ipthns - of course, you kind of forget that a year from now the new college graduate will go and change that function to update some internal state and half the tests will fail. :-) using a function in a short-circuiting code puts a non-intuitive and hard to discover requirement on that function. meanwhile, here are some examples of legitimate functions that return a value that needs to be checked and modify state - InterlockExchange(), AddRef()/Release(), LoadLibrary() – Franci Penov Dec 16 '09 at 04:09
True ... something about the following code does not make sense: if (LoadLibrary() && a == 5) or: if (LoadLibrary() && CheckLock() { ... } I cannot give a proper name to the reason why it is BAD, it just is. Now that you mentioned it, I have seen bad short-circuiting before. – Hamish Grubijan Dec 16 '09 at 04:32
Designing something to have externally observable (logically, not perf) side-effects AND return a value is probably not good to begin with, and shouldn't survive a code review. Low code cohesion leads to difficulty understanding the code and, in this example, bugs. If it is for perf reasons, profile first, name the method something obvious and nasty, and wash your hands twice. – Merlyn Morgan-Graham Dec 17 '09 at 03:34
@Merlyn - by that logic, the ++ and -- operators should not exist, as they have externally observable side-effect AND return value. Also, any-non-const (state-modifying) class method can be only void. – Franci Penov Dec 17 '09 at 03:49

score 2 · Answer 5 · edited May 23 '17 at 11:48

This is a case of premature optimization (When is optimisation premature?). But the compiler will generate the same code (MSVC 2008 Debug mode):

if (!bVal)
    bVal = true;

if (bVal == false)
    bVal = true;

//translates to

; 70   :         if (!bVal)

cmp BYTE PTR $T5793[ebp], 0
jne SHORT $LN9@wmain
push    OFFSET $LN10@wmain
call    __RTC_UninitUse
add esp, 4
$LN9@wmain:
movzx   eax, BYTE PTR _bVal$[ebp]
test    eax, eax
jne SHORT $LN2@wmain

; 71   :             bVal = true;

mov BYTE PTR $T5793[ebp], 1
mov BYTE PTR _bVal$[ebp], 1
$LN2@wmain:

; 72   :         
; 73   :         if (bVal == false)

cmp BYTE PTR $T5793[ebp], 0
jne SHORT $LN11@wmain
push    OFFSET $LN10@wmain
call    __RTC_UninitUse
add esp, 4
$LN11@wmain:
movzx   eax, BYTE PTR _bVal$[ebp]
test    eax, eax
jne SHORT $LN1@wmain

; 74   :             bVal = true;

mov BYTE PTR $T5793[ebp], 1
mov BYTE PTR _bVal$[ebp], 1

score 1 · Answer 6 · answered Dec 16 '09 at 00:57

Have you profiled and found evaluating the condition to be a hot spot? If not, then do whatever your coding standard requires.

Both should compile to the same code anyway. You can check by inspecting the assembly code generated by the compiler for both cases.

Would VS2008 c++ compiler optimize the following if statement?

6 Answers6

Linked