ClojureScript

make-array signature differs from clojure

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Completed
  • Affects Version/s: 1.7.48
  • Fix Version/s: None
  • Component/s: None
  • Labels:
  • Patch:
    Code

Description

When I was porting clj code using a two-dimensional array I noticed the cljs version only accepts size, instead of dim & more-dims.

Signature in cljs: ([size] [type size])
signature int clj: ([type len] [type dim & more-dims])

Activity

Hide
David Nolen added a comment -

Unlike on the JVM there's no efficient way to allocate multi-dimensional arrays. An enhancement patch that includes a modification to the docstring that warns about the computational cost is welcome.

Show
David Nolen added a comment - Unlike on the JVM there's no efficient way to allocate multi-dimensional arrays. An enhancement patch that includes a modification to the docstring that warns about the computational cost is welcome.
Hide
Lars Andersen added a comment -

What I want is for the signatures to be the same, so I don't have to use reader conditionals to get the same behavior on the two platforms.

When I'm asking for a two-dimensional array, I'm presumably going to do work that is O(m*n), so even though the pre-allocation is O(m*n) too it won't affect the runtime of the program significantly.

Show
Lars Andersen added a comment - What I want is for the signatures to be the same, so I don't have to use reader conditionals to get the same behavior on the two platforms. When I'm asking for a two-dimensional array, I'm presumably going to do work that is O(m*n), so even though the pre-allocation is O(m*n) too it won't affect the runtime of the program significantly.
Hide
David Nolen added a comment -

Lars, yes I understood As I said patch welcome. This is a very ClojureScript newbie friendly ticket.

Show
David Nolen added a comment - Lars, yes I understood As I said patch welcome. This is a very ClojureScript newbie friendly ticket.
Hide
António Nuno Monteiro added a comment -

Submitted patch with suggested changes

Show
António Nuno Monteiro added a comment - Submitted patch with suggested changes
Hide
Erik Assum added a comment -

@Antonio: Out of curiosity, isn't the "will run in polynomial time" a bit unprecise, since I guess all array-allocations run in polynomial time?

Show
Erik Assum added a comment - @Antonio: Out of curiosity, isn't the "will run in polynomial time" a bit unprecise, since I guess all array-allocations run in polynomial time?
Hide
António Nuno Monteiro added a comment -

You're right. I misunderstood what David mentioned regarding the JVM performance. should it just say "Note that there is no efficient way to allocate multi-dimensional arrays in JavaScript" and stop there?

Show
António Nuno Monteiro added a comment - You're right. I misunderstood what David mentioned regarding the JVM performance. should it just say "Note that there is no efficient way to allocate multi-dimensional arrays in JavaScript" and stop there?
Hide
Mike Fikes added a comment -

This patch fails for me if I try using it with make-array in operator position. In fact, there's an underlying issue CLJS-1555 that probably needs to be fixed first, and then a patch could be made that takes care of the extra arity in the macro version of make-array as well.

Show
Mike Fikes added a comment - This patch fails for me if I try using it with make-array in operator position. In fact, there's an underlying issue CLJS-1555 that probably needs to be fixed first, and then a patch could be made that takes care of the extra arity in the macro version of make-array as well.
Hide
Andrea Richiardi added a comment -

I launched ./script/test today for another issue and got:

{{Analyzing file:/home/kapitan/git/clojurescript/src/main/cljs/cljs/core.cljs
WARNING: Use of undeclared Var cljs.core/apply at line 369 /home/kapitan/git/clojurescript/src/main/cljs/cljs/core.cljs}}

Show
Andrea Richiardi added a comment - I launched ./script/test today for another issue and got: {{Analyzing file:/home/kapitan/git/clojurescript/src/main/cljs/cljs/core.cljs WARNING: Use of undeclared Var cljs.core/apply at line 369 /home/kapitan/git/clojurescript/src/main/cljs/cljs/core.cljs}}

People

Vote (0)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved: