Rewrite the debugger

[Malabarba]

This is the clojure counterpart to clojure-emacs/cider#1149
This completely rewrites the debugger (again) to be simpler and (I believe) more reliable.

The big complication with the previous debugger were macros.

• You can't just instrument everything inside a macro, that will surely break stuff.
• You can't expand the macro before instrumenting it, because then you completely lose track of the original code, you won't be able to know how to find a given sexp on the original source file, and you will end up wrapping brakpoints around code that the user didn't write.

The previous debugger took a lesson from Edebug and tried to fix the first option by instrumenting the macro arguments according to a specification. This worked surprisingly well, but:

• It's complicated as hell
• If the user defines their own macros, they're unlikely to follow the specification we expect, which means it won't be debuggable.
• Even within the clojure.core namespace it was difficult to test everything (for instance, in writing this, I just realised we still don't support letfn).

The version proposed here goes with the second option instead, by using something that's not available in emacs-lisp, metadata.

1. First, we walk through the code and attach metadata to every object inside it, recording the location of this object in the code.
2. Then we macroexpand everything, so we don't have to worry about macros. We still need to provide special hadling for special-forms, but they are limited in number (see instrument-special-form).
3. Then we walk through the code again, looking for the metadata we had written. Wherever we find it, we know this correponds to user code and we know its original location, so we can use it to make a breakpoint.

Looking at the diffs, it should go without saying that this is much simpler. About half the original code was dedicated to parsing macros.

Notable differences

• This also introduces a new way of instrumenting. Before, we had to call instrument on a top-level sexp, and this would instrument everything. Now, we call instrument-tagged-code on a top-level sexp, and it will only instrument objects with a :cider-breakfunction metadata. In particular, this means that if you call instrument-tagged-code on a clean sexp, it won't do anything. You have to call debug-reader or breakpoint-reader on the sexp first, but this is stomething that already happened if the sexp was read with a #dbg or #bp reader macro.
• Instead of the hacky way we did previously (where cider.el sent us an “eval” message with the code manually wrapped in a instrument-and-eval), the wrap-debug middleware now hooks cleanly into the “eval” op. If neither one of the reader macros (#dbg or #bp) are present in the received code, the message just falls through for regular evaluation. If one of the macros was present, wrap-debug just adds its own evaluator to the message before letting it fall through.
• For now, “eval” messages need a file and a point to be properly debuggable. But I've been thinking of getting rid of that. We could instead just popup a temp buffer displaying the code being debugged (this would be easier and wouldn't break if the user edited the source).

[expez]

It's customary to use :else to denote the default branch in clojure.

[Malabarba]

👍

[expez]

macrostep extends edebug in an interesting way, letting you expand a macro and then step through the expanded form in the debugger. It reads to me like this functionality is within reach now, if we want it.

I've just given this a cursory glance because I was curious but this looks like a sound approach!

[Malabarba]

@expez I think it would be possible. We would essentially have to use debugger's "eval in lexical scope" functionality where the code would be like (instrument-code (debug-reader (macroexpand 'CODE-AT-POINT))). If nothing goes wrong, that would start a new debugging session inside the current one.

[Malabarba]

Tests passing.

[Malabarba]

cljfmt indented this line like this, but it looks like a bug to me.

[bbatsov]

Definitely a bug. You should probably report it upstream.

[Malabarba]

FIled weavejester/cljfmt#42