[Patch] inline.h should not define inline functions with "extern" linkage

[Patch] inline.h should not define inline functions with "extern" linkage

[Matthias Koeppe]

I am now building Guile from CVS HEAD with a version of the Sun Forte
C compiler that supports the "inline" keyword.  

The "inline" keyword does not imply static linkage, and in fact
inline.h defines the functions `scm_cell' and `scm_double_cell'
explicitly with "extern" linkage.  

Therefore, every file that includes inline.h defines a copy of these
two functions, each copy with external linkage.  This makes it
impossible to link Guile.  (I do not know why this would work with
gcc; it is definitely wrong.)

The patch below fixes it.  inline.c defines the functions with
external linkage, and every file including inline.h defines static
inline copies.

The same trick needs to be applied to the external/inline functions
in numbers.h.  The patch fixes them as well.

Index: libguile/inline.h
===================================================================
RCS file: /cvs/guile/guile-core/libguile/inline.h,v
retrieving revision 1.15
diff -u -c -r1.15 inline.h
*** libguile/inline.h	5 Apr 2003 19:10:23 -0000	1.15
--- libguile/inline.h	19 Jun 2003 12:45:59 -0000
***************
*** 53,59 ****
  
  #if defined SCM_C_INLINE && ! defined SCM_INLINE_C_INCLUDING_INLINE_H
  /* definitely inlining */
! extern SCM_C_INLINE
  #endif
  SCM
  scm_cell (scm_t_bits car, scm_t_bits cdr)
--- 53,59 ----
  
  #if defined SCM_C_INLINE && ! defined SCM_INLINE_C_INCLUDING_INLINE_H
  /* definitely inlining */
! static SCM_C_INLINE
  #endif
  SCM
  scm_cell (scm_t_bits car, scm_t_bits cdr)
***************
*** 145,151 ****
  
  #if defined SCM_C_INLINE && ! defined SCM_INLINE_C_INCLUDING_INLINE_H
  /* definitely inlining */
! extern SCM_C_INLINE
  #endif
  SCM
  scm_double_cell (scm_t_bits car, scm_t_bits cbr,
--- 145,151 ----
  
  #if defined SCM_C_INLINE && ! defined SCM_INLINE_C_INCLUDING_INLINE_H
  /* definitely inlining */
! static SCM_C_INLINE
  #endif
  SCM
  scm_double_cell (scm_t_bits car, scm_t_bits cbr,
Index: libguile/numbers.h
===================================================================
RCS file: /cvs/guile/guile-core/libguile/numbers.h,v
retrieving revision 1.72
diff -u -c -r1.72 numbers.h
*** libguile/numbers.h	30 May 2003 09:39:34 -0000	1.72
--- libguile/numbers.h	19 Jun 2003 12:45:59 -0000
***************
*** 272,282 ****
--- 272,284 ----
  
  
  /* bignum internal functions */
+ #if !defined(SCM_NUMBERS_INLINE_H)
  SCM_API SCM scm_i_mkbig (void);
  SCM_API SCM scm_i_normbig (SCM x);
  SCM_API int scm_i_bigcmp (SCM a, SCM b);
  SCM_API SCM scm_i_dbl2big (double d);
  SCM_API double scm_i_big2dbl (SCM b);
+ #endif 
  SCM_API SCM scm_i_short2big (short n);
  SCM_API SCM scm_i_ushort2big (unsigned short n);
  SCM_API SCM scm_i_int2big (int n);
Index: libguile/numbers.c
===================================================================
RCS file: /cvs/guile/guile-core/libguile/numbers.c,v
retrieving revision 1.191
diff -u -c -r1.191 numbers.c
*** libguile/numbers.c	4 Jun 2003 16:09:38 -0000	1.191
--- libguile/numbers.c	19 Jun 2003 12:45:59 -0000
***************
*** 55,63 ****
  #include "libguile/strings.h"
  
  #include "libguile/validate.h"
  #include "libguile/numbers.h"
  #include "libguile/deprecation.h"
- 
  \f
  
  /*
--- 55,63 ----
  #include "libguile/strings.h"
  
  #include "libguile/validate.h"
+ #include "libguile/numbers_inline.h"
  #include "libguile/numbers.h"
  #include "libguile/deprecation.h"
  \f
  
  /*
***************
*** 124,189 ****
  \f
  
  static const char s_bignum[] = "bignum";
- 
- SCM_C_INLINE SCM
- scm_i_mkbig ()
- {
-   /* Return a newly created bignum. */
-   SCM z = scm_double_cell (scm_tc16_big, 0, 0, 0);
-   mpz_init (SCM_I_BIG_MPZ (z));
-   return z;
- }
- 
- SCM_C_INLINE static SCM
- scm_i_clonebig (SCM src_big, int same_sign_p)
- {
-   /* Copy src_big's value, negate it if same_sign_p is false, and return. */
-   SCM z = scm_double_cell (scm_tc16_big, 0, 0, 0);
-   mpz_init_set (SCM_I_BIG_MPZ (z), SCM_I_BIG_MPZ (src_big));
-   if (!same_sign_p) mpz_neg (SCM_I_BIG_MPZ (z), SCM_I_BIG_MPZ (z));
-   return z;
- }
- 
- SCM_C_INLINE int
- scm_i_bigcmp (SCM x, SCM y)
- {
-   /* Return neg if x < y, pos if x > y, and 0 if x == y */
-   /* presume we already know x and y are bignums */
-   int result = mpz_cmp (SCM_I_BIG_MPZ (x), SCM_I_BIG_MPZ (y));
-   scm_remember_upto_here_2 (x, y);
-   return result;
- }
- 
- SCM_C_INLINE SCM
- scm_i_dbl2big (double d)
- {
-   /* results are only defined if d is an integer */
-   SCM z = scm_double_cell (scm_tc16_big, 0, 0, 0);
-   mpz_init_set_d (SCM_I_BIG_MPZ (z), d);
-   return z;
- }
- 
- SCM_C_INLINE double
- scm_i_big2dbl (SCM b)
- {
-   double result = mpz_get_d (SCM_I_BIG_MPZ (b));
-   scm_remember_upto_here_1 (b);
-   return result;
- }
- 
- SCM_C_INLINE SCM
- scm_i_normbig (SCM b)
- {
-   /* convert a big back to a fixnum if it'll fit */
-   /* presume b is a bignum */
-   if (mpz_fits_slong_p (SCM_I_BIG_MPZ (b)))
-     {
-       long val = mpz_get_si (SCM_I_BIG_MPZ (b));
-       if (SCM_FIXABLE (val))
-         b = SCM_MAKINUM (val);
-     }
-   return b;
- }
  
  SCM_DEFINE (scm_exact_p, "exact?", 1, 0, 0, 
              (SCM x),
--- 124,129 ----
*** /dev/null	Thu Jun 19 13:27:05 2003
--- libguile/numbers_inline.h	Thu Jun 19 14:37:45 2003
***************
*** 0 ****
--- 1,119 ----
+ #ifndef SCM_NUMBERS_INLINE_H
+ #define SCM_NUMBERS_INLINE_H
+ 
+ /* Copyright (C) 1995,1996,1998,2000,2001,2003 Free Software Foundation, Inc.
+  *
+  * This library is free software; you can redistribute it and/or
+  * modify it under the terms of the GNU Lesser General Public
+  * License as published by the Free Software Foundation; either
+  * version 2.1 of the License, or (at your option) any later version.
+  *
+  * This library is distributed in the hope that it will be useful,
+  * but WITHOUT ANY WARRANTY; without even the implied warranty of
+  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+  * Lesser General Public License for more details.
+  *
+  * You should have received a copy of the GNU Lesser General Public
+  * License along with this library; if not, write to the Free Software
+  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+  */
+ 
+ \f
+ 
+ #include "libguile/__scm.h"
+ #include "libguile/_scm.h"
+ #include <gmp.h>
+ 
+ #if defined SCM_C_INLINE && ! defined SCM_NUMBERS_INLINE_C_INCLUDING_NUMBERS_INLINE_H
+ /* definitely inlining */
+ static SCM_C_INLINE
+ #endif
+ SCM
+ scm_i_mkbig ()
+ {
+   /* Return a newly created bignum. */
+   SCM z = scm_double_cell (scm_tc16_big, 0, 0, 0);
+   mpz_init (SCM_I_BIG_MPZ (z));
+   return z;
+ }
+ 
+ #if defined SCM_C_INLINE && ! defined SCM_NUMBERS_INLINE_C_INCLUDING_NUMBERS_INLINE_H
+ /* definitely inlining */
+ static SCM_C_INLINE
+ #else
+ static
+ #endif
+ SCM
+ scm_i_clonebig (SCM src_big, int same_sign_p)
+ {
+   /* Copy src_big's value, negate it if same_sign_p is false, and return. */
+   SCM z = scm_double_cell (scm_tc16_big, 0, 0, 0);
+   mpz_init_set (SCM_I_BIG_MPZ (z), SCM_I_BIG_MPZ (src_big));
+   if (!same_sign_p) mpz_neg (SCM_I_BIG_MPZ (z), SCM_I_BIG_MPZ (z));
+   return z;
+ }
+ 
+ #if defined SCM_C_INLINE && ! defined SCM_NUMBERS_INLINE_C_INCLUDING_NUMBERS_INLINE_H
+ /* definitely inlining */
+ static SCM_C_INLINE
+ #endif
+ int
+ scm_i_bigcmp (SCM x, SCM y)
+ {
+   /* Return neg if x < y, pos if x > y, and 0 if x == y */
+   /* presume we already know x and y are bignums */
+   int result = mpz_cmp (SCM_I_BIG_MPZ (x), SCM_I_BIG_MPZ (y));
+   scm_remember_upto_here_2 (x, y);
+   return result;
+ }
+ 
+ #if defined SCM_C_INLINE && ! defined SCM_NUMBERS_INLINE_C_INCLUDING_NUMBERS_INLINE_H
+ /* definitely inlining */
+ static SCM_C_INLINE
+ #endif
+ SCM
+ scm_i_dbl2big (double d)
+ {
+   /* results are only defined if d is an integer */
+   SCM z = scm_double_cell (scm_tc16_big, 0, 0, 0);
+   mpz_init_set_d (SCM_I_BIG_MPZ (z), d);
+   return z;
+ }
+ 
+ #if defined SCM_C_INLINE && ! defined SCM_NUMBERS_INLINE_C_INCLUDING_NUMBERS_INLINE_H
+ /* definitely inlining */
+ static SCM_C_INLINE
+ #endif
+ double
+ scm_i_big2dbl (SCM b)
+ {
+   double result = mpz_get_d (SCM_I_BIG_MPZ (b));
+   scm_remember_upto_here_1 (b);
+   return result;
+ }
+ 
+ #if defined SCM_C_INLINE && ! defined SCM_NUMBERS_INLINE_C_INCLUDING_NUMBERS_INLINE_H
+ /* definitely inlining */
+ static SCM_C_INLINE
+ #endif
+ SCM
+ scm_i_normbig (SCM b)
+ {
+   /* convert a big back to a fixnum if it'll fit */
+   /* presume b is a bignum */
+   if (mpz_fits_slong_p (SCM_I_BIG_MPZ (b)))
+     {
+       long val = mpz_get_si (SCM_I_BIG_MPZ (b));
+       if (SCM_FIXABLE (val))
+         b = SCM_MAKINUM (val);
+     }
+   return b;
+ }
+ 
+ #endif  /* SCM_NUMBERS_INLINE_H */
+ 
+ /*
+   Local Variables:
+   c-file-style: "gnu"
+   End:
+ */
Index: libguile/Makefile.am
===================================================================
RCS file: /cvs/guile/guile-core/libguile/Makefile.am,v
retrieving revision 1.187
diff -u -c -r1.187 Makefile.am
*** libguile/Makefile.am	29 May 2003 14:39:13 -0000	1.187
--- libguile/Makefile.am	19 Jun 2003 12:47:49 -0000
***************
*** 98,110 ****
      gh_io.c gh_list.c gh_predicates.c goops.c gsubr.c guardians.c hash.c    \
      hashtab.c hooks.c init.c inline.c ioext.c keywords.c	    \
      lang.c list.c							    \
!     load.c macros.c mallocs.c modules.c numbers.c objects.c objprop.c	    \
      options.c pairs.c ports.c print.c procprop.c procs.c properties.c	    \
      random.c rdelim.c read.c root.c rw.c scmsigs.c script.c simpos.c smob.c \
      sort.c srcprop.c stackchk.c stacks.c stime.c strings.c strop.c	    \
      strorder.c strports.c struct.c symbols.c threads.c throw.c values.c	    \
      variable.c vectors.c version.c vports.c weaks.c
  
  DOT_X_FILES = alist.x arbiters.x async.x backtrace.x boolean.x chars.x	  \
      continuations.x debug.x deprecation.x deprecated.x dynl.x dynwind.x   \
      environments.x eq.x							  \
--- 98,110 ----
      gh_io.c gh_list.c gh_predicates.c goops.c gsubr.c guardians.c hash.c    \
      hashtab.c hooks.c init.c inline.c ioext.c keywords.c	    \
      lang.c list.c							    \
!     load.c macros.c mallocs.c modules.c numbers.c numbers_inline.c objects.c objprop.c	    \
      options.c pairs.c ports.c print.c procprop.c procs.c properties.c	    \
      random.c rdelim.c read.c root.c rw.c scmsigs.c script.c simpos.c smob.c \
      sort.c srcprop.c stackchk.c stacks.c stime.c strings.c strop.c	    \
      strorder.c strports.c struct.c symbols.c threads.c throw.c values.c	    \
      variable.c vectors.c version.c vports.c weaks.c
  
  DOT_X_FILES = alist.x arbiters.x async.x backtrace.x boolean.x chars.x	  \
      continuations.x debug.x deprecation.x deprecated.x dynl.x dynwind.x   \
      environments.x eq.x							  \
***************
*** 185,191 ****
      goops.h gsubr.h guardians.h hash.h hashtab.h hooks.h init.h		      \
      inline.h ioext.h							      \
      iselect.h keywords.h lang.h list.h load.h macros.h mallocs.h modules.h    \
!     net_db.h numbers.h objects.h objprop.h options.h pairs.h ports.h posix.h  \
      regex-posix.h print.h procprop.h procs.h properties.h random.h ramap.h    \
      rdelim.h read.h root.h rw.h scmsigs.h validate.h script.h simpos.h smob.h \
      snarf.h socket.h sort.h srcprop.h stackchk.h stacks.h stime.h strings.h   \
--- 185,191 ----
      goops.h gsubr.h guardians.h hash.h hashtab.h hooks.h init.h		      \
      inline.h ioext.h							      \
      iselect.h keywords.h lang.h list.h load.h macros.h mallocs.h modules.h    \
!     net_db.h numbers.h numbers_inline.h objects.h objprop.h options.h pairs.h ports.h posix.h  \
      regex-posix.h print.h procprop.h procs.h properties.h random.h ramap.h    \
      rdelim.h read.h root.h rw.h scmsigs.h validate.h script.h simpos.h smob.h \
      snarf.h socket.h sort.h srcprop.h stackchk.h stacks.h stime.h strings.h   \


-- 
Matthias Koeppe -- http://www.math.uni-magdeburg.de/~mkoeppe


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: [Patch] inline.h should not define inline functions with "extern" linkage

[Kevin Ryde]

Matthias Koeppe <mkoeppe@merkur.math.uni-magdeburg.de> writes:
>
> The "inline" keyword does not imply static linkage, and in fact
> inline.h defines the functions `scm_cell' and `scm_double_cell'
> explicitly with "extern" linkage.  

"extern inline" is a gcc-ism, as far as I'm aware.  We use it in
gmp.h, but only with gcc.

> The patch below fixes it.  inline.c defines the functions with
> external linkage, and every file including inline.h defines static
> inline copies.

One possibility would be to do these things as macros, so as to avoid
depending on the compiler having an inline, or whether it only
actually inlines at certain optimization levels.

You can imagine the sort of thing, setting a variable given to the
macro, rather than a return value,

#define SCM_I_MKBIG(big)
  do {
    SCM  __scm_i_mkbig__temp = scm_double_cell (scm_tc16_big, 0, 0, 0);
    mpz_init (SCM_I_BIG_MPZ (__scm_i_mkbig__temp));
    (big) = __scm_i_mkbig__temp;
  } while (0)


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: [Patch] inline.h should not define inline functions with "extern" linkage

[Matthias Koeppe]

Kevin Ryde <user42@zip.com.au> writes:

> Matthias Koeppe <mkoeppe@merkur.math.uni-magdeburg.de> writes:
>>
>> The "inline" keyword does not imply static linkage, and in fact
>> inline.h defines the functions `scm_cell' and `scm_double_cell'
>> explicitly with "extern" linkage.  
>
> "extern inline" is a gcc-ism, as far as I'm aware.  We use it in
> gmp.h, but only with gcc.

Yes.  

>> The patch below fixes it.  inline.c defines the functions with
>> external linkage, and every file including inline.h defines static
>> inline copies.
>
> One possibility would be to do these things as macros, so as to avoid
> depending on the compiler having an inline, or whether it only
> actually inlines at certain optimization levels.
>
> You can imagine the sort of thing, setting a variable given to the
> macro, rather than a return value,
>
> #define SCM_I_MKBIG(big)
>   do {
>     SCM  __scm_i_mkbig__temp = scm_double_cell (scm_tc16_big, 0, 0, 0);
>     mpz_init (SCM_I_BIG_MPZ (__scm_i_mkbig__temp));
>     (big) = __scm_i_mkbig__temp;
>   } while (0)

CVS Guile uses a different method to deal with possibly inlined
functions.  

inline.c defines ("extern", non-"inline") versions for use in
user programs.

If the C compiler recognizes an "inline" keyword, inline.h defines
"inline" versions of the functions.  However, there the gcc-ism of
"extern inline" was used.  My patch gets rid of this gcc-ism.  This
makes the compilation work with the Sun Forte compiler.

I have adapted this approach to the inline functions in numbers.c.

-- 
Matthias Koeppe -- http://www.math.uni-magdeburg.de/~mkoeppe


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: [Patch] inline.h should not define inline functions with "extern" linkage

[Marius Vollmer]

Matthias Koeppe <mkoeppe@mail.math.uni-magdeburg.de> writes:

> I am now building Guile from CVS HEAD with a version of the Sun Forte
> C compiler that supports the "inline" keyword.  
>
> The "inline" keyword does not imply static linkage, and in fact
> inline.h defines the functions `scm_cell' and `scm_double_cell'
> explicitly with "extern" linkage.  
>
> Therefore, every file that includes inline.h defines a copy of these
> two functions, each copy with external linkage.  This makes it
> impossible to link Guile.  (I do not know why this would work with
> gcc; it is definitely wrong.)
>
> The patch below fixes it.  inline.c defines the functions with
> external linkage, and every file including inline.h defines static
> inline copies.

We should keep the "extern inline" when compiling with GCC, it is the
Right Thing to do.  For non-GCC compilers, we can use "static inline".

I have changed the code in inline.h to, for example

    #if defined SCM_C_INLINE && ! defined SCM_INLINE_C_INCLUDING_INLINE_H
    /* definitely inlining */
    #ifdef __GNUC__
    extern
    #else
    static
    #endif
    SCM_C_INLINE
    #endif
    SCM
    scm_double_cell (scm_t_bits car, scm_t_bits cbr,
                     scm_t_bits ccr, scm_t_bits cdr)
    {
       ...

Ok?

> The same trick needs to be applied to the external/inline functions
> in numbers.h.  The patch fixes them as well.

I think we can leave numbers.h alone.  The inline functions are not
part of the public API and it is not important that they are inlined
outside of numbers.c, so we don't need to put them into a header file.

We could make them "static inline", with a little bit of work.  Would
that be worthwhile?

-- 
GPG: D5D4E405 - 2F9B BCCC 8527 692A 04E3  331E FAF8 226A D5D4 E405


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel