id	summary	reporter	owner	description	type	status	priority	milestone	component	version	resolution	keywords	cc	difficulty
847	##sys#foreign-pointer-argument is declared to accept a pointer only, but is possibly invoked with a boolean	sjamaan		"When you use let-location or define-foreign-variable and declare a pointer type, you can set it to a NULL value by setting it to #f in Scheme and this works correctly.  However, the scrutinizer thinks that's bad:

{{{
$ cat test.scm
(define-foreign-variable bar c-pointer)

(set! bar #f)

(let-location ((x c-pointer #f))
  ((foreign-lambda void ""printf"" c-string c-pointer) ""%p\n"" x))
$ csc -scrutinize test.scm
Warning: at toplevel:
  (test.scm:3) in procedure call to `##sys#foreign-pointer-argument', expected argument #1 of type `pointer', but was given an argument of type `boolean'

Warning: at toplevel:
  (test.scm:5) in procedure call to `##sys#foreign-pointer-argument', expected argument #1 of type `pointer', but was given an argument of type `boolean'
}}}

The underlying problem is that {{{##sys#foreign-pointer-argument}}} is declared as accepting only pointer types (which is correct, it will barf otherwise), but the initializer/set value is {{{#f}}}, a boolean which is allowed as a value representing a NULL pointer.  The {{{if}}} check in library.scm:1077 takes care of this situation, but the scrutinizer is unable to verify that the boolean isn't {{{#t}}} when it passes the if and takes the {{{##sys#foreign-pointer-argument}}} branch (and it might be!).  This output be seen more clearly when calling ""csc -debug 2 test.scm"".

We could do a few things here:

1) Add a {{{##core#the}}} assertion that states the argument is definitely a pointer.  That kind of kills the scrutinizer's effectiveness at warning, moving the error to runtime (it's also incorrect in case anything else is passed).
2) Map {{{#t}}} to {{{#f}}} (a NULL pointer eventually in the C translation). That's inconsistent with the rest of core.
3) Map {{{#t}}} to an error.  This still moves the error to a runtime one but keeps the scrutinizer warnings for all non-boolean types.
4) Keep it the way it is.  That will give bogus warnings on this kind of code.
5) Remove {{{##sys#foreign-pointer-argument}}} from types.db or change it to accept a boolean.  The former again gets rid of the advantage of having the scrutinizer and the latter is blatantly incorrect.
6) Change the {{{##sys#foreign-pointer-argument}}} to accept booleans and only return {{{C_SCHEME_FALSE}}} and barf on {{{C_SCHEME_TRUE}}}.
7) Augment the type system with yet another special case for #f, adding a ""false"" type that is a subtype of ""boolean"".

I don't know which option I like most (or dislike least). 7 is definitely the most ""thorough"" option, similar to the special handling of null, pair and list(-of) types, but it raises the complexity of the scrutinizer another notch.  Other acceptable solutions would probably either be 3 or 6, which both basically come down to the same thing.  I think whatever we do, we should probably postpone it until after the next release unless someone comes up with a clever, noninvasive alternative of course :)"	defect	closed	major	4.9.0	scrutinizer	4.7.x	fixed