<< Back to previous view

[CLJS-1766] Set literals in REPL end up reified as ArrayMap backed PersistentHashSets. Created: 28/Aug/16  Updated: 28/Aug/16

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

Type: Defect Priority: Minor
Reporter: Thomas Mulvaney Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: repl


 Description   

Entering a set literal in the REPL with more than 8 elements should create a PHM backed set but instead it is array backed.

Example (in REPL):
cljs.user=> (type (.-hash-map #{1 2 3 4 5 6 7 8 9}))
cljs.core/PersistentArrayMap

This means operations such as `get` and `contains?` end up doing long scans and are slower than a user would expect.






[CLJS-1765] Empty iterator for some hash maps with a nil key Created: 27/Aug/16  Updated: 28/Aug/16

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

Type: Defect Priority: Major
Reporter: Mike Fikes Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-1765.patch    
Patch: Code and Test

 Description   

Two specific examples:

cljs.user=> (.hasNext (-iterator {:a 1 :b 2 :c 3 :d 4 :e 5 :f 6 :g 7 :h 8 nil 9}))
false
cljs.user=> (.hasNext (-iterator (hash-map nil 1))))
false

This can affect "user-level" code as follows:

cljs.user=> (sequence (map identity) {:a 1 :b 2 :c 3 :d 4 :e 5 :f 6 :g 7 :h 8 nil 9})
()
cljs.user=> (sequence (map identity) (hash-map nil 1))
()


 Comments   
Comment by Thomas Mulvaney [ 27/Aug/16 5:27 PM ]

I've attached a simple patch with some tests.

Comment by Mike Fikes [ 27/Aug/16 6:22 PM ]

Confirmed CA at http://clojure.org/community/contributors

Comment by Mike Fikes [ 27/Aug/16 6:35 PM ]

The first ^boolean in the form (or ^boolean (not seen) ^boolean (.hasNext root-tier)) could be omitted.

The patch passes unit tests for me (both JVM and self-host) and causes the correct output to appear for the examples

Comment by Thomas Mulvaney [ 28/Aug/16 4:05 AM ]

Thanks Mike Fikes, I omitted that boolean flag per your suggestion. Updated patch now attached.





[CLJS-1762] Bump Closure Compiler Created: 22/Aug/16  Updated: 27/Aug/16

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

Type: Task Priority: Minor
Reporter: António Nuno Monteiro Assignee: António Nuno Monteiro
Resolution: Unresolved Votes: 0
Labels: closure

Attachments: Text File CLJS-1762.patch    

 Description   

A new version of the Closure Compiler features an optimization that results in faster compilation, as well as some some changes to ES6-related features.



 Comments   
Comment by Juho Teperi [ 27/Aug/16 4:39 AM ]

Here is my version of the change.

Updating cljs.closure was quite straightforward, just removed all the checks and changed a few constructor calls.

I think I found all places to update the Closure version: pom.template.xml, project.clj and script/bootstrap. Bootstrap script also needed a change to account for changes closure-compiler filename.

  • ES6ModuleLoader has been replaced with ModuleLoader, it no longer
    needs to be passed to ProcessCommonJSModules etc. constructors
  • This version requires the version 20160822 and any checks needed to
    work with older versions have been removed
  • Closure-compiler jar name has been changed so lib/ and closure/
    directories should be removed before running bootstrap
Comment by Juho Teperi [ 27/Aug/16 4:55 AM ]

While tests pass with the patch, looks like module processing is broken. Tested with https://github.com/mneise/circle-color





[CLJS-1764] Double warning for undeclared Var Created: 26/Aug/16  Updated: 26/Aug/16

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

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

A regression occurred where an undeclared Var in a {{require}}d file causes two diagnostics:

$ more src/foo/core.cljs
(ns foo.core)

(def x 2)

abc
$ rm -rf .cljs_node_repl
$ java -cp cljs-1.9.227.jar:src clojure.main -m cljs.repl.node
ClojureScript Node.js REPL server listening on 52749
To quit, type: :cljs/quit
cljs.user=> *clojurescript-version*
"1.9.227"
cljs.user=> (require 'foo.core)
WARNING: Use of undeclared Var foo.core/abc at line 5 /Users/mfikes/Desktop/src/foo/core.cljs
WARNING: Use of undeclared Var foo.core/abc at line 5 /Users/mfikes/Desktop/src/foo/core.cljs
nil
cljs.user=> :cljs/quit
$ rm -rf .cljs_node_repl
$ java -cp cljs-1.9.211.jar:src clojure.main -m cljs.repl.node
ClojureScript Node.js REPL server listening on 56704
To quit, type: :cljs/quit
cljs.user=>  *clojurescript-version*
"1.9.211"
cljs.user=> (require 'foo.core)
WARNING: Use of undeclared Var foo.core/abc at line 5 /Users/mfikes/Desktop/src/foo/core.cljs
nil
cljs.user=> :cljs/quit





[CLJS-1763] Defining a var that clashes with `cljs.core` throws a compiler error instead of warning Created: 24/Aug/16  Updated: 24/Aug/16  Resolved: 24/Aug/16

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

Type: Defect Priority: Major
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1763.patch    
Patch: Code and Test

 Comments   
Comment by António Nuno Monteiro [ 24/Aug/16 10:43 AM ]

Attached patch with fix and test.

Comment by David Nolen [ 24/Aug/16 11:40 AM ]

fixed https://github.com/clojure/clojurescript/commit/7a06d008fadf56b11dba0f9e2ab97e61059f44fc





[CLJS-1761] Allow parallel Transit analysis cache writes Created: 19/Aug/16  Updated: 22/Aug/16

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

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: Mike Fikes
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-1761.patch    

 Description   

With CLJS-1759, Transit analysis cache writes are being serialized. This ticket asks that analysis be done to find the root cause, so we can fix it and removed the workaround in CLJS-1759.



 Comments   
Comment by Francis Avila [ 19/Aug/16 1:53 PM ]

I suspect these lines in transit are a race condition:

https://github.com/cognitect/transit-java/blob/3f359c434264f675460bc2662dde2a1b2d8e9559/src/main/java/com/cognitect/transit/impl/WriterFactory.java#L74-L76

newHandlerCache is a cognitect.transit.impl.Cache instance, which is just a normal LinkedHashMap with size 10.

In between the containsKey() call and the get() call a cached item may have been evicted, causing the JsonEmitter to have a null map handler, causing the "Not supported: class java.lang.Integer" exception to bubble out of the emit() call inside write().

I think the fix is to call newHandlerCache.get(customHandlers) by itself and check for null instead of looking up twice. (I am not a seasoned Java dev, so I am not sure if concurrent reads are safe here without synchronization.) The synchronized block should probably also monitor the cache itself instead of the factory class.

Comment by Francis Avila [ 19/Aug/16 1:55 PM ]

Also, all of this is just from reading the code and your stacktrace, i.e., it is purely hypothesis.

Comment by Francis Avila [ 19/Aug/16 2:19 PM ]

On the other hand, the transit writer is created repeatedly with the same handler map instance, so I don't know why the write handler cache would ever have more than one entry (its limit is 10).

Comment by Mike Fikes [ 19/Aug/16 10:38 PM ]

Francis has logged a ticket against transit-java https://github.com/cognitect/transit-java/issues/20 and I've done some further experimentation and recorded what I found in that ticket.

Comment by Francis Avila [ 22/Aug/16 7:04 AM ]

This patch uses write-handler-map and read-handler-map to completely pre-assemble the transit handler maps and sidestep all handler cache construction caching. (I think this is a good thing to do even without these transit problems.)

If there are indeed races related handler construction and caching, this patch should sidestep them all and allow fully parallel use of transit.





[CLJS-1634] Track bound dynamic variables to support binding in async mechanisms. Created: 26/Apr/16  Updated: 20/Aug/16

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

Type: Enhancement Priority: Minor
Reporter: Christian Weilbach Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: cljs, enhancement
Environment:

Any cljs version.



 Description   

The issue has been raised before:

While the reasoning behind the proposal is still valid, the original approach has made no progress due to the performance penalty. I have implemented a simplified approach with mutable JavaScript datastructures to minimize the performance impact. Because we are single-threaded we can use js assignment and don't need to port Clojure's binding frame. A small penalty is paid by the user of binding (see benchmark8) and a higher one by async mechanisms capturing and restoring the bindings (benchmark1-7):

https://gist.github.com/whilo/a8ef2cd3f0e033d3973880a2001be32a

I would provide patches to ClojureScript, if this looks like a worthwhile approach.



 Comments   
Comment by Antonin Hildebrand [ 30/Apr/16 6:05 AM ]

Just for record I commented on it here: https://gist.github.com/whilo/a8ef2cd3f0e033d3973880a2001be32a#gistcomment-1764489. Not sure if GitHub sends out notifications about new gist comments.

Comment by Christian Weilbach [ 30/Apr/16 6:18 AM ]

Thanks for pointing it out. David Nolen has also pointed out prototype chains to address this issue and now I see what he meant. I am not familiar enough with the internals of "this" in JavaScript, but one problem I see is that you need to distinguish dynamic vars at the call site. The advantage of using an object directly and capturing and restoring the frame explicitly when you enter and leave the code is that call sites are totally unaffected. The cost is only paid a little at the binding site and mostly in async libraries (bound-fn). But I might still need to look further into "this" . I have not got the gist comment from github.

Comment by Antonin Hildebrand [ 30/Apr/16 7:23 AM ]

Correct.

> you need to distinguish dynamic vars at the call site

I agree. My initial motivation was to solve a bit different problem without cooperation from library authors. I didn't want to modify ClojureScript behaviour and wanted to be just touching own code or doing trivial changes in library forks. Just wanted to share my thoughts about the implementation.

I have a feeling that solving this "async context" problem will be difficult. You will need async library authors to adapt their libraries. And users unaware of this will be running into issues anytime they step outside of bound-fn aware async libraries (for example using raw js interop). I believe Angular people solved this robustly in https://github.com/angular/zone.js. The implementation is quite scary monkey-patching, but if they were able to wrap all existing async calls at lowest level, maybe we could just build on top of their foundation and use zone.js as parallel mechanism for `binding`.

Comment by Christian Weilbach [ 02/May/16 4:58 PM ]

The angle I am coming from is roughly described here: https://github.com/fullcontact/full.monty/pull/9#issuecomment-131152058

I only found out at the very end when I had supervision of go-channels completely implemented, that the cljs binding was not behaving like the Clojure one. Arguments pro/contra binding in Clojure are also referenced. The zone monkey patching looks very heavy and prone to cause hairy bugs. It is noteworthy that Clojure does not embrace bindings, but keeps them always thread-local. So there you also have to use bound-fn or something similar whenever code is executed concurrently. core.async for instance uses the Clojure mechanism to push (capture) and pop (restore) bindings. I would like to have this in ClojureScript as well. I think one should not retain all bindings automatically, but rather allow the library author to handle dynamic bindings. I only track the supervisor binding for instance. For ClojureScript as for Clojure libraries and wrappers this should be fine. Pure JavaScript libraries usually have their own binding concepts like zone.js, right?

Tracking bindings is neither for free in Clojure nor in ClojureScript and it is an important design goal to embrace the host. In fact originally I tried to capture and restore all bindings. My benchmarks for tracking more dynamic vars (instead of just the currently active binding), were linearly more expansive than rebinding fewer selected vars and become prohibitive when you reach a few hundred.

Comment by Antonin Hildebrand [ 02/May/16 5:16 PM ]

I would be happy if your proposal went through. It would help my use-cases as well.

I'm going to explore zone.js when I get some spare time. I will try do write a wrapper library and implement an alternative mechanism to bindings using zone.js. I would like to provide this functionality as a library without a need to modify ClojureScript compiler or involvement from library authors.

Comment by Christian Weilbach [ 03/May/16 1:39 AM ]

Ok, I am curious how well this will work. Would this work with the state-machine transformation of core.async?

Comment by Antonin Hildebrand [ 03/May/16 3:25 AM ]

I believe so. Core.async state machine uses only setTimeout and goog.async.nextTick. We can teach zone.js to deal with nextTick by setting goog.async.nextTick.wrapCallback_ with zone wrapping. Also if user decided to use any async API in their go blocks it should work, because zone.js will carry proper zone binding over async boundaries.

Comment by Antonin Hildebrand [ 03/May/16 3:50 AM ]

I have opened this issue in zone.js: https://github.com/angular/zone.js/issues/342

Comment by Christian Weilbach [ 26/May/16 8:56 AM ]

Hey. Have you made any progress with implementing a small cljs demo with zone.js yet?

Comment by Antonin Hildebrand [ 27/May/16 5:57 AM ]

Hi Christian. No, unfortunately I didn't get to it.

Comment by Christian Weilbach [ 07/Aug/16 4:26 PM ]

Interestingly to implement the Common Lisp like condition system chouser presented here https://www.youtube.com/watch?v=zp0OEDcAro0 the dynamic binding working over async boundaries is also important.

Comment by Antonin Hildebrand [ 08/Aug/16 5:53 PM ]

Christian, please have a look at my implementation:
https://github.com/binaryage/cljs-zones

I have implemented the prototype trick as a library. It is just a gist of the idea, didn't spend time to make it robust yet and ES3-compatible. Re-binding frames should be as cheap as changing pointers (inside JS runtime).

Comment by Christian Weilbach [ 10/Aug/16 6:10 AM ]

Very nice work! I am checking it out atm. Nice that it is self-contained. (The Klipse version throws a goog.object not found error for me btw.)

Comment by Christian Weilbach [ 16/Aug/16 4:05 PM ]

I have updated the gist to incorporate cljs-zones in benchmark 8, 9 and 10. It is a bit faster in restoring dynamic bindings, but not much:

https://gist.github.com/whilo/a8ef2cd3f0e033d3973880a2001be32a

But there is a significant performance penalty on var access:

full.binding_test.benchmark10()
core.cljs:150 [], ((fn [] v1)), 10000 runs, 1 msecs
core.cljs:150 [], ((fn [] (zones/get v1))), 10000 runs, 4 msecs
null
full.binding_test.benchmark10()
core.cljs:150 [], ((fn [] v1)), 10000 runs, 2 msecs
core.cljs:150 [], ((fn [] (zones/get v1))), 10000 runs, 6 msecs
null
full.binding_test.benchmark10()
core.cljs:150 [], ((fn [] v1)), 10000 runs, 3 msecs
core.cljs:150 [], ((fn [] (zones/get v1))), 10000 runs, 4 msecs

Is this penalty addressable?

One other issue is that JavaScript prototype manipulations through "this" would interfere with the cljs binding mechanism, but this might be acceptable.

Comment by Antonin Hildebrand [ 16/Aug/16 4:17 PM ]

Thanks for measuring it. I didn't really get to benchmarking yet.

Did you run the benchmarks under :advanced optimizations?

zones/get currently emits a call to goog.object/get, I'm not sure if this gets inlined by closure compiler, but if not, we can probably improve it by generating raw js object access:
https://github.com/binaryage/cljs-zones/blob/fdfa1421c39d64c9c6b9efbff474b7677f908197/src/lib/zones/core.clj#L94

> One other issue is that JavaScript prototype manipulations through "this" would interfere with the cljs binding mechanism
Can you elaborate? I don't understand how it would interfere.

Comment by Christian Weilbach [ 17/Aug/16 4:10 AM ]

With advanced compilation I get:

full.binding_test.benchmark10()
client.js:278 [], ((fn [] v1)), 10000 runs, 1 msecs
client.js:278 [], ((fn [] (zones/get v1))), 10000 runs, 2 msecs
null
full.binding_test.benchmark10()
client.js:278 [], ((fn [] v1)), 10000 runs, 2 msecs
client.js:278 [], ((fn [] (zones/get v1))), 10000 runs, 3 msecs
null
full.binding_test.benchmark10()
client.js:278 [], ((fn [] v1)), 10000 runs, 3 msecs
client.js:278 [], ((fn [] (zones/get v1))), 10000 runs, 2 msecs
null
full.binding_test.benchmark10()
client.js:278 [], ((fn [] v1)), 10000 runs, 1 msecs
client.js:278 [], ((fn [] (zones/get v1))), 10000 runs, 2 msecs
null
full.binding_test.benchmark10()
client.js:278 [], ((fn [] v1)), 10000 runs, 1 msecs
client.js:278 [], ((fn [] (zones/get v1))), 10000 runs, 3 msecs
null
full.binding_test.benchmark10()
client.js:278 [], ((fn [] v1)), 10000 runs, 1 msecs
client.js:278 [], ((fn [] (zones/get v1))), 10000 runs, 1 msecs

But this is a very simple test. If you see a better way to benchmark it, go ahead . When the chains get prototype chains get longer, it might be more painful to walk the chain for each global lookup.

> Can you elaborate? I don't understand how it would interfere.
There has been a proposal somewhere to pass "this" in all cljs functions instead of null as first argument. Your explicit model of a zone is pretty safe I think, but if people would interefere with prototypes in JS, then this might break the "this" approach in subtle ways. Without passing "this" as a direct argument and doing it in a more JS way, you have to setup the zones all the time instead of using the scope chain, which might also contribute to the managing cost. So the "this" approach might help with performance, but I am not sure. You still have the chain overhead. But I am no JS expert, you know much better than me what you are doing.

Comment by Christian Weilbach [ 20/Aug/16 5:37 AM ]

My last comment was a little bit confusing. I only see the problem with an impact on dereferencing vars. In the benchmark above the prototype chain is very short (v1 can be directly resolved) and already has a significant penalty, but I think the penalty gets even worse for not rebound root vars if you have a higher nesting level for the binding and the chain gets longer. Can you address this somehow?





[CLJS-1759] Errors writing transit analysis cache if parallel build Created: 19/Aug/16  Updated: 19/Aug/16  Resolved: 19/Aug/16

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

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

Attachments: Text File CLJS-1759.patch    
Patch: Code

 Description   
  1. It fails only if parallel builds enabled
  2. When so, it fails, maybe 30 to 50% of the time
  3. If I put a coarse-grained mutex around the write calls, it succeeds
  4. It appears to occur under heave write load 3 concurrent writes

Here is an example of one of the failures:

Exception in thread "main" clojure.lang.ExceptionInfo: failed compiling file:out/cljs/source_map/base64.cljs {:file #object[java.io.File 0x125a8ab6 "out/cljs/source_map/base64.cljs"]}, compiling:(/Users/mfikes/Projects/planck/planck-cljs/script/build.clj:16:1)
        at clojure.lang.Compiler.load(Compiler.java:7391)
        at clojure.lang.Compiler.loadFile(Compiler.java:7317)
        at clojure.main$load_script.invokeStatic(main.clj:275)
        at clojure.main$script_opt.invokeStatic(main.clj:335)
        at clojure.main$script_opt.invoke(main.clj:330)
        at clojure.main$main.invokeStatic(main.clj:421)
        at clojure.main$main.doInvoke(main.clj:384)
        at clojure.lang.RestFn.invoke(RestFn.java:408)
        at clojure.lang.Var.invoke(Var.java:379)
        at clojure.lang.AFn.applyToHelper(AFn.java:154)
        at clojure.lang.Var.applyTo(Var.java:700)
        at clojure.main.main(main.java:37)
Caused by: clojure.lang.ExceptionInfo: failed compiling file:out/cljs/source_map/base64.cljs {:file #object[java.io.File 0x125a8ab6 "out/cljs/source_map/base64.cljs"]}
        at clojure.core$ex_info.invokeStatic(core.clj:4617)
        at clojure.core$ex_info.invoke(core.clj:4617)
        at cljs.compiler$compile_file$fn__3456.invoke(compiler.cljc:1386)
        at cljs.compiler$compile_file.invokeStatic(compiler.cljc:1352)
        at cljs.compiler$compile_file.invoke(compiler.cljc:1332)
        at cljs.closure$compile_file.invokeStatic(closure.clj:474)
        at cljs.closure$compile_file.invoke(closure.clj:465)
        at cljs.closure$eval5203$fn__5204.invoke(closure.clj:541)
        at cljs.closure$eval5139$fn__5140$G__5128__5147.invoke(closure.clj:431)
        at cljs.closure$compile_from_jar.invokeStatic(closure.clj:523)
        at cljs.closure$compile_from_jar.invoke(closure.clj:511)
        at cljs.closure$eval5209$fn__5210.invoke(closure.clj:551)
        at cljs.closure$eval5139$fn__5140$G__5128__5147.invoke(closure.clj:431)
        at cljs.closure$compile_task$fn__5294.invoke(closure.clj:821)
        at cljs.closure$compile_task.invokeStatic(closure.clj:819)
        at cljs.closure$compile_task.invoke(closure.clj:812)
        at cljs.closure$parallel_compile_sources$fn__5300.invoke(closure.clj:848)
        at clojure.lang.AFn.applyToHelper(AFn.java:152)
        at clojure.lang.AFn.applyTo(AFn.java:144)
        at clojure.core$apply.invokeStatic(core.clj:646)
        at clojure.core$with_bindings_STAR_.invokeStatic(core.clj:1881)
        at clojure.core$with_bindings_STAR_.doInvoke(core.clj:1881)
        at clojure.lang.RestFn.invoke(RestFn.java:425)
        at clojure.lang.AFn.applyToHelper(AFn.java:156)
        at clojure.lang.RestFn.applyTo(RestFn.java:132)
        at clojure.core$apply.invokeStatic(core.clj:650)
        at clojure.core$bound_fn_STAR_$fn__4671.doInvoke(core.clj:1911)
        at clojure.lang.RestFn.invoke(RestFn.java:397)
        at clojure.lang.AFn.run(AFn.java:22)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
        at java.lang.Thread.run(Thread.java:745)
Caused by: java.lang.RuntimeException: java.lang.Exception: Not supported: class java.lang.Integer
        at com.cognitect.transit.impl.WriterFactory$1.write(WriterFactory.java:129)
        at cognitect.transit$write.invokeStatic(transit.clj:149)
        at cognitect.transit$write.invoke(transit.clj:146)
        at cljs.analyzer$write_analysis_cache.invokeStatic(analyzer.cljc:3127)
        at cljs.analyzer$write_analysis_cache.invoke(analyzer.cljc:3114)
        at cljs.compiler$emit_source.invokeStatic(compiler.cljc:1283)
        at cljs.compiler$emit_source.invoke(compiler.cljc:1232)
        at cljs.compiler$compile_file_STAR_$fn__3433.invoke(compiler.cljc:1304)
        at cljs.compiler$with_core_cljs.invokeStatic(compiler.cljc:1154)
        at cljs.compiler$with_core_cljs.invoke(compiler.cljc:1145)
        at cljs.compiler$compile_file_STAR_.invokeStatic(compiler.cljc:1293)
        at cljs.compiler$compile_file_STAR_.invoke(compiler.cljc:1289)
        at cljs.compiler$compile_file$fn__3456.invoke(compiler.cljc:1374)
        ... 29 more


 Comments   
Comment by Mike Fikes [ 19/Aug/16 9:48 AM ]

The attached patch introduces a coarse-grained mutex as indicated in item (3) of the description, providing a work-around for the issue.

Comment by Mike Fikes [ 19/Aug/16 10:43 AM ]

A follow up enhancement ticket that would potentially lead to removing the (presumed) workaround in the patch: CLJS-1761

Comment by David Nolen [ 19/Aug/16 10:44 AM ]

fixed https://github.com/clojure/clojurescript/commit/002708e530b6b3449151d3d077883beeadb92f94





[CLJS-1760] Self-host: test-cljs-1757 failing in test-self-parity Created: 19/Aug/16  Updated: 19/Aug/16  Resolved: 19/Aug/16

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

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: bootstrap, spec

Attachments: Text File CLJS-1760.patch    
Patch: Code

 Description   
$ script/test-self-parity
Testing with Node
WARNING: baz is a single segment namespace at line 1
WARNING: Use of undeclared Var cljs.spec$macros/gen at line 77

Testing cljs.core-test

Testing cljs.reader-test

Testing clojure.string-test

Testing clojure.data-test

Testing cljs.letfn-test

Testing cljs.reducers-test

Testing cljs.binding-test

Testing cljs.macro-test

Testing cljs.top-level

Testing cljs.ns-test.foo

Testing foo.ns-shadow-test

Testing cljs.import-test

Testing cljs.spec-test

ERROR in (test-cljs-1757) (TypeError:NaN:NaN)
expected: (s/exercise-fn (quote cljs.spec-test/cljs-1757-x))
  actual: #object[TypeError TypeError: Cannot read property 'call' of undefined]

Testing cljs.clojure-alias-test

Ran 215 tests containing 17428 assertions.
0 failures, 1 errors.


 Comments   
Comment by António Nuno Monteiro [ 19/Aug/16 10:25 AM ]

Attached path with fix.

Comment by Mike Fikes [ 19/Aug/16 10:39 AM ]

LGTM. It is the “correct” fix; I missed when putting together CLJS-1720. All tests pass for me with António's patch.

Comment by David Nolen [ 19/Aug/16 10:42 AM ]

fixed https://github.com/clojure/clojurescript/commit/86a83d720beb44deb5d55d7d9c0bc2d5174816a3





[CLJS-1751] port fix lost type hints in map destructuring Created: 15/Aug/16  Updated: 18/Aug/16  Resolved: 18/Aug/16

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

Type: Enhancement Priority: Minor
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1751.patch    
Patch: Code

 Comments   
Comment by António Nuno Monteiro [ 15/Aug/16 3:24 PM ]

Attached patch with fix.

Comment by David Nolen [ 18/Aug/16 4:08 PM ]

fixed https://github.com/clojure/clojurescript/commit/b6c48c700a788d2b19513170e3231e043afe9752





[CLJS-1758] QuickStart guide fails at browser REPL step with "TypeError: parentElm is null" Created: 18/Aug/16  Updated: 18/Aug/16  Resolved: 18/Aug/16

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

Type: Defect Priority: Minor
Reporter: Tim McCormack Assignee: Unassigned
Resolution: Not Reproducible Votes: 0
Labels: None


 Description   

When I follow http://clojurescript.org/guides/quick-start up to the browser REPL step, I get an error in the browser console and the REPL never connects. The browser error is TypeError: parentElm is null on line 482 of crosspagechannel.js: parentElm.appendChild(iframeElm);.

I have put up a self-contained example to demonstrate the bug: https://github.com/timmc/sscce-CLJS-1758

  • I am using this command to run the REPL: rlwrap java -cp cljs-1.9.216.jar:src clojure.main repl.clj
  • Refreshing the browser does not help.
  • Using Chromium 52.0.2743.116 instead of Firefox ESR 45.3.0 does not help.
$ java -version
java version "1.8.0_31"
Java(TM) SE Runtime Environment (build 1.8.0_31-b13)
Java HotSpot(TM) 64-Bit Server VM (build 25.31-b07, mixed mode)

$ uname -a
Linux bc-timmc 3.16.0-4-amd64 #1 SMP Debian 3.16.7-ckt25-2+deb8u3 (2016-07-02) x86_64 GNU/Linux
(Debian GNU/Linux 8.5)


 Comments   
Comment by Tim McCormack [ 18/Aug/16 11:27 AM ]

CLJS 1.9.89 fails the same way.

Comment by António Nuno Monteiro [ 18/Aug/16 11:51 AM ]

Not a bug. Your index.html file needs to at least have a `<body>` tag.
All html examples in the quickstart provide valid HTML, you just chose not to use it.

Comment by Tim McCormack [ 18/Aug/16 12:41 PM ]

Ah, of course! I compared the other files with the demo, but not that one!

I could have sworn the body element was always created automatically in the DOM whether or not it was declared. TIL.

(And confirmed, it works now.)

ETA: Not sure which resolution to pick, so leaving open.





[CLJS-1756] Add test.check JAR to the bootstrap script Created: 17/Aug/16  Updated: 18/Aug/16  Resolved: 18/Aug/16

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

Type: Enhancement Priority: Minor
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1756.patch    
Patch: Code

 Comments   
Comment by António Nuno Monteiro [ 17/Aug/16 10:03 AM ]

Added patch with fix.

Comment by David Nolen [ 18/Aug/16 3:59 PM ]

fixed https://github.com/clojure/clojurescript/commit/b8a14ee78394a866c592b7daa9909137ab43528c





[CLJS-1757] cljs.spec/exercise-fn doesn't work when passed a quoted symbol Created: 17/Aug/16  Updated: 17/Aug/16  Resolved: 17/Aug/16

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

Type: Defect Priority: Major
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: spec

Attachments: Text File CLJS-1757.patch    
Patch: Code and Test

 Description   
cljs.user=> (require '[clojure.spec :as s])
nil
cljs.user=> (defn a [b] 2)
#'cljs.user/a
cljs.user=> (s/fdef a :args (s/cat ::first number?) :ret #(= % 2))
cljs.user/a
cljs.user=> (s/exercise-fn `a)
clojure.lang.ExceptionInfo: Assert failed: (symbol? sym) at line 1 <cljs repl> {:file "<cljs repl>", :line 1, :column 1, :root-source-info {:source-type :fragment, :source-form (s/exercise-fn (quote cljs.user/a))}, :tag :cljs/analysis-error}


 Comments   
Comment by António Nuno Monteiro [ 17/Aug/16 12:31 PM ]

Attached patch with fix and test.

Comment by David Nolen [ 17/Aug/16 7:32 PM ]

fixed https://github.com/clojure/clojurescript/commit/74db88011dd6dd64f55521f074f0315231be0e12





[CLJS-1754] Add boolean? generator Created: 16/Aug/16  Updated: 17/Aug/16  Resolved: 17/Aug/16

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

Type: Defect Priority: Major
Reporter: Matt Burbidge Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: cljs, spec

Attachments: Text File CLJS-1754.patch    
Patch: Code and Test

 Description   

In clojure I can do
`(gen/generate (s/gen int?)) ;; => 1094`
or
`(gen/generate (s/gen boolean?)) ;; => false`.

In clojurescript I can do
`(gen/generate (s/gen int?)) ;; => -308`.
But in clojurescript I can't do
`(gen/generate (s/gen boolean?)) ;; => Error: Unable to construct gen at: [] for: function cljs$core$boolean_QMARK_{return (x === true) || (x === false);}].`

As far as I can tell it's because there is no boolean generator.



 Comments   
Comment by António Nuno Monteiro [ 17/Aug/16 10:04 AM ]

Added patch with fix and test.

Note that running self parity tests now depends on CLJS-1756 being in place.

Comment by David Nolen [ 17/Aug/16 10:40 AM ]

fixed https://github.com/clojure/clojurescript/commit/2a7454837244cf7de3dfed1e48f46f86c33a1809





[CLJS-1755] Support sourcesContent in source maps Created: 16/Aug/16  Updated: 16/Aug/16

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

Type: Enhancement Priority: Minor
Reporter: Daniel Compton Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: sourcemap


 Description   

This issue adds sourcesContent support for source maps: https://github.com/google/closure-compiler/issues/1890. This means that your source maps can include your source as well in one bundled file. This makes handling sourcemaps much easier for things like error tracking services. It could also simplify config for source mapping as everything is included in the source map and you don't need to specify relative paths, e.t.c.

This will need to wait for the next release of the Closure Compiler.






[CLJS-1536] REPL def symbol init collision Created: 03/Jan/16  Updated: 16/Aug/16

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

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-1536.patch    
Patch: Code and Test

 Description   

In a REPL, if you try def where the init is a local matching the symbol being defined, then analysis fails.

(let [x 1]
  (def x x))

This can be verified in script/noderepljs and you can see it is some bad interaction with REPL var emission because if :def-emits-var false is added to the script, things work.



 Comments   
Comment by Txus Bach [ 04/Jan/16 3:01 PM ]

Confirmed that evaluating this breaks:

(require 'clojure.tools.reader
         'cljs.analyzer
         'cljs.compiler)

(let [env '{:ns {:name cljs.user}
            :def-emits-var true
            :locals {}}]
  (->> "(let [x 1] (def x 3))"
       clojure.tools.reader/read-string
       (cljs.analyzer/analyze env)
       cljs.compiler/emit-str))

(When emitting, not when just analyzing.)

Removing `:def-emits-var true` makes the bug disappear.

Looking into how to fix it – any clues are welcome, it's my first time around the codebase

Comment by Txus Bach [ 04/Jan/16 3:45 PM ]

Seems that var-ast is returning nil, because resolve-var doesn't return a map with an :ns key. Not sure if this code should work, but it returns nil:

(let [env {:ns {:name 'cljs.user},
 :def-emits-var true,
 :locals
 {'x
  {:init
   {:op :constant,
    :env
    {:ns {:name 'cljs.user},
     :def-emits-var true,
     :locals {},
     :line nil,
     :column nil,
     :context :expr},
    :form 1,
    :tag 'number},
   :name 'x,
   :binding-form? true,
   :op :var,
   :env {:line nil, :column nil},
   :column nil,
   :line nil,
   :info {:name 'x, :shadow nil},
   :tag 'number,
   :shadow nil,
   :local true}}}]
  (ana/resolve-var env 'x))

Will continue tomorrow. It's so much fun!

Comment by António Nuno Monteiro [ 16/Aug/16 4:29 PM ]

Attached patch with fix and test.

Comment by Mike Fikes [ 16/Aug/16 6:27 PM ]

I tried António's patch with various expressions in the REPL and couldn't find a way to break it.





[CLJS-1346] Support require outside of ns Created: 18/Jul/15  Updated: 16/Aug/16

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

Type: Enhancement Priority: Major
Reporter: Jonathan Boston Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None


 Description   

Makes http://dev.clojure.org/jira/browse/CLJS-1277 useful.



 Comments   
Comment by António Nuno Monteiro [ 16/Aug/16 3:40 PM ]

My initial thoughts to tackle this one would be:

  • add support for the `require` special form (along with `require-macros`, `use`, `use-macros`, `refer-clojure` and `import` for completeness) in the analyzer.
  • have `cljs.analyzer/parse-ns` recognize more than the `:ns` AST :op, merging `require`s/`uses` etc into NS requires. Use the 'cljs.user NS by default if no NS form found.
  • this means that `cljs.closure/find-cljs-dependencies` can get at the `require`d dependencies in the build pipeline.

Pending questions:

  • How to disallow `require` etc. except at the top level?
  • Possibly only even allow it strictly after the NS form, or one or the other?




[CLJS-1752] stest/instrument never throws or :ret and :fn Created: 16/Aug/16  Updated: 16/Aug/16  Resolved: 16/Aug/16

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

Type: Defect Priority: Minor
Reporter: Jan Hein Hoogstad Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: cljs, spec


 Description   

When I replace the spec for the following function:

(defn create [query]
  (with-meta (map->Query query) {:spec ::specs/query}))

from:

(spec/fdef create
           :args (spec/cat :query ::specs/query)
           :ret ::specs/query
           :fn #(spec/valid? ::specs/meta (-> %1 :ret :meta)))

(stest/instrument `create)

to

(spec/fdef create
           :args (spec/cat :query ::specs/query)
           :ret string?
           :fn #(spec/valid? ::specs/meta (-> %1 :ret :meta)))

(stest/instrument `create)

it does not throw. As a matter of fact, I didn't get it to throw on any value that I give the :ret and :fn function. Removing the record with a plain map did not make a difference...



 Comments   
Comment by David Nolen [ 16/Aug/16 10:24 AM ]

This is by design.

Comment by Jan Hein Hoogstad [ 16/Aug/16 10:34 AM ]

Thanks for the quick reply, but it is not clear to me why? Can you maybe point me to an explanation?

Cheer!

Comment by Jan Hein Hoogstad [ 16/Aug/16 11:06 AM ]

Never mind, found it. Hadn't noticed it before...





[CLJS-1753] cljs.pprint does not print default tagged literals/elements Created: 16/Aug/16  Updated: 16/Aug/16

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

Type: Defect Priority: Major
Reporter: Miroslav Kubicek Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: bug
Environment:

Win 7 64bit



 Description   

Hi there!
I am having troubles making the cljs pretty print (cljs.pprint/pprint) behave the same way as the regular cljs or clj print function when it goes down to tagged elements that are by default used to denote custom records.

See example below - pr, pr-str, print and print-str functions all use the default approach toward creating (edn-like) tagged element for MyRecord and all produce the same result:
#cljs.user.MyRecord{:value "a"}

On the other hand pprint just ignores the record's tag and simply just traverses/prints it as a map:
{:value "a"}

Is there some setting and/or parameter in cljs.pprint namespace I am missing? I looked briefly at the code, but it seems it uses print-str by default - so maybe it just traverses the graph depth-first and does not check for every node's type? At this stage this seems like a bug to me as the expected behavior of the pprint function is that it would behave the same way print and other core functions do.

THIS WORKS:

cljs.user=> (defrecord MyRecord [value])
cljs.user/MyRecord
cljs.user=> (pr (MyRecord. "a"))
#cljs.user.MyRecord{:value "a"}
nil
cljs.user=> (pr-str (MyRecord. "a"))
"#cljs.user.MyRecord{:value \"a\"}"
cljs.user=> (print (MyRecord. "a"))
#cljs.user.MyRecord{:value a}
nil
cljs.user=> (print-str (MyRecord. "a"))
"#cljs.user.MyRecord{:value a}"

BUT THIS DOESN'T:

cljs.user=> (cljs.pprint/pprint (MyRecord. "a"))
{:value "a"}
("{:value \"a\"}\n")
cljs.user=> (with-out-str (cljs.pprint/pprint (MyRecord. "a")))
"{:value \"a\"}\n"

According to github the head revision of the cljs.pprint namespace has not changed since 1.7.28 so I'd assume all versions up to the current one are affected.

Thanks for help!






[CLJS-1750] With `:language-out :ecmascript5-strict` goog.global is undefined Created: 15/Aug/16  Updated: 15/Aug/16

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

Type: Defect Priority: Minor
Reporter: Dusan Maliarik Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: closure


 Description   

When using `:language-out :ecmascript5-strict` the compiled code ends up in a wrapper where `this` resolves to `undefined`. This break goog/base.js where `this` is assigned to `goog.global`.



 Comments   
Comment by David Nolen [ 15/Aug/16 2:34 PM ]

This doesn't seem like a bug but rather a faithful interpretation of strict mode - see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Strict_mode.





[CLJS-1749] Missing `cljs.spec.impl.gen/double*` Created: 15/Aug/16  Updated: 15/Aug/16  Resolved: 15/Aug/16

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

Type: Defect Priority: Minor
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1749.patch    
Patch: Code

 Comments   
Comment by António Nuno Monteiro [ 15/Aug/16 12:05 PM ]

Attached patch with fix.

Comment by David Nolen [ 15/Aug/16 12:53 PM ]

fixed https://github.com/clojure/clojurescript/commit/cd144c205ca08759869d2c32157bcc6a60173d0c





[CLJS-1747] Port `clojure.spec/assert` over to ClojureScript Created: 15/Aug/16  Updated: 15/Aug/16  Resolved: 15/Aug/16

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

Type: Defect Priority: Major
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1747.patch    
Patch: Code and Test

 Comments   
Comment by António Nuno Monteiro [ 15/Aug/16 11:28 AM ]

Attached patch with fix and test.

Comment by David Nolen [ 15/Aug/16 12:52 PM ]

fixed https://github.com/clojure/clojurescript/commit/862950da92581b60abc3bc615305461a7e1c1bfb





[CLJS-1748] Nth doesn't throw error for strings or arrays Created: 15/Aug/16  Updated: 15/Aug/16

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

Type: Defect Priority: Minor
Reporter: Thomas Mulvaney Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-1748.patch    
Patch: Code and Test

 Description   

Calling nth on a string or array with an out of bounds index should throw an error but returns an empty string or nil.

(nth "012" 3) -> ""
(nth (array 0 1 2) 3) nil

Calling nth on a string or array with a negative index and a not-found value returns an empty string or nil.

(nth "012" -1 :not-found) -> ""
(nth (array 0 1 2) -1 :not-found) nil






[CLJS-1746] Log the result of loading a dependency Created: 14/Aug/16  Updated: 15/Aug/16  Resolved: 15/Aug/16

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

Type: Enhancement Priority: Trivial
Reporter: Andrea Richiardi Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1746.patch    

 Description   

It would be great to add a (debug-prn "Loading result: " res) in load-deps so that we see in the logs why a dependency failed to load.
It is a one line patch and I can take care of it.



 Comments   
Comment by Andrea Richiardi [ 14/Aug/16 9:16 PM ]

Patch attached

Comment by David Nolen [ 15/Aug/16 7:41 AM ]

fixed https://github.com/clojure/clojurescript/commit/aaf18ec17700ea81f48a2429fcaa74cc7e507cb2





[CLJS-1657] Self-host: Implicit macro loading with alias Created: 01/Jun/16  Updated: 14/Aug/16  Resolved: 14/Aug/16

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

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 1
Labels: bootstrap

Attachments: Text File CLJS-1657.patch    
Patch: Code and Test

 Description   

If a namespace relies on implicit macro loading (described here https://github.com/clojure/clojurescript/wiki/Differences-from-Clojure#namespaces), and an alias is used, it is possible for aliased symbol resolution to fail.

This can be reproduced by adding a 10th case in self-host.test/test-load-and-invoke-macros covering this situation:

(let [st (cljs/empty-state)]
        ;; Rely on implicit macro loading (ns loads its own macros), with an alias
        (cljs/eval-str st
          "(ns cljs.user (:require [foo.core :as foo]))"
          nil
          {:eval node-eval
           :load (fn [{:keys [macros]} cb]
                   (if macros
                     (cb {:lang :clj :source "(ns foo.core) (defmacro add [a b] `(+ ~a ~b))"})
                     (cb {:lang :clj :source "(ns foo.core (:require-macros foo.core))"})))}
          (fn [{:keys [value error]}]
            (is (nil? error))
            (cljs/eval-str st
              "(foo/add 300 500)"
              nil
              {:eval    node-eval
               :context :expr}
              (fn [{:keys [error value]}]
                (is (nil? error))
                (is (= 800 value))
                (inc! l))))))

This will result in:

Testing self-host.test

FAIL in (test-load-and-invoke-macros) (at .../clojurescript/builds/out-self/core-self-test.js:11545:454)
expected: (nil? error)
  actual: (not (nil? #error {:message "ERROR", :data {:tag :cljs/analysis-error}, :cause #object[TypeError TypeError: Cannot read property 'call' of undefined]}))

FAIL in (test-load-and-invoke-macros) (at .../clojurescript/builds/out-self/core-self-test.js:11548:49)
expected: (= 800 value)
  actual: (not (= 800 nil))


 Comments   
Comment by Mike Fikes [ 01/Jun/16 7:48 AM ]

Analysis: This is because, this bit of code https://github.com/clojure/clojurescript/blob/19510523ad9de3f16832d287bae86f701e8b4263/src/main/clojure/cljs/analyzer.cljc#L1817-L1820
has a branch that only works in non-bootstrap ClojureScript.

You can also work around the issue (or gain a better understanding) in several ways (if you can control the affected loading namespace—this won't work if this affects code down in a library you are loading):

1. You can add :include-macros true
2. You can load the self-macro-loading namespace twice. (For example, if at the REPL, you can (require [foo.core :as foo]) twice.) This causes the analysis state to be set up so that on the second require, it is taken care of in the first clause of the or in the linked code above.
3. You can even assoc-in enough state prior to the load so that you are effectively doing (2).

In all of these cases, when it fails, vs. when it succeeds, you can see a difference in the :require-macros key in the analysis map of the loading namespace ('cljs.user, for example). When it fails, you will see that this key is empty and when it succeeds, you will see that the key is populated, and in particular, with the foo alias.

In the case where you don't use an alias, implicit macro loading will fail in a way that won't be visible: The :require-macros key will not be set up as described above, but qualified symbol resolution will still succeed (foo.core/add in the example will resolve to foo.core$macros/add), probably simply owing to the way resolution is performed.

Comment by Mike Fikes [ 14/Aug/16 1:48 PM ]

As António Monteiro points out, this is no longer reproducible. My recommendation would be to add the additional unit test in the description and to also remove the workaround on this line

https://github.com/clojure/clojurescript/blob/756fa9bb196a97e0ae40fd644da5e492e0336c1c/src/test/self/self_parity/test.cljs#L233-L237

(and the two calls to it below), and ultimately with a patch to lock down these unit tests, resolve this as fixed.

Comment by António Nuno Monteiro [ 14/Aug/16 5:22 PM ]

As per Mike Fikes's feedback, attached a patch that adds the proposed unit test and removes the workaround for the issue from self parity tests.

Comment by David Nolen [ 14/Aug/16 7:05 PM ]

fixed https://github.com/clojure/clojurescript/commit/bc7f1d4c9602770e7fdcbcb2d6e39f36ab5f46a1





[CLJS-1544] cljs.test REPL reload support Created: 13/Jan/16  Updated: 14/Aug/16

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

Type: Defect Priority: Minor
Reporter: David Nolen Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

When adding a test to a test ns that uses cljs.test and re-loading (via require + :reload) that namespace in the REPL after saving the file - invoking run-tests does not include the newly added test.






[CLJS-1742] Add docstring for new refer-clojure REPL special Created: 12/Aug/16  Updated: 14/Aug/16  Resolved: 14/Aug/16

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

Type: Enhancement Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: docstring

Attachments: Text File CLJS-1742-2.patch     Text File CLJS-1742.patch    
Patch: Code

 Description   

Add a docstring for the new REPL special refer-clojure introduced with CLJS-1730.



 Comments   
Comment by António Nuno Monteiro [ 14/Aug/16 1:58 PM ]

Attached patch with fix.

Comment by António Nuno Monteiro [ 14/Aug/16 2:09 PM ]

Attached revised CLJS-1742-2.patch which drops reference to `inclusion` (used with Clojure's `:only` filter, not supported in ClojureScript)

Comment by David Nolen [ 14/Aug/16 2:36 PM ]

fixed https://github.com/clojure/clojurescript/commit/63e0d7380ee5c45d74a93613aafe982ee600e990





[CLJS-1745] refer-clojure doesn't pull in previously excluded vars Created: 14/Aug/16  Updated: 14/Aug/16

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

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Observe this behavior in Clojure:

user=> (ns foo.core (:refer-clojure :exclude [juxt partial]))
nil
foo.core=> juxt
CompilerException java.lang.RuntimeException: Unable to resolve symbol: juxt in this context, compiling:(NO_SOURCE_PATH:0:0)
foo.core=> partial
CompilerException java.lang.RuntimeException: Unable to resolve symbol: partial in this context, compiling:(NO_SOURCE_PATH:0:0)
foo.core=> (refer-clojure :exclude [])
nil
foo.core=> juxt
#object[clojure.core$juxt 0x2f62ea70 "clojure.core$juxt@2f62ea70"]
foo.core=> partial
#object[clojure.core$partial 0x38aa816f "clojure.core$partial@38aa816f"]

Compare with ClojureScript:

$ script/noderepljs
ClojureScript Node.js REPL server listening on 49402
To quit, type: :cljs/quit
cljs.user=> (ns foo.core (:refer-clojure :exclude [juxt partial]))
nil
foo.core=> juxt
nil
foo.core=> partial
nil
foo.core=> (refer-clojure :exclude [])
nil
foo.core=> juxt
nil
foo.core=> partial
nil





[CLJS-1623] using `with-redefs` to add meta to a function via `with-meta` fails when using advanced compilation Created: 12/Apr/16  Updated: 14/Aug/16  Resolved: 14/Aug/16

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

Type: Defect Priority: Minor
Reporter: Justin Cowperthwaite Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None


 Description   

Given the following:

(ns test.cljs)

(enable-console-print!)

(defn print-foobar [] (println "foobar"))

(defn test-redefs-meta []
  (print-foobar)
  (with-redefs [print-foobar (with-meta print-foobar {:foo "bar"})]
    (print-foobar)))

(test-reefs-meta)

running with `:optimizations :none`, it correctly prints:

foobar
footer

However, running with `:optimizations :advanced`, it prints:

foobar
main.js:232 Uncaught TypeError: te is not a function(anonymous function) @ main.js:232

Reproduced on r1.8.40 and current master (29eb8e0).



 Comments   
Comment by Justin Cowperthwaite [ 14/Apr/16 5:42 PM ]

it seems that the actual issue is with having :static-fns true (as is default under :optimizations :advanced)

Comment by David Nolen [ 06/May/16 2:01 PM ]

This issue needs to provide some hypothesis of what is going wrong.

Comment by António Nuno Monteiro [ 14/Aug/16 12:35 PM ]

I don't think this is a bug. If you want to use `with-redefs` with statics-fns, the var needs to be marked `:dynamic`.

The following revised example works:

(defn ^:dynamic print-foobar [] (println "foobar"))

(defn test-redefs-meta []
  (print-foobar)
  (with-redefs [print-foobar (with-meta print-foobar {:foo "bar"})]
    (print-foobar)))

(test-redefs-meta)
Comment by David Nolen [ 14/Aug/16 1:47 PM ]

No interest in making this work. If you want something to be redefinable with `with-redefs` in production you need to mark those vars ^:dynamic.





[CLJS-1274] Allow assignment to namespace-qualified names in current namespace Created: 17/May/15  Updated: 14/Aug/16  Resolved: 14/Aug/16

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: 0.0-3211
Fix Version/s: Next

Type: Enhancement Priority: Minor
Reporter: lvh Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1274-2.patch     Text File CLJS-1274-3.patch     Text File CLJS-1274.patch    
Patch: Code and Test

 Description   

In Clojure, you can def to a namespace-qualified name as long as it's in the current namespace. You can't do that in Clojurescript. This makes writing some macros that def to a local name a bit more annoying, since the syntax-quote will automatically namespace-qualify any symbols, so you have to write the somewhat unsightly and not super obvious ~'sym.



 Comments   
Comment by António Nuno Monteiro [ 14/Aug/16 10:25 AM ]

Attached patch with fix and test.

Comment by António Nuno Monteiro [ 14/Aug/16 11:23 AM ]

Attached a revised patch with a better error message as per Mike Fikes's feedback.

Comment by António Nuno Monteiro [ 14/Aug/16 11:33 AM ]

Patch CLJS-1274-3.patch is the one that should be considered.

Comment by Mike Fikes [ 14/Aug/16 11:47 AM ]

Confirmed that all tests pass for me locally and that this fundamentally works downstream in self-host, emitting a helpful diagnostic related to the pseudo-namespace used for macro namespaces:

cljs.user=> (def cljs.user/bar 3)
#'cljs.user/bar
cljs.user=> bar
3
cljs.user=> (require-macros 'foo.core)
nil
cljs.user=> (source foo.core/deffoo)
(defmacro deffoo [value]
  `(def foo ~value))
nil
cljs.user=> (foo.core/deffoo 5)
            ⬆
Can't def ns-qualified name in namespace foo.core$macros at line 1
Comment by David Nolen [ 14/Aug/16 11:55 AM ]

fixed https://github.com/clojure/clojurescript/commit/756fa9bb196a97e0ae40fd644da5e492e0336c1c





[CLJS-1242] Add cljs.core/pattern? predicate Created: 03/May/15  Updated: 14/Aug/16  Resolved: 14/Aug/16

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

Type: Enhancement Priority: Minor
Reporter: Brandon Bloom Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: patch

Attachments: Text File CLJS-1242_v01.patch    
Patch: Code and Test

 Description   

Just like http://dev.clojure.org/jira/browse/CLJS-1241 , this helps with clj/cljs cross-platform development.



 Comments   
Comment by Brandon Bloom [ 03/May/15 2:38 PM ]

See also http://dev.clojure.org/jira/browse/CLJ-1720

Comment by Brandon Bloom [ 10/May/15 11:36 PM ]

Note that there already is a cljs.core/regexp?, since it's used in a few places by core.

Comment by Samuel Miller [ 27/Jul/15 9:27 PM ]

The patch needs to be updated because of the folder changes

Tested on Linux with V8, SpiderMonkey, and Nashorn. Repljs also functions correctly.

You could have the `pattern?` function point to the `regexp?` function. Maybe there is some reason to keep separate (ability to have non-regex patterns?)

Comment by António Nuno Monteiro [ 14/Aug/16 11:48 AM ]

this would be the same as cljs.core/regexp?

Comment by David Nolen [ 14/Aug/16 11:51 AM ]

Already handled





[CLJS-1029] Investigate how ns aliasing can be supported in ClojureScript macro files Created: 11/Feb/15  Updated: 14/Aug/16  Resolved: 14/Aug/16

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

Type: Task Priority: Minor
Reporter: David Nolen Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None


 Description   

Currently we just require the file. Perhaps possible to control compilation of the macro file such that ClojureScript ns aliases are established. This may not bear fruit but worth thinking about.



 Comments   
Comment by David Nolen [ 11/Feb/15 4:05 PM ]

Any design ideas along this path needs to keep .cljc files in mind.

Comment by David Nolen [ 14/Aug/16 9:07 AM ]

Would introduce a lot of magic for a small payoff.





[CLJS-1744] rest produces nil for larger maps Created: 13/Aug/16  Updated: 14/Aug/16  Resolved: 14/Aug/16

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

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


 Description   

CLJS-1739 illustrated an error where rest can produce nil for maps of size 9 or larger.

With code like this

(doseq [i (range 1 20)]
  (prn [i (last (take (inc i) (iterate rest (zipmap (range i) (range i)))))]))

you can demonstrate that while CLJS-1739 addressed it for 9, a similar issue occurs at 17.



 Comments   
Comment by David Nolen [ 13/Aug/16 10:54 PM ]

You mean rest not next, right?

Comment by Mike Fikes [ 13/Aug/16 11:07 PM ]

Yes, the first sentence in the description should have said rest can produce nil.

The doseq above produces output that looks like this (note how it produces nil values starting at 17:

[1 ()]
[2 ()]
[3 ()]
[4 ()]
[5 ()]
[6 ()]
[7 ()]
[8 ()]
[9 ()]
[10 ()]
[11 ()]
[12 ()]
[13 ()]
[14 ()]
[15 ()]
[16 ()]
[17 nil]
[18 nil]
[19 nil]
nil
Comment by David Nolen [ 14/Aug/16 8:25 AM ]

fixed https://github.com/clojure/clojurescript/commit/709cc57d003590945329ea8745b799e75ac4ba88





[CLJS-1743] Transient{Hash,Array}Map should support IFn like in Clojure Created: 13/Aug/16  Updated: 13/Aug/16

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

Type: Enhancement Priority: Trivial
Reporter: Thomas Mulvaney Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-1743.patch    
Patch: Code and Test

 Description   

Users should be able to invoke transient maps like in Clojure.






[CLJS-1740] Self-host: Need to port more of CLJS-1733 Created: 12/Aug/16  Updated: 13/Aug/16  Resolved: 13/Aug/16

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

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: bootstrap

Attachments: Text File CLJS-1740.patch    
Patch: Code

 Description   

With CLJS-1733 some changes were made that needed to be carried over to self-host.

CLJS-1738 took care of a simple argument refactoring, thus leading to script/test-self-parity passing.

But, script/test-self-host is still failing with failures surrounding unit tests involving the ability to disable analyze deps.

Looking further at the changes made for CLJS-1733, you can see two additional things that need to be ported over:

  1. cljs.js makes a few calls to the cljs.analyzer namespace were cljs.analyzer/analyze-deps is used internally and thus needs to be bound (this fixes script/test-self-host).
  2. With https://github.com/clojure/clojurescript/commit/d197bcb2bef327e00bcb39346258ed314a69b8d9 there is a missing that was previously calculated early in a let and its calculation was moved down to occur later at its use point.


 Comments   
Comment by Mike Fikes [ 12/Aug/16 8:09 PM ]

With the attached patch script/test-self-host goes from failing to passing.

Comment by David Nolen [ 13/Aug/16 10:23 AM ]

fixed https://github.com/clojure/clojurescript/commit/712a8d8f1e69b4f440088ce21d32f63a2e363988





[CLJS-1741] Add :rename to :refer-clojure in ns docstring Created: 12/Aug/16  Updated: 13/Aug/16  Resolved: 13/Aug/16

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

Type: Enhancement Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: docstring

Attachments: Text File CLJS-1741.patch    
Patch: Code

 Description   

Currently it indicates the only option is :exclude.



 Comments   
Comment by David Nolen [ 13/Aug/16 10:10 AM ]

fixed https://github.com/clojure/clojurescript/commit/0b8fd1271a13ba4f072f85c0f408dd4dfb174bf5





[CLJS-1737] Self-host: clojure alias implicit macro use regression Created: 11/Aug/16  Updated: 13/Aug/16  Resolved: 13/Aug/16

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

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: bootstrap

Attachments: Text File CLJS-1737.patch    
Patch: Code and Test

 Description   

If you add a new test case to cljs.clojure-alias-test

(deftest use-macros
  (s/def ::even? (s/and number? even?))
  (is? (s/valid? ::even? 2)))

then script/test-self-parity will fail with

ERROR in (use-macros) (TypeError:NaN:NaN)
Uncaught exception, not in assertion.
expected: nil
  actual: #object[TypeError TypeError: Cannot read property 'call' of undefined]

and issue warnings while failing:

WARNING: Can't take value of macro cljs.spec/def at line 14
WARNING: Can't take value of macro cljs.spec/and at line 14

This appears to be a regression associated with the change made for CLJS-1716. If you check with the previous commit, it will pass. (Note that the proposed unit test needs to be revised slightly to use is instead of is? in order to be applicable to the older code.)



 Comments   
Comment by António Nuno Monteiro [ 13/Aug/16 8:42 AM ]

Attached patch with fix and test.

Comment by Mike Fikes [ 13/Aug/16 9:09 AM ]

I tested this locally and it passes script/test-self-parity. With CLJS-1740 it passes script/test-self-host.

I also tried it downstream with Planck and it works there:

cljs.user=> (require '[clojure.spec :as s])
nil
cljs.user=> (s/def ::even? (s/and number? even?))
:cljs.user/even?
cljs.user=> (s/valid? ::even? 3)
false
cljs.user=> (s/valid? ::even? 4)
true

I also tried the above successfully in script/noderepljs, and I successfully ran script/test (both with and without CLJS-1740).

Comment by David Nolen [ 13/Aug/16 10:07 AM ]

fixed https://github.com/clojure/clojurescript/commit/8a49ce62bb5f53167bc44d75ada31156bcec38b0





[CLJS-1696] Alias clojure.spec.gen => cljs.spec.impl.gen Created: 29/Jun/16  Updated: 13/Aug/16

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

Type: Defect Priority: Minor
Reporter: Shaun LeBron Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-1696.patch    

 Description   

A special case for http://dev.clojure.org/jira/browse/CLJS-1692 to correctly alias clojure.spec.gen.



 Comments   
Comment by Shaun LeBron [ 29/Jun/16 12:02 PM ]

code and test

Comment by David Nolen [ 29/Jun/16 2:04 PM ]

I'm not excited about special casing this one. I'd rather wait to see if people actually use this namespace frequently enough for this to be desirable.

Comment by Shaun LeBron [ 29/Jun/16 10:37 PM ]

yeah, i guess the question is who should be responsible for handling this special case-- the user with reader conditionals or the compiler with this patch.

for extra context, I asked about this last week in slack: https://clojurians.slack.com/archives/clojurescript/p1466866626001218

Comment by Oliver George [ 13/Aug/16 4:41 AM ]

Just to bump this, I see references to requiring clojure.spec.gen in a few places now. The Clojure spec guide mentions it in project setup for generators, and the Clojure spec screencast on customizing generators.

One possibly related point. It seems necessary to require `clojure.test.check` in order for requiring `cljs.spec.impl.gen` to work.





[CLJS-1739] seq on map literal with 9 elements leads to rest producing nil Created: 12/Aug/16  Updated: 12/Aug/16  Resolved: 12/Aug/16

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

Type: Defect Priority: Major
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 1
Labels: None


 Description   

Observe that (take 11 (iterate rest {:a 1 :b 2 :c 3 :d 4 :e 5 :f 6 :g 7 :h 8 :i 9})) produces a nil instead of () right near the end.

({:e 5, :g 7, :c 3, :h 8, :b 2, :d 4, :f 6, :i 9, :a 1} ([:g 7] [:c 3] [:h 8] [:b 2] [:d 4] [:f 6] [:i 9] [:a 1]) ([:c 3] [:h 8] [:b 2] [:d 4] [:f 6] [:i 9] [:a 1]) ([:h 8] [:b 2] [:d 4] [:f 6] [:i 9] [:a 1]) ([:b 2] [:d 4] [:f 6] [:i 9] [:a 1]) ([:d 4] [:f 6] [:i 9] [:a 1]) ([:f 6] [:i 9] [:a 1]) ([:i 9] [:a 1]) ([:a 1]) nil ())


 Comments   
Comment by Rohit Aggarwal [ 12/Aug/16 5:21 PM ]

The following code works as expected:

(take 11 (iterate rest (array-map :a 1 :b 2 :c 3 :d 4 :e 5 :f 6 :g 7 :h 8 :i 9)))
Comment by David Nolen [ 12/Aug/16 6:26 PM ]

fixed https://github.com/clojure/clojurescript/commit/7fe544696ca7dd45785980097f6033f9274e433b





[CLJS-1738] Self-host: need to update call to check-use-macros-inferring-missing Created: 12/Aug/16  Updated: 12/Aug/16  Resolved: 12/Aug/16

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

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: bootstrap

Attachments: Text File CLJS-1738.patch    
Patch: Code

 Description   

script/test-self-parity fails owing to a slight simplification in arguments to check-use-macros-inferring-missing. Also, should incorporate call to check-rename-macros-inferring-missing.



 Comments   
Comment by David Nolen [ 12/Aug/16 3:55 PM ]

fixed https://github.com/clojure/clojurescript/commit/5d30d79b9e1fc3394f02de66504b496933458825





[CLJS-1733] Macro inference issue for macros & runtime vars with the same name Created: 11/Aug/16  Updated: 12/Aug/16  Resolved: 12/Aug/16

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

Type: Defect Priority: Minor
Reporter: Thomas Heller Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None


 Description   

The new :rename feature has several cases that are currently not covered and should either throw or warn.

A) :refer-clojure does not account for inline macros.

(ns some.test
  (:refer-clojure :rename {+ plus}))

(plus 1 2) ;; produces cljs.core._PLUS_.cljs$core$IFn$_invoke$arity$2((1),(2));
(cljs.core/+ 1 2) ;; produces ((1) + (2));

The macro version should be used automatically as well.

B) Conflicting :refer and :rename

(:require [clojure.string :refer (blank? starts-with?) :rename {blank? starts-with?}])

The :refer starts-with? is chosen and the rename is ignored. The :rename should either throw or emit a warning.

C) Conflicting :rename

(:require [some.ns :refer (foo bar) :rename {foo baz bar baz}])

Both renames want to use the alias baz. This should warn or throw as well.



 Comments   
Comment by David Nolen [ 12/Aug/16 12:14 PM ]

The problem is not specific to :rename. Even with a normal :require :refer you will see that the only the runtime var will get considered if the name is shared.

Comment by David Nolen [ 12/Aug/16 2:07 PM ]

Solved the typical refer case https://github.com/clojure/clojurescript/commit/b49c19819f46a78e41a1e3c9147b1127a006664d

Comment by David Nolen [ 12/Aug/16 2:27 PM ]

fixed https://github.com/clojure/clojurescript/commit/d77bc7164bbcd565b794cbb321d567082d58d269





[CLJS-1706] cljs.reader support for namespaced map literal Created: 15/Jul/16  Updated: 11/Aug/16

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

Type: Defect Priority: Major
Reporter: Thomas Heller Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Clojure 1.9 extends support for namespaced maps and cljs.reader needs to be able to read them.

#:example{:key "value"} currently results in a Could not find tag parser for :example in ... error.



 Comments   
Comment by Thomas Heller [ 17/Jul/16 3:20 AM ]

I wanted to start implementing this and started looking at the "spec" [1]. It mentions support for #:: and #::alias, but since cljs.reader does not have access to aliases (or the current ns) I do not know how to proceed. Should it just throw then encountering #:: since it isn't really all that useful in a EDN context?

http://dev.clojure.org/jira/browse/CLJ-1910

Comment by David Nolen [ 10/Aug/16 1:56 PM ]

What does the Clojure EDN reader do here?

Comment by Linus Ericsson [ 11/Aug/16 2:26 PM ]

The clojure.edn-reader seems to catch that it doesn't have a default namespace, nor any aliases.

user> (clojure.edn/read-string "#:a {:b 1}")
{:a/b 1}

user> (clojure.edn/read-string "#::a {:b 1}")
RuntimeException Namespaced map must specify a valid namespace: :a  clojure.lang.EdnReader$NamespaceMapReader.invoke (EdnReader.java:494)

user> (clojure.edn/read-string "#:: {:b 1}")
RuntimeException Invalid token: :  clojure.lang.Util.runtimeException (Util.java:221)

This seems to be sane defaults also for cljs.reader.

Also see the commit adding namespaces maps in clojure





[CLJS-1735] Self-host: cljs.spec speced-vars instance Created: 11/Aug/16  Updated: 11/Aug/16  Resolved: 11/Aug/16

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

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: bootstrap

Attachments: Text File CLJS-1735.patch    
Patch: Code

 Description   

Similar to CLJS-1656 but a new instance here: https://github.com/clojure/clojurescript/blob/85bab0736aae442d55d7f25ffc502b5195afe5c1/src/main/cljs/cljs/spec/test.cljc#L105



 Comments   
Comment by David Nolen [ 11/Aug/16 12:32 PM ]

fixed https://github.com/clojure/clojurescript/commit/afcbae8a768cf432a8786c8be462f68daceab32c





[CLJS-1736] cljs.spec.test: checkable-syms* called with 0-arity Created: 11/Aug/16  Updated: 11/Aug/16  Resolved: 11/Aug/16

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

Type: Defect Priority: Major
Reporter: Mike Fikes Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None


 Description   

On this line https://github.com/clojure/clojurescript/blob/85bab0736aae442d55d7f25ffc502b5195afe5c1/src/main/cljs/cljs/spec/test.cljc#L235
checkable-sums* is called with no arguments, but the function is only defined for a single argument arity.



 Comments   
Comment by David Nolen [ 11/Aug/16 12:31 PM ]

fixed https://github.com/clojure/clojurescript/commit/5b34a6d9fab1ff77fe3aee24413ad0783e015e9f





[CLJS-1708] Self-host: [iu]nstrument-1 needs to qualify [iu]nstrument-1* Created: 15/Jul/16  Updated: 11/Aug/16  Resolved: 11/Aug/16

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

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: bootstrap

Attachments: Text File CLJS-1708-2.patch     Text File CLJS-1708.patch    
Patch: Code

 Description   

The instrument-1 and unstrument-1 macros refer to instrument-1* and unstrument-1* functions which need to be qualified for bootstrap (otherwise they resolve as being in cljs.spec.test$macros).



 Comments   
Comment by Mike Fikes [ 27/Jul/16 8:36 AM ]

Initial patch no longer applies; attaching revised CLJS-1708-2.patch.

Comment by Mike Fikes [ 10/Aug/16 4:18 PM ]

The updated attached patch is still relevant to master in a bootstrapped context.

Comment by David Nolen [ 11/Aug/16 8:14 AM ]

fixed https://github.com/clojure/clojurescript/commit/85bab0736aae442d55d7f25ffc502b5195afe5c1





[CLJS-1730] Support `refer-clojure` special function in REPLs Created: 10/Aug/16  Updated: 11/Aug/16  Resolved: 11/Aug/16

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

Type: Enhancement Priority: Minor
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1730.patch    
Patch: Code

 Description   

Now that we have support for `:rename` in require specifications, the `refer-clojure` function at the REPL can be a useful addition.



 Comments   
Comment by António Nuno Monteiro [ 10/Aug/16 3:26 PM ]

Attached patch with fix.

Comment by David Nolen [ 11/Aug/16 8:13 AM ]

fixed https://github.com/clojure/clojurescript/commit/39f6d7bae0919fe54e847db2fdcfd621cc6f930d





[CLJS-1707] Self-host: with-instrument-disabled needs to qualify *instrument-enabled* Created: 15/Jul/16  Updated: 11/Aug/16  Resolved: 11/Aug/16

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

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: bootstrap

Attachments: Text File CLJS-1707.patch    
Patch: Code

 Description   

The unqualified *instrument-enabled* symbol resolves to cljs.spec.test$macros/*instrument-enabled* in bootstrap. This can be fixed by qualifying the symbol.



 Comments   
Comment by Mike Fikes [ 10/Aug/16 4:26 PM ]

The attached patch is still relevant to master in a bootstrapped context.

Comment by David Nolen [ 11/Aug/16 8:12 AM ]

fixed https://github.com/clojure/clojurescript/commit/b04b7c6efcb861e948e0c6ffdf5529c13c09e141





[CLJS-1515] Self-host: Allow :file key in cljs.js/*load-fn* callback Created: 17/Dec/15  Updated: 11/Aug/16  Resolved: 11/Aug/16

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

Type: Enhancement Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: bootstrap

Attachments: Text File CLJS-1515-1.patch     Text File CLJS-1515-2.patch     Text File CLJS-1515-3.patch     Text File CLJS-1515-4.patch     Text File CLJS-1515-5.patch     Text File CLJS-1515-6.patch     Text File CLJS-1515-7.patch    
Patch: Code and Test

 Description   

Bootstrapped ClojureScript is abstracted away from direct I/O by use of a *load-fn* callback. A result is that when a namespace is loaded, the :file attribute associated with def s in [:cljs.analyzer/namespaces 'foo.ns :defs] in the AST is nil, because cljs.analyzer/*cljs-file* cannot be set to a meaningful value.

This ticket asks for an extension to *load-fn*, allowing a :file key to be optionally included by cljs.js clients, and for cljs.analyzer/*cljs-file* to be bound to that value in appropriate places in cljs.js so that the :file info appears in the AST.

One rationale for this :file attribute is that it makes it easier for clients of cljs.js to look up the file for a def, say, for use when implementing a source REPL special, for example.



 Comments   
Comment by Andrea Richiardi [ 17/Dec/15 4:31 PM ]

Initial patch, adding a :file key to load-fn and a :file-env key inside opts and then assigning it to cljs.analyzer/cljs-file in eval-str. This approach can be discussed and we can create an ad-hoc function for binding. It felt right there.
Moreover, cljs.analyzer/cljs-file gets overridden every time with the payload coming from load-fn.
All this was very quickly done in order to have a feedback from who's more expert than me about the consequences. This is also my very first ClojureScript patch

Comment by Mike Fikes [ 17/Dec/15 5:33 PM ]

I tried this patch. It is working fine for me when loading namespaces, but if I use cljs.js/analyze-str where the string is an ns form referring other namespaces loaded via *load-fn*, along with a def, things are off. (I have that ns referring macros from a clj file and a symbol from a cljs file, and the clj file gets associated with the top-level def and the macro, and the def in the referred file ends up with nil.

As a minor aside, the patch has a spurious whitespace change at the end.

Comment by Mike Fikes [ 17/Dec/15 5:56 PM ]

With respect to the last comment: The patch employs the pattern of conveying the :file passed in the cb via a :file-env opt to the consuming fn. It is consumed in eval-str* but not in analyze-str*. If the same logic is added to analyze-str* then the problem mentioned in the last comment goes away.

Comment by David Miller [ 17/Dec/15 6:48 PM ]

I'm hopeful someone will assign this to a responsible party. I am not that person.

Comment by Andrea Richiardi [ 17/Dec/15 7:21 PM ]

sorry David (Miller) and thanks Mike, I will rework it, adding some tests as well

Comment by Andrea Richiardi [ 17/Dec/15 7:23 PM ]

By the way this makes me think that maybe a better choice is to consider this a side effect and directly modify cljs.analyzer/*cljs-file* returning from *load-fn*, who knows how many other spots I am not covering...

Comment by Mike Fikes [ 18/Dec/15 5:36 AM ]

Two more comments:

1) Broadening the scope of the binding doesn't appear to work properly for me. But things do work if the bindings are done as in the patch now (next to where the other bindings are done).

2) Perhaps :file should be only set if the :lang being called back with is :clj. Maybe this could at least be documented. (It is not clear to me if it is useful for :js, as the patch is setting ana/*cljs-file*.)

Comment by Andrea Richiardi [ 18/Dec/15 10:27 AM ]

About 2), is any AST generated for .js files at all? If yes maybe then we should add it too...I need to explore that code path as well.

Comment by Andrea Richiardi [ 18/Dec/15 3:33 PM ]

So basically with ana/*cljs-file* binding the :file in :meta is not changed at all (I fixed following Mike's advice) but :file is, are we ok with this? In replumb (from planck) we check both so no problem, nonetheless it would be great to know why..

:defs {foo {:protocol-inline nil, :meta {:file bootstrap-test.core, :line 3, :column 7, :end-line 3, :end-column 10, :arglists (quote ([a b]))}, :name bootstrap-test.core/foo, :variadic false, :file /.../clojurescript/src/test/self/bootstrap_test/core.cljs, :end-column 10, :method-params ([a b]), :protocol-impl nil, :arglists-meta (nil nil), :column 1, :line 3, :end-line 3, :max-fixed-arity 2, :fn-var true, :arglists (quote ([a b]))}}, :require-macros nil, :doc nil

Comment by Andrea Richiardi [ 18/Dec/15 3:44 PM ]

It looks like the information in :meta comes directly from the multimethod parse which I dont' think we can change easily. So either we override :file in :meta or we leave as it is with a note in the documentation for :file in *load-fn*

Comment by Andrea Richiardi [ 18/Dec/15 4:10 PM ]

About :js files at least to me it looks like the only trace of importing, say, goog.sting in the AST is in the :imports of the parent namespace. No :file key anywhere, but please correct me if I am wrong as the AST is difficult to untangle

Comment by Andrea Richiardi [ 18/Dec/15 5:29 PM ]

Patch and test

Comment by Mike Fikes [ 18/Dec/15 7:43 PM ]

Comments on {{CLJS-1515-2.patch}} (mostly just opinion):

  1. (Opinion): Introduces new public API, especially with respect to AST exposure. Perhaps fn could instead be added to the test namespace.
  2. (Opinion): I wouldn't try anything complicated to try to patch up the :file that is in the :meta map. (Maybe we'll ultimately figure out why setting cljs.analyzer/*cljs-file* is insufficient for that bit.)
  3. (Opinion): For the :file docstring, I'd avoid mentioning AST. (Even though that was the true motivation for this ticket.) I'd only indicate that it represents the location where :source was obtained. (Which I guess would leave open it being perfectly fine for clients to provide it in the case that :lang is :js.)
  4. script/test-self-host passes for me.
  5. Inadvertent whitespace changes in append-source-map.
Comment by Andrea Richiardi [ 18/Dec/15 7:49 PM ]

1. Sorry Mike I don't understand when you say fn...what do you mean? Can you expand?
2. Yes and it would change a lot of code, that's why I didn't even try
3. Ok can change that, but where should be mentioned that we are modifying :file but not inside :meta?
4. Great!
5. You know I really tried hard not to have that, I will try again to disable all the auto indent my emacs has.

Comment by Mike Fikes [ 18/Dec/15 8:30 PM ]

1. The three new public functions in cljs.js: (var-ast, ns-ast, file->lang) could perhaps be moved to be utility functions in the self-host test namespace.
3. Dunno about the :meta question. But on the :lang :js question, perhaps the patch should only bind :cljs.analyzer/*cljs-file* if :lang :clj?

Comment by Andrea Richiardi [ 18/Dec/15 8:38 PM ]

1. I know it looks like they are used in test only, but I put them there as public because both replumb and planck use them and I was kind of "proposing" this kind of AST utils to be part of the official API (so that the poor dev does not have to go through cljs.analyzer in order to query the AST. I understand if no though.
3. This I don't really know, and seek guidance. I have not noticed any significant change in the AST for .js file, maybe *cljs-file* is never queried in that code path. I could not even find a way to test it. But I could, of course, be very wrong.

Comment by Andrea Richiardi [ 21/Dec/15 2:13 PM ]

This puts the utils functions in the test namespace for now, maybe thinking about exposing some API in the future.

Comment by Andrea Richiardi [ 21/Dec/15 8:19 PM ]

About :js:

  • it looks like the analyze-str code path simply recurs to fetch the next dep. So I guess that branch does not touch the AST.
  • for the require code path it looks like it -> is -> similar.

Therefore I don't see the point in adding :file for :js and I will not bind *cljs-file* if this is the case, as you suggested.

Comment by Andrea Richiardi [ 21/Dec/15 9:48 PM ]

Patch #4 changes the conveying key to :cljs-file, after Mike's good suggestion, and moves the assoc to the (condp ... :clj) branch only. I also added a test to check that *cljs-file* does not match the file path when in the :js branch.

Comment by Andrea Richiardi [ 21/Dec/15 11:56 PM ]

Another note, the *cljs-file* test works because the binding form does not actually restore the old value when it exits...In Clojure it would not probably work.

^ This is plain wrong, I was not considering the "when" my tests are executed, please disregard.

Comment by Mike Fikes [ 23/Dec/15 5:13 PM ]

CLJS-1515-4.patch LGTM.

Details: I tested against current ClojureScript master, using downstream Planck to load regular and macro namespaces and the :file portion of the AST gets properly updated. This also occurs if I instead use cljs.js/analyze-str passing in an ns form that causes code to be loaded. Additionally unit tests (regular and bootstrap) pass for me. I think this patch is functionally good to go.

Comment by David Nolen [ 26/Dec/15 6:54 AM ]

Copying goog.string into the source tree is not desirable. Please fix the tests to remove this. If you must, copy it to a temporary a location from the Google Closure Library JAR and remove it after the test has completed, thanks.

Comment by Andrea Richiardi [ 26/Dec/15 2:20 PM ]

Patch 5 avoid copying string.js and re-uses self_host/test.js.

Comment by Andrea Richiardi [ 26/Dec/15 2:22 PM ]

Done what you asked

Comment by Mike Fikes [ 05/Feb/16 8:05 PM ]

CLJS-1515-5.patch no longer applies

Comment by Andrea Richiardi [ 14/Feb/16 9:17 PM ]

Reapplied and re-tested. Works

Testing with Node

Testing self-host.test

Ran 8 tests containing 47 assertions.
0 failures, 0 errors.
Comment by Andrea Richiardi [ 10/Aug/16 7:07 PM ]

All self-test pass!

Comment by David Nolen [ 11/Aug/16 8:11 AM ]

fixed https://github.com/clojure/clojurescript/commit/cc65ad3a87d9d3f6e51938bd3df8b9052e68d76a





[CLJS-1732] Add docstrings for new use and use-macros REPL specials Created: 10/Aug/16  Updated: 11/Aug/16  Resolved: 11/Aug/16

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

Type: Enhancement Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: docstring

Attachments: Text File CLJS-1732.patch    
Patch: Code

 Description   

To go along with CLJS-1729



 Comments   
Comment by David Nolen [ 11/Aug/16 8:10 AM ]

fixed https://github.com/clojure/clojurescript/commit/60d534977ace15667c38cf2c2609e27ad72f539a





[CLJS-1720] Self-host: Qualify symbols and namespaced keywords in spec macros Created: 31/Jul/16  Updated: 11/Aug/16  Resolved: 11/Aug/16

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

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: bootstrap

Attachments: Text File CLJS-1720-2.patch     Text File CLJS-1720-3.patch     Text File CLJS-1720.patch    
Patch: Code

 Description   

There are currently a few symbols and namespaced keywords in the cljs.spec macros namespace that either need to be qualified for proper operation, or should be.

The symbols fall into the category of calls to runtime functions in the cljs.spec namespace, and need qualification in order to avoid the $macros suffix. These comprise with-gen and gen.

In terms of keywords, an example that causes a failure is ::kvs->map in keys*: It resolves to :cljs.spec$macros/kvs->map which doesn't match the :cljs.spec/kvs->map spec registered near the bottom of the cljs.spec runtime namespace.

An example that doesn't cause an outright failure, but arguably inhibits its proper use by client code is ::nil and ::pred in the nilable macro. Ideally these would resolve to :cljs.spec/nil and :cljs.spec/pred so that client code can rely on these namespaced symbols identifying the branches.

Given the nilable example, you could argue that the intent is that all namespaced keywords in the cljs.spec macro namespace that employ :: resolve in :cljs.spec so that they can be used not simply as unique identifiers (the intent is not simply to avoid potential collisions), but so that they can be used as stable identifiers.

The set of keywords that should be qualified comprises: ::kind-form, ::kfn, ::conform-all, ::kvs->map, ::nil, and ::pred



 Comments   
Comment by Mike Fikes [ 10/Aug/16 9:43 PM ]

Patch no longer applies; attaching re-baselined CLJS-1720-2.patch

Comment by Mike Fikes [ 10/Aug/16 9:55 PM ]

Sorry David, I botched that last re-baseline. Attaching a corrected CLJS-1720-3.patch.

Comment by David Nolen [ 11/Aug/16 8:09 AM ]

fixed https://github.com/clojure/clojurescript/commit/20b08fe442a24c46526c812f6c52ad8c7d3f6c9b





[CLJS-1731] Self-host: do_template problem with script/test-self-parity Created: 10/Aug/16  Updated: 11/Aug/16  Resolved: 11/Aug/16

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

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: bootstrap

Attachments: Text File CLJS-1731.patch    
Patch: Code and Test

 Description   
Testing cljs.spec-test

ERROR in (conform-explain) (TypeError:NaN:NaN)
Uncaught exception, not in assertion.
expected: nil
  actual: #object[TypeError TypeError: Cannot read property 'do_template' of undefined]


 Comments   
Comment by Mike Fikes [ 10/Aug/16 4:04 PM ]

The attached patch makes some slight adjustments to the self-host parity unit test namespace loading logic so that it can successfully load the clojure.template namespace.

Note that the patch fixes this issue, but at the same time it reveals that there are now 6 new test failures related to cljs.spec.

Comment by Mike Fikes [ 10/Aug/16 10:05 PM ]

Good news: The changes in CLJS-1720 cover the newly failing unit tests mentioned above.

Comment by David Nolen [ 11/Aug/16 8:08 AM ]

fixed https://github.com/clojure/clojurescript/commit/eaba75c30c79785f457e7356b7539ac95741fa4b





[CLJS-1734] :import in ns silently discards imported classes with the same name Created: 11/Aug/16  Updated: 11/Aug/16

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

Type: Defect Priority: Minor
Reporter: Thomas Heller Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   
(ns some.ns
  (:import goog.fx.Transition.EventType
           goog.history.EventType))

Both classes are named EventType and the second will effectively remove the first one without warning or error.






[CLJS-1630] Add unit test for static dispatch Created: 21/Apr/16  Updated: 10/Aug/16

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

Type: Enhancement Priority: Minor
Reporter: Yehonathan Sharvit Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-1630.patch    

 Description   

This unit test is an edge case that illustrates why in the code of `emit :invoke` we must stay with `call` for the high order case where static information is missing .



 Comments   
Comment by Yehonathan Sharvit [ 10/Aug/16 10:52 PM ]

Could someone take a look at this unit test?





[CLJS-1715] Self-host: Symbols in cljs.spec.test/instrument Created: 27/Jul/16  Updated: 10/Aug/16

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

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: bootstrap

Attachments: Text File CLJS-1715-2.patch     Text File CLJS-1715.patch    
Patch: Code

 Description   

For bootstrapped ClojureScript, within cljs.spec.test/instrument:

1. There is a use of speced-vars that conditionally needs the $macros suffix (analogous to CLJS-1656).
2. The references to the runtime fns instrumentable-syms and distinct-by need to be qualified (otherwise their expansions use $macros.



 Comments   
Comment by Mike Fikes [ 10/Aug/16 4:12 PM ]

Previous patch no longer applies. Attaching a rebased patch. This patch is still relevant to master.





[CLJS-1665] Use Javascript array to create a collection in cljs.reader Created: 03/Jun/16  Updated: 10/Aug/16  Resolved: 10/Aug/16

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

Type: Enhancement Priority: Minor
Reporter: Rohit Aggarwal Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: performance

Attachments: Text File CLJS-1665.patch    
Patch: Code

 Description   

For performance, we should be using a Javascript array to create an ArrayMap/HashMap/Set/List/Vector instead of creating them out of a Vector. Most of the underlying data types do have methods to convert from an array to that type.

The big change is that cljs.reader/read-delimited-list now returns an array instead of a vector.

Benchmarking code:

(def nums (range 20))
(def nums-map (into {} (map (fn [i] [i i]) nums)))

(simple-benchmark [s "{:foo 1 :bar 2}"] (reader/read-string s) 1000000)
(simple-benchmark [s (pr-str nums-map)] (reader/read-string s) 100000)
(simple-benchmark [s (pr-str nums)] (reader/read-string s) 100000)
(simple-benchmark [s (pr-str (vec nums))] (reader/read-string s) 100000)
(simple-benchmark [s (pr-str (set nums))] (reader/read-string s) 100000)

Results (All times in msecs):

Engine Benchmark Old New Improvement
v8 Small Map 6620 5516 20.01%
SpiderMonkey Small Map 11929 10606 12.47%
JSC Small Map 5101 4158 22.68%
v8 Large Map 6075 5548 9.50%
SpiderMonkey Large Map 13070 11933 9.53%
JSC Large Map 4777 4273 11.79%
v8 List 2308 2280 1.23%
SpiderMonkey List 6167 4777 29.10%
JSC List 1891 1737 8.87%
v8 Vector 2276 2242 1.52%
SpiderMonkey Vector 5239 4700 11.47%
JSC Vector 1832 1684 8.79%
v8 Set 3362 3271 2.78%
SpiderMonkey Set 7283 6880 5.86%
JSC Set 2771 2619 5.80%


 Comments   
Comment by David Nolen [ 10/Aug/16 2:24 PM ]

fixed https://github.com/clojure/clojurescript/commit/c98e9a768f003f178b36d001a26cbfcd95fae3c6

Comment by Rohit Aggarwal [ 10/Aug/16 3:00 PM ]

Thank you David!





[CLJS-1186] add :postamble option to compiler Created: 02/Apr/15  Updated: 10/Aug/16  Resolved: 10/Aug/16

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

Type: Enhancement Priority: Minor
Reporter: Michael Bradley Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: cljs

Attachments: Text File cljs_1186.patch    

 Description   

Similar to CLJS-723:

1) :postamble's value will be a vector of paths
2) the compiled output is appended with the contents of the files at those paths
3) the generated source map points to the correct/adjusted line numbers



 Comments   
Comment by David Nolen [ 12/Feb/16 4:42 PM ]

Would like to hear more use cases for this one.

Comment by David Nolen [ 10/Aug/16 2:42 PM ]

Not going to do this one.





[CLJS-1556] Invalid code emit for obj literal Created: 31/Jan/16  Updated: 10/Aug/16  Resolved: 10/Aug/16

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

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1556.patch    
Patch: Code and Test

 Description   

For some legal ClojureScript expressions involving #js object literals, invalid JavaScript is emitted. Specifically this has to do with object literals appearing at the beginning of statements where the opening brace can be interpreted as the beginning of a JavaScript block.

One way to reproduce this is to evaluate

(do #js {:a 1} 
    #js {:b 2})

in the Node REPL. In this case it is the first object literal that causes the problem; the second one emitted follows a return statement and is OK.

Rationale for marking it as minor: This appears to only really occur in places where the object literal won't actually be used.



 Comments   
Comment by David Nolen [ 10/Aug/16 2:38 PM ]

fixed https://github.com/clojure/clojurescript/commit/f0b8531ab53ff2af0a47b122faac17feb1c36413





[CLJS-1607] bug with `specify!` in JS prototypes with `static-fns` true Created: 23/Mar/16  Updated: 10/Aug/16  Resolved: 10/Aug/16

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

Type: Defect Priority: Minor
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 2
Labels: None
Environment:

affects 1.8.34


Attachments: Text File CLJS-1607.patch    
Patch: Code and Test

 Description   

compiling this code with `:static-fns` true

(ns bug.core)

(defprotocol IBug
  (bug [this other] "A sample protocol"))

(defn MyBug [])
(specify! (.-prototype MyBug)
  IBug
  (bug [this other]
    "bug")
  Object
  (foo [this]
    (bug this 3))) ;; line 13

causes the following warning:

WARNING: Use of undeclared Var bug.core/x14072 at line 13


 Comments   
Comment by António Nuno Monteiro [ 23/Mar/16 1:42 PM ]

narrowed it down to this line (https://github.com/clojure/clojurescript/blob/f0ac4c92006ac618516c11e9ca3527904d35d4af/src/main/clojure/cljs/compiler.cljc#L936) being called in `:advanced` because it passes the check of cljs-static-fns in that case

Comment by António Nuno Monteiro [ 29/Jul/16 7:16 AM ]

Upon further investigation, this is actually specific to `:static-fns true`, not advanced compilation.

Comment by António Nuno Monteiro [ 29/Jul/16 9:15 AM ]

Attached patch with fix and test.

Comment by David Nolen [ 10/Aug/16 2:36 PM ]

fixed https://github.com/clojure/clojurescript/commit/273bcd33522b085fe98948598528fcf2c90a778c





[CLJS-1591] Compilation time go up significantly when nesting multimethods Created: 25/Feb/16  Updated: 10/Aug/16  Resolved: 10/Aug/16

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

Type: Defect Priority: Minor
Reporter: Marian Schubert Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: cljs, compiler

Attachments: Text File CLJS-1591.patch    

 Description   

Code like this takes 140 seconds to compile on my machine. Regular functions don't seem to trigger this behaviour.

(ns slow.core)

(defmulti my-multimethod (fn [x] :whatever))

(defn this-is-sloow-to-compile []
  (my-multimethod
   (my-multimethod
    (my-multimethod
     (my-multimethod
      (my-multimethod
       (my-multimethod
        (my-multimethod
         (my-multimethod
          (my-multimethod
           (my-multimethod
            (my-multimethod
             (my-multimethod
              (my-multimethod
               (my-multimethod
                (my-multimethod
                 (my-multimethod
                  (my-multimethod
                   (my-multimethod
                    (my-multimethod
                     (my-multimethod {})))))))))))))))))))))

$ rm -rf target/ && ./scripts/release 
Building ...
Analyzing jar:file:/Users/maio/.m2/repository/org/clojure/clojurescript/1.7.228/clojurescript-1.7.228.jar!/cljs/core.cljs
Compiling src/slow/core.cljs        <-- here it spends most of the time
Applying optimizations :advanced to 11 sources
... done. Elapsed 141.85386191 seconds

Whole project is here https://github.com/maio/slow-cljs-build



 Comments   
Comment by Mike Fikes [ 04/Mar/16 4:32 PM ]

Hrm. This fix is evidently in the Closure compiler used by 1.7.228: https://github.com/google/closure-compiler/issues/1049

Comment by Thomas Heller [ 06/Mar/16 6:25 AM ]

@mfikes the slowdown is not related to the Closure Compiler since it happens when compiling cljs->js not when optimizing.

The reason for the slowdown is due to the arguments of a multimethod call being analyzed twice (or more in case of deep nesting).

See [1] for the problematic code.

multimethods are not fn-var? so the or does not short circuit and (all-values? argexprs) is reached. This forces the argexprs lazy-seq (thereby analyzing the args). Since the args are not all-values? the else-branch of the if is taken, which then later causes the args to be analyzed again. My math is weak but I'm not mistaken this is O(n!), explaining the dramatic slowdown.

Every var that is not fn-var? is affected by this:

(defn test [& args] :whatever)
(def my-multimethod test)
;; or
(def my-multimethod
  (reify
    IFn
    (-invoke [a] :whatever)))

One solution would be to fix all-values? that instead of running through analyze it could just check whether all args are fixed literals (ie. not list? but all of number? string? symbol? keyword? etc.).

I'm not really sure why the else-branch in [1] exists at all but I assume it is to work around some JS quirks. I will hold off on writing a patch until I figure out why the extra let introduced in the else-branch is needed.

[1] https://github.com/clojure/clojurescript/blob/f58dcdf4dc37ef52d4eb1e2b7c994282bf6351f5/src/main/clojure/cljs/analyzer.cljc#L2257-L2263

PS: forgot to add that this does not happen with :static-fns false since it also prevents the else from being reached.

Comment by Thomas Heller [ 06/Mar/16 6:55 AM ]

The else was introduced in CLJS-855 and is sort of required for invokes without arity information and :static-fns true.

Changing all-values? to just check literals instead of analyzing should be a valid solution.

Comment by Thomas Heller [ 14/Mar/16 8:31 AM ]

The patch removes the extra analyze and instead just checks the few cases that can actually be used without assignment first.

This removes the slowdown while keeping all the functionality.

Comment by David Nolen [ 10/Aug/16 2:32 PM ]

fixed https://github.com/clojure/clojurescript/commit/1589e5848ebb56ab451cb73f955dbc0b01e7aba0





[CLJS-1713] cljs.spec/explain-data returns different data structure than clojure.spec/explain-data Created: 24/Jul/16  Updated: 10/Aug/16  Resolved: 10/Aug/16

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

Type: Defect Priority: Minor
Reporter: Marshall Abrams Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: spec
Environment:

Tested in Clojure 1.9.0-alpha10 and Clojurescript 1.9.89



 Description   

The structure of the data returned by cljs.spec/explain-data seems to be unnecessarily different from the structure of data returned by clojure.spec/explain-data. Since, as I understand it, explain.data is mean to be used programmatically, this means that code has to be changed, unnecessarily, it seems, to work with both dialects.

Ignoring trivial differences that are to be expected going from one dialect to the other, in Clojure, the value of the top-level key :problems is a sequence of maps, each of which includes a :path key and value. In Clojurescript, the value of :problems is a map in which those vectors that were the values of :path in Clojure are instead keys, whose values are the rest of the corresponding Clojure maps.

Example:

(s/def ::a #(> % 0))
(s/def ::b (s/or :lt0 #(< % 0.0) :gt1 #(> % 1.0)))
(s/def ::all (s/keys :req-un [::a ::b]))
(s/explain ::all {:a -1 :b 0.5})
(pprint (s/explain-data ::all {:a -1 :b 0.5}))

In Clojure 1.9.0-alpha10, the last line returns:

#:clojure.spec{:problems ({:path [:a], :pred (> % 0), :val -1, :via [:user/all :user/a], :in [:a]}
                          {:path [:b :lt0], :pred (< % 0.0), :val 0.5, :via [:user/all :user/b], :in [:b]}
                          {:path [:b :gt1], :pred (> % 1.0), :val 0.5, :via [:user/all :user/b], :in [:b]})}

In Clojurescript 1.9.89, the same line returns:

{:cljs.spec/problems {[:a] {:pred (> % 0), :val -1, :via [:cljs.user/all :cljs.user/a], :in [:a]}, 
                      [:b :lt0] {:pred (< % 0), :val 0.5, :via [:cljs.user/all :cljs.user/b], :in [:b]},
                      [:b :gt1] {:pred (> % 1), :val 0.5, :via [:cljs.user/all :cljs.user/b], :in [:b]}}}


 Comments   
Comment by Marshall Abrams [ 24/Jul/16 11:58 AM ]

I just realized that Clojurescript 1.9.89 is still based on Clojure 1.9.0-alpha7, in which the data structure is like in the Clojurescript last example above. That explains the difference in the examples, and I assume that the explain-data return values will eventually be harmonized between the dialects, with future releases of Clojurescript (possibly after further changes in Clojure's explain-data return values, since this seems to be in flux). Apologies if this is just a "noise" ticket that shouldn't have been opened.

Comment by David Nolen [ 10/Aug/16 2:18 PM ]

fixed in master





[CLJS-1638] :elide-asserts disables atom validators in :advanced Created: 07/May/16  Updated: 10/Aug/16  Resolved: 10/Aug/16

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

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

Attachments: Text File CLJS-1638.patch    
Patch: Code and Test

 Description   

Background: In Clojure, *assert* does not affect atom validators.

$ java -jar cljs.jar
Clojure 1.8.0
user=> (set! *assert* false)
false
user=> (let [a (atom 1 :validator pos?)]
    (reset! a 0)
    (println @a))
IllegalStateException Invalid reference state  clojure.lang.ARef.validate (ARef.java:33)

In ClojureScript, :elide-asserts affects atom validators, but only in :advanced:

build.clj:

(require 'cljs.build.api)

(cljs.build.api/build "src" 
  {:main          'foo.core
   :target        :nodejs
   :output-to     "main.js"
   :optimizations :advanced
   :elide-asserts true})

(System/exit 0)

src/foo/core.cljs:

(ns foo.core
  (:require [cljs.nodejs :as nodejs]))

(nodejs/enable-util-print!)

(defn -main [& args]
  (let [a (atom 1 :validator pos?)]
    (reset! a 0)
    (println @a)))

(set! *main-cli-fn* -main)

Build with: java -cp cljs.jar:src clojure.main build.clj
Run with node main.js

You will see that with :advanced, the code prints 0, but with :none the atom validator rejects the reset! attempt.



 Comments   
Comment by Mike Fikes [ 07/May/16 9:33 PM ]

Attached CLJS-1638.patch.

Note: Even though the patch adds a new test, the test simply excercises triggering atom validator behavior (to help ensure no regression). Since we don't have tests built with :elide-asserts true, to confirm the fix, you need to manually run through the repro in this ticket's description.

The patch does not attempt to fuse the resulting two nested when-not constructs so as to preserve the outer fast JavaScript null check emitted which tests if a validator has been set—the intent being to preserve fast path behavior in the common case of not having a validator.

Comment by David Nolen [ 10/Aug/16 2:14 PM ]

fixed https://github.com/clojure/clojurescript/commit/73e99298cc19569855de40eeb6ab1c21d25987b3





[CLJS-1640] Use the unshaded version of the closure compiler Created: 16/May/16  Updated: 10/Aug/16  Resolved: 10/Aug/16

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

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


 Description   

The current version of clojurescript (1.8.51) depends on [com.google.javascript/closure-compiler "v20160315"]. That artifact is actually a fat jar - it includes all the dependencies (including guava & protobuf). This causes duplicate class warnings in some tools.

There was a recent change in the closure compiler that created an unshaded artifact [com.google.javascript/closure-compiler-unshaded "v20160315"] that doesn't bundle it's dependencies. It would be nice if clojurescript changed it's dependency to that one



 Comments   
Comment by David Nolen [ 21/May/16 5:04 PM ]

Thanks for bringing this up. Sounds like a good idea.

Comment by David Nolen [ 10/Aug/16 2:12 PM ]

fixed https://github.com/clojure/clojurescript/commit/e8d14e690894aa57c4efa2d1fd425ed16c8a1553





[CLJS-1721] 3-arity get-in fails on types which do not implement ILookup Created: 02/Aug/16  Updated: 10/Aug/16  Resolved: 10/Aug/16

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

Type: Defect Priority: Minor
Reporter: Thomas Mulvaney Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1721.patch    

 Description   

The following are examples which fail:

(get-in {:a "data"} [:a 0] :not-found)
(get-in {:a (array 0 1 2 3)} [:a 0] :not-found)

Whilst the 2-arity version succeeds.



 Comments   
Comment by Thomas Mulvaney [ 04/Aug/16 7:57 AM ]

Also worth noting that the 3-arity version is quicker than the 2-arity version on long paths because it bails when a lookup fails. For example (get-in {} [:a :b :c :d :e :f]) is slower than (get-in {} [:a :b :c :d :e :f] :not-found) which bails out as soon a lookup fails. The former continues to call `get` on nil for the rest of the keys.

Perhaps it would be better for the 2-arity version to make a call to the 3-arity version passing nil as the last argument to take advantage of the early return. Would be happy to update the patch to include this.

Comment by David Nolen [ 10/Aug/16 1:51 PM ]

fixed https://github.com/clojure/clojurescript/commit/817b44d34795696eda473776e355fb956d3aa5ae





[CLJS-1728] Update doc for ns for new :rename capability Created: 09/Aug/16  Updated: 10/Aug/16  Resolved: 10/Aug/16

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

Type: Enhancement Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: docstring

Attachments: Text File CLJS-1728.patch    
Patch: Code

 Description   

Update the docstring for the ns special to cover the :rename option landed with CLJS-1508.



 Comments   
Comment by David Nolen [ 10/Aug/16 1:43 PM ]

fixed https://github.com/clojure/clojurescript/commit/db896ff44c521a5323feb0de07f7ea9464b78215





[CLJS-1729] Support `use` special function in REPLs Created: 10/Aug/16  Updated: 10/Aug/16  Resolved: 10/Aug/16

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

Type: Enhancement Priority: Minor
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1729.patch    
Patch: Code

 Description   

While `:use` is supported at the NS declaration level, REPLs only support `import` and `require`.

Supporting `use` and `use-macros` in REPLs would be a useful addition.



 Comments   
Comment by António Nuno Monteiro [ 10/Aug/16 11:00 AM ]

Attached patch with fix

Comment by David Nolen [ 10/Aug/16 1:42 PM ]

fixed https://github.com/clojure/clojurescript/commit/8c6bbf8bc9f5b3e04fd8b172e725894256076e75





[CLJS-1508] Extend ns form to support :rename option Created: 09/Dec/15  Updated: 09/Aug/16  Resolved: 09/Aug/16

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

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 1
Labels: None

Attachments: Text File CLJS-1508-1.patch     Text File CLJS-1508.patch    
Patch: Code and Test

 Description   

Support a :rename option to :require. Following :rename should be a map of symbol to symbol. Refer to clojure.core/refer for semantics with respect to this allowing avoidance of clashes.

An REPL example:

(require '[fipp.edn :refer (pprint) :rename {pprint fipp}])


 Comments   
Comment by António Nuno Monteiro [ 01/Aug/16 7:24 AM ]

Added patch with fix and tests.

I'd appreciate some initial feedback on this one.

Comment by David Nolen [ 08/Aug/16 7:51 AM ]

Flowing a new kind of value (maps) through this code seems like a big complication in an area where the code is already not particularly straightforward. I would also be a little concerned about downstream complexity (other people also have to make this check if using the analyzer for some reason).

Comment by António Nuno Monteiro [ 09/Aug/16 1:18 PM ]

Attached CLJS-1508-1.patch which takes feedback into account

Comment by David Nolen [ 09/Aug/16 4:26 PM ]

fixed https://github.com/clojure/clojurescript/commit/d388905d67d107e2afbc4d5d4be9f7f935063177





[CLJS-1350] Compiler support for browser REPL Created: 19/Jul/15  Updated: 09/Aug/16  Resolved: 09/Aug/16

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

Type: Enhancement Priority: Major
Reporter: David Nolen Assignee: Unassigned
Resolution: Completed Votes: 2
Labels: None


 Description   

Currently the browser REPL experience could be considerably enhanced just by eliminating manual configuration in source. Instead REPL configuration could happen via a compiler option. This would make REPL support considerably more robust in the face of user errors while developing.



 Comments   
Comment by David Nolen [ 09/Aug/16 4:15 PM ]

fixed https://github.com/clojure/clojurescript/commit/e6f37a018c5d611994174db55883ee611ab62006





[CLJS-1723] pr-writer-ex-info does not print false values and tries to reimplement map printer Created: 04/Aug/16  Updated: 08/Aug/16

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

Type: Defect Priority: Trivial
Reporter: Antonin Hildebrand Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-1723.patch    
Patch: Code

 Description   

While working on CLJS-1722 I spotted two issues with pr-writer-ex-info:

1) if data or cause happen to be "false", they are skipped in printing
2) the code tries to mimic map printer

This patch fixes both issues. It turns out printing real info map yields exactly same string output and plays better with cljs-devtools printer (which newly sees real map instead of soup of strings and values).

For reference: pr-writer-ex-info was implemented in CLJS-983



 Comments   
Comment by David Nolen [ 08/Aug/16 7:39 AM ]

Can you explain why you think 1 is a problem? data should be a map. cause should be an error but I suppose that could be a problem if you have a rethrow which wraps some throwable value (which of course could be anything).

Comment by Antonin Hildebrand [ 08/Aug/16 2:32 PM ]

Disclaimer: I don't have much experience with Clojure and almost none with Java.

Just by reading the code I didn't see any checks in ex-info (or in ExceptionInfo ctor) to restrict data or cause. So I assumed anything can be passed in and printer should print it as-is without additional logic. I assumed that checking for nil values was intentional abbreviation when CLJS-983 was implemented. And I assumed that false value was supposed to be printed. At least I personally would want it to be distinguished from nil. Or there should be no abbreviation at all.





[CLJS-1727] Regression when evaluating non-sequential forms at the REPL Created: 05/Aug/16  Updated: 08/Aug/16  Resolved: 08/Aug/16

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

Type: Defect Priority: Major
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None
Environment:

Affects master as of commit 8524d7b1ac7c1c418ec394e89322e341070414ca


Attachments: Text File CLJS-1727.patch    
Patch: Code

 Description   

this commit [1] assumes that the forms evaluated at the REPL are always sequential by comparing `(first form)` with 'ns.
This makes it impossible to evaluate e.g. numbers or symbols.

[1]: https://github.com/clojure/clojurescript/commit/8524d7b1ac7c1c418ec394e89322e341070414ca



 Comments   
Comment by António Nuno Monteiro [ 05/Aug/16 7:39 AM ]

Attached patch with fix.

Comment by David Nolen [ 08/Aug/16 7:36 AM ]

fixed https://github.com/clojure/clojurescript/commit/2cc8f921cb1e56dbeb901e61a7149d283ad78bc7





[CLJS-884] with-redefs don't restores previous values when used inside go block Created: 09/Nov/14  Updated: 07/Aug/16

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

Type: Defect Priority: Trivial
Reporter: Vladimir Iakovlev Assignee: Unassigned
Resolution: Unresolved Votes: 4
Labels: None
Environment:

clojure 1.6, clojurescript 0.0-2371, core.async 0.1.346.0-17112a-alpha


Attachments: Text File cljs_884.patch    

 Description   

For example I have code and test:

(defn test-fn [] :original)

(deftest ^:async test-with-redefs-async
  (go (is (= (test-fn) :original))
      (with-redefs [test-fn (fn [] :mocked)]
        (is (= (test-fn) :mocked)))
      (is (= (test-fn) :original))
      (done)))

And test failed in last `is`:

FAIL in (test-with-redefs-async) (:)
expected: (= (test-fn) :original)
  actual: (not (= :mocked :original))


 Comments   
Comment by Vladimir Iakovlev [ 09/Nov/14 1:26 PM ]

I attached little patch with fix. With it `with-redefs` forced to use previous value for restoring bindings.

But I don't know how to add test for this issue, because it requires core.async. So I tested with:

(defn test-fn [] :original)

(deftest test-with-redefs-sync
  (is (= (test-fn) :original))
  (with-redefs [test-fn (fn [] :mocked)]
    (is (= (test-fn) :mocked)))
  (is (= (test-fn) :original)))

(deftest ^:async test-with-redefs-async
  (go (is (= (test-fn) :original))
      (with-redefs [test-fn (fn [] :mocked)]
        (is (= (test-fn) :mocked)))
      (is (= (test-fn) :original))
      (done)))
Comment by Yehonathan Sharvit [ 07/Aug/16 2:49 AM ]

Why this bug is not adressed?

Comment by Antonin Hildebrand [ 07/Aug/16 4:57 PM ]

It might be related to CLJS-1634. The likely reason: it is technically very hard to solve without paying non-negligible runtime price for it.

Comment by Yehonathan Sharvit [ 07/Aug/16 11:26 PM ]

The problem mentioned in http://dev.clojure.org/jira/browse/CLJS-1634 is much wider.

The current issue only deals with the fact that `with-redefs` doen't restore previous values when used inside go block.

There is a very simple solution to this specific issue here: https://nvbn.github.io/2014/11/05/protocols-for-testing/ (also written by Vladimir).





[CLJS-1726] demunge is too agreesive and incorrect in some cases Created: 04/Aug/16  Updated: 04/Aug/16

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

Type: Enhancement Priority: Minor
Reporter: Antonin Hildebrand Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

I have implemented some "demunging" logic in cljs-devtools (and dirac) to present original user-friendly names in UI.

During my testing I spotted some wrong edge-cases and incorrect behaviours of demunge:

1) it is too aggressive in replacing dollars - some dollars can be "real" dollars as part of original name
2) it does not revert js-reserved? transformation applied during munging
3) it is oblivious to underscores/dashes - some underscores were "real" underscores before munging
(this may be not complete)

I have worked around those issues on my side and implemented some heuristics[1] based on context, but it is far from perfect.

I'm not sure how to properly fix those, so I wanted to open a ticket with discussion. Maybe people will have some clever ideas.

Currently munging is lossy and we probably don't want to touch it for compatibility reasons.
Maybe we could mark original underscores and dollars in some way, so demunge could properly skip them.

1) One strategy could be to use some (rare) unicode characters, but that would be problematic for people to type.
2) Another strategy could be to escape original dollars and underscores somehow (using more dollars and underscores .
3) Better ideas?

[1] https://github.com/binaryage/cljs-devtools/blob/52899e61e33373df36be8dcb23c69377936821b2/src/lib/devtools/munging.cljs#L154-L185






[CLJS-1725] defrecord does not set cljs$lang$ctorStr Created: 04/Aug/16  Updated: 04/Aug/16

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

Type: Defect Priority: Trivial
Reporter: Antonin Hildebrand Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-1725.patch    
Patch: Code and Test

 Description   

While implementing some demunging code[1] for cljs-devtools, I stumbled upon inconsistency in type->str. It works for deftypes but not for defrecords.

The problem is that defrecord does not set! cljs$lang$ctorStr field for some reason. This has been likely overlooked, because type->str is not used often, just for some rare error exceptions as far I can see.

Anyways, this patch fixes it by copy&pasting it from deftype. A better solution to avoid future problems like this would be to extract shared code between deftypes and defrecords, but that seems to be a lot of work.

The patch also adds some tests. I had to add some utility macros to run those tests only in simple mode, because type->str is not supposed to work under advanced builds.
Also test for CLJS-1722 was added to be uncommented after potentially merging it.

[1] https://github.com/binaryage/cljs-devtools/blob/52899e61e33373df36be8dcb23c69377936821b2/src/lib/devtools/munging.cljs#L56-L60






[CLJS-1724] Include IIterable in fast-path-protocols Created: 04/Aug/16  Updated: 04/Aug/16

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

Type: Enhancement Priority: Trivial
Reporter: Antonin Hildebrand Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-1724.patch    

 Description   

While exploring CLJS record instances printed via cljs-devtools v0.8.0, I spotted that IIterable is not a fast-path protocol:

https://dl.dropboxusercontent.com/u/559047/iiterable-slow-path.png

Please notice that IIterable is the only white label in the protocol list there.






[CLJS-1722] Upgrade ExceptionInfo to proper deftype Created: 03/Aug/16  Updated: 04/Aug/16

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

Type: Enhancement Priority: Minor
Reporter: Antonin Hildebrand Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-1722.patch    
Patch: Code

 Description   

Currently ExceptionInfo is implemented as a raw constructor function which inherits from js/Error with some ad-hoc javascript-level patches to satisfy a tiny subset of deftype functionality (mainly for printing).

Unfortunately this does not play well with cljs-devtools[1]. This problem surfaced when I started playing with ExceptionInfo and cljs-devtools v0.8 which newly supports printing deftypes[2]. ExceptionInfo does not contain getBasis, cljs$lang$type, cljs$lang$ctorStr and similar machinery.

My proposed patch implements ExceptionInfo as a proper deftype and does some patch-work to provide backward compatibility. I'm pretty sure we must not break current contract of ExceptionInfo constructor accepting 3 args and synthesizing other fields on the fly in constructor.

Implementation details:
1) first we define ExceptionInfo as normal deftype (to get a template)
2) then we remember reference to ExceptionInfo in ExceptionInfoTypeTemplate
3) then we redefine ExceptionInfo with raw constructor function which should mimic original behaviour (by scraping newly created js/Error instance, but calling ExceptionInfoTypeTemplate to do proper deftype initialization)
4) then we copy keys from ExceptionInfoTypeTemplate over ExceptionInfo
5) then we set ExceptionInfo's prototype to be ExceptionInfoTypeTemplate's prototype
6) then we fix ExceptionInfo's prototype's constructor to point to our re-defined constructor function
7) then we patch ExceptionInfo's prototype to inherit from js/Error (note this clobbers ExceptionInfoTypeTemplate as well - but we don't care about it)

This effectively gives us properly working ExceptionInfo deftype with redefined constructor function wrapping deftype's constructor for backwards compatibility.
We also patch ExceptionInfo's prototype to inherit from js/Error the same was as original code did.

Note: With working deftype, we can move IPrintWithWriter and toString implementation to the deftype itself.

[1] https://github.com/binaryage/cljs-devtools/issues/23
[2] https://github.com/binaryage/cljs-devtools/releases/tag/v0.8.0



 Comments   
Comment by Thomas Heller [ 04/Aug/16 4:25 AM ]

Why not just add the missing getBasis, cljs$lang$type, cljs$lang$ctorStr bits per set!?

The patch looks like it would mess up advanced compilation although that is just an instinct not something I verified, did you?

Comment by Antonin Hildebrand [ 04/Aug/16 4:44 AM ]

I ran clojurescript tests and I assumed they run also against advanced-mode build. During development when my tests were failing I saw error messages about minified names.

This may seem as a hacky solution, but IMO the original code was also a hack. My hack will stay up-to-date with future changes to deftype implementation. I can imagine people would forget to update this part when touching deftype.

btw. there is another patch coming related to discrepancies between deftype and defrecord. That could have been avoided if defrecord shared common implementation with deftype.

Comment by Thomas Heller [ 04/Aug/16 6:27 AM ]

Closure is usually very strict about re-defining stuff but I guess my instincts were wrong, the tests should cover advanced.

My issue with this is that deftype is for defining Clojure-specific types. ExceptionInfo is not since it inherits from Error, just like you can't have a superclass in Clojure you can't in CLJS. So IF we were to change deftype in the future we might break things in unexpected ways that just re-use deftype but aren't actually deftype.

Yes, you have to do some house-keeping but you can't enforce the rules of deftype when dealing with inheritance.

Just my 2 cents, it has advantages to re-use deftype too (as you suggested).

Comment by Antonin Hildebrand [ 04/Aug/16 6:36 AM ]

Unfortunately I was unable to look up any comments or docs explaining the reasoning why we do that js/Error inheritance there.

My first attempt to "fix" ExceptionInfo was to simply implement it as an ordinary deftype. And that worked just fine (for my tests). Then I tried to re-implement original behaviours on top just to make it 100% compatible.

Comment by Antonin Hildebrand [ 04/Aug/16 12:47 PM ]

Just adding a motivational screenshot:

https://dl.dropboxusercontent.com/u/559047/CLJS-1722-example.png

Those yellow warnings are listing which properties are getting copied by gobject/extend call.
The expanded log item is new implementation logged via cljs-devtools v0.8.0.
The last log item is the old implementation logged via cljs-devtools v0.8.0 (cljs-devtools does not recognise ExceptionInfo as CLJS type, but detects IPrintWithWriter and uses it to present the value)





[CLJS-1600] Destructuring defprotocol fn args causes defrecord impls to silently fail Created: 11/Mar/16  Updated: 31/Jul/16

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

Type: Defect Priority: Minor
Reporter: James Henderson Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-1600-2.patch     Text File CLJS-1600.patch    
Patch: Code and Test

 Description   
(defprotocol IFoo
  (foo-fn [_ {:keys [a b] :as args}]))

(defrecord Foo []
  IFoo
  (foo-fn [_ {:keys [a b] :as args}]
    args))

(foo-fn (->Foo) {:a 1, :b 2})

returns

{:keys [1 2], :as {:a 1, :b 2}}

in CLJS, rather than

{:a 1, :b 2}

as Clojure does.

Redefining IFoo to

(defprotocol IFoo
  (foo-fn [_ args]))

causes the issue to go away.

While it's quite a minor bug, it was quite a hard one to track down, in practice - I didn't think to look at the protocol definition when the record was breaking, even after having used ClojureScript for a good few years!

Cheers,

James



 Comments   
Comment by António Nuno Monteiro [ 30/Jul/16 4:01 PM ]

Attached patch with fix and test.

Comment by Mike Fikes [ 31/Jul/16 1:54 PM ]

Hey António, the patch fails script/test-self-parity owing to the direct use of let and if-not outside of syntax-quote scope on line 1939. If you qualify them as core/let and core/if-not, then the tests pass in bootstrap.

Comment by António Nuno Monteiro [ 31/Jul/16 5:01 PM ]

Thanks for the feedback, Mike. I might have forgotten to run the self-parity test.
Attached CLJS-1600-2.patch which passes all tests.





[CLJS-1474] Error if reserved symbol is defined Created: 21/Oct/15  Updated: 31/Jul/16

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

Type: Enhancement Priority: Minor
Reporter: Martin Klepsch Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: newbie

Attachments: Text File CLJS-1474.patch    
Patch: Code and Test

 Description   

Currently a definition like

(defn set! [] ...)

will not cause any warning. Any usage of it (without :as namespace aliasing) however will not use the defined var but the set! special form.

A warning seems appropriate.



 Comments   
Comment by António Nuno Monteiro [ 30/Jul/16 1:19 PM ]

Attached patch with fix and test.

Comment by Mike Fikes [ 31/Jul/16 2:09 PM ]

I know David suggested that a hard error is probably the right thing to do for this one, but one consequence is that the cljs.spec/def macro cannot be defined in bootstrap with this change. (I haven't investigated thoroughly, but this may simply be the result of macros being processed as ClojureScript in bootstrap, and thus being subject to this new guard.)

Regardless of the root cause, you'll see this if you try to run script/test-self-parity:

#error {:message "Could not eval cljs.spec", :data {:tag :cljs/analysis-error}, :cause #error {:message "Can't def special form at line 51 ", :data {:file nil, :line 51, :column 1, :tag :cljs/analysis-error}}}

For reference: Line 51 currently points at the def macro: https://github.com/clojure/clojurescript/blob/e2db5d9ff8cb6a099ebc2a8cd379385bf4649b38/src/main/cljs/cljs/spec.cljc#L51





[CLJS-1636] Mark some symbols in core macros ns as private Created: 27/Apr/16  Updated: 31/Jul/16

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

Type: Enhancement Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-1636-2.patch     Text File CLJS-1636.patch    
Patch: Code

 Description   

There are some symbols in the core macros namespace that are not meant for external consumption. Some of these are marked private and some aren't. This ticket asks that the others be marked private as well.

An example of one symbol marked private is defcurried.
An example of one symbol not marked private is caching-hash.



 Comments   
Comment by Mike Fikes [ 27/Apr/16 8:21 AM ]

In CLJS-1636.patch, I checked and it appears nothing in the compiler codebase is explicitly using these symbols outside of the cljs.core namespace. But, it is still worth scanning through these to check if they make sense. For example js-debugger and js-comment are a couple that might actually be meant for public use, but it is difficult to tell.

Comment by Mike Fikes [ 27/Apr/16 2:43 PM ]

Note, that in #cljs-dev slack, there appears to be interest in caching-hash being public.

(I don't mind revising the patch to suit whatever is needed. At the same time, I'm certainly not in a position to take decisions on what is public API or not.)

Comment by Mike Fikes [ 27/Apr/16 2:43 PM ]

Note, that in #cljs-dev slack, there appears to be interest in caching-hash being public.

(I don't mind revising the patch to suit whatever is needed. At the same time, I'm certainly not in a position to take decisions on what is public API or not.)

Comment by Mike Fikes [ 31/Jul/16 1:33 PM ]

Patch no longer applies. Attaching updated patch.





[CLJS-1633] Improve error associated with invalid foreign-libs :file path Created: 26/Apr/16  Updated: 30/Jul/16

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

Type: Enhancement Priority: Minor
Reporter: Oliver George Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-1633.patch    
Patch: Code

 Description   

The current error reported when :advanced compiling with an invalid :foreign-libs :file path is effectively (slurp nil):

Caused by: java.lang.IllegalArgumentException: Cannot open <nil> as a Reader.

With a small patch this could be improved to provide a specific message and relevant context, something like:

Caused by: clojure.lang.ExceptionInfo: Unable to resolve url for foreign-deps source {:foreign true, :url nil, :source-url nil, :provides ["cljsjs.example-thing"], :requires (), :lines nil, :source-map nil, :file "broken-path-to-example-thing.js"}

I've created a simple repo based on the mies template to demonstrate the problem. Note that core.cljs requires the foriegn-lib which is defined in deps.clj (but with an invalid :file path). scripts/release.clj shows current behaviour. scripts/release-with-patch.clj shows proposed behaviour.

https://github.com/condense/apr26-foreign-libs-path-error.core.git

Below shows an isolated fix to cljs.closure/foreign-deps-str which checks for a nil url. An alternative approach might be to check at the point where the source maps are prepared (something like (assert (or url url-min) "xxx")) but this would be more likely to have side effects.

(defn foreign-deps-str [opts sources]
  (letfn [(to-js-str [ijs]
            (if-let [url (or (and (#{:advanced :simple} (:optimizations opts))
                                  (:url-min ijs))
                             (:url ijs))]
              (slurp url)
              (throw (ex-info "Unable to resolve url for foreign-deps source" ijs))))]
    (str (string/join "\n" (map to-js-str sources)) "\n")))

I'd be happy to prepare a patch if this seems like the right approach. Have signed contributor agreement.



 Comments   
Comment by David Nolen [ 06/May/16 1:56 PM ]

A patch for this small enhancement is welcome.

Comment by António Nuno Monteiro [ 30/Jul/16 4:38 PM ]

Attached patch with fix.





[CLJS-1490] Watch macro files in cljs.build.api/watch Created: 23/Nov/15  Updated: 30/Jul/16  Resolved: 30/Jul/16

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

Type: Enhancement Priority: Minor
Reporter: Sebastian Bensusan Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1490.patch    
Patch: Code

 Description   

The cljs.build.api/watch function is great for writing scripts or tooling but it doesn't track .clj macro files as noted in here. Direct users face unexpected behavior and tooling developers need to reimplement the watch functionality.

The undesired behavior can be tested with this repo (instructions there).

I propose to add the functionality into the watch function, or at least propose a mechanism so that tooling authors can reuse the functionality.



 Comments   
Comment by David Nolen [ 24/Nov/15 12:00 PM ]

Happy to see a fix for this.

Comment by António Nuno Monteiro [ 30/Jul/16 12:30 PM ]

Attached patch with fix.

Comment by David Nolen [ 30/Jul/16 12:39 PM ]

fixed https://github.com/clojure/clojurescript/commit/2267a8e5e9607aff8c940fa372dbdb35816350ba





[CLJS-1160] Source map js / cljs line number mixup Created: 23/Mar/15  Updated: 30/Jul/16  Resolved: 30/Jul/16

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

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: Unassigned
Resolution: Not Reproducible Votes: 0
Labels: None
Environment:

QuickStart Browser REPL Chrome OS X



 Description   

Note the source location for cljs.core.LazySeq.sval in the following 0.0-3126 stacktrace:

ClojureScript:cljs.user> (map ffirst (range))
Error: 0 is not ISeqable
	 cljs$core$seq (out/cljs/core.cljs:951:13)
	 cljs$core$first (out/cljs/core.cljs:960:7)
	 cljs$core$ffirst (out/cljs/core.cljs:1393:3)
	 cljs.core.map.cljs$core$map__2 (out/cljs/core.cljs:4046:30)
	 cljs.core.LazySeq.sval (out/cljs/core.cljs:11400:3)
	 cljs.core.LazySeq.cljs$core$ISeqable$_seq$arity$1 (out/cljs/core.cljs:2884:12)
	 cljs$core$seq (out/cljs/core.cljs:938:7)
	 cljs$core$pr_sequential_writer (out/cljs/core.cljs:8426:13)

It turns out that line 11400 is actually in core.js, but the source map logic is evidently getting tripped up by this one, and core.cljs gets reported with the JS line numbers (core.cljs doesn't even have that many lines).

Note that this also occurs on master (where CLJS-1154 and CLJS-1157 have landed):

ClojureScript:cljs.user> (map ffirst (range))
Error: 0 is not ISeqable
	 cljs.core/seq (out/cljs/core.cljs:951:20)
	 cljs.core/first (out/cljs/core.cljs:960:16)
	 cljs$core$ffirst (out/cljs/core.cljs:1393:11)
	 cljs.core.map.cljs$core$map__2 (out/cljs/core.cljs:4046:30)
	 cljs.core.LazySeq.sval (out/cljs/core.cljs:11400:3)
	 cljs.core.LazySeq.cljs$core$ISeqable$_seq$arity$1 (out/cljs/core.cljs:2884:12)
	 cljs.core/seq (out/cljs/core.cljs:938:25)
	 cljs$core$pr_sequential_writer (out/cljs/core.cljs:8426:20)


 Comments   
Comment by António Nuno Monteiro [ 30/Jul/16 9:32 AM ]

Can no longer repro on 1.9.89

cljs.user=> (map ffirst (range))
Error: 0 is not ISeqable
	 cljs.core/seq (out/cljs/core.cljs:1114:20)
	 cljs.core/first (out/cljs/core.cljs:1123:16)
	 cljs$core$ffirst (out/cljs/core.cljs:1634:11)
	 cljs.core.map.cljs$core$IFn$_invoke$arity$2 (out/cljs/core.cljs:4459:30)
	 cljs.core.LazySeq.sval (out/cljs/core.cljs:3217:18)
	 cljs.core.LazySeq.cljs$core$ISeqable$_seq$arity$1 (out/cljs/core.cljs:3271:12)
	 cljs.core/seq (out/cljs/core.cljs:1101:25)
	 cljs.core/pr-sequential-writer (out/cljs/core.cljs:9108:20)
	 cljs.core.LazySeq.cljs$core$IPrintWithWriter$_pr_writer$arity$3 (out/cljs/core.cljs:9344:35)




[CLJS-1719] Port destructuring namespaced keys and symbols Created: 29/Jul/16  Updated: 29/Jul/16  Resolved: 29/Jul/16

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

Type: Enhancement Priority: Major
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1719.patch    
Patch: Code and Test

 Description   

Port CLJ-1919 to Clojurescript



 Comments   
Comment by António Nuno Monteiro [ 29/Jul/16 6:44 PM ]

Attached patch with code and test.

Comment by David Nolen [ 29/Jul/16 8:46 PM ]

fixed https://github.com/clojure/clojurescript/commit/b44d0e07b5ff89c662d77a261d4d06bc7da92655





[CLJS-1653] cljs.spec: keys* causes exception Created: 29/May/16  Updated: 29/Jul/16  Resolved: 29/Jul/16

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

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

Attachments: Text File CLJS-1653-2.patch     Text File CLJS-1653.patch    
Patch: Code

 Description   

From https://clojure.org/guides/spec there is a keys* usage in the Entity Maps section. If tried with cljs.spec an exception is thrown:

cljs.user=> (require '[cljs.spec :as s])
nil
cljs.user=> (s/def ::server (s/keys* :req [::id ::host] :opt [::port]))
clojure.lang.ExceptionInfo: No method in multimethod 'parse' for dispatch value: & at line 1 <cljs repl> {:file "<cljs repl>", :line 1, :column 17, :tag :cljs/analysis-error}
	at clojure.core$ex_info.invokeStatic(core.clj:4617)
	at clojure.core$ex_info.invoke(core.clj:4617)
	at cljs.analyzer$error.invokeStatic(analyzer.cljc:580)
	at cljs.analyzer$error.invoke(analyzer.cljc:576)
	at cljs.analyzer$analyze_seq_STAR__wrap.invokeStatic(analyzer.cljc:2420)
	at cljs.analyzer$analyze_seq_STAR__wrap.invoke(analyzer.cljc:2419)
	at cljs.analyzer$analyze_seq.invokeStatic(analyzer.cljc:2442)
	at cljs.analyzer$analyze_seq.invoke(analyzer.cljc:2423)
	at cljs.analyzer$analyze_form.invokeStatic(analyzer.cljc:2555)
	at cljs.analyzer$analyze_form.invoke(analyzer.cljc:2551)
	at cljs.analyzer$analyze_STAR_.invokeStatic(analyzer.cljc:2602)
	at cljs.analyzer$analyze_STAR_.invoke(analyzer.cljc:2593)
	at cljs.analyzer$analyze.invokeStatic(analyzer.cljc:2618)
	at cljs.analyzer$analyze.invoke(analyzer.cljc:2605)
	at cljs.analyzer$analyze_seq.invokeStatic(analyzer.cljc:2443)
	at cljs.analyzer$analyze_seq.invoke(analyzer.cljc:2423)
	at cljs.analyzer$analyze_form.invokeStatic(analyzer.cljc:2555)
	at cljs.analyzer$analyze_form.invoke(analyzer.cljc:2551)
	at cljs.analyzer$analyze_STAR_.invokeStatic(analyzer.cljc:2602)
	at cljs.analyzer$analyze_STAR_.invoke(analyzer.cljc:2593)
	at cljs.analyzer$analyze.invokeStatic(analyzer.cljc:2618)
	at cljs.analyzer$analyze.invoke(analyzer.cljc:2605)
	at cljs.analyzer$analyze.invokeStatic(analyzer.cljc:2613)
	at cljs.analyzer$analyze.invoke(analyzer.cljc:2605)
	at cljs.analyzer$analyze.invokeStatic(analyzer.cljc:2612)
	at cljs.analyzer$analyze.invoke(analyzer.cljc:2605)
	at cljs.analyzer$parse_invoke_STAR_$ana_expr__2106.invoke(analyzer.cljc:2257)
	at clojure.core$map$fn__4785.invoke(core.clj:2646)
	at clojure.lang.LazySeq.sval(LazySeq.java:40)
	at clojure.lang.LazySeq.seq(LazySeq.java:49)
	at clojure.lang.Cons.next(Cons.java:39)
	at clojure.lang.PersistentVector.create(PersistentVector.java:73)
	at clojure.lang.LazilyPersistentVector.create(LazilyPersistentVector.java:44)
	at clojure.core$vec.invokeStatic(core.clj:377)
	at clojure.core$vec.invoke(core.clj:367)
	at cljs.analyzer$parse_invoke_STAR_.invokeStatic(analyzer.cljc:2264)
	at cljs.analyzer$parse_invoke_STAR_.invoke(analyzer.cljc:2235)
	at cljs.analyzer$parse_invoke.invokeStatic(analyzer.cljc:2273)
	at cljs.analyzer$parse_invoke.invoke(analyzer.cljc:2271)
	at cljs.analyzer$analyze_seq_STAR_.invokeStatic(analyzer.cljc:2417)
	at cljs.analyzer$analyze_seq_STAR_.invoke(analyzer.cljc:2414)
	at cljs.analyzer$analyze_seq_STAR__wrap.invokeStatic(analyzer.cljc:2421)
	at cljs.analyzer$analyze_seq_STAR__wrap.invoke(analyzer.cljc:2419)
	at cljs.analyzer$analyze_seq.invokeStatic(analyzer.cljc:2442)
	at cljs.analyzer$analyze_seq.invoke(analyzer.cljc:2423)
	at cljs.analyzer$analyze_form.invokeStatic(analyzer.cljc:2555)
	at cljs.analyzer$analyze_form.invoke(analyzer.cljc:2551)
	at cljs.analyzer$analyze_STAR_.invokeStatic(analyzer.cljc:2602)
	at cljs.analyzer$analyze_STAR_.invoke(analyzer.cljc:2593)
	at cljs.analyzer$analyze.invokeStatic(analyzer.cljc:2618)
	at cljs.analyzer$analyze.invoke(analyzer.cljc:2605)
	at cljs.analyzer$analyze_seq.invokeStatic(analyzer.cljc:2443)
	at cljs.analyzer$analyze_seq.invoke(analyzer.cljc:2423)
	at cljs.analyzer$analyze_form.invokeStatic(analyzer.cljc:2555)
	at cljs.analyzer$analyze_form.invoke(analyzer.cljc:2551)
	at cljs.analyzer$analyze_STAR_.invokeStatic(analyzer.cljc:2602)
	at cljs.analyzer$analyze_STAR_.invoke(analyzer.cljc:2593)
	at cljs.analyzer$analyze.invokeStatic(analyzer.cljc:2618)
	at cljs.analyzer$analyze.invoke(analyzer.cljc:2605)
	at cljs.repl$evaluate_form.invokeStatic(repl.cljc:450)
	at cljs.repl$evaluate_form.invoke(repl.cljc:440)
	at cljs.repl$eval_cljs.invokeStatic(repl.cljc:570)
	at cljs.repl$eval_cljs.invoke(repl.cljc:563)
	at cljs.repl$repl_STAR_$read_eval_print__6148.invoke(repl.cljc:875)
	at cljs.repl$repl_STAR_$fn__6154$fn__6163.invoke(repl.cljc:914)
	at cljs.repl$repl_STAR_$fn__6154.invoke(repl.cljc:913)
	at cljs.compiler$with_core_cljs.invokeStatic(compiler.cljc:1154)
	at cljs.compiler$with_core_cljs.invoke(compiler.cljc:1145)
	at cljs.repl$repl_STAR_.invokeStatic(repl.cljc:877)
	at cljs.repl$repl_STAR_.invoke(repl.cljc:761)
	at cljs.repl$repl.invokeStatic(repl.cljc:995)
	at cljs.repl$repl.doInvoke(repl.cljc:925)
	at clojure.lang.RestFn.invoke(RestFn.java:410)
	at user$eval6335.invokeStatic(NO_SOURCE_FILE:3)
	at user$eval6335.invoke(NO_SOURCE_FILE:3)
	at clojure.lang.Compiler.eval(Compiler.java:6927)
	at clojure.lang.Compiler.eval(Compiler.java:6890)
	at clojure.core$eval.invokeStatic(core.clj:3105)
	at clojure.main$eval_opt.invokeStatic(main.clj:288)
	at clojure.main$eval_opt.invoke(main.clj:282)
	at clojure.main$initialize.invokeStatic(main.clj:308)
	at clojure.main$null_opt.invokeStatic(main.clj:342)
	at clojure.main$null_opt.invoke(main.clj:339)
	at clojure.main$main.invokeStatic(main.clj:421)
	at clojure.main$main.doInvoke(main.clj:384)
	at clojure.lang.RestFn.invoke(RestFn.java:421)
	at clojure.lang.Var.invoke(Var.java:383)
	at clojure.lang.AFn.applyToHelper(AFn.java:156)
	at clojure.lang.Var.applyTo(Var.java:700)
	at clojure.main.main(main.java:37)
Caused by: java.lang.IllegalArgumentException: No method in multimethod 'parse' for dispatch value: &
	at clojure.lang.MultiFn.getFn(MultiFn.java:156)
	at clojure.lang.MultiFn.invoke(MultiFn.java:251)
	at cljs.analyzer$analyze_seq_STAR_.invokeStatic(analyzer.cljc:2416)
	at cljs.analyzer$analyze_seq_STAR_.invoke(analyzer.cljc:2414)
	at cljs.analyzer$analyze_seq_STAR__wrap.invokeStatic(analyzer.cljc:2421)
	... 85 more


 Comments   
Comment by Mike Fikes [ 30/May/16 10:55 AM ]

The attached patch works around the issue by qualifying the reference to cljs.spec/&.

With it, the example in the docstring works:

(s/conform (s/cat :i1 integer? :m (s/keys* :req-un [::a ::c]) :i2 integer?) [42 :a 1 :c 2 :d 4 99])
{:i1 42, :m {:a 1, :c 2, :d 4}, :i2 99}

(There is probably a more correct solution that probably involves changing the analyzer, which would lead to users being able to refer & and use it as (& ...), but this patch would get us by for now if desired.)

Comment by Mike Fikes [ 27/Jul/16 10:05 AM ]

Original patch no longer applies; attaching CLJS-1653-2.patch.

Comment by David Nolen [ 29/Jul/16 3:13 PM ]

fixed https://github.com/clojure/clojurescript/commit/5a78c9fb881f08b1f9578f9cd9dd265bbdd5c720





[CLJS-1700] Support clojure.* aliasing when not in vector Created: 03/Jul/16  Updated: 29/Jul/16  Resolved: 29/Jul/16

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

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1700.patch    
Patch: Code and Test

 Description   

While this works:

(ns foo.core
  (:require [clojure.test]))

this does not:

(ns foo.core
  (:require clojure.test))


 Comments   
Comment by António Nuno Monteiro [ 27/Jul/16 10:59 AM ]

Attached patch with fix and test.

Comment by David Nolen [ 29/Jul/16 3:07 PM ]

fixed https://github.com/clojure/clojurescript/commit/689f6d3197a710daf55aa234b81e4468afd9ff9e





[CLJS-1717] remove unnecessary map from equiv-map Created: 29/Jul/16  Updated: 29/Jul/16  Resolved: 29/Jul/16

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

Type: Enhancement Priority: Trivial
Reporter: Thomas Mulvaney Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1717.patch    
Patch: Code

 Description   

Simplifies code and removes a sequence allocation speeding up equiv-map.



 Comments   
Comment by David Nolen [ 29/Jul/16 3:05 PM ]

fixed https://github.com/clojure/clojurescript/commit/82dce0b1ed322af25e1ebe2cf91e3afec9e249ec





[CLJS-1716] No longer possible to use same alias for :require-macros and :require Created: 27/Jul/16  Updated: 29/Jul/16  Resolved: 29/Jul/16

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

Type: Defect Priority: Minor
Reporter: Juho Teperi Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None
Environment:

1.9.146 / 8643c55


Attachments: Text File CLJS-1716.patch    
Patch: Code

 Description   

It was possible possible to use same alias for :require-macros and :require:

(ns hello-world.core
  (:require-macros [cljs.core.async.macros :as a])
  (:require [cljs.core.async :as a]))

(a/go
  (a/<! (a/timeout 1000))
  (println "hello"))

This works with 1.9.89 but on 1.9.146 this causes a warning:

WARNING: Use of undeclared Var cljs.core.async/go at line 5 src/hello_world/core.cljs

According to anmonteiro at Slack, this is caused by commit https://github.com/clojure/clojurescript/commit/41a62bfd916208d30bf621aefba56c5dce031ce5

Based on discussion at Slack, this feature is non intentional and fixing this is low-priority.

Minimal test project is available at https://github.com/Deraen/problems/tree/cljs-1692-same-alias
This still uses lein to retrieve deps as using core.async with cljs.jar proved complicated.



 Comments   
Comment by António Nuno Monteiro [ 27/Jul/16 6:27 PM ]

Attached patch with fix

Comment by Juho Teperi [ 28/Jul/16 4:40 AM ]

I can confirm that the patch works.

Comment by David Nolen [ 29/Jul/16 3:02 PM ]

fixed https://github.com/clojure/clojurescript/commit/82a2a293e7b0b30f78399f997ec26d808473b8e2





[CLJS-1718] Foreign lib files should be placed in a location that matches their namespace Created: 29/Jul/16  Updated: 29/Jul/16

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

Type: Defect Priority: Minor
Reporter: António Nuno Monteiro Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Affects master



 Description   

Using several foreign libs with the same file name ends up just including one of them, as the files are placed at the root of the `:output-dir`.

A solution for this would be placing those files in a location that matches their `:provides` namespace.






[CLJS-1223] cljs.repl/doc for unqualified .. macro Created: 24/Apr/15  Updated: 29/Jul/16  Resolved: 29/Jul/16

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

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None
Environment:

Note: Also affects 0.0-3211 (not yet available in JIRA pulldown)
Quick Start cljs.jar
OS X



 Description   

In the REPL, (doc ..) fails, while it succeeds for similar macros like (doc ->), and it also succeeds for ns-qualified: (doc cljs.core/..).

$ java -jar cljs.jar -m cljs.repl.node
ClojureScript Node.js REPL server listening on 54359
To quit, type: :cljs/quit
cljs.user=> *clojurescript-version*
"0.0-3211"
cljs.user=> (doc ..)
-------------------------
/.
  nil

nil
cljs.user=> (doc cljs.core/..)
-------------------------
cljs.core/..
([x form] [x form & more])
Macro
  form => fieldName-symbol or (instanceMethodName-symbol args*)

  Expands into a member access (.) of the first member on the first
  argument, followed by the next member on the result, etc. For
  instance:

  (.. System (getProperties) (get "os.name"))

  expands to:

  (. (. System (getProperties)) (get "os.name"))

  but is easier to write, read, and understand.

nil


 Comments   
Comment by António Nuno Monteiro [ 29/Jul/16 6:42 AM ]

Can no longer repro

$ java -jar cljs.jar -m cljs.repl.node
ClojureScript Node.js REPL server listening on 52864
To quit, type: :cljs/quit
cljs.user=> *clojurescript-version*
"1.9.89"
cljs.user=> (doc ..)
-------------------------
cljs.core/..
([x form] [x form & more])
Macro
  form => fieldName-symbol or (instanceMethodName-symbol args*)

  Expands into a member access (.) of the first member on the first
  argument, followed by the next member on the result, etc. For
  instance:

  (.. System (getProperties) (get "os.name"))

  expands to:

  (. (. System (getProperties)) (get "os.name"))

  but is easier to write, read, and understand.

nil
cljs.user=>
Comment by David Nolen [ 29/Jul/16 7:41 AM ]

fixed





[CLJS-1241] Add cljs.core/boolean? predicate Created: 03/May/15  Updated: 29/Jul/16  Resolved: 29/Jul/16

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

Type: Enhancement Priority: Minor
Reporter: Brandon Bloom Assignee: Unassigned
Resolution: Completed Votes: 2
Labels: patch

Attachments: Text File CLJS-1241_v01.patch    
Patch: Code and Test

 Description   

I'm constantly re-implementing this predicate...

It's also important for clj/cljs compatibility.



 Comments   
Comment by Brandon Bloom [ 03/May/15 2:32 PM ]

See also: http://dev.clojure.org/jira/browse/CLJ-1719

Comment by Mike Fikes [ 03/May/15 2:52 PM ]

I tried this patch and it works correctly for me.

The docstring uses "typeof". Perhaps "type of" was intended?

cljs.user=> (doc boolean?)
-------------------------
cljs.core/boolean?
([x])
  Returns true if the typeof x is boolean, false otherwise.

nil
Comment by Brandon Bloom [ 03/May/15 7:41 PM ]

I did mean "typeof", which is a javascript-ism. A better doc string may be less javascript-specific...

Comment by Mike Fikes [ 31/Jan/16 3:17 PM ]

Patch CLJS-1241_v01.patch no longer applies on master.

Comment by Brandon Bloom [ 31/Jan/16 7:13 PM ]

Not going to bother updating patch until there is movement on http://dev.clojure.org/jira/browse/CLJ-1719

Comment by António Nuno Monteiro [ 29/Jul/16 7:06 AM ]

This has been implemented in Clojurescript as of https://github.com/clojure/clojurescript/commit/73ab8ff8f4610a6f11cf64cc09e7173dcada5dc0

Comment by David Nolen [ 29/Jul/16 7:41 AM ]

Added to Clojure 1.9.0 alphas and ClojureScript 1.9.X





[CLJS-1683] js->clj passes opts incorrectly Created: 15/Jun/16  Updated: 29/Jul/16  Resolved: 29/Jul/16

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

Type: Defect Priority: Trivial
Reporter: Oliver George Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None


 Description   

This seems be a bug but nil-punning makes it work.

A good reason for fixing is that inspecting the code leads you to think the calling convention is different.

([x] (js->clj x {:keywordize-keys false}))

should be this (remove map)

([x] (js->clj x :keywordize-keys false))

in

(defn js->clj
  "Recursively transforms JavaScript arrays into ClojureScript
  vectors, and JavaScript objects into ClojureScript maps.  With
  option ':keywordize-keys true' will convert object fields from
  strings to keywords."
  ([x] (js->clj x {:keywordize-keys false}))
  ([x & opts]
    (let [{:keys [keywordize-keys]} opts
          keyfn (if keywordize-keys keyword str)
          f (fn thisfn [x]

So calling (js->clj x) becomes (js->clj x {:keywordize-keys false}) which means opts is [{:keywordize-keys false}] but is destructured as a map.

Result is keywordize-keys is nil rather than false. No drama.

(It got me because I was unsure how to call with :keywordize-keys set. I looked at that code, swapped false with true and was confused when it didn't work. (keywordized-keys was still nil.)



 Comments   
Comment by António Nuno Monteiro [ 29/Jul/16 7:24 AM ]

This has been fixed in https://github.com/clojure/clojurescript/commit/0fcbef2bf6be543ce72de4797701fdd55b332922

Comment by David Nolen [ 29/Jul/16 7:40 AM ]

fixed





[CLJS-1681] Make instrument-all / unstrument-all return nil Created: 11/Jun/16  Updated: 27/Jul/16  Resolved: 27/Jul/16

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

Type: Enhancement Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Not Reproducible Votes: 0
Labels: None

Attachments: Text File CLJS-1681.patch    
Patch: Code and Test

 Description   

In Clojure, instrument-all and unstrument-all return nil, but in ClojureScript will return the last var instrumented / unstrumented by the resulting macroexpansion.



 Comments   
Comment by Mike Fikes [ 11/Jun/16 6:01 PM ]

The attached CLJS-1681 broadens the scope a little and also takes care of instrument-ns / unstrument-ns.

Comment by Mike Fikes [ 27/Jul/16 9:29 AM ]

The attached patch no longer applies (not just mechanically but also because the functions involved have evolved while tracking Clojure).





[CLJS-1297] defrecord does not emit IKVReduce protocol Created: 03/Jun/15  Updated: 27/Jul/16

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

Type: Defect Priority: Major
Reporter: Daniel Skarda Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: newbie

Attachments: Text File CLJS-1297-19-July-2015.patch    

 Description   

Records are maps and in Clojure they support reduce-kv (IKVReduce protocol).
This is not true in ClojureScript:

(defrecord Foobar [x y])
 (reduce-kv assoc {} (Foobar. 1 2))

Fails wit Error: No protocol method IKVReduce.-kv-reduce defined for type : [object Object]



 Comments   
Comment by David Nolen [ 03/Jun/15 7:25 PM ]

Just seems like an oversight. Patch welcome, this one is a relatively easy one.

Comment by Daniel Skarda [ 04/Jun/15 2:53 AM ]

OK

I checked Clojure implementation. Records do not implement any reduce protocol on their own. For IKVReduce records use default implementation using reduce and destructuring. Is this approach OK?

Recently Alex Miller implemented many optimizations of reduce protocols in Clojure. Eg range returns an object which implements IReduce protocol so reduce (and transducers in general) can take advantage of it. Any plans for such optimizations in ClojureScript?

;;clojure/src/clj/clojure/core.clj:6523
;;slow path default
clojure.lang.IPersistentMap
(kv-reduce 
  [amap f init]
  (reduce (fn [ret [k v]] (f ret k v)) init amap))
Comment by David Nolen [ 04/Jun/15 9:05 AM ]

Going with the Clojure implementation is fine. Yes all of the optimizations in 1.7.0 are on the table for ClojureScript but these are separate issues from this one.

Comment by Samuel Miller [ 16/Jul/15 10:39 PM ]

Mind if I take this as my first cljs bug? Poking around quickly I think I know what needs to happen.

Comment by David Nolen [ 17/Jul/15 5:21 AM ]

Sure! Have you submitted your CA yet?

Comment by Samuel Miller [ 17/Jul/15 7:13 PM ]

Yes, I did yesterday.

Comment by Samuel Miller [ 20/Jul/15 9:52 PM ]

Here is a potential patch. I implemented a basic IKVreduce based on Daniel Skarda's comment. Note: I am a little fuzzy on macros still so please look over what I have. There is probably a better way. Also added a test for reduce-kv on records.

I ran the test on Linux on V8 and SpiderMonkey. I plan to get JSC and Nashorn working and tested this week but if someone wants to test them out before that would be great.

Comment by Sebastian Bensusan [ 23/Jul/15 6:45 PM ]

Experience report:

I just tested the patch in the Node Repl and it seems to work:

cljs.user=> (defrecord A [a b])
cljs.user/A
cljs.user=> (reduce-kv (fn [m k v] (assoc m k (inc v))) {} (A. 1 2))
{:a 2, :b 3}

and the provided tests passed in Spidermonkey, V8, and Nashorn (I don't have JSC installed).

For completeness: before applying the patch the same code fails with:

Error: No protocol method IKVReduce.-kv-reduce defined for type : [object Object]
Comment by David Nolen [ 10/Aug/15 10:22 PM ]

Is this the same approach taken by Clojure?

Comment by Samuel Miller [ 10/Aug/15 10:36 PM ]

You can see the relevant current Clojure code here...
https://github.com/clojure/clojure/blob/master/src/clj/clojure/core.clj#L6526
I think it is the same. I literally just tried to translate it over into CLJS. I might of understood something wrong though.

Comment by David Nolen [ 11/Aug/15 6:10 AM ]

Yes that's the slow path. Please use the implementation used by defrecord instead. If defrecord doesn't have one then this patch is OK.

Comment by Samuel Miller [ 11/Aug/15 8:48 PM ]

As far as I can tell there is no implementation on defrecord itself however there are separate implementations on the the java classes PersistentVector, PersistentArrayMap, PersistentTreeMap, and PersistenHashMap in pure java. I am not sure if you would want to do something similar for Clojurescript.

I can also spend some time trying to make a more performant version.

Comment by António Nuno Monteiro [ 27/Jul/16 7:38 AM ]

Confirmed that Clojure uses the slow path via the IPersistentMap implementation in defrecord
https://github.com/clojure/clojure/blob/d920ad/src/clj/clojure/core.clj#L6712

Patch still applies and can also confirm it works for me.





[CLJS-1714] Clojurescript github README.md should identify source Clojure version Created: 24/Jul/16  Updated: 26/Jul/16

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

Type: Enhancement Priority: Trivial
Reporter: Marshall Abrams Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

It would be helpful if the Clojurescript README at https://github.com/clojure/clojurescript/blob/master/README.md explicitly identified the Clojure version on which the current stable release of Clojurescript is based, or the Clojure version(s) which the Clojurescript release most closely tracks. This is especially helpful during periods when the Clojurescript release recommended in README.md is based on a Clojure alpha or even beta version, since there may be significant changes between Clojure versions in this case, so users might expect more similarity between the latest public releases of the two dialects than is warranted. Of course one can also go through changes.md to figure out what Clojure version is the basis of each Clojurescript release, but I think it's worth making it easier for users to notice.



 Comments   
Comment by David Nolen [ 25/Jul/16 2:08 PM ]

ClojureScript depends only on final releases of Clojure. This dependency is expressed in the pom.xml file which Maven based tooling and its users already understand. I don't see a compelling reason to include yet another thing we have to remember to update in the README.

Comment by Marshall Abrams [ 25/Jul/16 9:46 PM ]

First, I'm not sure if what I wrote was clear. I'm not talking about a dependency on the Clojure version used to compile to Javascript. I'm talking about the relationship between a Clojurescript version and the Clojure version that was ... ported to the Clojurescript release.

I agree it would a pain to try to remember to update this each time the Clojurescript version changed. That was one of my reservations, but I thought the point was important enough-especially for new users-that it seemed worth suggesting. I suppose that if there was already a line in the README about it, that will help, but I know that it's still easy to forget to update that kind of thing. Can't be helped.

Here are some reasons to consider updating the README with the Clojure version(s) that are the "source" of a Clojurescript version:

It's very useful information what version of Clojure the current Clojurescript release is most similar to. That way, information about the Clojure version is useful for Clojurescript.

At present, the official releases of Clojurescript listed in the README is based on what are currently alpha Clojure releases. Since the Clojure releases are alpha releases, they're changing quickly, so whatever's documented on the Clojure website may be out of sync with the Clojurescript release. One needs to know if that's so. (If all I want from the current Clojurescript release is what's in the previous non-alpha/beta Clojure release, knowing the Clojure version that was the source probably doesn't matter if there have been no breaking changes in the alpha releases on which the current Clojurescript version is based, but if I want to experiment in Clojurescript with new Clojure features-spec in this case-I need to know if it might differ from the latest Clojure version. (explain-data changed from Clojure 1.9.0-alpha7 to alpha10, at least.)

(I think that the comment about Maven and pom.xml was based on assuming that I was talking about the Clojure version used to compile Clojurescript code. If so, you can stop reading this paragraph. If not, then I would say that though Maven may be routinely used by Clojurescript developers-I don't know-but Clojurescript has grown beyond a core community of hardcore users and developers. It's used for a lot of real work, and it's a good language for new users coming from Clojure or Javascript. There are books for sale, and good online tutorials. The wiki is quite good for new users now. All of the templates on the wiki use Leiningen or Boot. There's no mention of Maven. There is no need for most users to use Maven with Clojurescript, or to even know what a pom file is. In my case, I do know what a pom file is, and have read quite a few, but wouldn't have thought to look there. I've never used Maven, however. Leiningen has been enough.)

Those are my thoughts on the matter. Thanks much for considering it.





[CLJS-1540] Arity-1 version of js->clj should pass keyword arguments for default options, as expected by js->clj Created: 07/Jan/16  Updated: 22/Jul/16  Resolved: 22/Jul/16

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

Type: Defect Priority: Minor
Reporter: Nicolás Berger Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File js-clj-keyword-opts.patch    
Patch: Code

 Description   

Arity-1 version of js->clj is passing the map {:keywordize-keys false} as the default options of js->clj, when it should pass the keyword arguments :keywordize-keys false. It's working by luck, because keywordize-keys ends being nil by default, which is falsey. But it's confusing for anyone reading the code and trying to pass {:keywordize-keys true} with the expectation that it would keywordize the keys.

References: http://dev.clojure.org/jira/browse/CLJS-750 and https://groups.google.com/forum/#!topic/clojurescript/Dis6845WL5U



 Comments   
Comment by Mike Fikes [ 29/Jan/16 7:59 PM ]
Comment by David Nolen [ 04/Feb/16 4:05 PM ]

Patch looks OK, Nicolas have you submitted your CA? Thanks.

Comment by Nicolás Berger [ 04/Feb/16 5:00 PM ]

Yes I did, David

Comment by Martin Klepsch [ 21/Jul/16 6:13 AM ]

Just stumbled upon this as well so bump

Comment by David Nolen [ 22/Jul/16 7:36 AM ]

fixed https://github.com/clojure/clojurescript/commit/0fcbef2bf6be543ce72de4797701fdd55b332922





[CLJS-1709] clojure.data/diff throws an exception when comparing map keys of different types when used on sorted maps Created: 19/Jul/16  Updated: 22/Jul/16

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

Type: Defect Priority: Minor
Reporter: Thomas Scheiblauer Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: bug


 Description   

e.g.
(clojure.data/diff (sorted-map :foo 42) (sorted-map 1 42))
(clojure.data/diff (sorted-map :foo 42) (sorted-map "x" 42))
(clojure.data/diff (hash-map :foo 42) (sorted-map 1 42))
(clojure.data/diff (hash-map :foo 42) (sorted-map "x" 42))
will throw e.g.
Error: Cannot compare :foo to 1
while e.g.
(clojure.data/diff (hash-map :foo 42) (hash-map 1 42))
(clojure.data/diff (hash-map :foo 42) (hash-map "x" 2))
(clojure.data/diff (sorted-map :foo 42) (sorted-map :bar 42))
will not.

The same applies to Clojure with a different exception (e.g. "java.lang.Long cannot be cast to clojure.lang.Keyword")






[CLJS-1711] Compiler analysis cache issue with regex and Transit Created: 21/Jul/16  Updated: 22/Jul/16  Resolved: 22/Jul/16

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

Type: Defect Priority: Minor
Reporter: Rohit Aggarwal Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None
Environment:

Mac OS X. Bug reproduced on master branch and latest release


Attachments: Text File CLJS-1711.patch    
Patch: Code

 Description   

This issue was reported by Martin Klepsch on Slack.

The following code causes the compiler to throw an error if transit-clj is included as a dependency.

core.cljs
(ns hello-world.core)

(enable-console-print!)

(defn x [{:keys [extract]
          :or {extract #"\w"}}])
build.clj
(require 'cljs.build.api)

(cljs.build.api/build "src" {:output-to "out/main.js"})

I used the following command to compile the code:

java -cp cljs.jar:transit-java-0.8.311.jar:transit-clj-0.8.285.jar:src clojure.main build.clj

The error stack trace is:

Exception in thread "main" clojure.lang.ExceptionInfo: failed compiling file:src/hello_world/core.cljs {:file #object[java.io.File 0xaed0151 "src/hello_world/core.cljs"]}, compiling:(....build.clj:3:1)
	at clojure.lang.Compiler.load(Compiler.java:7391)
	at clojure.lang.Compiler.loadFile(Compiler.java:7317)
	at clojure.main$load_script.invokeStatic(main.clj:275)
	at clojure.main$script_opt.invokeStatic(main.clj:335)
	at clojure.main$script_opt.invoke(main.clj:330)
	at clojure.main$main.invokeStatic(main.clj:421)
	at clojure.main$main.doInvoke(main.clj:384)
	at clojure.lang.RestFn.invoke(RestFn.java:408)
	at clojure.lang.Var.invoke(Var.java:379)
	at clojure.lang.AFn.applyToHelper(AFn.java:154)
	at clojure.lang.Var.applyTo(Var.java:700)
	at clojure.main.main(main.java:37)
Caused by: clojure.lang.ExceptionInfo: failed compiling file:src/hello_world/core.cljs {:file #object[java.io.File 0xaed0151 "src/hello_world/core.cljs"]}
	at clojure.core$ex_info.invokeStatic(core.clj:4617)
	at cljs.compiler$compile_file$fn__2840.invoke(compiler.cljc:1368)
	at cljs.compiler$compile_file.invokeStatic(compiler.cljc:1338)
	at cljs.closure$compile_file.invokeStatic(closure.clj:471)
	at cljs.closure$fn__3897.invokeStatic(closure.clj:538)
	at cljs.closure$fn__3897.invoke(closure.clj:534)
	at cljs.closure$fn__3839$G__3832__3846.invoke(closure.clj:430)
	at cljs.closure$compile_sources$iter__4008__4012$fn__4013.invoke(closure.clj:870)
	at clojure.lang.LazySeq.sval(LazySeq.java:40)
	at clojure.lang.LazySeq.seq(LazySeq.java:49)
	at clojure.lang.Cons.next(Cons.java:39)
	at clojure.lang.RT.next(RT.java:688)
	at clojure.core$next__4341.invokeStatic(core.clj:64)
	at clojure.core$dorun.invokeStatic(core.clj:3033)
	at clojure.core$doall.invokeStatic(core.clj:3039)
	at cljs.closure$compile_sources.invokeStatic(closure.clj:867)
	at cljs.closure$build.invokeStatic(closure.clj:1995)
	at cljs.build.api$build.invokeStatic(api.clj:209)
	at cljs.build.api$build.invoke(api.clj:198)
	at cljs.build.api$build.invokeStatic(api.clj:201)
	at cljs.build.api$build.invoke(api.clj:198)
	at user$eval24.invokeStatic(build.clj:3)
	at user$eval24.invoke(build.clj:3)
	at clojure.lang.Compiler.eval(Compiler.java:6927)
	at clojure.lang.Compiler.load(Compiler.java:7379)
	... 11 more
Caused by: java.lang.RuntimeException: java.lang.Exception: Not supported: class java.util.regex.Pattern
	at com.cognitect.transit.impl.WriterFactory$1.write(WriterFactory.java:129)
	at cognitect.transit$write.invokeStatic(transit.clj:149)
	at cognitect.transit$write.invoke(transit.clj:146)
	at cljs.analyzer$write_analysis_cache.invokeStatic(analyzer.cljc:2871)
	at cljs.compiler$emit_source.invokeStatic(compiler.cljc:1268)
	at cljs.compiler$compile_file_STAR_$fn__2817.invoke(compiler.cljc:1287)
	at cljs.compiler$with_core_cljs.invokeStatic(compiler.cljc:1154)
	at cljs.compiler$compile_file_STAR_.invokeStatic(compiler.cljc:1278)
	at cljs.compiler$compile_file$fn__2840.invoke(compiler.cljc:1358)
	... 34 more
Caused by: java.lang.Exception: Not supported: class java.util.regex.Pattern
	at com.cognitect.transit.impl.AbstractEmitter.marshal(AbstractEmitter.java:176)
	at com.cognitect.transit.impl.JsonEmitter.emitMap(JsonEmitter.java:158)
	at com.cognitect.transit.impl.AbstractEmitter.emitMap(AbstractEmitter.java:70)
	at com.cognitect.transit.impl.AbstractEmitter.marshal(AbstractEmitter.java:166)
	at com.cognitect.transit.impl.JsonEmitter.emitMap(JsonEmitter.java:158)
	at com.cognitect.transit.impl.AbstractEmitter.emitMap(AbstractEmitter.java:70)
	at com.cognitect.transit.impl.AbstractEmitter.marshal(AbstractEmitter.java:166)
	at com.cognitect.transit.impl.AbstractEmitter.emitArray(AbstractEmitter.java:82)
	at com.cognitect.transit.impl.AbstractEmitter.marshal(AbstractEmitter.java:164)
	at com.cognitect.transit.impl.AbstractEmitter.emitArray(AbstractEmitter.java:87)
	at com.cognitect.transit.impl.AbstractEmitter.marshal(AbstractEmitter.java:164)
	at com.cognitect.transit.impl.AbstractEmitter.emitTagged(AbstractEmitter.java:34)
	at com.cognitect.transit.impl.AbstractEmitter.emitEncoded(AbstractEmitter.java:59)
	at com.cognitect.transit.impl.AbstractEmitter.marshal(AbstractEmitter.java:169)
	at com.cognitect.transit.impl.AbstractEmitter.emitArray(AbstractEmitter.java:87)
	at com.cognitect.transit.impl.AbstractEmitter.marshal(AbstractEmitter.java:164)
	at com.cognitect.transit.impl.AbstractEmitter.emitTagged(AbstractEmitter.java:34)
	at com.cognitect.transit.impl.AbstractEmitter.emitEncoded(AbstractEmitter.java:59)
	at com.cognitect.transit.impl.AbstractEmitter.marshal(AbstractEmitter.java:169)
	at com.cognitect.transit.impl.JsonEmitter.emitMap(JsonEmitter.java:158)
	at com.cognitect.transit.impl.AbstractEmitter.emitMap(AbstractEmitter.java:70)
	at com.cognitect.transit.impl.AbstractEmitter.marshal(AbstractEmitter.java:166)
	at com.cognitect.transit.impl.JsonEmitter.emitMap(JsonEmitter.java:158)
	at com.cognitect.transit.impl.AbstractEmitter.emitMap(AbstractEmitter.java:70)
	at com.cognitect.transit.impl.AbstractEmitter.marshal(AbstractEmitter.java:166)
	at com.cognitect.transit.impl.JsonEmitter.emitMap(JsonEmitter.java:158)
	at com.cognitect.transit.impl.AbstractEmitter.emitMap(AbstractEmitter.java:70)
	at com.cognitect.transit.impl.AbstractEmitter.marshal(AbstractEmitter.java:166)
	at com.cognitect.transit.impl.JsonEmitter.emitMap(JsonEmitter.java:158)
	at com.cognitect.transit.impl.AbstractEmitter.emitMap(AbstractEmitter.java:70)
	at com.cognitect.transit.impl.AbstractEmitter.marshal(AbstractEmitter.java:166)
	at com.cognitect.transit.impl.AbstractEmitter.marshalTop(AbstractEmitter.java:193)
	at com.cognitect.transit.impl.JsonEmitter.emit(JsonEmitter.java:28)
	at com.cognitect.transit.impl.WriterFactory$1.write(WriterFactory.java:126)
	... 42 more

It works perfectly fine if the transit dependency is removed.



 Comments   
Comment by Rohit Aggarwal [ 21/Jul/16 2:56 AM ]

A related issue is that for the compiler analysis cache, for edn, the code is using pr-str to output, which handles regular expressions just fine. But AFAIK, a regex isn't a type supported by edn and edn/read-string doesn't work with the string outputted by pr-str.

Not sure what to do with that.

Comment by David Nolen [ 22/Jul/16 7:34 AM ]

fixed https://github.com/clojure/clojurescript/commit/b07ba518ff8d17a65d092523364c0a9c1804af3a





[CLJS-1576] cljs.js sourcemap support throws on non-latin1 characters Created: 17/Feb/16  Updated: 22/Jul/16  Resolved: 22/Jul/16

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

Type: Defect Priority: Minor
Reporter: Matt Huebert Assignee: Matt Huebert
Resolution: Completed Votes: 0
Labels: bootstrap

Attachments: Text File CLJS-1576.patch     Text File CLJS-1576-rebased.patch    
Patch: Code

 Description   

In cljs.js/append-source-map we encode the source-map string in base64 without escaping non-latin1 characters. In Chrome, this throws the error: "DOMException: Failed to execute 'btoa' on 'Window': The string to be encoded contains characters outside of the Latin1 range."

Source: https://github.com/clojure/clojurescript/blob/master/src/main/cljs/cljs/js.cljs#L152

The problem & a couple of solutions are explained here: https://developer.mozilla.org/en-US/docs/Web/API/WindowBase64/Base64_encoding_and_decoding#The_Unicode_Problem



 Comments   
Comment by David Nolen [ 18/Feb/16 8:21 AM ]

Can bootstrapped users apply this and verify it works for them? Thanks.

Comment by Mike Fikes [ 18/Feb/16 10:03 AM ]

I tried this with Planck and I can confirmed that, with a function name in Japanese, sourceMappingURL does indeed change and then includes base-64 data that covers my entire set of functions (whereas previously it did not), but the Japanese function name appears to have been munged into some Latin-1 characters (which I suppose is the point of the patch).

With Planck, I can't confirm the overall functionality as Planck doesn't make use of this information with JavaScriptCore (it instead uses equivalent info stored in map files).

So, as far as I can tell, this patch is good in that it appears to be doing the right thing when run with the bootstrap compiler.

Comment by David Nolen [ 18/Mar/16 1:45 PM ]

OK, patch looks ok to me but it needs to be rebased to master.

Comment by Matt Huebert [ 21/Jul/16 7:06 AM ]

Same patch but rebased to master

Comment by David Nolen [ 22/Jul/16 7:28 AM ]

fixed https://github.com/clojure/clojurescript/commit/e8011e7fe95c81432ad9a1043422a807edb9c052





[CLJS-1712] Make PersistentHashSet implement IReduce Created: 21/Jul/16  Updated: 21/Jul/16

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

Type: Enhancement Priority: Minor
Reporter: Thomas Mulvaney Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: performance

Attachments: Text File difference-benchmark.txt     Text File into-benchmark.txt     Text File phs-reduce.patch     Text File union-benchmark.txt    
Patch: Code

 Description   

This improves speed of many reduce based operations on set which were falling back to seq-reduce, including code in `clojure.set` namespace such as `clojure.set/union` and `(into [] some-set)`.

I've included a few benchmarks I performed using `simple-benchmark` in a JavascriptCore environment (Planck REPL)



 Comments   
Comment by Rohit Aggarwal [ 21/Jul/16 3:35 PM ]

I think the code currently is faithful to Clojure's implementation of PersistentHashSet. So any change from that would probably require more thought and/or history.

Also someone else also raised a similar issue on ClojureScript mailing list.





[CLJS-1710] spec/double-in not implemented Created: 20/Jul/16  Updated: 20/Jul/16

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

Type: Defect Priority: Major
Reporter: Marshall Abrams Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: cljs, newbie, spec
Environment:

Clojurescript 1.9.89



 Description   

spec/double-in is available in Clojure 1.9.0-alpha10, but doesn't seem to be implemented yet in Clojurescript as of 1.9.89. I also tried 1.9.76: not there either.

cljs.user=> (require '[cljs.spec :as s])
nil
cljs.user=> (s/valid? #(and (>= % 0.0) (<= % 1.0)) 1.0)
----  Compiler Warning on   <cljs form>   line:1  column:12  ----

  Use of undeclared Var cljs.spec/double-in

  1  (s/valid? (s/double-in :min 0.0 :max 1.0) 1.0)
                ^---

----  Compiler Warning  ----
#object[TypeError TypeError: Cannot read property 'call' of undefined]
nil

(Newbie ticket. Apologies if this is a dupe ticket or doesn't belong here. I couldn't find any tickets that seemed to mention this issue.)






[CLJS-1703] cljs sources not resolved by devtools for 1.9.93 Created: 08/Jul/16  Updated: 11/Jul/16  Resolved: 11/Jul/16

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

Type: Defect Priority: Major
Reporter: Denis Johnson Assignee: David Nolen
Resolution: Not Reproducible Votes: 0
Labels: None
Environment:

Ubuntu 14.04 LTS 64bit
Chrome unstable Version 53.0.2783.2 dev (64-bit)
Chrome release Version 51.0.2704.106 (64-bit)



 Description   

Chrome devtools fails to show cljs file sources when compiled with org.clojure/clojurescript "1.9.93" sources are fine with "1.9.89".

1. Create a browser REPL project as guided in the Quick Start
2. open browser on http://localhost:9000
3. open chrome devtools, go to sources tab and all cljs files should be visible and include contents

I see the cljs files listed but no contents.



 Comments   
Comment by David Nolen [ 11/Jul/16 4:16 PM ]

Was already fixed in a later commit due to bad usage of clojure.string/index-of





[CLJS-1704] destructuring maps in protocol method signatures cause runtime values to be replaced with the destructuring map literal Created: 10/Jul/16  Updated: 11/Jul/16  Resolved: 11/Jul/16

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

Type: Defect Priority: Minor
Reporter: Bert Muthalaly Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: bug
Environment:

Can be reproduced with the clojurescript Quick Start jar and the code below.



 Description   

(defprotocol P
(f [this {:keys [x]}]))

(defrecord R []
P
(f [this m] m))

(f (R.) {:hello "world"}) ;; => {:keys [:hello]}

We should get the same map as output as we used as input - instead, we get the destructuring map literal.

The above code works as expected in a bare Clojure 1.8 REPL.



 Comments   
Comment by Bert Muthalaly [ 10/Jul/16 1:39 PM ]

Oh, I think I understand what's happening here.

We unconditionally splice the method signature of the protocol into the generated call to (. this <slot> ~@sig), so defn correctly destructures the map, but the invocation of the method macroexpands to (. this cljs$user$P$mtd$arity$2 this {:keys [x]}).

Can be reproduced with

(cljs.pprint/pprint (macroexpand-1 '(defprotocol P (mtd [this {:keys [x]}]))))

at a bare ClojureScript REPL.

My original rationale for supporting this was for protocol readability.

It is sometimes useful to approximate named arguments in protocol method signatures, especially because keyword arguments are not supported in protocol method signatures due to the fact that keyword destructuring is variadic[0].

If you could write destructuring forms in protocol method signatures (that had no effect), a reader of a protocol method can quickly understand the desired invocation.

Other solutions:

  • Spec the protocol arguments. More robust, but the information is no longer inline with the signature of the protocol method.
  • Document this in the method docstring.
  • Warn that destructuring forms are not supported - solves the problem of the user's map being replaced by the destructuring map at runtime.

[0]: https://groups.google.com/forum/#!topic/clojure/AHfyzXCgvTk

Comment by David Nolen [ 11/Jul/16 4:15 PM ]

It's invalid to use destructuring syntax in defprotocol





[CLJS-1705] Calling (done) within binding in an async test corrupts bound vars Created: 10/Jul/16  Updated: 11/Jul/16  Resolved: 11/Jul/16

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.36, 1.9.76, Next
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Trevor Schmidt Assignee: Unassigned
Resolution: Declined Votes: 1
Labels: asynchronous, cljs, test
Environment:

OS X 10.11.5

⟩ java -version
java version "1.8.0_72"
Java(TM) SE Runtime Environment (build 1.8.0_72-b15)
Java HotSpot(TM) 64-Bit Server VM (build 25.72-b15, mixed mode)



 Description   

In an async test, calling (done) within a (binding) corrupts bound vars.

repro/core.cljs
(ns repro.core
  (:require [clojure.string :as string]
            [cljs.test :refer-macros [async deftest is run-tests]]))

(def ^:dynamic *foo* nil)

(deftest async-binding-safe
  (async done
    (binding [*foo* {}]
      (is (some? *foo*)))
    (is (nil? *foo*))
    (done)))

(deftest async-binding-corruption
  (async done
    (is (nil? *foo*))
    (binding [*foo* {}]
      (is (some? *foo*))
      (done))))

(deftest async-binding-corrupt?
  (async done
    (is (nil? *foo*))
    (done)))

(enable-console-print!)

(run-tests 'repro.core)

The first test passes as expected, and because (done) is outside the binding, *foo* is reverted to nil. The second test passes, but leaves *foo* bound. The third test fails.



 Comments   
Comment by Trevor Schmidt [ 10/Jul/16 4:44 PM ]

clojure.string is not necessary for the repro, but I am unable to edit to remove it.

Comment by Antonin Hildebrand [ 10/Jul/16 5:28 PM ]

might be related to CLJS-1634

Comment by David Nolen [ 11/Jul/16 4:14 PM ]

This is expected behavior





[CLJS-1702] Warning or error when using private vars Created: 07/Jul/16  Updated: 07/Jul/16

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

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


 Description   

Currently no warning or error of any kind is given. Throwing an error and forcing users to go through vars is somewhat less attractive since vars dump information like file, line, etc. A warning would be a simple way to flag users that they are treading into dangerous territory. Downstream tooling error handling can make it a hard error if they like.






[CLJS-1701] cljs.spec impact on :advanced builds Created: 07/Jul/16  Updated: 07/Jul/16

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

Type: Defect Priority: Major
Reporter: Thomas Heller Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Investigate the impact of cljs.spec on :advanced builds.

Currently all specs are kept in the (private) cljs.spec/registry-ref atom. This atom is not understood by the Closure Compiler and cannot be eliminated as dead code. So even if specs are not used in "production" they still bloat the generated JS size. Some specs may be used at runtime and cannot not be removed, the gen parts however are probably never required in :advanced builds and should be omitted somehow.

In a test build (with 1.9.93) this adds 11kb (102kb vs 91kb) as soon as cljs.spec is :require'd somewhere and goes up with each defined spec.






[CLJS-1698] cljs.spec: every res call needs &env Created: 01/Jul/16  Updated: 06/Jul/16  Resolved: 06/Jul/16

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

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

Attachments: Text File CLJS-1698.patch    
Patch: Code

 Description   

Example of every working in Clojure, covering the salient case for this ticket:

Clojure 1.9.0-alpha8
user=> (require '[clojure.spec :as s])
nil
user=> (s/explain (s/every pos? :kind vector?) '(1))
val: (1) fails predicate: vector?
nil

In ClojureScript this causes an error to be thrown, starting with

clojure.lang.ExceptionInfo: Wrong number of args (-1) passed to: spec/res at line 1 <cljs repl> {:file "<cljs repl>", :line 1, :column 12, :root-source-info {:source-type :fragment, :source-form (s/explain (s/every pos? :kind vector?) (quote (1)))}, :tag :cljs/analysis-error}


 Comments   
Comment by Mike Fikes [ 01/Jul/16 6:24 PM ]

With the attached patch:

$ script/noderepljs
ClojureScript Node.js REPL server listening on 51132
To quit, type: :cljs/quit
cljs.user=> (require '[clojure.spec :as s])
nil
cljs.user=> (s/explain (s/every pos? :kind vector?) '(1))
val: (1) fails predicate: vector?

nil
cljs.user=>
Comment by David Nolen [ 06/Jul/16 7:43 AM ]

fixed https://github.com/clojure/clojurescript/commit/16bc7ace746e1c0d67f4628339c51aad0668c03d





[CLJS-1695] Self-host: Port cljs / clojure namespace aliasing Created: 25/Jun/16  Updated: 06/Jul/16  Resolved: 06/Jul/16

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

Type: Enhancement Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: bootstrap

Attachments: Text File CLJS-1695.patch    
Patch: Code and Test

 Description   

CLJS-1692 covers auto-aliasing clojure.* to cljs.* in JVM ClojureScript. This asks for the same for bootstrapped ClojureScript.



 Comments   
Comment by Mike Fikes [ 25/Jun/16 11:47 PM ]

This issue has the use of util/ns->source in common with CLJS-1657. (Perhaps a common solution could be found for both—like making use of something along the lines of the bootstrap source load function.)

Comment by Mike Fikes [ 02/Jul/16 3:30 PM ]

The attached patch employs a "try first and then fall-back and patch things up" strategy with respect to clojure.* -> cljs.*.

Comment by David Nolen [ 06/Jul/16 7:42 AM ]

fixed https://github.com/clojure/clojurescript/commit/16af9f651f09e5c3f91098270ffacb806b907302





[CLJS-1697] doc on inferred macros fails Created: 30/Jun/16  Updated: 06/Jul/16  Resolved: 06/Jul/16

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

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1697.patch    
Patch: Code

 Description   

If you make use of the new CLJS-1507 feature to infer a macro var in :refer, then doc fails on it.

Here is an example. See how it fails in foo.core but succeeds in bar.core:

$ script/noderepljs
ClojureScript Node.js REPL server listening on 52639
To quit, type: :cljs/quit
cljs.user=> (doc doc)
-------------------------
cljs.repl/doc
([name])
Macro
  Prints documentation for a var or special form given its name

nil
cljs.user=> (ns foo.core)
nil
foo.core=> (require '[cljs.repl :refer [doc]])
nil
foo.core=> (doc doc)
-------------------------
cljs.repl/doc
  nil

nil
foo.core=> (ns bar.core)
nil
bar.core=> (require-macros '[cljs.repl :refer [doc]])
nil
bar.core=> (doc doc)
-------------------------
cljs.repl/doc
([name])
Macro
  Prints documentation for a var or special form given its name

nil


 Comments   
Comment by Mike Fikes [ 03/Jul/16 9:55 AM ]

The root cause is that, when inferring a macro in :refer, the macro symbol mapping is left in :uses. This leads to other manifestations, like being able to apply var to such a symbol, etc.

The fix in the attached patch adds a little extra logic to remove missing uses from :uses (both in the AST being returned and, more importantly, from the compiler state).

Comment by David Nolen [ 06/Jul/16 7:40 AM ]

fixed https://github.com/clojure/clojurescript/commit/7264ca4ed0502fd69acea04e5d01c9bd1a3cb687





[CLJS-1699] Update docstring for ns Created: 03/Jul/16  Updated: 06/Jul/16  Resolved: 06/Jul/16

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

Type: Enhancement Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: docstring

Attachments: Text File CLJS-1699.patch    
Patch: Code

 Description   

Since doc for ns was added with CLJS-1307, ClojureScript gained support for bootstrap, and also has new features in the pipeline in dev (CLJS-1507 and CLJS-1692) which could be reflected in the docstring.



 Comments   
Comment by David Nolen [ 06/Jul/16 7:39 AM ]

fixed https://github.com/clojure/clojurescript/commit/ed6c100452676283ff2786d7bef4a11a196ae8b4





[CLJS-1447] IFn implementors have a broken call implementation, all args after 20th argument should be collected into a seq Created: 11/Sep/15  Updated: 05/Jul/16

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

Type: Task Priority: Major
Reporter: David Nolen Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Regular fns (which are just JavaScript fns) have no such limit. For IFn implementors we should not allow arities above 21 args, and we should transform the 21st arity into a var args signature.



 Comments   
Comment by François De Serres [ 18/Jun/16 9:13 AM ]

we should transform the 21st arity into a var args signature

Unless misunderstanding, can't do that. Var args sigs aren't allowed in protocols.

we should not allow arities above 21 args

Emitting an analyzer warning is what you want?

Comment by Antonin Hildebrand [ 05/Jul/16 6:07 PM ]

I believe I hit this problem in my code using core.async[1].

If it is not possible to implement ATM, I would kindly ask for a compiler warning at least. This thing manifested as a infinite recursive loop ending up in a cryptic stack overflow.

[1] https://github.com/binaryage/dirac/commit/cce56470975a287c0164e6f79cd525d6ed27a543





[CLJS-1446] autodoc + gh-pages for cljs.*.api namespaces Created: 11/Sep/15  Updated: 02/Jul/16

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

Type: Task Priority: Major
Reporter: David Nolen Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: newbie


 Comments   
Comment by W. David Jarvis [ 11/Sep/15 6:07 PM ]

I just tried to get this working - unfortunately, autodoc doesn't currently have support for ClojureScript. An issue is currently open on the GH project here but it doesn't look like it's seen any movement in nearly two years.

Comment by Tom Faulhaber [ 13/Sep/15 2:26 PM ]

I would love to see this work as well and, as the author of autodoc, am happy to help move it forward. I've added some commentary to the issue in autodoc about how to do this. If it's going to happen soon, though, I will need some help from the ClojureScript community as outlined over there.

Comment by David Nolen [ 14/Sep/15 10:42 AM ]

This ticket is about generating docs for Clojure code. Getting autodoc to work for ClojureScript files is worth pursuing but unrelated to this ticket.

Comment by Sebastian Bensusan [ 11/Oct/15 5:54 PM ]

I took at stab at this and only got it running using autodoc-0.9.0-standalone.jar from the command line. My results are not useful at all but those issues should be sorted out in autodoc.

David, do you have a preference in how the docs and artifacts needed should be managed? Should it be a lein plugin or can it be a script that assumes that the correct jars have been installed?

Comment by Tom Faulhaber [ 12/Oct/15 12:37 AM ]

Oh, I did misunderstand this and then didn't see David Nolen's follow-up until now. Let me take a look at whether I can make this happen pretty easily. I wouldn't think it would be too difficult. (Famous last words!)

Comment by Tom Faulhaber [ 02/Jul/16 2:14 AM ]

I have just closed the blocking issue in autodoc Issue 21, andSebastian Bensusan has successfully built a version of doc for the src/main/clojure/... stuff.

The next step is to flesh out what we want to push to http://clojure.github.io/clojurescript. I don't think that this is too hard. Then we can integrate it with the autodoc robot and get automatic updates.





[CLJS-1694] Self-host: Port macro var inference in :refer Created: 25/Jun/16  Updated: 29/Jun/16  Resolved: 29/Jun/16

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

Type: Enhancement Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: bootstrap

Attachments: Text File CLJS-1694.patch    
Patch: Code and Test

 Description   

CLJS-1507 covers JVM ClojureScript. This ticket requests the same for bootstrapped ClojureScript.



 Comments   
Comment by Mike Fikes [ 26/Jun/16 3:04 PM ]

Attached patch is a straightforward (intended to be faithful) port of the JVM ClojureScript feature.

Comment by David Nolen [ 29/Jun/16 2:09 PM ]

fixed https://github.com/clojure/clojurescript/commit/053d7f1ead6698b38e7ff656e0910ebc8bb8f729





[CLJS-1693] rel-output-path produces wrong path for :lib files Created: 25/Jun/16  Updated: 25/Jun/16

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

Type: Defect Priority: Major
Reporter: Ruslan Prokopchuk Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Bug presents in any environment, but break things only on Windows.



 Description   

Building with a cljsjs package where cljsjs package includes :libs in deps.cljs you find the "out" directory includes a subdirectory "file:".

An example from my specific case:
"out/file:/home/vagrant/.m2/repository/cljsjs/openlayers/3.3.0-0/openlayers-3.3.0-0.jar!/cljsjs/development/openlayers/ol/array.js"

':' is not permitted on Windows filesystem, which leads to compilation error.

It happens because rel-output-path here https://github.com/clojure/clojurescript/blob/17bcf2a091accb6f7caf1e8fa3954b490e9d34fa/src/main/clojure/cljs/closure.clj#L1515 detects closure-lib first (before taking in account that it is in jar) and passes it to lib-rel-path which acts as if file is located in project directory.

Another issue, but deeply related, is that on Windows, lib-rel-path breaks build even for :lib files located in project directory. It happens because of naive path prefix removal https://github.com/clojure/clojurescript/blob/17bcf2a091accb6f7caf1e8fa3954b490e9d34fa/src/main/clojure/cljs/closure.clj#L1500 because it tries to remove a subpath with native path separators, but path comes here in url-like form with forward slashes. Compiler then reports that path is not relative and aborts compilation.






[CLJS-1507] Implicit macro loading: macro var inference in :refer Created: 09/Dec/15  Updated: 24/Jun/16  Resolved: 24/Jun/16

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

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 2
Labels: None

Attachments: Text File CLJS-1507-partial.patch    

 Description   

Background: When employing ns, if a namespace is required or used, and that namespace itself requires or uses macros from its own namespace, then the macros will be implicitly required or used using the same specifications.

This ticket asks for a mild extension to the above, automatically inferring when referred symbols are macro vars in two cases:

  1. :require / :refer
  2. :use / :only

Here is a concrete example illustrating the cases:

Assume src/foo/core.clj contains a single macro:

(ns foo.core)

(defmacro unless [pred a b]
  `(if (not ~pred) ~a ~b))

and src/foo/core.cljs requires that macro namespace:

(ns foo.core
  (:require-macros foo.core))

Without loss of generality, assume we do the following in a REPL.

$ java -cp cljs.jar:src clojure.main -m cljs.repl.node
ClojureScript Node.js REPL server listening on 49522
To quit, type: :cljs/quit
cljs.user=> (require '[foo.core :refer [unless]])
;; Currently get an error

Currently, the code assumes that unless is a non-macro var and a "Referred var foo.core/unless does not exist" diagnostic is emitted. To satisfy this ticket, the code should consult the foo.core macro namespace for the existence of an unless macro, and refer it as if

(require-macros '[foo.core :refer [unless]])
had been issued.

Likewise, this should work for the :use / :only dual form: When

(ns bar.core (:use [foo.core :only [unless]]))
is issued.

Additionally, the same behavior should be ensured to work in bootstrapped mode.



 Comments   
Comment by Thomas Heller [ 10/Dec/15 4:26 AM ]

Great to see you pick this up.

My patch for CLJS-948 included an implementation for this if you need a reference, although some things have changed since then and the patch won't apply cleanly. Still the basic idea is still the same.

Comment by Thomas Heller [ 10/Dec/15 4:33 AM ]

Again, just for reference:
https://github.com/thheller/shadow-build/blob/7b2564e9aa33a93c1af90826c837e3f5d307a116/src/clj/shadow/cljs/util.clj#L214-L279

Is the full implementation in use by shadow-build ever since the :refer part was rejected from CLJS-948, that is also where my patch initially came from.

Comment by Mike Fikes [ 11/Dec/15 8:33 PM ]

Attaching a partial patch in case it leads to interesting commentary.

This patch was written prior to my knowledge of Thomas' prior work. I don't have an opinion yet on the values of either approach: This one differs simply by accident of being written independently.

This patch works by transforming things in the sugared domain, thus taking something like

(:require [foo.core :refer [bar unless]])

into

(:require [foo.core :refer [bar] :refer-macros [unless]])

This patch is incomplete because it determines whether a symbol is a macro by attempting to find its macroexpander, and thus encounters the need for the foo.core namespace to have been analyzed, which is not guaranteed.

But, this patch will work if you artificially (say, in a REPL), load foo.core by itself first. Evidently Thomas' approach encountered a similar issue, but he resolved it (presumably by forcing the needed analysis to occur).

Comment by Thomas Heller [ 12/Dec/15 4:09 AM ]

Mike: I had two separate goals with my implementation. In shadow-build I have a discovery phase that inspects all files and parses the ns form in case of .cljs files. I wanted this phase to be completely free of side-effects in regards to the compiler state. I did not want to touch the analyzer env or load macros since I do not know whether the discovered namespace is going to be used later. I only extract the basic requires/provides to build the dependency graphs via :main (forced by shadow-build). Actual compilation always happens in full dependency order and macros are loaded when needed with all checks. The discovery skips some basic checks it cannot know about yet, ie. it checks for self-require but doesn't load macros yet.

The approach chosen by parse-ns in cljs.analyzer is to capture the compiler env and reset it after parsing the ns, your approach moves side-effects into the parsing again which may become a problem later since it cannot reset {{require}}ing a Clojure ns.

Whether or not this is a problem I cannot say, it just didn't agree with my goals. YMMV.

Comment by Mike Fikes [ 12/Dec/15 7:35 AM ]

Thanks @thheller, I'll see if I can come up with a patch that implements the inference in ns-side-effects.

Comment by Mike Fikes [ 30/Mar/16 11:18 PM ]

It's been a while since I've dedicated any time to this one. Unassigned from me in case anyone else wants to take it up.

Comment by David Nolen [ 24/Jun/16 6:03 PM ]

fixed https://github.com/clojure/clojurescript/commit/783001a6786f8dca4a13fdeadc995716903b07f9





[CLJS-1692] Autoalias clojure.* to exisiting cljs.* namespaces if possible Created: 24/Jun/16  Updated: 24/Jun/16  Resolved: 24/Jun/16

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

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


 Comments   
Comment by David Nolen [ 24/Jun/16 3:04 PM ]

fixed https://github.com/clojure/clojurescript/commit/23632baa35f86de8866dede624545bc0cdf4a2bb





[CLJS-1061] Enhanced control over printing configuration Created: 23/Feb/15  Updated: 21/Jun/16  Resolved: 26/Apr/15

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: 0.0-3196
Fix Version/s: 0.0-3255

Type: Enhancement Priority: Major
Reporter: Joel Holdbrooks Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None


 Description   

enable-console-print! is convenient from some developers on a team who like println to print to the console. However, for others, like myself, it is more desirable to print directly to the REPL (e.g. w/in Emacs) instead of the browser console. On occasion enable-console-print! get's checked and can cause errors that result from calling println before the *print-fn* is set. It then becomes a hassle to either comment out the println or the enable-console-print!. Having a disable-console-print! which could restore the previous *print-fn*, whatever that may have been, would be nice to have.

As it currently stands, one must do this manually, which frankly is kind of awkward and not immediately obvious. It is much like having a light you can turn on but not off unless you cut the main power.



 Comments   
Comment by Thomas Heller [ 24/Feb/15 6:01 AM ]

I think the whole print-fn handling is not optimal. The problem is that each namespace can set it and will override the previous setting and the last one wins. While this is most likely one of "my" namespaces and "my" print-fn it may be someone else's while initializing (loading dependant namespaces). So you might run into issues where some log output goes to console and some goes to util.print or whichever other method the library author decided to use. Some libraries already call (enable-console-print!) as the very first thing their namespace.

What I recently added to shadow-build is an option to make this a compiler option (eg. {:print-fn :console}). Code can just use prn, println and never worry about that *print-fn* might not be set. I think this is a very convenient option that can default to :console and one less hurdle new users may stumble over. I don't need to initialize println in Clojure either.

Maybe this option should make it into core.

Comment by David Nolen [ 12/Mar/15 10:03 AM ]

Not going to do disable-console-print!. This ticket is important but it needs further consideration.

Comment by Joel Holdbrooks [ 12/Mar/15 5:39 PM ]

I'm okay with that. I'm just interested in the solution. Thanks for considering it.

Comment by David Nolen [ 15/Mar/15 10:26 AM ]

Putting this in the Backlog for now. It needs more hammock time. More solution ideas welcome on this ticket.

Comment by David Nolen [ 26/Apr/15 5:21 AM ]

Not going to do this. Development entry point namespaces are needed for things like REPLs anyways, move printing configuration there.

Comment by David Nolen [ 21/Jun/16 11:35 AM ]

this ticket is now subsumed by CLJS-1688 which addresses the broader problem of managing development time only side effects.





[CLJS-1688] :preloads compiler option for loading tooling code before the main ns Created: 20/Jun/16  Updated: 21/Jun/16  Resolved: 21/Jun/16

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

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


 Description   

The compiler should take a new `:preload` seq of symbols identifying namespaces (including Closure namespaces) that should be loaded before the main ns. The order of `:preload` will be respected but the recommendation will be that the namespaces are order independent. This will simplify loading tooling related side-effects which currently requires classpath busywork and requires boilerplate that will never see production.



 Comments   
Comment by David Nolen [ 21/Jun/16 11:31 AM ]

fixed https://github.com/clojure/clojurescript/commit/d187ad73ef673c3b1b0c3fe6b9ad3eb944057c3c





[CLJS-1691] spec internal compiler APIs Created: 21/Jun/16  Updated: 21/Jun/16

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

Type: Task Priority: Major
Reporter: David Nolen Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None





[CLJS-1690] spec the ClojureScript AST Created: 21/Jun/16  Updated: 21/Jun/16

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

Type: Task Priority: Major
Reporter: David Nolen Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None





[CLJS-1689] regression issue with cljs.spec.test/check-var Created: 21/Jun/16  Updated: 21/Jun/16  Resolved: 21/Jun/16

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

Type: Defect Priority: Major
Reporter: Wilker Lúcio da Silva Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1689.patch    
Patch: Code

 Description   

The cljs.spec.test/check-var is trying to use the `cljs.spec/fn-specs` function that is not available. Reproduction steps:

Wilkers-MacBook-Pro:Downloads wilkerlucio$ java -cp cljs.jar clojure.main -m cljs.repl.node
ClojureScript Node.js REPL server listening on 54604
To quit, type: :cljs/quit
cljs.user=> (require 'cljs.spec.test)
WARNING: Use of undeclared Var cljs.spec/fn-specs at line 80 file:/Users/wilkerlucio/Downloads/cljs.jar!/cljs/spec/test.cljs
WARNING: Use of undeclared Var cljs.spec/fn-specs at line 80 .cljs_node_repl/cljs/spec/test.cljs
nil
cljs.user=>


 Comments   
Comment by Mike Fikes [ 21/Jun/16 8:32 AM ]

Just need to follow a bit more of https://github.com/clojure/clojure/commit/4978bf5cee35f74df87c49720fa82de7287d60a5#diff-b0f91f319d0c76aadf667da929b08d37L526 and rename fn-spec to get-spec. Caught another place and fixed it in this patch as well.

Comment by David Nolen [ 21/Jun/16 8:34 AM ]

fixed https://github.com/clojure/clojurescript/commit/4628e011c193fe25a60a527bfa6771f2ff5403a1





[CLJS-1687] Self-host: cljs.spec: inst-in-range? and int-in-range? need qualification Created: 20/Jun/16  Updated: 20/Jun/16  Resolved: 20/Jun/16

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

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: bootstrap

Attachments: Text File CLJS-1687.patch    
Patch: Code and Test

 Description   

The runtime fns ints-in-range? and int-in-range? need to be qualified for self-host. (Otherwise they resolve to the $macros pseudo-namespace.



 Comments   
Comment by David Nolen [ 20/Jun/16 4:29 PM ]

fixed https://github.com/clojure/clojurescript/commit/416f322c25624b042e63e64a0754d5aaf48e552e





[CLJS-1686] cljs.spec: c alias needs expansion in int-in Created: 20/Jun/16  Updated: 20/Jun/16  Resolved: 20/Jun/16

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

Type: Defect Priority: Major
Reporter: Mike Fikes Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None


 Description   

See https://github.com/clojure/clojurescript/blob/5da67c1d13db7b7a4b347548184869097c5efa74/src/main/cljs/cljs/spec.cljc#L433

There is an instance of c/int that needs the same treatment given in https://github.com/clojure/clojurescript/commit/8477f19dcf67a8f305b46f2fd2e793586e027263



 Comments   
Comment by David Nolen [ 20/Jun/16 1:17 PM ]

fixed https://github.com/clojure/clojurescript/commit/0336696f4e805e96d1130f75a0e16241f96b55e1





[CLJS-1684] cljs.spec changes from Clojure master Created: 16/Jun/16  Updated: 17/Jun/16  Resolved: 17/Jun/16

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

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


 Description   

https://github.com/clojure/clojure/commit/aa9b5677789821de219006ece80836bd5c6c8b9b
https://github.com/clojure/clojure/commit/4978bf5cee35f74df87c49720fa82de7287d60a5
https://github.com/clojure/clojure/commit/20f67081b7654e44e960defb1e4e491c3a0c2c8b

the uri & bytes generator commits are interesting but less critical



 Comments   
Comment by David Nolen [ 17/Jun/16 3:18 PM ]

fixed https://github.com/clojure/clojurescript/commit/7a4e70f8848274f5db82ded9d823db772b9e5cbc





[CLJS-1685] Incorrectly lazy subvec when start param is nil Created: 17/Jun/16  Updated: 17/Jun/16

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

Type: Defect Priority: Minor
Reporter: Alf Kristian Støyle Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

ClojureScript 1.9.36 on Mac and Windows



 Description   

subvec in ClojureScript does not fail when start param is nil. This is different than in regular Clojure.

In Clojure:

(def foo (subvec nil 1))
CompilerException java.lang.IndexOutOfBoundsException, compiling:(form-init4645269128697935824.clj:1:10) 
(def foo (subvec nil nil))
CompilerException java.lang.NullPointerException, compiling:(form-init4645269128697935824.clj:1:10)

In ClojureScript:

(def foo (subvec nil 1))
#object[Error Error: Index out of bounds]
   cljs.core/build-subvec (jar:file:/Users/stoyle/.m2/repository/org/clojure/clojurescript/1.9.36/clojurescript-1.9.36.jar!/cljs/core.cljs:5316:16)
   Function.cljs.core.subvec.cljs$core$IFn$_invoke$arity$3 (jar:file:/Users/stoyle/.m2/repository/org/clojure/clojurescript/1.9.36/clojurescript-1.9.36.jar!/cljs/core.cljs:5328:7)
   Function.cljs.core.subvec.cljs$core$IFn$_invoke$arity$2 (jar:file:/Users/stoyle/.m2/repository/org/clojure/clojurescript/1.9.36/clojurescript-1.9.36.jar!/cljs/core.cljs:5326:7)
   cljs$core$subvec (jar:file:/Users/stoyle/.m2/repository/org/clojure/clojurescript/1.9.36/clojurescript-1.9.36.jar!/cljs/core.cljs:5319:1)
=> nil
(def foo (subvec nil nil))
=> #'user/foo

foo is of course not usable after this:

foo
#object[Error Error: No protocol method IIndexed.-nth defined for type null: ]
   cljs.core/missing-protocol (jar:file:/Users/stoyle/.m2/repository/org/clojure/clojurescript/1.9.36/clojurescript-1.9.36.jar!/cljs/core.cljs:264:4)





[CLJS-1497] `find` on an associative collection does not return collection key Created: 30/Nov/15  Updated: 15/Jun/16

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

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


 Description   

Instead find returns the passed in key. This means metadata on the key will appear to be lost. Related to CLJS-1496.



 Comments   
Comment by Rohit Aggarwal [ 15/Jun/16 3:39 PM ]

Proposed Plan

IAssociative protocol has a function called -entry-at which has been commented out. This function needs to be implemented which will return the necessary data structure, similar to the way it has been done in Clojure.

An example of its implementation for PersistentArrayMap is:

(-entry-at
 [coll k]
 (let [idx (array-map-index-of coll k)]
   (when-not (neg? idx)
     [(aget arr idx) (aget arr (inc idx))])))

We will need to implement this for all the collections which implement that protocol.

A failing test case:

(deftest test-find-meta-cljs-1497
  (let [k        [1 2 3]
        m        {:my "meta"}
        v        1
        xs       {(with-meta k m) v}
        [k' v']  (find xs k)]
    (is (= k k'))
    (is (= v v'))
    (is (= m (meta k')))))




[CLJS-1587] Duplicate keys via quoting Created: 24/Feb/16  Updated: 15/Jun/16

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

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None

Attachments: File CLJS-1587.diff    
Patch: Code and Test

 Description   

For

#{1 '1}

you get

#{1 1}


 Comments   
Comment by Peter Schuck [ 03/Mar/16 10:01 PM ]

This happens for the has-set macro and the hash-set literal. Here's what I get from the repl

cljs.user=> (hash-set 1 '1 2 '2 3 '3 4 '4 5)
#{1 2 3 4 5}
cljs.user=> (hash-set 1 '1 2 '2 3 '3 4 '4)
#{1 1 2 2 3 3 4 4}
cljs.user=> #{ 1 '1 2 '2 3 '3 4 '4}
#{2 1 4 4 3 2 1 3}
cljs.user=> #{ 1 '1 2 '2 3 '3 4 '4 5}
#{2 1 4 4 3 2 5 1 3}
cljs.user=> #{ 1 '1 2 '2 3 '3 4 '4 5 '5}
#{2 5 1 4 4 3 2 5 1 3}
cljs.user=> (apply hash-set [1 '1 2 '2 3 '3 4 '4])
#{1 2 3 4}

Calling hash-set as a function gives the correct results. The hash-set macro gives the incorrect results until we have more then 8 elements and uses the fromArray method on PersistentHashSet to build the set instead of creating a literal PersistentArrayMap for the set. The literal notation is incorrect no matter how many elements there are.

Comment by Rohit Aggarwal [ 15/Jun/16 8:02 AM ]

The underlying problem for both is the same in that a PersistentHashSet is being created directly using a PersistentArrayMap where the keys are the elements from the provided sequence. It manifests itself in two places though.

Comment by Rohit Aggarwal [ 15/Jun/16 9:32 AM ]

I've taken the approach that if we see a quoted constant then don't create the PersistentHashSet directly and instead go via the fromArray function.

Patch has the fix and a test.





[CLJS-1682] :foreign-libs with module conversion does not works properly if it is used form deps.cljs Created: 13/Jun/16  Updated: 14/Jun/16

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

Type: Defect Priority: Minor
Reporter: Andrey Antukh Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: closure, compiler, foreign-libs
Environment:

Linux, openjdk8


Attachments: Text File CLJS-1682.patch    
Patch: Code

 Description   

When :foreign-libs is used for consume commonjs (or amd) modules from clojurescript using the `deps.cljs` mechanism, an unexpected "internal compiler error" is raised. When the same configuration is attached on the build script, everything works as expected.

Simple way to reproduce that, create sample directory tree:

mkdir tmp;
cd tmp;
mkdir -p src/testapp
mkdir -p src/vendor
touch src/testapp/core.cljs
touch src/vendor/greeter.js

Download the latest compiler or copy the recently build one from master:

wget https://github.com/clojure/clojurescript/releases/download/r1.9.36/cljs.jar

Create the sample cljs file:

;; tmp/src/testapp/core.cljs
(ns testapp.core
  (:require [cljs.nodejs :as nodejs]
            [greeter.core :as g]))

(nodejs/enable-util-print!)

(defn -main
  [& args]
  (println (g/sayHello "Ciri")))

(set! *main-cli-fn* -main)

Create the sample commonjs module:

"use strict";

exports.sayHello = function(name) {
  return `Hello ${name}!`;
};

Create the build script (that works):

;; tmp/build.clj
(require '[cljs.build.api :as b])

(b/build "src"
 {:main 'testapp.core
  :output-to "out/main.js"
  :output-dir "out"
  :target :nodejs
  :language-in  :ecmascript6
  :language-out :ecmascript5
  :foreign-libs [{:file "vendor/greeter.js"
                  :module-type :commonjs
                  :provides ["greeter.core"]}]
  :verbose true})

And compile this using the following command:

java -cp cljs.jar:src clojure.main build.clj

This will generate successfully the final artifact that can be successufully executed with node:

node out/main.js
# => "Hello Ciri!"

But, if you remove the `:foreign-libs` from the build script and create a new `src/deps.cljs` file
with the following content:

{:foreign-libs [{:file "vendor/greeter.js"
                 :module-type :commonjs
                 :provides ["greeter.core"]}]}

And try compile it:

$ java -cp cljs.jar:src clojure.main build.clj
Copying jar:file:/home/niwi/tmp/cljs.jar!/cljs/core.cljs to out/cljs/core.cljs
Reading analysis cache for jar:file:/home/niwi/tmp/cljs.jar!/cljs/core.cljs
Compiling out/cljs/core.cljs
Using cached cljs.core out/cljs/core.cljs
Copying jar:file:/home/niwi/tmp/cljs.jar!/cljs/nodejs.cljs to out/cljs/nodejs.cljs
Compiling out/cljs/nodejs.cljs
Compiling src/testapp/core.cljs
Copying jar:file:/home/niwi/tmp/cljs.jar!/cljs/nodejs.cljs to out/cljs/nodejs.cljs
Copying jar:file:/home/niwi/tmp/cljs.jar!/cljs/nodejscli.cljs to out/cljs/nodejscli.cljs
Copying jar:file:/home/niwi/tmp/cljs.jar!/goog/base.js to out/goog/base.js
Copying jar:file:/home/niwi/tmp/cljs.jar!/goog/string/string.js to out/goog/string/string.js
Copying jar:file:/home/niwi/tmp/cljs.jar!/goog/object/object.js to out/goog/object/object.js
Copying jar:file:/home/niwi/tmp/cljs.jar!/goog/string/stringbuffer.js to out/goog/string/stringbuffer.js
Copying jar:file:/home/niwi/tmp/cljs.jar!/goog/debug/error.js to out/goog/debug/error.js
Copying jar:file:/home/niwi/tmp/cljs.jar!/goog/dom/nodetype.js to out/goog/dom/nodetype.js
Copying jar:file:/home/niwi/tmp/cljs.jar!/goog/asserts/asserts.js to out/goog/asserts/asserts.js
Copying jar:file:/home/niwi/tmp/cljs.jar!/goog/array/array.js to out/goog/array/array.js
Copying file:/home/niwi/tmp/src/vendor/greeter.js to out/greeter.js
Exception in thread "main" java.lang.RuntimeException: INTERNAL COMPILER ERROR.
Please report this problem.

null
  Node(SCRIPT): vendor/greeter.js:1:0
[source unknown]
  Parent: NULL, compiling:(/home/niwi/tmp/build.clj:3:1)
        at clojure.lang.Compiler.load(Compiler.java:7391)
        at clojure.lang.Compiler.loadFile(Compiler.java:7317)
        at clojure.main$load_script.invokeStatic(main.clj:275)
        at clojure.main$script_opt.invokeStatic(main.clj:335)
        at clojure.main$script_opt.invoke(main.clj:330)
        at clojure.main$main.invokeStatic(main.clj:421)
        at clojure.main$main.doInvoke(main.clj:384)
        at clojure.lang.RestFn.invoke(RestFn.java:408)
        at clojure.lang.Var.invoke(Var.java:379)
        at clojure.lang.AFn.applyToHelper(AFn.java:154)
        at clojure.lang.Var.applyTo(Var.java:700)
        at clojure.main.main(main.java:37)
Caused by: java.lang.RuntimeException: INTERNAL COMPILER ERROR.
Please report this problem.

[...]


 Comments   
Comment by Andrey Antukh [ 13/Jun/16 5:42 AM ]

Also happens with `cljs.jar` build from master.

Comment by Rohit Aggarwal [ 14/Jun/16 5:40 AM ]

[Compiler noob here]

Here is what is causing the issue:

In src/main/clojure/cljs/closure.clj in process-js-modules function, in the first case :foreign-libs is being set in opts and in the second failing case :ups-foreign-libs is being set in opts.

I am investigating the root of this.

Comment by Rohit Aggarwal [ 14/Jun/16 6:11 AM ]

A fix is to that set foreign-libs keyword in opts to a union of both foreign-libs and ups-foreign-libs.

I've verified that it works for both the above given examples. But I don't know enough about the compiler to propose this change.

Comment by Rohit Aggarwal [ 14/Jun/16 10:57 AM ]

Attaching patch with fixes this problem. The patch keeps the two sets of data (ups-foreign-libs, foreign-libs) separate.

I've run all the tests and they pass.





[CLJS-1674] cljs.test/run-all-tests fails when passed a regexp variable Created: 09/Jun/16  Updated: 13/Jun/16  Resolved: 10/Jun/16

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

Type: Defect Priority: Minor
Reporter: Erik Ouchterlony Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: bug, test


 Description   
(let [re #".*re.*"] 
  (cljs.test/run-all-tests re))

clojure.lang.ExceptionInfo: clojure.lang.Symbol cannot be cast to java.util.regex.Pattern at line 1 <cljs repl> {:file "<cljs repl>", :line 1, :column 21, :tag :cljs/analysis-error}



 Comments   
Comment by David Nolen [ 10/Jun/16 1:35 PM ]

This is not a bug. run-all-tests is a macro, this case cannot be made to work.

Comment by Erik Ouchterlony [ 13/Jun/16 3:52 AM ]

Wouldn't it be possible to defer the filtering until run-time?

The reason I think this is important is that in a live programming setting, using e.g. om or reagent, it is possible to run the tests while the program is running, displaying the result in a component. It would be really nice to be able to filter test at run-time as well, selecting the filter from the UI.





[CLJS-1678] variadic defn can be called for missing fixed arities, overlapping arity Created: 11/Jun/16  Updated: 12/Jun/16

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

Type: Defect Priority: Minor
Reporter: Francis Avila Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

For defns with a variadic arity: if invoked with a missing fixed arity, they use the variadic method instead of erroring; if invoked with a fixed arity that is the max fixed arity, variadic mathod instead of fixed form is invoked.

(defn f-hole
  ([a] 1)
  ([a b c d & args] "4 or more"))

(f-hole 1 2) ; =>"4 or more", should be error

(defn f-overlap-mfa
  ([a b] 2)
  ([a b & c] "2+"))

(f-overlap-mfa 1) ;=> "2+", should be error
(f-overlap-mfa 1 2) ;=> "2+", should be 2
(f-overlap-mfa 1 2 3) ;=> "2+", correct

A way to fix the f-hole bug is to emit a "case X:" into the switch statement for all X with no signature or less than max-fixed-arity.

The f-overlap-mfa I'm not sure why is happening and didn't investigate deeply.



 Comments   
Comment by Francis Avila [ 11/Jun/16 8:31 AM ]

Sorry, filed against CLJ instead of CLJS!

Comment by Rohit Aggarwal [ 12/Jun/16 9:41 AM ]

The behaviour I am seeing for f-overlap-mfa is:

(f-overlap-mfa 1) ;=> "2+"
(f-overlap-mfa 1 2) ;=> 2
(f-overlap-mfa 1 2 3) ;=> "2+"

So the two argument result is different for me than you, Francis Avila.

The call with just one argument does give a warning though:

WARNING: Wrong number of args (1) passed to cljs.user/f-overlap-mfa





[CLJS-1641] Multi-arity defn copies arguments unnecessarily for all cases Created: 16/May/16  Updated: 12/Jun/16

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

Type: Defect Priority: Minor
Reporter: Stephen Brady Assignee: Stephen Brady
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-1641.patch    
Patch: Code

 Description   

Background:

Passing js arguments as a parameter to another function is a known performance issue. Copying arguments to an array addresses this, and this approach has been taken to handle args passing for variadic functions in previous patches such as:
https://github.com/clojure/clojurescript/commit/dccc5d0feab9809cb56bd4d7257599a62232ac0d
https://github.com/clojure/clojurescript/commit/f09bbe62e99e11179dec6286fbb46265c12f4737

This commit (https://github.com/clojure/clojurescript/commit/576fb6e054dd50ec458a3c9e4172a5a0002c7aea) introduced a macro path for `defn` forms.

Current Behavior and Impact:

In the case of multi-arity defn forms, the macro path generates an array copy of the arguments variable regardless of whether it is used or necessary. In the case of multiple arities but no variadic arity, copying arguments is unnecessary as arguments will not be passed to the variadic method for the given function. In the case of multiple arities with a variadic case, an args array copy is needed but should be isolated to that case alone; currently, the array copy is performed before checking the arguments length, causing all cases to incur an (unused) args array copy.

Relevant code: https://github.com/clojure/clojurescript/blob/master/src%2Fmain%2Fclojure%2Fcljs%2Fcore.cljc#L2827-L2843

Recommended Change:

  • Do not perform an args array copy before switching on arguments length
  • Perform an args array copy within the variadic dispatch case

Patch forthcoming.



 Comments   
Comment by David Nolen [ 21/May/16 5:03 PM ]

Thanks! Will take a look.

Comment by David Nolen [ 10/Jun/16 1:32 PM ]

This probably needs to be updated in the compiler as well.

Comment by Stephen Brady [ 10/Jun/16 2:43 PM ]

The compiler already isolates the args to array copying behavior in the variadic case. The unnecessary copying is isolated to the defn macro.

These are the only two calls to `emit-arguments-to-array`:

Variadic function: https://github.com/clojure/clojurescript/blob/master/src/main/clojure/cljs/compiler.cljc#L743
Multi-arity with variadic case: