On Fri, Jan 3, 2014 at 6:59 AM, Tom Lane <t...@sss.pgh.pa.us> wrote: > David Rowley <dgrowle...@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
