zainab-ali / pair-tree.el

An Emacs Lisp cons cell explorer
GNU General Public License v3.0
51 stars 3 forks source link

Error message wrong-type-argument listp #1

Closed ram535 closed 3 years ago

ram535 commented 3 years ago

When M-x pair-tree '((2 . (turtle doves)) (3 french hens) 4 (calling . birds) partridge).

I get this message.

pair-tree: Wrong type argument: listp, "ooooooooooooo
ooooooooooooo
ooooooooooooo"

This is the message I get M-x toggle-debug-on-error.

Debugger entered--Lisp error: (wrong-type-argument listp "ooooooooooooo\nooooooooooooo\nooooooooooooo")
  pair-tree--write-tree(#s(pair-tree--el :pos #s(pair-tree--pos :col 0 :line 0) :ext #s(pair-tree--pos :col 12 :line 2) :val nil :node (#s(pair-tree--el :pos #s(pair-tree--pos :col 0 :line 1) :ext #s(pair-tree--pos :col 3 :line 2) :val nil :node (#s(pair-tree--el :pos #s(pair-tree--pos :col 0 :line 2) :ext #s(pair-tree--pos :col 0 :line 2) :val 2 :node nil :hanging t) . #s(pair-tree--el :pos #s(pair-tree--pos :col 1 :line 1) :ext #s(pair-tree--pos :col 3 :line 2) :val nil :node (#s(pair-tree--el :pos ... :ext ... :val turtle :node nil :hanging t) . #s(pair-tree--el :pos ... :ext ... :val nil :node ... :hanging nil)) :hanging nil)) :hanging t) . #s(pair-tree--el :pos #s(pair-tree--pos :col 4 :line 0) :ext #s(pair-tree--pos :col 12 :line 2) :val nil :node (#s(pair-tree--el :pos #s(pair-tree--pos :col 4 :line 1) :ext #s(pair-tree--pos :col 7 :line 2) :val nil :node (#s(pair-tree--el :pos ... :ext ... :val 3 :node nil :hanging t) . #s(pair-tree--el :pos ... :ext ... :val nil :node ... :hanging nil)) :hanging t) . #s(pair-tree--el :pos #s(pair-tree--pos :col 8 :line 0) :ext #s(pair-tree--pos :col 12 :line 2) :val nil :node (#s(pair-tree--el :pos ... :ext ... :val 4 :node nil :hanging t) . #s(pair-tree--el :pos ... :ext ... :val nil :node ... :hanging nil)) :hanging nil)) :hanging nil)) :hanging nil))
  pair-tree(((2 turtle doves) (3 french hens) 4 (calling . birds) partridge))
  funcall-interactively(pair-tree ((2 turtle doves) (3 french hens) 4 (calling . birds) partridge))
  call-interactively(pair-tree record nil)
  command-execute(pair-tree record)
  execute-extended-command(nil "pair-tree" nil)
  funcall-interactively(execute-extended-command nil "pair-tree" nil)
  call-interactively(execute-extended-command nil nil)
  command-execute(execute-extended-command)

This is my configuration.

(use-package pair-tree
:straight (pair-tree :type git :host github :repo "zainab-ali/pair-tree.el"))

GNU Emacs 28.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.22.30, cairo version 1.15.10) of 2020-03-09

(image-type-available-p 'svg) ;;t
zainab-ali commented 3 years ago

Thanks for reporting this.

It seems to be related to treating strings as arrays of characters, although I'd be surprised if string handling was any different.

I haven't been able to reproduce this on my own Emacs snapshot, built today:

 GNU Emacs 28.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.21, cairo version 1.16.0)

If possible, would you mind running the following code in your *scratch* buffer?

(mapcar (lambda (it) (char-to-string it)) 
"ooooooooooooo\nooooooooooooo\nooooooooooooo")

Let me know if that triggers the same error (although I'd be very surprised if it did).

ram535 commented 3 years ago

It throws this:

("o" "o" "o" "o" "o" "o" "o" "o" "o" "o" "o" "o" ...)

zainab-ali commented 3 years ago

Thanks for checking. I suspect it's a bug in the use of dash's anaphoric --map.

What version of dash are you using?

I can reproduce it using any version between the following two commits:

92393c7 * | Clean up core definitions (Jan 6)
7583e65 * Revert --map to using mapcar (Jan 11)

This implementation has an offending call to pop:

(defmacro --map (form list)
  "Eval FORM for each item in LIST and return the list of results.
Each element of LIST in turn is bound to `it' before evaluating
BODY.
This is the anaphoric counterpart to `-map'."
  (declare (debug (form form)))
  (let ((l (make-symbol "list"))
        (r (make-symbol "res")))
    `(let ((,l ,list) ,r it)
       (ignore it)
       (while ,l
         (setq it (pop ,l))
         (push ,form ,r))
       (nreverse ,r))))

It's possibly inlined, and hence doesn't appear in the stack trace.

ram535 commented 3 years ago

It seems I am using the last dash version 2.17.0

I am not a elisp ninja. I do not know what you are refering with:

I suspect it's a bug in the use of dash's anaphoric --map

This implementation has an offending call to pop:

(defmacro --map (form list) "Eval FORM for each item in LIST and return the list of results. Each element of LIST in turn is bound to it' before evaluating BODY. This is the anaphoric counterpart to-map'." (declare (debug (form form))) (let ((l (make-symbol "list")) (r (make-symbol "res"))) `(let ((,l ,list) ,r it) (ignore it) (while ,l (setq it (pop ,l)) (push ,form ,r)) (nreverse ,r)))) It's possibly inlined, and hence doesn't appear in the stack trace.

zainab-ali commented 3 years ago

Apologies for the jargon vomit! I was so engrossed in the problem that I didn't explain myself properly.

Since both our Emacs are the same, and mapcar doesn't throw an error, I think we have different dash versions.

Unfortunately all versions of dash since January 2020 will have the version number "2.17.0". The Emacs package versioning system is a bit bizarre.

One way to check what dash you're using is to look at the date in the dash.el file path.

On my Emacs, I can do this with:

If you're using a version between 6th Jan 2021 and 11th Jan 2021, then you may see different code. In this case, you'll need to upgrade dash to the latest version. You can do this using the package menu with M-x list-packages, marking dash with U and executing with x.

ram535 commented 3 years ago

Thanks for the explanation. I use straight to download packages and not the build-in emacs method. straight-pull-all command updates all the packages that are installed. That is how I always update all my installed package. straight creates the straight directory inside .emacsd directory.

In the straight directory are three other directories:

In the repos directory is where straight download the packages from melpa, github or wherever the source. In the build directory is where straight compiles the packages (I think). The modified directory, I guess where straight puts packages that you modify (not sure).

This is how I installed your package using straight.

(use-package pair-tree
:straight (pair-tree :type git :host github :repo "zainab-ali/pair-tree.el"))

I went to check --map in ~/.emacs.d/straight/repos/dash.el/dash.el file.

(defmacro --map (form list)
  "Eval FORM for each item in LIST and return the list of results.
Each element of LIST in turn is bound to `it' before evaluating
FORM.

This is the anaphoric counterpart to `-map'."
  (declare (debug (def-form form)))
  `(mapcar (lambda (it) (ignore it) ,form) ,list))

It is the same as your last message.

zainab-ali commented 3 years ago

Straight looks nice! I use nix myself, so also don't use the typical package update workflow.

I think we can assume that your dash version is up to date, or at the very least isn't a direct suspect.

It's difficult to see what's wrong from the trace as the elisp has been optimized. Emacs re-wrote the pair-tree--write--tree function to make it more performant, so I can't easily compare the trace with the code.

Could you un-optimize it by evaluating the original definition?

  1. Go to the definition with M-x find-function pair-tree--write-tree
  2. With your point anywhere within the function body, call M-x eval-defun. This loads the un-optimized code into Emacs.
  3. Call M-x pair-tree '((2 . (turtle doves)) (3 french hens) 4 (calling . birds) partridge) once again.

Hopefully, you will see a more detailed trace that looks a bit like this:

Debugger entered--Lisp error: (wrong-type-argument listp "ooooooooooooo\nooooooooooooo\nooooooooooooo")
  cdr("ooooooooooooo\nooooooooooooo\nooooooooooooo")
  (setq list (cdr list))
  (prog1 list (setq list (cdr list)))
  (car-safe (prog1 list (setq list (cdr list))))
  (setq it (car-safe (prog1 list (setq list (cdr list)))))
  (while list (setq it (car-safe (prog1 list (setq list (cdr list))))) (setq res (cons (if (not (equal it 10)) (pair-tree--display-image it) (char-to-string it)) res)))
  (let ((list grid) res it) (ignore it) (while list (setq it (car-safe (prog1 list (setq list (cdr list))))) (setq res (cons (if (not (equal it 10)) (pair-tree--display-image it) (char-to-string it)) res))) (nreverse res))
  (string-join (let ((list grid) res it) (ignore it) (while list (setq it (car-safe (prog1 list (setq list (cdr list))))) (setq res (cons (if (not (equal it 10)) (pair-tree--display-image it) (char-to-string it)) res))) (nreverse res)))
  (let* ((grid (pair-tree--grid (progn (or (progn (and ... t)) (signal 'wrong-type-argument (list ... tree))) (aref tree 2)))) (display-grid (string-join (let ((list grid) res it) (ignore it) (while list (setq it (car-safe ...)) (setq res (cons ... res))) (nreverse res))))) (let (go inhibit-read-only) (setq go #'(lambda (el) (pair-tree--write-el el) (if (progn (or ... ...) (aref el 4)) (progn (funcall go ...) (funcall go ...))))) (setq inhibit-read-only t) (set (make-local-variable 'pair-tree--tree) tree) (erase-buffer) (goto-char (point-min)) (insert display-grid) (funcall go tree)))
(pair-tree--write-tree(#s(pair-tree--el :pos #s(pair-tree--pos :col 0 :line 0) :ext #s(pair-tree--pos :col 12 :line 2) :val nil :node (#s(pair-tree--el :pos #s(pair-tree--pos :col 0 :line 1) :ext #s(pair-tree--pos :col 3 :line 2) :val nil :node (#s(pair-tree--el :pos #s(pair-tree--pos :col 0 :line 2) :ext #s(pair-tree--pos :col 0 :line 2) :val 2 :node nil :hanging t) . #s(pair-tree--el :pos #s(pair-tree--pos :col 1 :line 1) :ext #s(pair-tree--pos :col 3 :line 2) :val nil :node (#s(pair-tree--el :pos ... :ext ... :val turtle :node nil :hanging t) . #s(pair-tree--el :pos ... :ext ... :val nil :node ... :hanging nil)) :hanging nil)) :hanging t) . #s(pair-tree--el :pos #s(pair-tree--pos :col 4 :line 0) :ext #s(pair-tree--pos :col 12 :line 2) :val nil :node (#s(pair-tree--el :pos #s(pair-tree--pos :col 4 :line 1) :ext #s(pair-tree--pos :col 7 :line 2) :val nil :node (#s(pair-tree--el :pos ... :ext ... :val 3 :node nil :hanging t) . #s(pair-tree--el :pos ... :ext ... :val nil :node ... :hanging nil)) :hanging t) . #s(pair-tree--el :pos #s(pair-tree--pos :col 8 :line 0) :ext #s(pair-tree--pos :col 12 :line 2) :val nil :node (#s(pair-tree--el :pos ... :ext ... :val 4 :node nil :hanging t) . #s(pair-tree--el :pos ... :ext ... :val nil :node ... :hanging nil)) :hanging nil)) :hanging nil)) :hanging nil)))
  pair-tree(((2 turtle doves) (3 french hens) 4 (calling . birds) partridge))

I'll be able to correlate that with the code.

Thank you very much for bearing with me on this.

ram535 commented 3 years ago

I am glad, I am helping. Plus I am learning new stuff. I have seen some videos about nix in youtube. It looks very interesting. I wonder how you manage your packages in nix.

I followed your instructions and no errors were thrown. But the contrary it works!.

image

So I am guessing the error is cause by how straight load pair-tree?

This the structure of how straight set up pair-tree.

~/.emacs.d/straight/build/pair-tree/

  lrwxrwxrwx 1 last last  60 Jan 18 02:52 pair-tree.el -> ~/.emacs.d/straight/repos/pair-tree.el/pair-tree.el
  -rw-rw-r-- 1 last last 42K Jan 18 02:52 pair-tree.elc
  -rw-rw-r-- 1 last last 569 Jan 18 02:52 pair-tree-autoloads.el

~/.emacs.d/straight/repos/pair-tree.el/

  drwxrwxr-x 2 last last 4.0K Jan 18 02:52 test
  -rw-rw-r-- 1 last last  35K Jan 18 02:52 LICENSE
  -rw-rw-r-- 1 last last 1.2K Jan 18 02:52 README.md
  -rw-rw-r-- 1 last last 121K Jan 18 02:52 example.png
  -rw-rw-r-- 1 last last  24K Jan 18 02:52 pair-tree.el

If I understand correctly, straight loads pair-tree.elc when starting emacs. But for some reason it causes and error because it is optimize?

If you think, it is straight the cause of the error, I could write to the developer that made straight. He always responses to messages in gitter straight channel.

zainab-ali commented 3 years ago

I'm glad it's fixed!

I've tried to reproduce it by spinning up a new user with a straight-controlled Emacs (it turns out this is pretty quick to do with nix), but oddly enough everything ran smoothly.

This is the config I used, the first snippet being taken from straight.el:

;; -*- lexical-binding: t; -*-

(defvar bootstrap-version)
(let ((bootstrap-file
       (expand-file-name "straight/repos/straight.el/bootstrap.el" user-emacs-directory))
      (bootstrap-version 5))
  (unless (file-exists-p bootstrap-file)
    (with-current-buffer
        (url-retrieve-synchronously
         "https://raw.githubusercontent.com/raxod502/straight.el/develop/install.el"
         'silent 'inhibit-cookies)
      (goto-char (point-max))
      (eval-print-last-sexp)))
  (load bootstrap-file nil 'nomessage))

(straight-use-package 'use-package)
(straight-use-package 'dash)
(straight-use-package
 '(pair-tree :type git :host github :repo "zainab-ali/pair-tree.el"))

I'm not sure why dash was needed too - that might be a bug in me configuring the package.

So maybe it's not just straight, but straight on your system?

To be absolutely sure, could you add the :no-byte-compile flag to the use-package statement and check if that resolves the issue? According to the straight.el docs you can disable byte compilation using that option:

(use-package pair-tree
  :straight (pair-tree
             :type git :host github
             :repo "zainab-ali/pair-tree.el"
             :no-byte-compile t))

If that works, byte compiling must be the culprit and you can ask the straight.el maintainer (be sure to mention me too — it's a pretty interesting issue).

As an aside, I'll take a look at sorting out the SVGs - that white background isn't what I was aiming for.

ram535 commented 3 years ago

It does work with that configuration.

(use-package pair-tree
  :straight (pair-tree
             :type git :host github
             :repo "zainab-ali/pair-tree.el"
             :no-byte-compile t))

I will make an issue in the straight repo about this unexpected behavior.

Thanks.

progfolio commented 3 years ago

First off, neat package!

I'm not sure why dash was needed too - that might be a bug in me configuring the package.

straight.el parses the package metadata for dependencies. The regexp we were using was a little strict and your package uses an unconventional amount of semicolons for that section (#4 should take care of that). I've relaxed the regexp to be more tolerant, but I still think it's a good idea to stick with convention.