Fork me on GitHub
#clojure-dev
<
2019-10-31
>
plexus13:10:28

(defprotocol IFoo
  (dest [this]))

(deftype Foo [^:volatile-mutable dst]
  IFoo
  (dest [this]
    (locking this
      (set! dst 123))))
;; Works ok

(deftype Foo [^:volatile-mutable dst]
  IFoo
  (dest [this]
    (let [dest (locking this
                 (set! dst 123))]
      dest)))
;; Unhandled clojure.lang.Compiler$CompilerException
;; Caused by java.lang.IllegalArgumentException
;; Cannot assign to non-mutable: dst
would this be a Clojure bug?

Alex Miller (Clojure team)13:10:06

I'd guess it's probably due to not allowing closing over mutables (https://clojure.atlassian.net/browse/CLJ-274)

ghadi16:10:16

It's because try blocks are lifted into separate functions

Alex Miller (Clojure team)14:10:31

or do you need the syntax (set! (.dst this) 123) for this?

Alex Miller (Clojure team)14:10:15

user=> (doc set!)
-------------------------
set!
  (set! var-symbol expr)
  (set! (. instance-expr instanceFieldName-symbol) expr)
  (set! (. Classname-symbol staticFieldName-symbol) expr)
Special Form
  Used to set thread-local-bound vars, Java object instance
fields, and Java class static fields.

  Please see 
  Used to set thread-local-bound vars, Java object instance
fields, and Java class static fields.

Alex Miller (Clojure team)14:10:49

From the link, "When the first operand is a symbol, it must resolve to a global var."

dominicm14:10:53

The advantage of clojure test for all the things is: - outputs (editor integration, tap, junit, cli exit code) - selection (metadata) - tooling (parallels, pretty diffs) These are (mostly, excepting perhaps editor integration) generic to generative, unit and integration tests I think? I think that's what you're getting at, but I'm not sure. It's worth making sure we all know what we're talking about.

plexus14:10:19

(.dst this) seems to work, thanks @alexmiller!

dominicm14:10:27

I'd summarize it by saying it's using different keys on vars to locate tests to clojure.test, and uses those as different inputs.

andy.fingerhut16:10:09

So is it just an accident of implementation that it happens to work sometimes with the (set! symbol expr) variant, i.e. when used inside of a deftype to mutate fields of the type instances?

andy.fingerhut16:10:54

Because as far as I can tell it does work in core.rrb-vector across dozens if not hundreds of uses.

ghadi16:10:43

It’s the fact that it is wrapped in locking, which has an internal try block

plexus20:10:03

The locking alone isn't the issue though, it's only the combination of locking and let

ghadi16:10:09

If that is the Q

Alex Miller (Clojure team)16:10:53

why does (set! (.dst this) 123) work then?

Alex Miller (Clojure team)16:10:04

I assume it's some difference in compilation but I didn't look any more closely at it

Alex Miller (Clojure team)16:10:28

I guess it doesn't have to close over the mutable in that case, just invokes via interop?

👍 4
ghadi16:10:59

Yeah I guess so? Seems accidental

Alex Miller (Clojure team)16:10:19

I mean, this is exactly the sort of place where you should be able to do this kind of thing

plexus20:10:03

The locking alone isn't the issue though, it's only the combination of locking and let

bronsa23:10:50

only locals are closed over, the let + locking combination causes the try to be lifted into a fn while just locking doesn't cause let puts the try block in a different compilation context

bronsa23:10:06

not trivial to fix cause changing the compilation of mutable locals (e.g. transforming foo to (.foo this)) is a potentially breaking change when closing over is done intentionally (capturing value vs capturing reference)

bronsa23:10:20

the hoisting patch that compiles hoisted functions as methods would fix that

❤️ 4
hiredman23:10:38

The auto hoisting is the source of a number of oddities

bronsa23:10:11

afaict that hoisting is a workaround for a compiled bug wrt locals (not sure if a bug or intentional compilation choice), but alternatives are possibile - - t.e.jvm iirc doesn't need that