Fork me on GitHub
#cljs-dev
<
2018-06-26
>
Roman Liutikov09:06:43

Is this a bug?

clojure => (int \z) ;; 122
cljs => (int \z) ;; 0 (warning all arguments must be numbers)

Roman Liutikov09:06:22

for this to work in cljs int should do .charCodeAt on a string

mfikes11:06:52

It would be nice if int were consistent, this appears to be a hosty difference in that, in Java, char is easily treated as a numeric.

pesterhazy11:06:25

I noticed this today as well. Trouble is that Javascript doesn't distinguish between char and string

mfikes11:06:36

Yeah, I would chalk it up as a hosty difference that, if attempted to smooth over, would be at odds with perf.

pesterhazy11:06:38

Plus, what would (int "aa") return?

pesterhazy11:06:03

(Given that "aa" and \a have the same type)

mfikes11:06:48

This doesn't work in Clojure, which is related.

(subvec (into [] (range 256)) \a)
I think it is water under the bridge, but perhaps Clojure's int should never have worked on character types.

Alex Miller (Clojure team)12:06:32

That is one of its primary uses, so disagree

mfikes11:06:44

(If you take a peek at ClojureScript's subvec you would see that messing with int would all of the sudden either make that code start working and/or slow it down.)

Roman Liutikov11:06:01

Agreed about overall perf impact. Not sure if this should be addressed via type hint?

bhauman11:06:29

@mfikes I'm tracking down the printing behavior during the ClojureScript test run

bhauman11:06:57

it seems to be a side effect of calling prop/for-all in two different tests

bhauman11:06:04

which is strange

mfikes11:06:37

Odd. At least it didn't cause a dancing frog to jump out of a cigar box 🙂

4
bhauman12:06:17

actually its two different defspecs I commented them out and then it ran no problem

bhauman12:06:36

and defspec is only used twice in the cljs test suite, and I get the same behavior in chrome and safari, what the heck is defspec doing

mfikes12:06:20

You get the print dialog opening in Chrome as well. WTF?

bhauman12:06:12

yes two different times, exactly when defspec is called

mfikes12:06:00

Canary detected an issue with Spector. I'm inclined to believe that Spector is making an unwarranted assumption about vec and created an issue there https://github.com/nathanmarz/specter/issues/261 Please chime in if you think my view is misplaced.

bhauman12:06:45

@mfikes (set-print-fn! js/print) in the test runner

bhauman12:06:07

that's the cause

mfikes12:06:13

Those unit tests aren't often executed directly from within a browser. So, unrelated completely to your test result display code.

bhauman12:06:43

and they run fast now

bhauman12:06:51

so very cool

mfikes12:06:00

Nice. The attempt to print also explained the lag?

bhauman12:06:23

I think the initial lag is all the loading involved

bhauman12:06:38

This was a good reminder that I need renderers for test.check output though

mfikes12:06:38

It is hilarious that js/print can either mean sense 2a "to make a copy of by impressing paper against an inked printing surface" or 2d: "to display on a surface (such as a computer screen) for viewing" (Merriam Webster)

mfikes13:06:00

The issue mentioned above regarding Specter has been fixed downstream https://github.com/nathanmarz/specter/commit/e7abb2b5384b0b64d871fb347be7c34a15473eb2 and Canary is passing again.

👍 8
mikerod00:06:16

@mfikes any background info on why those change was needed? I’d have thought it is equivalent as far as correctness goes.

mfikes00:06:35

@U0LK1552A That particular bit of code is being used in a context which extends a protocol to cljs.core/Subvec. A consequence of the optimization on master (for CLJS-2442) is that vec directly returns any subvec passed to it. This causes the Specter code to dispatch back to the same extension for cljs.core/Subvec, and thus leads to infinite recursion. The intent of the Specter code is to convert the subvec to a persistent vector instance (not a subvec of a persistent vector), so into [] suffices to achieve the intent.

mfikes00:06:10

TL;DR, types matter to that particular code—with the difference between cljs.core/Subvec and cljs.core/PersistentVector being important.

mikerod00:06:16

Ah ok. That makes sense. I know specter does a lot of digging at types to try to achieve high performance etc. so it makes sense that kind of edge case could creep in. Thanks for the details.

mfikes14:06:26

@ambrosebs It is skipping ClojureScript source compilation and returning "cached" JavaScript associated with the load4.core macros namespace. (The test is essentially checking that this results in load4.core$macros being placed in the *loaded* namespace set.)

ambrosebs14:06:28

hmm is compiler/emit involved in any way?

ambrosebs15:06:25

I assume it is because I made compiler/emit error. Perhaps more usefully to me: is a defmethod on ISeq enough to dispatch an empty LazySeq to that method in self-hosting?

ambrosebs15:06:45

There's almost definitely some bigger misunderstanding I'm having. I think I've hit a wall in trying to guess what self-hosting will do.

ambrosebs15:06:22

I'm trying to add a :quote op to the analyzer that gets used on any 'quote form, then the quoted value becomes a :const child of the :quote node. This is my progress so far https://github.com/clojure/clojurescript/commit/d849cbd1d40fca8e5623abdaadb46fc929998f28 and the build failure https://travis-ci.org/frenchy64/clojurescript/builds/396685198?utm_source=github_status&amp;utm_medium=notification

ambrosebs15:06:53

the change is pretty invasive, emit-constant needs many more cases.

ambrosebs15:06:41

the github link is only an approximation of the actual built code, I rebased and nerfed the original commit

mfikes18:06:49

@ambrosebs One problem is that, the defmethods in that revision currently have protocol names in the :cljs branches, but instead need type names.

mfikes18:06:37

^ In that spot IVector should be changed to PersistentVector

mfikes18:06:39

But that raises a problem if you need to also catch Subvec or MapEntry, or anything else that would satisfy vector?

ambrosebs18:06:06

ah. Are there any examples that I can copy of a defmulti that dispatches intelligently on any possible CLJS type?

ambrosebs18:06:20

is this better as a cond?

mfikes18:06:41

I ran in to that very problem here and ended up with a solution that was "open" because it uses defmulti but a bit tied to types: https://github.com/planck-repl/planck/blob/master/planck-cljs/src/planck/io.cljs#L388

ambrosebs18:06:40

ok I'll give it a shot

mfikes18:06:56

Yeah, maybe a cond. I suspect there aren't many bits of code extending emit-constant

mfikes18:06:08

The ability to do that in this blog post was arguably nice http://blog.fikesfarm.com/posts/2016-01-22-clojurescript-eval.html but I wonder if that aspect is really being used out there

ambrosebs18:06:17

hmm I was going to rename emit-constant to emit-constant* and make emit-constant a function that emits metadata for each constant (then calls the multimethod)

ambrosebs18:06:05

didn't consider I might care about breaking code by renaming

mfikes18:06:02

To be honest, I would be surprised if people are extending the compiler with their own defmethods. Hrm.

ambrosebs18:06:17

ok let's assume it's a non-issue. Is there a better dispatch function for this defmulti to be compatible with protocols?

ambrosebs19:06:10

perhaps a good compromise is to have a cond for the base types and then default to a multimethod for tagged literals and such

ambrosebs22:06:07

@mfikes do you know what emit-constant case (or predicate) I can use to handle this constant?

ambrosebs22:06:45

failed compiling constant: [object Object]; function (val){ this.val = val; } is not a valid ClojureScript constant.

ambrosebs22:06:09

this is in self-hosting mode

mfikes22:06:21

I wonder what the associated source is. All of these things would involve a quote in the source?

ambrosebs23:06:08

and yes I think so, all involve quote

mfikes02:06:59

@ambrosebs If you revise the "not a valid ClojureScript constant" error string slightly to include (pr-str x), you will see #object[cljs.tagged-literals.JSValue]

mfikes02:06:55

Planck REPL shows

cljs.user=> (require 'cljs.tagged-literals)
nil
cljs.user=> (cljs.tagged-literals/->JSValue 1)
#object[cljs.tagged-literals.JSValue]
cljs.user=> (type *1)
cljs.tagged-literals/JSValue
so it is an open question as to why this isn't dispatching to https://github.com/frenchy64/clojurescript/pull/19/commits/ba0475124317dbcc4f371b84977e96f8be67371c#diff-55b85385d2d0bfb6dc20d59ed982d5c8R386

ambrosebs20:06:56

is there a difference between the values #object[cljs.tagged-literals.JSValue] and cljs.tagged-literals/JSValue?

ambrosebs20:06:15

I don't know what #object means

ambrosebs20:06:01

is that an instance?

ambrosebs20:06:23

ok I figured it was. Not sure who's in charge of printing #object tho.

ambrosebs21:06:14

now I'm really confused xD

mfikes21:06:15

Hmm. Well, I don't have an appreciation for the big picture of what is going on here.

ambrosebs21:06:00

how do I get a REPL that's like running script/test-self-parity?