chicken-hackers
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Chicken-hackers] PATCH: wrong type for alist-delete!


From: Peter Bex
Subject: Re: [Chicken-hackers] PATCH: wrong type for alist-delete!
Date: Thu, 11 Feb 2016 22:06:13 +0100
User-agent: Mutt/1.5.21 (2010-09-15)

On Thu, Jan 14, 2016 at 03:14:50PM +0100, Jörg F. Wittenberger wrote:
> Hi all,
> 
> the srfi document wants a different type for alist-delete! than types.db
> declares.

After all the discussion at the start, this thread kind of petered out.
So let's try again.  Attached is a patch that got a bit larger as I was
working on it and spotted more and more inconsistencies and problems:

1) The types declared for memq, memv, member, assq, assv and [r]assoc
were also too specific, asserting that they accepted a list of pairs of
the same object types in each car and the same object types in
each cdr of the pairs in the list.  Especially the type for assoc
was ambitious but completely wrong :)

2) I noticed a lot of these alist- procedures were marked "#:enforce",
but they won't enforce that the list is a proper list, nor that it
contains only pairs: If it can stop early it will, without verifying
anything after the found pair.  The "assoc" definition didn't have this
annotation, which I think is correct, so I've also removed it from the
others.

3) I've made the types of all higher order alist and list traversing
procedures such that the predicate receives the argument that was 
passed by the user first, and the thing in the list that it's comparing
against second.  This meant I had to change two procedure bodies in
data-structures.scm.  We had a short discussion about this on IRC, and
we concluded that SRFI-1 is explicit about argument order while our
data-structures documentation isn't explicit:

 "The comparison procedure is used to compare the element keys ki of
   alist's entries to the key parameter in this way: (= key ki).
   Thus, one can reliably remove all entries of alist whose key is
   greater than five with (alist-delete 5 alist <)"

If this is too large a change, I'd be okay with changing the types so
they're (procedure (* a) *) instead of (procedure (a *) *), but since
we don't specify anything about this, I'd argue any program which
relies on argument order is wrong anyway.  At least this way we get
a modicum of type sanity checking, which is worthwhile IMO.

4) I noticed that the type of alist-cons was completely wrong:
    (alist-cons a b lst) isn't equivalent to (cons a (cons b lst)),
    but to (cons (cons a b) lst)!  Also, I took the opportunity to
    mark it #:pure, because it's just two conses, and cons is pure too.
   Come to think of it, would it be worthwhile to just rewrite it to
    cons calls using a "specialization" for (* * *)?

The attached patches are for master and chicken-5.  SRFI-1 got moved
out to an egg for chicken-5, so if this commit is applied, the srfi-1
changes should (probably manually?) be extracted and applied to the
srfi-1 egg as well.

Cheers,
Peter

Attachment: 0001-Fix-type-signatures-of-a-few-alist-procedures.chicken-5.patch
Description: Text Data

Attachment: 0001-Fix-type-signatures-of-a-few-alist-procedures.master.patch
Description: Text Data

Attachment: signature.asc
Description: Digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]