Re: [Chicken-hackers] [PATCH] Fix a logical bug in the optimizer

[felix . winkelmann]

> On Mon, Mar 27, 2017 at 08:37:05PM +0200, lemonboy wrote:
> > As promised here's the patch, sorry for the noise.
>
> I don't understand this code at all.  Why does it (touch) and then
> scan the first expression again, and what does "escaped" mean exactly?

The second scan of "first" was incorrect and this patch fixes that.
"Escaped" means that user code (possibly) called external code, so
all assumptions about toplevel bindings are invalid from now on.
Setting "escaped" to #t could actually exit from this pass completely, but
I assume it doesn't because of the (currently commented) out warning
that is issued, and so to allow scanning all code for this particular case.

This pass should probably be rewritten and simplifed.

>
> Besides, why does touch reset "previous" completely?  IIUC, this would
> mean something like the following results in keeping all the preceding
> definitions:
>
> (set! foo 1)
> (if (read) (print "read something") (print "read false"))
> (set! foo 2)
>
> The same with any procedure call, right?

I don't fully understand your remark, but the point is that the a conditional
results in an unknown execution path. A call on the other hand may
result in all possible things (access of the 1st def of "foo" above, for
example). As I said, this pass should be rewritten.

>
> Also, should this also treat the newly added "define-toplevel" core
> expression like it treats set!?

Yes, it should.


felix