Re: [Chicken-hackers] [PATCH] various scrutinizer bugfixes

[Felix]

> 
> I'm probably overlooking something, but it looks like the now-first
> "or" case (around line 1016) walks the types twice.
> Once by over-all-instantiations and then again in the anonymous
> procedure it passes as argument for "combine", which does
> (if (or exact all) (every (cut match1 t1 <>) (cdr t2)) #t).
> 
> Isn't it simpler/more correct to do this?
> 
> (if (or exact all)
>     (every (cut match1 t1 <>) (cdr t2))
>     (over-all-instantiations (cdr t2) typeenv (cut match1 t1 <>)))
> 
> IIUC, the reason all entries _must_ be walked is so they end up
> in "trail".  Is that correct?

Not in trail, but they all have to match with uninstantiated type-variables.
For example:

(: foo (forall (a) ((list-of a) -> a)))

then calling foo with a union type like "(or T1 ...)" must match every
case in "exact" mode (used in specialization). Otherwise the first T
would unify the type variable "a" with some type, and the next match
with T2 would use those type-variable bindings (already instantiated
for the first T).  "over-all-instantiations" matches each type in a
type-set, then un-instantiates the type-variable bindings and matches
the next. If some match succeeded and we are not in "exact" mode, then
all is well. If in "exact" mode, we have to match again with the
type-variables bound to an "(or ...)" type constructed from all
possible instantiations. Did I say that it is complicated?

> 
> I won't pretend to fully understand it but the bits I do understand look
> fine and it works (no "make check" errors, and indeed the irregex
> warnings are gone except for two that really are bugs in irregex),
> so I've gone ahead and cherry-picked it (my earlier comment can be
> looked at later, if it's even correct).  It's now on master.

Thanks very much. 

> 
> I noticed there are still some other warnings left:
> 
> == library.scm ==
> 
> Warning: at toplevel:
>   assignment of value of type `(procedure current-input-port (#!rest) 
> undefined)' to toplevel variable `current-input-port' does not match declared 
> type `(procedure current-input-port (#!optional port) port)'
> 
> Warning: at toplevel:
>   assignment of value of type `(procedure current-output-port (#!rest) 
> undefined)' to toplevel variable `current-output-port' does not match 
> declared type `(procedure current-output-port (#!optional port) port)'
> 
> Warning: at toplevel:
>   assignment of value of type `(procedure current-error-port (#!rest) 
> undefined)' to toplevel variable `current-error-port' does not match declared 
> type `(procedure current-error-port (#!optional port) port)'
> 
> Warning: at toplevel:
>   assignment of value of type `(procedure current-exception-handler (#!rest) 
> undefined)' to toplevel variable `current-exception-handler' does not match 
> declared type `(procedure current-exception-handler (#!optional (procedure 
> (*) noreturn)) procedure)'
> 

The reason for this is that these procedures are "faked" parameters - they 
behave like
parameters and are declared as such in types.db, but I notice now that they do 
not
return the new value when set. A patch fixing these will be posted in a short 
while.

> == posixunix.scm ==
> 
> Note: at toplevel:
>   expression returns a result of type `(procedure ttysize1772 (fixnum (or 
> boolean pointer) (or boolean pointer)) . *)', but is declared to return 
> `(procedure (fixnum pointer pointer) fixnum)', which is not a subtype
> 

This is caused by a wrong types.db entry for the internal pointer-type checking
routine.

> 
> I also see and-let* still gives bogus(?) warnings (chicken-install.scm shows
> one, for example)
> 

That's right. There can't be much done about it, besides not using
"and-let*".  The warnings are just "notes" and indicate an unneeded
test which *may* be unintended. Any syntax that expands into
conditionals that check non-boolean values for being true will trigger
these notes. These notes are only shown in verbose mode, though.


cheers,
felix