Re: the new gc asserts in master

[Ludovic Courtès]

Hello Han-Wen,

Han-Wen Nienhuys <address@hidden> writes:

> Over the last weeks I have seen little discussion on your patches, eg.

Please, refrain from resorting to personal attacks.

>   commit 582a4997abc8b34ac6caf374fda8ea3ac65bd571
>   Author: Ludovic Courtès <address@hidden>
>   Date:   Mon Aug 25 11:20:02 2008 +0200
>
>       Use $(GCC_CFLAGS) for `-Werror' et al. so that it's not used to compile
>       Gnulib code.
>
>   commit c95514b3b41c8e335ada863f8abb99cc4af9abe1
>   Author: Ludovic Courtès <address@hidden>
>   Date:   Thu Aug 14 00:15:03 2008 +0200
>
>       Remove the now useless `qthreads.m4'.

These patches are trivial IMO and contain a descriptive git log and
ChangeLog entry.  Compare this with the ten-or-so patches that you
committed, which included a far-from-trivial rework of the GC.

>From my observations, Neil, Kevin and I have always worked this way,
although this was never formally specified.  Perhaps Neil can comment?

> The commit message says,
>
>     Set SRCPROP{PLIST,COPY} through a macro, so SCM_DEBUG_CELL_ACCESSES 
> compiles.
>
> how much clearer do you want this message to be?

Sorry if I missed something but my understanding was that you were
referring to a GC fix, which it isn't.  Re-reading the thread, it seems
I indeed missed the point, and I apologize.  For my defense, I'd say
that I didn't expect a "GC cleanup" to touch all that.

>>>> even the lazy smob case I wrote about here:
>>>>
>>>>     http://thread.gmane.org/gmane.lisp.guile.user/6372
>>> I would classify the use of mark bits outside of the mark phase as outside
>>> of the defined API.  If you want to have weak pointer semantics, use
>>> a weak hashtable, or implement reference counting on the C side.
>> 
>> That's a reasonable argument, but it's something we should not change
>> without discussing it first.  For instance, it may be important to study
>> why Guile-GNOME had to resort to this, and how it could avoid it,
>> instead of just gratuitously breaking it.
>
> I'm not suggesting to change without discussing; this message rather
> is the start of the discussion.

Good.  However, my understanding of Andy's message was that the "GC
cleanups" already changed that, making Guile-GNOME's recipe fail.

> From ccd010e15ec0ddf285b75911739e85866d2d865c Mon Sep 17 00:00:00 2001
> From: Han-Wen Nienhuys <address@hidden>
> Date: Wed, 27 Aug 2008 10:48:06 -0300
> Subject: [PATCH] Kludge around x86-64 GC runtime checks.
>
> 2008-08-27  Han-Wen Nienhuys  <address@hidden>
>
>       * gc.c (scm_i_gc): Don't sanity check numbers on x64, while we
>       investigate a real fix.
>
> ---
>  libguile/gc.c      |    4 ++--
>  2 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/libguile/gc.c b/libguile/gc.c
> index 8e8769c..a0b3080 100644
> --- a/libguile/gc.c
> +++ b/libguile/gc.c
> @@ -597,9 +597,9 @@ scm_i_gc (const char *what)
>    scm_i_sweep_all_segments ("GC", &scm_i_gc_sweep_stats);
>    scm_check_deprecated_memory_return ();
>  
> +#if (SCM_DEBUG_CELL_ACCESSES == 0 && SCM_SIZEOF_UNSIGNED_LONG == 4)

x86-64 is not the only arch with 4-byte long long integers.

I'm still in favor of "git revert" since the log message makes it clear
which patch was reverted and why.  "We" can then take our time and work
out a proper fix, and finally re-merge the patch plus its fix.
Furthermore, in the eventuality where none of us eventually finds a fix,
`master' is left in the previous state, which is better IMO.

Would you like that solution?

Thanks,
Ludo'.