Buffer freezes on (def ^{:macro})

[borkdude]

Expected behavior

When I have (def ^{:macro}) in my buffer (as the only code), I should see no freezing of emacs.

Actual behavior

Emacs with clojure-mode freezes.

Steps to reproduce the problem

Put (def ^{:macro}) as the sole code in a clojure buffer. Try to move the cursor a bit back and forth.

When Emacs freezes, send:

pkill -USR2 Emacs

and then see something like:

Debugger entered--entering a function:
* #f(compiled-function () #<bytecode 0x1fe4d41470e9>)()
  imenu--generic-function(((nil clojure-match-next-def 0)))
  imenu-default-create-index-function()
  imenu--make-index-alist(t)
  which-function()
  which-func-update-1(#<window 3 on foo.clj>)
  which-func-update()
  apply(which-func-update nil)
  timer-event-handler([t 0 0 500000 t which-func-update nil idle 0])
  recursive-edit()
  debug(lambda)
* thing-at-point--end-of-sexp()
  bounds-of-thing-at-point(sexp)
  clojure-match-next-def()
  imenu--generic-function(((nil clojure-match-next-def 0)))
  imenu-default-create-index-function()
  imenu--make-index-alist(t)
  which-function()
  which-func-update-1(#<window 3 on foo.clj>)
  which-func-update()
  apply(which-func-update nil)
  timer-event-handler([t 0 0 500000 t which-func-update nil idle 0])

@vemv hinted that this looked suspicious:

clojure-mode/clojure-mode.el

Line 749 in c339353

(while (not found?)

Environment & Version information

clojure-mode version

clojure-mode (version 5.13.10)

Emacs version

27.2

Operating system

macOS

[vemv]

I can repro this by invoking (clojure-match-next-def) in clojure-mode latest.

I could also fix it by changing here:

clojure-mode/clojure-mode.el

Lines 757 to 758 in c339353

	(if (char-equal ?^ (char-after def-beg))
	(progn (forward-sexp) (backward-sexp))

-(progn (forward-sexp) (backward-sexp))
+(forward-sexp)

I can't make sense of (progn (forward-sexp) (backward-sexp)), it seems a no-op that naturally would cause an infinite loop in conjunction with the (while (not found?)?

With my changed version, it completes and the function still does what I believe it does.

I have no idea of what imenu--generic-function is though, so perhaps someone else could take it from here?

[vemv]

...according to Git blame, all this code is remarkably old and therefore presumably stable.

Maybe this Imenu thing is not used enough to surface such bugs, does that sound likely?

[magnars]

Maybe this Imenu thing is not used enough to surface such bugs, does that sound likely?

I use imenu all the time, but then again I rarely type (def ^{:macro}) in a buffer, to be fair.

[vemv]

@magnars or @borkdude - PR welcome with the described change or similar if you find that it fixes the thing / doesn't visibly break sth else (more freezes / bad Imenu integration)

Since I don't use Imenu I wouldn't feel at ease proposing a fix myself.

Cheers - V

[OknoLombarda]

I've looked into this problem and managed to figure out the cause of infinite loop, I think. This (progn (forward-sexp) (backward-sexp)) code is used to skip a var metadata and move point to var name. For example, if we have this definition

(def ^String foo "bar")

before moving, the point will be just after the word String and consequent calls to (forward-sexp) and (backward-sexp) will move point just before the word foo, but if you're just typing the definition and there's no var name yet ((def ^String)) then this will bring the point back to ^, causing infinite loop. Solution proposed by @vemv will not work in cases where metadata symbol is the last sexp before closing parenthesis, because it will infinitely try to go to next sexp, with no success. #595 is example of such case

I'm new to Clojure and don't know a single thing about Emacs Lisp, but here's a solution I came up with

clojure-mode/clojure-mode.el

Line 757 in c339353

(if (char-equal ?^ (char-after def-beg))

-            (if (char-equal ?^ (char-after def-beg))
-                (progn (forward-sexp) (backward-sexp))
+            (when (char-equal ?^ (char-after def-beg))
+              (progn (forward-sexp) (backward-sexp)))
+            (when (or (not (char-equal ?^ (char-after (point))))
+                      (and (char-equal ?^ (char-after (point))) (= def-beg (point))))
               (setq found? t)
               (when (string= deftype "defmethod")
                 (setq def-end (progn (goto-char def-end)
                                      (forward-sexp)
                                      (point))))
              (set-match-data (list def-beg def-end)))

So, when metadata symbol is encountered, function will try to move to the beginning of the next sexp, and if the point is still where it was before, then it should exit the loop

[vemv]

Released as clojure-mode 5.15.0, which will be available within a couple hours

[borkdude]

Thanks!