magnars/dash.el

-flatten-n fails on lists containing cons cells

Closed this issue · 2 comments

Recently, I have got some issues with -flatten-n function from this package, most importantly in alphapapa/org-ql#179. The function worked correctly before, but it seems to have got a bug in a recent commit.

For example, if you evaluate the following expression:

(-flatten-n 1 '(((1 . 2)) ((3 . 4))))

It should return '((1 . 2) (3 . 4)), but it throws the following error instead:

Wrong type argument: listp, 2

I experience this error in several places containing (-flatten-n 1), and I was able to resolve them by replacing it with (apply #'append).

-flatten-n is defined as follows in dash.el:

(defun -flatten-n (num list)
  "Flatten NUM levels of a nested LIST.

See also: `-flatten'"
  (declare (pure t) (side-effect-free t))
  (-last-item (--iterate (--mapcat (-list it) it) list (1+ num))))

It depends on several other functions in this package, and I noticed that -list and --iterate were changed in the following commits:

  • 1d58249 Write -iterate in terms of --iterate 2 months ago
  • 1152feb Deprecate variadic -list 2 months ago

Could you investigate these suspicious commits for fixing the issue with -flatten-n? (apply #'append) works as a replacement for (-flatten-n 1), but I don't know what will be the performant implementation for those functions.

Thanks for catching this!

The bug was in this part of my rewrite of --iterate as a macro:

(dotimes (_ ,n)
  (push it ,res)
  (setq it ,form))

Here, form was evaluated one too many times at the end of the loop.

While fixing this I also found and fixed the following bug that the macro has suffered from since day 1:

(let (l) (--iterate (push 1 l) (push 0 l) 0) l)

This returns (0), when it should actually return ()!

(apply #'append) works as a replacement for (-flatten-n 1), but I don't know what will be the performant implementation for those functions.

(apply #'append) will always be faster than (-flatten-n 1), even after my simplification of -flatten-n in commit 0b9cdba.

The fix is now included in versions 2.18.1 and 2.18.1.0.20210228 on GNU ELPA and GNU-devel ELPA, respectively.