Why does SBCL (or Common lisp in general?) dislike my loop?

Question

Recursive find-dir-upwards works as expected, but find-dir-upwards-loop refuses to compile, complaining about type mismatch (even with added type annotations) under SBCL (Win64, Portacle 1.4, SBCL 2.0.0).

What did I do wrong ?

(defun find-dir-upwards-loop (dir marker-file)
  (loop for prev = nil then curr
        for curr = dir then (uiop:pathname-parent-directory-pathname curr)
        until (equal curr prev)
        with f = (merge-pathnames marker-file curr)
        when (uiop:file-exists-p f) return curr))

(defun find-dir-upwards (dir marker-file)
  (let ((f (merge-pathnames marker-file dir)))
    (if (uiop:file-exists-p f)
        dir
        (let ((parent (uiop:pathname-parent-directory-pathname dir)))
          (unless (equal dir parent)
            (find-dir-upwards parent marker-file))))))

Please post the error message. – Barmar Oct 27 '21 at 15:57 — Barmar, Oct 27 '21 at 15:57
try macroexpanding the loop to see how it is translated – coredump Oct 27 '21 at 22:22 — coredump, Oct 27 '21 at 22:22

score 3 · Answer 1 · answered Oct 28 '21 at 22:03

The WITH after FOR part of the answer:

(loop for x = 0 then (+ x 1)
      with y = (+  x 1)
      when (> x 10) return (list x y))

This is a sequence of events:

the X variable will be established
the Y variable will be established and initialized to (+ x 1)
the X variable will be set to 0
...

As you can see, in operation 1) X will be a variable, but it won't be initialized to 0. This would happen in operation 3). Thus in operation 2) X will not have been initialized to 0 -> its value might be NIL. Adding 1 to NIL is an error.

Thanks, super helpful. It's interesting that '.. for x from 0 to 10' does both create and initialize x before the WITH clause. (Or at least, in my stack, similar to OP). — Halbert, Nov 01 '21 at 19:16

score 2 · Accepted Answer · answered Oct 28 '21 at 10:52

2

You can't (portably) put a with after an until. Look at the grammar in the Hyperspec:

loop [name-clause] {variable-clause}* {main-clause}* => result*

With is a variable-clause, but until is a termination-test, which is a main-clause. All main-clauses must come after all variable-clauses.

answered Oct 28 '21 at 10:52

Svante

50,694
11
78
122

Thanks, inlining the merge-pathnames call / getting rid of the with still looks better than using let, and it works. – MrDispatch Oct 29 '21 at 13:39

Gwang-Jin Kim · Answer 3 · 2021-10-27T22:49:01.220

Simplified code

After some while thinking, I realized you can simplify your original function to:

(defun find-dir-upwards (dir marker-file)
  (let ((parent (uiop:pathname-parent-directory-pathname dir)))
    (unless (equal dir parent)
      (if (probe-file (merge-pathnames marker-file dir))
          dir
          (find-dir-upwards parent marker-file)))))

And its correspoding loop function is:

(defun find-dir-upwards (dir marker-file)
  (loop for parent = (uiop:pathname-parent-directory-pathname dir)
        unless (equal dir parent)
        do (if (probe-file (merge-pathnames marker-file dir))
               (return dir)
               (setf dir parent))))

As @Rezo pointed out in this post, probe-file is portable between different Common Lisp implementations, while uiop:file-exists-p is not.

Original answer

I would define the loop differently.

Use the (let ((dir dir)) (loop ... (setf dir (<some updater function>)))) idiom to have some reference variable(s) which are updated every run through the loop - until the condition is met.

(defun find-dir-upwards (dir marker-file)
  (let ((f (merge-pathnames marker-file dir))
        (dir dir))
    (loop for parent = (uiop:pathname-parent-directory-pathname dir)
          unless (equal dir parent)
          do (if (uiop:file-exists-p f) ;; if marker-file in dir
                 (return dir)           ;; then arrived at destination
                 (progn
                   (setf dir parent)    ;; otherwise climb up one dir-level
                   (setf f (merge-pathnames marker-file dir)))))))

Which is then further dissectable to:

(defun marker-file-in-dir-p (dir marker-file)
  (uiop:file-exists-p (merge-pathnames marker-file dir)))

(defun find-dir-upwards (dir marker-file)
  (let ((dir dir))
    (loop for parent = (uiop:pathname-parent-directory-pathname dir) ;; parent from dir
          unless (equal dir parent)
          do (if (marker-file-in-dir-p dir marker-file)
                 (return dir)
                 (setf dir parent)))))

Thanks for the probe-file pointer. [After Svante pointed out why it was bugging out,] Inlining merge-pathnames call instead of introducing the f binding (would be uglier using let) made it work while still looking pretty good. — MrDispatch, Oct 29 '21 at 13:59

score 0 · Answer 4 · answered Oct 28 '21 at 02:47

Something about combining the step form of 'FOR' and a 'WITH' creates this problem. IT seems like it tried to initialize the 'with' before the 'for' creates x, but, not always. I haven't solved it, but I have had this issue before. For example:

(loop for x = 0 then (+ x 1)
      with y = (+  x 1)
      when (> x 10) return (list x y))

Yields:

Value of X in (+ X 1) is NIL, not a NUMBER.
   [Condition of type SIMPLE-TYPE-ERROR]

Restarts:
 0: [RETRY] Retry SLIME REPL evaluation request.
 1: [*ABORT] Return to SLIME's top level.
 2: [ABORT] abort thread (#<THREAD "new-repl-thread" RUNNING {10044CC083}>)

Backtrace:
  0: (SB-C::%COMPILE-TIME-TYPE-ERROR (NIL) NUMBER #<unused argument> (X) "(+ X 1)" NIL)
  1: ((LAMBDA ()))
  2: (SB-INT:SIMPLE-EVAL-IN-LEXENV (LOOP FOR X = 0 THEN ...) #<NULL-LEXENV>)
  3: (EVAL (LOOP FOR X = 0 THEN ...))
 --more--

But removing either the step form, or the with, seems to work fine:

CL-USER> (loop for x from 0 to 10
              with y = (+  x 1)
              collect (list x y))
((0 1) (1 1) (2 1) (3 1) (4 1) (5 1) (6 1) (7 1) (8 1) (9 1) (10 1))
CL-USER> (loop for x = 0 then (+ x 1)
              when (> x 10) return x)
11

Why does SBCL (or Common lisp in general?) dislike my loop?

4 Answers4

Simplified code

Original answer