Fwd: guile-1.8 1.8.5+1-4 MIGRATED to testing
FYI, the Debian Lenny release will include Guile 1.8.5. Thanks especially to Rob for making this happen, but also to many others who helped with bug reporting, fixing and advice. Neil -- Forwarded message -- From: Debian testing watch <[EMAIL PROTECTED]> Date: 2008/8/31 Subject: guile-1.8 1.8.5+1-4 MIGRATED to testing To: [EMAIL PROTECTED] FYI: The status of the guile-1.8 source package in Debian's testing distribution has changed. Previous version: 1.8.4+1-2 Current version: 1.8.5+1-4 -- This email is automatically generated; the Debian Release Team <[EMAIL PROTECTED]> is responsible. See http://release.debian.org/testing-watch/ for more information.
Re: [PATCH] Avoid `SCM_VALIDATE_LIST ()'
2008/9/1 Han-Wen Nienhuys <[EMAIL PROTECTED]>:
>
> On a tangent, is anyone still seriously considering to run Emacs atop GUILE?
Running a whole Emacs on top of Guile? - no.
Running some Emacs Lisp code on top of Guile? - yes.
But I admit that what I have in mind is still vaporware right now. So
I guess that if Guile's current infrastructure for trying to support
Elisp became a serious obstacle to progress, we should not rule out
dropping it.
> + for (; !SCM_NULL_OR_NIL_P (lst); lst = SCM_CDR (lst))
> +{
> + SCM_VALIDATE_CONS (2, lst);
>
> Looks cleaner to use SCM_CONS_P (or whatever it is called) as loop guard,
> so it is obviously correct, and crash if the lst is not properly terminated
> after the loop (- perhaps only if we're not compiling in optimizing mode).
I don't think we should do that. I agree that the code would _look_
cleaner and more obviously correct, but in fact the code _is_ correct,
and changing it would lose the Elisp support. And I don't think the
code as it stands is (anywhere near) so obscure as to justify losing
that.
Regards,
Neil
Re: [PATCH] Avoid `SCM_VALIDATE_LIST ()'
Hi,
Han-Wen Nienhuys <[EMAIL PROTECTED]> writes:
> - return scm_c_memq (x, lst);
> + for (; !SCM_NULL_OR_NIL_P (lst); lst = SCM_CDR (lst))
> +{
> + SCM_VALIDATE_CONS (2, lst);
>
> Looks cleaner to use SCM_CONS_P (or whatever it is called) as loop guard,
> so it is obviously correct, and crash if the lst is not properly terminated
> after the loop (- perhaps only if we're not compiling in optimizing mode).
Two things: `SCM_NULL_OR_NIL_P ()' is different from `scm_is_pair ()',
and `SCM_NULL_OR_NIL_P ()' can be passed any object so it does work if
LST is a dotted list.
> On a tangent, is anyone still seriously considering to run Emacs atop GUILE?
There's Ken Reaburn's attempt at http://www.mit.edu/~raeburn/guilemacs/ ,
and there's also the Elisp support that's under `lang'. I don't think
the former is really maintained. The latter isn't actively maintained
either but I think it's in a pretty good shape. Neil?
> - SCM from_here;
> + SCM from_here, hare;
>
> you could do the init to lst right here. IMO it's neater not to have
> uninitialized
> memory locations during program execution.
I find it nicer to have initialization right before the first use. I'd
write it like this:
for (from_here = hare = lst;
!SCM_NULL_OR_NIL_P (from_here);
from_here = SCM_CDR (from_here))
but I was trying to minimize changes.
> - Copyright (C) 2000, 2001, 2006 Free Software Foundation, Inc.
> + Copyright (C) 2000, 2001, 2006, 2008 Free Software Foundation, Inc.
>
> Can we do this in one fell swoop, adding 2008 to all files?
I wouldn't do that. I think updating the copyright year *when* a change
is made is better: it allows people to see at a glance whether a file
has been changed at all recently and avoids pointless commits. I use
this Emacs hook, which makes it painless:
(add-hook 'write-file-hooks 'copyright-update)
However, the GNU maintainer's guide (see (info "(maintain) Copyright
Notices")) prefers the other way:
To update the list of year numbers, add each year in which you have
made nontrivial changes to the package. (Here we assume you're using a
publicly accessible revision control server, so that every revision
installed is also immediately and automatically published.) When you
add the new year, it is not required to keep track of which files have
seen significant changes in the new year and which have not. It is
recommended and simpler to add the new year to all files in the
package, and be done with it for the rest of the year.
Thanks,
Ludo'.
Re: [PATCH] Avoid `SCM_VALIDATE_LIST ()'
2008/9/1 Ludovic Courtès <[EMAIL PROTECTED]>:
> Hello,
>
> This is a followup to this discussion:
>
> http://thread.gmane.org/gmane.lisp.guile.devel/7194
>
> The attached patch changes several list-related functions
reverse!, memq, memv, member, filter, filter!
SRFI-1: concatenate, concatenate!, member, remove, remove!
> so that they
> don't validate their input with `SCM_VALIDATE_LIST ()' since it's O(n).
I'm afraid I don't get your rationale, because all these functions are
O(n) anyway. (For reverse*, filter* and concatenate* I believe this
is obvious. For mem* and remove*, they should always be O(n) in
practice because it would be stupid to design a list structure where
the element being looked for or removed was not equally likely to be
anywhere along the list.)
I know you gave an example in the above-referenced thread, but IMO
that was a pathological one. Did you find the use of
SCM_VALIDATE_LIST () causing a problem in real practical code?
> A side-effect (besides performance improvements) is that all these
> functions will now happily traverse circular lists, and will silently
> deal with dotted lists. This is acceptable behavior IMO.
Are you sure about traversing circular lists? From my reading of your
patch, I would expect:
(memq 'not-in-the-list some-circular-list)
=>
(don't know, still waiting...)
:-)
What do reverse* do on a circular list? What do concatenate* do?
There are a few more specific comments below, but on balance, at the
moment, I'm seeing more disadvantages here than benefit...
Regards,
Neil
> diff --git a/NEWS b/NEWS
> index c2bed17..cfcd43b 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -56,6 +56,13 @@ When you use GDS to evaluate Scheme code from Emacs, you
> can now use
> This makes these internal functions technically not callable from
> application code.
>
> +** Remove argument type checking with `list?' in some list-related functions
It's easy to read that as just "Remove argument type checking". Could
we say "Improve scalability of some list-related functions" instead,
and leave the type checking detail to the following para?
> +
> +Several list-related functions (e.g., `memq', `list-copy', etc.) used
> +R5RS `list?' to validate their arguments. However, `list?' has linear
> +complexity, so these functions have been changed to not resort to
> +`list?'.
> +
> ** `guile-config link' now prints `-L$libdir' before `-lguile'
> ** Fix memory corruption involving GOOPS' `class-redefinition'
> ** Fix build issue on Tru64 and ia64-hp-hpux11.23 (`SCM_UNPACK' macro)
> diff --git a/libguile/list.c b/libguile/list.c
> index a1a79a4..8b0a2e4 100644
> --- a/libguile/list.c
> +++ b/libguile/list.c
> @@ -1,4 +1,4 @@
> -/* Copyright (C) 1995,1996,1997,2000,2001,2003,2004
> +/* Copyright (C) 1995,1996,1997,2000,2001,2003,2004,2008
> * Free Software Foundation, Inc.
> *
> * This library is free software; you can redistribute it and/or
> @@ -367,15 +367,19 @@ SCM_DEFINE (scm_reverse_x, "reverse!", 1, 1, 0,
> "@code{reverse!}")
> #define FUNC_NAME s_scm_reverse_x
> {
> - SCM_VALIDATE_LIST (1, lst);
>if (SCM_UNBNDP (new_tail))
> new_tail = SCM_EOL;
>else
> -SCM_VALIDATE_LIST (2, new_tail);
> +SCM_ASSERT (scm_is_pair (new_tail) || SCM_NULL_OR_NIL_P (new_tail),
> + new_tail, 2, FUNC_NAME);
Why does new_tail need to satisfy this condition? Please either add a
comment to explain, or just remove.
>while (!SCM_NULL_OR_NIL_P (lst))
> {
> - SCM old_tail = SCM_CDR (lst);
> + SCM old_tail;
> +
> + SCM_VALIDATE_CONS (1, lst);
> +
> + old_tail = SCM_CDR (lst);
>SCM_SETCDR (lst, new_tail);
>new_tail = lst;
>lst = old_tail;
Note that if SCM_VALIDATE_CONS fails half way through the "list", the
input list will have been destructively modified such that it is
neither the same as when it started, nor a complete reversed list. Is
that a concern?
> @@ -546,7 +550,8 @@ SCM_DEFINE (scm_list_copy, "list-copy", 1, 0, 0,
>SCM * fill_here;
>SCM from_here;
>
> - SCM_VALIDATE_LIST (1, lst);
> + SCM_ASSERT (scm_is_pair (lst) || SCM_NULL_OR_NIL_P (lst),
> + lst, 1, FUNC_NAME);
Why impose this condition specifically on the head of the provided
list? Please either explain or remove.
>
>newlst = SCM_EOL;
>fill_here = &newlst;
> @@ -613,8 +618,13 @@ SCM_DEFINE (scm_memq, "memq", 2, 0, 0,
> "returned.")
> #define FUNC_NAME s_scm_memq
> {
> - SCM_VALIDATE_LIST (2, lst);
> - return scm_c_memq (x, lst);
> + for (; !SCM_NULL_OR_NIL_P (lst); lst = SCM_CDR (lst))
> +{
> + SCM_VALIDATE_CONS (2, lst);
> + if (scm_is_eq (SCM_CAR (lst), x))
> + return lst;
> +}
> + return SCM_BOOL_F;
> }
> #undef FUNC_NAME
>
> @@ -629,9 +639,9 @@ SCM_DEFINE (scm_memv, "memv", 2, 0, 0,
> "returned.")
> #define FUNC_NAME s_scm_memv
> {
> - SCM_VALIDATE_LIST (2, lst);
>for (; !SCM_NULL_OR_NIL_P (lst); lst = SCM_CDR (lst))
> {
> +
Re: [PATCH] Avoid `SCM_VALIDATE_LIST ()'
Hi Neil,
"Neil Jerram" <[EMAIL PROTECTED]> writes:
>> so that they
>> don't validate their input with `SCM_VALIDATE_LIST ()' since it's O(n).
>
> I'm afraid I don't get your rationale, because all these functions are
> O(n) anyway.
You're right, but it's always better to traverse the list once rather
than twice. :-)
It doesn't feel right to impose that overhead "just" for the sake of
type-checking.
Type-checking using `list?' in these cases was actually overzealous
since neither RnRS nor SRFI-1 require it.
Note: We could change these functions to still diagnose what `list?'
diagnoses while avoiding `SCM_VALIDATE_LIST ()' such that only one list
traversal is done. It boils down to implementing the tortoise and the
hare in each function, as shown in the `list-copy' patch.
> Did you find the use of SCM_VALIDATE_LIST () causing a problem in real
> practical code?
What does that mean? Real practical code using `memq' behaves just as
if it called `memq' twice, that's it. Whether that is a "problem"
depends on how often that happens, how long the list is, and how long
you're willing to wait. :-)
>> A side-effect (besides performance improvements) is that all these
>> functions will now happily traverse circular lists, and will silently
>> deal with dotted lists. This is acceptable behavior IMO.
>
> Are you sure about traversing circular lists? From my reading of your
> patch, I would expect:
>
> (memq 'not-in-the-list some-circular-list)
> =>
> (don't know, still waiting...)
Yes, that's what I meant by "happily traverse circular lists". :-)
> What do reverse* do on a circular list? What do concatenate* do?
They keep reversing or concatenating---and it takes long! :-)
> There are a few more specific comments below, but on balance, at the
> moment, I'm seeing more disadvantages here than benefit...
Again, if the disadvantage is that circular lists are not diagnosed, we
can add tortoise-and-hare protection to each function while still
avoiding double traversal. I'm not sure it's worth it, though.
>> +** Remove argument type checking with `list?' in some list-related functions
>
> It's easy to read that as just "Remove argument type checking". Could
> we say "Improve scalability of some list-related functions" instead,
> and leave the type checking detail to the following para?
Right.
>> @@ -367,15 +367,19 @@ SCM_DEFINE (scm_reverse_x, "reverse!", 1, 1, 0,
>> "@code{reverse!}")
>> #define FUNC_NAME s_scm_reverse_x
>> {
>> - SCM_VALIDATE_LIST (1, lst);
>>if (SCM_UNBNDP (new_tail))
>> new_tail = SCM_EOL;
>>else
>> -SCM_VALIDATE_LIST (2, new_tail);
>> +SCM_ASSERT (scm_is_pair (new_tail) || SCM_NULL_OR_NIL_P (new_tail),
>> +new_tail, 2, FUNC_NAME);
>
> Why does new_tail need to satisfy this condition? Please either add a
> comment to explain, or just remove.
It's a mechanical change: the code used to check for a proper list, I
just changed it to check for a list (i.e., '() or a pair).
> Note that if SCM_VALIDATE_CONS fails half way through the "list", the
> input list will have been destructively modified such that it is
> neither the same as when it started, nor a complete reversed list. Is
> that a concern?
(I think that was `reverse!'.)
Portable RnRS code and code written without looking at `list.c' must use
`list?' before calling `reverse!' to protect from such situations. So,
to me, that's not a concern.
Now, one could argue that there's code out there that relies on Guile's
undocumented over-restriction on input arguments. That's always
possible, but hopefully sufficiently rare.
>> @@ -546,7 +550,8 @@ SCM_DEFINE (scm_list_copy, "list-copy", 1, 0, 0,
>>SCM * fill_here;
>>SCM from_here;
>>
>> - SCM_VALIDATE_LIST (1, lst);
>> + SCM_ASSERT (scm_is_pair (lst) || SCM_NULL_OR_NIL_P (lst),
>> + lst, 1, FUNC_NAME);
>
> Why impose this condition specifically on the head of the provided
> list?
So that "(list-copy 1)" raises an exception, rather than being silently
ignored (as is the case with `list-copy' from `srfi-1.c'). Again, a
mechanical change.
Thanks,
Ludo'.
Re: [PATCH] Avoid `SCM_VALIDATE_LIST ()'
Ludovic Courtès escreveu: >> On a tangent, is anyone still seriously considering to run Emacs atop GUILE? > > There's Ken Reaburn's attempt at http://www.mit.edu/~raeburn/guilemacs/ , > and there's also the Elisp support that's under `lang'. I don't think > the former is really maintained. The latter isn't actively maintained > either but I think it's in a pretty good shape. Neil? What is the intended use case of running Elisp in GUILE ? Is anyone using it for anything? >> - Copyright (C) 2000, 2001, 2006 Free Software Foundation, Inc. >> + Copyright (C) 2000, 2001, 2006, 2008 Free Software Foundation, Inc. >> >> Can we do this in one fell swoop, adding 2008 to all files? > > I wouldn't do that. I think updating the copyright year *when* a change > is made is better: it allows people to see at a glance whether a file > has been changed at all recently I think that git log FILE is the reliable and precise way to check that. The headers are so much boilerplate that I pretty much ignore all of them. > and avoids pointless commits. We could add 2008 to all files in a single commit, and avoid poluting diffs with header blah blah for an entire year. > However, the GNU maintainer's guide (see (info "(maintain) Copyright > Notices")) prefers the other way: > > To update the list of year numbers, add each year in which you have > made nontrivial changes to the package. (Here we assume you're using a > publicly accessible revision control server, so that every revision > installed is also immediately and automatically published.) When you > add the new year, it is not required to keep track of which files have > seen significant changes in the new year and which have not. It is > recommended and simpler to add the new year to all files in the > package, and be done with it for the rest of the year. -- Han-Wen Nienhuys - [EMAIL PROTECTED] - http://www.xs4all.nl/~hanwen
Re: [PATCH] Avoid `SCM_VALIDATE_LIST ()'
Neil Jerram escreveu: > 2008/9/1 Ludovic Courtès <[EMAIL PROTECTED]>: >> Hello, >> >> This is a followup to this discussion: >> >> http://thread.gmane.org/gmane.lisp.guile.devel/7194 >> >> The attached patch changes several list-related functions > > reverse!, memq, memv, member, filter, filter! > SRFI-1: concatenate, concatenate!, member, remove, remove! > >> so that they >> don't validate their input with `SCM_VALIDATE_LIST ()' since it's O(n). > > I'm afraid I don't get your rationale, because all these functions are > O(n) anyway. (For reverse*, filter* and concatenate* I believe this > is obvious. For mem* and remove*, they should always be O(n) in > practice because it would be stupid to design a list structure where > the element being looked for or removed was not equally likely to be > anywhere along the list.) All these functions are O(n), but by prefixing the list check you're doubling the running time (eg. in the case of reverse!). If you are doing memq? for something you already know to somewhere in front of the list, the list? check will slow it down much worse. -- Han-Wen Nienhuys - [EMAIL PROTECTED] - http://www.xs4all.nl/~hanwen
