> But the comment does not explain that this test has to be in that
> order, or the compiler will for non-constant arguments evalute
> the (now) right-side first. E.g. if I understand this correctly:
>
> + if (!(zero_is_valid) && unlikely((val) == 0.0)
>
> would have the same problem of evaluating "zero_is_valid" (which
> might be an isinf(arg1) || isinf(arg2)) first and so be the same thing
> we try to avoid with the macro? Maybe adding this bit of info to the
> comment makes it clearer?
Added.
> Also, a few places use the macro as:
>
> + CHECKFLOATVAL(result, true, true);
>
> which evaluates to a complete NOP in both cases. IMHO this could be
> replaced with a comment like:
>
> + // No CHECKFLOATVAL() needed, as both inf and 0.0 are valid
>
> (or something along the lines of "no error can occur"), as otherwise
> CHECKFLOATVAL() implies to the casual reader that there are some checks
> done, while in reality no real checks are done at all (and hopefully
> the compiler optimizes everything away, which might not be true for
> debug builds).
I don't know why those trigonometric functions don't check for
overflow/underflow like all the rest of float.c. I'll submit another
patch to make them error when overflow/underflow.
The new version is attached.
