<< Back to previous view

[CLJS-2474] with-meta on lazy-seq causes separate realization Created: 20/Jan/18  Updated: 20/Jan/18

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-2474.patch    
Patch: Code and Test

 Description   

Calling with-meta on a lazy-seq causes a separate realization, which is problematic if the lazy sequence is not deterministically realized. Observe this behavior in the following REPL interaction, which doesn't occur in Clojure:

cljs.user=> (defn random [] (lazy-seq (cons (rand) (random))))
#'cljs.user/random
cljs.user=> (def xs (random))
#'cljs.user/xs
cljs.user=> (def ys (with-meta xs {:foo 1}))
#'cljs.user/ys
cljs.user=> (take 3 xs)
(0.608952482736965 0.09053783024879025 0.3209446424968001)
cljs.user=> (take 3 ys)
(0.33162671415201395 0.5719320838932136 0.8606932935816556)

See CLJ-1800 for some insight into how Clojure avoids this, preserving the immutability of the return value of lazy-seq even in the face of adding meta.



 Comments   
Comment by Mike Fikes [ 20/Jan/18 4:51 PM ]

The attached patch is derived from the one in CLJ-1800. (Michael Blume has signed the CCA, so this derived patch should be fine from that perspective.)

This approach is important because in ClojureScript (unlike Clojure), calling with-meta on the return value of lazy-seq doesn't realize the first element. In other words, going with the CLJ-1800 approach preserves the existing laziness in ClojureScript.

The patch fundamentally works by tying the with-meta result back to the original sequence with a delayed call to seq on the original sequence. This is not part of included tests, but key to understanding how it achieves immutability is that while (not (identical? xs ys)), this is true: (identical? (rest xs) (rest ys)).





[CLJS-1800] Defer to tools.reader for cljs.reader functionality Created: 30/Sep/16  Updated: 20/Jan/18  Resolved: 19/Jan/18

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

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

Approval: Vetted

 Description   

We already have a dependency on tools.reader and maintaining our own EDN reader is just an unnecessary burden.



 Comments   
Comment by Aleš Roubíček [ 02/Oct/16 1:02 AM ]

I'm fiddling with this and found two differences in reading (according to cljs.reader-tests):

  1. cljs.tools.reader reads the @ macro as 'clojure.core/deref instead of 'deref
  2. cljs.tools.reader does not read small maps as PersistentArrayMap

Shoud this be solved by patching cljs.tools.reader or by changing cljs.reader-tests?

Comment by David Nolen [ 04/Oct/16 1:35 PM ]

Questions should be asked about 2). 1) seems fine.

Comment by Rohit Aggarwal [ 05/Oct/16 4:11 AM ]

I think fixing cljs.tools.reader/read-map to return a PersistentArrayMap or a PersistentHashMap is a relatively easy fix and should be raised in TRDR bug tracker.

Comment by Aleš Roubíček [ 26/Oct/16 7:04 AM ]

I did the patch for cljs.tools.reader http://dev.clojure.org/jira/browse/TRDR-40

Comment by David Nolen [ 08/Jul/17 4:39 PM ]

fixed https://github.com/clojure/clojurescript/commit/07ee2250af02b25f232111890c0f40f23150768d

Comment by Sam Umbach [ 12/Jan/18 9:18 PM ]

I believe this was fixed in the cljs 1.9.854 release but the "Fix Version/s" currently shows 1.9.671. I don't have the power to fix the metadata on this JIRA, but I'm sure someone does... @dnolen?

Comment by Sam Umbach [ 20/Jan/18 2:12 PM ]

Thanks, David!!





[CLJS-2469] ChunkedCons chunk-next returns empty seq instead of nil Created: 13/Jan/18  Updated: 20/Jan/18

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

Type: Defect Priority: Minor
Reporter: A. R Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None

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

 Description   

There is a bug in ChunkedCons. In Clojure ChunkedCons (correctly) always calls seq in chunked-next. In CLJS it's not done. But since ChunkedCons has to be lazy it pretty much always gets an (empty) lazy seq as the "more" part.

Bug:

(-> (map inc (vec (range 64)))
    seq
    chunk-next
    seq
    chunk-next)

Returns and empty sequence instead of nil. This hasn't surfaced yet since nothing calls chunk-next on a ChunkedCons (yet).



 Comments   
Comment by A. R [ 13/Jan/18 7:26 AM ]

Found another bug that surfaced: Current implementation calls -seq on more which could be nil and this would blow up. So the patch also make a tiny change to -next also just calling seq on more. Pretty straight forward.

Comment by David Nolen [ 19/Jan/18 3:20 PM ]

This patch needs a test.

Comment by A. R [ 20/Jan/18 12:27 AM ]

Test's added.





[CLJS-2171] Non deterministic compilation failure Created: 05/Jul/17  Updated: 19/Jan/18  Resolved: 19/Jan/18

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

Type: Defect Priority: Major
Reporter: Nicola Mometto Assignee: David Nolen
Resolution: Completed Votes: 2
Labels: None

Attachments: Text File CLJS-2171-2.patch     Text File CLJS-2171.patch    
Patch: Code
Approval: Accepted

 Description   

Since updating to 1.9.655, we're randomly seeing exception thrown during build, using the following compiler options:

:optimizations :advanced
:infer-externs true
:cache-analysis true
:parallel-build true
:recompile-dependents false

The stracktrace is as follows:

10:50:04 Exception in thread "main" clojure.lang.ExceptionInfo: failed compiling file:target/dist/public/s/static/js/auth/deps/cljs/core/async/impl/buffers.cljs {:file #object[java.io.File 0x697fc544 "target/dist/public/s/static/js/auth/deps/cljs/core/async/impl/buffers.cljs"]}, compiling:(/tmp/form-init936318712789593796.clj:1:72)
10:50:04 	at clojure.lang.Compiler.load(Compiler.java:7441)
10:50:04 	at clojure.lang.Compiler.loadFile(Compiler.java:7367)
10:50:04 	at clojure.main$load_script.invokeStatic(main.clj:277)
10:50:04 	at clojure.main$init_opt.invokeStatic(main.clj:279)
10:50:04 	at clojure.main$init_opt.invoke(main.clj:279)
10:50:04 	at clojure.main$initialize.invokeStatic(main.clj:310)
10:50:04 	at clojure.main$null_opt.invokeStatic(main.clj:344)
10:50:04 	at clojure.main$null_opt.invoke(main.clj:341)
10:50:04 	at clojure.main$main.invokeStatic(main.clj:423)
10:50:04 	at clojure.main$main.doInvoke(main.clj:386)
10:50:04 	at clojure.lang.RestFn.applyTo(RestFn.java:137)
10:50:04 	at clojure.lang.Var.applyTo(Var.java:702)
10:50:04 	at clojure.main.main(main.java:37)
10:50:04 Caused by: clojure.lang.ExceptionInfo: failed compiling file:target/dist/public/s/static/js/auth/deps/cljs/core/async/impl/buffers.cljs {:file #object[java.io.File 0x697fc544 "target/dist/public/s/static/js/auth/deps/cljs/core/async/impl/buffers.cljs"]}
10:50:04 	at clojure.core$ex_info.invokeStatic(core.clj:4725)
10:50:04 	at clojure.core$ex_info.invoke(core.clj:4725)
10:50:04 	at cljs.compiler$compile_file$fn__6002.invoke(compiler.cljc:1471)
10:50:04 	at cljs.compiler$compile_file.invokeStatic(compiler.cljc:1436)
10:50:04 	at cljs.compiler$compile_file.invoke(compiler.cljc:1412)
10:50:04 	at cljs.closure$compile_file.invokeStatic(closure.clj:497)
10:50:04 	at cljs.closure$compile_file.invoke(closure.clj:488)
10:50:04 	at cljs.closure$eval8017$fn__8018.invoke(closure.clj:566)
10:50:04 	at cljs.closure$eval7953$fn__7954$G__7942__7961.invoke(closure.clj:450)
10:50:04 	at cljs.closure$compile_from_jar.invokeStatic(closure.clj:548)
10:50:04 	at cljs.closure$compile_from_jar.invoke(closure.clj:536)
10:50:04 	at cljs.closure$eval8023$fn__8024.invoke(closure.clj:576)
10:50:04 	at cljs.closure$eval7953$fn__7954$G__7942__7961.invoke(closure.clj:450)
10:50:04 	at cljs.closure$compile_task$fn__8111.invoke(closure.clj:862)
10:50:04 	at cljs.closure$compile_task.invokeStatic(closure.clj:858)
10:50:04 	at cljs.closure$compile_task.invoke(closure.clj:850)
10:50:04 	at cljs.closure$parallel_compile_sources$fn__8121.invoke(closure.clj:889)
10:50:04 	at clojure.lang.AFn.applyToHelper(AFn.java:152)
10:50:04 	at clojure.lang.AFn.applyTo(AFn.java:144)
10:50:04 	at clojure.core$apply.invokeStatic(core.clj:657)
10:50:04 	at clojure.core$with_bindings_STAR_.invokeStatic(core.clj:1963)
10:50:04 	at clojure.core$with_bindings_STAR_.doInvoke(core.clj:1963)
10:50:04 	at clojure.lang.RestFn.invoke(RestFn.java:425)
10:50:04 	at clojure.lang.AFn.applyToHelper(AFn.java:156)
10:50:04 	at clojure.lang.RestFn.applyTo(RestFn.java:132)
10:50:04 	at clojure.core$apply.invokeStatic(core.clj:661)
10:50:04 	at clojure.core$bound_fn_STAR_$fn__6752.doInvoke(core.clj:1993)
10:50:04 	at clojure.lang.RestFn.invoke(RestFn.java:397)
10:50:04 	at clojure.lang.AFn.run(AFn.java:22)
10:50:04 	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
10:50:04 	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
10:50:04 	at java.lang.Thread.run(Thread.java:745)
10:50:04 Caused by: java.lang.ClassCastException: clojure.lang.Var$Unbound cannot be cast to java.util.concurrent.Future
10:50:04 	at clojure.core$deref_future.invokeStatic(core.clj:2288)
10:50:04 	at clojure.core$deref.invokeStatic(core.clj:2310)
10:50:04 	at clojure.core$deref.invoke(core.clj:2296)
10:50:04 	at cljs.analyzer$dump_specs.invokeStatic(analyzer.cljc:3620)
10:50:04 	at cljs.analyzer$dump_specs.invoke(analyzer.cljc:3611)
10:50:04 	at cljs.analyzer$write_analysis_cache.invokeStatic(analyzer.cljc:3644)
10:50:04 	at cljs.analyzer$write_analysis_cache.invoke(analyzer.cljc:3639)
10:50:04 	at cljs.compiler$emit_source.invokeStatic(compiler.cljc:1357)
10:50:04 	at cljs.compiler$emit_source.invoke(compiler.cljc:1287)
10:50:04 	at cljs.compiler$compile_file_STAR_$fn__5979.invoke(compiler.cljc:1381)
10:50:04 	at cljs.compiler$with_core_cljs.invokeStatic(compiler.cljc:1206)
10:50:04 	at cljs.compiler$with_core_cljs.invoke(compiler.cljc:1195)
10:50:04 	at cljs.compiler$compile_file_STAR_.invokeStatic(compiler.cljc:1370)
10:50:04 	at cljs.compiler$compile_file_STAR_.invoke(compiler.cljc:1363)
10:50:04 	at cljs.compiler$compile_file$fn__6002.invoke(compiler.cljc:1459)
10:50:04 	... 29 more

The error originates from this line https://github.com/clojure/clojurescript/blob/master/src/main/clojure/cljs/analyzer.cljc#L3697



 Comments   
Comment by Mike Fikes [ 19/Jan/18 2:28 PM ]

This seems to be a race between using Vars in the cljs.spec.alpha namespace and the namespace being initialized when compiling with parallel build true.

To see that this can happen, define a function

(defn try-it []
  (try
    (when-let [spec-ns (find-ns 'cljs.spec.alpha)]
      @@(ns-resolve spec-ns 'registry-ref))
    (catch Throwable t
      (.printStackTrace t)
      false)))

and start calling it in a tight loop in another thread

(.start (Thread. #(while (not (try-it)))))

Then

(require 'cljs.spec.alpha)

this can result in

java.lang.NullPointerException
	at clojure.core$deref_future.invokeStatic(core.clj:2208)
	at clojure.core$deref.invokeStatic(core.clj:2228)
	at clojure.core$deref.invoke(core.clj:2214)
	at user$try_it.invokeStatic(NO_SOURCE_FILE:3)
	at user$try_it.invoke(NO_SOURCE_FILE:1)
	at user$eval3$fn__4.invoke(NO_SOURCE_FILE:8)
	at clojure.lang.AFn.run(AFn.java:22)
	at java.lang.Thread.run(Thread.java:748)

followed by a repro of the Unbound issue:

java.lang.ClassCastException: clojure.lang.Var$Unbound cannot be cast to java.util.concurrent.Future
	at clojure.core$deref_future.invokeStatic(core.clj:2206)
	at clojure.core$deref.invokeStatic(core.clj:2228)
	at clojure.core$deref.invoke(core.clj:2214)
	at user$try_it.invokeStatic(NO_SOURCE_FILE:3)
	at user$try_it.invoke(NO_SOURCE_FILE:1)
	at user$eval3$fn__4.invoke(NO_SOURCE_FILE:8)
	at clojure.lang.AFn.run(AFn.java:22)
	at java.lang.Thread.run(Thread.java:748)

The race can be avoided by polling. Define

(defn- polling-ns-resolve
  "Repeatedly calls ns-resolve until the var returned is non-nil and bound,
  waiting between calls by supplied delay."
  [ns sym delay]
  (let [var (ns-resolve ns sym)]
    (if (and var (bound? var))
      var
      (do
        (Thread/sleep delay)
        (recur ns sym delay)))))

and with this there is no issue

(defn try-it' []
  (try
    (when-let [spec-ns (find-ns 'cljs.spec.alpha)]
      @@(polling-ns-resolve spec-ns 'registry-ref 10))
    (catch Throwable t
      (.printStackTrace t)
      false)))

(.start (Thread. #(while (not (try-it')))))

(require 'cljs.spec.alpha)

The attached patch adds a polling-ns-resolve and calls it at the problematic call site as a presumptive fix.

Comment by Mike Fikes [ 19/Jan/18 2:39 PM ]

Note: Dan Sutton reported in #clojurescript Slack that they saw a build failure with a similar stack trace and that they are using parallel build and that he recently added the first use of core spec alpha in their codebase.

Comment by David Nolen [ 19/Jan/18 3:16 PM ]

Let's try a patch with `load-mutex` which allows us to lock around requires. It seems to me it might work sinc this looks like a race where the ns object is present but no vars have been bound yet.

Comment by Mike Fikes [ 19/Jan/18 3:30 PM ]

I agree, this is much simpler and seems like it would work. In an attempt to avoid contention, the attached CLJS-2171-2.patch acquires the mutex only after it is known that the cljs.spec.alpha has been (or is being) loaded. Importantly, the ns-resolve calls only occur after the mutex has been acquired. Unable to thoroughly test this apart from running unit tests and trying it on a few projects (which I plan to do).

Comment by Mike Fikes [ 19/Jan/18 3:43 PM ]

I've tried this on two downstream projects that make use of spec with :parallel-build true with the 2nd patch, without any obvious issues (the compiler didn't lock up, etc.)

Comment by David Nolen [ 19/Jan/18 3:58 PM ]

fixed https://github.com/clojure/clojurescript/commit/9ddd356d344aa1ebf9bd9443dd36a1911c92d32f





[CLJS-2473] Infer character literals to have string type Created: 16/Jan/18  Updated: 19/Jan/18  Resolved: 19/Jan/18

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

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

Attachments: Text File CLJS-2473.patch    
Patch: Code and Test
Approval: Accepted

 Description   

In JVM ClojureScript, (+ \a 1) will not trigger an analysis error, but this will under self-hosted ClojureScript owing to the fact that character literals are read in as strings before analysis.

By revising the analyzer to include an additional (instance? Character form) in analyze-form we can improve type inference for JVM ClojureScript.



 Comments   
Comment by A. R [ 17/Jan/18 1:20 AM ]

By coercing the character to to a string afterwards we could also solve CLJS-2087 :

#{"a" \a}
Comment by David Nolen [ 19/Jan/18 3:08 PM ]

fixed https://github.com/clojure/clojurescript/commit/56da266bcd5357e437f12d4b36e41b0fbaae230f





[CLJS-2395] (str (goog.date.Date. 2017 11 8)) behaves differently with no optimizations than with :simple or :advanced Created: 08/Nov/17  Updated: 19/Jan/18

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

Type: Defect Priority: Minor
Reporter: Eero Helenius Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Zip Archive cljs-goog-date-str-bug.zip    

 Description   

With :optimizations :none, (str (goog.date.Date. 2017 11 8)) returns 20171208, but with :simple or :advanced, it returns 1512684000000.

I attached a simple test case with a README.



 Comments   
Comment by A. R [ 08/Nov/17 6:41 AM ]

Closure compiler changed at some point:

$cljs$core$str$$.$cljs$core$IFn$_invoke$arity$1$ = function $Qd($x$jscomp$279$$) {
  return null == $x$jscomp$279$$ ? "" : "" + $x$jscomp$279$$;
};

It used to keep our [x].join("") code.

Workaround could be to emit a [x, ""].join("")

See CLJS-890 for details

Comment by A. R [ 19/Jan/18 1:05 AM ]

Reported it a bit ago: https://github.com/google/closure-compiler/issues/2782

They're now rewriting x.join("") to String:

https://github.com/google/closure-compiler/commit/b6973ec7b37f3890c8b0e11456afa95d79aaffab

which would also fix CLJS-1631 .





[CLJS-2344] de-dupe externs Created: 31/Aug/17  Updated: 16/Jan/18

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

Type: Enhancement Priority: Minor
Reporter: David Nolen Assignee: Sameer Rahmani
Resolution: Unresolved Votes: 1
Labels: closure

Attachments: Text File CLJS-2344.patch    
Patch: Code
Approval: Vetted

 Description   

We could possibly call distinct on externs to remove dupes in the common case where externs may appear multiple times accidentally on the classpath.



 Comments   
Comment by David Nolen [ 16/Jan/18 8:09 AM ]

I don't think we want to silently dedupe. We probably want to also warn so users can fix the issue. The only reason to even do this ticket is because the Closure warning is so unfriendly and fails the build.

Comment by Sameer Rahmani [ 16/Jan/18 8:33 AM ]

got it, I'll improve the patch





[CLJS-2472] ES6 Iterators should use IIterable if possible Created: 15/Jan/18  Updated: 15/Jan/18

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

Type: Enhancement Priority: Minor
Reporter: A. R Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: performance


 Description   

Motivation:

ES6 Iterables: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Iteration_protocols

Use in React: https://github.com/facebook/react/blob/30dac4e78de02fb427ee82013160ae875128d7a2/packages/react/src/ReactElementValidator.js#L195-L204

(defn es6-iterator**
  [coll]
  (if (implements? IIterable coll)
    (let [it (-iterator ^not-native coll)]
      #js{:next (fn []
                  (if ^boolean (.hasNext it)
                    #js{:value (.next it), :done false}
                    #js{:value nil, :done true}))})
    ;; Fallback to naive first/next iterator:
    (ES6Iterator. (seq coll))))

;; Tests can use round trip:
(es6-iterator-seq (es6-iterator (hash-map 0 1 2 3)))

(defn es6-iter-consume
  [it]
  (while (not (.-done (.next it)))))

(dotimes [_ 3]
  (let [xs (vec (range 3000))
        runs 1000]
    (simple-benchmark []
      (es6-iter-consume (es6-iterator xs)) runs)
    (simple-benchmark []
      (es6-iter-consume (es6-iterator** xs)) runs)))

About a 4x speedup in Chrome. Also note that much less garbage is produced.






[CLJS-2471] ChunkedSeq should implemented ICounted Created: 15/Jan/18  Updated: 15/Jan/18

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

Type: Enhancement Priority: Minor
Reporter: A. R Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

ChunkedSeq in clojure implements Counted, should do the same in CLJS. Implementation is straight forward.






[CLJS-2470] ArrayChunk.reduce doesn't honor end param Created: 15/Jan/18  Updated: 15/Jan/18

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

Type: Defect Priority: Minor
Reporter: A. R Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Ex:

(reduce max (array-chunk #js[0 1 2 3] 0 1))
;; => 3

Currently not an issue since end is always arr.length for our usage of ArrayChunk, but since it's a public API users might pass a different end param.

This can easily be fixed after CLJS-2466 which properly implements the reduce for ArrayChunk.






Generated at Sat Jan 20 21:08:17 CST 2018 using JIRA 4.4#649-r158309.