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

[Peter Bex]

On Mon, Sep 19, 2011 at 01:16:01PM +0200, Felix wrote:
> This commit fixes several problems with the scrutinizer:
> 
> - instantiations of type-variables need to be done over all alternatives
>   in a union ("or") type

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?

> - swapped order of "or"-type matching to handle cases where
>   matching two union types failed where they shouldn't
> - resolution of type-variables may require type-simplification
> - subtype-checking uses existing typeenv machinery
> - commented out unused typecheck-generation code for the time being
> - adds a few testcase
> 
> Note that without this patch the current "master" version of chicken
> produces incorrect and disturbing warnings when compiling itself
> (specifically "irregex.scm").
> 
> This patch is also available as commit 4c574b04 in the "felix-pending"
> branch.

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.

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)'

== 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

The one from posixunix seems a bug in the scrutinizer, the other ones
are probably shortcomings in the flow analysis.

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

Cheers,
Peter
-- 
http://sjamaan.ath.cx
--
"The process of preparing programs for a digital computer
 is especially attractive, not only because it can be economically
 and scientifically rewarding, but also because it can be an aesthetic
 experience much like composing poetry or music."
                                                        -- Donald Knuth