Re: [PATCH] tree-il->scheme improvements

[Andy Wingo]

Hi Mark!

On Fri 02 Mar 2012 00:40, Mark H Weaver <address@hidden> writes:

>> Here's a significantly refactored version of my 'tree-il->scheme'
>> improvements.

All look great to me, please push.  Some time soon after you do it,
would you mind also handling the merge to master?

Only a few small points:

> +(define (decompile-tree-il e env opts)
> +
> +  (define use-derived-syntax?
> +    (cond ((memq #:use-derived-syntax? opts) => cadr)
> +          (else #t)))


WDYT about

   (define* (do-decompile e env #:key (avoid-lambda? #t) ... 
                          #:allow-other-keys)
     ...)

   (define (decompile-tree-il e env opts)
     (apply do-decompile e env opts))

instead of manually handling the keywords?

> +           (('if ('eqv? (? atom? v) ('quote a)) #t ('eqv? v ('quote b)))
> +            `(memv ,v '(,a ,b)))

This is not quite correct -- memv returns the rest of the list if it
matches.  Peval transforms memv in test context to the string of eqv?
conditionals, because it knows that a list is true -- but if the source
program contained (list (if (eqv? x 'a) #t (eqv? x 'b))), that's not the
same as (list (memv x '(a b))).  OTOH if you know you're in a test
context, this is a valid transformation to make.

> +           (('let ((v e)) ('case v clauses ...))
> +            (=> failure)

I never knew this => failure feature of match :)

> +    ;; Repeatedly strip suffix of the form "-N", where N is a string
> +    ;; that could be produced by number->string given a positive
> +    ;; integer.  In other words, the first digit of N may not be 0.
> +    (define compute-base-name
> +      (let ((digits (string->char-set "0123456789")))
> +        (define (base-name-string str)
> +          (let* ((i (string-skip-right str digits)))
> +            (if (and i (< (1+ i) (string-length str))
> +                     (eq? #\- (string-ref str i))
> +                     (not (eq? #\0 (string-ref str (1+ i)))))
> +                (base-name-string (substring str 0 i))
> +                str)))
> +        (lambda (sym)
> +          (string->symbol (base-name-string (symbol->string sym))))))

Pretty nasty, but we should continue this conversation in the other
thread.  Maybe it would be made more clear by procedures like `gensym?'
and `gensym-base' or something.  Dunno.

OK, everything else looks great to me.  Thanks for the opportunity to
review!

Andy
-- 
http://wingolog.org/