Hi,
On 2020-02-12 13:15:22 -0500, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > I'd just rename the macro to the name of the inline function. No need to
> > have a verbose change in all callsites just to update the name imo.
>
> +1, that's what I had in mind too. That does suggest though that we
> ought to make sure the macro has single-eval behavior, so that you
> don't need to know it's a macro.
We'd have to store 'val' in a local variable for that I think. Not the
prettiest, but also not a problem.
I do wonder if we're just punching ourselves in the face with the
signature of these checks. Part of the problem here really comes from
using the same function to handle a number of different checks.
I mean something like dtof's
check_float4_val((float4) num, isinf(num), num == 0);
where the num == 0 is solely to satisfy the check function is a bit
stupid.
And the reason we have these isinf(arg1) || isinf(arg2) parameters is
also largely because we force the same function to be used in cases
where we have two inputs, rather than just one.
For most places it'd probably end up being easier to read and to
optimize if we just wrote them as
if (unlikely(isinf(result)) && !isinf(arg))
float_overflow_error();
and when needed added a
else if (unlikely(result == 0) && arg1 != 0.0)
float_underflow_error();
the verbose piece really is the error, not the error check. Sure, there
are more complicated cases like
if (unlikely(isinf(result)) && (!isinf(arg1) || !isinf(arg2)))
but that's still not very complicated.
Greetings,
Andres Freund
