Deprecate aset-*?

52 views
Skip to first unread message

Chas Emerick

unread,
Jul 3, 2011, 3:03:10 PM7/3/11
to cloju...@googlegroups.com
I'm no expert in array handling in Clojure, but I know people that are. I see David Nolen saying[1]:

> aset-xxxx usage is not correct. Always use aset.


One of my coauthors, Christophe Grand, said in private correspondence:

> I don't see a single reason to prefer aset-* over aset. Those variations of aset and aget are both legacy and treacherous, luring the unknowing coder with their specific names... alas in this case more specific doesn't mean more efficient.

Can we go ahead and get :deprecated meta and deprecation documentation into 1.3 so they can be on their way out sooner rather than later? (For that matter, I see that `aset-int` is used in `make-array`, which is presumably bad for all the same reasons.)

If so, I'll open a ticket and add a patch.

- Chas

[1] http://groups.google.com/group/clojure/msg/290cf8ad49e40921

David Powell

unread,
Jul 4, 2011, 9:02:23 AM7/4/11
to cloju...@googlegroups.com
 
Can we go ahead and get :deprecated meta and deprecation documentation into 1.3 so they can be on their way out sooner rather than later?  (For that matter, I see that `aset-int` is used in `make-array`, which is presumably bad for all the same reasons.)

I recently ran into an issue where I had used aset in Clojure 1.2, and it worked fine, but when I upgraded to Clojure 1.3, aset failed, but aset-int fixed the problem.

The issue was with multi-dimension arrays.  

(def a (make-array Integer/TYPE 10 10))
(aset a 0 1 2)

In clojure 1.2, this works.  In clojure 1.3, it throws:

IllegalArgumentException argument type mismatch
        java.lang.reflect.Array.set (:-2)
        clojure.core/aset (core.clj:3448)
        clojure.core/apply (core.clj:604)
        clojure.core/aset (core.clj:3451)
        user/eval1 (NO_SOURCE_FILE:2)

Another workaround was:

(aset a 0 1 (Integer. 2))

... though that seems a bit ugly.

I presume the issue is that the value parameter of aset is now a primitive long, rather than an int, this gets boxed into a java.lang.Long, which then fails because Java is expecting a java.lang.Integer.

-- 
Dave

Kevin Downey

unread,
Jul 4, 2011, 4:00:50 PM7/4/11
to cloju...@googlegroups.com

Not 100% but I think this is a representation change, in 1.3 number liberals are longs so of course not asetable with an int array

> --
> You received this message because you are subscribed to the Google Groups "Clojure Dev" group.
> To post to this group, send email to cloju...@googlegroups.com.
> To unsubscribe from this group, send email to clojure-dev...@googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/clojure-dev?hl=en.
>

ataggart

unread,
Jul 4, 2011, 9:40:00 PM7/4/11
to Clojure Dev
The deeper problem is that the same exception occurs even when calling
(aset a 0 1 (int 2)). This is due to
Compiler.HostExpr.emitBoxReturn(...), which normally just converts
primitive return types into their boxed form, but also has a special-
case for boxing int returns into Longs. Until recently [1], floats
were similarly turned into Doubles. I just tested a similar change,
removing the boxing of ints to Longs, and it causes the int cast to
solve the exception. It's not clear to me what the intent of the old
boxing was, so I'm unsure what miight break (though it builds and
tests fine).


[1] https://github.com/clojure/clojure/commit/6fb09f402c5448070a2efc64ebd64285480b263f


On Jul 4, 1:00 pm, Kevin Downey <redc...@gmail.com> wrote:
> Not 100% but I think this is a representation change, in 1.3 number liberals
> are longs so of course not asetable with an int array
Reply all
Reply to author
Forward
0 new messages