Fork me on GitHub
#clojure-dev
<
2019-01-10
>
slipset18:01:37

I just came across https://dev.clojure.org/jira/browse/CLJ-2464 and was wondering, if one were to create a patch for this. Would the correct implementation be to let PersistentVector$TransientVector implement IPersistenStack, and then implement both peek and pop for PersistentVector$TransientVector?

Alex Miller (Clojure team)19:01:53

It may not be trivial to fix without significantly changing the class hierarchy. There is at least one other similar ticket in the system that has a similar feel.

Alex Miller (Clojure team)19:01:34

I don’t have time to boot all that context into memory right now

slipset19:01:54

Then I’ll just leave it be and have a coffee instead.

slipset19:01:15

And by the way, happy new year to you, Alex!

mikerod19:01:49

Ran across an interesting exception and had me wondering a bit here. Exception was of the form: java.lang.IllegalArgumentException: find not supported on type: my.ns.RecordType$reify__26254 This was from a callsite looking like this (find (:x (->my.ns.RecordType)) :y) The (:x (->my.ns.RecordType)) is a keyword callsite, I think it is being optimized via the KeywordLookupSite logic. This is one of the only places on the record def I’d see a “reify inner class” - coming from the defrecord impl of clojure.core/getLookupThunk This looks like this

(clojure.core/reify
  clojure.lang.ILookupThunk
  (clojure.core/get
      [thunk gtarget]
      (if
          (clojure.core/identical? (clojure.core/class gtarget) gclass)
          (. gtarget -x)
          thunk)))

mikerod19:01:08

I’m thinking that this is returning the thunk itself, instead of the target thing

mikerod19:01:16

Maybe because (clojure.core/identical? (clojure.core/class gtarget) gclass) is false?

mikerod19:01:24

This is happening in a multi-thread situation

mikerod19:01:45

I’m wondering: why can this if go down the false branch? What happens then?

mikerod20:01:16

returning the thunk is pretty different than returning the target object I’d think, so the things get returns is odd. I guess it is checked by the caller with an identical? check prob

Alex Miller (Clojure team)20:01:28

maybe you reloaded the code and have a record instance from an old version of the record class?

mikerod20:01:48

yeah, so maybe a classloader thing

mikerod20:01:50

haven’t ruled it out

mikerod20:01:55

but this code in particular did confuse me

Alex Miller (Clojure team)20:01:55

I’m just guessing, have never looked at this code

mikerod20:01:09

dang, thought you’d be an expert on the kw lookup site stuff

mikerod20:01:37

haha, indeed

mikerod20:01:11

it’s fine, I’ll dig more at things. and yeah, certainly could just be classloader problems.

Alex Miller (Clojure team)20:01:50

looking at that code you posted, it looks wrong

Alex Miller (Clojure team)20:01:41

or rather the branches look inconsistent in what they return

Alex Miller (Clojure team)20:01:00

doesn’t it seem like the thunk should be invoked at the end?

Alex Miller (Clojure team)20:01:11

(thunk) instead of thunk ?

Alex Miller (Clojure team)20:01:46

if it’s not invoked it returns the thunk and that seems to match the error you get

ghadi20:01:57

IIRC returning the thunk itself is a sentinel value that means that the inline cache is wrong

ghadi20:01:47

it’s a neat way of avoiding an (Object.) or ::none sentinel

mikerod20:01:22

Thanks @ghadi I suspected the sentinel

mikerod20:01:55

The exception I’m getting is odd. Makes me think find ends up trying to be called on the thunk itself

Alex Miller (Clojure team)20:01:05

that’s exactly what it looks like

mikerod20:01:13

Even with classloader mix ups. Just doesn’t seem like something I’d expect to happen

mikerod20:01:44

I’d expect the inner thunk detail to be transparent. But who knows. Maybe I’ll be able to recreate something here enough to dig through.

Alex Miller (Clojure team)20:01:56

can’t say I’ve seen anything like it before

mikerod20:01:22

Example was someone using clara-rules session within a core.async go-block. So have a bit to dig through I’m sure.