compiling with -DSCM_DEBUG=1

compiling with -DSCM_DEBUG=1

[Ken Raeburn]

__scm.h suggests defining SCM_DEBUG this as a way of turning on all  
debugging options, so I tried adding -DSCM_DEBUG=1 to CPPFLAGS on the  
configure command line, using the latest git version (3bcf189).  But  
the test programs in the tree don't build on my Mac, because  
scm_i_expensive_validation_check isn't exported from the library.  The  
patch below seems to fix it.

Then -- both on the Mac and on GNU/Linux (RHEL4) -- compiling occam- 
channel.scm fails with:

> GUILE_AUTO_COMPILE=0 ../meta/uninstalled-env guile-tools compile -o  
> "ice-9/occam-channel.go" "../../module/ice-9/occam-channel.scm"
> Non-pair accessed with SCM_C[AD]R: `#<procedure #f (#{class\  
> 1972}# . #{initargs\ 1971}#)>'
> make[2]: *** [ice-9/occam-channel.go] Abort trap

--- a/libguile/gc.h
+++ b/libguile/gc.h
@@ -248,7 +248,7 @@ SCM_INTERNAL void scm_i_ensure_marking(void);
  SCM_API int scm_debug_cell_accesses_p;
  SCM_API int scm_expensive_debug_cell_accesses_p;
  SCM_API int scm_debug_cells_gc_interval ;
-void scm_i_expensive_validation_check (SCM cell);
+SCM_API void scm_i_expensive_validation_check (SCM cell);
  #endif

  SCM_INTERNAL scm_i_pthread_mutex_t scm_i_gc_admin_mutex;

Re: compiling with -DSCM_DEBUG=1

[Ludovic Courtès]

Hi!

Ken Raeburn <raeburn@raeburn.org> writes:

> --- a/libguile/gc.h
> +++ b/libguile/gc.h
> @@ -248,7 +248,7 @@ SCM_INTERNAL void scm_i_ensure_marking(void);
>  SCM_API int scm_debug_cell_accesses_p;
>  SCM_API int scm_expensive_debug_cell_accesses_p;
>  SCM_API int scm_debug_cells_gc_interval ;
> -void scm_i_expensive_validation_check (SCM cell);
> +SCM_API void scm_i_expensive_validation_check (SCM cell);
>  #endif
>
>  SCM_INTERNAL scm_i_pthread_mutex_t scm_i_gc_admin_mutex;

Please apply this one.

I guess Guile is seldom compiled with `SCM_DEBUG' defined.

Thanks,
Ludo'.

Re: compiling with -DSCM_DEBUG=1

[Ken Raeburn]

On Sep 1, 2009, at 15:35, Ludovic Courtès wrote:
>> --- a/libguile/gc.h
>> +++ b/libguile/gc.h
>> @@ -248,7 +248,7 @@ SCM_INTERNAL void scm_i_ensure_marking(void);
>> SCM_API int scm_debug_cell_accesses_p;
>> SCM_API int scm_expensive_debug_cell_accesses_p;
>> SCM_API int scm_debug_cells_gc_interval ;
>> -void scm_i_expensive_validation_check (SCM cell);
>> +SCM_API void scm_i_expensive_validation_check (SCM cell);
>> #endif
>>
>> SCM_INTERNAL scm_i_pthread_mutex_t scm_i_gc_admin_mutex;
>
> Please apply this one.

Done.  The C code now builds; the Scheme compilation bug is still  
there, though.

Ken

Re: compiling with -DSCM_DEBUG=1

[Ken Raeburn]

On Sep 3, 2009, at 17:04, Ken Raeburn wrote:
> [...] Scheme compilation bug is still there, though.

I'm still not sure where the bug is, but here's what I've traced  
through so far;

The error is happening in eval.i.c, line number in the high 800s (I've  
got a bunch of tracing lines added in my copy):

   while (!scm_is_null (args))
     {
       /* More arguments than specifiers => CLASS != ENV */
       SCM class_of_arg = scm_class_of (SCM_CAR (args));
       if (!scm_is_eq (class_of_arg, SCM_CAR (z)))   // <== HERE!
	goto next_method;
       args = SCM_CDR (args);
       z = SCM_CDR (z);
     }

At this line, at one point during the compilation of debugger.scm, z  
is not a pair.  (Earlier the problem was triggering somewhere else for  
me, now it's in debugger.scm; not sure why.)

Some tracing of the containing do loop indicates that these are the  
sorts of values used before the error happens:

do-loop with hash_value 0 arg1 (#<struct 10a6370:1092060> #<struct  
10a6d40:10b94e0>) z (no-method)
do-loop with hash_value 0 arg1 (#<struct 10a6370:115cb0> #<struct  
10a6d40:115a60>) z (no-method)
do-loop with hash_value 0 arg1 (#<struct 10a6370:1010de0> #<struct  
10a6d40:1010db0>) z (#<struct 10a73a0:10a6370> #<struct  
10a0bc0:10a6d40> . #<procedure #f (#{gf\ 2116}# #{m\ 2117}#)>)
do-loop with hash_value 0 arg1 (#<primitive-generic eqv?> #<struct  
10a6d40:1099310>) z (#<struct 10a73a0:10a6370> #<struct  
10a0bc0:10a6d40> . #<procedure #f (#{gf\ 2116}# #{m\ 2117}#)>)
[...]
do-loop with hash_value 3 arg1 (#<<generic> apply-generic (0)>  
#<<method> (<generic> <top>) 10bbd80>) z (#<<entity-class> <generic>  
10a59a0> #<<class> <method> 10a6420> . #<procedure #f (#{gf\ 2116}# #{m 
\ 2117}#)>)

But then:
do-loop with hash_value 0 arg1 (#<<entity-class> <generic> 10a59a0>  
#:name compute-applicable-methods) z (#<<class> <entity-class>  
10a6910> #<<class> <keyword> 10a3b90> . #<procedure #f (#{class\  
2660}# . #{initargs\ 2659}#)>)

Here, the arg list has three elements instead of the two in the calls  
shown above, but the initial 'z' value isn't any longer; they're  
walked in parallel in that while loop, 'z' runs out first, and SCM_CAR  
is applied to a non-pair:

Non-pair accessed with SCM_C[AD]R: `#<procedure #f (#{class\ 2660}# .  
#{initargs\ 2659}#)>'

Assuming the bug doesn't jump out as obvious to someone from this,  
I'll dig into it a little more later.

Ken

Re: compiling with -DSCM_DEBUG=1

[Ken Raeburn]

Okay, I found some more time to look into it.  I have a patch that now  
passes "make && make install && make check" with SCM_DEBUG==1.

There was an additional issue in goops.c where SCM_C[AD]R get used  
with objects that have just been verified to be structs, not pairs.   
Since there don't seem to be high-level macros for modifying those  
fields, I used the low-level cell-word access macros instead.

Ken

     Fix run-time errors in building and tests with SCM_DEBUG==1.

     * eval.i.c (CEVAL): Stop comparing arg list and specifiers when  
out of
       specifiers.
     * objects.c (scm_mcache_lookup_cmethod): Likewise.

     * goops.c (scm_sys_modify_instance, scm_sys_modify_class): Don't  
use
       SCM_CAR and SCM_CDR to access fields of a class object.

diff --git a/libguile/eval.i.c b/libguile/eval.i.c
index 25abf6c..c9d3beb 100644
--- a/libguile/eval.i.c
+++ b/libguile/eval.i.c
@@ -847,7 +847,7 @@ dispatch:
  		{
  		  SCM args = arg1; /* list of arguments */
  		  z = SCM_SIMPLE_VECTOR_REF (method_cache, hash_value);
-		  while (!scm_is_null (args))
+		  while (scm_is_pair (z) && !scm_is_null (args))
  		    {
  		      /* More arguments than specifiers => CLASS != ENV */
  		      SCM class_of_arg = scm_class_of (SCM_CAR (args));
diff --git a/libguile/goops.c b/libguile/goops.c
index d1beab3..eecb652 100644
--- a/libguile/goops.c
+++ b/libguile/goops.c
@@ -1652,12 +1652,13 @@ SCM_DEFINE (scm_sys_modify_instance, "%modify- 
instance", 2, 0, 0,
     */
    SCM_CRITICAL_SECTION_START;
    {
-    SCM car = SCM_CAR (old);
-    SCM cdr = SCM_CDR (old);
-    SCM_SETCAR (old, SCM_CAR (new));
-    SCM_SETCDR (old, SCM_CDR (new));
-    SCM_SETCAR (new, car);
-    SCM_SETCDR (new, cdr);
+    scm_t_bits word0, word1;
+    word0 = SCM_CELL_WORD_0 (old);
+    word1 = SCM_CELL_WORD_1 (old);
+    SCM_SET_CELL_WORD_0 (old, SCM_CELL_WORD_0 (new));
+    SCM_SET_CELL_WORD_1 (old, SCM_CELL_WORD_1 (new));
+    SCM_SET_CELL_WORD_0 (new, word0);
+    SCM_SET_CELL_WORD_1 (new, word1);
    }
    SCM_CRITICAL_SECTION_END;
    return SCM_UNSPECIFIED;
@@ -1674,13 +1675,14 @@ SCM_DEFINE (scm_sys_modify_class, "%modify- 
class", 2, 0, 0,

    SCM_CRITICAL_SECTION_START;
    {
-    SCM car = SCM_CAR (old);
-    SCM cdr = SCM_CDR (old);
-    SCM_SETCAR (old, SCM_CAR (new));
-    SCM_SETCDR (old, SCM_CDR (new));
+    scm_t_bits word0, word1;
+    word0 = SCM_CELL_WORD_0 (old);
+    word1 = SCM_CELL_WORD_1 (old);
+    SCM_SET_CELL_WORD_0 (old, SCM_CELL_WORD_0 (new));
+    SCM_SET_CELL_WORD_1 (old, SCM_CELL_WORD_1 (new));
      SCM_STRUCT_DATA (old)[scm_vtable_index_vtable] = SCM_UNPACK (old);
-    SCM_SETCAR (new, car);
-    SCM_SETCDR (new, cdr);
+    SCM_SET_CELL_WORD_0 (new, word0);
+    SCM_SET_CELL_WORD_1 (new, word1);
      SCM_STRUCT_DATA (new)[scm_vtable_index_vtable] = SCM_UNPACK (new);
    }
    SCM_CRITICAL_SECTION_END;
diff --git a/libguile/objects.c b/libguile/objects.c
index e82fb9d..7f20899 100644
--- a/libguile/objects.c
+++ b/libguile/objects.c
@@ -131,7 +131,7 @@ scm_mcache_lookup_cmethod (SCM cache, SCM args)
        long j = n;
        z = SCM_SIMPLE_VECTOR_REF (methods, i);
        ls = args; /* list of arguments */
-      if (!scm_is_null (ls))
+      if (!scm_is_null (ls) && scm_is_pair (z))
  	do
  	  {
  	    /* More arguments than specifiers => CLASS != ENV */
@@ -140,7 +140,7 @@ scm_mcache_lookup_cmethod (SCM cache, SCM args)
  	    ls = SCM_CDR (ls);
  	    z = SCM_CDR (z);
  	  }
-	while (j-- && !scm_is_null (ls));
+	while (j-- && !scm_is_null (ls) && scm_is_pair (z));
        /* Fewer arguments than specifiers => CAR != CLASS or `no- 
method' */
        if (!scm_is_pair (z)
            || (!SCM_CLASSP (SCM_CAR (z)) && !scm_is_symbol (SCM_CAR  
(z))))

Re: compiling with -DSCM_DEBUG=1

[Ken Raeburn]

BTW, the bdw-gc branch with my patch and SCM_DEBUG==1 still fails  
tests on my Mac.

In guardians.c, line 169, SCM_CAR is applied to a non-pair:

Running popen.test
Running ports.test

scm_error_pair_access
Non-pair accessed with SCM_C[AD]R: `ERROR: In procedure symbol->string:
ERROR: Wrong type argument in position 1 (expecting symbol):  
#<guardian 124e300 (reachable: 15 unreachable: 1)>
FAIL: check-guile

I use a modified scm_error_pair_access() that prints the function's  
name (as seen above) and then sleeps a while, so I can attach gdb and  
get this stack trace before letting it resume:

#0  0x9487546e in __semwait_signal ()
#1  0x948752ef in nanosleep$UNIX2003 ()
#2  0x948cae71 in sleep$UNIX2003 ()
#3  0x002ea943 in scm_error_pair_access (non_pair=0x11d9180) at ../../ 
libguile/pairs.c:50
#4  0x002c8e35 in finalize_guarded (ptr=0x11d91f0,  
finalizer_data=0x11d9188) at ../../libguile/guardians.c:169
#5  0x000a0866 in GC_invoke_finalizers ()
#6  0x000a2ce9 in GC_generic_malloc_many ()
#7  0x000ab01a in GC_malloc ()
#8  0x002ea987 in scm_cell [inlined] () at inline.h:66
#9  0x002ea987 in scm_cons (x=0x9487546e, y=0x9487546e) at ../../ 
libguile/pairs.c:66
#10 0x002d0f5a in scm_make_list (n=0x4, init=0xb2920) at ../../ 
libguile/list.c:121
#11 0x002c8bbc in finalize_guarded (ptr=0x12615e8,  
finalizer_data=0x12615a0) at ../../libguile/guardians.c:125
#12 0x000a0866 in GC_invoke_finalizers ()
#13 0x000a2ce9 in GC_generic_malloc_many ()
#14 0x000ab01a in GC_malloc ()
#15 0x002ea987 in scm_cell [inlined] () at inline.h:66
#16 0x002ea987 in scm_cons (x=0x9487546e, y=0x9487546e) at ../../ 
libguile/pairs.c:66
#17 0x002d0f5a in scm_make_list (n=0x4, init=0xb2920) at ../../ 
libguile/list.c:121
#18 0x002c8bbc in finalize_guarded (ptr=0x239ab00,  
finalizer_data=0x239aaa8) at ../../libguile/guardians.c:125
#19 0x000a0866 in GC_invoke_finalizers ()
#20 0x000a2ce9 in GC_generic_malloc_many ()
#21 0x000ab01a in GC_malloc ()
#22 0x002ea987 in scm_cell [inlined] () at inline.h:66
#23 0x002ea987 in scm_cons (x=0x9487546e, y=0x9487546e) at ../../ 
libguile/pairs.c:66
#24 0x002d0f5a in scm_make_list (n=0x4, init=0xb2920) at ../../ 
libguile/list.c:121
#25 0x002c8bbc in finalize_guarded (ptr=0x22b2fe8,  
finalizer_data=0x22b2f90) at ../../libguile/guardians.c:125
#26 0x000a0866 in GC_invoke_finalizers ()
#27 0x000a2ce9 in GC_generic_malloc_many ()
#28 0x000ab01a in GC_malloc ()
#29 0x002ea987 in scm_cell [inlined] () at inline.h:66
#30 0x002ea987 in scm_cons (x=0x9487546e, y=0x9487546e) at ../../ 
libguile/pairs.c:66
#31 0x002d0f5a in scm_make_list (n=0x4, init=0xb2920) at ../../ 
libguile/list.c:121
#32 0x002c8bbc in finalize_guarded (ptr=0x22c3f58,  
finalizer_data=0x22c3f10) at ../../libguile/guardians.c:125
#33 0x000a0866 in GC_invoke_finalizers ()
#34 0x000a2ce9 in GC_generic_malloc_many ()
#35 0x000ab01a in GC_malloc ()
#36 0x002ea987 in scm_cell [inlined] () at inline.h:66
#37 0x002ea987 in scm_cons (x=0x9487546e, y=0x9487546e) at ../../ 
libguile/pairs.c:66
#38 0x002d0f5a in scm_make_list (n=0x4, init=0xb2920) at ../../ 
libguile/list.c:121
#39 0x002c8bbc in finalize_guarded (ptr=0x11db6a0,  
finalizer_data=0x11db648) at ../../libguile/guardians.c:125
#40 0x000a0866 in GC_invoke_finalizers ()
#41 0x000a2ce9 in GC_generic_malloc_many ()
#42 0x000ab01a in GC_malloc ()
#43 0x002ea987 in scm_cell [inlined] () at inline.h:66
#44 0x002ea987 in scm_cons (x=0x9487546e, y=0x9487546e) at ../../ 
libguile/pairs.c:66
#45 0x002d0f5a in scm_make_list (n=0x4, init=0xb2920) at ../../ 
libguile/list.c:121
#46 0x002c8bbc in finalize_guarded (ptr=0x122b8f0,  
finalizer_data=0x122b8a8) at ../../libguile/guardians.c:125
#47 0x000a0866 in GC_invoke_finalizers ()
#48 0x000a2ce9 in GC_generic_malloc_many ()
#49 0x000ab01a in GC_malloc ()
(More stack frames follow...)
[...]
#108 0x002d0f5a in scm_make_list (n=0x4, init=0xb2920) at ../../ 
libguile/list.c:121
#109 0x002c8bbc in finalize_guarded (ptr=0x11d7e08,  
finalizer_data=0x11d7d88) at ../../libguile/guardians.c:125
#110 0x000a0866 in GC_invoke_finalizers ()
#111 0x000a2ce9 in GC_generic_malloc_many ()
#112 0x000ab01a in GC_malloc ()
#113 0x0033f01e in scm_cell [inlined] () at inline.h:120
#114 0x0033f01e in vm_debug_engine (vp=0x6a1fa0, program=<value  
temporarily unavailable, due to optimizations>, argv=0xbfffa688,  
nargs=7) at inline.h:523
#115 0x00331f5e in scm_c_vm_run (vm=0x69b2e8, program=0x4, argv=0x4,  
nargs=4) at ../../libguile/vm.c:381
[...]

Ken

Re: compiling with -DSCM_DEBUG=1

[Andy Wingo]

On Sun 06 Sep 2009 02:37, Ken Raeburn <raeburn@raeburn.org> writes:

> BTW, the bdw-gc branch with my patch and SCM_DEBUG==1 still fails tests
> on my Mac.
>
> In guardians.c, line 169, SCM_CAR is applied to a non-pair:
>
> Running popen.test
> Running ports.test
>
> scm_error_pair_access
> Non-pair accessed with SCM_C[AD]R: `ERROR: In procedure symbol->string:
> ERROR: Wrong type argument in position 1 (expecting symbol): #<guardian
> 124e300 (reachable: 15 unreachable: 1)>
> FAIL: check-guile
>
> I use a modified scm_error_pair_access() that prints the function's name
> (as seen above) and then sleeps a while, so I can attach gdb and  get
> this stack trace before letting it resume:
>
> #0  0x9487546e in __semwait_signal ()
> #1  0x948752ef in nanosleep$UNIX2003 ()
> #2  0x948cae71 in sleep$UNIX2003 ()
> #3  0x002ea943 in scm_error_pair_access (non_pair=0x11d9180) at ../../
> libguile/pairs.c:50
> #4  0x002c8e35 in finalize_guarded (ptr=0x11d91f0,
> finalizer_data=0x11d9188) at ../../libguile/guardians.c:169
> #5  0x000a0866 in GC_invoke_finalizers ()
> #6  0x000a2ce9 in GC_generic_malloc_many ()
> #7  0x000ab01a in GC_malloc ()
> #8  0x002ea987 in scm_cell [inlined] () at inline.h:66
> #9  0x002ea987 in scm_cons (x=0x9487546e, y=0x9487546e) at ../../
> libguile/pairs.c:66
> #10 0x002d0f5a in scm_make_list (n=0x4, init=0xb2920) at ../../
> libguile/list.c:121
> #11 0x002c8bbc in finalize_guarded (ptr=0x12615e8,
> finalizer_data=0x12615a0) at ../../libguile/guardians.c:125
> #12 0x000a0866 in GC_invoke_finalizers ()

Thanks for catching this bug. I'm sure Ludovic will look into it. We
should not release a 1.9.3 with bugs like this one.

A
-- 
http://wingolog.org/

Re: compiling with -DSCM_DEBUG=1

[Ludovic Courtès]

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

Hi Ken,

Ken Raeburn <raeburn@raeburn.org> writes:

> BTW, the bdw-gc branch with my patch and SCM_DEBUG==1 still fails
> tests on my Mac.
>
> In guardians.c, line 169, SCM_CAR is applied to a non-pair:
>
> Running popen.test
> Running ports.test
>
> scm_error_pair_access
> Non-pair accessed with SCM_C[AD]R: `ERROR: In procedure symbol->string:
> ERROR: Wrong type argument in position 1 (expecting symbol):
> #<guardian 124e300 (reachable: 15 unreachable: 1)>

Does that mean it’s this whole string that’s accessed with SCM_C[AD]R?

> I use a modified scm_error_pair_access() that prints the function's
> name (as seen above)

Hmm, I don’t see the function name, except ‘symbol->string’ above, but
I’d expect it to be part of the string that’s accessed as a pair.

> and then sleeps a while, so I can attach gdb and get this stack trace
> before letting it resume:
>
> #0  0x9487546e in __semwait_signal ()
> #1  0x948752ef in nanosleep$UNIX2003 ()
> #2  0x948cae71 in sleep$UNIX2003 ()
> #3  0x002ea943 in scm_error_pair_access (non_pair=0x11d9180) at ../../
> libguile/pairs.c:50
> #4  0x002c8e35 in finalize_guarded (ptr=0x11d91f0,
> finalizer_data=0x11d9188) at ../../libguile/guardians.c:169

I cannot reproduce it here without SCM_DEBUG but with this simple patch
instead:


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

diff --git a/libguile/guardians.c b/libguile/guardians.c
index 580e212..ec00df6 100644
--- a/libguile/guardians.c
+++ b/libguile/guardians.c
@@ -1,4 +1,4 @@
-/* Copyright (C) 1998,1999,2000,2001, 2006, 2008 Free Software Foundation, Inc.
+/* Copyright (C) 1998,1999,2000,2001, 2006, 2008, 2009 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
@@ -159,13 +159,16 @@ finalize_guarded (GC_PTR ptr, GC_PTR finalizer_data)
       g->zombies = zombies;
     }
 
-  if (proxied_finalizer != SCM_BOOL_F)
+  if (scm_is_true (proxied_finalizer))
     {
       /* Re-register the finalizer that was in place before we installed this
 	 one.  */
       GC_finalization_proc finalizer, prev_finalizer;
       GC_PTR finalizer_data, prev_finalizer_data;
 
+      if (!scm_is_pair (proxied_finalizer))
+	abort ();
+
       finalizer = (GC_finalization_proc) SCM2PTR (SCM_CAR (proxied_finalizer));
       finalizer_data = SCM2PTR (SCM_CDR (proxied_finalizer));

[-- Attachment #3: Type: text/plain, Size: 22 bytes --]


Any hints?

Ludo’.

Re: compiling with -DSCM_DEBUG=1

[Ken Raeburn]

On Sep 7, 2009, at 05:22, Ludovic Courtès wrote:
>> Non-pair accessed with SCM_C[AD]R: `ERROR: In procedure symbol- 
>> >string:
>> ERROR: Wrong type argument in position 1 (expecting symbol):
>> #<guardian 124e300 (reachable: 15 unreachable: 1)>
>
> Does that mean it’s this whole string that’s accessed with SCM_C[AD]R?

I'm not sure... it should be printing a value after the quote; I guess  
it's encountering an error trying to print, as well.

>> I use a modified scm_error_pair_access() that prints the function's
>> name (as seen above)
>
> Hmm, I don’t see the function name, except ‘symbol->string’ above, but
> I’d expect it to be part of the string that’s accessed as a pair.

Sorry, I meant scm_error_pair_access() prints out  
"scm_error_pair_access" to let me know it's been called.

> I cannot reproduce it here without SCM_DEBUG but with this simple  
> patch
> instead:

> Any hints?

I would think that would do it, but I'm not seeing it either.  Still  
looking...
(BTW, for SCM_DEBUG=1 I also had to comment out a debugging check  
using SCM_GC_MARK_P in gc.c, since the macro doesn't exist any more.)

Ken

Re: compiling with -DSCM_DEBUG=1

[Neil Jerram]

Hi Ken,

I've been trying to reproduce the guardian finalisation problem that you
see with SCM_DEBUG==1 but, like Ludovic, I haven't had any luck.  With
SCM_DEBUG=1 for the whole build (plus the SCM_GC_MARK_P change), I'm
afraid my machine grinds to a halt when it gets to goops.c - i.e. it
never finishes compiling goops.c, even when left for several hours.  And
with a more focussed patch - setting SCM_DEBUG_PAIR_ACCESSES to 1 only
in guardians.c, and DEBUG_GUARDIANS - I don't see any problem.

So now I've come back to your patch - since it mostly looks good, and I
imagine could help goops.c - and have a question.

Ken Raeburn <raeburn@raeburn.org> writes:

> --- a/libguile/eval.i.c
> +++ b/libguile/eval.i.c
> @@ -847,7 +847,7 @@ dispatch:
>  		{
>  		  SCM args = arg1; /* list of arguments */
>  		  z = SCM_SIMPLE_VECTOR_REF (method_cache, hash_value);
> -		  while (!scm_is_null (args))
> +		  while (scm_is_pair (z) && !scm_is_null (args))
>  		    {
>  		      /* More arguments than specifiers => CLASS != ENV */
>  		      SCM class_of_arg = scm_class_of (SCM_CAR (args));

Your change looks straightforward, but the "More arguments than
specifiers" comment makes it look as though it might have been
intentional to let the !scm_is_pair (z) case through.

I don't fully understand what the comment means, though.  Do you?
Specifically, what is the sense of `CLASS != ENV'?

This concern/question also applies to the two other similar changes.

> diff --git a/libguile/goops.c b/libguile/goops.c
> index d1beab3..eecb652 100644
> --- a/libguile/goops.c
> +++ b/libguile/goops.c
> @@ -1652,12 +1652,13 @@ SCM_DEFINE (scm_sys_modify_instance, "%modify-
> instance", 2, 0, 0,
>     */
>    SCM_CRITICAL_SECTION_START;
>    {
> -    SCM car = SCM_CAR (old);
> -    SCM cdr = SCM_CDR (old);
> -    SCM_SETCAR (old, SCM_CAR (new));
> -    SCM_SETCDR (old, SCM_CDR (new));
> -    SCM_SETCAR (new, car);
> -    SCM_SETCDR (new, cdr);
> +    scm_t_bits word0, word1;
> +    word0 = SCM_CELL_WORD_0 (old);
> +    word1 = SCM_CELL_WORD_1 (old);
> +    SCM_SET_CELL_WORD_0 (old, SCM_CELL_WORD_0 (new));
> +    SCM_SET_CELL_WORD_1 (old, SCM_CELL_WORD_1 (new));
> +    SCM_SET_CELL_WORD_0 (new, word0);
> +    SCM_SET_CELL_WORD_1 (new, word1);
>    }
>    SCM_CRITICAL_SECTION_END;
>    return SCM_UNSPECIFIED;
> @@ -1674,13 +1675,14 @@ SCM_DEFINE (scm_sys_modify_class, "%modify-
> class", 2, 0, 0,
>
>    SCM_CRITICAL_SECTION_START;
>    {
> -    SCM car = SCM_CAR (old);
> -    SCM cdr = SCM_CDR (old);
> -    SCM_SETCAR (old, SCM_CAR (new));
> -    SCM_SETCDR (old, SCM_CDR (new));
> +    scm_t_bits word0, word1;
> +    word0 = SCM_CELL_WORD_0 (old);
> +    word1 = SCM_CELL_WORD_1 (old);
> +    SCM_SET_CELL_WORD_0 (old, SCM_CELL_WORD_0 (new));
> +    SCM_SET_CELL_WORD_1 (old, SCM_CELL_WORD_1 (new));
>      SCM_STRUCT_DATA (old)[scm_vtable_index_vtable] = SCM_UNPACK (old);
> -    SCM_SETCAR (new, car);
> -    SCM_SETCDR (new, cdr);
> +    SCM_SET_CELL_WORD_0 (new, word0);
> +    SCM_SET_CELL_WORD_1 (new, word1);
>      SCM_STRUCT_DATA (new)[scm_vtable_index_vtable] = SCM_UNPACK (new);
>    }
>    SCM_CRITICAL_SECTION_END;

These changes look good.  Would you like to commit them?

Regards,
        Neil

Re: compiling with -DSCM_DEBUG=1

[Ken Raeburn]

On Oct 18, 2009, at 18:44, Neil Jerram wrote:
> I've been trying to reproduce the guardian finalisation problem that  
> you
> see with SCM_DEBUG==1 but, like Ludovic, I haven't had any luck.  With
> SCM_DEBUG=1 for the whole build (plus the SCM_GC_MARK_P change), I'm
> afraid my machine grinds to a halt when it gets to goops.c - i.e. it
> never finishes compiling goops.c, even when left for several hours.   
> And
> with a more focussed patch - setting SCM_DEBUG_PAIR_ACCESSES to 1 only
> in guardians.c, and DEBUG_GUARDIANS - I don't see any problem.

I wouldn't be surprised if it's platform- and compiler-dependent.

I've seen cases in Emacs, too, where turning on debugging versions of  
macros causes compilation time to explode, especially if it results in  
macros using their arguments multiple times when the arguments  
themselves have now become complicated expressions.  Several hours is  
a new one on me, though...  Often it helps to define inline or static  
functions to encapsulate the work, instead of macros.

I haven't tried debug builds in a while.  Now that the path problems  
and other stuff have been taking care of, I should get back to that.   
I'll probably want debugging options for my work with Emacs.

> Your change looks straightforward, but the "More arguments than
> specifiers" comment makes it look as though it might have been
> intentional to let the !scm_is_pair (z) case through.
>
> I don't fully understand what the comment means, though.  Do you?
> Specifically, what is the sense of `CLASS != ENV'?

Unfortunately, I don't; I was hoping someone more familiar with it  
could check it.
Even if it's intended to fetch word 0 from a non-pair object at the  
end, SCM_CAR is the wrong way to do it.

>> diff --git a/libguile/goops.c b/libguile/goops.c

> These changes look good.  Would you like to commit them?

Sure, I'll do that in a little bit...

Ken

Re: compiling with -DSCM_DEBUG=1

[Andy Wingo]

On Mon 19 Oct 2009 15:52, Ken Raeburn <raeburn@raeburn.org> writes:

> On Oct 18, 2009, at 18:44, Neil Jerram wrote:
>
>> Your change looks straightforward, but the "More arguments than
>> specifiers" comment makes it look as though it might have been
>> intentional to let the !scm_is_pair (z) case through.
>>
>> I don't fully understand what the comment means, though.  Do you?
>> Specifically, what is the sense of `CLASS != ENV'?
>
> Unfortunately, I don't; I was hoping someone more familiar with it could
> check it.
> Even if it's intended to fetch word 0 from a non-pair object at the end,
> SCM_CAR is the wrong way to do it.

FWIW, this will change when GOOPS dispatch lands in the VM. So it might
not be worth worrying about.

Andy
-- 
http://wingolog.org/

Re: compiling with -DSCM_DEBUG=1

[Ken Raeburn]

At Andy's suggestion, re-posting the still-pending part that needs  
review.  Without these changes, the code in the loops applies SCM_CAR  
to non-pair objects.

GUILE_AUTO_COMPILE=0					\
	../meta/uninstalled-env			\
	guile-tools compile -Wunbound-variable -o "ice-9/debugger.go" "../../ 
guile/module/ice-9/debugger.scm"
Non-pair accessed with SCM_C[AD]R: `#<procedure #f (#{class\ 2865}# .  
#{initargs\ 2866}#)>'

This patch is my best guess, but I'm not very familiar with the code...

     SCM_DEBUG fix: Don't apply SCM_CAR to non-pairs when walking  
argument
     lists in method cache matching.

     * libguile/eval.i.c (CEVAL): Don't apply SCM_CAR to non-pairs when
       walking argument lists in method cache matching.
     * libguile/objects.c (scm_mcache_lookup_cmethod): Likewise.

diff --git a/libguile/eval.i.c b/libguile/eval.i.c
index d9ec6cd..e2e79c2 100644
--- a/libguile/eval.i.c
+++ b/libguile/eval.i.c
@@ -846,7 +846,7 @@ dispatch:
  		{
  		  SCM args = arg1; /* list of arguments */
  		  z = SCM_SIMPLE_VECTOR_REF (method_cache, hash_value);
-		  while (!scm_is_null (args))
+		  while (scm_is_pair (z) && !scm_is_null (args))
  		    {
  		      /* More arguments than specifiers => CLASS != ENV */
  		      SCM class_of_arg = scm_class_of (SCM_CAR (args));
diff --git a/libguile/objects.c b/libguile/objects.c
index f686c3a..09336cc 100644
--- a/libguile/objects.c
+++ b/libguile/objects.c
@@ -131,7 +131,7 @@ scm_mcache_lookup_cmethod (SCM cache, SCM args)
        long j = n;
        z = SCM_SIMPLE_VECTOR_REF (methods, i);
        ls = args; /* list of arguments */
-      if (!scm_is_null (ls))
+      if (!scm_is_null (ls) && scm_is_pair (z))
  	do
  	  {
  	    /* More arguments than specifiers => CLASS != ENV */
@@ -140,7 +140,7 @@ scm_mcache_lookup_cmethod (SCM cache, SCM args)
  	    ls = SCM_CDR (ls);
  	    z = SCM_CDR (z);
  	  }
-	while (j-- && !scm_is_null (ls));
+	while (j-- && !scm_is_null (ls) && scm_is_pair (z));
        /* Fewer arguments than specifiers => CAR != CLASS or `no- 
method' */
        if (!scm_is_pair (z)
            || (!SCM_CLASSP (SCM_CAR (z)) && !scm_is_symbol (SCM_CAR  
(z))))

Re: compiling with -DSCM_DEBUG=1

[Neil Jerram]

Ken Raeburn <raeburn@raeburn.org> writes:

> At Andy's suggestion, re-posting the still-pending part that needs
> review.  Without these changes, the code in the loops applies SCM_CAR
> to non-pair objects.
>
> GUILE_AUTO_COMPILE=0					\
> 	../meta/uninstalled-env			\
> 	guile-tools compile -Wunbound-variable -o "ice-9/debugger.go"
> "../../
> guile/module/ice-9/debugger.scm"
> Non-pair accessed with SCM_C[AD]R: `#<procedure #f (#{class\ 2865}#
> . #{initargs\ 2866}#)>'
>
> This patch is my best guess, but I'm not very familiar with the code...

I don't know...  Even though this code is destined for the dustbin soon
anyway, I'd prefer if we understood it!  I'll have another go at working
it out over the weekend.

   Neil

Re: compiling with -DSCM_DEBUG=1

[Ludovic Courtès]

Hello,

Ken Raeburn <raeburn@raeburn.org> writes:

> GUILE_AUTO_COMPILE=0					\
> 	../meta/uninstalled-env			\
> 	guile-tools compile -Wunbound-variable -o "ice-9/debugger.go"
> "../../
> guile/module/ice-9/debugger.scm"
> Non-pair accessed with SCM_C[AD]R: `#<procedure #f (#{class\ 2865}#
> . #{initargs\ 2866}#)>'

Can you come up with a simple test case that reproduces the problem?

Besides, the patch looks harmless to me.

Thanks,
Ludo’.

Re: compiling with -DSCM_DEBUG=1

[Neil Jerram]

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

> Ken Raeburn <raeburn@raeburn.org> writes:
>
>> At Andy's suggestion, re-posting the still-pending part that needs
>> review.  Without these changes, the code in the loops applies SCM_CAR
>> to non-pair objects.
>>
>> GUILE_AUTO_COMPILE=0					\
>> 	../meta/uninstalled-env			\
>> 	guile-tools compile -Wunbound-variable -o "ice-9/debugger.go"
>> "../../
>> guile/module/ice-9/debugger.scm"
>> Non-pair accessed with SCM_C[AD]R: `#<procedure #f (#{class\ 2865}#
>> . #{initargs\ 2866}#)>'
>>
>> This patch is my best guess, but I'm not very familiar with the code...
>
> I don't know...  Even though this code is destined for the dustbin soon
> anyway, I'd prefer if we understood it!  I'll have another go at working
> it out over the weekend.

OK, I think I mostly understand this now.  The method cache is a vector,
and this code expects each entry in the vector to look like

(TYPE1 ... ENV FORMALS FORM1 ...)

where TYPE1 ... are the types (classes) that the method is specialised
for, ENV is the environment where the method was defined, FORMALS are
the formals and FORM1 ... is the method's code.

So the comment "More arguments than specifiers => CLASS != ENV" means
"We don't have to check for reaching the end of the list of TYPEs,
because then we'll be comparing the class of the next actual arg against
ENV, and there is no possibility of those matching, so we'll exit the
loop."

Which makes sense, if the cache entry really does have the above form.
But apparently it doesn't.  I added this just inside the loops:

      if (!scm_is_pair (z))
	{
	  scm_write (z, scm_current_output_port ());
	  scm_newline (scm_current_output_port ());
	  scm_write (SCM_SIMPLE_VECTOR_REF (method_cache,
					    hash_value),
		     scm_current_output_port ());
	  scm_newline (scm_current_output_port ());
	  abort ();
	}

and then the build fails when it gets to compiling (ice-9 debugger):

GUILE_AUTO_COMPILE=0					\
	../meta/uninstalled-env			\
	guile-tools compile -Wunbound-variable -o "ice-9/debugger.go" "ice-9/debugger.scm"
#<procedure #f (#{class\ 2865}# . #{initargs\ 2866}#)>
(#<<class> <entity-class> 8c77990> #<<class> <keyword> 8c7a640> . #<procedure #f (#{class\ 2865}# . #{initargs\ 2866}#)>)

So there's at least this case where the cache entry has a different
form.  Actually I think this is a general change that Andy made a while
back - i.e. to form the old (ENV FORMALS FORM1 ...) part into a
procedure and store that in the cache instead.

Andy, can you confirm this?  Subject to that, I think that Ken's
proposed patch is correct, but it would also be good to remove or
clarify the "CLASS != ENV" comments, and to update the comments that
describe the cache entry layout (in objects.c and
oop/goops/dispatch.scm).

Thanks,
        Neil

Re: compiling with -DSCM_DEBUG=1

[Andy Wingo]

Hi,

On Sat 31 Oct 2009 16:42, Neil Jerram <neil@ossau.uklinux.net> writes:

> Neil Jerram <neil@ossau.uklinux.net> writes:
>
>> Ken Raeburn <raeburn@raeburn.org> writes:
>>
>>> At Andy's suggestion, re-posting the still-pending part that needs
>>> review.  Without these changes, the code in the loops applies SCM_CAR
>>> to non-pair objects.
>>>
>>> GUILE_AUTO_COMPILE=0					\
>>> 	../meta/uninstalled-env			\
>>> 	guile-tools compile -Wunbound-variable -o "ice-9/debugger.go"
>>> "../../
>>> guile/module/ice-9/debugger.scm"
>>> Non-pair accessed with SCM_C[AD]R: `#<procedure #f (#{class\ 2865}#
>>> . #{initargs\ 2866}#)>'
>>>
>>> This patch is my best guess, but I'm not very familiar with the code...
>>
>> I don't know...  Even though this code is destined for the dustbin soon
>> anyway, I'd prefer if we understood it!  I'll have another go at working
>> it out over the weekend.
>
> OK, I think I mostly understand this now.  The method cache is a vector,
> and this code expects each entry in the vector to look like
>
> (TYPE1 ... ENV FORMALS FORM1 ...)
[...]
> So there's at least this case where the cache entry has a different
> form.  Actually I think this is a general change that Andy made a while
> back - i.e. to form the old (ENV FORMALS FORM1 ...) part into a
> procedure and store that in the cache instead.
>
> Andy, can you confirm this?

This diagnosis sounds about right. Actually, you will never see the env
formals form1 ..., it's all in the new improper-list format. I haven't
determined if Ken's patch is correct, though.

Andy
-- 
http://wingolog.org/

Re: compiling with -DSCM_DEBUG=1

[Neil Jerram]

Andy Wingo <wingo@pobox.com> writes:

>> So there's at least this case where the cache entry has a different
>> form.  Actually I think this is a general change that Andy made a while
>> back - i.e. to form the old (ENV FORMALS FORM1 ...) part into a
>> procedure and store that in the cache instead.
>>
>> Andy, can you confirm this?
>
> This diagnosis sounds about right. Actually, you will never see the env
> formals form1 ..., it's all in the new improper-list format. I haven't
> determined if Ken's patch is correct, though.

Thanks, Andy.  I'm confident now that your patch is correct, Ken, so
please could you apply it?  Also please let me know if you're happy to
do the other changes (mostly comment updates) that I suggested to go
with it, or if you'd prefer me to do those.

Regards,
        Neil

Re: compiling with -DSCM_DEBUG=1

[Ken Raeburn]

On Nov 14, 2009, at 08:47, Neil Jerram wrote:
> Thanks, Andy.  I'm confident now that your patch is correct, Ken, so
> please could you apply it?  Also please let me know if you're happy to
> do the other changes (mostly comment updates) that I suggested to go
> with it, or if you'd prefer me to do those.

Sure, I'll look at updating the comments too.  Based on my current  
understanding, though, I'm seeing more code updates that might be  
desirable, like not checking SCM_CLASSP(SCM_CAR(z)) etc, if a non-pair  
is the one and only indication of hitting the end of the specifiers.   
Even if we don't change the code (since it still works), new comments  
reflecting my understanding would make the code seem non-sensical.  I  
just tried a quick check changing those tests to assert calls, and  
everything seems fine.  So, I'll revise my patch...

I may be overlooking something in my old email -- what were the non- 
comment updates you suggested?

Ken

Re: compiling with -DSCM_DEBUG=1

[Ken Raeburn]

On Nov 14, 2009, at 12:07, Ken Raeburn wrote:
> On Nov 14, 2009, at 08:47, Neil Jerram wrote:
>> Thanks, Andy.  I'm confident now that your patch is correct, Ken, so
>> please could you apply it?  Also please let me know if you're happy  
>> to
>> do the other changes (mostly comment updates) that I suggested to go
>> with it, or if you'd prefer me to do those.
>
> Sure, I'll look at updating the comments too.  Based on my current  
> understanding, though, I'm seeing more code updates that might be  
> desirable, like not checking SCM_CLASSP(SCM_CAR(z)) etc, if a non- 
> pair is the one and only indication of hitting the end of the  
> specifiers.

Here's my revised patch.  I've simplified the check, and it still  
passes the tests (except the options tests that were just committed  
with a log message indicating that they don't pass) and doesn't crash  
under SCM_DEBUG.  More details in the comments might be nice, I think,  
but I'm not familiar enough with that part of the code to fill them  
in, and I'd rather get back to my Emacs hacking than spend a lot more  
time on that right now.

Answering Ludo's question from Oct 31 that I meant to get back to  
sooner: No, I don't have a simple test case yet, other than building  
the tree and letting the compiler run; I'm not sure yet what needs to  
be done exactly to trigger the bug.

     SCM_DEBUG fix: Don't apply SCM_CAR to non-pairs when walking  
argument
     lists in method cache matching.

     * libguile/eval.i.c (CEVAL): Don't apply SCM_CAR to non-pairs when
       walking argument lists in method cache matching.  Don't check for
       CLASSP or symbol in the car slot, since the end of the specifier
       list is a non-pair.
     * libguile/objects.c (scm_mcache_lookup_cmethod): Likewise.  Update
       comments to reflect new structure of method cache entry.
     * module/oops/goops/dispatch.scm: Update comments here too.

diff --git a/libguile/eval.i.c b/libguile/eval.i.c
index 5b4604a..dffbe9f 100644
--- a/libguile/eval.i.c
+++ b/libguile/eval.i.c
@@ -839,21 +839,19 @@ dispatch:
  		{
  		  SCM args = arg1; /* list of arguments */
  		  z = SCM_SIMPLE_VECTOR_REF (method_cache, hash_value);
-		  while (!scm_is_null (args))
+		  /* More arguments than specifiers => z = CMETHOD, not a pair.
+		   * Fewer arguments than specifiers => CAR != CLASS or
+		   * `no-method'.  */
+		  while (scm_is_pair (z) && !scm_is_null (args))
  		    {
-		      /* More arguments than specifiers => CLASS != ENV */
  		      SCM class_of_arg = scm_class_of (SCM_CAR (args));
  		      if (!scm_is_eq (class_of_arg, SCM_CAR (z)))
  			goto next_method;
  		      args = SCM_CDR (args);
  		      z = SCM_CDR (z);
  		    }
-		  /* Fewer arguments than specifiers => CAR != CLASS */
                    if (!scm_is_pair (z))
                      goto apply_vm_cmethod;
-                  else if (!SCM_CLASSP (SCM_CAR (z))
-                           && !scm_is_symbol (SCM_CAR (z)))
-                    goto apply_memoized_cmethod;
  		next_method:
  		  hash_value = (hash_value + 1) & mask;
  		} while (hash_value != cache_end_pos);
diff --git a/libguile/objects.c b/libguile/objects.c
index f686c3a..b05c9c7 100644
--- a/libguile/objects.c
+++ b/libguile/objects.c
@@ -57,12 +57,12 @@ SCM scm_metaclass_operator;
   *
   * Format #1:
   * (SCM_IM_DISPATCH ARGS N-SPECIALIZED
- *   #((TYPE1 ... ENV FORMALS FORM ...) ...)
+ *   #((TYPE1 ... . CMETHOD) ...)
   *   GF)
   *
   * Format #2:
   * (SCM_IM_HASH_DISPATCH ARGS N-SPECIALIZED HASHSET MASK
- *   #((TYPE1 ... ENV FORMALS FORM ...) ...)
+ *   #((TYPE1 ... . CMETHOD) ...)
   *   GF)
   *
   * ARGS is either a list of expressions, in which case they
@@ -73,9 +73,6 @@ SCM scm_metaclass_operator;
   * SCM_IM_DISPATCH expressions in generic functions always
   * have ARGS = the symbol `args' or the iloc #@0-0.
   *
- * Need FORMALS in order to support varying arity.  This
- * also avoids the need for renaming of bindings.
- *
   * We should probably not complicate this mechanism by
   * introducing "optimizations" for getters and setters or
   * primitive methods.  Getters and setter will normally be
@@ -131,19 +128,18 @@ scm_mcache_lookup_cmethod (SCM cache, SCM args)
        long j = n;
        z = SCM_SIMPLE_VECTOR_REF (methods, i);
        ls = args; /* list of arguments */
-      if (!scm_is_null (ls))
+      /* More arguments than specifiers => z = CMETHOD, not a pair.
+       * Fewer arguments than specifiers => CAR != CLASS or `no- 
method'.  */
+      if (!scm_is_null (ls) && scm_is_pair (z))
  	do
  	  {
-	    /* More arguments than specifiers => CLASS != ENV */
  	    if (! scm_is_eq (scm_class_of (SCM_CAR (ls)), SCM_CAR (z)))
  	      goto next_method;
  	    ls = SCM_CDR (ls);
  	    z = SCM_CDR (z);
  	  }
-	while (j-- && !scm_is_null (ls));
-      /* Fewer arguments than specifiers => CAR != CLASS or `no- 
method' */
-      if (!scm_is_pair (z)
-          || (!SCM_CLASSP (SCM_CAR (z)) && !scm_is_symbol (SCM_CAR  
(z))))
+	while (j-- && !scm_is_null (ls) && scm_is_pair (z));
+      if (!scm_is_pair (z))
  	return z;
      next_method:
        i = (i + 1) & mask;
diff --git a/module/oop/goops/dispatch.scm b/module/oop/goops/ 
dispatch.scm
index 88abf80..6a450c1 100644
--- a/module/oop/goops/dispatch.scm
+++ b/module/oop/goops/dispatch.scm
@@ -53,9 +53,9 @@
  ;;; Method cache
  ;;;

-;; (#@dispatch args N-SPECIALIZED #((TYPE1 ... ENV FORMALS  
FORM1 ...) ...) GF)
+;; (#@dispatch args N-SPECIALIZED #((TYPE1 ... . CMETHOD) ...) GF)
  ;; (#@dispatch args N-SPECIALIZED HASHSET MASK
-;;             #((TYPE1 ... ENV FORMALS FORM1 ...) ...)
+;;             #((TYPE1 ... . CMETHOD) ...)
  ;;             GF)

  ;;; Representation

Re: compiling with -DSCM_DEBUG=1

[Neil Jerram]

Ken Raeburn <raeburn@raeburn.org> writes:

> I may be overlooking something in my old email -- what were the non-
> comment updates you suggested?

Actually nothing - misremembering on my part.  Sorry for the confusion.

    Neil

Re: compiling with -DSCM_DEBUG=1

[Neil Jerram]

Ken Raeburn <raeburn@raeburn.org> writes:

> Here's my revised patch.  I've simplified the check, and it still
> passes the tests (except the options tests that were just committed
> with a log message indicating that they don't pass) and doesn't crash
> under SCM_DEBUG.

Thanks.  The revised patch looks correct to me, so please commit it.

>  More details in the comments might be nice, I think,
> but I'm not familiar enough with that part of the code to fill them
> in, and I'd rather get back to my Emacs hacking than spend a lot more
> time on that right now.

Sounds good to me!

Regards,
        Neil

Re: compiling with -DSCM_DEBUG=1

[Ken Raeburn]

On Nov 15, 2009, at 17:25, Neil Jerram wrote:
> Ken Raeburn <raeburn@raeburn.org> writes:
>
>> Here's my revised patch.  I've simplified the check, and it still
>> passes the tests (except the options tests that were just committed
>> with a log message indicating that they don't pass) and doesn't crash
>> under SCM_DEBUG.
>
> Thanks.  The revised patch looks correct to me, so please commit it.

Andy's just changed a bunch of stuff affecting these files; I've  
remerged my changes, but I'm not sure if they're needed any more.   
I'll try to examine this further tomorrow.

Ken

Re: compiling with -DSCM_DEBUG=1

[Andy Wingo]

On Mon 16 Nov 2009 07:08, Ken Raeburn <raeburn@raeburn.org> writes:

> On Nov 15, 2009, at 17:25, Neil Jerram wrote:
>> Ken Raeburn <raeburn@raeburn.org> writes:
>>
>>> Here's my revised patch.  I've simplified the check, and it still
>>> passes the tests (except the options tests that were just committed
>>> with a log message indicating that they don't pass) and doesn't crash
>>> under SCM_DEBUG.
>>
>> Thanks.  The revised patch looks correct to me, so please commit it.
>
> Andy's just changed a bunch of stuff affecting these files; I've
> remerged my changes, but I'm not sure if they're needed any more.   I'll
> try to examine this further tomorrow.

Sorry about that. There is only one copy of that code now, and it is in
goops.c. I don't think I've changed the contents of that code though.

Regards,

Andy
-- 
http://wingolog.org/

Re: compiling with -DSCM_DEBUG=1

[Ken Raeburn]

On Nov 16, 2009, at 01:08, Ken Raeburn wrote:
> Andy's just changed a bunch of stuff affecting these files; I've  
> remerged my changes, but I'm not sure if they're needed any more.   
> I'll try to examine this further tomorrow.

I had to run the test suite rather than just finish the build to  
reproduce the failure, but it's still there.  After Andy's rearranging  
and rewriting of much code, the eval.i.c patch isn't needed, and the  
code to be changed in objects.c is now in goops.c.  My appropriately- 
rearranged patch has been committed...

Ken

Re: compiling with -DSCM_DEBUG=1

[Neil Jerram]

Ken Raeburn <raeburn@raeburn.org> writes:

> On Nov 16, 2009, at 01:08, Ken Raeburn wrote:
>> Andy's just changed a bunch of stuff affecting these files; I've
>> remerged my changes, but I'm not sure if they're needed any more.
>> I'll try to examine this further tomorrow.
>
> I had to run the test suite rather than just finish the build to
> reproduce the failure, but it's still there.  After Andy's rearranging
> and rewriting of much code, the eval.i.c patch isn't needed, and the
> code to be changed in objects.c is now in goops.c.  My appropriately- 
> rearranged patch has been committed...

Top marks for persistence!

     Neil