Re: Bug in example code for isnan in autoconf info

[Eric Blake]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

According to David Cournapeau on 9/20/2008 9:53 AM:

Hi David, and thanks for the report.

>     In section 5.5.1 (portability of C functions), there is a mention of
> isnan and isinf. A portable replacement for isinf is suggested when not
> available. Unfortunately, the function is wrong on two important aspects:
> 
>     - isnan(x-x) returns true for x being inf and Nan. But isinf(NaN) is
> false

Indeed, a correct definition of isinf must first filter out NaN.  I'm
installing the patch below:

>     - also, some compilers optimize the x-x to 0, this making it fail too.

Ah, but we already mention that fact in the next paragraph, and declare
that such compilers are likely to cause other problems in floating point,
such that this is not the least of your worries.

> For 2, I would suggest using ((x) + (-x)) instead of ((x)-(x)). I am not
> really familiar with what IEEE-754 permits, but ((x) + (-x)) seems to
> work much more often than ((x)-(x)) for compilers which optimize
> aggressively.

There is no difference in those two expressions for an IEEE-754-complaint
implementation.  Again, if you are worried about floating point corner
cases, then don't use a compiler that incorrectly "optimizes" floating
point on your behalf.  And if you are worried about speed (for example,
with gcc's -ffast-math option), don't expect floating point corner cases
to be reliable.

More importantly, there are two common bugs even among many
implementations that provide isnan, which are both fixed by gnulib.  One
is that some implementations only provide a working isnan if you link with
- -lm, which can be overkill for some uses of NaN (think of a printf
replacement, which does not otherwise need -lm).  The other is that many
x86 implementations fail to recognize the x86 'pseudo-NaN' format, leading
to surprising results if you use a union to generate bit patterns that are
not valid long doubles via normal arithmetic operations (yes, such code is
undefined according to C, but on the other hand, od used to core dump if
you asked it to print such bit patterns as long double).

- --
Don't work too hard, make some time for fun as well!

Eric Blake             address@hidden
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkjVeBAACgkQ84KuGfSFAYARuACgyk2pTvrgNHOG3w90QJPk5+fm
tOAAn2vx5v8xsYGkxmcb2KguTW2pbzhM
=c7w1
-----END PGP SIGNATURE-----

>From b83de7e880bfb22ae4d0b4d2a9fcd46bdfcc8b95 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Sat, 20 Sep 2008 16:19:40 -0600
Subject: [PATCH] Fix sample isinf definition.

* doc/autoconf.texi (Function Portability) <isinf>: Filter out NaN
first.
* THANKS: Update.
Reported by David Cournapeau.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog         |    8 ++++++++
 THANKS            |    1 +
 doc/autoconf.texi |   17 +++++++++++------
 3 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index f0feca6..c277148 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2008-09-20  Eric Blake  <address@hidden>
+
+       Fix sample isinf definition.
+       * doc/autoconf.texi (Function Portability) <isinf>: Filter out NaN
+       first.
+       * THANKS: Update.
+       Reported by David Cournapeau.
+
 2008-09-16  Eric Blake  <address@hidden>
 
        Fix Erlang regression, introduced 2006-11-17.
diff --git a/THANKS b/THANKS
index f39e949..366bf62 100644
diff --git a/doc/autoconf.texi b/doc/autoconf.texi
index 2b1d165..c6a7f3f 100644
--- a/doc/autoconf.texi
+++ b/doc/autoconf.texi
@@ -4318,7 +4318,9 @@ Function Portability
 @code{isinf} incorrectly returns true for a finite @code{long double}
 argument that is outside the range of @code{double}.
 
-To work around this porting mess, you can use code like the following.
+The best workaround for these issues is to use gnulib modules
address@hidden and @code{isnan} (@pxref{Gnulib}).  But a lighter weight
+solution involves code like the following.
 
 @smallexample
 #include <math.h>
@@ -4338,17 +4340,20 @@ Function Portability
     (sizeof (x) == sizeof (long double) ? isinf_ld (x) \
      : sizeof (x) == sizeof (double) ? isinf_d (x) \
      : isinf_f (x))
-static inline int isinf_f  (float       x) @{ return isnan (x - x); @}
-static inline int isinf_d  (double      x) @{ return isnan (x - x); @}
-static inline int isinf_ld (long double x) @{ return isnan (x - x); @}
+static inline int isinf_f  (float       x)
address@hidden return !isnan (x) && isnan (x - x); @}
+static inline int isinf_d  (double      x)
address@hidden return !isnan (x) && isnan (x - x); @}
+static inline int isinf_ld (long double x)
address@hidden return !isnan (x) && isnan (x - x); @}
 #endif
 @end smallexample
 
 Use @code{AC_C_INLINE} (@pxref{C Compiler}) so that this code works on
 compilers that lack the @code{inline} keyword.  Some optimizing
 compilers mishandle these definitions, but systems with that bug
-typically have missing or broken @code{isnan} functions anyway, so it's
-probably not worth worrying about.
+typically have many other floating point corner-case compliance problems
+anyway, so it's probably not worth worrying about.
 
 @item @code{malloc}
 @c @fuindex malloc
-- 
1.6.0.2