Clojurians Log v2

I've created an issue https://github.com/babashka/sci/issues/942 Hopefully it is clear enough

thanks!

Works well! Even some quick tries with transducers. I could find one more case specific to map i think:

["(str (map (fn [a] a) 1))"  [1 6]] ;=> [1 1]

I don't think that case is solvable due to laziness, clojure itself also doesn't know the location when you execute this: test.clj

(str (
      map
      (fn [a] a)
      1))

user=> (load-file "/tmp/test.clj")
Execution error (IllegalArgumentException) at java.lang.Object/toString (Object.java:270).
Don't know how to create ISeq from: java.lang.Long

if I would radically re-engineer SCI in how the stack trace is produced, I could maybe make it work...

Yeah can imagine this one is tricky. But to be fair I think Sci's error reporting is close to be better than Clojure's error reporting 😎

just out of curiosity, what are you building with SCI on the JVM?

Something like pcp , like a web framework, but it is very early days

But I try to use the error information to display inline errors. A bit like babashka

I use it with hiccup, so if the error information is not precise the displayed error is not very helpful

In general with the column information you normally get very nice errors with Sci

I'm using the stacktrace information to highlight the exact location in the code

The problem is that lazy sequences are only triggered during the html building, so the Sci stacktraces don't point to the line of a for call

To illustrate this problem:

;; line 1, column 1

(try (sci/eval-string "(str 
                         (for [[a] 1] a))") 
     (catch Exception e 
       (select-keys (ex-data e) [:line :column])))

;; vs doall, line 2, column 26

(try (sci/eval-string "(str 
                         (doall (for [[a] 1] a)))") 
     (catch Exception e 
       (select-keys (ex-data e) [:line :column])))

A quick hack is to change the 'for' binding to a for wrapped with doall

But I would prefer to do this only in the context of hiccup and not for the whole Sci context. Not sure if this can be done from a macro?

The other thing I was thinking about is to port the Hiccup interpreter to Sci and do something with the stacktraces, but I'm not sure if I can manipulate the stacktrace in a sensible manner this way

It seems to be only an issue when something goes wrong in the destructuring or in the arguments of for or map et al. If there is an issue in the body of the macro or function the stacktrace is correct

e.g.

(try (sci/eval-string "(str
                         (for [a 1] (/ a 0) a))")
     (catch Exception e
       (select-keys (ex-data e) [:line :column])))
=> {:line 2, :column 26}

hey @jeroenvandijk let's see...

yeah I think this is just how laziness works unfortunately, not much SCI can do here I'm afraid

you're using SCI directly for hiccup, so this isn't related to bb or scittle or so right?

Yeah Sci directly for hiccup

This is another hack I found

(sci/eval-string* (sci/init {:deny '[for]
                             :bindings {'for2 (macrofy (fn [&form & args]
                                                         (list (with-meta 'doall (meta &form)) (apply for-macro/expand-for &form args))))
                                        'html (macrofy
                                               (fn [_ _ body]
                                                 (clojure.walk/postwalk (fn [x]
                                                                          (if (= x 'for)
                                                                            (with-meta 'for2 (meta x))
                                                                            x))
                                                                        body)))}

                             })
                  "(html (for [[i] (range 100)] (/ i 0)))")

you could just do this:

{:namespaces {'clojure.core {'for ...}}}

yeah true, but then i would do it everywhere, even if I am not rendering html. I was hoping to isolate more to hiccup

I was also wondering if there is a way to access the ctx from a macro, but i think this is not possible, right?

you could use with-redefs in your macro, but also this would not play well with laziness

the way to access the ctx from a macro would be something like this:

(require '[sci.ctx-store :as store])
(def ctx (sci/init ..))
(store/reset-ctx! ctx)

hmmm i will give that I try. I think in sci you don't have to worry about thread safety for with-redefs

> I think in sci you don't have to worry about thread safety for with-redefs hmm, it depends if you're running SCI in multiple threads

I do, but maybe I'm still safe because of sci/fork. Although not sure

the built-in vars are all shared

ah i see

so maybe it would be the safest to make a combination of what we had. copy the SCI for macro into a new var, add this to clojure.core and then use with-redefs

or what you could also do

make the for macro in SCI dynamic and then use binding

then it would be thread-safe

ah cool, yeah that's smart

I will give it a try, thanks!

I can't seem to bind the macro to the dynamic var. Here is some of my experiment. I'm probably missing something

(require '[sci.impl.for-macro :as for-macro])

(def for-macro (sci/new-var 'for nil #_for-macro/expand-for {;:sci/macro true 
                                                             :dynamic true}))


(sci/eval-string* (sci/init {:namespaces {'hiccup {'eager-for (macrofy (fn [&form & args]
                                                                           '(throw (ex-info "for eager" {}))
                                                                           #_(list (with-meta 'doall (meta &form))
                                                                                   (apply for-macro/expand-for &form args))))
                                                   'html (macrofy (fn [_ _ & body]

                                                                    (concat (list 'binding '[clojure.core/for hiccup/eager-for])
                                                                            body)))}
                                          'clojure.core {'for for-macro
                                                         '*for* for-macro}}})
"(binding [*for* hiccup/eager-for] (*for* 2))"

                 #_ "(str
(binding [*for* hiccup/eager-for] 
  (clojure.core/*for* 2 ;#_[[i] (range 100)] (/ i 0)
)))"
  #_                "(str
(hiccup/html 
(for [[i] (range 100)] (/ i 0))))")

I think you meant to make for a dynamic var, right? In clojure this would not be possible AFAIK

There's probably something hard-coded in SCI concerning for But something like this isn't go work anyway:

(sci/eval-string* ctx "(defmacro my-for [& args] 42) (binding [for @#'my-for] (for [i [1 2 3]] i))")

so never mind what I said, too optimistic when not trying it for real in a REPL ;)

ah too bad, was worth a try though

so yeah I think your postwalk thing might be ok, but at the risk that you would also replace for local variables and function arguments etc ;)

Thanks for your help. I'll play with it and see how it goes

One more question, since it is possible to point to the exact location in case of an error in the body of a lazy sequence e.g. in the case of (for [i (range 10)] (/ i 0)) or (map (fn [i] (/ i 0)) (range 10)), sci can point out the exact location of /. Shouldn't this be possible somehow for the destructuring bit for instance? I tried to look at the code, but I don't see directly where all this is happening

hmm, you may be right, can you maybe post a SCI issue about a better error location reporting for your case? perhaps it's just a matter of fixing some metadata somewhere

Fixed:

$ clj -M:babashka/dev -e '(str (let [[a] 1] a))'
----- Error --------------------------------------------------------------------
Type:     java.lang.UnsupportedOperationException
Message:  nth not supported on this type: Long
Location: <expr>:1:6

----- Context ------------------------------------------------------------------
1: (str (let [[a] 1] a))
        ^--- nth not supported on this type: Long

Hm, for should also still be improved

for is a lot harder due to laziness probably

Can imagine. I will have a look at your fix. I didn't know where to look actually to fix it

o I wait, I do see something improving here:

1: (str (for [[a] [0]] (/ 1 a)))
        ^--- nth not supported on this type: Long

Thanks!

anyway, I'll also dig a little further

> o I wait, I do see something improving here: I think that's a major improvement

Looks small here, but in a bigger context it is significant

are you using this from JVM or CLJS?

From the JVM

pushed another improvement for for. feel free to post more cases

Cool! I found one more. Do you want me to post it here?

Failing:

["(str (map (fn [[a]] a) [0]))" [1 6]] ;=> [1 1]
           ["(str (filter (fn [[a]] a) [0]))" [1 6]] ;=> [1 1]

Failing:

["(str (if-let [[a] 0] a))" [1 6]] ;=> [1 1]
["(str (when-let [[a] 0] a))" [1 6]] ;=> [1 1]
["(str (if-some [[a] 1] a))" [1 6]] ;=> [1 1]
["(str (when-some [[a] 1] a))" [1 6]] ;=> [1 1]

Ok:

["(str (loop [[a] 0] a))" [1 6]]
["(str (letfn [(f [[a]] a)] (f 1)))" [1 27]]

thanks!

np!

all of the above work now

also handled most doseq cases

Wow, that's amazing. Thank you!