Re: [Chicken-hackers] [PATCH] adding support for letrec*

[Peter Bex]

On Thu, Aug 01, 2013 at 12:44:28PM +0200, Felix wrote:
> The attached patch adds support for "letrec*".

Thanks a lot!

> A few issues that should be noted:
> 
> - It is possible to detect access to uninitialized letrec as required
>   in R6RS (but not in R7RS) and done in a few R5RS Schemes
>   (e.g. Scheme48). This has an acceptable cost in interpreted code but
>   may be too expensive for compiled code.

Here's a thought: could it be done by putting an annotation in the
variable's plist in WALK, when processing ##core#letrec?  It could list
the preceding variables of the same letrec (those it shouldn't referr to).
Then the scrutinizer might be able to give off a warning (or error)
once it sees the value is coming from a preceding letrec-binding.

Doing it at runtime sounds trickier.

On second thought, this is a weird corner of the spec.
This happens in Scheme48:

> (let ((a 1)) (letrec ((a 2) (b a)) b))
Error: LETREC variable used before its value has been produced

If letrec really is to let like letrec* is to let*, then it should
see the outer a, I think.  However, the spec explicitly says its
an error, so I think we should try to emulate this behaviour as well.

I'll try my hand at a patch for this.

> - I have added some rather silly little tests. Better tests are probably
>   worth adding.

I don't know of any better tests.  We can grow them as needed, when we
find bugs or edge cases that need to be nailed down.

> - It turned out that the optimizer will reorder bindings and blindly
>   propagate values, as in this example:
> 
>   (letrec ((foo 1)
>            (bar foo))
>      bar))             ; <- will return 1
> 
>   Wether this is desirable and what should be done here is not
>   entirely clear to me. Personally, I don't care that much about this,
>   as it should only be an issue in case of incorrect code (which is
>   silently "repaired" in this case).

If my suggestion about annotating variables works out, this problem
should be resolved, I think.

> I have run all tests and so far things seem to work ok.

Same here.  I've pushed this code, and we'll see what Salmonella
says in the daily diff.

> If "letrec*" is intended to be used in the chicken-core, then it will
> be essential to provide a bootstrapping tarball (in other words, a
> development snapshot) and perhaps even add the change to stability, to
> increase the chance that bootstrapping tarballs are available with
> which to compile the system.

I think it's about time we tagged 4.8.2.  I think this can happen soon
enough.  4.8.2 has an insane list of changes since 4.8.1 already.

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