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 <address@hidden> 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