Cider: Can't debug forms that contain macros that use &env

[weissjeffm]

I know there aren't a lot of macros that use it, but core.async does. It's a pretty widely used library and it's one that is particularly hard to debug without cider's debug facility.

Expected behavior

(defn foo [a]
 #dbg
 (clojure.core.async/go-loop [] a)))

Evaling that form should work.

Actual behavior

ExceptionInfo Could not resolve var: a clojure.core/ex-info (core.clj:4593)

The error occurs in clojure.walk/macroexpand-all. I brought this up on IRC, and the issue is with macroexpand-all, which uses an empty env.

Apparently https://github.com/ztellman/riddley has a macroexpander that does this properly.

Steps to reproduce the problem

Open a cider repl, open a source file, paste the form above, inside it hit C-M-x.

Environment & Version information

CIDER version information

;; CIDER 0.13.0snapshot (package: 20160513.959) (Seattle), nREPL 0.2.12
;; Clojure 1.7.0, Java 1.8.0_92

Lein/Boot version

Emacs version

24.5.1

Operating system

OS X 10.11 "El Capitan"

[weissjeffm]

I think the actual issue is in cider-nrepl, should I move this bug report over there?

[Malabarba]

That's not necessary. You can leave it here. Sorry I haven't had time to look into it yet.

[Malabarba]

I've confirmed this happens for me too. I'd rather not add an extra dependency just to workaround a bug in clojure.core. So I think we'll have to borrow code from riddley (as little as possible), although I haven't actually verified that riddley's macroexpander indeed fixes this.

[ckoparkar]

I was trying out riddley in cider-nrepl, but it din't solve the issue. Not sure if it was the right approach;
What I did;

1. I think that instrument-tagged-code - instrument.clj:288 is the fn which calls macroexpand-all causing the &env problem.
2. macroexpand-all - meta.clj:36 then calls macroexpand from clojure.core.
3. I required the whole riddley library for now, and tried riddley.walk/macroexpand, but the &env problem is still there.

Is this correct ? Or do we need to substitute macroexpand-all somewhere else ?

[Malabarba]

I don't know. Maybe you can try replacing our own macroexpand-all in instrument.clj:288 with riddley's version. The debugger is probably not going to work when you do that, but if that gets rid of the &env-related exception at least it's an indication of a possible solution. :-P

[ckoparkar]

@Malabarba Yep, it works. What's great is that the debugger also partially works. locals and eval seem to work. It does mean that riddley is doing something right. Should we look into what parts of code we require here ? Although it is a pretty small library, so I am assuming most of it would be needed for this.

[weissjeffm]

I'm really interested in being able to debug core.async, so anything I can do to help, let me know.

I took a look at what riddley's macroexpand-all depends on, and it's basically the entire library. It's 335 lines of clojure plus a few lines of java.

[Malabarba]

It's small, but these things have an effect. :-)
Because of how nrepl works, all libraries that CIDER requires are loaded at startup (and in cases like lein, more than once) so they end up stacking to the sad startup times we have now.

Does the library also preserve metadata when expanding?
If so, then maybe we can use it. Otherwise, we have no choice but to figure out how to immitate it.

[ckoparkar]

I thought so. After reading all the comments on #1717. I'm not sure if it preserves the metadata. I'll take a look.

[weissjeffm]

riddley.walk> (meta (macroexpand-all (with-meta '(+ 1 1) {:foo :bar})))
nil

Looks like it does not, assuming I'm testing it right.

[weissjeffm]

I ran the same test on jvm.tools.analyzer's macroexpand (which supposedly also preserves &env). It also returns nil.

[kuberlog]

Do we have a strategy on fixing this yet?

[kuberlog]

Where is nrepl middleware typically installed if I wanted to play around with a fix?

[ckoparkar]

@SlightlyCyborg What I typically do is;
1) Clone the cider-nrepl project and follow the steps given in the Contributing section to set it up.
2) Then, cider-jack-in from the cider-nrepl project and implement the changes.

The jar is usually installed in ~/.m2/repository/cider/cider-nrepl/ directory.

[kuberlog]

I still don't fully understand how &env works and why macroexpand-all doesn't work with macros that use &env. Regardless, I made a bug report to the Clojure JIRA concerning clojure.walk/macroexpand-all as I would expect a function named macroexpand-all should be able to handle all valid macros.

Perhaps this fix will be as easy as waiting for a clojure.core patch

[kuberlog]

I am confused as to why you say that riddley does not preserve meta-data. Through my experimentation I run the following

(meta (r/macroexpand-all '(def x "docstring" (+ 1 2))))
>> {:line 33, :column 45}

(meta (second (r/macroexpand-all '(def ^:private x "docstring" (+ 1 2)))))
>> {:private true}

To me that looks like the meta data is preserved

However, when I run it on a simple expression like (+ 1 1) I do not get the meta data:

(meta (macroexpand-all (with-meta '(+ 1 1) {:foo :bar})))
>> nil

When I run with-meta on a def form it does work however

(meta (macroexpand-all (with-meta '(def foo 1) {:foo :bar})))
>> {:foo :bar}

Also when I run macroexpand-all on a def form it preserves the line numbers in meta

(meta (macroexpand-all '(def foo 1)))
>> {:line 352, :column 39}

[kuberlog]

Ok. I fixed the riddley code so that (macroexpand-all '(+ 1 1)) doesn't clobber the meta data. I am going to submit a pull request to the riddley repo and then commence writing the code to fix cider's debugger.
https://github.com/ztellman/riddley/pull/23

Question: should I add the riddley code directly into the nrepl or add it as a dependency?

[kuberlog]

I added the locally install riddley patch into cider-nrepl and used its macroexpand-all instead of meta/macroexpand all in intrument.clj and it works. I changed project name to "0.14.0-HACKED" and then locally installed that and required it in profiles. Everything seems to be working. I don't know the best way to submit a pull request on this, but I guess I should wait for the riddley code to make it to clojars, which the author says will be done in a couple of days anyway. What is the best way to submit a pull request to cider-nrepl? Should I make a version number change or just push make a pull request where I just change the deps and the code?

[ckoparkar]

Ok. I fixed the riddley code so that (macroexpand-all '(+ 1 1)) doesn't clobber the meta data.

Awesome! So are you able to debug core.async functions using the patched version of riddley ?

What is the best way to submit a pull request to cider-nrepl? Should I make a version number change or just push make a pull request where I just change the deps and the code?

You can submit a PR with just the code changes. We can bump the version when we cut a new release, which is usually every 3 months :-) 0.13.0 was just released about a month ago.

[kuberlog]

I can debug async.core with the new riddley code, however if you mark an entire function for debugging with a go routine in it, what happens when you run it is that the debugger will go inside the go routine (into the new thread, it is marked after all), but when that go routine is finished, debugger control is never given back to the original thread which just blocks forever.

Here is a basic example:

(defn fooit [foo] (go [] foo)  (+ 2 3))

(fooit 1)

Upon running fooit, after marking fooit for debugging, the debugger will highlight both the (+ 2 3) form and the foo inside the (go [] foo) form as if both are being run by the debugger simultaneously. After pressing next the debugger quits and the nrepl just blocks forever or at least until I C-c C-c in the repl to interupt

This works though:

(defn fooit [foo] (go [] #break (+ 1 foo))  (+ 2 3))

I don't think the debugger is meant to handle multiple threads running code within the same instrumented form.

[Malabarba]

Thanks for looking into this @SlightlyCyborg

Indeed, the debugger will get stuck if you debug multiple threads simultaneously. That's not really an issue on the Clojure side, it's an issue on the Emacs side. When debugging two threads, cider.el receives two messages from the debugger asking for input, but it's not designed to support that so the second message causes cider.el to forget about the first. That's why the first code snippet gets stuck (because cider.el forgot to prompt you for a key).

[kuberlog]

Ya. No problem.

I see. Thanks for the explanation. It sounds like it would be quite a bit
of work to support simultaneous threads in the debugger. I am imagining a system that spawns a debug buffer for each thread

(map #(go (+ 1 1)) (range 2000))

😱

Although, those are pseudo threads. So, no problemo!

😎

https://github.com/clojure/core.async/blob/43139e44b04dd09bb5faa555ea8ebd2ca09e25ce/src/main/clojure/clojure/core/async/impl/exec/threadpool.clj#L20
Spawning 8 buffers doesn't sound too bad.

[Malabarba]

That would be an option. You don't need to associate buffers with threads, you just need to spawn a new debug buffer if the previous one already has debug-mode active.

[stardiviner]

I got the same problem. And followed links to be directed to here. After reading all comments in this issue, I know current CIDER still not able to handle this.

I'm curious about CIDER has any plan on solve this?

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution and understanding!

[stale[bot]]

This issues been automatically closed due to lack of activity. Feel free to re-open it if you ever come back to it.

[darkleaf]

Is it possible to reopen this issue?

[gonewest818]

Seems like this conversation was productive up to the point it stalled ... I can reopen so people can wrap up thoughts one way or the other.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution and understanding!

[bbatsov]

Perhaps @FiV0 or @suvratapte would be interested in tackling this issue. :-)

[suvratapte]

@bbatsov : Okay. I will take a look. :)

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution and understanding!

[stale[bot]]

This issues been automatically closed due to lack of activity. Feel free to re-open it if you ever come back to it.