[PATCH] Remove some duplicate if conditions

David Rowley <dgrowleyml@gmail.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.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

David Rowley <dgrowleyml@gmail.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