Fork me on GitHub
#clojure-dev
<
2021-09-20
>
borkdude19:09:23

Not sure if I should post this in #cljs-dev or here :) It seems CLJS case behaves different with respect to ^:const than JVM Clojure. https://github.com/clj-kondo/clj-kondo/issues/1388#issuecomment-923203438 Is this a bug in CLJS or an intentional difference? @mfikes

hiredman19:09:36

It is a bug that has been enshrined never to be fixed

borkdude19:09:25

a culturally adopted bug then?

hiredman19:09:10

It is a bug that when reported, the issue was closed because fixing it will break existing code

borkdude19:09:44

should a linter account for "people expect this to work in CLJS" or should it say: consider the const unused when it's only used in the case here?

ghadi19:09:27

anyone have a link to the report?

borkdude19:09:56

it seems to be intentionally included here: https://clojure.atlassian.net/browse/CLJS-806

thanks3 2
hiredman19:09:05

that is for const specifcally

hiredman19:09:38

and just makes const inline with how case on cljs treats symbols generally, which is at odds with how case treats symbols in clojure

borkdude19:09:58

yeah, I was specifically referring to the const context

borkdude19:09:11

which is afaik the only different situation between clj and cljs

hiredman19:09:38

cljs will, last I checked, evaluate symbols

hiredman19:09:47

or atleast will sometimes

hiredman19:09:56

because sometimes it falls back to a cond

borkdude19:09:05

when I remove ^:const from the metadata, it yielded the same result as clojure

borkdude19:09:26

so at least for that example, the ^:const made the difference

borkdude19:09:19

I am surprised that CLJS intentionally was made to behave different from Clojure here.

borkdude19:09:30

Also asked this question in #cljs-dev

hiredman19:09:52

https://clojure.atlassian.net/browse/CLJS-2209 huh, maybe it is only the const case, but I could have sworn

🙏 2
borkdude20:09:47

@hiredman oh wow,

(def ^:const ALPHA 3)

(defn f [x]
  (case x
    ALPHA 1
    ;; :foo 2
    2))

(prn (f 3))
if you uncomment the line in the case expression, then the last expression will return 2, else 1... documented here by @mfikes https://blog.fikesfarm.com/posts/2015-06-15-clojurescript-case-constants.html all in all, it seems easy to shoot yourself in the foot here

hiredman20:09:24

ClojureScript 1.10.758
cljs.user=> (def ^:const x 1)
#'cljs.user/x
cljs.user=> (case 1 x 1)
1
cljs.user=> (let [x 2] (case 1 x 1))
Execution error (Error) at (<cljs repl>:1).
No matching clause: 1

cljs.user=>
is also fun

borkdude20:09:45

huh wow, the local can shadow the const var so it won't be inlined? cool

borkdude20:09:05

yeah, I guess it makes sense, but another thing to watch out for

ghadi12:09:52

second example is a bug. test constants should not be evaluated

borkdude13:09:28

@U050ECB92 can you clarify what is the second example? I lost track

ghadi13:09:06

the let fromn hiredman

borkdude13:09:36

which test constant is evaluated there?

borkdude13:09:58

I think CLJS compiles that as x the symbol

borkdude13:09:36

cljs.user=> (str (fn [] (let [x 2] (case 1 x 1))))
"function (){\nvar x = (2);\nvar G__23 = (1);\nif(cljs.core._EQ_.call(null,new cljs.core.Symbol(null,\"x\",\"x\",(-555367584),null),G__23)){\nreturn (1);\n} else {\nthrow (new Error([\"No matching clause: \",cljs.core.str.cljs$core$IFn$_invoke$arity$1(G__23)].join('')));\n\n}\n}"

ghadi13:09:37

the symbol x as test constant

ghadi13:09:51

docstring of case in clojure says: test constants are not evaluated

borkdude13:09:07

yeah, that's what happens here too: 1 is compared against the symbol x

ghadi13:09:52

So maybe not a bug

ghadi13:09:13

evaluating symbols in a test-constant seems wrong

ghadi13:09:25

sorry cljs / mike

borkdude13:09:39

to summarize: cljs only does this under strict circumstances: the symbol refers to a const and all test constants are primitives of the same type (numbers or strings). Given this, it seems easy enough to shoot yourself in the foot, but both David and Mike have written about it as if this is a conscious design decision, so I think it's a ship that has sailed. So I'll get rid of a false positive in clj-kondo for this.

borkdude13:09:42

I can see a use case for inlining constants in case, it's nice to not have to inline them yourself without having to write a macro, but it comes with trade-offs

didibus05:09:10

Isn't the only point of const the inlining itself? I think prior in CLJS, const was basically a useless annotation, since it had no effect.