Re: [PATCH] Remove some duplicate if conditions

On Fri, Jan 3, 2014 at 6:59 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> David Rowley <dgrowleyml(at)gmail(dot)com> writes:
> > I've attached a simple patch which removes some duplicate if conditions
> > that seemed to have found their way into the code.
>
> > These are per PVS-Studio's warnings.
>
> -1. If PVS-Studio is complaining about this type of coding, to hell with
> it; it should just optimize away the extra tests and be quiet about it,
> not opinionate about coding style. What you propose would cause logically
> independent pieces of code to become tied together, and I don't find that
> to be an improvement. It's the compiler's job to micro-optimize where
> possible, and most compilers that I've seen will do that rather than tell
> the programmer he ought to do it.
>
>
I had imagined that PVS-Studio would have highlighted these due to it
thinking that it may be a possible bug rather than a chance for
optimisation. Here's some real world examples of bugs it has found:
http://www.viva64.com/en/examples/v581/
I understand that the compiler will likely optimise some of these cases,
most likely I would guess cases that test simple local variables. I've not
tested any compiler, but I imagined that the duplicate check
in fe-protocol3.c would be the less likely to be optimized as it may be
hard for the compiler to determine that conn->verbosity can't be changed
from within say, appendPQExpBuffer. I sent the patch in because after
looking at the fe-protocol3.c case I just found it weird and I had to spend
a bit of time to determine that the code was not meant to check
conn->verbosity against some other constant. This case just seems plain
weird to be a duplicate if condition to me.

The other case that's in the patch I don't really care so much for either,
I only added it since I had already written the fix for the fe-protocol3.c
one.

Regards

David Rowley