This is the mail archive of the guile@sourceware.cygnus.com mailing list for the Guile project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: Name change: SCMWORD --> scm_word_t


On 12 Mar 2000, Mikael Djurfeldt wrote:

> Mikael Djurfeldt <mdj@mdj.nada.kth.se> writes:
> 
> > Mikael Djurfeldt <mdj@mdj.nada.kth.se> writes:
> > 
> > >   SCM_UNPACK (x)
> > >   SCM_UNPACK_CAR (x)
> > >   SCM_PACK (x)
> > 
> > Actually, I settled for this.  (You may of course protest! :)

Congratulations for this change!

However, by playing around with defining SCM as a union instead of a void*
I was able to discover a couple of bugs.  However, it showed that using a
union would introduce some other problems:  It seems not to be possible to
compare two unions via the operators == and !=, at least the compiler is
complaining.  That's a pity, since we would not like to have to unpack SCM
values just for the purpose of comparison.  Hoever, I ignored these types
of errors and found the following problems:

arbiters.c, line 87:  The value scm_tc16_arbiter should be packed before
calling SCM_RETURN_NEWSMOB.  Or, scm_tc16_arbiter could be made a SCM
value from the start.

diff -u -r1.18 arbiters.c
--- arbiters.c  2000/03/12 18:29:38     1.18
+++ arbiters.c  2000/03/13 08:00:50
@@ -84,7 +84,7 @@
 "Arbiters are a way to achieve process synchronization.")
 #define FUNC_NAME s_scm_make_arbiter
 {
-  SCM_RETURN_NEWSMOB (scm_tc16_arbiter, name);
+  SCM_RETURN_NEWSMOB (SCM_PACK (scm_tc16_arbiter), name);
 }
 #undef FUNC_NAME
 
async.c, line 289:  First, scm_tc16_async should be packed also (see
above).  Second, the implicit conversion of async to a SCM value is
illegal.  A question is:  is it allowed to call SCM_PACK with something
else than a value of type scm_bits_t?  If not, an additional cast as in
the following patch is required.
line 394, 411:  SCM_INUM already delivers a C value.  Unpacking the result
of SCM_INUM thus is illegal.

diff -u -r1.31 async.c
--- async.c     2000/03/12 18:29:38     1.31
+++ async.c     2000/03/13 08:00:51
@@ -286,7 +286,7 @@
     = (struct scm_async *) scm_must_malloc (sizeof (*async), FUNC_NAME);
   async->got_it = 0;
   async->thunk = thunk;
-  SCM_RETURN_NEWSMOB (scm_tc16_async, async);
+  SCM_RETURN_NEWSMOB (SCM_PACK (scm_tc16_async), SCM_PACK ((scm_bits_t) async));
 }
 #undef FUNC_NAME
 
@@ -385,12 +385,12 @@
   unsigned int old_n;
 
 
-  SCM_VALIDATE_INUM (1,n);
+  SCM_VALIDATE_INUM (1, n);
 
   old_n = scm_tick_rate;
 
 
-  scm_desired_tick_rate = SCM_UNPACK (SCM_INUM (n));
+  scm_desired_tick_rate = SCM_INUM (n);
   scm_async_rate = 1 + scm_async_rate - scm_async_clock;
   scm_async_clock = 1;
   return SCM_MAKINUM (old_n);
@@ -408,7 +408,7 @@
   unsigned int old_n;
   SCM_VALIDATE_INUM (1,n);
   old_n = scm_switch_rate;
-  scm_desired_switch_rate = SCM_UNPACK (SCM_INUM (n));
+  scm_desired_switch_rate = SCM_INUM (n);
   scm_async_rate = 1 + scm_async_rate - scm_async_clock;
   scm_async_clock = 1;
   return SCM_MAKINUM (old_n);

async.h, line 55:  Before casting to an actual C pointer, a SCM value
should be unpacked.

diff -u -r1.12 async.h
--- async.h     2000/03/02 20:54:42     1.12
+++ async.h     2000/03/13 08:00:51
@@ -52,7 +52,7 @@
 
 ^L
 #define SCM_ASYNCP(X)  (SCM_NIMP(X) && (scm_tc16_async == SCM_GCTYP16 (X)))
-#define SCM_ASYNC(X)   ((struct scm_async *)SCM_CDR (X))
+#define SCM_ASYNC(X)   ((struct scm_async *) SCM_UNPACK(SCM_CDR (X)))
 
 struct scm_async
 {


numbers.h:  Testing a SCM value to be an INUM requires unpacking.  Making
an INUM from a C value requires packing.

diff -u -r1.25 numbers.h
--- numbers.h   2000/03/12 18:29:38     1.25
+++ numbers.h   2000/03/13 08:00:55
@@ -64,16 +64,16 @@
  * SCM_INUMP (SCM_CAR (x)) can give wrong answers.
  */
 
-#define SCM_INUMP(x)   (2 & (int)(x))
+#define SCM_INUMP(x)   (2 & SCM_UNPACK (x))
 #define SCM_NINUMP(x)  (!SCM_INUMP(x))
 
 #ifdef __TURBOC__
 /* shifts of more than one are done by a library call, single shifts are
  * performed in registers
  */
-# define SCM_MAKINUM(x) ((SCM) (((SCM_UNPACK(x)<<1)<<1)+2L))
+# define SCM_MAKINUM(x) (SCM_PACK ((((x)<<1)<<1)+2L))
 #else
-# define SCM_MAKINUM(x) ((SCM)((SCM_UNPACK(x)<<2)+2L))
+# define SCM_MAKINUM(x) (SCM_PACK (((x)<<2)+2L))
 #endif /* def __TURBOC__ */
 
 

pairs.h:  Assuming that the arguments for SET_CAR and SET_CDR are already
valid SCM values, another call to pack is illegal.  Also, in order to use 
scm_tc16_allocated as a SCM value, it has to be packed first.

diff -u -r1.18 pairs.h
--- pairs.h     2000/03/12 18:29:38     1.18
+++ pairs.h     2000/03/13 08:00:56
@@ -105,8 +105,8 @@
 #define SCM_CAR(x) (((scm_cell *)(SCM2PTR(x)))->car)
 #define SCM_CDR(x) (((scm_cell *)(SCM2PTR(x)))->cdr)
 #define SCM_GCCDR(x) SCM_PACK(~1L & SCM_UNPACK (SCM_CDR(x)))
-#define SCM_SETCAR(x, v) (SCM_CAR(x) = SCM_PACK(v))
-#define SCM_SETCDR(x, v) (SCM_CDR(x) = SCM_PACK(v))
+#define SCM_SETCAR(x, v) (SCM_CAR(x) = (v))
+#define SCM_SETCDR(x, v) (SCM_CDR(x) = (v))
 
 #define SCM_CARLOC(x) (&SCM_CAR (x))
 #define SCM_CDRLOC(x) (&SCM_CDR (x))
@@ -168,7 +168,7 @@
            { \
               _into = scm_freelist; \
               scm_freelist = SCM_CDR(scm_freelist);\
-               SCM_SETCAR(_into, scm_tc16_allocated); \
+               SCM_SETCAR(_into, SCM_PACK (scm_tc16_allocated)); \
               ++scm_cells_allocated; \
            } \
        } while(0)


struct.h:  Making a SCM* from a SCM value requires unpacking first.

diff -u -r1.24 struct.h
--- struct.h    2000/03/12 18:29:39     1.24
+++ struct.h    2000/03/13 08:00:57
@@ -78,8 +78,8 @@
                                         (no hidden words) */
 
 #define SCM_STRUCTP(X)                 (SCM_NIMP(X) && (SCM_TYP3(X) == scm_tc3_cons_gloc))
-#define SCM_STRUCT_DATA(X)             ((SCM*)(SCM_CDR(X)))
-#define SCM_STRUCT_VTABLE_DATA(X)      ((SCM *)(SCM_UNPACK (SCM_CAR(X)) - 1))
+#define SCM_STRUCT_DATA(X)             ((SCM *) (SCM_UNPACK (SCM_CDR(X))))
+#define SCM_STRUCT_VTABLE_DATA(X)      ((SCM *) (SCM_UNPACK (SCM_CAR(X)) - 1))
 #define SCM_STRUCT_LAYOUT(X)           (SCM_STRUCT_VTABLE_DATA(X)[scm_vtable_index_layout])
 #define SCM_STRUCT_VTABLE(X)           (SCM_STRUCT_VTABLE_DATA(X)[scm_vtable_index_vtable])
 #define SCM_STRUCT_PRINTER(X)          (SCM_STRUCT_VTABLE_DATA(X)[scm_vtable_index_printer])



tags.h:  PTR2SCM and SCM2PTR require packing and unpacking.  NOTE:  the
line numbers of the following patch are not correct with respect to the
current cvs version.

@@ -85,12 +106,12 @@
  * to scm_vector elts, functions, &c are not munged.
  */
 #ifdef _UNICOS
-# define SCM2PTR(x) ((int) (x) >> 3)
-# define PTR2SCM(x) (((SCM) (x)) << 3)
+# define SCM2PTR(x) ((SCM_UNPACK (x)) >> 3)
+# define PTR2SCM(x) (SCM_PACK ((x) << 3))
 # define SCM_POINTERS_MUNGED
 #else
-# define SCM2PTR(x) (x)
-# define PTR2SCM(x) ((SCM) (x))
+# define SCM2PTR(x) (SCM_UNPACK (x))
+# define PTR2SCM(x) (SCM_PACK (x))
 #endif /* def _UNICOS */
 
 ^L



These were the bugs that I found just at the very beginning of the
compilation process, when trying to compile the files alist, arbiters, 
async and backtrace.  I hope to report more as soon as my time
allows.  Please note that I also added a couple of SCM_UNPACKS around SCM
values that were compared using == and !=, but I assume that these patches
are not supposed to get into guile.

Just if you are interested, here's the definitions for SCM, SCM_PACK and
SCM_UNPACK that I have used so far:

typedef union { scm_bits_t n; } SCM;
static SCM scm_pack(scm_bits_t b) { SCM s; s.n = b; return s; }
#define SCM_UNPACK(x) ((x).n)
#define SCM_PACK(x) (scm_pack (x))


Best regards
Dirk Herrmann


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]