Fix for _Complex_I problems

Fix for _Complex_I problems

[Neil Jerram]

[-- Attachment #1: Type: text/plain, Size: 364 bytes --]

You've both recently reported a compilation problem with _Complex_I,
compiling with GCC on Solaris and AIX.

My proposed fix for this is attached; please can you review and let me
know if you have any comments?

(Should I also attach the regenerated ./configure, so you can run that
and check that it gives the expected result on your OSs?)

Thanks,
        Neil


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: complex.patch --]
[-- Type: text/x-diff, Size: 3370 bytes --]

Index: configure.in
===================================================================
RCS file: /cvsroot/guile/guile/guile-core/configure.in,v
retrieving revision 1.268.2.37
diff -u -r1.268.2.37 configure.in
--- configure.in	6 Feb 2008 22:27:59 -0000	1.268.2.37
+++ configure.in	9 Feb 2008 18:43:21 -0000
@@ -715,6 +716,30 @@
   [AC_DEFINE(HAVE_CRYPT,1,
              [Define to 1 if you have the `crypt' function.])])
 
+# When compiling with GCC on some OSs (Solaris, AIX), _Complex_I doesn't work;
+# in the reported cases so far, 1.0fi works well instead.
+if test "$ac_cv_type_complex_double" = yes; then
+  AC_MSG_CHECKING([for i])
+  AC_TRY_COMPILE([
+#if HAVE_COMPLEX_H
+#include <complex.h>
+#endif
+complex double z;
+],[
+z = _Complex_I;
+],[AC_DEFINE(GUILE_I,_Complex_I,[The imaginary unit (positive square root of -1).])
+   AC_MSG_RESULT([_Complex_I])],[AC_TRY_COMPILE([
+#if HAVE_COMPLEX_H
+#include <complex.h>
+#endif
+complex double z;
+],[
+z = 1.0fi;
+],[AC_DEFINE(GUILE_I,1.0fi)
+   AC_MSG_RESULT([1.0fi])],[ac_cv_type_complex_double=no
+                            AC_MSG_RESULT([not available])])])
+fi
+
 # glibc 2.3.6 (circa 2006) and various prior versions had a bug where
 # csqrt(-i) returned a negative real part, when it should be positive
 # for the principal root.
Index: libguile/numbers.c
===================================================================
RCS file: /cvsroot/guile/guile/guile-core/libguile/numbers.c,v
retrieving revision 1.281.2.12
diff -u -r1.281.2.12 numbers.c
--- libguile/numbers.c	6 Feb 2008 13:17:49 -0000	1.281.2.12
+++ libguile/numbers.c	9 Feb 2008 18:43:23 -0000
@@ -162,11 +162,12 @@
 #endif
 }
 
-
+#if defined (GUILE_I)
 /* For an SCM object Z which is a complex number (ie. satisfies
    SCM_COMPLEXP), return its value as a C level "complex double". */
 #define SCM_COMPLEX_VALUE(z)                                    \
-  (SCM_COMPLEX_REAL (z) + _Complex_I * SCM_COMPLEX_IMAG (z))
+  (SCM_COMPLEX_REAL (z) + GUILE_I * SCM_COMPLEX_IMAG (z))
+#endif
 
 /* Convert a C "complex double" to an SCM value. */
 #if HAVE_COMPLEX_DOUBLE
@@ -6011,7 +6012,7 @@
 {
   if (SCM_COMPLEXP (z))
     {
-#if HAVE_COMPLEX_DOUBLE && HAVE_CLOG
+#if HAVE_COMPLEX_DOUBLE && HAVE_CLOG && defined (GUILE_I)
       return scm_from_complex_double (clog (SCM_COMPLEX_VALUE (z)));
 #else
       double re = SCM_COMPLEX_REAL (z);
@@ -6045,7 +6046,7 @@
       /* Mingw has clog() but not clog10().  (Maybe it'd be worth using
          clog() and a multiply by M_LOG10E, rather than the fallback
          log10+hypot+atan2.)  */
-#if HAVE_COMPLEX_DOUBLE && HAVE_CLOG10
+#if HAVE_COMPLEX_DOUBLE && HAVE_CLOG10 && defined (GUILE_I)
       return scm_from_complex_double (clog10 (SCM_COMPLEX_VALUE (z)));
 #else
       double re = SCM_COMPLEX_REAL (z);
@@ -6077,7 +6078,7 @@
 {
   if (SCM_COMPLEXP (z))
     {
-#if HAVE_COMPLEX_DOUBLE && HAVE_CEXP
+#if HAVE_COMPLEX_DOUBLE && HAVE_CEXP && defined (GUILE_I)
       return scm_from_complex_double (cexp (SCM_COMPLEX_VALUE (z)));
 #else
       return scm_c_make_polar (exp (SCM_COMPLEX_REAL (z)),
@@ -6111,7 +6112,7 @@
 {
   if (SCM_COMPLEXP (x))
     {
-#if HAVE_COMPLEX_DOUBLE && HAVE_USABLE_CSQRT
+#if HAVE_COMPLEX_DOUBLE && HAVE_USABLE_CSQRT && defined (GUILE_I)
       return scm_from_complex_double (csqrt (SCM_COMPLEX_VALUE (x)));
 #else
       double re = SCM_COMPLEX_REAL (x);

Re: Fix for _Complex_I problems

[Neil Jerram]

[-- Attachment #1: Type: text/plain, Size: 551 bytes --]

Neil Jerram <neil@ossau.uklinux.net> writes:

> You've both recently reported a compilation problem with _Complex_I,
> compiling with GCC on Solaris and AIX.
>
> My proposed fix for this is attached; please can you review and let me
> know if you have any comments?

Actually I'd like to make a small change to that; please refer to the
new attached patch instead of the previous one.

> (Should I also attach the regenerated ./configure, so you can run that
> and check that it gives the expected result on your OSs?)

(This question still stands.)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: complex.patch --]
[-- Type: text/x-diff, Size: 3410 bytes --]

Index: configure.in
===================================================================
RCS file: /cvsroot/guile/guile/guile-core/configure.in,v
retrieving revision 1.268.2.37
diff -u -r1.268.2.37 configure.in
--- configure.in	6 Feb 2008 22:27:59 -0000	1.268.2.37
+++ configure.in	9 Feb 2008 19:00:07 -0000
@@ -715,6 +716,30 @@
   [AC_DEFINE(HAVE_CRYPT,1,
              [Define to 1 if you have the `crypt' function.])])
 
+# When compiling with GCC on some OSs (Solaris, AIX), _Complex_I doesn't work;
+# in the reported cases so far, 1.0fi works well instead.
+if test "$ac_cv_type_complex_double" = yes; then
+  AC_MSG_CHECKING([for i])
+  AC_TRY_COMPILE([
+#if HAVE_COMPLEX_H
+#include <complex.h>
+#endif
+complex double z;
+],[
+z = _Complex_I;
+],[AC_DEFINE(GUILE_I,_Complex_I,[The imaginary unit (positive square root of -1).])
+   AC_MSG_RESULT([_Complex_I])],[AC_TRY_COMPILE([
+#if HAVE_COMPLEX_H
+#include <complex.h>
+#endif
+complex double z;
+],[
+z = 1.0fi;
+],[AC_DEFINE(GUILE_I,1.0fi)
+   AC_MSG_RESULT([1.0fi])],[ac_cv_type_complex_double=no
+                            AC_MSG_RESULT([not available])])])
+fi
+
 # glibc 2.3.6 (circa 2006) and various prior versions had a bug where
 # csqrt(-i) returned a negative real part, when it should be positive
 # for the principal root.
Index: libguile/numbers.c
===================================================================
RCS file: /cvsroot/guile/guile/guile-core/libguile/numbers.c,v
retrieving revision 1.281.2.12
diff -u -r1.281.2.12 numbers.c
--- libguile/numbers.c	6 Feb 2008 13:17:49 -0000	1.281.2.12
+++ libguile/numbers.c	9 Feb 2008 19:00:08 -0000
@@ -162,11 +162,12 @@
 #endif
 }
 
-
+#if defined (GUILE_I)
 /* For an SCM object Z which is a complex number (ie. satisfies
    SCM_COMPLEXP), return its value as a C level "complex double". */
 #define SCM_COMPLEX_VALUE(z)                                    \
-  (SCM_COMPLEX_REAL (z) + _Complex_I * SCM_COMPLEX_IMAG (z))
+  (SCM_COMPLEX_REAL (z) + GUILE_I * SCM_COMPLEX_IMAG (z))
+#endif
 
 /* Convert a C "complex double" to an SCM value. */
 #if HAVE_COMPLEX_DOUBLE
@@ -6011,7 +6012,7 @@
 {
   if (SCM_COMPLEXP (z))
     {
-#if HAVE_COMPLEX_DOUBLE && HAVE_CLOG
+#if HAVE_COMPLEX_DOUBLE && HAVE_CLOG && defined (SCM_COMPLEX_VALUE)
       return scm_from_complex_double (clog (SCM_COMPLEX_VALUE (z)));
 #else
       double re = SCM_COMPLEX_REAL (z);
@@ -6045,7 +6046,7 @@
       /* Mingw has clog() but not clog10().  (Maybe it'd be worth using
          clog() and a multiply by M_LOG10E, rather than the fallback
          log10+hypot+atan2.)  */
-#if HAVE_COMPLEX_DOUBLE && HAVE_CLOG10
+#if HAVE_COMPLEX_DOUBLE && HAVE_CLOG10 && defined (SCM_COMPLEX_VALUE)
       return scm_from_complex_double (clog10 (SCM_COMPLEX_VALUE (z)));
 #else
       double re = SCM_COMPLEX_REAL (z);
@@ -6077,7 +6078,7 @@
 {
   if (SCM_COMPLEXP (z))
     {
-#if HAVE_COMPLEX_DOUBLE && HAVE_CEXP
+#if HAVE_COMPLEX_DOUBLE && HAVE_CEXP && defined (SCM_COMPLEX_VALUE)
       return scm_from_complex_double (cexp (SCM_COMPLEX_VALUE (z)));
 #else
       return scm_c_make_polar (exp (SCM_COMPLEX_REAL (z)),
@@ -6111,7 +6112,7 @@
 {
   if (SCM_COMPLEXP (x))
     {
-#if HAVE_COMPLEX_DOUBLE && HAVE_USABLE_CSQRT
+#if HAVE_COMPLEX_DOUBLE && HAVE_USABLE_CSQRT && defined (SCM_COMPLEX_VALUE)
       return scm_from_complex_double (csqrt (SCM_COMPLEX_VALUE (x)));
 #else
       double re = SCM_COMPLEX_REAL (x);

Re: Fix for _Complex_I problems

[Rainer Tammer]

Hello,


Neil Jerram wrote:
> You've both recently reported a compilation problem with _Complex_I,
> compiling with GCC on Solaris and AIX.
>
> My proposed fix for this is attached; please can you review and let me
> know if you have any comments?
>
>   
OK, I have tested the second patch on AIX 6.1
> (Should I also attach the regenerated ./configure, so you can run that
> and check that it gives the expected result on your OSs?)
>
>   

I had to regenerate the configure scripts with the latest 
autoconf/automake/libtool/config.guess because of AIX 6.1.

This is my configure call:

export CONFIG_SHELL=/usr/bin/bash
export CONFIG_ENV_ARGS=/usr/bin/bash
export CC=gcc
export CXX=g++
export PATH=/opt/freeware/bin:$PATH
export LDFLAGS=-Wl,-brtl
export CFLAGS="-fno-strict-aliasing -D_USE_IRS"

./configure --enable-dynamic-linking --prefix=/opt/freeware 
--enable-shared --disable-static > configure.log 2>&1


Furthermore I had to fix this:

libguile/srfi-14.c:  redefined ->  #define _GNU_SOURCE   -> config.h
libguile/posix.c:  redefined ->  #define _GNU_SOURCE   -> config.h
libguile/Makefile:  @LTLIBINTL@ gets not replaced with -lintl


This is the result of the make check:

gmake  check-TESTS
gmake[4]: Entering directory 
`/daten/source/guile-1.8.3/test-suite/standalone'
PASS: test-system-cmds
PASS: test-require-extension
PASS: test-num2integral
PASS: test-round
PASS: test-gh
PASS: test-asmobs
PASS: test-list
PASS: test-unwind
PASS: test-conversion
Backtrace:
In unknown file:
   ?: 117  [resolve-interface (ice-9 debug)]
     ...
   ?: 118  (let* (# # # # ...) (and # #) (if # public-i #))
   ?: 119* [resolve-module (ice-9 debug)]
   ?: 120  (let* ((full-name #)) (let* (#) (if already # #)))
   ?: 121  (let* ((already #)) (if already (if # # already) (begin # #)))
   ?: 122* [nested-ref #<module (guile) 2006e130> (%app modules ice-9 
debug)]
   ?: 123  [loop #<module (guile) 2006e130> (%app modules ice-9 debug)]
     ...
   ?: 124  [loop ...
   ?: 125*  [module-ref #<module (guile) 2006e130> %app #f]
   ?: 126   (let* ((variable #)) (if (and variable #) (variable-ref 
variable) ...))
   ?: 127*  [module-variable #<module (guile) 2006e130> %app]
   ?: 128   [module-search #<procedure module-local-variable #> # %app]
      ...
   ?: 129   (or (fn m v) (loop (module-uses m)))
   ?: 130*  [module-local-variable #<module (guile) 2006e130> %app]
   ?: 131   (let* ((b #)) (or (and # b) (and # #)))
   ?: 132*  [module-obarray-ref ...
   ?: 133*   [module-obarray #<module (guile) 2006e130>]
   ?: 134    (if (eq? # #) (struct-ref obj 0) (%record-type-error # obj))
   ?: 135*   [eq? ...
   ?: 136*    (struct-vtable obj)

<unnamed port>: In expression (struct-vtable obj):
<unnamed port>: Stack overflow
guile --user-srfi=1,10 fails to run
FAIL: test-use-srfi
==================================
1 of 10 tests failed
Please report to bug-guile@gnu.org
==================================
gmake[4]: *** [check-TESTS] Error 1

I am not sure why this fails now ... never had this problem.

> Thanks,
>         Neil
>
>   

Bye
  Rainer

Re: Fix for _Complex_I problems

[Neil Jerram]

Rainer Tammer <tammer@tammer.net> writes:

> OK, I have tested the second patch on AIX 6.1

Thanks.  From the fact that compile succeeds, I assume the _Complex_I
fix was good, so I'll go ahead with committing that.

> I had to regenerate the configure scripts with the latest
> autoconf/automake/libtool/config.guess because of AIX 6.1.

Understood.  This is the last thing I want to try to address before we
release 1.8.4.

> Furthermore I had to fix this:
>
> libguile/srfi-14.c:  redefined ->  #define _GNU_SOURCE   -> config.h
> libguile/posix.c:  redefined ->  #define _GNU_SOURCE   -> config.h
> libguile/Makefile:  @LTLIBINTL@ gets not replaced with -lintl

Were those hidden because the make stopped at numbers.c before?

In any case, I think we should probably revisit those after 1.8.4;
otherwise there's too much uncertainty about exactly how the auto*
steps happened.

> <unnamed port>: In expression (struct-vtable obj):
> <unnamed port>: Stack overflow

Hmm, that happens on Mac Powerbook too.  I did want to address this
before 1.8.4, but as it's a Scheme-level error only, I now think it's
better for us to get 1.8.4 out - so we'll look at this again after the
1.8.4 release.

> Bye
>  Rainer

Thanks!,
     Neil

Re: Fix for _Complex_I problems

[Rainer Tammer]

Hello,

Neil Jerram wrote:

[...]
> Thanks.  From the fact that compile succeeds, I assume the _Complex_I
> fix was good, so I'll go ahead with committing that.
>
>   
OK
>> I had to regenerate the configure scripts with the latest
>> autoconf/automake/libtool/config.guess because of AIX 6.1.
>>     
>
> Understood.  This is the last thing I want to try to address before we
> release 1.8.4.
>
>   
This sounds good.
>> Furthermore I had to fix this:
>>
>> libguile/srfi-14.c:  redefined ->  #define _GNU_SOURCE   -> config.h
>> libguile/posix.c:  redefined ->  #define _GNU_SOURCE   -> config.h
>> libguile/Makefile:  @LTLIBINTL@ gets not replaced with -lintl
>>     
>
> Were those hidden because the make stopped at numbers.c before?
>
>   
I think this is caused by the new configure tools ...
If I run autoreconf I get the following warnings:

# autoreconf 2.61a
configure.in:76: warning: AC_COMPILE_IFELSE was called before 
AC_USE_SYSTEM_EXTENSIONS
../../lib/autoconf/specific.m4:383: AC_USE_SYSTEM_EXTENSIONS is expanded 
from...
../../lib/autoconf/specific.m4:454: AC_MINIX is expanded from...
configure.in:76: the top level
configure.in:76: warning: AC_RUN_IFELSE was called before 
AC_USE_SYSTEM_EXTENSIONS
configure.in:76: warning: AC_COMPILE_IFELSE was called before 
AC_USE_SYSTEM_EXTENSIONS
../../lib/autoconf/specific.m4:383: AC_USE_SYSTEM_EXTENSIONS is expanded 
from...
../../lib/autoconf/specific.m4:454: AC_MINIX is expanded from...
configure.in:76: the top level
configure.in:76: warning: AC_RUN_IFELSE was called before 
AC_USE_SYSTEM_EXTENSIONS
autoreconf: configure.in: AM_GNU_GETTEXT is used, but not 
AM_GNU_GETTEXT_VERSION
configure.in:76: warning: AC_COMPILE_IFELSE was called before 
AC_USE_SYSTEM_EXTENSIONS
../../lib/autoconf/specific.m4:383: AC_USE_SYSTEM_EXTENSIONS is expanded 
from...
../../lib/autoconf/specific.m4:454: AC_MINIX is expanded from...
configure.in:76: the top level
configure.in:76: warning: AC_RUN_IFELSE was called before 
AC_USE_SYSTEM_EXTENSIONS
configure.in:76: warning: AC_COMPILE_IFELSE was called before 
AC_USE_SYSTEM_EXTENSIONS
../../lib/autoconf/specific.m4:383: AC_USE_SYSTEM_EXTENSIONS is expanded 
from...
../../lib/autoconf/specific.m4:454: AC_MINIX is expanded from...
configure.in:76: the top level
configure.in:76: warning: AC_RUN_IFELSE was called before 
AC_USE_SYSTEM_EXTENSIONS
configure.in:76: warning: AC_COMPILE_IFELSE was called before 
AC_USE_SYSTEM_EXTENSIONS
../../lib/autoconf/specific.m4:383: AC_USE_SYSTEM_EXTENSIONS is expanded 
from...
../../lib/autoconf/specific.m4:454: AC_MINIX is expanded from...
configure.in:76: the top level
configure.in:76: warning: AC_RUN_IFELSE was called before 
AC_USE_SYSTEM_EXTENSIONS


In guile-readline/configure the following part is messed up:

PACKAGE_VERSION='-n 1.8.3
'
PACKAGE_STRING='guile-readline -n 1.8.3
'

This should be:

PACKAGE_VERSION='-n 1.8.3'
PACKAGE_STRING='guile-readline -n 1.8.3'


> In any case, I think we should probably revisit those after 1.8.4;
> otherwise there's too much uncertainty about exactly how the auto*
> steps happened.
>
>   
>> <unnamed port>: In expression (struct-vtable obj):
>> <unnamed port>: Stack overflow
>>     
>
> Hmm, that happens on Mac Powerbook too.  I did want to address this
> before 1.8.4, but as it's a Scheme-level error only, I now think it's
> better for us to get 1.8.4 out - so we'll look at this again after the
> 1.8.4 release.
>
>   
No problem.
> Thanks!,
>      Neil
>
>
>   
Bye
  Rainer

Re: Fix for _Complex_I problems

[Greg Troxel]

I cannot test on Solaris; I was reporting a problem that a pkgsrc user had.

The fix looks like it will work, but I am not comfortable with it.  It
mucks up the source with local defines to hack around lack of standards
compliance.  Reading the second diff, there's much less of this.

As far as I can tell, C99 says that if complex.h is there then
_Complex_I has to work.  This is the reason why I'm being cranky about
defining around it.

I would prefer to use the compile test that you wrote, but to undefine
HAVE_COMPLEX_H if it fails, thus falling back to not using the
(therefore broken) complex support at all.  I had meant to suggest this,
and I'm sorry if I didn't manage to do that.

Adding a remedial

#define _Complex_I 1.0fi

to the beginning of numbers.c if _Complex_I weren't defined would seem
less objectionable, and also work.

Thanks for paying attention to this.

Re: Fix for _Complex_I problems

[Greg Troxel]

Looking over your patch again, I feel quite a bit better about it.

It checks for the standard first, and

  -
  +#if defined (GUILE_I)
   /* For an SCM object Z which is a complex number (ie. satisfies
      SCM_COMPLEXP), return its value as a C level "complex double". */
   #define SCM_COMPLEX_VALUE(z)                                    \
  -  (SCM_COMPLEX_REAL (z) + _Complex_I * SCM_COMPLEX_IMAG (z))
  +  (SCM_COMPLEX_REAL (z) + GUILE_I * SCM_COMPLEX_IMAG (z))
  +#endif

really is very close to a remedial define of _Complex_I.  So the impact
on conforming systems is small, and limited to a reader of the code
having to chase the GUILE_I definition.

+# When compiling with GCC on some OSs (Solaris, AIX), _Complex_I doesn't work;
+# in the reported cases so far, 1.0fi works well instead.

I would add

# This is a workaround for the failure of these systems to conform to C99.

So this all seems ok to me.

Re: Fix for _Complex_I problems

[Neil Jerram]

Greg Troxel <gdt@ir.bbn.com> writes:

> Looking over your patch again, I feel quite a bit better about it.

[...]

> I would add
>
> # This is a workaround for the failure of these systems to conform to C99.
>
> So this all seems ok to me.

Hi Greg,

Many thanks for your review, thoughts and second thoughts.  I agree
that it's a good idea to add the extra sentence that you've suggested,
so I'll do that now.

Regards,
        Neil

Re: Fix for _Complex_I problems

[Neil Jerram]

Greg Troxel <gdt@ir.bbn.com> writes:

> Adding a remedial
>
> #define _Complex_I 1.0fi
>
> to the beginning of numbers.c if _Complex_I weren't defined would seem
> less objectionable, and also work.

Just one detail for the record: unfortunately that wouldn't work for
GCC on AIX, because there:

- complex.h does define _Complex_I: #define _Complex_I __I
- the AIX compiler provides __I, so complex.h works with the AIX
  compiler
- GCC does not provide __I, so the setup doesn't work with GCC (but
  `#ifdef _Complex_I' is still true)

Regards,
    Neil

Re: Fix for _Complex_I problems

[Rainer Tammer]

Hello Greg,

Greg Troxel wrote:
> I cannot test on Solaris; I was reporting a problem that a pkgsrc user had.
>
> The fix looks like it will work, but I am not comfortable with it.  It
> mucks up the source with local defines to hack around lack of standards
> compliance.  Reading the second diff, there's much less of this.
>
> As far as I can tell, C99 says that if complex.h is there then
> _Complex_I has to work.  This is the reason why I'm being cranky about
> defining around it.
>
>   
On AIX _Complex_I is working if you use the IBM compiler (inline 
function). But if you use gcc then
you are doomed...
> I would prefer to use the compile test that you wrote, but to undefine
> HAVE_COMPLEX_H if it fails, thus falling back to not using the
> (therefore broken) complex support at all.  I had meant to suggest this,
> and I'm sorry if I didn't manage to do that.
>
> Adding a remedial
>
> #define _Complex_I 1.0fi
>
> to the beginning of numbers.c if _Complex_I weren't defined would seem
> less objectionable, and also work.
>
>   
On AIX:

complex.h:
#define complex    _Complex

/*.
 * a constant expression of type const float _Complex with the
 * value of the imaginary unit. (a number i such that i**2 =-1).
 * __I is provided by the AIX xlc C99 compiler.
 * WARNING: DO NOT USE __I DIRECTLY in an application. Always
 * use _Complex_I .
 */
#define _Complex_I    __I

/*
 * _Imaginary_I should be a constant expression of type
 * const float _Imaginary with the value of the imaginary unit.
 * This is optional in C99.
 * This is not supported in the AIX xlc C99 compiler.
 */

/*.
 * C99 requires this definition of the
 * very common variable "I", to use as a simpler way
 * to say _Complex_I. Mathematicians who would say
 * "3i" will now say in C "3 * I".
 */
#undef I
#define I _Complex_I


> Thanks for paying attention to this.
>
>
>   
Bye
  Rainer