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: Fri, 12 Feb 2016 20:34:15 +0100
User-agent: Mutt/1.5.21 (2010-09-15)

On Fri, Feb 12, 2016 at 08:50:21AM +0100, Peter Bex wrote:
> It occurred to me that maybe the scrutinizer is smart enough to merge
> lists of pairs of (known) mixed types, and it actually is!
> 
> (use srfi-1)
> (print (assoc 'x '((1 . 2) (#f . 4)) (lambda (x y) (> x y))))
> 
> When compiled, this will tell you:
> Warning: at toplevel:
>   (test.scm:2) in procedure call to `assoc', expected argument #3 of type 
> `(procedure ((or false fixnum) symbol) *)', but was given an argument of type 
> `(procedure (number number) boolean)'
> 
> So I guess my patch needlessly weakens the type declarations.  I'll send
> a new patch before next week that applies the same kind of aggressively
> specific type to the alist procedures.

With the attached patch, you get the same message, but with the arguments
reversed which is, as discussed, more consistent:

Warning: at toplevel:
  (test.scm:3) in procedure call to `assoc', expected argument #3 of type 
`(procedure (symbol (or false fixnum)) *)' but was given an argument of type 
`(procedure (number number) boolean)'

And if you change srfi-1 to data-structures, and assoc to rassoc,
the current master doesn't give a compile time warning, but with this
patch you'll get the same warning with both CHICKEN versions:

Warning: at toplevel:
  (test.scm:2) in procedure call to `rassoc', expected argument #3 of type 
`(procedure (symbol (or false fixnum)) *)' but was given an argument of type 
`(procedure (number number) boolean)'

Now, whether the signature of "rassoc" should differ from that of "assoc"
in core is another matter.  But at least we get better error checking
now, and potentially better performance too.

I've also simplified the (forall (a b) ...) for memq, memv and the
CHICKEN 5 version of member to (forall (a) ...), because the "a" type
didn't add anything.  For those procedures accepting a comparison
procedure, it does matter, so there I've kept it.  I've augmented the
type information for rassoc, alist-cons, alist-copy, and alist-delete[!]
so the types are very precise.  I also noticed that alist-update
included an extra argument in its type after the comparison procedure,
which the code does not accept, so I've removed that 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]