Fork me on GitHub
#clojure-dev
<
2019-07-08
>
dominicm06:07:43

Given that (.getResource (var-get clojure.lang.Compiler/LOADER) "cognitect/test_runner.clj") works, why wouldn't (require 'cognitect.test-runner) given that the failure is FileNotFoundException Could not locate Clojure resource on classpath: cognitect/test_runner.clj clojure.lang.RT.loadResourceScript (RT.java:388). Disclaimer: I'm using a custom classloader.

dominicm06:07:02

Ah, clojure uses getResourceAsStream instead of getResource, I think the class loader is broken for AsStream with files. Bummer.

slipset07:07:18

Would a ticket for improving this error message be welcome?

slipset07:07:24

09:33 $ clj
Clojure 1.10.0
user=> (let [{:keys [foo]} 3] (println "foo"))
Syntax error (UnsupportedOperationException) compiling at (REPL:1:1).
Can't type hint a primitive local
user=>

mfikes16:07:33

Seems like ^ is a bug (it shouldn't fail at all)

ghadi17:07:00

what's slightly confusing is that there are no type hints in the user code, it's coming from destructuring macroexpansion

ghadi17:07:41

I'd be curious to see if this repro was distilled from a different scenario in the wild @slipset

slipset17:07:56

@ghadi No, I was rewriting some code at work and made the mistake of trying to destructure a number. So apart from naming the key foo and trying to print it, this was code I actually wrote.

slipset17:07:13

It comes from here:

slipset17:07:46

user=> *e
#error {
 :cause "Can't type hint a primitive local"
 :via
 [{:type clojure.lang.Compiler$CompilerException
   :message "Syntax error compiling at (1:1)."
   :data #:clojure.error{:phase :compile-syntax-check, :line 1, :column 1, :source "NO_SOURCE_PATH"}
   :at [clojure.lang.Compiler analyze "Compiler.java" 6808]}
  {:type java.lang.UnsupportedOperationException
   :message "Can't type hint a primitive local"
   :at [clojure.lang.Compiler$LocalBindingExpr <init> "Compiler.java" 6015]}]
 :trace
 [[clojure.lang.Compiler$LocalBindingExpr <init> "Compiler.java" 6015]
  [clojure.lang.Compiler analyzeSymbol "Compiler.java" 7297]
  [clojure.lang.Compiler analyze "Compiler.java" 6768]
  [clojure.lang.Compiler analyze "Compiler.java" 6745]
  [clojure.lang.Compiler$InvokeExpr parse "Compiler.java" 3888]
  [clojure.lang.Compiler analyzeSeq "Compiler.java" 7108]
  [clojure.lang.Compiler analyze "Compiler.java" 6789]
  [clojure.lang.Compiler analyze "Compiler.java" 6745]
  [clojure.lang.Compiler$HostExpr$Parser parse "Compiler.java" 1020]

slipset17:07:27

So the macroexpanded thing looks like:

user=> (clojure.pprint/pprint (macroexpand '(let [{:keys [foo]} 3] (println foo))))
(let*
 [map__165
  3
  map__165
  (if
   (clojure.core/seq? map__165)
   (clojure.lang.PersistentHashMap/create (clojure.core/seq map__165))
   map__165)
  foo
  (clojure.core/get map__165 :foo)]
 (println foo))
nil
user=>

slipset17:07:18

Which at least seems to have a redundant call to seq in the true branch of the if test.

slipset17:07:32

Compare this with planck:

slipset17:07:47

cljs.user=> (macroexpand '(let [{:keys [foo]} 3] (println foo)))
(let* [map__24 3
       map__24 (if (cljs.core/implements? cljs.core/ISeq map__24) (cljs.core/apply cljs.core/hash-map map__24) map__24)
       foo (cljs.core/get map__24 :foo)]
  (println foo))
cljs.user=>

bronsa18:07:06

repro

user=> (let [x 1] (when (seq? x) (.first ^clojure.lang.ISeq x)))
Syntax error (UnsupportedOperationException) compiling at (REPL:1:27).
Can't type hint a primitive local

bronsa18:07:52

doesn't seem to me like that type hint is needed

slipset18:07:02

Even if it is needed, wouldn’t it be safer to add it at the point where create is called?

bronsa18:07:07

yeah it was needed when the macro expanded to (c.l.PHM/create gmapseq) instead of (c.l.PHM/create (seq gmapseq))

bronsa18:07:20

it's useless right now

slipset18:07:07

Ah, so we need to somehow coerce/typehint the gmap local to get the correct version of c.l.PHM/create?

bronsa18:07:24

no, the current implementation doesn't need it

bronsa18:07:52

a previous implementation did, and so the type hinting was in place, but then a call to seq was introduced which rendered the type hint unnecessary but i guess it was just forgotten there

devn18:07:05

JIRA seems really slow. Not sure if there's something wrong, but for instance I am stuck at a loading screen here: https://clojure.atlassian.net/projects/TCLI/issues

slipset18:07:26

so could we just do (c.l.PHM/create gmap) (since we know from the if thest that gmap is a seq?

bronsa18:07:28

-                           gmapseq (with-meta gmap {:tag 'clojure.lang.ISeq})
                            defaults (:or b)]
                        (loop [ret (-> bvec (conj gmap) (conj v)
-                                      (conj gmap) (conj `(if (seq? ~gmap) (clojure.lang.PersistentHashMap/create (seq ~gmapseq)) ~gmap))
+                                      (conj gmap) (conj `(if (seq? ~gmap) (clojure.lang.PersistentHashMap/create (seq ~gmap)) ~gmap))

bronsa18:07:32

we could do this now

ghadi18:07:34

@devn I think there are some 'issues' with it... simple_smile

devn18:07:46

yeah, i clicked a couple times and now it seems to load All Issues

ghadi18:07:48

also the SSO gets confused

bronsa18:07:10

I for some reason can't login from firefox just on linux

8
ghadi18:07:01

@bronsa @slipset this seems really low on the priority list. the input code doesn't make sense, and removing that type hint (while it is unnecessary) will make it silently be accepted

seancorfield18:07:24

The issues page never loads (just spins) if you're logged into JIRA with a different account (or perhaps just not logged into your Clojure account?).

ghadi18:07:55

another possibility would be forbidding numbers on the right hand side of a map/vector destructuring

seancorfield18:07:09

You can do other stuff in JIRA with projects but the issues page won't load for un-auth'd accounts @devn

ghadi18:07:11

with defmacro specs

slipset18:07:04

No worries, that’s why I asked before creating an issue.

slipset18:07:46

I guess my main point was that while happy it didn’t fail silently, I was a bit surprised by the error message, and since those have been a priority lately, I figured it would be worth mentioning.

ghadi18:07:32

it's a surprising error for sure, totally unrelated to the input code

ghadi18:07:46

just don't want to jump to a solution

ghadi18:07:33

https://github.com/clojure/core.specs.alpha/blob/master/src/main/clojure/clojure/core/specs/alpha.clj#L53 perhaps init-exprs shouldn't be any? -- maybe they can be sensitive to the left hand side

slipset18:07:46

“Non-sensical destructuring fails with surprising error message”

slipset18:07:02

Probably my best Jira summary ever.

ghadi18:07:16

sounds good to me

mfikes18:07:56

Is associative destructuring sugar on top of get? Or is that never specified anywhere? (The reason I ask is that, if so, it seems to be related to whether get on a number is meant to return nil.)

ghadi18:07:23

yes and yes, (get 3 :foo) returns nil

ghadi18:07:21

this is a number at macroexpansion time, and I don't think that can ever make sense

ghadi18:07:44

(let [{:keys [a]} 42] ...)

slipset18:07:08

if (get 3 :foo) makes sense then I guess (let [{:keys [foo]} 3]..) makes sense as well?

mfikes18:07:33

Cool, just trying to keep it straight in my head (outside of this nonsensical example). Associative destructuring is sugar for get and sequential is sugar for nth with nil for unknown, in my simplified mental model.

souenzzo02:07:10

clj -e '(let [m (list :a 42) {:keys [a]} m] (prn [a (get m :a)]))'

ghadi18:07:04

(get 3 :foo) does not make sense, even though it (correctly) doesn't blow up

ghadi18:07:31

but most of the time you don't know it's a constant

mfikes18:07:33

Right, and we will never really know whether (get 3 :foo) is meant to return nil or not, I bet.

ghadi18:07:50

we do -- it is supposed to return nil

ghadi18:07:04

says rich

ghadi18:07:20

there's an explicit bottom-out in RT.getFrom()

ghadi18:07:38

get on something that doesn't know how to get returns nil

mfikes18:07:05

Yeah, that's what I resorted to as the spec of the behavior as well. Using the impl as the spec.

ghadi18:07:41

this one is by design

slipset18:07:08

are you implying there are things in Clojure which are not by design?

mfikes18:07:10

If we accept that, then Eric's last point makes sense.

ghadi18:07:49

(get 3 :foo) returns the correct thing (nil) but makes no sense from a user perspective

ghadi18:07:37

destructuring is slightly different in that you have the opportunity to check a spec at macroexpansion time

slipset18:07:43

I agree with @ghadi, since

user=> (def lol 3)
#'user/lol
user=> (let [{:keys [foo]} lol] (println foo))
works as expected

ghadi18:07:23

that works because it is not known that it is a compile time constant

ghadi18:07:34

there's no inference across var deref

ghadi18:07:59

((fn [x] (get x :foo)) 3) would be another example

slipset18:07:59

mmm, I guess my point was that if the above example would fail with an error, I’d be a little unhappy, because it’s not always clear that a var contains a constant

slipset18:07:40

where as (let [{:keys [foo]} 3] ...) is just plain, well, non-sensical.

slipset18:07:53

(and I’m not suggesting a rewrite/refactor, just being curious)

slipset19:07:19

are there reasons (like performance), for duplicating the code in get(Object coll, Object key, Object notFound) and get(Object coll, Object key)?

slipset19:07:51

I’d imagine that the latter could be implemented in terms of the former?

slipset19:07:26

(too long since I programmed Java to remember if there are problems related to overloading static fns)

ghadi19:07:58

performance for that one

ghadi19:07:22

get(m k not-found) on a java.util.Map has to check contains first, then call get

ghadi19:07:54

though on an ILookup is simply m.valAt(k, notfound)

ghadi19:07:17

same reason get() and getFrom() is split, too: performance. Put the fastpath into a small, fast method

ghadi19:07:31

nth / nthFrom, same deal

slipset19:07:42

So that it can be inlined?

slipset19:07:17

Cool, thanks 🙂