Re: [Chicken-hackers] [PATCH] Return the result of EXP from (assert EXP)

[Peter Bex]

On Fri, Nov 15, 2013 at 03:40:45PM +1300, Evan Hanson wrote:
> The documentation for `assert` says the result of (assert EXP) is EXP,
> but it currently expands to (##core#undefined). This patch makes the
> code behave like the docs say, since that's the more useful behavior
> IMHO.

Thanks, that's a lot better.  Pushed.

> For reasons I don't fully grok yet, this causes the scrutinizer to catch
> an additional case in scrutiny-tests-2.scm, which is indeed a valid
> scrutiny ("scrutin", single unit of scrutiny?), so I've added that to
> the expected output as well; please let me know if you see anything
> obviously funky going on there.

I'm afraid there's still a bug in the scrutinizer.  The preceding "list?"
predicate seems to be coded wrong: the pair is in the asserted list for
things that should match the predicate, however it doesn't match.

This test is bad and probably has been since the beginning of the
scrutinizer.  I assume this has the same root cause as #1039:
https://bugs.call-cc.org/ticket/1039

Fun thing: if you correct the test by moving "p" to the second list,
it still passes!


Now, knowing this, I can explain the reason there's a difference after
you changed the assert macro (which shouldn't be even considering the
broken test):

In the original situation, the preceding "pair?" predicate gets asserted
directly on "p", thereby triggering the types.db rule for "pair?".
Because it is a predicate, if the check passes, the variable is
immediately assumed to be of that type (ie, pair, which is).

By introducing a temporary variable, the scrutinizer only applies the
asserted type to the temporary variable.  It's not clever enough to
understand that it can "lift" this type to the original variable from
which the temporary variable got its value.  This means the type gets
"trapped" by the LET in the assert macro's expansion.

Having said that, I have no idea how to fix this, right now.  There's
not really a type hierarchy, and the handling is pretty hairy and ad-hoc.
I think we should attempt to fix this before releasing 4.9.0, though!

I've filed bug #1063 to track this.  We could correct the test now,
but we should fix the root cause, see it fail and *then* correct the
test.  And we should try to devise a test which passes in the new
situation but fails in the old one.

Cheers,
Peter
-- 
http://www.more-magic.net