Code-walking with Flatten is Lossy

[thephoeron]

While the intended use of flatten is to eliminate unnecessary complexity, @csrhodes has pointed out that it can be unintentionally lossy, when a proper code-walker would be preferred.

@hoytech — it would be worthwhile to have your perspective here, since it's your code and book; is it better to treat this code as a utility library which should be updated to reflect best-practices, or keep this as a reference implementation of the code examples in the book?

[hoytech]

Completely agree with @csrhodes: flatten can only be used for very particular types of code-walking. It's really only appropriate for determining symbols used in a block of code, and even then can only be used in macros which can handle false-positives in their input.

In the case of flatten, please don't modify this function itself since it has a commonly-understood meaning. It is actually one of a few utilities in LOL that are straight copies of utilities published by Paul Graham (the others are aif and alambda).

That said, if you come up with other cool code-walking utils, by all means merge them into the let-over-lambda repo, although I suspect it may make more sense to create repos for them separately.

BTW, thank you very much for maintaining this repo of the LOL code and its corresponding quicklisp project.

[thephoeron]

@hoytech : Thanks for your input! Mostly it's been no trouble maintaining your code---until the change to the backquote implementation in SBCL 1.2.2. A lot of people were asking in the Lisp community for Quicklisp distributions of the LOL code and ISAAC, and I happened to be sitting on versions that I had made for myself. So I figured I should make them available.

Unfortunately I had to make one change to flatten to support the new backquote implementation. See issue https://github.com/thephoeron/let-over-lambda/issues/1 for that discussion.

It relies on a "safety feature" (this version incorporates improvements suggested by @csrhodes, per issue #2):

#+sbcl
(eval-when (:compile-toplevel :execute)
  (handler-case
      (progn
        (sb-ext:assert-version->= 1 2 2)
        (setq *features* (remove 'old-sbcl *features*)))
    (error ()
      (pushnew 'old-sbcl *features*))))

flatten is currently defined as follows:

(defun flatten (x)
  (labels ((rec (x acc)
                (cond ((null x) acc)
                      #+(and sbcl (not lol::old-sbcl))
                      ((typep x 'sb-impl::comma) (rec (sb-impl::comma-expr x) acc))
                      ((atom x) (cons x acc))
                      (t (rec
                           (car x)
                           (rec (cdr x) acc))))))
    (rec x nil)))

It is an ugly hack, but it gets at the right code in the new implementation of backquote.

Do you think there's a better solution?

[priyadarshan]

Just for future users interested in this topic, the code-walker comments talked about can be found here: http://christophe.rhodes.io/notes/blog/posts/2014/naive_vs_proper_code-walking/

Thank you so much for let-over-lambda.

[hoytech]

Ahh gotcha. Sorry I meant please don't change the interface to flatten. Portability fixes like the one in question are of course fine.

Not knowing too much of the specifics of the SBCL comma issue, I don't off-hand know of a better way. I'm sure the solution you and @csrhodes implemented is fine.

Cheers,

Doug

[thephoeron]

Cool. My intention is to keep the library as true to your book as possible, so no, I won't be changing any interfaces. Thanks!