<< Back to previous view

[CLJS-858] resolve-existing-var does not work for vars from other namespaces Created: 16/Sep/14  Updated: 16/Sep/14  Resolved: 16/Sep/14

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: David Nolen Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None


 Comments   
Comment by David Nolen [ 16/Sep/14 6:29 PM ]

fixed https://github.com/clojure/clojurescript/commit/877b1ab2e6bb1c09d1988348d6cb384f8ba16414





[CLJS-857] change deftype*/defrecord* special forms to include their inline methods decls Created: 15/Sep/14  Updated: 15/Sep/14

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Nicola Mometto Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File 0001-change-deftype-defrecord-special-forms-to-include-th.patch    
Patch: Code

 Description   

Currently the deftype* and defrecord* special forms don't include their inline method declarations, this means that code like:

(deftype foo [x] Object (toString [_] x))

is macroexpanded into something that looks like:

(deftype* foo [x] pmask) (extend-type foo Object (toString [_] x)) ...

The issue with this approach is that we want to treat "x" in the extend-type expression as if it was inside the lexical scope of the deftype*, to do so the analyzer attaches a metadata flag to the extend-type call with the deftype* local fields.

This is not ideal and complicates needlessly the analyzer, I propose to simply move the extend-type call as a parameter to the deftype* special form, this way no special handling for local fields will be needed:

(deftype foo [x] pmask (extend-type foo Object (toString [_] x)))

since the extend-type code is effectively inside the lexical scope of the deftype.

Everything said above applies to defrecord* aswell, this patch implements what I proposed and refactors a bit the code in the analyzer to remove the code duplication in the defrecord* and deftype* parse implementations.

In addition, the current approach was unfeasable to implement for tools.analyzer.js so taking this patch would mean making cljs.analyzer and tools.analyzer.js special-form compatible.






[CLJS-856] On Node.js, (seq (array 1 2 3)) throws exception Created: 12/Sep/14  Updated: 15/Sep/14  Resolved: 15/Sep/14

Status: Resolved
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Peter Schwarz Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: nodejs
Environment:

Os X
Node.js 0.10.31
Clojurescript 0.0-2311



 Description   

Calling (seq (array 1 2 3)) throws the following exception:

Error: 1,2, 3 is not ISeqable
    at seq (cljs/core.cljs:38:9)
    at <cljs repl>:1:79
    at <cljs repl>:5:3
    at Socket.eval (eval at <anonymous> ([eval]:1:20), <anonymous>:70:29)
    at Socket.emit (events.js:95:17)
    at Socket.<anonymous> (_stream_readable.js:764:14)
    at Socket.emit (events.js:92:17)
    at emitReadable_ (_stream_readable.js:426:10)
    at emitReadable (_stream_readable.js:422:5)
    at readableAddChunk (_stream_readable.js:165:9)

However, calling (seq (aclone (array 1 2 3))) correctly results in (1 2 3)

Viewing it in a repl, one does see the following:

ClojureScript:cljs.user> (array 1 2 3)
#<1,2,3>
ClojureScript:cljs.user> (aclone (array 1 2 3))
#js [1 2 3]

Not sure if that is helpful, but the array created does seem to slightly different.



 Comments   
Comment by David Nolen [ 15/Sep/14 12:24 PM ]

In Node.js JS instances may be created in other VM contexts. The same is true for moving JS objects between frames in the browser. You need to handle this case yourself via extend-type.





[CLJS-855] Combinatorial code explosion with nested functions of unknown arity Created: 12/Sep/14  Updated: 16/Sep/14

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Brendan Younger Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

ClojureScript 0.0-2322


Attachments: File test-advanced.js     File test-whitespace.js    

 Description   

The following code, when compiled with :advanced optimizations leads to 32 repetitions of the inner number 2. On less trivial examples, it leads to multi-megabyte outputs very quickly. I have attached the compiler output for :whitespace and :advanced for the latest version of the CLJS compiler.

(def good {:a {:b {:c {:d {:e 1}}}}})

(def a identity)
(def b identity)

(def bad (apply (fn [x y] (x (y (x (y (x 2)))))) [a b]))


 Comments   
Comment by Francis Avila [ 12/Sep/14 4:48 PM ]

Is this more a non-native vs native function issue than an arity knowledge issue? Does whitespace compilation explode the same way if compiled with :static-fns true?

A solution might be to only emit the `x.call(null, arg)` form if we don't know whether `x` is a cljs function at compile time. However the arity dispatch wrapper function is not optimizable by v8 right now because of how it uses `arguments` so it would be a performance regression in the non-native case.

But another thing to consider is that google advance compilation output is intended to be compressed: test-whitespace.js and test-advanced.js both gzip to exactly 379 bytes. So maybe this is a non-problem?

Comment by Brendan Younger [ 12/Sep/14 10:46 PM ]

This is an unknown arity issue, not one of native versus non-native. The indirection introduced by the explicit apply function leads to the duplication of code. Compiling with :static-fns doesn't materially change anything with :whitespace or :advanced compilation.

Whether or not gzip is able to undo the code explosion, the browser still has to parse O(2^{nesting depth}) characters which means the parsing time is increased in proportion and, likely, the internal representation of the javascript is as well. For a mobile phone with constrained memory and processing power, this is a big deal. I would argue that any code duplication leading to an exponential increase in code size is a serious defect.

Comment by David Nolen [ 15/Sep/14 12:22 PM ]

Brendan do you have an actual example where this issue matters in practice? Thanks.

Comment by micha niskin [ 16/Sep/14 5:58 PM ]

David, I have seen this with Hoplon applications. Hoplon code tends to have the sort of deeply nested expressions you see there, because the DOM tree can be both quite broad and quite deep.

In a real application we found that when attempting to compile with advanced optimizations GClosure would generate gigabytes and gigabytes of JavaScript and never finish. Without optimizations the application would take five minutes or more to load in FireFox and Safari (Chrome, for some reason, was okay, which threw us off initially.)

In fact, I ended up writing a macro to perform a sort of SSA unfolding and flattening in order to work around the issue. The macro basically transforms this:

(a (b (c (d) (e) (f))))

into this:

(let [x1 (d)
      x2 (e)
      x3 (f)
      x4 (c x1 x2 x3)
      x5 (b x4)
      x6 (a x5)]
  x6)

The macro is here: https://github.com/tailrecursion/hoplon/blob/5.10.23/src/tailrecursion/hoplon.clj#L101-L105 and it's applied automatically to the contents of the <body> of the document here: https://github.com/tailrecursion/hoplon/blob/5.10.23/src/tailrecursion/hoplon.clj#L119-L124

When I wrap large expressions with this macro the problem is mitigated. It's obviously a filthy hack and not an optimal solution by any means, but we can't make real applications without it.





[CLJS-854] In Clojurescript IE8 cljs.reader can't read numbers Created: 10/Sep/14  Updated: 16/Sep/14

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Zubair Quraishi Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

In Chrome and Firefox:

(-> "{:s \"string\", :l 42, :k :keyword, :d 1.337}" (cljs.reader/read-string) (prn-str))

returns

{:s "string", :d 1.337, :k :keyword, :l 42}

:but in IE8 it returns:

{:s "string", :d 1.337, :k :keyword, :l 0}

See this thread:

https://github.com/JulianBirch/cljs-ajax/issues/46



 Comments   
Comment by Thomas Heller [ 10/Sep/14 7:37 AM ]

FWIW I just tested this on IE8 via the console: cljs.reader.read_string("42") => 0

Any Integer seems to fail.

Comment by Zubair Quraishi [ 10/Sep/14 7:58 AM ]

What do I do now with this JIRA issue to find out if it can be resolved? Do I need to assign it to someone, or am I supposed to fix it myself, I'm not really sure.

Comment by Nicola Mometto [ 10/Sep/14 12:12 PM ]

cljs.reader has no relation whatsoever with tools.reader, the title is misleading

Comment by Thomas Heller [ 10/Sep/14 12:30 PM ]

@Zubair: you could attempt a fix, I guess the error is somewhere in here:

https://github.com/clojure/clojurescript/blob/master/src/cljs/cljs/reader.cljs#L112

Too busy at the moment to take a closer look myself.

Comment by Francis Avila [ 10/Sep/14 1:09 PM ]

Actually I'd start with the regex that matches int. It has a non-capturing group in it which historically has had some cross-browser inconsistencies. See if that regex by itself matches int literals in IE or not.

Unfortunately I don't have any IEs to test.

Comment by Zubair Quraishi [ 10/Sep/14 2:47 PM ]

Ok, thanks, I could do that. Do you know if there is a guide which can show me how to do this. I saw the official guide here:

https://github.com/clojure/clojurescript/wiki/Windows-Setup

But it seems to use Clojure 1.3. Is this still the way to set it up on Windows?

Comment by Zubair Quraishi [ 10/Sep/14 2:48 PM ]

Or do I just set up and amend clojure reader on its own?

Comment by Zubair Quraishi [ 10/Sep/14 3:11 PM ]

OK, I'm looking in the reader now, so did you mean the line:

(def int-pattern (re-pattern "^([-+]?)(?0)|([1-9][0-9]*)|0[xX]([0-9A-Fa-f]+)|0([0-7]+)|([1-9][0-9]?)[rR]([0-9A-Za-z]+))(N)?$"))

Comment by Zubair Quraishi [ 16/Sep/14 2:51 AM ]

I am still looking at this and trying to find the error cases. Even 0 on its own fails in IE8. See:

http://connecttous.co/connecttous/connecttous.html

: and click on login to see how your browser parses different numbers:

I am still trying to track down the problem area, and currently looking around here:

(defn read-number
  [reader     initch]

  (loop [
         buffer   (gstring/StringBuffer. initch)
         ch       (read-char reader)
         ]
    
    (if (or (nil? ch) (whitespace? ch) (macros ch))
      
      (do
        (unread reader ch)
        (let [s (.toString buffer)]
          (or (match-number s)
              (reader-error reader "Invalid number format [" s "]"))))
      
      (recur (do (.append buffer ch) buffer) (read-char reader)))))




[CLJS-853] ^:metadata not working on fns (Cljs 0.0-2322) Created: 10/Sep/14  Updated: 10/Sep/14

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Peter Taoussanis Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: bug, metadata
Environment:

Clojure: 1.7.0-alpha2
ClojureScript: 0.0-2322
tools.reader: 0.8.8



 Description   

Hi there, just ran into some unexpected behaviour:

;;; Clojure
(meta ^:foo [])                      ; {:foo true}
(meta ^:foo (fn []))                  ; {:foo true}
(meta (with-meta []    {:foo true})) ; {:foo true}
(meta (with-meta (fn []) {:foo true})) ; {:foo true}

;;; ClojureScript 0.0-2322
(meta ^:foo [])                      ; {:foo true}
(meta ^:foo (fn []))                  ; nil         <--
(meta (with-meta []    {:foo true})) ; {:foo true}
(meta (with-meta (fn []) {:foo true})) ; {:foo true}

Also tried with a random set of older dependencies:
Clojure: 1.6.0
ClojureScript: 0.0-2277
tools.reader: 0.8.5

Same result, so this seems to have been around for a while.
Does this seem like it might be a Cljs or tools.reader bug?
Shall I file somewhere else?

Thanks a lot, cheers!



 Comments   
Comment by David Nolen [ 10/Sep/14 5:44 PM ]

Definitely a bug and a patch is most welcome.





Generated at Tue Sep 16 18:33:45 CDT 2014 using JIRA 4.4#649-r158309.