<< Back to previous view

[CLJS-1714] Clojurescript github README.md should identify source Clojure version Created: 24/Jul/16  Updated: 25/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.





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

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

Type: Defect Priority: Minor
Reporter: Marshall Abrams Assignee: Unassigned
Resolution: Unresolved 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.





[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-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-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-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-1708] Self-host: [iu]nstrument-1 needs to qualify [iu]nstrument-1* Created: 15/Jul/16  Updated: 15/Jul/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-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).






[CLJS-1707] Self-host: with-instrument-disabled needs to qualify *instrument-enabled* Created: 15/Jul/16  Updated: 15/Jul/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-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.






[CLJS-1706] cljs.reader support for namespaced map literal Created: 15/Jul/16  Updated: 17/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: 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





[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-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-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-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-1700] Support clojure.* aliasing when not in vector Created: 03/Jul/16  Updated: 06/Jul/16

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

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


 Description   

While this works:

(ns foo.core
  (:require [clojure.test]))

this does not:

(ns foo.core
  (:require clojure.test))





[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-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-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-1696] Alias clojure.spec.gen => cljs.spec.impl.gen Created: 29/Jun/16  Updated: 29/Jun/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





[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-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-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-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-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-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-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-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-1683] js->clj passes opts incorrectly Created: 15/Jun/16  Updated: 15/Jun/16

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

Type: Defect Priority: Trivial
Reporter: Oliver George Assignee: Unassigned
Resolution: Unresolved 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.)






[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-1681] Make instrument-all / unstrument-all return nil Created: 11/Jun/16  Updated: 11/Jun/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-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.





[CLJS-1680] Self-host: Don't require items no longer provided by Closure Created: 11/Jun/16  Updated: 11/Jun/16  Resolved: 11/Jun/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-1680.patch    
Patch: Code

 Description   

goog.array.ArrayLike no longer provided https://github.com/google/closure-library/commit/bf758d3c2ef81b91e7d73608f68ee3c327b709d4
goog.crypt.JpegEncoder no longer provided https://github.com/google/closure-library/commit/756182c7c566898ba0847d80b0c87389f7c037b6

With https://github.com/clojure/clojurescript/commit/aaa281d5cfef89385a484ad5f204ce6973f01222 we minimally need to remove these from the auxiliary namespace used in the self-host testing infrastructure.



 Comments   
Comment by David Nolen [ 11/Jun/16 12:13 PM ]

fixed https://github.com/clojure/clojurescript/commit/178c2c0daa5a52691d9e591425d55273e6176db3





[CLJS-1679] Self-host: Incorporate spec tests Created: 11/Jun/16  Updated: 11/Jun/16  Resolved: 11/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-1679.patch    
Patch: Code and Test

 Description   

cljs.spect-test was added with this commit https://github.com/clojure/clojurescript/commit/391d5cf86fbbf3d110ef6bbfc263d8132518a172

This ticket asks for the same for self-host.



 Comments   
Comment by Mike Fikes [ 11/Jun/16 9:06 AM ]

Attached CLJS-1679.patch:

Adds cljs.spec-test to the self-host test suite

To do so involves some adjustments to self-host
loading in this suite (a workaround for CLJS-1657
for some of the spec namespaces) and the need
to skip loading code for cljs.core macros namespace.

This also catches and fixes one production code
issue in the cljs.spec macros namespace: the symbol
instrument-enabled needs to be qualified so that
it refers to the runtime namespace.

Comment by David Nolen [ 11/Jun/16 12:14 PM ]

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





[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-1677] Requiring [goog] breaks an :advanced build, but the compiler exits successfully Created: 09/Jun/16  Updated: 10/Jun/16

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

Type: Defect Priority: Minor
Reporter: Daniel Compton Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

A single file with the following in it is enough to break a build:

(ns goog-error.core
  (:require [goog]))

with this error

Jun 10, 2016 11:18:03 AM com.google.javascript.jscomp.LoggerErrorManager println
SEVERE: ERROR - Duplicate input: file:/Users/danielcompton/.m2/repository/org/clojure/google-closure-library/0.0-20151016-61277aea/google-closure-library-0.0-20151016-61277aea.jar!/goog/base.js

Jun 10, 2016 11:18:03 AM com.google.javascript.jscomp.LoggerErrorManager printSummary
WARNING: 1 error(s), 0 warning(s)
ERROR: JSC_DUPLICATE_INPUT. Duplicate input: file:/Users/danielcompton/.m2/repository/org/clojure/google-closure-library/0.0-20151016-61277aea/google-closure-library-0.0-20151016-61277aea.jar!/goog/base.js at (unknown source) line (unknown line) : (unknown column)

however the ClojureScript compiler exits successfully without throwing an error. The build looks successful, but the file produced doesn't work. Should the ClojureScript compiler throw on these kinds of errors, or otherwise indicate failure?



 Comments   
Comment by David Nolen [ 10/Jun/16 8:27 AM ]

We should look into why the namespace validation that checks where a ns exists or not isn't already catching this case.





[CLJS-1676] Unused local in ObjMap / IKVReduce Created: 09/Jun/16  Updated: 09/Jun/16

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

Type: Defect Priority: Trivial
Reporter: Stuart Hinson Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-1676.patch    

 Description   

Local len isn't used in ObjMap / IKVReduce

https://github.com/clojure/clojurescript/blob/463de005b81d4a00951188e8b8d38a61d684c18e/src/main/cljs/cljs/core.cljs#L5792






[CLJS-1675] Update Closure Library Created: 09/Jun/16  Updated: 10/Jun/16  Resolved: 10/Jun/16

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

Type: Enhancement Priority: Major
Reporter: Wilker Lúcio da Silva Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None


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

Cut a new release today.





[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-1673] Can't s/instrument a multimethod Created: 09/Jun/16  Updated: 10/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: Oliver George Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None


 Description   

Looks like it's not possible to instrument a multimethod on 1.9.36.

The tests below pass on 1.9.14 but fail on 1.9.36. Equivalent code on 1.9.0-alpha5 passes.

(ns hello-world.core
  (:require [cljs.spec :as s :include-macros true]
            [cljs.test :refer-macros [deftest is run-tests]]))

(enable-console-print!)


(defmulti testmm :type)
(defmethod testmm :default [_])
(defmethod testmm :good [_] "good")

(s/fdef testmm :args (s/cat :m map?) :ret string?)

(s/instrument #'testmm)

(deftest test-good
  (is (= "good" (testmm {:type :good}))))

(deftest test-bad-arg-fails
  (is (thrown? ExceptionInfo (testmm :bad-arg))))

(deftest test-bad-ret-fails
  (is (thrown? ExceptionInfo (testmm {:bad :ret}))))

(run-tests)


 Comments   
Comment by David Nolen [ 09/Jun/16 8:27 AM ]

Fail in what way? Including the output of the failure helps a lot.

Comment by Oliver George [ 09/Jun/16 5:42 PM ]

My bad. All pass on 1.9.14, but this is the output on 1.9.36.

Results are consistent with s/instrument having no effect.

Testing hello-world.core

FAIL in (test-bad-arg-fails) (cljs/test.js:432:14)
expected: (thrown? ExceptionInfo (testmm :bad-arg))
  actual: nil

FAIL in (test-bad-ret-fails) (cljs/test.js:432:14)
expected: (thrown? ExceptionInfo (testmm {:bad :ret}))
  actual: nil

Ran 3 tests containing 3 assertions.
2 failures, 0 errors.
Comment by David Nolen [ 10/Jun/16 1:36 PM ]

In the future please try to make real minimal cases. We don't need deftest code making this example more complicated.

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

fixed https://github.com/clojure/clojurescript/commit/2682a4a53b0002db35c9fa17879dab66b0a614ca





[CLJS-1672] port new preds, specs, and gens Created: 07/Jun/16  Updated: 09/Jun/16  Resolved: 09/Jun/16

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

Type: Enhancement Priority: Major
Reporter: Patrick Killean Assignee: Patrick Killean
Resolution: Completed Votes: 0
Labels: None

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

 Description   

port https://github.com/clojure/clojure/commit/58227c5de080110cb2ce5bc9f987d995a911b13e



 Comments   
Comment by Patrick Killean [ 07/Jun/16 10:01 PM ]

CLJS-1672.patch: Everything except the inapplicable numerics & spec test stuff

Comment by Mike Fikes [ 07/Jun/16 10:18 PM ]

For me, one of the unit tests is failing.

FAIL in (test-preds) (V_@builds/out-adv/core-advanced-test.js:1023:83)
(#object[If "function If(b){return null!=b?b.v&8388608||b.je?!0:b.v?!1:gb(Nc,b):gb(Nc,b)}"] #inst "2016-06-08T03:17:17.377-00:00")
expected: (= ((nth preds i) v) (nth row i))
  actual: (not (= true false))
Comment by Thomas Heller [ 08/Jun/16 3:15 AM ]

It would be nice to have some of the number stuff as well, maybe even with equivalents on the clojure side. Since we don't have Long maybe something like

  • pos-number?
  • neg-number?
  • nat-number?

Also some of the number related functions in cljs behave very differently from clojure:

CLJS:

(pos? nil) => false
(pos? "2") => true

CLJ:

(pos? nil) => NullPointerException   clojure.lang.Numbers.ops (Numbers.java:1013)
(pos? "2") => ClassCastException java.lang.String cannot be cast to java.lang.Number  clojure.lang.Numbers.isPos (Numbers.java:96)

Maybe we should address this (in a separate issue).

Comment by Rohit Aggarwal [ 08/Jun/16 5:31 AM ]

The failing test is:

(seqable? now)

It should be false but it is true.

Somehow the behaviour in a noderepl is different from the test.

Also, (seqable? now) is false when used in the browser in either :advanced or :none optimisations mode. This looks strange to me.

Comment by Patrick Killean [ 08/Jun/16 6:34 AM ]

I ran the tests independently and it was fine but with the other core tests it misbehaves (on node):

cljs.user=> (seqable? (js/Date.)) ;=> false
cljs.user=> (require 'cljs.core-test)
cljs.user=> (seqable? (js/Date.)) ;=> true
Comment by Rohit Aggarwal [ 08/Jun/16 6:42 AM ]

I figured out the problem.

In core-test.cljs on line 2262, javascript object is being extended to add ISeqable protocol to it. So now indeed (satisfies? ISeqable (js/Date.)) is true instead of false.

This is why the behaviour is different.

Comment by Patrick Killean [ 08/Jun/16 7:06 AM ]

^Nice catch Rohit. I think a predicate test ns is a good solution

Comment by Rohit Aggarwal [ 08/Jun/16 8:03 AM ]

Patrick Killean Thanks. I agree - either moving this test to a new ns or moving the extend-type and the associated test-extend-to-object to a different ns should fix the failing test.

Comment by David Nolen [ 08/Jun/16 8:34 AM ]

A new predicate test ns is OK. Looking over the long predicates, I do see that there's little benefit given the lack of longs. Heller's concerns about the difference in the predicate behavior is also orthogonal to this ticket.

Comment by David Nolen [ 08/Jun/16 8:46 AM ]

On second thought, given that we have goog.math.Long I don't see a reason to not implement these new long predicates?

Comment by David Nolen [ 08/Jun/16 8:54 AM ]

Also the patch should be modified so that all the new predicates have a ^boolean flag.

Comment by Patrick Killean [ 08/Jun/16 5:37 PM ]

CLJS-1672-1.patch:
Boolean hints, long predicates, new predicate test namespace

Comment by Thomas Heller [ 09/Jun/16 3:06 AM ]

I'm pretty sure goog.math.Long is not included by default so we'd need to :require it in cljs.core. Shouldn't hurt since the file itself doesn't have any other dependencies.

Comment by David Nolen [ 09/Jun/16 1:23 PM ]

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





[CLJS-1671] Bad cljs.spec interactive instrumentation session Created: 06/Jun/16  Updated: 06/Jun/16  Resolved: 06/Jun/16

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

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


 Description   
(ns solari.spectest
  (:require
    [cljs.spec :as s]))

(s/fdef instrument-test
        :args (s/cat :input string?)
        :ret (fn [x] true)
        :fn (fn [x] true))

(defn instrument-test [input]
  input)

(comment
  ;Trying the following in the browser repl

  (s/instrument #'instrument-test) ;works
  
  ;next try calling function
  (instrument-test "hi")
  
  ;with bad input, spec returns error as expected
  (instrument-test 3)

  (s/unstrument #'instrument-test) ;works
  
  (s/instrument-all) ;doesn't work
  
  )



solari.spectest=>   (s/instrument-all) ;doesn't work
clojure.lang.ExceptionInfo:  at line 1 <cljs repl> {:file "<cljs repl>", :line 1, :column 3, :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:594)
        at cljs.analyzer$error.invoke(analyzer.cljc:590)
        at cljs.analyzer$macroexpand_1.invokeStatic(analyzer.cljc:2430)
        at cljs.analyzer$macroexpand_1.invoke(analyzer.cljc:2426)
        at cljs.analyzer$analyze_seq.invokeStatic(analyzer.cljc:2460)
        at cljs.analyzer$analyze_seq.invoke(analyzer.cljc:2443)
        at cljs.analyzer$analyze_form.invokeStatic(analyzer.cljc:2575)
        at cljs.analyzer$analyze_form.invoke(analyzer.cljc:2571)
        at cljs.analyzer$analyze_STAR_.invokeStatic(analyzer.cljc:2622)
        at cljs.analyzer$analyze_STAR_.invoke(analyzer.cljc:2613)
        at cljs.analyzer$analyze.invokeStatic(analyzer.cljc:2638)
        at cljs.analyzer$analyze.invoke(analyzer.cljc:2625)
        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__6003.invoke(repl.cljc:876)
        at cljs.repl$repl_STAR_$fn__6009$fn__6018.invoke(repl.cljc:915)
        at cljs.repl$repl_STAR_$fn__6009.invoke(repl.cljc:914)
        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:878)
        at cljs.repl$repl_STAR_.invoke(repl.cljc:761)
        at figwheel_sidecar.repl$eval8112$fn__8113.invoke(repl.clj:148)
        at clojure.lang.MultiFn.invoke(MultiFn.java:238)
        at figwheel_sidecar.repl$eval8106$fn__8107.invoke(repl.clj:144)
        at clojure.lang.MultiFn.invoke(MultiFn.java:238)
        at figwheel_sidecar.repl$repl.invokeStatic(repl.clj:165)
        at figwheel_sidecar.repl$repl.invoke(repl.clj:153)
        at figwheel_sidecar.system$start_figwheel_repl.invokeStatic(system.clj:471)
        at figwheel_sidecar.system$start_figwheel_repl.invoke(system.clj:462)
        at figwheel_sidecar.system$figwheel_cljs_repl_STAR_.invokeStatic(system.clj:515)
        at figwheel_sidecar.system$figwheel_cljs_repl_STAR_.invoke(system.clj:513)
        at figwheel_sidecar.system$cljs_repl_STAR_.invokeStatic(system.clj:540)
        at figwheel_sidecar.system$cljs_repl_STAR_.invoke(system.clj:532)
        at figwheel_sidecar.system$cljs_repl.invokeStatic(system.clj:565)
        at figwheel_sidecar.system$cljs_repl.invoke(system.clj:560)
        at figwheel_sidecar.system$cljs_repl.invokeStatic(system.clj:563)
        at figwheel_sidecar.system$cljs_repl.invoke(system.clj:560)
        at figwheel_sidecar.repl_api$cljs_repl.invokeStatic(repl_api.clj:110)
        at figwheel_sidecar.repl_api$cljs_repl.invoke(repl_api.clj:103)
        at figwheel_sidecar.repl_api$cljs_repl.invokeStatic(repl_api.clj:107)
        at figwheel_sidecar.repl_api$cljs_repl.invoke(repl_api.clj:103)
        at user$eval37818.invokeStatic(form-init802425000002695685.clj:1)
        at user$eval37818.invoke(form-init802425000002695685.clj:1)
        at clojure.lang.Compiler.eval(Compiler.java:6942)
        at clojure.lang.Compiler.eval(Compiler.java:6905)
        at clojure.core$eval.invokeStatic(core.clj:3105)
        at clojure.core$eval.invoke(core.clj:3101)
        at clojure.main$repl$read_eval_print__8495$fn__8498.invoke(main.clj:240)
        at clojure.main$repl$read_eval_print__8495.invoke(main.clj:240)
        at clojure.main$repl$fn__8504.invoke(main.clj:258)
        at clojure.main$repl.invokeStatic(main.clj:258)
        at clojure.main$repl.doInvoke(main.clj:174)
        at clojure.lang.RestFn.invoke(RestFn.java:1523)
        at clojure.tools.nrepl.middleware.interruptible_eval$evaluate$fn__7186.invoke(interruptible_eval.clj:87)
        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.tools.nrepl.middleware.interruptible_eval$evaluate.invokeStatic(interruptible_eval.clj:85)
        at clojure.tools.nrepl.middleware.interruptible_eval$evaluate.invoke(interruptible_eval.clj:55)
        at clojure.tools.nrepl.middleware.interruptible_eval$interruptible_eval$fn__7231$fn__7234.invoke(interruptible_eval.clj:222)
        at clojure.tools.nrepl.middleware.interruptible_eval$run_next$fn__7226.invoke(interruptible_eval.clj:190)
        at clojure.lang.AFn.run(AFn.java:22)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
        at java.lang.Thread.run(Thread.java:745)
Caused by: java.lang.NullPointerException
        at clojure.core$namespace.invokeStatic(core.clj:1554)
        at clojure.core$namespace.invoke(core.clj:1548)
        at cljs.spec$speced_vars_STAR_$fn__23506.invoke(spec.cljc:284)
        at clojure.core.protocols$iter_reduce.invokeStatic(protocols.clj:49)
        at clojure.core.protocols$fn__7829.invokeStatic(protocols.clj:75)
        at clojure.core.protocols$fn__7829.invoke(protocols.clj:75)
        at clojure.core.protocols$fn__7771$G__7766__7784.invoke(protocols.clj:13)
        at clojure.core$reduce.invokeStatic(core.clj:6545)
        at clojure.core$reduce.invoke(core.clj:6527)
        at cljs.spec$speced_vars_STAR_.invokeStatic(spec.cljc:282)
        at cljs.spec$speced_vars_STAR_.invoke(spec.cljc:275)
        at cljs.spec$speced_vars_STAR_.invokeStatic(spec.cljc:277)
        at cljs.spec$speced_vars_STAR_.invoke(spec.cljc:275)
        at cljs.spec$instrument_all.invokeStatic(spec.cljc:417)
        at cljs.spec$instrument_all.invoke(spec.cljc:413)
        at clojure.lang.AFn.applyToHelper(AFn.java:156)
        at clojure.lang.AFn.applyTo(AFn.java:144)
        at clojure.core$apply.invokeStatic(core.clj:650)
        at clojure.core$apply.invoke(core.clj:641)
        at cljs.analyzer$macroexpand_1_STAR_$fn__1828.invoke(analyzer.cljc:2383)
        at cljs.analyzer$macroexpand_1_STAR_.invokeStatic(analyzer.cljc:2382)
        at cljs.analyzer$macroexpand_1_STAR_.invoke(analyzer.cljc:2369)
        ... 68 more
solari.spectest=>


 Comments   
Comment by Leon Talbert [ 06/Jun/16 9:58 AM ]

So it looks like the source of the problem is calling fdef on a function that doesn't exist yet.
This causes a nil value to be placed into _speced_vars. Once this happens all subsequent calls
to instrument-all fail.

Comment by David Nolen [ 06/Jun/16 1:00 PM ]

Does instrumenting a var before it exists work in Clojure, I would suspect not? I guess we should just throw here.

Comment by David Nolen [ 06/Jun/16 4:39 PM ]

fixed https://github.com/clojure/clojurescript/commit/103aa6e5770242d8bdf9d234a85fca9e6dc918cf





[CLJS-1670] "Differences-from-Clojure" should mention var/namespace conflict Created: 05/Jun/16  Updated: 05/Jun/16  Resolved: 05/Jun/16

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

Type: Enhancement Priority: Minor
Reporter: Phill Wolf Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: documentation


 Description   

A Google-Group thread[1] raised a difference between Clojure and ClojureScript: you may name a var and a sub-namespace (as it were) the same in Clojure, but you will have to rename one or the other when porting to ClojureScript.

I did not see this difference called out on the page of "Differences-from-Clojure" [2]. It would be helpful there, especially for readers not well versed in the implications of using Google Closure's conventions for libraries and dependencies.

[1] https://groups.google.com/forum/#!topic/clojurescript/4Fvwx2Jf1nU

[2] https://github.com/clojure/clojurescript/wiki/Differences-from-Clojure



 Comments   
Comment by Alex Miller [ 05/Jun/16 2:54 PM ]

I moved this from the CLJ project to CLJS.

Comment by David Nolen [ 05/Jun/16 3:06 PM ]

The wiki has been updated.





[CLJS-1669] Self-host: s/fdef ns-qualify *ns* name field access Created: 04/Jun/16  Updated: 05/Jun/16  Resolved: 05/Jun/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-1699.patch    
Patch: Code

 Description   

If you evaluate

(s/fdef foo :args ::s/any :ret ::s/any)

in a self-host context, the code will ultimately call cljs.spec/ns-qualify which involves a form (.name *ns*). While this style of field access works in Clojure, in order to work in self-host, the form needs to be (.-name *ns*).

You can imitate the same in a regular JVM ClojureScript REPL by comparing

(.name (find-ns 'cljs.user))
(.-name (find-ns 'cljs.user))


 Comments   
Comment by David Nolen [ 05/Jun/16 9:07 AM ]

fixed https://github.com/clojure/clojurescript/commit/1dab2d684a4bfb1b2ad31979600b392df689f6ba





[CLJS-1668] Self-host: Macro checking support Created: 04/Jun/16  Updated: 05/Jun/16  Resolved: 05/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-1668-2.patch     Text File CLJS-1668-3.patch     Text File CLJS-1668.patch    
Patch: Code

 Description   

Add support for macro checking in self-host.

In essence, like https://github.com/clojure/clojurescript/commit/d6440795c22e46d2a2f8ab585fb6cfabf62cc147 but perhaps with appropriate code in :cljs reader conditional branches.



 Comments   
Comment by Mike Fikes [ 04/Jun/16 11:14 PM ]

The attached CLJS-1668.patch works. One twist is that you need to refer to a macro symbol using the $macros suffix, but there's probably no easy way around that for now. As a concrete example from the Spec guide, you can set things if you use cljs.core$macros/declare as the symbol passed to s/fdef.

Of interest in the patch, worthy of feedback, is the use of the construct:

^::no-resolve cljs.spec/macroexpand-check

This appears to be efficient (as the JavaScript var is either nil or not), and this works in self-host where the analyzer has direct access to check whether the var is nil.

Comment by Mike Fikes [ 04/Jun/16 11:31 PM ]

Whoops. The first patch fails to pass script/test-self-parity (owing to cljs.spec being nil) Attaching a revised patch CLJS-1688-2.patch that first checks for non-nil using an and.

Comment by David Nolen [ 05/Jun/16 9:25 AM ]

We don't want to leak out $macros, it's an implementation detail. We should automatically do the right thing if we are in a macro namespace. I believe there should be enough information in &env to figure this out in cljs.spec/fdef no? If not we should address this in the same patch.

Comment by Mike Fikes [ 05/Jun/16 11:29 AM ]

I agree. Perhaps one additional aspect to consider is macro-functions (like cljs.core/inc). The CLJS-1668-3.patch would perhaps lead to the ability to catch misuse when these are used either as macros or functions. Instead of detecting and adding $macros inside of fdef, the patch experiments with going the opposite direction: removing $macros in order to find a spec. That way, a single spec would cover the logically equivalent macro and function.

With this, you can see what might be desirable behavior:

cljs.user=> (require '[cljs.spec :as s])
nil
cljs.user=> (s/fdef cljs.core/inc :args number? :ret number?)
cljs.core/inc
cljs.user=> (inc "abc")
            ⬆
Call to cljs.core$macros/inc did not conform to spec:
val: ("abc") fails at: [:args] predicate: number?
:cljs.spec/args  ("abc")
 at line 1
cljs.user=> (map inc [1])
(2)
cljs.user=> (s/instrument cljs.core/inc)
#'cljs.core/inc
cljs.user=> (map inc [1])
Call to [object Object] did not conform to spec:
val: (1) fails at: [:args] predicate: number?
:cljs.spec/args  (1)

Interestingly, you can see that something else is going on with the map inc example at the bottom. This also occurs in a JVM ClojureScript REPL with master and is unrelated to the attached patch; I'd like to understand what's going on with it.

Comment by Mike Fikes [ 05/Jun/16 11:49 AM ]

Actually, in the example in the previous comment, it is the spec I wrote which is bad.

Here is a revised transcript showing the desired behavior with a proper spec:

cljs.user=> (require '[cljs.spec :as s])
nil
cljs.user=> (s/fdef cljs.core/inc
       #_=>   :args (s/cat :x number?)
       #_=>   :ret number?)
cljs.core/inc
cljs.user=> (inc 1)
2
cljs.user=> (inc "abc")
            ⬆
Call to cljs.core$macros/inc did not conform to spec:
In: [0] val: "abc" fails at: [:args :x] predicate: number?
:cljs.spec/args  ("abc")
 at line 1
cljs.user=> (s/instrument cljs.core/inc)
#'cljs.core/inc
cljs.user=> (map inc [1])
(2)
cljs.user=> (map inc ["abc"])
Call to [object Object] did not conform to spec:
In: [0] val: "abc" fails at: [:args :x] predicate: number?
:cljs.spec/args  ("abc")
Comment by David Nolen [ 05/Jun/16 1:43 PM ]

What I'm suggesting would cover us for both macros that shadow fns and the runtime fns. If the fdef is in a macro namespace it covers the macro. If it's a runtime ns it covers the runtime fn. That is a single spec isn't going to cover syntax and runtime behavior - it's desirable to able to spec both.

Comment by Mike Fikes [ 05/Jun/16 2:13 PM ]

Trying with patch 2, relying on the idea that an unqualified name in a macros namespace will automatically have the $macros suffix added internally as an implementation detail, this is a promising experiment: (It is also consistent with the way you can call functions from within macro namespaces in bootstrap as illustrated in the bottom half of http://blog.fikesfarm.com/posts/2016-01-05-clojurescript-macros-calling-functions.html)

$ cat src/foo/core.cljc
(ns foo.core
  (:require [#?(:clj clojure.spec :cljs cljs.spec) :as s]))

(defmacro my-inc
  [x]
  `(inc ~x))

(s/fdef my-inc
 :args (s/cat :x number?)
 :ret number?)
$ cat src/foo/core.cljs
(ns foo.core
  (:require-macros foo.core)
  (:require [cljs.spec :as s]))

(defn my-inc
  [x]
  (inc x))

(s/fdef my-inc
 :args (s/cat :x number?)
 :ret number?)

Using this pair of source files in Planck:

$ planck -c src
Planck 1.15
ClojureScript 1.9.39
    Docs: (doc function-name-here)
          (find-doc "part-of-name-here")
  Source: (source function-name-here)
    Exit: Control+D or :cljs/quit or exit or quit
 Results: Stored in vars *1, *2, *3, an exception in *e

cljs.user=> (require '[foo.core :include-macros true]
       #_=>          '[cljs.spec :as s])
nil
cljs.user=> (foo.core/my-inc 1)
2
cljs.user=> (foo.core/my-inc "a")
            ⬆
Call to foo.core$macros/my-inc did not conform to spec:
In: [0] val: "a" fails at: [:args :x] predicate: number?
:cljs.spec/args  ("a")
 at line 1
cljs.user=> (apply foo.core/my-inc [1])
2
cljs.user=> (apply foo.core/my-inc ["a"])
"a1"
cljs.user=> (s/instrument foo.core/my-inc)
#'foo.core/my-inc
cljs.user=> (apply foo.core/my-inc ["a"])
Call to [object Object] did not conform to spec:
In: [0] val: "a" fails at: [:args :x] predicate: number?
:cljs.spec/args  ("a")

	cljs.core/ExceptionInfo (cljs/core.cljs:10149:11)
	conform! (cljs/spec.cljs:894:54)
	G__12411__delegate (cljs/spec.cljs:924:135)
	cljs/lang/applyTo (cljs/spec.cljs:965:26)
	cljs.core/apply (cljs/core.cljs:3563:1)

cljs.user=>

(Note, I'm doing :include-macros true to work around CLJS-1657)

In script/noderepljs, (slightly modified to include my source path):

$ script/noderepljs
ClojureScript Node.js REPL server listening on 49477
To quit, type: :cljs/quit
cljs.user=> (require '[foo.core :include-macros true]
                     '[cljs.spec :as s])
nil
cljs.user=> (foo.core/my-inc 1)
2
cljs.user=> (foo.core/my-inc "a")
clojure.lang.ExceptionInfo: Call to foo.core/my-inc did not conform to spec:
In: [0] val: "a" fails at: [:args :x] predicate: number?
:clojure.spec/args  ("a")
 at line 1 <cljs repl> {:file "<cljs repl>", :line 1, :column 1, :tag :cljs/analysis-error}
	at clojure.core$ex_info.invokeStatic(core.clj:4631)
	at clojure.core$ex_info.invoke(core.clj:4631)
	at cljs.analyzer$error.invokeStatic(analyzer.cljc:594)
	at cljs.analyzer$error.invoke(analyzer.cljc:590)
	at cljs.analyzer$macroexpand_1.invokeStatic(analyzer.cljc:2432)
	at cljs.analyzer$macroexpand_1.invoke(analyzer.cljc:2428)
	at cljs.analyzer$analyze_seq.invokeStatic(analyzer.cljc:2462)
	at cljs.analyzer$analyze_seq.invoke(analyzer.cljc:2445)
	at cljs.analyzer$analyze_form.invokeStatic(analyzer.cljc:2577)
	at cljs.analyzer$analyze_form.invoke(analyzer.cljc:2573)
	at cljs.analyzer$analyze_STAR_.invokeStatic(analyzer.cljc:2624)
	at cljs.analyzer$analyze_STAR_.invoke(analyzer.cljc:2615)
	at cljs.analyzer$analyze.invokeStatic(analyzer.cljc:2640)
	at cljs.analyzer$analyze.invoke(analyzer.cljc:2627)
	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__6619.invoke(repl.cljc:876)
	at cljs.repl$repl_STAR_$fn__6625$fn__6634.invoke(repl.cljc:915)
	at cljs.repl$repl_STAR_$fn__6625.invoke(repl.cljc:914)
	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:878)
	at cljs.repl$repl_STAR_.invoke(repl.cljc:761)
	at cljs.repl$repl.invokeStatic(repl.cljc:996)
	at cljs.repl$repl.doInvoke(repl.cljc:926)
	at clojure.lang.RestFn.invoke(RestFn.java:410)
	at user$eval6810.invokeStatic(NO_SOURCE_FILE:3)
	at user$eval6810.invoke(NO_SOURCE_FILE:3)
	at clojure.lang.Compiler.eval(Compiler.java:6942)
	at clojure.lang.Compiler.eval(Compiler.java:6905)
	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: Call to foo.core/my-inc did not conform to spec:
In: [0] val: "a" fails at: [:args :x] predicate: number?
:clojure.spec/args  ("a")

	at clojure.spec$macroexpand_check.invokeStatic(spec.clj:560)
	at clojure.spec$macroexpand_check.invoke(spec.clj:549)
	at clojure.lang.Var.invoke(Var.java:383)
	at cljs.analyzer$macroexpand_1_STAR_.invokeStatic(analyzer.cljc:2383)
	at cljs.analyzer$macroexpand_1_STAR_.invoke(analyzer.cljc:2369)
	... 41 more
cljs.user=> (apply foo.core/my-inc [1])
2
cljs.user=> (apply foo.core/my-inc ["a"])
"a1"
cljs.user=> (s/instrument foo.core/my-inc)
#'foo.core/my-inc
cljs.user=> (apply foo.core/my-inc ["a"])

repl:13
throw e__6482__auto__;
^
Error: Call to [object Object] did not conform to spec:
In: [0] val: "a" fails at: [:args :x] predicate: number?
:cljs.spec/args  ("a")

    at new cljs$core$ExceptionInfo (/Users/mfikes/Projects/clojurescript/.cljs_node_repl/cljs/core.js:34473:10)
    at Function.cljs.core.ex_info.cljs$core$IFn$_invoke$arity$3 (/Users/mfikes/Projects/clojurescript/.cljs_node_repl/cljs/core.js:34549:9)
    at Function.cljs.core.ex_info.cljs$core$IFn$_invoke$arity$2 (/Users/mfikes/Projects/clojurescript/.cljs_node_repl/cljs/core.js:34545:26)
    at cljs$core$ex_info (/Users/mfikes/Projects/clojurescript/.cljs_node_repl/cljs/core.js:34531:26)
    at cljs.spec.spec_checking_fn.conform_BANG_ (/Users/mfikes/Projects/clojurescript/.cljs_node_repl/cljs/spec.js:892:25)
    at cljs.spec.spec_checking_fn.G__15208__delegate (/Users/mfikes/Projects/clojurescript/.cljs_node_repl/cljs/spec.js:922:136)
    at Function.cljs.spec.spec_checking_fn.G__15208.cljs$lang$applyTo (/Users/mfikes/Projects/clojurescript/.cljs_node_repl/cljs/spec.js:963:8)
    at Function.cljs.core.apply.cljs$core$IFn$_invoke$arity$2 (/Users/mfikes/Projects/clojurescript/.cljs_node_repl/cljs/core.js:12759:10)
    at cljs$core$apply (/Users/mfikes/Projects/clojurescript/.cljs_node_repl/cljs/core.js:12730:24)
    at repl:1:105
cljs.user=>
Comment by David Nolen [ 05/Jun/16 2:35 PM ]

fixed https://github.com/clojure/clojurescript/commit/6bd816f0fc6f0c3369b9b03f2561b219de6a69fc





[CLJS-1667] bad describe* for and-spec-impl Created: 03/Jun/16  Updated: 05/Jun/16  Resolved: 05/Jun/16

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

Type: Defect Priority: Minor
Reporter: Oliver George Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None


 Description   

(cljs.spec/form (cljs.spec/and string?)) returns (s/and cljs.core/string?) where s/and should be cljs.spec/and.

Based on or-spec-impl I think the fix is to replace:

(describe* [_] `(s/and ~@forms))

with

(describe* [_] `(and ~@forms))


 Comments   
Comment by Oliver George [ 03/Jun/16 10:44 PM ]

(I can see one other case in cljs.spec which references s/with-instrument-disabled)

Comment by David Nolen [ 05/Jun/16 9:13 AM ]

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





[CLJS-1666] Flag to optionally disable transit analysis cache encoding Created: 03/Jun/16  Updated: 10/Jun/16

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

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


 Description   

We need a flag to explicitly disable the transit encoding - this is to work around code that creates unencodeable forms for one reason or another. EDN had become more lax about encoding random Objects which allowed this stuff to go under the radar in the past.



 Comments   
Comment by Wilker Lúcio da Silva [ 06/Jun/16 9:10 PM ]

I'm having a compilation issue with ClojureScript 1.9.36, I'm writing an app that uses untangled and this file https://github.com/untangled-web/untangled-client/blob/f42088c84b059562a48455a71daa6e4ea08d286c/src/untangled/client/data_fetch.cljs fails to compile with Caused by: java.lang.RuntimeException: java.lang.Exception: Not supported: class cljs.tagged_literals.JSValue this issue doesn't happen with 1.9.14

Comment by David Nolen [ 06/Jun/16 10:06 PM ]

We should investigate whether just handling JSValue + other cases like clojure.lang.Var is good enough.

Comment by David Nolen [ 08/Jun/16 8:53 AM ]

JSValue encoding/decoding landed in master fixing part of the issue. Still need more information about the Var case however.





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

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

Type: Enhancement Priority: Minor
Reporter: Rohit Aggarwal Assignee: Unassigned
Resolution: Unresolved 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%





[CLJS-1664] Self-host: The filename aux.cljs is a problem on windows. Created: 02/Jun/16  Updated: 05/Jun/16  Resolved: 05/Jun/16

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

Type: Defect Priority: Minor
Reporter: Oliver George Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: bootstrap

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

 Description   

As I understand AUX is a reserved word dating back to MS-DOS Device Driver days. It seems many windows features are hardwired to reject that pattern in filenames.

In my case running windows 10:

  • I can't git clone the full repo (aux.cljs is skipped).
  • Downloading and unzipping throws an error on that file.

I suspect the best solution is to rename that file.



 Comments   
Comment by Mike Fikes [ 02/Jun/16 9:30 PM ]

I created this particular file when adding the capability to run the compiler unit tests under self host. The choice of filename is arbitrary. In fact, it is just an auxiliary namespace is employed to cause certain files to be dumped to the compiler output directory. It could be named anything and there should be no other files making use of that particular namespace name.

Comment by Mike Fikes [ 03/Jun/16 6:36 PM ]

The attached patch simply moves the file to a new name (while updating the namespace in the ns form).

Comment by David Nolen [ 05/Jun/16 9:16 AM ]

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





[CLJS-1663] Calling instrumented multi-arity function causes exception Created: 02/Jun/16  Updated: 02/Jun/16  Resolved: 02/Jun/16

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

Type: Defect Priority: Minor
Reporter: Oliver George Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None


 Description   

Repeatable using lein mies with [clojurescript "1.9.14"] using script/build.bat (and script/release.bat)

(ns spec-multiarity.core
  (:require [cljs.spec :as s :include-macros true]))

(enable-console-print!)

(defn adder
  ([a] a)
  ([a b] (+ a b)))
  
(s/fdef adder
  :args (s/cat :a integer? :b (s/? integer?))
  :ret integer?)

(do 
  (s/instrument #'adder)
  (adder 1))
core.cljs:8 Uncaught TypeError: spec_multiarity.core.adder.cljs$core$IFn$_invoke$arity$1 is not a functionspec_multiarity$core$adder 
@ core.cljs:8cljs.core.apply.cljs$core$IFn$_invoke$arity$2 
@ core.cljs:3572cljs$core$apply 
@ core.cljs:3563(anonymous function) 
@ spec.cljs:290cljs.spec.spec_checking_fn.G__8367__delegate 
@ spec.cljs:289cljs.spec.spec_checking_fn.G__8367 
@ spec.cljs:284(anonymous function) @ core.cljs:30


 Comments   
Comment by Oliver George [ 02/Jun/16 9:10 AM ]

Same code works on [org.clojure/clojure "1.9.0-alpha4"]

Comment by David Nolen [ 02/Jun/16 3:52 PM ]

Do not open tickets that reference build tools other than ClojureScript, thanks. Multi-arity fns are handled with s/or + s/cat. That's the only thing that would need to be fixed if it doesn't work.

Comment by David Nolen [ 02/Jun/16 4:49 PM ]

I looked at this a bit more closely and I was mistaken. Looking into it.

Comment by David Nolen [ 02/Jun/16 6:14 PM ]

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





[CLJS-1662] Optimize seq (&) destructuring (as per latest 0aa3467 in Clojure) Created: 02/Jun/16  Updated: 03/Jun/16  Resolved: 02/Jun/16

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

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

Attachments: Text File CLJS-1662-Optimize+seq+destructuring-2.patch     Text File CLJS-1662-Optimize seq destructuring.patch    
Patch: Code

 Description   

I've applied the changes which have been committed by Rich in this commit. All the tests are passing on v8, Spidermonkey and JSC engines on OS X. I've also tested using ./script/test-self-host and ./script/test-self-parity.

Benchmarking code:

(simple-benchmark [v (into [] (range 1000000))]
                  (loop [[x & xs] v]
                    (if xs
                      (recur xs)
                      x))
                  100)

Results (all times are in msecs):

- v8 Spidermoney JSC
Old 19070 18053 9116
New 10323 15238 6353


 Comments   
Comment by Rohit Aggarwal [ 02/Jun/16 7:34 AM ]

For reference, the performance improvements for Clojure are mentioned in this post.

Comment by Rohit Aggarwal [ 02/Jun/16 3:31 PM ]

I've updated the benchmarking code after conversation with David Nolen on Slack.

Comment by Rohit Aggarwal [ 02/Jun/16 3:34 PM ]

New benchmarks are below.

Code:

(println "\n")
(println ";; Destructuring a sequence")
(simple-benchmark [v (into [] (range 1000000))]
                  (loop [[x & xs] v]
                    (if-not (nil? xs)
                      (recur xs)
                      x))
                  1000)

Timing: (in msecs)

- v8 SpiderMonkey JSC
Old 160503 131297 123059
New 85183 118355 55245
Comment by David Nolen [ 02/Jun/16 6:18 PM ]

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

Comment by Rohit Aggarwal [ 03/Jun/16 3:41 AM ]

Thank you!





[CLJS-1661] cljs.spec: non-spec'ed fn var printing Created: 01/Jun/16  Updated: 10/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: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

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

 Description   

If you try to instrument an unspec'ed var, you will get a warning that prints [object Object]:

ClojureScript Node.js REPL server listening on 53013
To quit, type: :cljs/quit
cljs.user=> (require '[cljs.spec :as s])
nil
cljs.user=> (s/instrument inc)

repl:22
throw e__4797__auto__;
^
Error: Fn at [object Object] is not spec'ed.
...


 Comments   
Comment by Mike Fikes [ 01/Jun/16 11:22 PM ]

With the attached patch, prints:

Error: Fn at #'cljs.core/inc is not spec'ed.
Comment by David Nolen [ 10/Jun/16 9:15 AM ]

fixed https://github.com/clojure/clojurescript/commit/1bece47344089934f46db509756b32b102f1309b





[CLJS-1660] cljs.spec: Always return var from instrument / unstrument Created: 01/Jun/16  Updated: 06/Jun/16  Resolved: 06/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: None

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

 Description   

In Clojure, instrument and unstrument always return the associated var. In ClojureScript, calling instrument on an already-instrumented var returns nil. (And, likewise for unstrument with the fix for CLJS-1659.)

This ticket asks that ClojureScript behave like Clojure in this case.



 Comments   
Comment by Mike Fikes [ 06/Jun/16 4:56 PM ]

Attached CLJS-1660-2.patch, as previous no longer applies.

Comment by David Nolen [ 06/Jun/16 10:05 PM ]

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





[CLJS-1659] cljs.spec: stack-overflow calling unstrumented fn Created: 01/Jun/16  Updated: 06/Jun/16  Resolved: 06/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: David Nolen
Resolution: Not Reproducible Votes: 0
Labels: None

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

 Description   

Affects 1.9.14. After unstrumenting, calling the fn results in stack overflow.

$ java -jar cljs.jar -m cljs.repl.node
ClojureScript Node.js REPL server listening on 54613
To quit, type: :cljs/quit
cljs.user=> (require '[cljs.spec :as s])
nil
cljs.user=> (defn ranged-rand
  "Returns random integer in range start <= rand < end"
  [start end]
  (+ start (rand-int (- end start))))
#'cljs.user/ranged-rand
cljs.user=> (s/fdef ranged-rand
  :args (s/and (s/cat :start integer? :end integer?)
               #(< (:start %) (:end %)))
  :ret integer?
  :fn (s/and #(>= (:ret %) (-> % :args :start))
             #(< (:ret %) (-> % :args :end))))
cljs.user/ranged-rand
cljs.user=> (s/instrument #'ranged-rand)
#'cljs.user/ranged-rand
cljs.user=> (s/unstrument #'ranged-rand)
#'cljs.user/ranged-rand
cljs.user=> (ranged-rand 3 7)
repl:13
throw e__4797__auto__;
^
RangeError: Maximum call stack size exceeded
    at cljs.core.Var.call.G__8117__3 (/Users/mfikes/Downloads/.cljs_node_repl/cljs/core.js:3682:120)
    at cljs.core.Var.call.G__8117 [as call] (/Users/mfikes/Downloads/.cljs_node_repl/cljs/core.js:3827:19)
    at cljs.core.Var.call.G__8117__3 (/Users/mfikes/Downloads/.cljs_node_repl/cljs/core.js:3682:120)
    at cljs.core.Var.call.G__8117 [as call] (/Users/mfikes/Downloads/.cljs_node_repl/cljs/core.js:3827:19)
    at cljs.core.Var.call.G__8117__3 (/Users/mfikes/Downloads/.cljs_node_repl/cljs/core.js:3682:120)
    at cljs.core.Var.call.G__8117 [as call] (/Users/mfikes/Downloads/.cljs_node_repl/cljs/core.js:3827:19)
    at cljs.core.Var.call.G__8117__3 (/Users/mfikes/Downloads/.cljs_node_repl/cljs/core.js:3682:120)
    at cljs.core.Var.call.G__8117 [as call] (/Users/mfikes/Downloads/.cljs_node_repl/cljs/core.js:3827:19)
    at cljs.core.Var.call.G__8117__3 (/Users/mfikes/Downloads/.cljs_node_repl/cljs/core.js:3682:120)
    at cljs.core.Var.call.G__8117 [as call] (/Users/mfikes/Downloads/.cljs_node_repl/cljs/core.js:3827:19)
cljs.user=>

Also happens in bootstrap (downstream in Planck). The same sequence does not occur in Clojure with 1.9.0-alpha3.



 Comments   
Comment by Mike Fikes [ 01/Jun/16 9:28 PM ]

Owing to a leftover bit of the original Clojure implementation,
while unstrument* causes the desired side effect on instrumented-vars,
it results in setting the value of the unstrumented var to the var
(as opposed to the desired original raw function value).

The fix is to eliminate the return of the var and do the swap in the
body of when block, following the same pattern employed in instrument*.

Comment by Mike Fikes [ 06/Jun/16 5:03 PM ]

Exact same patch applied with CLJS-1671.





[CLJS-1658] implements? may report false positives Created: 01/Jun/16  Updated: 02/Jun/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

Attachments: Text File implements-false-positives.patch     Text File implements-false-positives-with-sentinel.patch    
Patch: Code

 Description   

The cljs.core/implements? checks whether a protocol is implemented via a property on the tested object. When implementing the protocol this property will be set to true.

// the implementation
protocol.check.Dummy.prototype.protocol$check$TheProtocol$ = true;

// the check (implements? TheProtocol x)
((!((x == null)))?(((false) || (x.protocol$check$TheProtocol$))?true:false):false)

// only the relevant bit
x.protocol$check$TheProtocol$

This works fine under :none but :advanced may rename this to something like x.A. If you now try to check (implements? TheProtocol js/window) for example it may (or may not) report a false positive. For larger projects the likelyhood of creating a false positives goes way up since window contains all the variables created by the advanced compiled js.

The attached patch changes the patch the emitted code to check x.protocol$check$TheProtocol$ === true which reduces the chance of a false positive by a lot. We might chose another sentinel value instead to reduce the chance some more, this seems good enough though.



 Comments   
Comment by Thomas Heller [ 01/Jun/16 2:13 PM ]

Implemented the same change for cljs.core/satisfies?.

Comment by Thomas Heller [ 01/Jun/16 2:18 PM ]

Benchmark on my MBP indicate that the change comes with a slight performance cost.

// v8 with patch
;;; satisfies?
[coll (list 1 2 3)], (satisfies? ISeq coll), 1000000 runs, 10 msecs
[coll [1 2 3]], (satisfies? ISeq coll), 1000000 runs, 27 msecs

//v8 without patch
;;; satisfies?
[coll (list 1 2 3)], (satisfies? ISeq coll), 1000000 runs, 8 msecs
[coll [1 2 3]], (satisfies? ISeq coll), 1000000 runs, 19 msecs
Comment by Thomas Heller [ 01/Jun/16 2:28 PM ]
// spidermonkey with patch
;;; satisfies?
[coll (list 1 2 3)], (satisfies? ISeq coll), 1000000 runs, 68 msecs
[coll [1 2 3]], (satisfies? ISeq coll), 1000000 runs, 146 msecs

// spidermonkey without patch
;;; satisfies?
[coll (list 1 2 3)], (satisfies? ISeq coll), 1000000 runs, 63 msecs
[coll [1 2 3]], (satisfies? ISeq coll), 1000000 runs, 149 msecs
Comment by Thomas Heller [ 01/Jun/16 2:35 PM ]

JavascriptCore

// jsc with path
;;; satisfies?
[coll (list 1 2 3)], (satisfies? ISeq coll), 1000000 runs, 7 msecs
[coll [1 2 3]], (satisfies? ISeq coll), 1000000 runs, 14 msecs

// jsc without patch
;;; satisfies?
[coll (list 1 2 3)], (satisfies? ISeq coll), 1000000 runs, 6 msecs
[coll [1 2 3]], (satisfies? ISeq coll), 1000000 runs, 13 msecs
Comment by Thomas Heller [ 02/Jun/16 3:40 AM ]

Added a second patch that uses a sentinel value as the protocol marker. Currently just a plain empty javascript object.

Running the benchmarks shows no real difference to just checking "true".

I also tried using a number or string as the sentinel value but could not find any significant difference either.

// emitted code on protocol impls
protocol.check.Dummy.prototype.protocol$check$TheProtocol$ = cljs.core.PROTOCOL_SENTINEL;

// the check (identical?)
cljs.core.PROTOCOL_SENTINEL === x.protocol$check$TheProtocol$




[CLJS-1657] Self-host: Implicit macro loading with alias Created: 01/Jun/16  Updated: 01/Jun/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: 1
Labels: bootstrap


 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.





[CLJS-1656] Self-host: cljs.spec: speced-vars* fn not resolving Created: 31/May/16  Updated: 10/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: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: bootstrap

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

 Description   

If you load cljs.spec.test in self-hosted ClojureScript, the reference to speced-vars* in the following location will not resolve:

https://github.com/clojure/clojurescript/blob/19510523ad9de3f16832d287bae86f701e8b4263/src/main/cljs/cljs/spec/test.cljc#L22

This is because it is a function in a macros namespace. In self-hosted ClojureScript, this function can be resolved if qualified using the $macros pseudo-namespace suffix.



 Comments   
Comment by Mike Fikes [ 31/May/16 3:32 PM ]

The attached patch resolves the issue by simply using the $macros suffix iff under self-hosted ClojureScript.

(I tried to revise the code to instead call the speced-vars macro, but that doesn't appear easy to accomplish given rest args are involved and you can't apply the macro.)

Comment by David Nolen [ 10/Jun/16 9:17 AM ]

fixed https://github.com/clojure/clojurescript/commit/868c6e426df0a021b3223b47f08ddfa01e492608





[CLJS-1655] cljs.spec: conformer docstring indicates :clojure.spec/invalid Created: 30/May/16  Updated: 10/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: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: docstring

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

 Description   

:cljs.spec/invalid should be returned by a conformer. viz:

cljs.user=> (s/conform ::even 3)
:cljs.spec/invalid


 Comments   
Comment by David Nolen [ 10/Jun/16 9:26 AM ]

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





[CLJS-1654] cljs.spec: var name in s/fdef non-conformance Created: 30/May/16  Updated: 10/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: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

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

 Description   

The below is taken from the spec guide. When you call a function instrumented with s/fdef with non-conforming args, the error message emitted fails to print the function var name and instead includes the string [object Object]. Here is a repro in the Node REPL:

$ script/noderepljs
ClojureScript Node.js REPL server listening on 50584
To quit, type: :cljs/quit
cljs.user=> (require '[cljs.spec :as s])
nil
cljs.user=> (defn ranged-rand
  "Returns random integer in range start <= rand < end"
  [start end]
  (+ start (rand-int (- end start))))
#'cljs.user/ranged-rand
cljs.user=> (s/fdef ranged-rand
  :args (s/and (s/cat :start integer? :end integer?)
               #(< (:start %) (:end %)))
  :ret integer?
  :fn (s/and #(>= (:ret %) (-> % :args :start))
             #(< (:ret %) (-> % :args :end))))
cljs.user/ranged-rand
cljs.user=> (s/instrument #'ranged-rand)
#'cljs.user/ranged-rand
cljs.user=> (ranged-rand 8 5)

repl:13
throw e__6035__auto__;
^
Error: Call to [object Object] did not conform to spec:
val: {:start 8, :end 5} fails at: [:args] predicate: (< (:start %) (:end %))
:cljs.spec/args  (8 5)

    at new cljs$core$ExceptionInfo (/Users/mfikes/Projects/clojurescript/.cljs_node_repl/cljs/core.js:34294:10)
    at Function.cljs.core.ex_info.cljs$core$IFn$_invoke$arity$3 (/Users/mfikes/Projects/clojurescript/.cljs_node_repl/cljs/core.js:34370:9)
    at Function.cljs.core.ex_info.cljs$core$IFn$_invoke$arity$2 (/Users/mfikes/Projects/clojurescript/.cljs_node_repl/cljs/core.js:34366:26)
    at cljs$core$ex_info (/Users/mfikes/Projects/clojurescript/.cljs_node_repl/cljs/core.js:34352:26)
    at cljs.spec.spec_checking_fn.conform_BANG_ (/Users/mfikes/Projects/clojurescript/.cljs_node_repl/cljs/spec.js:891:25)
    at cljs.spec.spec_checking_fn.G__14509__delegate (/Users/mfikes/Projects/clojurescript/.cljs_node_repl/cljs/spec.js:921:136)
    at cljs.spec.spec_checking_fn.G__14509 (/Users/mfikes/Projects/clojurescript/.cljs_node_repl/cljs/spec.js:958:27)
    at repl:1:111
    at repl:9:3
    at repl:14:4


 Comments   
Comment by Mike Fikes [ 30/May/16 10:34 AM ]

The attached patch causes the following to be emitted instead for the textual part of the error:

Error: Call to #'cljs.user/ranged-rand did not conform to spec:
val: {:start 8, :end 5} fails at: [:args] predicate: (< (:start %) (:end %))
:cljs.spec/args  (8 5)
Comment by David Nolen [ 10/Jun/16 9:28 AM ]

fixed https://github.com/clojure/clojurescript/commit/81fb24e2adc96c53834486b706b975336a4a0d23





[CLJS-1653] cljs.spec: keys* causes exception Created: 29/May/16  Updated: 30/May/16

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

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

Attachments: 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.)





[CLJS-1652] Self-host: Avoid alias so cljs.spec loadable Created: 28/May/16  Updated: 28/May/16  Resolved: 28/May/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-1652.patch    
Patch: Code

 Description   

The cljs.spec macro namespace employs the Clojure alias function to establish c as an alias for clojure.core. This prevents this namespace from being compiled as ClojureScript and thus bootstrapped ClojureScript environments cannot load it.

Proposed fix: Simply avoid alias and inline the namespace to form qualified symbols.



 Comments   
Comment by David Nolen [ 28/May/16 8:21 PM ]

fixed https://github.com/clojure/clojurescript/commit/8477f19dcf67a8f305b46f2fd2e793586e027263





[CLJS-1651] Self-host: Cannot replace core macro-function Created: 28/May/16  Updated: 28/May/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: bootstrap


 Description   

Replace double to multiply by two in self host and you will see that operator-position resolution chooses the core macro:

ClojureScript Node.js REPL server listening on 54425
To quit, type: :cljs/quit
cljs.user=> (require 'cljs.js)
nil
cljs.user=> (def st (cljs.js/empty-state))
#'cljs.user/st
cljs.user=> (cljs.js/eval-str st "(defn double [x] (* 2 x))" nil {:eval cljs.js/js-eval :context :expr} identity)
WARNING: double already refers to: cljs.core/double being replaced by: cljs.user/double at line 1
{:ns cljs.user, :value #object[cljs$user$double "function cljs$user$double(x){
return ((2) * x);
}"]}
cljs.user=> (cljs.js/eval-str st "[(double 3) (apply double [3])]" nil {:eval cljs.js/js-eval :context :expr} identity)
{:ns cljs.user, :value [3 6]}

The correct result above would be [6 6].






[CLJS-1650] cljs.reader returns hash-maps irrespective of the size of the map Created: 27/May/16  Updated: 02/Jun/16  Resolved: 02/Jun/16

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

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

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

 Description   

cljs.reader/read-string currently returns a hashmap irrespective of the size of the map in the edn string.

This behaviour is inconsistent with the way cljs.core is making sure to use the correct type of map - arraymap/hashmap and even Clojure's clojure.edn/read-string.



 Comments   
Comment by Rohit Aggarwal [ 27/May/16 4:43 AM ]

Including a patch which fixes this behaviour along with a test.

Comment by David Nolen [ 02/Jun/16 3:51 PM ]

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

Comment by Rohit Aggarwal [ 02/Jun/16 4:00 PM ]

cheers!





[CLJS-1649] Possible issue with in cljs.reader or cljs.core/PersistentHashMap Created: 26/May/16  Updated: 27/May/16  Resolved: 26/May/16

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

Type: Defect Priority: Major
Reporter: Rohit Aggarwal Assignee: David Nolen
Resolution: Completed Votes: 1
Labels: None
Environment:

Clojurescript version 1.8.51

Tested this on multiple browsers (Chrome/Safari/Firefox) and node.js versions on Linux and OS X.


Attachments: Text File CLJS-1649-1.patch     Text File CLJS-1649-2.patch     Text File CLJS-1649-3.patch     Text File CLJS-1649.patch     File core.cljs     PNG File ScreenShot.png    

 Description   

See attached source code core.cljs for the code.

I apologize for this very specific code example. Its been extracted from a larger app and this very specific arrangement of things is required to trigger the issue.

Problem

A map being returned by cljs.reader/read-string is behaving like a map in the sense of key lookups.

Walkthrough of the code.

(See attached Snapshot.png for sample output)

Line 7 - declare the edn string

Lines 9-43 - declare an unused map called date-formatters

Lines 45-66 - declare an unused map called COLOURS

Lines 68-77 - this is the real crux of the problem.

Line 70 - convert an edn string to a cljs datastructure using {{cljs.reader/read-string}. The datastructure is a map and more specifically a PersistentHashMap. Its called hm-data.

Line 71 - convert the hash-map from previous line to another map using (into {} hm-map). This datastructure is called am-data

Line 72 - print the type of hm-data. Sample output cljs.core/PersistentHashMap

Line 73 - print the type of am-data. Sample output cljs.core/PersistentArrayMap

Line 74-75 - compare the set of keys of the two data structures. result is true

Line 76 - Check if :version key is present in am-data. Sample output true

Line 77 - Check if :version key is present in hm-data. Sample output false

This is the problem. We already established from line 74-75 that both maps have the same keys. But Lines 76 and 77 are contradicting that fact.

Also key looks on the hm-data fail which succeed for am-data.

How to get the expected behaviour

  • Remove unused cljs.pprint require in the ns macro. We are not using this. So it shouldn't have any effect on the problem imho.
  • Remove unused date-formatters, COLOURS. Again we are not using this and it shouldn't have any effect on the working of the problem imho.
  • Remove a few key-vals from either of date-formatters, COLOURS. Even removing a single pair of key-vals would work. Again we are not using this and it shouldn't have any effect on the working of the problem.

If we were to make the initial edn string to a smaller one, the program would also work as expected. But with the original string, the above three things would also make it work.



 Comments   
Comment by Rohit Aggarwal [ 26/May/16 3:25 AM ]

I've tried using cljs.tools.reader/read-string from tools.reader library and it has the same behaviour.

Comment by Rohit Aggarwal [ 26/May/16 4:18 AM ]

Sorry the problem should state:

Problem

A map being returned by cljs.reader/read-string is not behaving like a map in the sense of key lookups.

Comment by Saurabh Kapoor [ 26/May/16 5:19 AM ]

I used http://app.klipse.tech/ (on Chrome) to quickly replicate the defect by pasting the attached file. I find that the output is as mentioned in the defect: Line 77 returns false.

Also, once the program is run on Klipse, I can no longer use that session to compile any programs. The following error is shown:

#error {:message "ERROR", :data {:tag :cljs/analysis-error}, :cause #object[TypeError TypeError: Cannot read property 'call' of undefined]}

Could it be that some global state gets changed for the worse?

Comment by Thomas Heller [ 26/May/16 11:06 AM ]

This seems to be related to other hashing issues we have seen before.

eg.
http://dev.clojure.org/jira/browse/CLJS-830
http://dev.clojure.org/jira/browse/CLJS-1380

I added a little snippet of code that would print the hash of the :version keyword. The version created by reader/read-string is different from a standard code one.

[:version 425292698]
[:read :version 430259761]
(defn run-ver
  [f]
  (let [data  (.readFileSync fs f "utf8")
        proj  (reader/read-string data)]
    (doseq [k (keys proj)]
      (prn [:read k (hash k)]))
    (println (keys proj))
    (println (:version proj))
    (if (:version proj)
      (println ":version key is present")
      (println ":version key is not present!!"))))

(defn main []
  (prn [:version (hash :version)])
  (run-ver "data.edn"))

This explains why array-map works since it only compares the keys and does not check the hash.

Comment by Rohit Aggarwal [ 26/May/16 12:37 PM ]

Continuing on the investigation by Thomas Heller, the issue seems to be in cljs.core/hash-string which uses a javascript object as a cache. If I replace that version with a version which doesn't use a cache, the code works as expected!

Comment by Rohit Aggarwal [ 26/May/16 1:04 PM ]

Alternatively, replacing hash-string with the following also fixes the problem:

(defn hash-string [k]
  (when (> string-hash-cache-count 255)
    (set! string-hash-cache (js-obj))
    (set! string-hash-cache-count 0))
  (if (nil? k)
    0
    (let [h (aget string-hash-cache k)]
      (if (number? h)
        h
        (add-to-string-hash-cache k)))))
Comment by Rohit Aggarwal [ 26/May/16 1:41 PM ]

The above solution works because javascript stores keys in an object as a string.

To highlight this:

(set! cljs.core/string-hash-cache (js-obj))
(set! cljs.core/string-hash-cache-count 0)
(println (hash-string "null"))
(println (hash-string nil))

produces

3392903
3392903

whereas,

(set! cljs.core/string-hash-cache (js-obj))
(set! cljs.core/string-hash-cache-count 0)
(println (hash-string nil))
(println (hash-string "null"))

produces

0
0

With the proposed replacement function, the hash-string function produces the right answer, namely 3392903 for "null" and 0 for nil irrespective of the order of operation.

Comment by Rohit Aggarwal [ 26/May/16 1:54 PM ]

I am attaching proposed fix to the issue along with a description of the problem.

Comment by Rohit Aggarwal [ 26/May/16 1:57 PM ]

Updated patch with better message grammar.

Comment by Rohit Aggarwal [ 26/May/16 3:33 PM ]

Updated the patch with a test which checks if the hash-string of nil is 0 and of "null" is non-zero.

Comment by Rohit Aggarwal [ 26/May/16 3:39 PM ]

I didn't upload the right file earlier.

It should be fixed now

Comment by David Nolen [ 26/May/16 3:43 PM ]

fixed https://github.com/clojure/clojurescript/commit/9887580fc731e3194e1619ba603d30e2548cc768

Comment by Rohit Aggarwal [ 26/May/16 4:55 PM ]

thank you!

Comment by Rohit Aggarwal [ 27/May/16 7:18 AM ]

For future reference, the reason why the patch fixes the problem:

  1. When the system starts cache contains nil as "null" and its value is 0.
  2. We compute hash of :version taking that 0 into account.
  3. Loads of operations such that the cache is reset.
  4. We calculate the hash of "null" as it is in a set. Now the value in the cache of "null" is something other than 0.
  5. We create a new :version taking the new value of "null" cache entry and try to compare its hash to earlier one and they do not match.




[CLJS-1648] Getting Source Info into ex-info data for Analysis Errors. Created: 25/May/16  Updated: 10/Jun/16  Resolved: 10/Jun/16

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

Type: Enhancement Priority: Minor
Reporter: Bruce Hauman Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: error-reporting, repl

Attachments: Text File CLJS-1648.patch     Text File CLJS-1648.patch     Text File CLJS-1648.patch     PNG File Screen Shot 2016-05-27 at 8.52.25 AM.png    
Patch: Code

 Description   

With an eye towards improving Analysis Error messages in the REPL.

It would be nice to have the original source of the evaluated form available in the ex-data of the Analysis Error.



 Comments   
Comment by Bruce Hauman [ 25/May/16 12:52 PM ]

This patch is working fine.

Comment by David Nolen [ 26/May/16 3:23 PM ]

Lets not flip the argument order for the optional argument - `env name`.

Comment by Bruce Hauman [ 27/May/16 8:00 AM ]

Gotcha, swapped the args, updated the patch and just for fun uploaded a screenshot of figwheel using this patch.

Comment by Bruce Hauman [ 31/May/16 10:01 AM ]

This is the latest patched against master, lein tests run clean.

Comment by David Nolen [ 10/Jun/16 1:57 PM ]

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





[CLJS-1647] Parallel-build catches all compile exceptions Created: 24/May/16  Updated: 26/May/16  Resolved: 26/May/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

Attachments: Text File CLJS-1647.patch    

 Description   

compile-task used by parallel-build catches all the exceptions from -compile: https://github.com/clojure/clojurescript/blob/master/src/main/clojure/cljs/closure.clj#L826-L828. The exceptions are just printed. This means that build tooling will be unable to control how to show these errors the the user (e.g. Figwheel or Boot-cljs HUD).

Currently if multiple exceptions are found parallel, all would be printed out.

So that parallel-build would work similar to normal build, it should throw a single exception. I don't think it matters too much which exception this is, if multiple occur at the same time.

I propose following fix:

  • remove debug-prn printting the exception
  • save the exception to failed atom
  • check the failed atom state at parallel-compile-sources and throw the exception if it exists


 Comments   
Comment by Juho Teperi [ 24/May/16 3:37 PM ]

Tested the patch with boot-cljs and figwheel and it works.

Comment by David Nolen [ 26/May/16 3:12 PM ]

https://github.com/clojure/clojurescript/commit/fc827fa5b0f1b290523f304fd84cf0a6c2a7bfa2





[CLJS-1646] Internal compiler error when importing Showdown via CommonJS Created: 24/May/16  Updated: 26/May/16

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

Type: Defect Priority: Minor
Reporter: Juan Pedro Bolivar Puente Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

cljs 1.8.51, clojure 1.8, lein-cljsbuild 1.1.3



 Description   

Hi!

I am trying to use Showdown 1.4.1 from NPM [1] using a `:commonjs` foreign-lib. Sadly, compilation crashes with the following error:

java.lang.RuntimeException: INTERNAL COMPILER ERROR.
Please report this problem.

null
Node(SCRIPT): node_modules/showdown/dist/showdown.js:1:0
[source unknown]
Parent: NULL
at com.google.javascript.jscomp.ES6ModuleLoader.normalizeInputAddress(ES6ModuleLoader.java:118)
at com.google.javascript.jscomp.ProcessCommonJSModules.inputToModuleName(ProcessCommonJSModules.java:89)
at com.google.javascript.jscomp.ProcessCommonJSModules$ProcessCommonJsModulesCallback.visitScript(ProcessCommonJSModules.java:336)
at com.google.javascript.jscomp.ProcessCommonJSModules$ProcessCommonJsModulesCallback.visit(ProcessCommonJSModules.java:245)
at com.google.javascript.jscomp.NodeTraversal.traverseBranch(NodeTraversal.java:623)
at com.google.javascript.jscomp.NodeTraversal.traverse(NodeTraversal.java:297)
at com.google.javascript.jscomp.NodeTraversal.traverseEs6(NodeTraversal.java:564)
at com.google.javascript.jscomp.ProcessCommonJSModules.process(ProcessCommonJSModules.java:85)
at cljs.closure$eval5486$fn__5487.invoke(closure.clj:1541)
at clojure.lang.MultiFn.invoke(MultiFn.java:233)
at cljs.closure$write_javascript.invokeStatic(closure.clj:1601)
at cljs.closure$write_javascript.invoke(closure.clj:1578)
at cljs.closure$source_on_disk.invokeStatic(closure.clj:1624)
at cljs.closure$source_on_disk.invoke(closure.clj:1619)
at cljs.closure$output_unoptimized$fn__5536.invoke(closure.clj:1662)
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.RT.seq(RT.java:521)
at clojure.core$seq__4357.invokeStatic(core.clj:137)
at clojure.core$filter$fn__4812.invoke(core.clj:2700)
at clojure.lang.LazySeq.sval(LazySeq.java:40)
at clojure.lang.LazySeq.seq(LazySeq.java:49)
at clojure.lang.RT.seq(RT.java:521)
at clojure.core$seq__4357.invokeStatic(core.clj:137)
at clojure.core$map$fn__4785.invoke(core.clj:2637)
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$str$fn__4419.invoke(core.clj:546)
at clojure.core$str.invokeStatic(core.clj:544)
at clojure.core$str.doInvoke(core.clj:533)
at clojure.lang.RestFn.applyTo(RestFn.java:139)
at clojure.core$apply.invokeStatic(core.clj:646)
at clojure.core$apply.invoke(core.clj:641)
at cljs.closure$deps_file.invokeStatic(closure.clj:1340)
at cljs.closure$deps_file.invoke(closure.clj:1337)
at cljs.closure$output_deps_file.invokeStatic(closure.clj:1360)
at cljs.closure$output_deps_file.invoke(closure.clj:1359)
at cljs.closure$output_unoptimized.invokeStatic(closure.clj:1670)
at cljs.closure$output_unoptimized.doInvoke(closure.clj:1653)
at clojure.lang.RestFn.applyTo(RestFn.java:139)
at clojure.core$apply.invokeStatic(core.clj:648)
at clojure.core$apply.invoke(core.clj:641)
at cljs.closure$build.invokeStatic(closure.clj:1981)
at cljs.closure$build.invoke(closure.clj:1882)
at cljs.build.api$build.invokeStatic(api.clj:210)
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 cljsbuild.compiler$compile_cljs$fn__5771.invoke(compiler.clj:60)
at cljsbuild.compiler$compile_cljs.invokeStatic(compiler.clj:59)
at cljsbuild.compiler$compile_cljs.invoke(compiler.clj:48)
at cljsbuild.compiler$run_compiler.invokeStatic(compiler.clj:168)
at cljsbuild.compiler$run_compiler.invoke(compiler.clj:122)
at user$eval5908$iter_59445948$fn5949$fn_5967.invoke(form-init8819033363931377476.clj:1)
at user$eval5908$iter_59445948$fn_5949.invoke(form-init8819033363931377476.clj:1)
at clojure.lang.LazySeq.sval(LazySeq.java:40)
at clojure.lang.LazySeq.seq(LazySeq.java:49)
at clojure.lang.RT.seq(RT.java:521)
at clojure.core$seq__4357.invokeStatic(core.clj:137)
at clojure.core$dorun.invokeStatic(core.clj:3024)
at clojure.core$doall.invokeStatic(core.clj:3039)
at clojure.core$doall.invoke(core.clj:3039)
at user$eval5908.invokeStatic(form-init8819033363931377476.clj:1)
at user$eval5908.invoke(form-init8819033363931377476.clj:1)
at clojure.lang.Compiler.eval(Compiler.java:6927)
at clojure.lang.Compiler.eval(Compiler.java:6917)
at clojure.lang.Compiler.load(Compiler.java:7379)
at clojure.lang.Compiler.loadFile(Compiler.java:7317)
at clojure.main$load_script.invokeStatic(main.clj:275)
at clojure.main$init_opt.invokeStatic(main.clj:277)
at clojure.main$init_opt.invoke(main.clj:277)
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.NullPointerException
... 85 more

[1] https://www.npmjs.com/package/showdown



 Comments   
Comment by Juan Pedro Bolivar Puente [ 24/May/16 12:39 PM ]

Forgot to mention, this is the line in my :foreign-libs

{:file "node_modules/showdown/dist/showdown.js"
:provides ["showdown"]
:module-type :commonjs}

Same problem occurs if I use showdown.min.js





[CLJS-1645] Bug in closure/compiled-file Created: 24/May/16  Updated: 26/May/16  Resolved: 26/May/16

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

Type: Defect Priority: Minor
Reporter: Mike Kaplinskiy Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: File patch.diff    
Patch: Code

 Description   

It's currently referencing the wrong namespace on a ::keyword.



 Comments   
Comment by David Nolen [ 26/May/16 3:17 PM ]

this is not a bug, it's intentional





[CLJS-1644] Recursive protocol method for a record never terminates Created: 23/May/16  Updated: 26/May/16

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

Type: Defect Priority: Minor
Reporter: Brian Stiles Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Any



 Description   

When extending a protocol for a record, a recursive method can never terminate in some cases.

(defrecord R [a])

(defprotocol P
  (f [x]))

(extend-protocol P
  R
  (f [x]
    (if x
      (recur nil)
      x))
  default
  (f [x]
    (if x
      (recur nil)
      x)))

(prn (f 1))        ; #1
(prn (f (R. 1)))   ; #2

prn call #1 prints nil as expected, but prn call #2 never terminates.

It looks like the compiler creates a variable assigned to this within the while loop such that the test of "x" is always really testing this when it should be testing the value of x passed in by the call to recur.

Note, I'm testing ClojureScript 1.8.51. The "Affects Version/s" field above only gives 1.7.228 as the most recent version.



 Comments   
Comment by Brian Stiles [ 23/May/16 4:14 AM ]

Actually, "always really testing 'this' when it should be testing the value of x passed in by the call to recur" is only true if the type of the value represented by x remains the same. If, as in the example, the type changes, the call should be dispatched to the properly matching method implementation.

This seems to behave as expected in Clojure.





[CLJS-1643] Emit more informative error when emitting a type which has no emit multimethod case Created: 21/May/16  Updated: 21/May/16

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

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


 Description   

Users may accidentally splice in Clojure functions and other things in macros that work in Clojure which cannot work in ClojureScript. We may want to include a default warning for the most common/likely error cases.






[CLJS-1642] cljs.core/reductions does not respect 'reduced'. Created: 21/May/16  Updated: 21/May/16  Resolved: 21/May/16

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

Type: Defect Priority: Major
Reporter: Brian Stiles Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None
Environment:

Any



 Description   

The following should return ([] [1] [1 2] [1 2 3] :stop) but throws an exception because the cljs.core/Reduced instance is passed to conj.

(reductions
(fn [xs x]
(if (= 4 x)
(reduced :stop)
(conj xs x)))
[] [1 2 3 4 5])

Same problem as existed in Clojure: http://dev.clojure.org/jira/browse/CLJ-1185

The fix in the patch attached to CLJ-1185 should suffice.



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

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





[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: https://github.com/clojure/clojurescript/blob/master/src/main/clojure/cljs/compiler.cljc#L812

Comment by Francis Avila [ 11/Jun/16 8:33 AM ]

From what I can see copy-arguments is only ever used with an immediately-constructed empty array as the dest in the pattern:

`(let [arr# (array)]
  (copy-arguments arr#)
  ;;...
  )

Perhaps it should change to have the exact same behavior as emit-arguments-to-array, i.e. it should take a start index and expand to the name of the destination array. The advantages of this approach are 1) it can preallocate the array to the correct size and 2) your patch no longer needs a slice call--you can avoid allocating two arrays instead of just one. (These two reasons are why I implemented emit-arguments-to-array the way I did.)

I can't think of a way to implement emit-arguments-to-array as a macro without emitting a wrapping js function which messes up the scope, but you could do this:

(defmacro copy-arguments [dest-arr startslice]
  `(loop [i# 0]
     (when (< i# (alength ~dest-arr))
       (aset ~dest-arr i# (aget (js-arguments) (+ i# startslice)))
       (recur (inc i#)))))

And then at the callsite:

`(let [variadic-args-arr# (make-array (- (alength (js-arguments)) ~maxfa))]
  (copy-arguments variadic-args-arr# ~maxfa)
    (let [argseq# (new ^::ana/no-resolve cljs.core/IndexedSeq
                       variadic-args-arr# 0 nil)]
      ;; ...
    ))

However, there are some bugs around arity handling that the above would not solve: CLJS-1678

Comment by Stephen Brady [ 11/Jun/16 8:27 PM ]

Francis, thanks for commenting on this. The patch that I submitted simply moves where/when `copy-arguments` is called. Other than that, I preserved all other aspects of the existing implementation, including how the array is built up and then made into an IndexedSeq. The line diffing in the patch implies that I changed a lot more than I really did. Agreed with your point that `emit-arguments-to-array` is more efficient and precise. Intentionally, I did not try to alter/improve/correct anything in this patch beyond solving the objective in the issue: not unnecessarily copying arguments.

Glad you've reported CLJS-1678 as I've observed this too. This buggy behavior shows up in several places. Beyond what this issue-patch attempts to address, in general, my observation is that we could probably clear out some under-brush that's accumulated as the compiler has matured with regards to arguments handling and code generated for multi-arity and/or variadic functions, apply / applyTo, and implementations of IFn. Seems like there are several opportunities to emit less javascript, create fewer intermediates, and shorten the call chain.

So to reiterate, this patch - despite its superficial appearance - changes very little and just moves the call to copy-arguments to the appropriate place. The benefit is:

For multi-arity functions with no variadic arity, no code for copying the arguments to an array is emitted at all (which in aggregate turns out to be a decent amount). Obviously, at runtime, no array will be created.

For multi-arity functions with a variadic arity, the code for copying the arguments remains but is isolated to the variadic case, and so if the function is called but will dispatch to one of the fixed arities, again, no array will be created.





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

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

Type: Defect Priority: Major
Reporter: Mike Kaplinskiy Assignee: David Nolen
Resolution: Unresolved 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.





[CLJS-1639] Invalid file path for cljs.core.load_file on windows Created: 13/May/16  Updated: 21/May/16

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

Type: Defect Priority: Minor
Reporter: Jay Lee Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: bug
Environment:

Windows 10, CLJS 1.8.51



 Description   

When I tried to build reagent based app with nodejs target,
I got an invalid file path generated case which is basically loading external javascript file.
I captured the image as following:

At line 4, the path should have double backward-slash on windows.

I has built a CLJS app which is based on reagent framework with nodejs target. The build environment is somewhat strange but I have a case to use it. Here is a reproduce steps.

  1. Open command prompt on windows 10 and execute command as following:
    lein new figwheel sample00 – --reagent
  2. Open project.clj file and update one of dependencies:
    project.clj
    ;; ...
    :dependencies [[org.clojure/clojure "1.8.0"]
                     [org.clojure/clojurescript "1.8.51"]
                     [org.clojure/core.async "0.2.374"
                      :exclusions [org.clojure/tools.reader]]
    
                     [reagent "0.6.0-alpha" :exclusions [cljsjs/react]]
                     [cljsjs/react-with-addons "0.14.3-0"]
                     ]
    
    ;; ...
    :cljsbuild {:builds [{:id "dev"
                          ;; ... 
                          :compiler {
                            ;; ...
                            :target :nodejs
                          }
                         }]
               }
    ;; ...
  3. Build with lein cljsbuild once dev
  4. Open <project_root>\resources\public\js\compiled\out\reagent\impl\util.js
  5. At line number 4 in my environment, the generated code is
    cljs.core.load_file("resources\public\js\compiled\out\react-with-addons.inc.js");
    However I believe the correct path string should be cljs.core.load_file("resources\\public\\js\\compiled\\out
    react-with-addons.inc.js");
    .

Backward-slash needs to be double on Windows env.
When I lunched doo test command with nodejs target, it complained about the given path cannot be loaded.

Thanks.



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

This ticket is in danger of being closed. The ticket should demonstrate a reproducible bug without relying on any 3rd party tools or libraries. No Leiningen, Figwheel, or Reagent. Please demonstrate the Windows issue with only ClojureScript.

Thanks.





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

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

Type: Defect Priority: Major
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved 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.





[CLJS-1637] Missing docstrings for a few vars Created: 30/Apr/16  Updated: 06/May/16  Resolved: 06/May/16

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

Type: Defect Priority: Trivial
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: docstring

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

 Description   

Docstrings are missing for a few vars, where Clojure has docstrings that can be copied verbatim or used as a basis for ClojureScript:

symbol
newline
special-symbol?
defonce



 Comments   
Comment by David Nolen [ 06/May/16 1:45 PM ]

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





[CLJS-1636] Mark some symbols in core macros ns as private Created: 27/Apr/16  Updated: 27/Apr/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.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.)





[CLJS-1635] Var type implements IEquiv but not IHash Created: 26/Apr/16  Updated: 06/May/16  Resolved: 06/May/16

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

Type: Defect Priority: Minor
Reporter: Chris Vermilion Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: deftype
Environment:

Tested on OS X 10.11, Chrome.


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

 Description   

The Var type implements IEquiv based on the var's symbol, but not IHash. That means that two vars with the same symbol compare equal but don't hash equal, which will cause strange results if you put them in a hash-{map,set}:

cljs.user=> (def foo "bar")
#'cljs.user/foo
cljs.user=> (= #'foo #'foo)
true
cljs.user=> (= (hash #'foo) (hash #'foo))
false

Patch forthcoming.



 Comments   
Comment by Chris Vermilion [ 26/Apr/16 10:41 PM ]

Patch note: The patch fixes the issue but I haven't added a test. It didn't seem like the hash behavior of basic types was tested in general, but moreover while I think this behavior is desirable I'm not sure it should be guaranteed. Happy to write a test if that would be useful.

Comment by Chris Vermilion [ 26/Apr/16 10:48 PM ]

Aside for the curious on how this came up at all: the Schema library uses Vars to specify recursive schemas, and does internal caching by with a map keyed by schemas themselves. If you defined the same recursive schema multiple times, the results would be unpredictable, since two equivalent recursive schemas would compare equal but wouldn't necessarily be interchangeable as map keys.

Comment by David Nolen [ 06/May/16 1:42 PM ]

Chris have you submitted your CA yet?

Comment by Chris Vermilion [ 06/May/16 2:02 PM ]

Yep

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

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





[CLJS-1634] Track bound dynamic variables to support binding in async mechanisms. Created: 26/Apr/16  Updated: 27/May/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: 0
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.





[CLJS-1633] Improve error associated with invalid foreign-libs :file path Created: 26/Apr/16  Updated: 06/May/16

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

Type: Enhancement Priority: Minor
Reporter: Oliver George Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 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.





[CLJS-1632] Typos, c/e, and consistency of docs/params Created: 24/Apr/16  Updated: 25/Apr/16  Resolved: 25/Apr/16

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

Type: Enhancement Priority: Trivial
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

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

 Description   

Fixes typos, consistency, making params match docs, etc.

Covers what was previously CLJS-1510, CLJS-1522, CLJS-1526, CLJS-1534, CLJS-1535, and other similar changes.



 Comments   
Comment by David Nolen [ 25/Apr/16 6:15 AM ]

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





[CLJS-1631] The str function should handle JavaScript symbols Created: 21/Apr/16  Updated: 21/Apr/16

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

Type: Enhancement Priority: Minor
Reporter: Roman Scherer Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

The str function should handle primitive JavaScript symbols produced by Symbol.for. At the moment the str function raises an exception, because it runs into some JavaScript safety checks by using implicit string coersion via (.join #js [x] "").
More info on the safety check here: http://www.2ality.com/2014/12/es6-symbols.html
This ticket is also related to:

(def x (.for js/Symbol "x"))
(str x)
TypeError: Cannot convert a Symbol value to a string
    at Array.join (native)
    at Function.cljs.core.str.cljs$core$IFn$_invoke$arity$1 (/home/roman/workspace/clojurescript/.cljs_node_repl/cljs/core.js:9561:12)
    at Object.cljs$core$str [as str] (/home/roman/workspace/clojurescript/.cljs_node_repl/cljs/core.js:9543:22)
    at repl:1:100
    at repl:9:3
    at repl:14:4
    at Object.exports.runInThisContext (vm.js:54:17)
    at Domain.<anonymous> ([stdin]:41:34)
    at Domain.run (domain.js:228:14)
    at Socket.<anonymous> ([stdin]:40:25)

Calling the toString method on a symbol directly works

(.toString x)
;;=> "Symbol(x)"





[CLJS-1630] Add unit test for static dispatch Created: 21/Apr/16  Updated: 23/Apr/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 .






[CLJS-1629] Fix warning about duplicate test-pr-str Created: 20/Apr/16  Updated: 06/May/16  Resolved: 06/May/16

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

Type: Enhancement Priority: Trivial
Reporter: Roman Scherer Assignee: Roman Scherer
Resolution: Completed Votes: 0
Labels: None

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

 Description   

There are 2 definitions of test-pr-str in core_test.cljs. The 2nd definition shadows the 1st which results in some tests not beeing run.



 Comments   
Comment by Roman Scherer [ 20/Apr/16 7:59 AM ]

The attached patch moves all pr-str tests into a single test-pr-str definition.

Comment by Mike Fikes [ 26/Apr/16 2:02 PM ]

Hey Roman, now these execute, they fail in bootstrap. Try running script/test-self-parity.

It looks like the root cause is that some of the test rely on ordering in hash-based collections. (A similar fix for this kind of stuff was done in CLJS-1592.)

Comment by Roman Scherer [ 26/Apr/16 3:10 PM ]

Mike, I updated the patch and the tests now also run in bootstrap. I also changed the test name to test-printing and added testing sections for each kind of print.

Comment by Mike Fikes [ 26/Apr/16 3:20 PM ]

LGTM

Comment by David Nolen [ 06/May/16 1:55 PM ]

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





[CLJS-1628] Make instances of js/Symbol printable Created: 20/Apr/16  Updated: 21/Apr/16

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

Type: Enhancement Priority: Minor
Reporter: Roman Scherer Assignee: Roman Scherer
Resolution: Unresolved Votes: 0
Labels: None

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

 Description   

At the moment instances of js/Symbol are not printable. Trying to
print them or enter them in the REPL causes an exception.

cljs.user> (.for js/Symbol "react.element")
#object[TypeError TypeError: Cannot convert a Symbol value to a string]

Symbols are supported in all major browsers, except Internet Explorer and Nashorn.
https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Symbol



 Comments   
Comment by Roman Scherer [ 20/Apr/16 10:23 AM ]

The attached patch adds an additional clause to `pr-writer-impl` and
implements the printing of Symbol instances, in the same way as it is
done for other JavaScript objects. Here's an example of a printed
js/Symbol:

(.for js/Symbol "react.element")
;;=> #object[Symbol "react.element"]

@david: Regarding your point about shims, do you think the
implementation of `js-symbol-defined?`, which was used for the ES6
iterator support, is enough for this patch? I'm not too familiar with
JavaScript and not sure if this already addressed the "shim" issue.

Another thing I stumbled upon is, that my test currently generates a
compiler warning when using the default compiler options. The warning
is generated when compiling the following ClojureScript form:

(.for js/Symbol "react.element")

The following snippet shows the warning and the generated code from my
test:

WARNING - Keywords and reserved words are not allowed as unquoted
property names in older versions of JavaScript. If you are targeting
newer versions of JavaScript, set the appropriate language_in option.

try{var values__13328__auto__ = (function (){var x__6628__auto__ = cljs.core.pr_str.cljs$core$IFn$_invoke$arity$variadic(cljs.core.array_seq([Symbol.for("react.element")], 0));

I think this has nothing to do with this patch, but with the emitted
code not being legal Ecmascript 3, since "for" is a reserved word.

The warning goes away when changing the :language-in option to
something newer than Ecmascript 3, or doing something like this:

((gobj/get js/Symbol "for") "react.element")

So, the questions is: Should the ClojureScript compiler handle those
reserved words when they appear in a function call or property lookup?

If that's the case I would leave the warning in that patch, and open
another issue for this new problem.

What do you think?

Roman

Comment by Roman Scherer [ 20/Apr/16 11:33 AM ]

Ok, given advanced mode and renaming of symbols I think there's not much the ClojureScript compiler can do about this.
I think I'll change the test to use ((gobj/get js/Symbol "for") "react.element") to remove the warning.
https://github.com/google/closure-compiler/wiki/FAQ#i-get-invalid-property-id-errors-but-it-works-on-firefox

Comment by David Nolen [ 20/Apr/16 2:21 PM ]

ECMAScript 3 is not an issue for people who are ok with outputting ECMAScript 5. The goog.typeOf of check on "symbol" does not seem like it would work with Symbol shims, have you confirmed?

Comment by Roman Scherer [ 21/Apr/16 10:57 AM ]

David, yes you are right about goog.typeOf, it returns "object" when
js/Symbol is implemented via shims. I updated the patch and tested it
against the following shim implementation in Nashorn so far.

Start a Nashorn REPL.

./script/nashornrepljs

js/Symbol does not exist in Nashorn yet.

(exists? js/Symbol)
;;=> false

Load core-js shim into Nashorn.

(js/load "https://raw.githubusercontent.com/zloirock/core-js/master/client/shim.min.js")
(print (.for js/Symbol "x"))
;;=> #object[Symbol "x"]

I gave up on this shim implementation because I got an error when
trying to load the minified js into Nashorn.

https://github.com/medikoo/es6-symbol

This is another shim implementation, but it only enhances js/Symbol
when it already exists, so not useful in the context of Nashorn.

https://github.com/paulmillr/es6-shim

Comment by Roman Scherer [ 21/Apr/16 2:07 PM ]

Which implementations do support js/Symbol?

JavaScriptCore, NodeJS, SpiderMonkey, Chrome and Firefox support
js/Symbol. Evaluating (instance? js/Symbol (.for js/Symbol "x")) on
those engines returns false. On all those implementations symbols
generated via (.for js/Symbol "x") are primitive values and can be
identified via typeof. See the "Using the typeof operator with
symbols" section in [1].

Nashorn and Rhino do not support js/Symbol.

[1] https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Symbol

Comment by Roman Scherer [ 21/Apr/16 2:27 PM ]

Looking around I found that the exception caused by printing symbols
happens because the str [1] function uses implicit string conversion
to convert it's argument into a string. This is explained in the
"Coercing a symbol to string" section of [2]. One way to solve the
problem is to use (.toString x) instead.

(.join #js [(.for js/Symbol "x")] "")
;;=> TypeError: Symbol is not a constructor

(.toString (.for js/Symbol "x"))
;;=> "Symbol(x)"

[2] https://github.com/clojure/clojurescript/blob/master/src/main/cljs/cljs/core.cljs#L2695
[2] http://www.2ality.com/2014/12/es6-symbols.html

Comment by Roman Scherer [ 21/Apr/16 2:38 PM ]

(= (goog/typeOf x) "symbol") vs (instance? js/Symbol x)

Symbols are primitive types and must be constructed via
Symbol("x"). They can't be constructed via new Symbol("x"), trying
to do so raises a TypeError. This is explained in more detail in the
"Safety checks" section of [1]. The "Using the typeof operator with
symbols" section of [2] explains how to identify symbols. It has to be
done via typeof, because symbols are primitive types and not
instances of Symbol.

[1] http://www.2ality.com/2014/12/es6-symbols.html
[2] https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Symbol

Comment by Roman Scherer [ 21/Apr/16 3:02 PM ]

I think we have 2 options now to fix exceptions when printing symbols:

  • Change str to handle symbols as well.
  • Implement custom printing for symbols.

I think I would go for the first option. I think calling str on a
symbol can't be seen as implicitly coercing a symbol to a string, I
would say this is as explicit as it can get and we don't have to raise
an exception in this case. I think str is basically the Clojure
equivalent to toString in JavaScript, at least this is what the out
of date doc string of str says.

The file CLJS-1628-fix-str has an implementation of the first option.

Comment by Francis Avila [ 21/Apr/16 3:35 PM ]

Changing str to use .toString is not possible because of CLJS-847: there are unconfirmable reports that .toString() causes TypeError on Safari 6.0.5.

This problem has spilled over into the unresolved CLJS-890. This is a long thread, so see my recap comment, my failed efforts to reproduce the bug and my proposed final patch, which can be easily extended for the symbol case with a new "symbol" clause in the case statement.

Comment by Roman Scherer [ 21/Apr/16 4:20 PM ]

Thanks Francis, I'll take a look at this discussion.

Comment by Roman Scherer [ 21/Apr/16 5:12 PM ]

Since one can still make so called Symbol wrapper objects via the
Object() function [1], I added one more test case for this:

(js/Object (.for js/Symbol "x"))

Also the updated patch defines the test case only if js/Symbol exists?
in the JavaScript engine.

[1] https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Symbol





[CLJS-1627] jsdoc parsing fails to recognize union types, breaking resolution Created: 18/Apr/16  Updated: 08/Jun/16

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

Type: Defect Priority: Minor
Reporter: Patrick Killean Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: patch

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

 Description   

The Closure Spec For Union Types states that parentheses are necessary for union type expressions. Trying this ...

(defn foo
  "@param {(IBar|IMap)} x"
  [x] 
  ...)

Raises a Closure Error :

...ERROR - Bad type annotation. expected closing }
* @param {user.(IBar|user.IMap)}

This is because comp/resolve-types treats the parentheses as a part of the type tokens and incorrect var resolution occurs as a result. In addition, the compiler emits multiple resolved types separated by "|" characters but does not enclose them in parentheses to create a valid union type.



 Comments   
Comment by Patrick Killean [ 18/Apr/16 4:36 PM ]

This patch includes:

  • comp/resolve-types now removes parentheses when present and emits them when >1 type is detected. This makes parenthesis use optional and existing code remains unbroken (with the added benefit that it may work now)
  • changes to comp/resolve-type
    1. checks for js globals like document or window which are recognized by closure
    2. allows dot.delimited.forms to pass through so we can use types defined in externs and avoid unnecessary resolution
    3. uses ana/resolve-existing-var with a "unresolved jsdoc type" warning
    4. checks if a resolved var is a protocol and warns otherwise. This is more informative than Closure's standard unrecognized type error
  • a test for comp/resolve-types
Comment by David Nolen [ 21/Apr/16 12:45 PM ]

Thanks will try to look more closely at this tomorrow.

Comment by David Nolen [ 23/Apr/16 2:03 PM ]

The patch is getting there, please remove the `js-doc-type` meta stuff. Just extend the signature of resolve-existing-var to take an additional parameter - the confirm-var-exists handler.

Comment by Patrick Killean [ 09/May/16 4:58 PM ]

CLJS-1627-1.patch:
resolve-existing-var now has an additional arity that accepts a missing-var handler passed to confirm-existing-var

Comment by Patrick Killean [ 10/May/16 6:16 AM ]

This has revealed a problem where deftype + defrecord using Object protocols emit resolved names when really they shouldn't. For example : "@implements {cljs.core.async.impl.timers.Object}" --> Bad Type Annotation

Since Object is a special case simply excluding it from the comments should fix it. Another patch incoming

Comment by Patrick Killean [ 10/May/16 7:42 AM ]

CLJS-1627-2.patch:
The emit* methods for deftype and defrecord now filter out Object protocols.

This produced an interesting result! With no more bad type annotations, static analysis can now proceed... and it has alot to say. Theres all kinds of info now about arity discrepencies (particularly cljs.core.IndexedSeq), type mismatches, and more. It even includes a type coverage percentage. Lots to parse here but very cool.

Comment by Patrick Killean [ 18/May/16 4:26 PM ]

CLJS-1627-3.patch:

  • fix require extern
  • add type application support for Array & Object
  • GC likes uppercase for Object & Array, lowercase for string, number.
  • support for explicit nullable types, variable typed arg
  • function type context modifiers this + new

Missing is the GC 'record type' . It also may be useful to fill out the node externs for common types

Comment by Patrick Killean [ 20/May/16 11:42 AM ]

CLJS-1627-4.patch:

  • fix a few problems in last patch
  • add record type support. Everything here should be covered




[CLJS-1626] cljs.test for bootstrap Created: 18/Apr/16  Updated: 21/Apr/16  Resolved: 21/Apr/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-1626-1.patch    
Patch: Code

 Description   

Make it so that the cljs.test namespace can work with bootstrap ClojureScript.

Currently the known obstacles that prevent this largely surround macros, but they are known to be solvable. (It has been done successfully downstream in Planck.)

Primary goals:

  • Preserve existing functionality and performance for regular JVM ClojureScript users of cljs.test
  • Make it so that cljs.test namespace can be loaded in any bootstrap environment that makes use of cljs.js and implements load functions supporting macros.
  • Add scripted facilities so that the ClojureScript compiler's unit tests can be executed (perhaps in Node if installed) as an auxiliarry test capability (like script/test but for self-host)

Secondary goals:

  • Ensure that it is possible for downstream ClojureScript projects that target bootstrap can make use cljs.test to run their test suites in bootstrap mode. (The ClojureScript compiler wouldn't provide any scripts for this—it would just strive to ensure that this is possible.)
  • Support custom asserts assert-expr in bootstrap mode which are written in ClojureScript (as compared to Clojure for the JVM-based approach). (This has been shown to be possible with downstream Planck.)

Rationale:

While bootstrap is of lower priority than regular JVM-based ClojureScript, it is still a burden to support it: It is easy to make a change to the compiler, but forget a conditional branch for bootstrap, or some such, thus causing a regression for bootstrap. With this capability, we'd have the ability to run the very same compiler unit tests in bootstrap mode, thus drastically reducing the support burden for bootstrap's existence.

Secondarily, this would help downstream libraries confidently target bootstrap as an environment by facilitating those libraries in running their own unit tests using cljs.test.



 Comments   
Comment by Mike Fikes [ 20/Apr/16 2:10 PM ]

Attaching CLJS-1616-1.patch for feedback. (It might be good to go.)

It makes some very minor changes to the production code so that cljs.test can be loaded in bootstrap environments.

The renames

src/main/cljs/cljs/test.clj → src/main/cljs/cljs/test.cljc
src/main/clojure/cljs/analyzer/api.clj → src/main/clojure/cljs/analyzer/api.cljc

actually involve very few changes after renames.

In addition to running the regular ClojureScript tests, I've confirmed that you can build an uberjar and still properly {{(require '[cljs.test :refer-macros [deftest is]])}} in a REPL. I've also confirmed that tools.reader, which runs CLJS tests can still do so with these changes.

In addition it adds a new script/test-self-parity which will run (most of) the compiler unit tests in bootstrap mode in Node. This is done by bringing up an :optimizations :none environment in Node and using the cljs.js capability to load the compiler unit tests along with cljs.test. Of interest is that this script pulls out the clojure.template namespace so it can be loaded. (Downstream projects that might end using script/test-self-parity as an example of how to run their own unit tests in bootstrap mode would need to do something similar.)

(Note: If we can come up with a better name than "parity", willing to revise it. script/test-self-host was taken.)

This patch does not address writing custom assert assert-expr in ClojureScript by treating the defaulti as being in the cljs.test namespace (as is illustrated in http://blog.fikesfarm.com/posts/2016-02-25-custom-test-asserts-in-planck.html), but I did confirm that you can easily achieve the same by simply working with cljs.test$macros/assert-expr.

Comment by Mike Fikes [ 21/Apr/16 10:14 AM ]

I've tested this patch using a downstream example repository that illustrates testing a library targeting bootstrap. It also makes use of a custom test assert. This shows that all of the secondary goals in the ticket description are met.

(The downstream repo, for those interested: https://github.com/mfikes/titrate)

Comment by David Nolen [ 21/Apr/16 12:43 PM ]

fixed https://github.com/clojure/clojurescript/commit/28e040d41f3a83901b6391898f51c4bfc2622452





[CLJS-1625] Clojurescript macros used in named function are expanded two times because the analyzer performs a two pass analysis when analyzing named functions Created: 16/Apr/16  Updated: 19/Apr/16

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

Type: Defect Priority: Minor
Reporter: Ewen Grosjean Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-1625.patch    

 Comments   
Comment by Ewen Grosjean [ 16/Apr/16 9:41 AM ]

During the first analysis of named functions, only the function definition is analyzed in order to know its function-ness/arity. Its body is only analyzed during the second pass.

Comment by Kevin Downey [ 17/Apr/16 12:09 AM ]

http://dev.clojure.org/jira/browse/CLJS-1617 seems to add a similar issue





[CLJS-1624] Avoid the use of JSC_HOME for tests Created: 15/Apr/16  Updated: 19/Apr/16  Resolved: 19/Apr/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: None

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

 Description   

For script/test, we have devs set JSC_HOME and make use of this to decide when to run tests with jsc. (See https://github.com/clojure/clojurescript/wiki/Running-the-tests)

There is a recent change in WebKit (http://trac.webkit.org/changeset/194606) which causes a warning to appear in command-line apps that make use of JavaScriptCore (Planck is an example), essentially indicating that JCS_HOME=/path... is not a valid option.

It turns out that JSC_HOME is not needed in order to run jsc. If we simply avoid using JSC_HOME altogether, and instead check if jsc is on the path (which occurs when the PATH is set up per the Running the Tests wiki page, then all is good.



 Comments   
Comment by Mike Fikes [ 15/Apr/16 9:26 AM ]

Successfully tested the changes in CLJS-1624.patch on both OS X and Linux. For Linux, tested on Ubuntu 14.04 with the libjavascriptcoregtk-3.0-bin package installed, and it finds jsc on the path and the compiler unit tests successfully run.

Comment by David Nolen [ 19/Apr/16 2:11 PM ]

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





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

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

Type: Defect Priority: Minor
Reporter: Justin Cowperthwaite Assignee: Unassigned
Resolution: Unresolved 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.





[CLJS-1622] `with-redefs` can cause `&` argument to be assigned incorrectly under advanced compilation Created: 12/Apr/16  Updated: 12/Apr/16

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

Type: Defect Priority: Minor
Reporter: Peter Jaros Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Given the following:

(ns with-redefs-bug.core)

(enable-console-print!)

(defn a-function [arg-1 arg-2 & args]
  nil)

(with-redefs [a-function (fn [& args] args)]
  (prn (a-function :arg-1))
  (prn (a-function :arg-1 :arg-2))
  (prn (a-function :arg-1 :arg-2 :arg-3)))

Under :optimizations :none, this code correctly prints:

(:arg-1)
(:arg-1 :arg-2)
(:arg-1 :arg-2 :arg-3)

However, under :optimizations :advanced, this code prints:

(:arg-1)
(:arg-1 :arg-2)
:arg-1

That is, as long as the function is called with exactly or less than the number of non-variadic arguments in the original function bound to a-function, args is (correctly) bound to a seq of all the arguments, but if any more arguments are given, args is bound to the first argument.

Also, under either compilation, the following warning is generated:

WARNING: Wrong number of args (1) passed to with-redefs-bug.core/a-function at line 9

That surprises me, but since it happens under both methods, perhaps it's intentional.



 Comments   
Comment by Peter Jaros [ 12/Apr/16 4:21 PM ]

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

Comment by Mike Fikes [ 12/Apr/16 8:24 PM ]

This is actually not specific to :optimizations :advanced, but to :static-fns true.





[CLJS-1621] Foreign libs modules of different type doesn't compile together Created: 09/Apr/16  Updated: 19/Apr/16  Resolved: 19/Apr/16

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

Type: Defect Priority: Minor
Reporter: Roman Liutikov Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

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

 Description   

Given the following config

{:file "my-modular-lib/logger.js"
  :provides ["logger"]
  :module-type :commonjs}
 {:file "my-modular-lib/index.js"
  :provides ["msglogger"]
  :module-type :es6}

and these JavaScript modules

logger.js

exports.default = function log(msg) {
  return console.log(msg);
};

index.js

import log from './logger';

export function logMsg(msg) {
  return log(msg);
};

compiler throws an error

ERROR: JSC_ES6_MODULE_LOAD_ERROR. Failed to load module "./logger" at my-modular-lib/index.js line 1 : 10

Closure Compiler is not able to find logger.js because it is not included into compilation process of index.js file.
This happens because foreign libs are being filtered out by module type at closure.clj#L1501
Changing filtering to something like this: (filter #(or (= (:module-type %) :es6) (= (:module-type %) :commonjs))) makes build pass successfully.



 Comments   
Comment by Roman Liutikov [ 11/Apr/16 2:55 AM ]

This patch allows compiling foreign libs with dependencies of different module types (AMD, CommonJS, and ES6).

Comment by Mike Fikes [ 13/Apr/16 11:44 AM ]

I successfully tested this in 2 ways:

1) It doesn't break downstream bootstrap Planck (theoretically it can't, but tested anyway).
2) I tested to ensure that this works dynamically in a REPL (per CLJS-1313). There is one probably innocuous warning, but things appear to work properly:

$ java -jar cljs.jar 
Clojure 1.8.0
user=> (require
  '[cljs.repl :as repl]
  '[cljs.repl.node :as node])
nil
user=> (def foreign-libs [{:file "my-modular-lib/logger.js"
  :provides ["logger"]
  :module-type :commonjs}
 {:file "my-modular-lib/index.js"
  :provides ["msglogger"]
  :module-type :es6}])
#'user/foreign-libs
user=> (cljs.repl/repl (node/repl-env) :foreign-libs foreign-libs)
ClojureScript Node.js REPL server listening on 49586
WARNING: JSC_INVALID_ES3_PROP_NAME. Keywords and reserved words are not allowed as unquoted property names in older versions of JavaScript. If you are targeting newer versions of JavaScript, set the appropriate language_in option. at my-modular-lib/logger.js line 1 : 8
To quit, type: :cljs/quit
cljs.user=> (require '[msglogger :refer [logMsg]])
nil
cljs.user=> (logMsg "Hi")
Hi
nil
Comment by Roman Liutikov [ 13/Apr/16 2:02 PM ]

I have also tested it with Leiningen. Having JS modules which depends on modules of different type compilation is successfull without optimizations and with advanced level.

Comment by Maria Geller [ 13/Apr/16 5:16 PM ]

Also tested it with my personal projects which use JS modules and it looks good for me.

Comment by David Nolen [ 19/Apr/16 2:15 PM ]

fixed https://github.com/clojure/clojurescript/commit/56611b5f653e5b8cbeaff9d975cf9f73b755f9c3





[CLJS-1620] In JavaScript ES2015 modules default export name is munged to default$ Created: 08/Apr/16  Updated: 08/Apr/16

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

Type: Defect Priority: Minor
Reporter: Roman Liutikov Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

When using a foreign lib which is ES2015 module with default export, the value which is being exported is assigned to a property default on a namespace object. In ClojureScript code this means one should call to default var of that namespace. However in complied output of ClojureScript code the name default is getting munged to default$.

export default function inc(v) {
  return v + 1;
}
(ns cljs-example.core
  (:require [lib.inc :as lib]))

(lib/default 0)
goog.provide("module$lib$inc");
function inc$$module$lib$inc(v){return v+1}
module$lib$inc.default=inc$$module$lib$inc
// Compiled by ClojureScript 1.8.40 {}
goog.provide('cljs_example.core');
goog.require('cljs.core');
goog.require('module$lib$inc');
module$lib$inc.default$.call(null,(0));

//# sourceMappingURL=core.js.map


 Comments   
Comment by David Nolen [ 08/Apr/16 2:42 PM ]

One possible path around this is to respect the Closure Compiler language setting when munging instead of blindly munging ECMA-262 keywords. A patch that adopts this approach would be welcome.





[CLJS-1619] cljs.test/is has trouble identifying functions Created: 07/Apr/16  Updated: 06/May/16  Resolved: 06/May/16

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

Type: Defect Priority: Minor
Reporter: Oliver George Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None


 Description   

In clojurescript, the `is` macro doesn't pretty print a predicate test defined with "def".

In the example below, I would expect "(is (amap? []))" to report "actual: (not (amap? []))".

;; using clojure.test
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

user=> (is (map? []))

FAIL in () (form-init8298503787610781742.clj:1)
expected: (map? [])
actual: (not (map? []))

user=> (def amap? map?)
user=> (is (amap? []))

FAIL in () (form-init8298503787610781742.clj:1)
expected: (amap? [])
actual: (not (amap? []))

;; using cljs.test
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

user=> (is (map? []))

FAIL in () (at eval (eval at figwheel$client$utils$eval_helper (http:12:4)
expected: (map? [])
actual: (not (map? []))

user=> (def amap? map?)
user=> (is (amap? []))

FAIL in () (at eval (eval at figwheel$client$utils$eval_helper (http:11:4)
expected: (amap? [])
actual: false



 Comments   
Comment by David Nolen [ 08/Apr/16 1:12 PM ]

Please do not report problems that involve third party tooling. Can you replicate this problem without Figwheel?

Comment by Oliver George [ 08/Apr/16 7:32 PM ]

Hi David

Yes, repeatable with what I believe is a minimal setup. I used the mies template and scripts/build.clj to test with this script.

(ns testerr.core
  (:require [cljs.test :refer-macros [deftest is testing run-tests]]))

(enable-console-print!)

(is (map? []) "Expected a map")

(def amap? map?)

(is (amap? []) "Expected a map")

Output is as before:

FAIL in () (:)
Expected a map
expected: (map? [])
  actual: (not (map? []))

FAIL in () (:)
Expected a map
expected: (amap? [])
  actual: false
Comment by Mike Fikes [ 12/Apr/16 9:27 PM ]

This might pose a challenge—without considering a radical restructuring—in JVM ClojureScript because the not decoration is computed at macro expansion time, where only static analysis metadata is available.

Clojure, of course, has an easier time with this, accessing the runtime. FWIW, I confirmed that bootstrapped ClojureScript can analogously just as easily pull this off via an eval and a fn? check.

Comment by David Nolen [ 06/May/16 1:59 PM ]

Not interested in this one.





[CLJS-1618] `extend-type string` doesnt work without wrapping the string object into `(str)` Created: 07/Apr/16  Updated: 10/Apr/16

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

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


 Description   

(ns my.car)

(defprotocol Car
(drive [this]))

(extend-type js/String
Car
(drive [this] (map #({"a" "A"} %) [this (str this)])))

(drive "a"); (nil "A") expected ("A" "A")

See the reproduction of the bug in a live environment with KLISPE here: http://app.klipse.tech/?sourcce=jira&cljs_in=(ns%20my.car)%0A%0A(defprotocol%20Car%0A%20%20(drive%20%5Bthis%5D))%0A%0A(extend-type%20js%2FString%0A%20%20Car%0A%20%20(drive%20%5Bthis%5D%20(map%20%23(%7B%22a%22%20%22A%22%7D%20%25)%20%5Bthis%20(str%20this)%5D)))%0A%0A%0A(drive%20%22a%22)%0A



 Comments   
Comment by Francis Avila [ 07/Apr/16 6:27 PM ]

This is because of boxing and the implementation of cljs.core._EQ_

By extending type js/String (the String object/class in javascript) instead of "string", your "this" will be the boxed string rather than the primitive string (in non-strict js mode--in strict mode it will be the primitive also). The (str this) is coercing the boxed string back to a primitive string.

The core issue is really:

(= (js/String. "a") "a") ;=> false
;; thus
({"a" "A"} (js/String. "a")) ;=> nil

You should really use

(extend-type string ...)
Comment by Francis Avila [ 07/Apr/16 6:41 PM ]

BTW this appears to be different from Clojure where primitives and boxed-primitives appear equal:

;; Clojure code
(= (String. "a") "a")
=> true
(= (Long. 1) 1)
=> true
(= (Long. 1) (long 1))
=> true

Not sure if clojurescript should try to replicate this more closely or not.

Clojurescript bottoms out with triple-equals in most cases, which is why primitives and boxes do not compare equal. To get them to compare equal would require adding special (instance? js/BOXED x) checks and some modifications to existing -equiv implementations which extend primitive types. (e.g. (extend-type number IEquiv ...) uses identical? without checking if the right-hand side is boxed or not.)

Comment by Mike Fikes [ 10/Apr/16 12:24 AM ]

As Francis alludes to, this is not a bug. If you do (doc extend-type), it indicates the type-sym can be string to cover the base type js/String, and it elaborates with

Note that, for example, string should be used instead of js/String.
and if a user does try to use js/String as the type-sym argument, a diagnostic is issued:

WARNING: Extending an existing JavaScript type - use a different symbol name instead of js/String e.g string at line 1




[CLJS-1617] Evaluation order of arguments to 'list' is right-to-left Created: 07/Apr/16  Updated: 17/Apr/16  Resolved: 08/Apr/16

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

Type: Defect Priority: Major
Reporter: Sam Roberton Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None


 Description   
(do
    (.log js/console "Evaluation order test")
    (let [result (list
                  (or
                    (.log js/console "1")
                    1)
                  (or
                    (.log js/console "2")
                    2))]
      (.log js/console "Result: " (pr-str result))))

prints

Evaluation order test
2
1
Result:  (1 2)


 Comments   
Comment by Kevin Downey [ 07/Apr/16 1:46 PM ]

`list` is sometimes a function and sometimes a macro in clojurecript, I am not sure how the compiler chooses between them, but this is behavior is the result of the macro version.

replacing the macro with something like

(core/defmacro list
  ([] '(.-EMPTY cljs.core/List))
  ([x & xs]
     `(let [x# ~x]
        (-conj (list ~@xs) x#))))

should fix it

Comment by Kevin Downey [ 07/Apr/16 1:49 PM ]

why the macro version exists, I have no idea

Comment by David Nolen [ 08/Apr/16 2:40 PM ]

fixed https://github.com/clojure/clojurescript/commit/29eb8e014f60d57c974fef46f8609ca0be0fd247

Comment by Kevin Downey [ 08/Apr/16 7:28 PM ]

this fix seems like it would cause macros as arguments to list to be expanded twice, which I know causes problems in clojure (a number of the macros in clojure.core have side effects at expansion time), maybe it is a bad idea in clojurescript as well

Comment by Mike Fikes [ 09/Apr/16 7:57 PM ]

Hey Kevin, the ~x appears only once on both branches of the if so evaluation should occur once. This test pans out:

(let [x (atom 0)]
  [(list (swap! x inc) (swap! x inc)) @x])

properly yielding [(1 2) 2]. (FWIW, an example of double evaluation in a macro is in CLJS-1518, which is solvable there by letting once as is done in this ticket's fix.)

Comment by Kevin Downey [ 10/Apr/16 1:27 AM ]

I am not talking about double evaluation, I am talking about double macro expansion.

The analyzer does macro expansion so the list macro that calls the analayzer is sort of like

(defmacro x [y]
  (macroexpand y)
  y)

y will then be macroexpanded again as compilation continues, so macroexpansion happens twice.

I suspect this is more of an issue in clojure than it is in clojurescript, because the compilation environment is isolated from the runtime environment in clojurescript. In clojure while hacking on the compiler, it would be very nice to analyze a form and then make decisions based on that analysis, and then re-analyze the form differently, but the double macro expansion breaks things (a long with other stateful bits in the compiler). Regardless it seems like a bad idea.

Comment by Mike Fikes [ 10/Apr/16 7:38 AM ]

Ahh, sorry Kevin. I completely missed what you were saying earlier.

So, perhaps your concern does come in to play with bootstrap ClojureScript where there is no separation (macros are expanded in the runtime environment, so side effects could have a consequence).

FWIW, I ran the ClojureScript compiler's unit tests in bootstrap ClojureScript with this change and they pass.

I suppose one corollary example of what you are saying is that it is no longer possible to evaluate

(macroexpand '(list (when-let) 1))
Comment by Kevin Downey [ 17/Apr/16 12:08 AM ]

http://dev.clojure.org/jira/browse/CLJS-1625 is a similar issue





[CLJS-1616] Self-host: improve documentation for compile-str Created: 06/Apr/16  Updated: 06/Apr/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


 Description   

It's not clear at all how to use the `opts` arguments for compile-str.

In the code - https://github.com/clojure/clojurescript/blob/c3899acf797eb6779c53b313f5606c5018360b83/src/main/cljs/cljs/js.cljs#L660 - we
only have
:load - library resolution function, see load-fn
:source-map - set to true to generate inline source map information

In fact, there is also :verbose and ::def-emits-var

They are not documented.

Are there more options?






[CLJS-1615] Inlining truth checks can lead to better optimisation results Created: 04/Apr/16  Updated: 08/Apr/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-1615.patch    
Patch: Code and Test

 Description   

I had a situation when DCE was (naively) expected but didn't happen (in :advanced compilation mode). I did some exploration and discovered that inlined truth check code is better understood by Closure Compiler and leads to expected optimisation (for some reasons).

I believe understanding this behaviour and exploiting it where desirable could lead to more predictable code generation without resorting to using cljs type hints.



 Comments   
Comment by Antonin Hildebrand [ 04/Apr/16 2:54 PM ]

The backstory as posted to #cljs-dev in Slack:

When @domkm requested proper dead code elimination in cljs-devtools, I got burnt by the need to explicitly specify `(if ^boolean js/goog.DEBUG …)` hint to get `:closure-defines` working under :advanced build[1]. It was unexpected to me that closure compiler cannot see that optimization and does not inline truth test for js/goog.DEBUG “constant”. So I started poking around and found a way how to aggressively inline checked truth checks in a compatible way (I believe). I also believe this could potentially open optimizations in other places. I think we should explore `@nosideeffects` annotation[2] and tag core functions where appropriate.

[1] https://github.com/binaryage/cljs-devtools/releases/tag/v0.5.3
[2] https://developers.google.com/closure/compiler/docs/js-for-compiler?hl=en#tag-nosideeffects

Comment by Francis Avila [ 05/Apr/16 3:11 PM ]

`@nosideeffects` should only be relevant for extern files where the compiler cannot see the implementation and know if the function is pure. Normally the compiler just analyzes the function to see if it side-effects.

This may be a performance win as well: it looks like advanced compile will unwrap the function expression entirely in some cases (both expression and statement contexts), so no more megamorphic calls to truth_ or even function object allocations.

However, I don't think there's a guarantee that the closure compiler will always understand enough to remove the need for the ^boolean hint for defines in all cases.





[CLJS-1614] Race condition in parse-ns can break compilation under parallel build Created: 03/Apr/16  Updated: 08/Apr/16  Resolved: 08/Apr/16

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

Type: Defect Priority: Major
Reporter: Nicolás Berger Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

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

 Description   

I found this issue by trying to compile reagent with parallel-build. Once parallel-build is enabled the compilation fails most of the times, creating warnings about different undeclared vars. The issue is not deterministic: the vars are usually different, and sometimes it even works fine, without warnings.

By running with :verbose true I found that the warnings always occur after a line about Reading analysis cache for file: <file name for a namespace with macros> (in the case of reagent, that's the reagent.interop namespace). For example:

[snip]
Compiling /home/nicolas/projects/reagent/src/reagent/dom.cljs
Reading analysis cache for file:/home/nicolas/projects/reagent/src/reagent/interop.cljs
WARNING: Use of undeclared Var reagent.dom/load-error at line 11 /home/nicolas/projects/reagent/src/reagent/dom.cljs
[snip]

After a lot of debugging I arrived at the following technical explanation and fix proposal (taken from the commit message in the attached patch):

parse-ns allows for running the parse in two ways: updating the global compiler env or not updating it. This is controlled via the :restore
option. To implement this, it isolates the parsing process by binding the compiler env to a snapshot of the compiler env when it starts. When the parsing is done, it checks the :restore option and if it's false it merges back parts of the resulting compiler env (the ::namespaces and ::constants keys) into the global compiler env.

The problem is that during the parsing, other compiler processes could have legitimately updated those same keys in the global compiler env, so this merge can occasionally result in compiler data loss.

The race condition can be avoided by using a different approach: only when :restore is true the compiler runs isolated from the global compiler env, by taking a snapshot of the env at the beginning. When :restore is false, no snapshot is taken: env/compiler binding is not modified, so the global compiler env is mutated in-place

I couldn't create a minimal repro, but I created a branch of the reagent project with parallel-build enabled. The branch can be found in https://github.com/nberger/reagent/tree/parallel-build. To run the build from a clean slate, run lein do clean, cljsbuild once client from the commmand line.

Versions affected: 1.8.40 and older (couldn't find 1.8.40 from the "Affects version/s" select)



 Comments   
Comment by Nicolás Berger [ 08/Apr/16 1:47 PM ]

Added new patch with the correct logic on whether to wrap with an isolated atom depending on the :restore option

Comment by David Nolen [ 08/Apr/16 2:02 PM ]

fixed https://github.com/clojure/clojurescript/commit/4e64079741e5d586c16c5ef2746aad3886d6d7d6





[CLJS-1613] Preserve Structural Sharing for clj->js Created: 03/Apr/16  Updated: 08/Apr/16  Resolved: 08/Apr/16

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

Type: Enhancement Priority: Minor
Reporter: Tom Raithel Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: clj->js
Environment:

OSX
java version "1.8.0_74"
Java(TM) SE Runtime Environment (build 1.8.0_74-b02)
Java HotSpot(TM) 64-Bit Server VM (build 25.74-b02, mixed mode)



 Description   

Hi, is there any possibility to keep structural sharing intact after calling clj->js on a collection?

It seems like clj->js just makes a deep copy from the collection on every call. I´m new to ClojureScript but it is really confusing that the equality-check against the clojure set is true and the check against the js object equals false.

 
(def set1 {:a [1 2 3]})
(def set2 (assoc set1 :b "foo"))
(.log js/console (== (get set1 :a) (get set2 :a)))
; -> true

(def set1js (clj->js2 set1))
(def set2js (clj->js2 set2))
(.log js/console (== (.-a set1js) (.-a set2js)))
; -> false


 Comments   
Comment by Thomas Heller [ 04/Apr/16 3:25 AM ]

This is just JS being JS. clj->js converts the entire structure, JS doesn't have a set type so we get a [1,2,3] javascript array.

[1,2,3] == [1,2,3]

is false in JS, nothing we can do about that. Generally you want to stay away from JS native collections as much as possible.

Comment by Tom Raithel [ 04/Apr/16 3:46 AM ]

Thanks for your response.

Yes, the two arrays are not the same because clj->js copies all the values over. I was just wondering if there may be a solution to pick up the real reference to the underlying JS object instead of just copying them over.

For example in this case, if I access the [1 2 3] array from the maps directly via js (not calling clj->js), the actual JS objects are equal:

(def a1 (.-tail (nth (.-arr set1) 1)))
(def a2 (.-tail (nth (.-arr set2) 1)))
(.log js/console a1)
(.log js/console a2)
(.log js/console (== a1 a2))
; -> true

I know that example above is hacky, but I just wanted to demonstrate that the structural sharing is already in place, it´s just not preserved when using clj->js.

This would be very helpful for applications, where only part of the code - for example the business layer - are written in ClojureScript and data is shared via plain JS objects.

Comment by Thomas Heller [ 04/Apr/16 4:22 AM ]

That this works is just pure coincidence. If you put a few more elements into the initial vector (eg. (def set1 {:a (into [] (range 500))})) the (def a1 (.-tail (nth (.-arr set1) 1))) will only contain a small part of the values. If you want all values in JS you would need to create a new array and lose the "shareable" reference.

Comment by Tom Raithel [ 04/Apr/16 4:34 AM ]

Ah I see, didn´t know that they are lazy evaluated. I will probably have to come up with another solution to that specific problem. Thx for your help Thomas.

Comment by Francis Avila [ 05/Apr/16 2:09 PM ]

Tom, the issue is not lazy evaluation, it's that a vector internally divides its values into multiple js arrays whose values never change after initialization.

I don't see how this idea is in any way feasible. clj->js cannot ever safely reuse the same underlying arrays from clojure collection objects because any mutation to the structure returned by clj->js would mutate the original clj collections.

At most what could be done is ensuring that clj objects of equal value produce identical js objects. However, even this I don't think is desirable in the general case because it would completely frustrate expectations from javascript consumers of your object. It also has other problems.

Suppose this did exist and you had a clj->js with this behavior:

(def a (clj->js [#{1 2 3} [1 2 3]]))
(def b (clj->js [#{1 2 3} [1 2 3]]))
(def c (clj->js #{1 2 3}))
(assert (identical? a b))
(assert (identical? (aget a 0) c)
(assert (not (identical? (aget a 0) (aget a 1))) ; I assume?

Now how would you avoid this scenario if you mutate something?

(aset a 0 3 "new")
; a is now #js[#js[1 2 3 "new"] #js[1 2 3]]
; b is now the same
; c is now #js[#js[1 2 3 "new"]]

; and subsequent clj->js calls are broken unless you go to extraordinary lengths to invalidate cached entries
(clj->js #{1 2 3}) ; => #js[1 2 3 "new"]

;; But suppose you don't like the above answer. Your options are either:
;; 1. Create a new cached copy with the correct mapping, breaking the identity with already-created values
(def d (clj->js #{1 2 3}))
; #js[1 2 3] again
(assert (not (identical? c d)) ; no more sharing

;; or 2. mutate the cached value back to the original value!
(def d (clj->js #{1 2 3}))
; a, b, c, d are now what they were before the (aset a 0 3 "new")

The core problem is that js data structures are all mutable so the only safe thing to do is clone everything. If you have some special case where you know that the js consumer of the clj->js call will not mutate anything and you don't mind the memory overhead of memoization to get the reference-equality you want, you can always implement a clj->js variant with the properties you desire.

Comment by Tom Raithel [ 07/Apr/16 2:19 PM ]

Hey Francis,

your first code block is not quite what I meant. I was proposing something that behaves exactly like the identical? check of clj objects. That means - regarding your example - I would expect to (assert (identical? (aget a 0) c) equals false because c has no relation to the a object.

I hacked together an extended version of clj->js which is basically storing all the already converted JS objects on to the clj object, so that the second clj->js call on the identical data returns the same object that was already transformed.

It may be a little bit hacky and probably agains all laws of cljs (I´m still new to it), but it works for me. I assume it would also be a bit faster then clj->js because if the value is already cached, there is no need for transformation.

(defn clj->cached-js
  "Recursively transforms ClojureScript values to JavaScript and cache its values.
sets/vectors/lists become Arrays, Keywords and Symbol become Strings,
Maps become Objects. Arbitrary keys are encoded to by key->js."
  [state]
  (when-not (nil? state)
    (if (satisfies? IEncodeJS state)
      (-clj->js state)
      (if (aget state "__cachedJs")
        (aget state "__cachedJs")
        (let [obj (cond
                    (keyword? state) (name state)
                    (symbol? state) (str state)
                    (map? state) (let [m (js-obj)]
                               (doseq [[k v] state]
                                 (aset m (key->js k) (clj->cached-js v)))
                               m)
                    (coll? state) (let [arr (array)]
                                (doseq [state (map clj->cached-js state)]
                                  (.push arr state))
                                arr)
                    :else state)]
          (aset state "__cachedJs" obj)
          obj)))))

Here are some tests that I wrote:

(def some-symbol "my symbol")
(def state {
  :symbol some-symbol
  :key :some-key
  :id 1234
  :user {
    :name "Tom",
    :age 33,
    :city "Wiesbaden"
  }
  :hobbies [
    {:name "code"}
    {:name "sleep"}
  ]
  :tags '("foo" "bar" "baz")
})

(t/deftest test-matches-original-behaviour
  (let [obj (core/clj->cached-js state)
        original-obj (clj->js state)]
    (t/is (= (js->clj original-obj) (js->clj obj)))))

(t/deftest test-change-hash-value
  (let [obj (core/clj->cached-js state)
        new-state (assoc-in state [:user :name] "Bert")
        new-obj (core/clj->cached-js new-state)]
    (t/is (not= new-obj obj))
    (t/is (not= (.-user new-obj) (.-user obj)))
    (t/is (= (.-user.name new-obj) "Bert"))
    (t/is (= (.-hobbies new-obj) (.-hobbies obj)))
    (t/is (= (.-tags new-obj) (.-tags obj)))
))

(t/deftest test-change-vector-value
  (let [obj (core/clj->cached-js state)
        new-state (assoc-in state [:hobbies 1 :name] "drink")
        new-obj (core/clj->cached-js new-state)]
    (t/is (not= new-obj obj))
    (t/is (= (.-user new-obj) (.-user obj)))
    (t/is (not= (.-hobbies new-obj) (.-hobbies obj)))
    (t/is (= (aget new-obj "hobbies" 0) (aget obj "hobbies" 0)))
    (t/is (not= (aget new-obj "hobbies" 1) (aget obj "hobbies" 1)))
    (t/is (= (aget new-obj "hobbies" 1 "name") "drink"))
    (t/is (= (.-tags new-obj) (.-tags obj)))
))

(t/deftest test-add-value-to-vector
  (let [obj (core/clj->cached-js state)
        new-state (assoc-in state
                            [:hobbies]
                            (let [hobbies (get state :hobbies)
                                 [before after] (split-at 1 hobbies)]
                              (vec (concat before [{:name "kid"}] after))))
        new-obj (core/clj->cached-js new-state)]
    (t/is (not= (.-hobbies new-obj) (.-hobbies obj)))
    (t/is (= (aget new-obj "hobbies" 0) (aget obj "hobbies" 0)))
    (t/is (not= (aget new-obj "hobbies" 1) (aget obj "hobbies" 1)))
    (t/is (= (aget new-obj "hobbies" 2) (aget obj "hobbies" 1)))
))

Of course it does not prevent you from manipulating the converted JS objects which subsequently would change other JS references, but that is - like Thomas Heller mentioned - just JS beeing JS. It is just not immutable, but you should treat it as such.

Don´t get me wrong, I believe that is not something that should be implemented into the ClojureScript itself. But this ticket helped me a lot in understanding the principle of clj->js. For now I will go with this solution. If you see some problems with this approach, please let me know.

Thanks for your help!

Comment by Francis Avila [ 08/Apr/16 1:48 PM ]

Consider using (def clj->cached-js (memoize clj->js)).





[CLJS-1612] Resolve ns aliases in syntax quote Created: 31/Mar/16  Updated: 23/Apr/16  Resolved: 23/Apr/16

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

Type: Defect Priority: Minor
Reporter: Tom Jack Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: patch

Attachments: Text File CLJS-1612-aux.patch     Text File CLJS-1612.patch     Text File resolve-ns-aliases-in-syntax-quote.patch     Text File self-host-resolve-ns-aliases-in-syntax-quote.patch    
Patch: Code and Test

 Description   

This test (cljs/syntax_quote_test.cljs) fails:

(ns cljs.syntax-quote-test
  (:require [cljs.test :as test :refer-macros [deftest is]]))

(deftest test-syntax-quote
  (is (= `foo 'cljs.syntax-quote-test/foo))
  (is (= `test/test-vars 'cljs.test/test-vars))
  (is (= `test/deftest 'cljs.test/deftest))
  (is (= `test/foo 'cljs.test/foo)))

The first assertion passes (since f240826034), but the remaining assertions fail: test is not resolved to cljs.test, so e.g. `test/foo is just 'test/foo.

Of course, the analogous clojure/syntax_quote_test.clj passes:

(ns clojure.syntax-quote-test
  (:require [clojure.test :as test :refer [deftest is]]))

(deftest test-syntax-quote
  (is (= `foo 'clojure.syntax-quote-test/foo))
  (is (= `test/test-vars 'clojure.test/test-vars))
  (is (= `test/deftest 'clojure.test/deftest))
  (is (= `test/foo 'clojure.test/foo)))

Attached patch resolve-ns-aliases-in-syntax-quote.patch contains the failing test and an attempted fix.



 Comments   
Comment by David Nolen [ 08/Apr/16 2:51 PM ]

This looks good, have you submitted a CA? Thanks.

Comment by Tom Jack [ 08/Apr/16 5:15 PM ]

Yeah, I'm "Thomas Jack" on the contributors list.

Comment by Mike Fikes [ 09/Apr/16 11:44 PM ]

The attached patch causes an issue for self-hosted ClojureScript for symbols in macros namespaces. One specific example is the aclone symbol referenced in the amap macro: With self-host, instead of resolving to cljs.core/aclone, it ends up with the macro-pseudonamespace: cljs.core$macros/aclone. Here is how to reproduce this:

Apply the patch and then start up script/noderepljs:

$ script/noderepljs 
ClojureScript Node.js REPL server listening on 51454
To quit, type: :cljs/quit
cljs.user=> (require 'cljs.js)
nil
cljs.user=> (def st (cljs.js/empty-state))
#'cljs.user/st
cljs.user=> (cljs.js/eval-str st "(def an-array (int-array 5 0))" nil {:eval cljs.js/js-eval :context :expr} identity)
{:ns cljs.user, :value #js [0 0 0 0 0]}
cljs.user=>  (cljs.js/eval-str st "(amap an-array idx ret (+ 1 (aget an-array idx)))" nil {:eval cljs.js/js-eval :context :expr} identity)
WARNING: No such namespace: cljs.core$macros, could not locate cljs/core$macros.cljs, cljs/core$macros.cljc, or Closure namespace "" at line 1 
WARNING: Use of undeclared Var cljs.core$macros/aclone at line 1 
{:error #error {:message "ERROR", :data {:tag :cljs/analysis-error}, :cause #object[TypeError TypeError: Cannot read property 'call' of undefined]}}

This affects the aclone symbol as well as several references to get, as far as I can tell.

Comment by Mike Fikes [ 09/Apr/16 11:48 PM ]

I forgot to add, apart from the regression issue described in the previous comment, the patch does fix the intended use case described in the ticket under self-hosted ClojureScript (causing aliases to expand properly).

Comment by Tom Jack [ 10/Apr/16 12:23 PM ]

D'oh, thanks!

Patch self-host-resolve-ns-aliases-in-syntax-quote.patch adds your example as a failing self-host/test, and fixes it in the stupidest way: just trim off "$macros" in resolve-symbol.

Not sure if this stupid fix is acceptable..

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

Yeah, Tom. I suspect that's a little heavy-handed. Here is a specific case it breaks in bootstrap:

Let's say you have this macros namespace:

(ns foo.macros)

(defn add*
  [a b]
  (+ a b))

(defmacro add
  [a b]
  `(add* ~a ~b))

The add* symbol needs to resolve properly and the $macros suffix is key. Without this latest patch, it does, but with it, it fails to do so. Here is the way to reproduce the issue with your patch:

$ script/noderepljs 
ClojureScript Node.js REPL server listening on 57728
To quit, type: :cljs/quit
cljs.user=> (require 'cljs.js)
nil
cljs.user=> (def st (cljs.js/empty-state))
#'cljs.user/st
cljs.user=> (cljs.js/eval st
'(ns foo.core (:require-macros foo.macros))
{:context :expr
 :eval cljs.js/js-eval
 :load         (fn [_ cb]
                 (cb {:lang   :clj
                          :source "(ns foo.macros) (defn add* [a b] (+ a b)) (defmacro add [a b] `(add* ~a ~b))"}))}
identity)
{:value nil}
cljs.user=> (cljs.js/eval st
'(foo.macros/add 1 2)
{:context :expr
 :eval cljs.js/js-eval}
identity)
WARNING: No such namespace: foo.macros, could not locate foo/macros.cljs, foo/macros.cljc, or Closure namespace ""
WARNING: Use of undeclared Var foo.macros/add*
TypeError: Cannot read property 'add_STAR_' of undefined
    at eval (eval at cljs$js$js_eval (/Users/mfikes/Projects/planck/planck-cljs/clojurescript/.cljs_node_repl/cljs/js.js:131:13), <anonymous>:1:11)
    at cljs$js$js_eval (/Users/mfikes/Projects/planck/planck-cljs/clojurescript/.cljs_node_repl/cljs/js.js:131:8)
    at cljs$js$eval_STAR_ (/Users/mfikes/Projects/planck/planck-cljs/clojurescript/.cljs_node_repl/cljs/js.js:1168:146)
    at Function.cljs.js.eval.cljs$core$IFn$_invoke$arity$4 (/Users/mfikes/Projects/planck/planck-cljs/clojurescript/.cljs_node_repl/cljs/js.js:1247:27)
    at cljs$js$eval (/Users/mfikes/Projects/planck/planck-cljs/clojurescript/.cljs_node_repl/cljs/js.js:1233:21)
    at repl:1:102
    at repl:9:3
    at repl:14:4
    at Object.exports.runInThisContext (vm.js:54:17)
    at Domain.<anonymous> ([stdin]:41:34)
cljs.user=>

Without your patch, the last bit will return.

{:value 3}
Comment by Tom Jack [ 10/Apr/16 5:16 PM ]

OK, the issue is beyond my comprehension, then. Your example does not make sense to me – I was surprised that it works as of master.

I will probably let someone who understands self-hosted cljs resolve the issue before I understand it enough to resolve the issue myself.

Comment by Mike Fikes [ 10/Apr/16 7:01 PM ]

Tom, part of me thinks your first patch is correct, and that it is aclone that needs to be qualified as cljs.core/aclone (and similarly for calls to get). I'll try an experiment along these lines and report back.

Comment by Mike Fikes [ 11/Apr/16 10:25 PM ]

The attached {{CLJS-1612-aux.patch}}, when applied with resolve-ns-aliases-in-syntax-quote.patch, results in the compiler core unit tests passing under bootstrap ClojureScript.

At the heart of what needs to be considered here is illustrated in the use of aclone in the amap macro. In particular, under bootstrap ClojureScript, syntax-quoting function symbol like aclone, when used in a macro, result in it being qualified with the current macro pseudo-namespace. (While this does not occur today for aclone, it does occur for this use case in bootstrap outside of cljs.core, so there is a tangential issue regarding cljs.core perhaps getting special treatment. But, regardless, with resolve-ns-aliases-in-syntax-quote.patch, the pseudo-namespace comes into play.)

So, if qualified as cljs.core$macros/aclone, this makes it so that aclone could be a regular function in the macros namespace, called by the macro. This is like the add* function in the example in the Bootstrapped ClojureScript portion at the bottom of http://blog.fikesfarm.com/posts/2016-01-05-clojurescript-macros-calling-functions.html

But, aclone is not a function in the macro namespace. It is a function in the runtime namespace. So, it needs to be qualified as such, as, cljs.core/aclone and the attached patch does just that. The attached patch does this for other function symbols that need the same treatment.

Now, the above analysis is based simply on the current behavior of the compiler, and not based on any notion of how the compiler should behave. An alternate view could be that the $macros suffix should never appear in qualified names, and resolution should just find the correct symbol. (So, a dual argument here is the fact that or can be called as cljs.core/or or cljs.core$macros/or, and both just work somehow, with the ideal publicized behavior being that the $macros suffix is an internal implementation detail, and users should always use forms like cljs.core/or. With this view, we'd instead strive to make self-host-resolve-ns-aliases-in-syntax-quote.patch work.

If we do want to go with the patch attached to this ticket, there are a few things we'd need to do:

1. Combine the patches into a single one. (Presuming a single patch is the only way it would be accepted.) If so, I'd have no problem if Tom wanted to combine this work into his and submit it under his name. (I've signed the CA, so this is OK.)
2. The only reason I currently know this patch works is because I built ClojureScript with it and ran the compiler test suite in bootstrap ClojureScript, but with a downstream bootstrap environment (Planck). I'd feel more comfortable if we added specific tests to the self-host test suite to provoke the failures that are needed to warrant the changes in this patch. (In other words, bootstrap is kind-of unacceptably fragile if we need to rely on something like Planck to ensure it is working correctly—the needed tests need to be with the ClojureScript compiler—ideally one day we'd get to the point where we can run the entire compiler's unit tests in bootstrap mode, but we aren't there now.)

Comment by Mike Fikes [ 17/Apr/16 7:52 AM ]

I'll take a stab at putting the above together.

Comment by Mike Fikes [ 17/Apr/16 10:24 AM ]

CLJS-1612.patch includes Tom Jack's original change plus additional revisions for bootstrap.

Comment by David Nolen [ 23/Apr/16 1:59 PM ]

The patch no longer applies to master, can we rebase?

Comment by Tom Jack [ 23/Apr/16 2:28 PM ]

Hmm, Mike's latest patch CLJS-1612.patch seems to apply to master for me.

$ git fetch -v --all
Fetching origin
From https://github.com/clojure/clojurescript
 = [up to date]      master     -> origin/master
 ...
$ git reset --hard origin/master
HEAD is now at 28e040d CLJS-1626: cljs.test for bootstrap
$ git am --keep-cr -s --ignore-whitespace < CLJS-1612.patch
Applying: CLJS-1612: Resolve ns aliases in syntax quote

Thanks, Mike, by the way, for the patch and explanation!

Comment by Mike Fikes [ 23/Apr/16 2:41 PM ]

Yeah, I can confirm what Tom's seeing. Here's my alternate verification:

$ cd /tmp
$ git clone https://github.com/clojure/clojurescript
Cloning into 'clojurescript'...
remote: Counting objects: 27250, done.
remote: Total 27250 (delta 0), reused 0 (delta 0), pack-reused 27250
Receiving objects: 100% (27250/27250), 10.25 MiB | 3.67 MiB/s, done.
Resolving deltas: 100% (12667/12667), done.
Checking connectivity... done.
$ cd clojurescript
$ curl -L http://dev.clojure.org/jira/secure/attachment/15609/CLJS-1612.patch | git apply
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  8472  100  8472    0     0  13148      0 --:--:-- --:--:-- --:--:-- 13155

After this, script/test, script/test-self-host, and script/test-self-parity all pass as well.

Comment by David Nolen [ 23/Apr/16 3:18 PM ]

fixed https://github.com/clojure/clojurescript/commit/1e794f0a9d6c03b12644f00cd8aa4f8a3e86ab83. Not sure why the patch wasn't applying for me before.





[CLJS-1611] Function arity dispatch returns arity Created: 27/Mar/16  Updated: 03/Jun/16  Resolved: 03/Jun/16

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

Type: Defect Priority: Minor
Reporter: Prince John Wesley Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: bug, compiler

Attachments: PNG File arity-0.png     PNG File arity-1.png     PNG File arity-2.png     Text File CLJS-1611-1.patch     Text File CLJS-1611.patch    
Patch: Code

 Description   

JS code generated from cljs.js/eval-str* and cljs.js/compile-str* functions are out of order(see the attachment).

Used the below compilation option to generate js code.
{ :load-macros true
:analyze-deps true
:static-fns false
:def-emits-var true
:context :expr }



 Comments   
Comment by Mike Fikes [ 30/Mar/16 10:55 PM ]

This is not a self-host issue, per se, is it? I think it is a general limitation of the work we did for :def-emits-var (in that it doesn't handle the multi-arity case). For example, in a non-self-host REPL:

$ java -jar cljs.jar -m cljs.repl.nashorn
To quit, type: :cljs/quit
cljs.user=> (defn fun-arity
  ([] "zero")
  ([_] "one"))
1
cljs.user=> (def a 3)
#'cljs.user/a

(The issue could still be fixed... just wanted to clarify whether this is a self-host or a general issue.)

Comment by Prince John Wesley [ 30/Mar/16 11:04 PM ]

Yes. its a general issue.

Comment by Mike Fikes [ 26/Apr/16 2:37 PM ]

Removed self-host from title and bootstrap from label.

Comment by Mike Fikes [ 02/Jun/16 8:20 PM ]

Attaching CLJS-1611.patch which fixes the multi-arity as well as the variadic case.

Variadic and multi-arity defn forms expand to code that perform the
needed def, followed by some side-effecting code. Instead of returning
the value of the last side-effecting form, capture and return the
value of the def so that the expanded defn returns the value of the
def, thus causeing :def-emits-vars to operate properly in REPLs.

Comment by Mike Fikes [ 02/Jun/16 10:11 PM ]

While CLJS-1611.patch appears to fix the issue when looking at its behavior in the REPL, it causes different JavaScript to be emitted which leads me to suspect the patch may not be suitable.

For a concrete example of what is produced, consider this namespace

(ns foo.core)

(defn g [& r] 13)

This would normally transpile to this:

// Compiled by ClojureScript 1.9.21 {:target :nodejs}
goog.provide('foo.core');
goog.require('cljs.core');
foo.core.g = (function foo$core$g(var_args){
var args__7612__auto__ = [];
var len__7605__auto___16505 = arguments.length;
var i__7606__auto___16506 = (0);
while(true){
if((i__7606__auto___16506 < len__7605__auto___16505)){
args__7612__auto__.push((arguments[i__7606__auto___16506]));

var G__16507 = (i__7606__auto___16506 + (1));
i__7606__auto___16506 = G__16507;
continue;
} else {
}
break;
}

var argseq__7613__auto__ = ((((0) < args__7612__auto__.length))?(new cljs.core.IndexedSeq(args__7612__auto__.slice((0)),(0),null)):null);
return foo.core.g.cljs$core$IFn$_invoke$arity$variadic(argseq__7613__auto__);
});

foo.core.g.cljs$core$IFn$_invoke$arity$variadic = (function (r){
return (13);
});

foo.core.g.cljs$lang$maxFixedArity = (0);

foo.core.g.cljs$lang$applyTo = (function (seq16504){
return foo.core.g.cljs$core$IFn$_invoke$arity$variadic(cljs.core.seq.call(null,seq16504));
});

//# sourceMappingURL=core.js.map

With the patch, this is emitted:

// Compiled by ClojureScript 1.9.21 {:target :nodejs}
goog.provide('foo.core');
goog.require('cljs.core');
var d__7614__auto___8040 = foo.core.g = (function foo$core$g(var_args){
var args__7615__auto__ = [];
var len__7607__auto___8041 = arguments.length;
var i__7608__auto___8042 = (0);
while(true){
if((i__7608__auto___8042 < len__7607__auto___8041)){
args__7615__auto__.push((arguments[i__7608__auto___8042]));

var G__8043 = (i__7608__auto___8042 + (1));
i__7608__auto___8042 = G__8043;
continue;
} else {
}
break;
}

var argseq__7616__auto__ = ((((0) < args__7615__auto__.length))?(new cljs.core.IndexedSeq(args__7615__auto__.slice((0)),(0),null)):null);
return foo.core.g.cljs$core$IFn$_invoke$arity$variadic(argseq__7616__auto__);
});
foo.core.g.cljs$core$IFn$_invoke$arity$variadic = ((function (d__7614__auto___8040){
return (function (r){
return (13);
});})(d__7614__auto___8040))
;

foo.core.g.cljs$lang$maxFixedArity = (0);

foo.core.g.cljs$lang$applyTo = ((function (d__7614__auto___8040){
return (function (seq8039){
return foo.core.g.cljs$core$IFn$_invoke$arity$variadic(cljs.core.seq.call(null,seq8039));
});})(d__7614__auto___8040))
;


//# sourceMappingURL=core.js.map

I haven't yet grokked the consequences of this. On the other hand, I have empirically seen a failure of Planck to be able to walk the namespace object trees in code generated with this patch.

Comment by Mike Fikes [ 03/Jun/16 7:27 AM ]

With a little more care with respect to what is emitted in the transpiled JavaScript, the revised CLJS-1611-1.patch meets our desire of returning the var in the REPL, but otherwise not affecting transpiled JavaScript. I've put this through some paces, evaluating forms in script/noderepljs, requiring code and looking at what gets put in .cljs_node_repl, and using it downstream with Planck and it appears to be doing what we want. I'd recommend others giving a spin as well.

Commit comment:

Variadic and multi-arity defn forms expand to code that perform the
needed def, followed by some side-effecting code. Instead of returning
the value of the last side-effecting form, conditionally return a var
for the name if :def-emits-vars is true, otherwise nil (which gets
elided from compiled JavaScript.)

Comment by Mike Fikes [ 03/Jun/16 8:40 AM ]

Rohit Aggarwal indicated "LGTM."

Comment by David Nolen [ 03/Jun/16 8:43 AM ]

fixed https://github.com/clojure/clojurescript/commit/358e5632544f1214ebd7cfc4d43c651ea78b25cb





[CLJS-1610] Refs api Created: 26/Mar/16  Updated: 28/Mar/16

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

Type: Enhancement Priority: Minor
Reporter: James Laver Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

It would be nice to support the refs API in clojurescript to ease porting of libraries which require it. Obviously usage of it would not bring concurrency benefits, but it would at least allow some existing clojure code to run.

I've satisfied myself it can be done reasonably easily (see https://github.com/jjl/clojurescript/tree/ref-support ), and I can't really see any downsides.

For the minute, this has been put into the cljs.stm namespace. I don't know whether it should be a feature you are automatically opted into, so I didn't put it into core. Thoughts?



 Comments   
Comment by David Nolen [ 28/Mar/16 6:44 AM ]

It's definitely worth considering. If you've done the work, attach an actual patch for review. In general we try to avoid links outside.





[CLJS-1609] Self-host: Expressions like `(+ 42)` and `(let [x 42] x)` are evaluated to nil. Created: 26/Mar/16  Updated: 14/Apr/16  Resolved: 14/Apr/16

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

Type: Defect Priority: Minor
Reporter: Yehonathan Sharvit Assignee: Unassigned
Resolution: Not Reproducible Votes: 0
Labels: bootstrap


 Description   

Expressions like `(+ 42)` and `(let [x 42] x)` are evaluated to nil.

(cljs/eval-str (cljs/empty-state) "(+ 42)" 'test {:eval cljs/js-eval} identity); => {:ns cljs.user, :value nil}

while

(cljs/eval-str (cljs/empty-state) "(+ 0 42)" 'test {:eval cljs/js-eval} identity); => {:ns cljs.user, :value 42}

(cljs/eval-str (cljs/empty-state) "(let [x 42] x)" 'test {:eval cljs/js-eval} identity); => {:ns cljs.user, :value nil}

while
(cljs/eval-str (cljs/empty-state) "[(let [x 42] x)]" 'test {:eval cljs/js-eval} identity); => {:ns cljs.user, :value [42]}



 Comments   
Comment by Mike Fikes [ 30/Mar/16 10:39 PM ]

If you are evaluating ClojureScript source that reduces expressions to values, then you should add :context :expr to the opts map. (See CLJS-1357.) Otherwise, I believe the default will be :statement. (See https://github.com/clojure/clojurescript/blob/c72e9c52156b3b348aa66857830c2ed1f0179e8c/src/main/clojure/cljs/analyzer.cljc#L512)

Comment by Yehonathan Sharvit [ 04/Apr/16 12:35 PM ]

I tried the following, but it didn't work

(def st (cljs/empty-state (fn [state] (assoc-in state [:options :context] :expr))))
(cljs/eval-str st "(let [x 2])" "bla" {:eval cljs/js-eval} identity); =>{:ns cljs.user, :value nil}

Comment by Mike Fikes [ 10/Apr/16 12:14 AM ]

Sorry, by opts, I was referring to the 4th argument to cljs.js/eval-str. Here's an example:

cljs.user=> (require '[cljs.js :as cljs])
nil
cljs.user=> (def st (cljs/empty-state))
#'cljs.user/st
cljs.user=> (cljs/eval-str st "(let [x 2] x)" "bla" {:eval cljs/js-eval :context :expr} identity)
{:ns cljs.user, :value 2}

On the other hand, if you explicitly set the context to be :statement, you will get the nil you reported:

cljs.user=> (cljs/eval-str st "(let [x 2] x)" "bla" {:eval cljs/js-eval :context :statement} identity)
{:ns cljs.user, :value nil}

I don't think this behavior is a bug for :statement context.

Comment by Yehonathan Sharvit [ 10/Apr/16 1:44 AM ]

Thanks Mike!
It works fine.

Comment by Mike Fikes [ 14/Apr/16 7:00 AM ]

Since it works, closing.





[CLJS-1608] Self-host: Alias-scoped keywords works only for first segment Created: 23/Mar/16  Updated: 23/Mar/16  Resolved: 23/Mar/16

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

Type: Defect Priority: Minor
Reporter: Andrea Richiardi Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: bootstrap
Environment:

ClojureScript 1.8.34



 Description   

An aliased keyword works only if the alias corresponds to the first segment, example:

(ns dummy.core (:require [clojure.string :as s])
cljs.user=> ::s/key
Invalid token: ::s/key

BUT

dummy.core=> (ns dummy.core (:require [clojure.string :as clojure]))
nil
dummy.core=> ::clojure/key
:clojure.string/key


 Comments   
Comment by Andrea Richiardi [ 23/Mar/16 5:42 PM ]

By adding this test, I was actually able to prove that this works here and I must made a mistake:

(deftest test-CLJS-1608
  (let [st (cljs/empty-state)]
    (cljs/eval-str st
      "(ns alias-load.core (:require [clojure.string :as s]))"
      nil
      {:ns      'cljs.user
       :context :expr
       :eval    cljs.js/js-eval
       :load    (fn [_ cb]
                  (cb {:lang :clj :source "(ns clojure.string)"}))}
      (fn [{:keys [error value]}]
        (is (nil? error))
        (cljs.js/eval-str st
          "::s/bar"
          nil
          {:ns      'alias-load.core
           :context :expr
           :eval    cljs.js/js-eval}
          (fn [{:keys [error value]}]
            (is (nil? error))
            (is (= :clojure.string/bar value))))))))
Comment by Andrea Richiardi [ 23/Mar/16 5:42 PM ]

Sorry for the noise





[CLJS-1607] Advanced compilation bug with `specify!` in JS prototypes Created: 23/Mar/16  Updated: 23/Mar/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: David Nolen
Resolution: Unresolved Votes: 1
Labels: None
Environment:

affects 1.8.34



 Description   

compiling this code with advanced optimizations

(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





[CLJS-1605] Some repl options missing from known option set Created: 21/Mar/16  Updated: 21/Mar/16  Resolved: 21/Mar/16

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

Type: Defect Priority: Minor
Reporter: Juho Teperi Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None
Environment:

Affects 1.8.34


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

 Description   

Related to http://dev.clojure.org/jira/browse/CLJS-1603

Not all options used by cljs.repl/repl* are currently included in known-repl-options. CLJS-1603 fixed most of unnecessary warnings as a warning is not shown if suggestion is not found, but some valid options still cause unnecessary warnings:

WARNING: Unknown option ':init'. Did you mean ':main'?



 Comments   
Comment by David Nolen [ 21/Mar/16 6:34 PM ]

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





[CLJS-1604] Self-host: cljs.js/compile-str causes a javascript error Created: 19/Mar/16  Updated: 14/Apr/16

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

Type: Defect Priority: Minor
Reporter: Yehonathan Sharvit Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: bootstrap, bug


 Description   

When requiring `cljs.js` and calling `cljs.js/compile-str` with `:optimizations :advanced`
I get the following error in the browser:
"Uncaught TypeError: Cannot set property 'rd' of undefined"

Steps to reproduce:

1. Make a directory
2. Copy shipping cljs.jar into the directory
3. Make an index.html, src/hello_world/core.cljs, and build.clj file with contents as below.
4. java -cp cljs.jar:src clojure.main build.clj
5. Open index.html with Chrome and view the JavaScriptConsole in Chrome.

index.html:

<html>
<body>
<script type="text/javascript" src="out/main.js"></script>
</body>
</html>
src/hello_world/core.cljs:
(ns hello-world.core
(:require [cljs.js :as cljs]))

(set! (.. js/window -cljs -user) #js {})

(cljs/compile-str (cljs/empty-state) "" indentity)

build.clj:

(require 'cljs.build.api)

(cljs.build.api/build "src"
{:output-to "out/main.js"
:optimizations :whitespace})

(System/exit 0)



 Comments   
Comment by Yehonathan Sharvit [ 19/Mar/16 5:31 PM ]

I need to fix the title of the issue: "Self-host: in advanced compilation - cljs.js/compile-str causes a javascript error"

Comment by Mike Fikes [ 30/Mar/16 11:14 PM ]

You can only use up to :optimizations :simple with self-host. See https://github.com/clojure/clojurescript/wiki/Optional-Self-hosting#production-builds

Discussion: One rationale for this is that the emitted code, in order to be executable, needs access to non-Closure-munged/DCEd symbols from the standard ClojureScript lib. Perhaps this limitation need only exist for eval-str, (while not for compile-str, analyze-str, etc.)

Comment by Mike Fikes [ 14/Apr/16 7:02 AM ]

I'd recommend closing this as declined (no plans exist to support self-host with :advanced).





[CLJS-1603] Only warn for misspelled comp/REPL opts Created: 18/Mar/16  Updated: 21/Mar/16  Resolved: 21/Mar/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: None
Environment:

Affects 1.8.34


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

 Description   

REPLs may have REPL-specific options which are added to the compiler/REPL option map and this triggers "unknown compiler option" warnings. For one downstream example, with Figwheel, you get

WARNING: Unknown option ':compiler-env'.
WARNING: Unknown option ':special-fns'.
WARNING: Unknown option ':warn-on-undeclared'.

Instead, only issue warnings when there are known suggestions within the Levenshtein distance threshold. This effectively limits the feature to its original use case of detecting minor misspellings per CLJS-1492.



 Comments   
Comment by Juho Teperi [ 20/Mar/16 5:04 PM ]

I don't know about REPL-specific options, but looks like currently warning is given for many options used directly on repl*. I think it would make sense to include these in known options?

At least the following are used on repl* but not included in known opts:

  • init
  • quit-prompt
  • eval
  • source-map-inline
  • wrap
  • repl-requires
  • compiler-env
  • bind-err
Comment by Mike Fikes [ 20/Mar/16 5:51 PM ]

Juho: Yep. Perhaps they would be added to this set (perhaps as a separate defect ticket?): https://github.com/clojure/clojurescript/blob/master/src/main/clojure/cljs/repl.cljc#L40-L43

Comment by David Nolen [ 21/Mar/16 1:43 PM ]

fixed https://github.com/clojure/clojurescript/commit/177b05fc5935c659e4829b7b1798e216b6797bec





[CLJS-1602] The `extend-type` raises unexpected exception on extending a js type with IFn. Created: 16/Mar/16  Updated: 18/Mar/16  Resolved: 18/Mar/16

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

Type: Defect Priority: Major
Reporter: Andrey Antukh Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None
Environment:

Linux, OpenJDK8



 Description   

When trying use `extend-type` (and `extend-protocol` that uses the first under the hood) raises the following exception:

Caused by: java.lang.UnsupportedOperationException: nth not supported on this type: Symbol
        at clojure.lang.RT.nthFrom(RT.java:947)
        at clojure.lang.RT.nth(RT.java:897)
        at cljs.core$adapt_ifn_invoke_params.invokeStatic(core.cljc:1330)
        at cljs.core$adapt_ifn_invoke_params.invoke(core.cljc:1330)
        at cljs.core$ifn_invoke_methods$fn__4744.invoke(core.cljc:1355)
        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.RT.seq(RT.java:521)
        at clojure.core$seq__4357.invokeStatic(core.clj:137)
        at clojure.core$map$fn__4785.invoke(core.clj:2637)
        at clojure.lang.LazySeq.sval(LazySeq.java:40)
        at clojure.lang.LazySeq.seq(LazySeq.java:56)
        at clojure.lang.ChunkedCons.chunkedNext(ChunkedCons.java:59)
        at clojure.lang.ChunkedCons.next(ChunkedCons.java:43)
        at clojure.lang.RT.next(RT.java:688)
        at clojure.core$next__4341.invokeStatic(core.clj:64)
        at clojure.core$butlast__4385.invokeStatic(core.clj:279)
        at clojure.core$butlast__4385.invoke(core.clj:277)
        at cljs.analyzer$analyze_do_statements_STAR_.invokeStatic(analyzer.cljc:1380)
        at cljs.analyzer$analyze_do_statements_STAR_.invoke(analyzer.cljc:1379)
        at cljs.analyzer$analyze_do_statements.invokeStatic(analyzer.cljc:1383)
        at cljs.analyzer$analyze_do_statements.invoke(analyzer.cljc:1382)
        at cljs.analyzer$eval1448$fn__1450.invoke(analyzer.cljc:1387)
        at clojure.lang.MultiFn.invoke(MultiFn.java:251)
        at cljs.analyzer$analyze_seq_STAR_.invokeStatic(analyzer.cljc:2368)
        at cljs.analyzer$analyze_seq_STAR_.invoke(analyzer.cljc:2366)
        at cljs.analyzer$analyze_seq_STAR__wrap.invokeStatic(analyzer.cljc:2373)
        ... 102 more

The minimal reproducible code (also happens in node repl):

(extend-type js/Date
  IFn
  (-invoke [this]
    (.valueOf this)))


 Comments   
Comment by David Nolen [ 18/Mar/16 12:37 PM ]

This is not a bug. The IFn implementations are malformed.

(extend-type js/Date
  IFn
  (-invoke
    ([this]
      (.valueOf this))))




[CLJS-1601] Optimize cljs.core dump Created: 12/Mar/16  Updated: 01/Apr/16

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

Type: Enhancement Priority: Minor
Reporter: Nikita Beloglazov Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: bootstrap

Attachments: Text File CLJS-1601.patch     Text File CLJS-1601.patch    

 Description   

When building cljs source that uses cljs.js namespace the final js file is quite huge: 6.4M. As described in wiki: https://github.com/clojure/clojurescript/wiki/Optional-Self-hosting it mostly consists of analysis cache of the cljs.core namespace. As a workaround, the wiki article suggests dumping cache to a separate file and load it at runtime instead of bundling in js binary. I think it is possible to have something in between that doesn't require additional efforts from a user and also optimizes the size of the js file. The idea that instead of dumping cache as raw clojure data-structure it is serialized to string. This way compiler won't compile cache into js (which adds a lot of code) and leave it a string. At runtime, this string will be parsed back to clojure using tools.reader.

Here is the proposal: https://gist.github.com/nbeloglazov/0bf163fb62fa4b61d446

Checking locally it reduces the size of js file from 6.4M to 2.7M which I think quite good. The downside is that now js has to do more work on runtime (parse huge string) when today it simply read js code and evaluates it. But I don't think if it's a big concern. If it is desired to keep all behavior a new option can be added for :dump-core compiler setting, something like :dump-core :string that enables string serialization of the cache.

Does it sound reasonable?



 Comments   
Comment by Nikita Beloglazov [ 27/Mar/16 8:54 PM ]

Attaching suggested fix. Analysis cache is serialized to string and read back to clojure datastructure when cljs.js is initialized.

Comment by David Nolen [ 28/Mar/16 6:39 AM ]

Please change the patch so this optional as you've suggested.

Comment by David Nolen [ 28/Mar/16 6:40 AM ]

Also have you submitted your Clojure CA yet?

Comment by Nikita Beloglazov [ 28/Mar/16 1:35 PM ]

Will do. Yes, I've submitted CA. I used my official name, Mikita Belahlazau there.

Comment by Nikita Beloglazov [ 29/Mar/16 12:16 AM ]

Updated patch that adds option to serialize core analysis cache as string. Possible values of :dump-core are :raw, :string, :none. Old true/false values supported for backward compatibility.

As for default, current patch uses :raw, but I think it makes more sense to use :string. Saving extra few mb of final js is quite good. I think most devs won't go deep into figuring out why js is big and just leave it as it is. Additional one-time parsing performance hit :string introduces acceptable: when :string is used, page loads in 1s while with :raw the time is ~800ms.





[CLJS-1600] Destructuring defprotocol fn args causes defrecord impls to silently fail Created: 11/Mar/16  Updated: 11/Mar/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: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 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






[CLJS-1599] UUIDs are not equal for upper/lower case strings Created: 07/Mar/16  Updated: 11/Mar/16

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

Type: Defect Priority: Minor
Reporter: Nikolay Durygin Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Windows 7x64



 Description   

UUIDs generated for strings in different case (one is upper and one is lower) are equal.

For example (= (uuid "071C600F-B72B-44AE-8A15-9366EA1BB9D9") (uuid "071c600f-b72b-44ae-8a15-9366ea1bb9d9")) returns false.

Spec http://www.itu.int/rec/T-REC-X.667/en says following:

6.5.4 Software generating the hexadecimal representation of a UUID shall not use upper case letters.
NOTE - It is recommended that the hexadecimal representation used in all human-readable formats be restricted to lower-case
letters. Software processing this representation is, however, required to accept both upper and lower case letters as specified
in 6.5.2.



 Comments   
Comment by Gary Fredericks [ 07/Mar/16 8:17 AM ]

Would this be a good time to change the internal representation from a string to either a pair of goog.math.Longs or a quartet of "32-bit" integer doubles?

Comment by Nikolay Durygin [ 09/Mar/16 2:22 AM ]

Is there any special need? Issue described above can be solved by lower casing all strings inside uuid. Another problem - the fact that uuid doesn't complain if non uuid format string is passed can be solved with regex.





[CLJS-1598] Honor printing of function values via IPrintWithWriter Created: 03/Mar/16  Updated: 08/Apr/16

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

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

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

 Description   

If a user wishes to define how function values are printed, allow that to be controlled via IPrintWithWriter with code like

(extend-type function
  IPrintWithWriter
  (-pr-writer [obj writer opts]
    ,,,))


 Comments   
Comment by Mike Fikes [ 03/Mar/16 10:28 AM ]

Can be tested manually:

$ script/nashornrepljs 
To quit, type: :cljs/quit
cljs.user=> inc
#object[cljs$core$inc "function cljs$core$inc(x){
return (x + (1));
}"]
cljs.user=> (extend-type function
  IPrintWithWriter
  (-pr-writer [obj writer _]
    (let [name (.-name obj)
          name (if (empty? name)
                 "Function"
                 name)]
      (write-all writer "#object[" name "]"))))
#object[Function]
cljs.user=> inc
#object[cljs$core$inc]
Comment by David Nolen [ 11/Mar/16 1:04 PM ]

The problem is this makes printing slower. For people using EDN as interchange format this may be a problem. Would need to see some numbers.

Comment by Antonin Hildebrand [ 08/Apr/16 2:11 PM ]

I'm not sure what is the difference between implements? and satisfies?. But by reading the code I would assume that it should be printed by this line:
https://github.com/clojure/clojurescript/blob/9a2be8bc665385be1ef866e2fd76b476c417d2bf/src/main/cljs/cljs/core.cljs#L9056-L9057

Don't we want to change implements? to satisfies? there? Not sure about (perf) implications.





[CLJS-1597] Redundant IPrintWithWriter test in pr-writer-impl Created: 03/Mar/16  Updated: 11/Mar/16  Resolved: 11/Mar/16

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

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

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

 Description   

This cond branch cannot be followed

https://github.com/clojure/clojurescript/blob/f58dcdf4dc37ef52d4eb1e2b7c994282bf6351f5/src/main/cljs/cljs/core.cljs#L9096-L9097

owing to this test

https://github.com/clojure/clojurescript/blob/f58dcdf4dc37ef52d4eb1e2b7c994282bf6351f5/src/main/cljs/cljs/core.cljs#L9049

and can be removed as effectively dead code.



 Comments   
Comment by David Nolen [ 11/Mar/16 1:08 PM ]

fixed https://github.com/clojure/clojurescript/commit/032077b52fd7c4359f18004f14faadab0d1a0782





[CLJS-1596] Self-host: :load-macros and :analyze-deps don't work in cljs.js Created: 01/Mar/16  Updated: 11/Mar/16  Resolved: 11/Mar/16

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

Type: Defect Priority: Minor
Reporter: Nikita Beloglazov Assignee: Mike Fikes
Resolution: Completed Votes: 0
Labels: bootstrap

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

 Description   

Playing with bootstrapped cljs I noticed that :analyze-deps doesn't work correctly. Setting it to false doesn't make any effect and deps are still being analyzed. I checked source code and it seems like :analyze-deps and :load-macros are always true regardless what you provide:

For example:
https://github.com/clojure/clojurescript/blob/master/src/main/cljs/cljs/js.cljs#L225

{
:*load-macros*  (or (:load-macros opts) true)
:*analyze-deps* (or (:analyze-deps opts) true)
}

Both these variables are always truthy. I think correct version is (:load-macros opts true) instead of using or in this case unless it is intentionally written this way to "disable" :load-macros/:analyze-deps options.



 Comments   
Comment by Mike Fikes [ 04/Mar/16 3:01 PM ]

Revised binding forms and added tests.

Comment by David Nolen [ 11/Mar/16 1:59 PM ]

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





[CLJS-1595] Update Closure Compiler to v20160208 Created: 29/Feb/16  Updated: 23/Apr/16  Resolved: 23/Apr/16

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

Type: Task Priority: Trivial
Reporter: Michael Zhou Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: clojurescript

Attachments: Text File update-closure-compiler-v20160208.patch    

 Description   

Now that Closure Cmopiler v20160208 has been pushed to Maven Central, update to that version.



 Comments   
Comment by Linus Ericsson [ 29/Feb/16 12:06 PM ]

Success report:

I applied update-closure-compiler-v20160208.patch to f58dcdf4 (22 feb) and successfully built clojurescript.

I added the resulting [org.clojure/clojurescript "1.8.28"] to a non-trivial reagent/figwheel based project.clj and updated its closure dependency as well.

I managed to start the repl and everything seem to work fine, these warnings are shown after upgrading clojurescript:

WARNING: Unknown option ':compiler-env'.
WARNING: Unknown option ':special-fns'.
WARNING: Unknown option ':warn-on-undeclared'.

I have not read all the changes to ClojureScript and therefore assume this is just some regression for changed compiler options.

$lein figwheel
Figwheel: Starting server at http://localhost:3449
Figwheel: Watching build - dev
Compiling "resources/public/js/compiled/frontlyft.js" from ["src" "../delat_src"]...
Successfully compiled "resources/public/js/compiled/frontlyft.js" in 1.693 seconds.
Figwheel: Starting CSS Watcher for paths ["resources/public/css"]
Launching ClojureScript REPL for build: dev
Figwheel Controls:
(stop-autobuild) ;; stops Figwheel autobuilder
(start-autobuild [id ...]) ;; starts autobuilder focused on optional ids
(switch-to-build id ...) ;; switches autobuilder to different build
(reset-autobuild) ;; stops, cleans, and starts autobuilder
(reload-config) ;; reloads build config and resets autobuild
(build-once [id ...]) ;; builds source one time
(clean-builds [id ..]) ;; deletes compiled cljs target files
(print-config [id ...]) ;; prints out build configurations
(fig-status) ;; displays current state of system
Switch REPL build focus:
:cljs/quit ;; allows you to switch REPL to another build
Docs: (doc function-name-here)
Exit: Control+C or :cljs/quit
Results: Stored in vars *1, *2, *3, *e holds last exception object
Prompt will show when Figwheel connects to your application
WARNING: Unknown option ':compiler-env'.
WARNING: Unknown option ':special-fns'.
WARNING: Unknown option ':warn-on-undeclared'.
To quit, type: :cljs/quit

These are the dependencies

:dependencies [[org.clojure/clojure "1.8.0"]
[org.clojure/clojurescript "1.8.28"]
[org.clojure/core.async "0.2.374"]
[reagent "0.5.1"]
[timothypratley/reanimated "0.1.4"]
[cljsjs/react "0.14.3-0"]
[com.andrewmcveigh/cljs-time "0.4.0"]
[com.google.javascript/closure-compiler "v20160208"]
[datascript "0.15.0"]
[cljs-http "0.1.39"]
[secretary "1.2.3" :exclusion [org.clojure/clojurescript]]]

:plugins [[lein-cljsbuild "1.1.2" :exclusion [org.clojure/clojure]]
[lein-figwheel "0.5.0-6"]]

Comment by Michael Zhou [ 23/Mar/16 4:12 PM ]

Is there an update on this?

Comment by David Nolen [ 23/Apr/16 3:51 PM ]

fixed https://github.com/clojure/clojurescript/commit/6ba817065113313a15b0f027c6491e0bc732f3e9

Comment by Michael Zhou [ 23/Apr/16 4:19 PM ]

Thanks!





[CLJS-1594] NaN and both infinities cannot be elements of a set Created: 27/Feb/16  Updated: 11/Mar/16  Resolved: 11/Mar/16

Status: Closed
Project: