mfikes / ambly

ClojureScript REPL into embedded JavaScriptCore
http://ambly.fikesfarm.com
Eclipse Public License 1.0
541 stars 21 forks source link

Demunging function symbols in stack traces #62

Closed mfikes closed 9 years ago

mfikes commented 9 years ago

Ambly has a custom stack trace printing implementation and can attempt to demunge symbols.

Perhaps some rules like all dollar signs map to . except last which maps to /: cljs$core$firstcljs.core/first

Also, would need to deal with the few special encodings: cljs$user$foo_BANG_cljs.user/foo!

mfikes commented 9 years ago

Chatted with Nolen in IRC about a potentially deeper solution:

[19:34:26] <mfikes> dnolen: Evidently source maps support mapping back to original name. Perhaps this could lead to a clean solution for demunging cljs$core$first ↦ cljs.core/first. I haven't read enough about it yet to know if it is viable.
[19:34:52] <dnolen> mfikes: I couldn't find any evidence that this actually works in browsers though
[19:35:04] <dnolen> back when I first worked on this none of the browser seemed to do the name mapping
[19:39:02] <mfikes> dnolen: Right. I suspected something like that could render it useless. At best we could use it for stack traces but browser support would be missing :(
[19:39:44] <dnolen> mfikes: oh right, useless in browser but perhaps useful in our own REPLs
[19:39:50] <dnolen> would happily take a patch for it
[19:39:59] <dnolen> should be simple actually since that name is probably already there
[19:40:08] <dnolen> in the value returned from stacktrace mapping
[19:40:39] <mfikes> dnolen: OK. Perhaps it is useful for stacktraces, and if it aligns with the spec perhaps browser support would come along later.
[19:40:48] <mfikes> dnolen: I'll take a look into it. :)
[19:41:08] <dnolen> mfikes: I think all you need to do is add :name to line 240 in cljs/repl.clj
[19:41:15] <dnolen> and add some conditional logic if it exists
[19:41:18] <dnolen> downstream
[19:45:34] <mfikes> dnolen: I recall you saying that the munging could be different for each JavaScript environment. Perhaps you were referring to how symbols appear in stacktraces.
[19:49:53] <mfikes> dnolen: You've given me enough direction to go on. :) I'll see where it leads.
[19:50:52] <dnolen> munging for different JS environment is not worth it
[19:51:00] <dnolen> it's just too complicated and everyone does it differently
[19:51:14] <dnolen> your original intuition is right one, get the original name from the source map
[19:51:27] <dnolen> instead of the one provided by the JS host
[19:52:10] <mfikes> cool
[19:53:05] <dnolen> mfikes: it's really browser targets that make the munging thing impossible, every browser REPL based client would need to either handle N popular targets (which stink, browser targets are an open world) or rely on us to handle them
[19:53:36] <dnolen> neither of these solutions seemed very ideal which is why I passed the browser function name through and just made sure it was relatively dicipherable
[19:54:17] <dnolen> mfikes: obviously for controlled situations like Ambly not much of a concern (similar for Rhino, Nashorn, Node.js)
[19:57:25] <mfikes> dnolen: I don't yet see where per-browser munging comes into play. I do see how each environment has its own stack trace format, and I'm guessing you are referring to additional munging that could occur there like `Object.seq` in one environment vs maybe just `seq` in another.
[19:57:55] <dnolen> mfikes: or look at Firefox which actually puts noise chars into the name
[19:58:05] <dnolen> Obj.cljs.core.first</cljs.core.first
[19:58:12] <dnolen> and other crazy stuff like that
[19:58:26] <mfikes> dnolen: Ahh.. Thanks. Crystal clear to me now. :)
[19:58:36] <dnolen> demunging is impossible because don't get back what we wrote out in the source
[19:59:13] <dnolen> which is why your name idea sounds way more promising
[20:02:34] <mfikes> Well, I'm going to add :name to line 240 now and try that with Ambly. That's too easy to pass up finding out what happens. :)
mfikes commented 9 years ago

Adding :name, as in the following

(defn mapped-line-and-column
   "Given a cljs.source-map source map data structure map a generated line
    and column back to the original line and column."
   [source-map line column]
-  (let [default [line column]]
+  (let [default [line column nil]]
     ;; source maps are 0 indexed for lines
     (if-let [columns (get source-map (dec line))]
       (vec
-        (map inc
+        (map #(%1 %2)
+          [inc inc identity]
           (map
             ;; source maps are 0 indexed for columns
             ;; multiple segments may exist at column
@@ -237,7 +238,7 @@
               (if-let [mapping (get columns (dec column))]
                 mapping
                 (second (first columns))))
-            [:line :col])))
+            [:line :col :name])))
       default)))

is tantalizingly close to solving it.

The demunged names are in fact in there. They are shifted one layer outward in the call stack, making it a little more challenging to establish the association, but it certainly looks solvable: https://gist.github.com/mfikes/d3cd31cddeecb918677e

mfikes commented 9 years ago

Fixed in ClojureScript master.