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 <raeb...@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
