Re: pgsql: Use new overflow aware integer operations.

[ back from Christmas break ]

Andres Freund <andres(at)anarazel(dot)de> writes:
> On December 22, 2017 7:52:54 PM GMT+01:00, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> I will not accept an implementation that spews compiler warnings
>> all over the place, which is what this one is doing. Please fix that,
>> or else I will.

> Are you seriously implying that I'm suggesting that we live with a warning / that I refuse to fix one? All I was saying is that I don't want to exactly define which value *result is set to in case of overflow. Without having resolved the discussion of semantics it just seemed pointless to start fixing...

Sorry, I was being unnecessarily grumpy there. I follow your point about
not wanting to constrain the implementation to yield the correct low-order
bits if we don't actually need that behavior ... but I'm still not happy
about the warnings.

What do you think of having the fallback code explicitly set the output
variable to zero (or any other fixed value) on overflow, like

#if defined(HAVE__BUILTIN_OP_OVERFLOW)
return __builtin_add_overflow(a, b, result);
#else
int32 res = (int32) a + (int32) b;

if (res > PG_INT16_MAX || res < PG_INT16_MIN)
+ {
+ *result = 0; /* just to keep compiler quiet */
return true;
+ }
*result = (int16) res;
return false;
#endif

I do not think this would cause any performance loss in our expected
usage, because reasonably bright compilers would detect that the store
is dead code and remove it. But less-bright compilers would not be
issuing warnings.

regards, tom lane