@mfikes not sure if I fully understand your point - you’re saying funky can expand w/o refering?

@dnolen

In Clojure, you need to refer both funky and add. In ClojureScript, doing the same doesn't work.

oh you’re just saying that eval won’t work?

even if you refer both macros in ClojureScript

A theory is that in ClojureScript (require-macros '[foo.core :refer [funky add]]), the add symbol doesn't propagate to the point where the eval is done, which makes some sense if the refer add really only affects the ClojureScript compiler environment, and doesn't really affect the Clojure macroexpansion environment.

The specific error you get is an inability to resolve add.

cljs.user=> (require-macros '[foo.core :refer [funky add]])
nil
cljs.user=> (funky (add 2 3))
clojure.lang.ExceptionInfo: java.lang.RuntimeException: Unable to resolve symbol: add in this context, compiling:(NO_SOURCE_PATH:1:8) at line 1 <cljs repl> {:file "<cljs repl>", :line 1, :column 1, :root-source-info {:source-type :fragment, :source-form (funky (add 2 3))}, :tag :cljs/analysis-error}

eliding a lot of the stack, this is at the bottom

Caused by: java.lang.RuntimeException: Unable to resolve symbol: add in this context
	at clojure.lang.Util.runtimeException(Util.java:221)
	at clojure.lang.Compiler.resolveIn(Compiler.java:7214)
	at clojure.lang.Compiler.resolve(Compiler.java:7158)
	at clojure.lang.Compiler.analyzeSymbol(Compiler.java:7119)
	at clojure.lang.Compiler.analyze(Compiler.java:6680)

ah hrm would need to see the full trace to understand

but yes it looks like Clojure wants to resolve the symbols after the result of macroexpansion

Here it is https://gist.github.com/mfikes/041ba4326e15a9d38911d4553796c4a5

oh not even

My theory: When funky is being macroexpanded, the eval is executed (during expansion), with an environment that has no clue what add is.

yes I mean the stacktrace shows that

I just found it interesting because Clojure somehow succeeds. It made me think that it might be a corner case not handled by ClojureScript. (Either way to difficult to handle, or not even worth handling.)

I can’t think of an obvious way to make that work, since yes the eval happens in Clojure

and Clojure can’t know about ClojureScript requires

so probably just a limitation around how macros work in ClojureScript

eval seems strange in this case anyhow - it would work if you called the analyzer macroexpand I think?

ok, looking at the SO question I understand a bit more - but yeah that doesn’t seem like something you should try in ClojureScript

Odd aside: It "works" in self-host (if I replace the eval with Planck's imitation of Clojure's eval) Right, it is pushing the boundaries of what is possible. The only eval I've cared about during macroexpansion are the three in cljs.spec.test.alpha (here's one https://github.com/clojure/clojurescript/blob/master/src/main/cljs/cljs/spec/test/alpha.cljc#L111)

@mfikes right to me it makes sense it works in self-host

since the compilation env and runtime environment are in fact the same

Yeah. To make that work in JVM-ClojureScript would involve some mind-bending conveyance of refers from ClojureScript into Clojure.

Interestingly, even if the ClojureScript compiler doesn't "convey" the refers, you can manually use refer in Clojure macro definition for the SO use case and then the symbols resolve. (http://stackoverflow.com/a/44031185/4284484)

Is https://dev.clojure.org/jira/browse/CLJS-1495 still and issue? Seems to work fine for me

An extremely simple but kind of ugly way to speed up mapv: Add a second (unused) arity in the reducing function, thus avoiding the f.call(null...) call within the reducer:

(-> (reduce (fn
                 ([x] x)
                 ([v o] (conj! v (f o)))) (transient []) coll)
       persistent!)

Speedup about 5x in FF and 4x in Chrome (production builds). Thoughts?

Other candidates: filterv, replace, frequencies, into-array, find-and-cache-best-method, merge, merge-with, max/min-key, preserving-reduced (used in cat), run! (also probably fn->comparator)

@rauh better fix the f.call issue?

@thheller How? I'm assuming the .call(null, ..) has a purpose?

ah wait ... :static-fns doesn't do the f.call right?

Yes they do if there is only one arity for the function.

sorry, was in a different context. but if adding a unused second arity fixes anything thats a bug IMHO

there should never be work arounds like that

do you have a full example?

Well this is embarrassing, I messed up a number in the benchmark. It's only like 30% faster. Nowhere near as significant. 😕

@rauh yeah the speed diff isn’t huge, I did a lot of testing around this

Could a compiler option be of interest that switches off emitting those .call calls? I'm pretty sure most CLJS code would "just work", no?

@rauh no interest

.call is how you can use non JS-fns as functions

I thought the .call(null, ...) is just so that this is set to the global object, no?

@rauh I don’t think you understand what I’m saying

vectors, set, maps etc. are all functions … but not JS functions

@rauh we do implement specific invokes for IFn, but I think this would need careful testing to see that this won’t break anything - so low priority

@dnolen Right, I see what you mean. I'm not sure the .call is ever necessary since it'll never be invoked that way (from CLJS only projects). At least the chrome coverage tool shows it's never invoked.

I also don't understand why the IFn protocols are again added to the .call function:

(.. :a -call -cljs$core$IFn$_invoke$arity$2)

yes now that I think about it

I believe we optimized fn invokes before data structure invokes, so we needed .call

@rauh that just seems like a bug

@rauh if you want to open an ticket with details from Chrome coverage tool for a typical project that would be cool

it would be nice to see that this works with something non-trivial, like Re-frame or om.next

I'll play around and write down my findings.

Alright, killed off both .call and .apply prototype set!'ed by extend-protocol in core.cljc. Only change needed was one line in datascript:

((.-rschema db) prop)

Changed it to

(let [rschema (.-rschema db)]
  (rschema property))

The "weird" (.. :a -call -cljs$core$IFn$_invoke$arity$2) was just because of the multi arity fn that get's set! on the prototype.call.

That was on a large cljs project, ~17k LOC. Rum + datascript

@rauh did you completely remove the .call emission and still works or only for fns

I didn't change the compiler emitting the .call. I only changed the extend-type macro emitting prototype.call + apply for anything that implements IFn's

But those .call would fail if they were ever used on IFn, as it did for datascript.

So my hypothesis is that this could be changed so these obj.call aren't emitted any longer.

Oh right I read better ;)

https://dev.clojure.org/jira/browse/CLJS-2041

@rauh right but your investigation is kind of a deal breaker

needing to fix DataScript means such a change would be off the table

No, don't say that 😞

I'll fix datascript myself 🙂

too much downstream damage sorry

you can’t test the universe

anyways that’s what I was afraid of above as I said

and you immediately ran into it

Is a compiler option a chance?

yes

but not a default

I mean all my other code worked without any issues. And I write some really weird code 🙂

I'm totally happy with just an option and non-default.

@rauh, for extra credit, warning on setting call apply would be a good transition thing

then we can fix this further down the road

Now I wonder, should this option also avoid emitting the multi arity dispatcher on multi arity functions? Probably yes

Patch attached. Not yet dealt the possible optimization of avoid emitting the multi arity dispatch fn.