<< Back to previous view

[CLJS-986] Add :target to the list of build options that should trigger recompilation Created: 21/Jan/15  Updated: 21/Jan/15

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

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





[CLJS-981] Better benchmarking infrastructure Created: 17/Jan/15  Updated: 17/Jan/15

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

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


 Description   

We should use ProcessBuilder to run the various benchmark scripts and control which benchmarks we test and which engines we run. Benchmarks should produce EDN data that can be written to a file, loaded into Incanter, etc.






[CLJS-978] Analysis caching doesn't account for constants table Created: 15/Jan/15  Updated: 16/Jan/15

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

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


 Description   

This means advanced optimizations cannot leverage analysis caching.



 Comments   
Comment by Thomas Heller [ 16/Jan/15 4:48 AM ]

Currently the constants use a generated id, we could instead use the constant itself as the key.

:test/keyword creates cljs.core.constant$keyword$123 where 123 is the sequential id which makes it unusable between compiles. We could instead turn it into cljs.core.constant$keyword$_COLON_test_SLASH_keyword (to reuse current munge). We would then just need to keep track of which namespace uses which constant and emit this as well in the analysis cache.

Might be missing something, will investigate a bit later.

Comment by David Nolen [ 16/Jan/15 5:59 AM ]

The constants table needs to work for all compile time constants - vectors, sets, maps, etc.





[CLJS-977] Subvec missing IKVReduce implementation Created: 15/Jan/15  Updated: 15/Jan/15

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

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





[CLJS-976] Node REPL breaks from uncaught exceptions Created: 14/Jan/15  Updated: 14/Jan/15

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

Type: Defect Priority: Major
Reporter: Joel Holdbrooks Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Certain errors raised by Node will cause a ClojureScript Node REPL to fail.

Steps to reproduce:

cljs.user> (require '[cljs.nodejs :as node])
cljs.user> (def fs (node/require "fs"))
cljs.user> (.createReadStream fs "fail")
;; => #<[object Object]>
cljs.user> (+ 1 1)
;; => java.lang.IllegalArgumentException: Value out of range for char: -1

A vanilla Node REPL will also fail under the same condition and terminate the session.

events.js:72
        throw er; // Unhandled 'error' event
              ^
Error: ENOENT, open 'fail'

The CLJS REPL should probably be a bit more durable and, instead of terminating, catch and report the error.

A gist of the above example can be found here: https://gist.github.com/noprompt/3d0e5cbfca7883727fcf.






[CLJS-975] preserve :reload & :reload-all in ns macro sugar Created: 13/Jan/15  Updated: 13/Jan/15

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

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


 Description   

:include-macros true and :refer-macros [...] are both sugar forms supported in :use and :require, they expand into :use-macros or :require-macros specs. We should make sure to preserve :reload and :reload-all when desugaring.



 Comments   
Comment by Thomas Heller [ 13/Jan/15 10:48 AM ]

Oops, didn't see there was an issue for this.

Sorry for the rant but please note my comment on:
https://github.com/clojure/clojurescript/commit/e408305344ef7b3658118377e530c78c2eb93cf3

I do not think REPL related "options" have anything to do in the parse function of the namespace. I outlined a different approach which I hope you'll consider. Of course all of this is motivated by shadow-build, so YMMV.

Comment by David Nolen [ 13/Jan/15 11:16 AM ]

While shadow-build is an interesting piece of technology it's not relevant for tickets like this. Please keep threads clear of non-issues.





[CLJS-972] Node.js REPL eats errors in required ns when using require Created: 12/Jan/15  Updated: 12/Jan/15

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

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





[CLJS-969] Node REPL fails with TypeError under Windows Created: 08/Jan/15  Updated: 08/Jan/15

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

Type: Defect Priority: Major
Reporter: Adrian Sampaleanu Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Windows 8.1, Node.js 0.10.35



 Description   

With latest source and following the steps outlined in The Node.js REPL of My Dreams, the following error is emitted when noderepljs is invoked:

$ ./script/noderepljs
To quit, type: :cljs/quit
ClojureScript Node.js REPL server listening on 58343
"Error evaluating:" (do (.require js/goog "cljs.core") (set! *print-fn* (.-print (js/require "util")))) :as "goog.require(\"cljs.core\");\r\n\r\ncljs.core._STAR_print_fn_STAR_ = require(\"
util\").print;\r\n"
TypeError: Cannot read property 'nameToPath' of undefined
    at Object.goog.require (repl:2:49)
    at repl:1:6
    at Socket.<anonymous> ([stdin]:30:80)
    at Socket.emit (events.js:95:17)
    at Socket.<anonymous> (_stream_readable.js:765:14)
    at Socket.emit (events.js:92:17)
    at emitReadable_ (_stream_readable.js:427:10)
    at emitReadable (_stream_readable.js:423:5)
    at readableAddChunk (_stream_readable.js:166:9)
    at Socket.Readable.push (_stream_readable.js:128:10)
ClojureScript:cljs.user>

Though the correct prompt appears, the REPL is non functional.



 Comments   
Comment by David Nolen [ 08/Jan/15 7:19 AM ]

Thanks for the report. Sadly there are no cycles to work on Windows issues ourselves. Patches from Windows users are alway welcome.





[CLJS-968] Metadata on function literal inside of a let produces invalid Javascript Created: 07/Jan/15  Updated: 23/Jan/15

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

Type: Defect Priority: Major
Reporter: Bobby Eickhoff Assignee: Nicola Mometto
Resolution: Unresolved Votes: 1
Labels: bug
Environment:

Originally found with [org.clojure/clojurescript "0.0-2496"]
Still reproducible with the latest cljsc (b5e9a5116259fc9f201bee4b9c6564f35306f9a5)



 Description   

Here is a minimal test case that produces the invalid Javascript:

(defn f []
  (let [a 0]
    ^{"meta" "data"}
    (fn [] true)))

The compiled Javascript includes the invalid token sequence "return return". (Per Chrome: Uncaught SyntaxError: Unexpected token return)

The problem does not occur if the metadata applies to a map literal instead of a function literal.
The problem only occurs when the function and metadata are inside of a let.



 Comments   
Comment by Bobby Eickhoff [ 07/Jan/15 9:45 PM ]

I forgot to try with-meta. Using with-meta does not produce this syntax error, so it's only a problem with the reader macro for metadata.

Comment by David Nolen [ 08/Jan/15 7:41 AM ]

Any quick thoughts about this one Nicola? Quite possibly a compiler issue on the CLJS side.

Comment by Nicola Mometto [ 08/Jan/15 8:07 AM ]

David, I understand why this happens but I don't know enough about how cljs's js emission to propose a fix.
The issue is that with this commit: https://github.com/clojure/clojurescript/commit/d54defd32d6c5ffcf6b0698072184fe8ccecc93a the following scenario is possible:

{:op :meta
 :env {:context :return}
 :expr {:op :fn
        :env {:context :expr}
        :methods [{:op :fn-method 
                   :env {:context :return} ..}]
        ..}
 ..}

i.e. analyze-wrap-meta changes the context of the :fn node to :expr but keeps the context of the :fn-methods to :return.

This causes both
https://github.com/clojure/clojurescript/blob/master/src/clj/cljs/compiler.clj#L575-L576
and
https://github.com/clojure/clojurescript/blob/master/src/clj/cljs/compiler.clj#L488 (https://github.com/clojure/clojurescript/blob/master/src/clj/cljs/compiler.clj#L233)

to be true and emit a "return".





[CLJS-967] "java.net.ConnectException: Connection refused" when running node repl Created: 07/Jan/15  Updated: 08/Jan/15

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

Type: Defect Priority: Major
Reporter: Brian Muhia Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: cljs, errormsgs
Environment:

Processor: 1.33GHz Intel Atom, Ubuntu 12.04, OpenJDK 1.7, Clojure 1.6.0, ClojureScript 0.0-2665, nodejs v0.10.26


Attachments: Text File backtrace.txt    

 Description   

I used trampoline like so:

rlwrap lein trampoline run -m clojure.main scripts/repl.clj

run the script repl.clj with contents:

(require
  '[cljs.repl :as repl]
  '[cljs.repl.node :as node])

(repl/repl* (node/repl-env)
  {:output-dir "out"
   :optimizations :none
   :cache-analysis true                
   :source-map true})

Instead of getting a repl, I got the exception: Exception in thread "main" java.net.ConnectException: Connection refused, compiling/home/brian/projects/essence-cljs-redux/scripts/repl.clj:3:30)

The full stack trace is attached in backtrace.txt.



 Comments   
Comment by David Nolen [ 08/Jan/15 7:21 AM ]

The issue is that we use a 300ms timeout to connect to the Node.js process, we need to instead redirect the process out and wait for the Node.js REPL server start string.





[CLJS-965] Make :foreign-libs more useful Created: 07/Jan/15  Updated: 08/Jan/15

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

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


 Description   

:foreign-libs almost does what we need to make working with JavaScript dependencies painless. They need to additionally specify :requires. :foreign-libs should automatically be preambled in advanced compilation, dependency order can be solved since they provide :requires. Coupled with the deps.clj support using JS deps from JARs would be as simple and as painless as CLJS JARs.

The following conditions must be met:

A) do not break existing usage of deps.clj - means we need to check :foreign-lib true in deps.clj
B) Support optimizations :none, means we must emit goog.addDependency in deps.js
C) Support optimizations :advanced, means we should sort in dependency order and place foreign between :preamble and Closure advanced build
D) users should be able to specify a JS file to use under :none to support debugging



 Comments   
Comment by Yehonathan Sharvit [ 08/Jan/15 1:46 PM ]

Is it currently possible to include a js file inside a cljs lib, without requiring from the library clients to download manually the js file?

Comment by David Nolen [ 08/Jan/15 1:51 PM ]

It is not possible and such a feature is out of scope.

Comment by Francis Avila [ 08/Jan/15 2:45 PM ]

Isn't this done all the time, even with foreign-libs? Or do I misunderstand the question? Yehonathan, what specific scenario are you thinking of? Serving a foreign-lib from a cdn?





[CLJS-961] REPL process should always be killable Created: 04/Jan/15  Updated: 04/Jan/15

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

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


 Description   

In the browser REPL case if user never connects browser cannot kill REPL with Ctrl-C or Ctrl-D.






[CLJS-960] On carriage return REPLs should always show new REPL prompt Created: 04/Jan/15  Updated: 04/Jan/15

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

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





[CLJS-957] Parallel compilation Created: 03/Jan/15  Updated: 06/Jan/15

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

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


 Description   

For non-interacting branches of the dependency graph we could compile in parallel.



 Comments   
Comment by Thomas Heller [ 03/Jan/15 3:06 PM ]

Just a heads-up: I attempted this in shadow-build.

The problem with this is macros, specifically their require. Granted you said non-interacting but suppose two namespaces require cljs.core.async.macros, they might trigger the clojure.core/require at the same time (parallel after all).

This resulted in a very weird exception: https://www.refheap.com/91731

I'm not sure how thread-safe clojure.core/require is supposed to be but it was as close as I was able to get tracking this weird error down. Removing reducers fixed everything. FWIW the gain was minimal to begin with.

Comment by David Nolen [ 03/Jan/15 3:09 PM ]

Making this really work I think largely depends on how much time you are willing to spend on the problem. Fork/join work stealing seems ideal for this not sure if you tried that. RE: macros, seems like you could use information collected from ns analysis to serialize calls to require.

Comment by Thomas Heller [ 03/Jan/15 3:28 PM ]

Lets throw some reducers at it was about as much thought I put into it.

Just beware of macros when you attempt to do something is all I wanted to say. Treating analysis seperate from cljs.compiler/emit might be a good idea though.

Comment by Dusan Maliarik [ 06/Jan/15 7:20 AM ]

I always thought parallel compilation belongs to the build tool (process), and should be parallelized per source file. Would there be any gain in using pmap in here ? It's used by lein-cljsbuild I suppose. Or parallelize this part?

Comment by David Nolen [ 06/Jan/15 7:54 AM ]

pmap won't work because of dependencies. A work queue approach is probably going to be most fruitful. Let's keep this thread clean unless you are actually going to work on this ticket and have a clear picture how to do it. Thanks.





[CLJS-948] Simplify macro usage Created: 02/Jan/15  Updated: 09/Jan/15

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

Type: Enhancement Priority: Major
Reporter: Thomas Heller Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None

Attachments: File cljs-948.diff     File cljs-948-no-passes.diff    

 Description   

The usage of macros from CLJS is very explicit and users of any given namespace must remember whether it comes with macros and which vars are macros. This leads to rather verbose :require statements which are confusing and lead to unexpected results if incomplete (eg. missing :refer-macros).

(ns my-ns
  (:require [my-app.other-ns :as other :refer (something) :refer-macros (something) :include-macros true]))

(something) ;; macro (when :refer-macros, otherwise no macro)
(other/thing) ;; maybe macro (only when :include-macros, otherwise no macro)

I think the user should not need to know whether something is a macro or not, they should just be able to use it.

I implemented a proof-of-concept that

(ns my-ns
  (:require [my-app.other-ns :refer (something)])

will work just find and do the correct thing with regards to macros (assuming the namespace has a corresponding clj namespace, :require-macros is unaffected). No changes to any code are necessary as the implementation uses ana/passes, it is also fully backwards compatible.

Implementation points which should be discussed

  • The CLJS namespace with macros currently has to opt-in through a bit of metadata (eg. (ns my-ns-with-macros {:load-macros true})), that might not be necessary and maybe should default to true.
  • The implementation assumes compilation happens in dependency order. shadow-build always does this, I'm not too sure about CLJS though. Given ana/analyze-deps equals true that is guaranteed but I'm not sure that is always the case.

If there is any interest I can provide a patch, until then refer to the proof-of-concept [1].

[1] https://github.com/thheller/shadow-build/blob/f37cfa598f1e90dd66e333d1e45580ea25650025/src/clj/shadow/cljs/passes.clj#L30-L82



 Comments   
Comment by David Nolen [ 02/Jan/15 11:25 AM ]

This is interesting. Will think about it.

Comment by Thomas Heller [ 02/Jan/15 1:15 PM ]

Is it ok if I leave implementation notes here?

Things that would need addressing should this be implemented:

  • cljs.analyzer/check-uses runs when parsing the ns form, since the macro passes run AFTER the parsing finished the check-uses will warn incorrectly if a :refer is not defined in CLJS but is a macro. Solution: check-uses should probably just become a pass itself which runs after the macro ones. Moving a side-effect out of the parse function is probably well worth it.
  • cljs.analyzer/load-macros would obsolete and the entire functionality it was toggling can be moved to the load-macros pass.
Comment by David Nolen [ 03/Jan/15 3:33 PM ]

I looked a bit at your code. It looks interesting. Definitely not going to do the namespace metadata bit. It seems to me this should work only if the required namespace loaded macros from a Clojure namespace of the same name.

Comment by Thomas Heller [ 03/Jan/15 3:36 PM ]

Yeah, probably. Also played with the idea of just looking for (io/resource (str (ns->path) ".clj")). If that is nil the namespace doesn't have macros, if its not null we require it.

But I like your approach of just using (ns my-ns (:require-macros [my-ns])) more.

Comment by David Nolen [ 03/Jan/15 3:40 PM ]

OK, let's try a patch for this. This is an interesting simplification that unifies Clojure & ClojureScript macro usage.

Comment by Thomas Heller [ 03/Jan/15 4:10 PM ]

Yay! Coming right up.

One issue though: I thought it would be a good idea to put the passes into a separate file (eg. cljs.analyzer.passes), that is currently not possible due to cljs.analyzer/analyze (it controls the passes defaults) [1]. cljs.analyzer cannot depend on cljs.analyzer.passes since it most likely depends on cljs.analyzer (circular dependency). Technically the passes currently only require the analyzer because of ::ana/namespaces but faking out a circular dependency is not a good idea.

If we remove the (or passes [infer-type]) and instead force opts to contain :passes (or bind passes beforehand) we should be fine.

Although I'm perfectly fine with putting the new passes into cljs.analyzer, just thought it would be cleaner not to.

Also, how should I address the invalid refer warning issue? Since the check-uses runs before the pass it can never find the macro. I was going to make check-uses a pass in itself if thats alright with you.

[1] https://github.com/clojure/clojurescript/blob/2d1e2167f5ab8bd76ac5c8bafd198990cc88d34a/src/clj/cljs/analyzer.clj#L1711

Comment by Thomas Heller [ 03/Jan/15 5:01 PM ]

I cleaned up the reference in shadow-build [1]. This is how a cljs.analyer.passes could look, the default passes would then be

[cljs.analyzer.passes/load-macros
 cljs.analyzer.passes/infer-macro-require
 cljs.analyzer.passes/infer-macro-use
 cljs.analyzer.passes/check-uses
 cljs.analyzer/infer-type]

This would basically make ana/load-macros obsolete since this can now be controlled via passes. Also addresses CLJS-955.

[1] https://github.com/thheller/shadow-build/blob/220d3bb0bef2cdb5696487b639ca5aaa169c56f2/src/clj/shadow/cljs/passes.clj

Comment by David Nolen [ 03/Jan/15 5:40 PM ]

This looks OK to me at the moment.

Comment by Thomas Heller [ 03/Jan/15 5:40 PM ]

Should stop refering to specific hashes, please see [1].

There is a slight issue with cljs.core itself. There is a bit of code that makes cljs.core macros special. I'm not quite sure why this is, since it would not be required if it did.

(ns cljs.core
  (:require-macros [cljs.core]))

Getting an error [2] on the recently changed def to (defonce print-fn). Haven't exactly figured out why this is yet.

[1] https://github.com/thheller/shadow-build/blob/master/src/clj/shadow/cljs/passes.clj
[2] https://www.refheap.com/61bc05e4b4a74a3b573797721

Comment by Thomas Heller [ 03/Jan/15 5:44 PM ]

Wait, are other namespaces allowed to use their own macros unqualified?

(ns my-ns
  (:require-macros [my-ns]))

(this-is-a-macro)
;; vs
(my-ns/this-is-a-macro)

I don't think so? The recent defonce seems to be an exception?

Comment by David Nolen [ 03/Jan/15 5:50 PM ]

yes cljs.core and core macros are an exception

Comment by Thomas Heller [ 03/Jan/15 5:54 PM ]

Yeah, but cljs.core calls all macros directly.

(defn ^boolean identical?
  "Tests if 2 arguments are the same object"
  [x y]
  (cljs.core/identical? x y))

The recently changed defonce vars are the exception here. Although even if I change them to cljs.core/defonce I still can't explain the error.

Comment by David Nolen [ 03/Jan/15 5:56 PM ]

The inlining macros are an exception, it's done that way to break circularity which would otherwise stack overflow the compiler.

Comment by Thomas Heller [ 03/Jan/15 7:09 PM ]

Alright, resolved that macro issue.

I updated [1] so the analyzer passes are initialized correctly and outside cljs.analyzer. The patch would also change the cljs.analyzer/parse 'def since that functionality is moved to cljs.analyzer.passes.

(when (and *analyze-deps* (seq @deps))
      (analyze-deps name @deps env opts))
    #_ (when (and *analyze-deps* (seq uses))
         (check-uses uses env))
    (set! *cljs-ns* name)
    #_ (when *load-macros*
         (load-core)
         (doseq [nsym (concat (vals require-macros) (vals use-macros))]
           (clojure.core/require nsym))
         (when (seq use-macros)
           (check-use-macros use-macros env)))

The question is where do we require cljs.analyzer.passes (must be required somewhere so the default is configured correctly)?

cljs.closure is probably the best bet.

Not a big fan of this alter-var-root but I'm not sure how we could do without. I doubt anyone but me currently uses cljs.analyzer/passes and therefore the default behavior of (or passes [infer-type]) always triggers. Since that would no longer work correctly we need to initialize to another default.

cljs.analyzer/parse-ns would also need to change again given the removal of load-macros. Same functionality can be achieved within the same binding though.

[1] https://github.com/thheller/shadow-build/blob/master/src/clj/shadow/cljs/passes.clj

Comment by Thomas Heller [ 03/Jan/15 8:16 PM ]

My head hurts, I need to get some sleep.

I cannot figure out why the attached patch does not work, but ./script/test produces errors in parse-ns.

Will get back to it tommorrow.

Exception in thread "main" java.lang.IllegalArgumentException: Don't know how to create ISeq from: clojure.lang.Symbol, compiling:(/Users/zilence/code/oss/clojurescript/bin/../bin/cljsc.clj:18:68)
	at clojure.lang.Compiler.load(Compiler.java:7142)
	at clojure.lang.Compiler.loadFile(Compiler.java:7086)
	at clojure.main$load_script.invoke(main.clj:274)
	at clojure.main$script_opt.invoke(main.clj:336)
	at clojure.main$main.doInvoke(main.clj:420)
	at clojure.lang.RestFn.invoke(RestFn.java:436)
	at clojure.lang.Var.invoke(Var.java:388)
	at clojure.lang.AFn.applyToHelper(AFn.java:160)
	at clojure.lang.Var.applyTo(Var.java:700)
	at clojure.main.main(main.java:37)
Caused by: java.lang.IllegalArgumentException: Don't know how to create ISeq from: clojure.lang.Symbol
	at clojure.lang.RT.seqFrom(RT.java:505)
	at clojure.lang.RT.seq(RT.java:486)
	at clojure.core$seq.invoke(core.clj:133)
	at cljs.analyzer$parse_ns$fn__1660.invoke(analyzer.clj:1757)
	at cljs.analyzer$parse_ns.invoke(analyzer.clj:1747)
	at cljs.analyzer$parse_ns.invoke(analyzer.clj:1741)
	at cljs.compiler$compile_root.invoke(compiler.clj:1069)
	at cljs.closure$compile_dir.invoke(closure.clj:358)
	at cljs.closure$eval3084$fn__3085.invoke(closure.clj:398)
	at cljs.closure$eval3020$fn__3021$G__3011__3028.invoke(closure.clj:306)
	at cljs.closure$eval3071$fn__3072.invoke(closure.clj:412)
	at cljs.closure$eval3020$fn__3021$G__3011__3028.invoke(closure.clj:306)
	at cljs.closure$build.invoke(closure.clj:1018)
	at cljs.closure$build.invoke(closure.clj:972)
	at user$eval3279.invoke(cljsc.clj:21)
	at clojure.lang.Compiler.eval(Compiler.java:6703)
	at clojure.lang.Compiler.load(Compiler.java:7130)
	... 9 more
Comment by Thomas Heller [ 04/Jan/15 5:40 AM ]

That is why you should probably not write code tired at 3am. Was using infer-tag as a pass instead of infer-type.

I decided to drop the cljs.analyzer.passes namespace and instead moved the passes to cljs.analyzer. The extra namespace introduced a weird circular dependency which cljs.analyzer/parse-ns couldn't quite handle.

The patch now applies cleanly, would probably be nice to have a test that actually tests that it does what its supposed to.

Comment by David Nolen [ 07/Jan/15 8:21 AM ]

The patch has not been updated with these changes.

Comment by Thomas Heller [ 07/Jan/15 8:37 AM ]

Sorry, my mistake. Fixed.

Comment by Thomas Heller [ 08/Jan/15 7:19 AM ]

One issue I found is related to caching. Depends on how/where cljs.analyzer/parse-ns is called.

If the new passes aren't executed and the result of that analysis are used to write out the cache it won't contain the new macros information. I believe cljs.analyzer/parse-ns does this.

On the next run if it was decided to use the cache, we are missing some information AND won't require the macros. This was true before the changes but since the dependant namespaces would require the macros on their own the issue was somewhat hidden. Now they assume that macros were already loaded, therefore the must require macros when reconstructing from cache.

How to do that best in CLJS I don't know since shadow-build does its own caching. The passes could be refactored so they can be called with the cached information as well.

Note that this issue was basically introduced by load-macros (CLJS-940), but since that flag is already gone it is now moved to the :load-macros option of parse-ns.

The simplest way to address this would be to always run the new passes, basically how it was before CLJS-940.

Comment by David Nolen [ 08/Jan/15 7:39 AM ]

Thomas to me this illustrates the pitfalls of trying to bring a multipass approach to an existing compiler that isn't already fundamentally multipass. It seems to me that an alternative approach is possible without bothering at all about passes. In the analyzer parse 'ns case for each required CLJS lib you could leverage parse-ns to see if it requires a macro file of the same name - if it does add an implicit :require-macros.

While I like the passes approach in theory it will just make it harder for contributors to understand the structure of the compiler, they have to look at two different approaches to determine where things happen.

Comment by Thomas Heller [ 08/Jan/15 7:56 AM ]

Well, yeah we could to away with the passes and do it all in (defmethod parse 'ns ...) but that doesn't change the issue as long as there is a way to toggle this at all (ie. :load-macros or load-macros). It is not really about whether it is in a pass or not.

I just liked using passes since it sort of allows cherry-picking what you want and don't want to run. infer-types doesn't need to run in parse-ns for example, but it always did before this.

But I'm not committed to passes, happy to move everything so it runs in the parse method.

Let me know, if I should rewrite the patch.

Comment by Thomas Heller [ 08/Jan/15 8:35 AM ]

Hmm, you are right. Will move it out of the passes.

The only reason it was in there in the first place is due to shadow-build. It was the only place I could put it to make it work without patching CLJS. But since we agree that this should be in core the passes option really doesn't make sense any more.

Going to deliver a new patch soon.

Comment by David Nolen [ 08/Jan/15 8:37 AM ]

The difference for inter-types is that is something that needs to be run for a large number of nodes - it's a real global pass.

Let's go with moving the logic in to parse 'ns. I still don't understand the *load-macros* issue you are describing. We absolutely need to be able to disable this and dependency analysis for legitimate uses of parse-ns.

Comment by Thomas Heller [ 08/Jan/15 9:42 AM ]

Attached the no-passes version. The does get a bit simpler since it doesn't have to manually patch the compiler env.

I'm not totally sure how the @reload logic works, maybe some adjustments are required.

Comment by Thomas Heller [ 08/Jan/15 9:53 AM ]

The *load-macros* issue manifests as follows:

(ns demo.ns1
  (:require-macros [demo.ns1 :as m]))

(ns demo.ns2
  (:require [demo.ns1 :as x :refer (a-macro)]))

Compile 1: Everything is fine, cache is written.
Compile 2:

  • ns1 not modified, parse-ns decides to use cached version
  • ns2 modified, compiler attempts to compile, fails with NullPointerException
Caused by: java.lang.NullPointerException
	at cljs.analyzer$get_expander.invoke(analyzer.clj:1532)
	at cljs.analyzer$macroexpand_1.invoke(analyzer.clj:1543)
	... 80 more

This is because the load from cache does not require the macro namespace. Before these changes demo.ns2 would have had a :require-macros or equivalent in the ns form which would then have require'd the macro ns. Now only demo.ns1 would trigger the require, but since it loads from cache it doesn't UNLESS *load-macros* is true. Since parse-ns does revive the cache (I think) it is not.

The require must be triggered somewhere before actual compilation happens. In shadow-build this is pretty simple but I'm not sure where to put this for CLJS.

Comment by Thomas Heller [ 09/Jan/15 10:25 AM ]

As per IRC the *load-macros* issues does not present in CLJS since it is true when it needs to be.





[CLJS-937] local fn name should be namespace munged Created: 30/Dec/14  Updated: 30/Dec/14

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

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


 Description   
(ns foo.bar)

(fn console []
  ...)

The local name should be foo$bar$console



 Comments   
Comment by David Nolen [ 30/Dec/14 3:12 PM ]

See CLJS-833





[CLJS-927] :recompile-dependents build option Created: 24/Dec/14  Updated: 27/Dec/14

Status: In Progress
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: David Nolen Assignee: David Nolen
Resolution: Unresolved Votes: 5
Labels: None


 Description   

It should be possible to optionally recompile dependent namespaces. This would make disk based caching less prone to strange errors because the user didn't do this manually.



 Comments   
Comment by David Nolen [ 25/Dec/14 12:55 AM ]

cljs.closure/add-dependencies looks like the right place to do this. For this to work we would need to always sort files into dependency order before compilation. While this is done for cljs.closure/add-dependencies it is not done for files encountered in the source directory as a result of the implementation of cljs.compiler/compile-root.

If files were always guaranteed to be compiled in dependency order we could then easily pass the dependency information along and use this in cljs.compiler/requires-compilation? - a file would then additionally require compilation if any of the namespaces it depends on was recompiled.





[CLJS-913] Error when trying to convert javascript object to clojure (js->clj obj) under :advanced compilation Created: 18/Dec/14  Updated: 18/Dec/14

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

Type: Defect Priority: Major
Reporter: Erik Wickstrom Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

ClojureScript 0.0-2496

Tested on OSX 10.10 with Chrome 39



 Description   

I'm trying to convert a javascript object (parsed JSON from a web service) into a clojure data structure. It works fine if I use :simple optimization with the closure compiler, however when I switch to :advanced optimization, I get the following error:

(let my-data #js {} ; see below for JSON
(.info js/console "converted to clojure" (str (js->clj my-data))))

Uncaught Error: No protocol method IEmptyableCollection.-empty defined for type object: [object Object]

Note that this also only seems to happen with this chunk of JSON and a few other samples (though according to all the JSON linters I've tried, it is valid). I've passed other input through without issue. So it is somewhat intermittent but deepish object hierarchies seem to be a commonality.

Here is the JSON (also posted here http://pastebin.com/PLffFrFf )

[{"address_components":[{"long_name":"11810","short_name":"11810","types":["postal_code"]},{"long_name":"16 de Septiembre","short_name":"16 de Septiembre","types":["neighborhood","political"]},{"long_name":"Miguel Hidalgo","short_name":"Miguel Hidalgo","types":["sublocality_level_1","sublocality","political"]},{"long_name":"Ciudad de México","short_name":"México D.F.","types":["locality","political"]},{"long_name":"Distrito Federal","short_name":"D.F.","types":["administrative_area_level_1","political"]},{"long_name":"Mexico","short_name":"MX","types":["country","political"]}],"formatted_address":"16 de Septiembre, Miguel Hidalgo, 11810 Ciudad de México, D.F., Mexico","geometry":{"bounds":{"Ea":{"j":19.4043293,"k":19.3997335},"wa":{"j":-99.21262619999999,"k":-99.2045263}},"location":{"D":-99.20755880000002,"k":19.402037},"location_type":"APPROXIMATE","viewport":{"Ea":{"j":19.4043293,"k":19.3997335},"wa":{"j":-99.21262619999999,"k":-99.2045263}}},"types":["postal_code"]},{"address_components":[{"long_name":"11810","short_name":"11810","types":["postal_code"]},{"long_name":"West Jakarta","short_name":"West Jakarta","types":["locality","political"]},{"long_name":"Kamal","short_name":"Kamal","types":["administrative_area_level_4","political"]},{"long_name":"Kalideres","short_name":"Kalideres","types":["administrative_area_level_3","political"]},{"long_name":"West Jakarta City","short_name":"West Jakarta City","types":["administrative_area_level_2","political"]},{"long_name":"Jakarta","short_name":"Jakarta","types":["administrative_area_level_1","political"]},{"long_name":"Indonesia","short_name":"ID","types":["country","political"]}],"formatted_address":"Kamal, Kalideres, West Jakarta City, Jakarta 11810, Indonesia","geometry":{"bounds":{"Ea":{"j":-6.095065399999999,"k":-6.110835},"wa":{"j":106.68747699999994,"k":106.71448510000005}},"location":{"D":106.70282500000008,"k":-6.101219},"location_type":"APPROXIMATE","viewport":{"Ea":{"j":-6.095065399999999,"k":-6.110835},"wa":{"j":106.68747699999994,"k":106.71448510000005}}},"types":["postal_code"]},{"address_components":[{"long_name":"11810","short_name":"11810","types":["route"]},{"long_name":"Příbram District","short_name":"Příbram District","types":["administrative_area_level_2","political"]},{"long_name":"Central Bohemian Region","short_name":"Central Bohemian Region","types":["administrative_area_level_1","political"]},{"long_name":"Czech Republic","short_name":"CZ","types":["country","political"]},{"long_name":"261 01","short_name":"261 01","types":["postal_code"]}],"formatted_address":"11810, 261 01, Czech Republic","geometry":{"bounds":{"Ea":{"j":49.7328257,"k":49.7102303},"wa":{"j":13.979755599999976,"k":13.986990699999978}},"location":{"D":13.982032200000049,"k":49.7225575},"location_type":"GEOMETRIC_CENTER","viewport":{"Ea":{"j":49.7328257,"k":49.7102303},"wa":{"j":13.979755599999976,"k":13.986990699999978}}},"types":["route"]}]

Here is the original post to the mailing list: https://groups.google.com/forum/#!topic/clojurescript/MZ9esXbn2tg






[CLJS-910] JavaScriptCore 0xbbadbeef EXC_BAD_ACCESS when evaluating (list 0 1 ... 18) Created: 16/Dec/14  Updated: 06/Jan/15

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

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

Embedded JavaScriptCore on iOS simulator
Connected via Weasel / simple-brepl
:whitespace optimization


Attachments: PNG File memory.png     Text File stacktrace.txt    

 Description   

If I evaluate

(list 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18)

JavaScriptCore exhibits a 0xbbadbeef EXC_BAD_ACCESS, with a fairly deep stacktrace:

(lldb) bt

  • thread #1: tid = 0x3f7e, 0x0111e583 JavaScriptCore`WTFCrash + 67, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0xbbadbeef)
    frame #0: 0x0111e583 JavaScriptCore`WTFCrash + 67
    frame #1: 0x011395a9 JavaScriptCore`WTF::fastMalloc(unsigned long) + 1929
    frame #2: 0x00c9cb56 JavaScriptCore`WTF::Vector<JSC::UnlinkedInstruction, 0ul, WTF::UnsafeVectorOverflow>::expandCapacity(unsigned long) + 86
    frame #3: 0x00c90f27 JavaScriptCore`JSC::BytecodeGenerator::emitGetById(JSC::RegisterID*, JSC::RegisterID*, JSC::Identifier const&) + 311
    frame #4: 0x00fd4617 JavaScriptCore`JSC::DotAccessorNode::emitBytecode(JSC::BytecodeGenerator&, JSC::RegisterID*) + 551
    ...

(Full stack trace attached as stacktrace.txt)

This only occurs with :whitespace optimization and does not under :advanced.

If you evaluate (list 0), it works, and so does (list 0 1), all the way up to 17. Interestingly, it gets progressively slower as you evaluate longer lists.



 Comments   
Comment by Mike Fikes [ 16/Dec/14 12:35 PM ]

While the EXC_BAD_ACCESS is arguably a bug in JavaScriptCore, it is likely provoked by excessive memory usage of the (list 0 1 ...) form. The attached memory.png shows what appears to be 2^n memory usage for evaluating a list of size n. This graph was produced while REPLd into an iOS device, monitoring memory use from Xcode.

Comment by Mike Fikes [ 18/Dec/14 11:28 AM ]

The construct

(list 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18)

works in r2014 (released Nov 6, 2013), but fails in r2024 (released Nov 8, 2013).

In r2014, the emitted JavaScript is:

cljs.core.list.call(null, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18)

while in r2024 the emitted JavaScript is:

cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null,
cljs.core.List.EMPTY, 18), 17), 16), 15), 14), 13), 12), 11), 10), 9), 8), 7), 6), 5), 4), 3), 2), 1), 0)

This particular commit, between r2014 and r2014 on Nov 7 2013, is likely when the emitted JavaScript changed (I haven't pulled down that specific revision):

https://github.com/clojure/clojurescript/commit/5bcbc4745f599e352c51e01b210755a88aa4bc5f#diff-b64165608bed8fb21a132890b4e2fca2R1279

Comment by Mike Fikes [ 18/Dec/14 12:07 PM ]

Knowing this, it is trivial to reproduce this in desktop Safari (and also see that it works fine in Chrome and Firefox).

If you go to clojurescript.net, or himera.herokuapp.com, and define a function that returns a list constructed with (list 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18), you can see that those websites are built using ClojureScript r2014 or earlier, as cljs.core.list.call(null, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18) appears in the reader literal for the returned function.

But, with either of those sites, if you evaluate the following, you will cause a severe performance problem in Safari:

(conj (conj (conj (conj (conj (conj (conj (conj (conj (conj (conj (conj (conj (conj (conj (conj (conj (conj (conj (conj (conj (conj '() 21) 20) 19) 18) 17) 16) 15) 14) 13) 12) 11) 10) 9) 8) 7) 6) 5) 4) 3) 2) 1) 0)

My hope is that knowing this will make it easier to profile (using desktop tools) what is giving Safari grief executing the resulting JavaScript.

Comment by Mike Fikes [ 18/Dec/14 2:38 PM ]

I don't understand JavaScriptCore's evaluation strategy. I found that if you manually revise the deeply nested composition of cljs.core._conj.call(...) invocations to extract a temporary var or two, as below, then the "doubling" effect is cut short, and the code executes quickly.

This revised code essentially builds a list of length 13 first, and then conses on 3 more elements, and 3 more.

var list0 = cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null,cljs.core.List.EMPTY, 18), 17), 16), 15), 14), 13), 12), 11), 10), 9), 8), 7), 6);

var list1 = cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, list0, 5), 4), 3);

cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, list1, 2), 1), 0)

I found that I can cause something similar to occur from the ClojureScript side by adding a do form, as in:

(conj (conj (conj (conj (conj (do nil
(conj (conj (conj (conj (conj (conj (conj (conj (conj (conj (conj (conj (conj (conj (conj (conj (conj '() 21) 20) 19) 18) 17) 16) 15) 14) 13) 12) 11) 10) 9) 8) 7) 6) 5)) 4) 3) 2) 1) 0)

Comment by Mike Fikes [ 18/Dec/14 7:00 PM ]

This JavaScriptCore perf problem is easily reproduced purely with JavaScript:

inc=function( x ) { return x + 1; }

alert(
inc.call(null, inc.call(null, inc.call(null, inc.call(null,
inc.call(null, inc.call(null, inc.call(null, inc.call(null,
inc.call(null, inc.call(null, inc.call(null, inc.call(null,
inc.call(null, inc.call(null, inc.call(null, inc.call(null,
inc.call(null, inc.call(null, inc.call(null, inc.call(null,
inc.call(null, 1))))))))))))))))))))))

Try this, at, say jsfiddle.net and you will see it max a core and use several GB in Safari, but not FireFox or Chrome.

Comment by Mike Fikes [ 19/Dec/14 9:00 AM ]

As indicated in this ticket's description, this problem doesn't occur with :advanced mode optimizations. Just to confirm, I produced the JavaScript with :pseudo-names set to true for (list 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18), and you can see that it doesn't use the problematic "call" construct:

$cljs$core$_conj$$($cljs$core$_conj$$($cljs$core$_conj$$($cljs$core$_conj$$($cljs$core$_conj$$($cljs$core$_conj$$($cljs$core$_conj$$($cljs$core$_conj$$($cljs$core$_conj$$($cljs$core$_conj$$($cljs$core$_conj$$($cljs$core$_conj$$($cljs$core$_conj$$($cljs$core$_conj$$($cljs$core$_conj$$($cljs$core$_conj$$($cljs$core$_conj$$($cljs$core$_conj$$($cljs$core$_conj$$($cljs$core$List$EMPTY$$, 18), 17), 16), 15), 14), 13), 12), 11), 10), 9), 8), 7), 6), 5), 4), 3), 2), 1), 0);

Correction 20-DEC-2014: Even with :advanced mode optimization, this occurs. The setting that is needed to avoid the "call" construct is {:static-fns true}, as was set for the above output. With {:static-fnc false}, the emitted coded under :advanced mode is:

$cljs$core$_conj$$.call(null, $cljs$core$_conj$$.call(null, $cljs$core$_conj$$.call(null, $cljs$core$_conj$$.call(null, $cljs$core$_conj$$.call(null, $cljs$core$_conj$$.call(null, $cljs$core$_conj$$.call(null, $cljs$core$_conj$$.call(null, $cljs$core$_conj$$.call(null, $cljs$core$_conj$$.call(null, $cljs$core$_conj$$.call(null, $cljs$core$_conj$$.call(null, $cljs$core$_conj$$.call(null, $cljs$core$_conj$$.call(null, $cljs$core$_conj$$.call(null, $cljs$core$_conj$$.call(null, $cljs$core$_conj$$.call(null, $cljs$core$_conj$$.call(null, $cljs$core$List$EMPTY$$, 18), 17), 17), 15), 14), 13), 12), 11), 10), 9), 8), 7), 6), 5), 4), 3), 2), 1);
Comment by Mike Fikes [ 19/Dec/14 9:09 AM ]

Since the fundamental problem is easily reproducible with pure JavaScript, I've opened a StackOverflow regarding it: http://stackoverflow.com/questions/27568249/javascriptcore-deeply-composed-call-performance

Comment by David Nolen [ 19/Dec/14 12:08 PM ]

That was the first thing I was going to suggest trying, repo in plain JavaScript. Thanks for doing this - I believe this may explain some oddities we've encountered elsewhere.

Comment by Mike Fikes [ 19/Dec/14 1:41 PM ]

I’ve filed rdar://19310764 with Apple, and copied it here for reference: http://openradar.appspot.com/radar?id=5864025753649152

Comment by Francis Avila [ 19/Dec/14 5:38 PM ]

Excellent debugging work. Shouldn't this be filed against webkit instead? http://www.webkit.org/quality/reporting.html. (I already searched for this issue in their bug tracker and found nothing.)

This still leaves open the question of whether CLJS should do anything to mitigate. Many of the cljs macros that shadow functions expand recursively--maybe they should expand to loops or reduces instead?

For example:

(defmacro list
  ([] '(.-EMPTY cljs.core/List))
  ([& xs]
    `(let [a# (array ~@(reverse xs))]
       (areduce a# i# l# (list)
         (. l# cljs$core$ICollection$_conj$arity$2 (aget a# i#))))))

Or maybe cheat and emit a ChunkedCons instead?

(defmacro cclist
  ([] '(.-EMPTY cljs.core/LIST))
  ([& xs]
    `(cljs.core/ChunkedCons.
       (cljs.core/ArrayChunk.
         (array ~@(reverse xs)) 0 ~(count xs))
       nil nil nil)))
Comment by Mike Fikes [ 19/Dec/14 6:54 PM ]

Thanks Francis. I've confirmed that it occurs in the WebKit Nightly r177573, and I've moved the ticket to https://bugs.webkit.org/show_bug.cgi?id=139847

Comment by Thomas Heller [ 20/Dec/14 4:19 AM ]

FWIW to toggle between fn.call(null, ...) and fn(...) you can use the compiler option {:static-fns true}. This works with all optimization levels (also :none) but I'm told it causes issues with the REPL. But if you don't use a REPL it is safe to use that option always, I have been for years. Maybe the best short term option.

Comment by Mike Fikes [ 20/Dec/14 8:43 AM ]

Given Thomas's comment, I now realize that my comments above regarding :whitespace and :advanced are incorrect. (My project.clj has a dev and release build, and in addition to a change in the optimization directive, I had a change in the :static-fns directive.)

The correction: This problem occurs for both :whitespace and :advanced, iff :static-fns is set to false.

Comment by David Nolen [ 20/Dec/14 3:22 PM ]

One thing I'm curious about is whether the issue manifests if you're not nesting calls to the same function? This should be easy to confirm with plain JavaScript.

Comment by Mike Fikes [ 20/Dec/14 4:26 PM ]

Hey David, it appears to still cause a problem even with distinct functions. (This fiddle exhibits the problem with distinct functions, so don't follow the link on mobile Safari: http://jsfiddle.net/mfikes/Lwr78cmk/1/ )

Comment by Mike Fikes [ 21/Dec/14 4:16 PM ]

Activity is occurring with the WebKit ticket:

1) It has been confirmed as reproducible
2) It appears to be imported into Apple's system (InRadar keyword added).

Additionally, I succeeded in reproducing the issue on an older PowerPC Mac OS X 10.4.11 Safari 4.1.3 (from 2010), so it is not a recent regression, FWIW.

Comment by Mike Fikes [ 03/Jan/15 7:43 PM ]

Note: CLJS-945 sets :static-fns true as the default.

Comment by David Nolen [ 03/Jan/15 7:49 PM ]

Mike this is true only for cljs.core.

Comment by Mike Fikes [ 06/Jan/15 1:42 PM ]

FWIW, the original ticket I filed with Apple (rdar://19310764) has subsequently been closed by Apple as a duplicate of rdar://19321122, which is the Apple ticket the WebKit team opened in association with https://bugs.webkit.org/show_bug.cgi?id=139847





[CLJS-904] timeout in start-evaluator is too short Created: 10/Dec/14  Updated: 10/Dec/14

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

Type: Defect Priority: Major
Reporter: John Chijioke Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

The timeout specified in cljs.browser.repl/start-evaluator is too short.
https://github.com/clojure/clojurescript/blob/master/src/cljs/clojure/browser/repl.cljs#L87

Usually I have to update it from 50 to 500 to get my repl working any time I update to a new version. Would be nice to have this factored in as I don't suspect it has any noticeable effect on any other functionality.






[CLJS-901] In memory based builds Created: 03/Dec/14  Updated: 08/Dec/14

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

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


 Description   

Currently builds are based one files on disk. It is desirable to be able to pass arbitrary ClojureScript programs as string or seq of forms and get the fully compiled source including dependencies as a string result.



 Comments   
Comment by Thomas Heller [ 03/Dec/14 9:31 AM ]

This and some other issues opened recently (CLJS-900, CLJS-851, CLJS-899, ...) have some overlap with what I have done in shadow-build [1]. Memory based builds are actually already possible cause it will only touch the disk when asked to, although the API could use some cleanup.

Anyways, might be worthwhile to coordinate these efforts to make CLJS more accessible for everyone.

[1] https://github.com/thheller/shadow-build





[CLJS-891] Defs in "parent" namespaces clash with "child" namespaces with the same name? Created: 28/Nov/14  Updated: 02/Dec/14

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

Type: Defect Priority: Major
Reporter: Russell Dunphy Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: bug, namespace
Environment:

Clojurescript 0.0-2371, OSX



 Description   

This has had me totally flummoxed for a few hours, but I have finally been able to create a minimal project replicating the problem based on the mies template: https://github.com/rsslldnphy/cljs-ns-issue.

The problem seems to happen when a "parent" namespace, for example `my-project.foo`, contains a def with the same name as a "child" namespace, eg if there is a namespace `my-project.foo.bar`, and a def named `bar` in `my-project.foo`, and both those namespaces are required by `my-project.core`, then calling functions in `my-project.foo.bar` ends up with an Uncaught TypeError: Cannot read property 'call' of undefined. Sometimes, depending on which ns requires which, I've also seen an Uncaught Error: Namespace "cljs_ns_issue.foo.bar" already declared.

I don't think I'm doing a particularly good job of explaining this so it might be easier to look at the code. The crux is: comment out this line and the code works, leave it in and you get an error.



 Comments   
Comment by Francis Avila [ 28/Nov/14 2:01 PM ]

Clojurescript implements namespaces with Google Closure compiler's require/provide system. Unfortunately that system does not have a hard distinction between names and namespaces like Clojure does but instead is more like a sloppy java classname. The crux of it is that vars and namespaces occupy the same tree of js objects and thus their names may not overlap.

Compare the cljs on the left with the emitted javascript (This isn't exactly what is happening, but only in essence):

(ns my-project.foo ; goog.provide('my_project.foo') // something like window.my_project = {foo: {}}
  (:require my-project.foo.bar)) //goog.require('my_project.foo.bar')

;; the "require" evaluates the other namespace, which sets
;; // window.my_project.foo = {bar:{}};
;; // my_project.foo.bar.baz = "some var in the bar ns";
;; Now window.my_project.foo.bar = {baz: "some var in the bar ns"};

(defn bar [] 1)         ; my_project.foo.bar = (function bar(){return 1;});
;; Now the js object that was the bar namespace is gone, replaced with this function.

(my-project.foo.bar/baz)
; my_project.foo.bar.baz.call() // Uncaught TypeError: Cannot read property 'call' of undefined.

;; Alternatively, if (ns my-project.foo.bar) got evaluated *after* my-project.foo namespace was
;; evaluated, then my-project.foo/bar is defined, and the emitted goog.provide('my-project.foo.bar')
;; throws "Uncaught Error: Namespace "my_project.foo.bar" already declared".

So basically this is a leaky abstraction. In Clojurescript, you cannot define a var whose ns-qualified name matches that of another namespace: the slash between name and namespace is not real.

I think the only possible things that can be done are either:

  • Warn at compile-time if a var and a namespace object-path clash. Obviously there may still be runtime-created vars.
  • Put namespace vars behind some magic name. E.g. (ns foo.bar)(def baz 1) becomes goog.provide('foo.bar.__NS__'); foo.bar.__NS__.baz = 1; This would significantly uglify cljs names exported to js (and break existing code), and the magic name could never be used as a var name.
Comment by David Nolen [ 02/Dec/14 6:07 AM ]

We actually already have some logic for this in place, we track namespace segments for this reason. This is a different manifestation of it than previously encountered. I think any further workarounds are likely more trouble than they are worth (debugging complications) - probably the best thing to do is report a warning if we detect a clash.





[CLJS-876] merged sourcemap doesn't account for output-wrapper Created: 25/Oct/14  Updated: 05/Nov/14

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

Type: Defect Priority: Major
Reporter: J. David Lowe Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File 876.alternative.patch     Text File 876.patch    

 Description   

When merging sourcemaps, the cljs.closure/optimize code correctly accounts for "preamble" lines that get prepended to closure compiler output. However, it fails to account for the extra lines added when :output-wrapper is true. As a result, the merged sourcemap is incorrect whenever :output-wrapper is true. (And for extra action-at-a-distance: :output-wrapper is implicitly set to true by lein-cljsbuild whenever :optimizations is :advanced.)

Too tired to make a patch tonight, but I'll go through the CA process and send a patch in a day or two, if nobody gets to it before then.



 Comments   
Comment by J. David Lowe [ 27/Oct/14 2:42 PM ]

Here are two patches, which both fix the problem.

876.patch is tiny, but I think it's probably a step in the wrong direction, because all future output prefixes will need the same fix applied to them.

876.alternative.patch is a deeper change, but I think is a better solution: it makes it much more apparent that all output prefixes should be added via make-preamble. The downside (IMO) is that the output-wrapper prefix and suffix are now separated a bit in the code.

Comment by David Nolen [ 05/Nov/14 6:46 AM ]

This patch approach looks OK but something more minimal is preferred to simplify patch reviewing. Please submit a patch which makes the most minimal set of code changes (it's preferred not to move code) use declare if necessary. Thanks!





[CLJS-871] .-default property access returns nil Created: 11/Oct/14  Updated: 14/Oct/14

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

Type: Defect Priority: Major
Reporter: Joel Holdbrooks Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None

Attachments: Text File 871.patch    
Patch: Code and Test

 Description   

Types defined with deftype/defrecord which have a default field will incorrectly return nil with property access. The following example will return nil.

(deftype Foo [default])

(let [foo (Foo. "bar")]
  (.-default foo))


 Comments   
Comment by Joel Holdbrooks [ 13/Oct/14 4:19 PM ]

Patch attached. I should point out that I had to borrow js-reserved from the compiler namespace and the warning message provided hard codes the munged symbol information instead of reusing the compiler's munge fn.

Comment by Joel Holdbrooks [ 13/Oct/14 9:41 PM ]

For the sake of history, I should provide more context to this patch (I'm unable to edit the issue title for some reason). It isn't just .-default it is any field name that is also a JavaScript identifier (eg. public, private, if).

Comment by David Nolen [ 14/Oct/14 5:26 AM ]

Please lift js-reserved and any helpers like munge into the shared namespace cljs.util so that logic an be shared and hard coding avoided. Thanks.

Comment by Joel Holdbrooks [ 14/Oct/14 5:03 PM ]

Are you sure, David? That might make this patch a bit more noisy. If it's not a problem I'm happy to do it.

Comment by David Nolen [ 14/Oct/14 6:06 PM ]

I'm sure, I'd like to avoid this kind of code duping. Cleaner in the end and better moving forward.





[CLJS-868] no arity warnings on recursive calls Created: 03/Oct/14  Updated: 03/Oct/14

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

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


 Description   

If a function recursively invokes itself within its own body the invoke will not be checked for arity mismatch.






[CLJS-867] specify with Object methods requires multi-arity style definition Created: 02/Oct/14  Updated: 02/Oct/14

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

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


 Description   
Object
(foo [] ...)

Throws an `nth not supported on this type: Symbol` exception.

Object
(foo ([] ...))

Is required even though this is not true for protocol implementations.






[CLJS-865] js-dependency-index won't work when running from an uberjar Created: 26/Sep/14  Updated: 03/Dec/14

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

Type: Defect Priority: Major
Reporter: Dan Johansson Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: None
Environment:

Window7
[org.clojure/clojure "1.6.0"]
[org.clojure/clojurescript "0.0-2322"]
java 1.7



 Description   

I'm having problems running an uberjar with [lein-light-nrepl "0.0.18"].
I've tracked the problem down to the filter in cljs.js-deps/goog-resource.
It expects to find goog/deps.js in the google jar file but it is in the uberjar file.



 Comments   
Comment by Dan Johansson [ 29/Sep/14 2:27 AM ]

I use this workaround for now:

(defn load-nrepl []
  (require 'cljs.js-deps)
  
  (alter-var-root (find-var 'cljs.js-deps/goog-resource)
                  (fn [of]
                    (fn [path]
                      (first
                       (enumeration-seq (.getResources (.getContextClassLoader (Thread/currentThread)) path))))))
  
  (require 'lighttable.nrepl.handler))

It just removes the filter.
I guess the filter is there to make sure you get the correct goog/deps.js file? I don't know what assumptions to make about build environments to suggest a generic solution.

Comment by Dan Johansson [ 29/Sep/14 2:28 AM ]

This is the error I get:

Caused by: java.lang.IllegalArgumentException: No implementation of method: :make-reader of protocol: #'clojure.java.io/IOFactory found for class: nil
	at clojure.core$_cache_protocol_fn.invoke(core_deftype.clj:544)
	at clojure.java.io$fn__8628$G__8610__8635.invoke(io.clj:69)
	at clojure.java.io$reader.doInvoke(io.clj:102)
	at clojure.lang.RestFn.invoke(RestFn.java:410)
	at cljs.js_deps$goog_dependencies_STAR_.invoke(js_deps.clj:241)
	at clojure.lang.AFn.applyToHelper(AFn.java:152)
	at clojure.lang.AFn.applyTo(AFn.java:144)
	at clojure.core$apply.invoke(core.clj:624)
	at clojure.core$memoize$fn__5097.doInvoke(core.clj:5846)
	at clojure.lang.RestFn.invoke(RestFn.java:397)
	at cljs.js_deps$js_dependency_index.invoke(js_deps.clj:265)
	at cljs.env$default_compiler_env.invoke(env.clj:45)
	at cljs.env$default_compiler_env.invoke(env.clj:42)
	at lighttable.nrepl.cljs__init.load(Unknown Source)
	at lighttable.nrepl.cljs__init.<clinit>(Unknown Source)
	... 52 more
Comment by James Cash [ 29/Sep/14 3:58 PM ]

I was able to work around this same issue by explicitly including the google-closure-library jar in the classpath (i.e. instead of `java -jar my-uber.jar`, `java -cp google-closure-library-$VERSION.jar:my-uber.jar my-uber.core`)

Comment by Taha Siddiqi [ 08/Oct/14 10:19 PM ]

This is what worked for me.

(defn goog-resource [original-fn path]
  (if-let [resource (io/resource path)]
    resource
    (original-fn path)))

(alter-var-root #'cljs.js-deps/goog-resource (fn [f] (partial goog-resource f)))
Comment by David Nolen [ 09/Oct/14 3:35 PM ]

I'd be happy to remove the hack for Google Closure if someone can demonstrate it's no longer needed. If I recall we now remove the empty files from the thirdy party JAR so this shouldn't be a problem anymore.

Comment by Joseph Moore [ 03/Dec/14 11:43 PM ]

Adding the google-closure library jar to my war file in immutant (from lein immutant war) fixed lighttable's nrepl under wildfly as well, thank you James





[CLJS-860] redundant analysis of source files in JARs Created: 18/Sep/14  Updated: 18/Sep/14

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

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


 Description   

Source files in JARs are analyzed once then analyzed again when copied to the output directory.






[CLJS-848] Support Google Closure modules Created: 31/Aug/14  Updated: 19/Jan/15

Status: In Progress
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Stephan Behnke Assignee: David Nolen
Resolution: Unresolved Votes: 2
Labels: None


 Description   

The Google Closure Compiler is able to emit multiple files after compilation, instead of just one, by using the '--modules' flag (see example here: http://stackoverflow.com/a/7827251/213730). Ergo, I would love to see the ability to customize the ClojureScript output that way!

My use case would primarily be to separate the library code (core, core.async etc.) from application code. The reasoning is that the libraries rarely change, but the app code constantly does. Also, when my app gets bigger, I could separate areas of my application into according files; libs.js, app.core.js, app.public.js, app.admin.js etc.

Note: I was about to create an issue on the Leiningen plugin but found there is one already (https://github.com/emezeske/lein-cljsbuild/issues/148). In it the plugin author says this is a compiler issue. But I wasn't able to find an issue on this tracker. So I created this one.

Updated proposed syntax (subject to change):

{:optimizations :advanced
 :output-dir "./target/client/production/"
 :cache-analysis true
 :output-modules {
  {:id :common
   :out "./resources/assets/js/common.js"  
   :entries '#{com.foo.common}}
  {:id :landing
   :out "./resources/assets/js/landing.js" 
   :entries '#{com.foo.landing}
   :deps #{:common}
  {:id :editor
   :out "./resources/assets/js/editor.js"
   :entries '#{com.foo.editor}
   :deps #{:common}}}}}


 Comments   
Comment by Thomas Heller [ 01/Sep/14 3:32 AM ]

I build https://github.com/thheller/shadow-build to get the module support.

Docs are a bit lacking at the moment but it should be pretty straightforward to use, happy to help if you have questions.

Comment by Stephan Behnke [ 01/Sep/14 5:31 AM ]

Is 'shadow-build' just overriding the final stage of the compiler and the rest stays up-to-date - or is it a fork?
Is there a reason you didn't try to get support for it in the main compiler (yet)?

Either way, it sounds very interesting! I'll dive into shadow-build next weekend, hopefully

Comment by David Nolen [ 01/Sep/14 3:49 PM ]

I'm actually now think it's a good idea to support this to allow breaking up large ClojureScript libraries that will be consumed by others as a plain JS dependency. I'd be happy to see GClosure modules support land in ClojureScript if someone is willing to start a design page with basic plan of the implementation strategy.

Comment by Thomas Heller [ 02/Sep/14 2:43 AM ]

@Stephan: shadow-build is not a fork, it is a replacement to the cljs.closure namespace (specifically cljs.closure/build) since a single build function is not flexible enough to do what I needed. The compiler/analyzer is untouched and is as up-to-date as you want (you provide which version of cljs you want to use, up to HEAD). The main reason I didn't try to get it into CLJS core itself is time, at the time I wrote it I needed to "get it done". Since then it just has been working so there was no need to have it in core really. I know of a couple other people using it, so it is working for others too.

@David: I'd be happy to maybe use shadow-build as a sort of reference implementation for the whole thing. I'm quite happy with it, some API/naming cleanup aside. But we should go through a proper design process first, since not all choices I made may be ideal for everyone. That being said: GClosure modules are ONLY meant to separate ONE build into multiple files. They are not what javascript commonly calls modules (since we have those basically through namespaces). There are some caveats when trying to share a build with the JS world. I will try to write up why I did what I did in shadow-build and the features it provides. I think all browser targeted builds will want those features when the project reaches a given size.

Comment by David Nolen [ 02/Sep/14 7:43 AM ]

@Thomas thanks. I've created an empty design page here: http://dev.clojure.org/display/design/Google+Closure+Modules

Comment by Thomas Heller [ 02/Sep/14 10:25 AM ]

I didn't know where to start so I did a sort of brain dump of the whole thing. Hope it is enough to get things started, happy to go into more detail.

Comment by Thomas Heller [ 01/Jan/15 4:33 PM ]

It should be noted that Closure Modules aren't actually about splitting files to seperate rarely changing code from frequently changing code since files from one :advanced build cycle cannot be shared with files from another.

It is still desireable to split files for other reasons but this is a common misunderstanding people keep running into with shadow-build.

Comment by David Nolen [ 02/Jan/15 11:21 AM ]

Thomas, this is understood. It's just about supporting a useful lfeature that Closure has that's missing from ClojureScript.

Comment by Thomas Heller [ 02/Jan/15 11:47 AM ]

Agreed.

I just forgot there was an open issue for this and re-reading the description I thought it might be confusing as to why one would want modules. My experience with shadow-build has shown that it is not obvious that files from one optimize cycle cannot the shared with files from another one. Doing so leads to strange errors, I just thought I should clarify that.

Comment by pengke [ 19/Jan/15 3:48 AM ]

hunger for this fix.
it's very painful for download one large js file at once

Comment by David Nolen [ 19/Jan/15 7:12 AM ]

Please do not comment your support for tickets it just generates more noise to sift through. Use the vote functionality.





[CLJS-836] Replace seq-based iterators with direct iterators for all non-seq collections that use SeqIterator Created: 08/Aug/14  Updated: 13/Dec/14

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

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

Attachments: Text File cljs_836_v1.patch    
Patch: Code and Test

 Description   

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



 Comments   
Comment by Peter Schuck [ 12/Dec/14 4:56 PM ]

This is a port of the patches / diffs at http://dev.clojure.org/jira/browse/CLJ-1499. I'm getting around a 2X perf improvement when running the benchmark quite.

Comment by David Nolen [ 13/Dec/14 8:43 AM ]

Thanks, looks great! Will check with Alex Miller about the status of CLJ-1499 and see if we can apply this one.





[CLJS-818] Externs don't get loaded when running under immutant as cljs.js-deps/find-js-classpath fails Created: 18/Jun/14  Updated: 01/Jul/14

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

Type: Defect Priority: Major
Reporter: James Cash Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Java 1.8.0_05, Clojure 1.6.0, clojurescript 0.0-2234, immutant 1.1.1


Attachments: Text File 818.patch     PNG File Screen Shot 2014-06-18 at 20.14.59 .PNG    

 Description   

When compiling clojurescript that relies on library-provided externs (e.g. Om needing React.js externs), the clojurescript is compiled without errors, but the generated javascript fails to work, due to the externs not being loaded. Externs don't get loaded, as cljs.js-deps/find-js-classpath doesn't find the javascript externs file. This occurs because it uses cljs.js-deps/all-classpath-urls, which filters out the immutant classloader, since org.immutant.core.ImmutantClassLoader is not an instance of java.net.URLClassLoader (and hence lacks a .getURLs method anyway).



 Comments   
Comment by Toby Crawley [ 19/Jun/14 9:23 AM ]

Chas: Is there a reason not to depend on dynapath here? This exact case is kinda why it exists

Comment by David Nolen [ 19/Jun/14 10:47 AM ]

Patch welcome for this.

Comment by James Cash [ 19/Jun/14 2:12 PM ]

Simply replacing cljs.js-deps/all-classpath-urls with dynapath.util/all-classpath-urls worked for me. I don't know if there are policies around adding dependencies to cljs, but the following patch is working for me. Would it be preferable to re-implement the functionality instead?

Comment by David Nolen [ 19/Jun/14 2:19 PM ]

We are not going to take on a dependency for this. The code should be copied over, thanks.

Comment by James Cash [ 19/Jun/14 3:46 PM ]

Due to the way dynapath works, I don't think a straightforward copying of the code will work, since it relies on a protocol. Backing up a step though, would it be reasonable for externs to be loaded via io/resource, in the same way that the :preamble is?

Comment by Toby Crawley [ 19/Jun/14 3:54 PM ]

Unfortunately, the code can't be copied over. Dynapath works by providing a protocol that providers/users of funky classloaders can implement, allowing libraries that use dynapath to access the dynamic features of those classloaders without having to care about the loader's concrete type. Dynapath itself provides implementations for j.n.URLClassLoader and c.l.DynamicClassloader by default, so libraries don't have to do anything special to access the dynamic features of those classes.

java.classpath also provides a similar mechanism that the Immutant classloader implements as well. If you are more open to dependencies that are under org.clojure, using that will work as well. Ideally, I'd like to see java.classpath subsume dynapath.

Comment by James Cash [ 19/Jun/14 4:23 PM ]

Made a new patch that sidesteps the all-classpath-urls issue by just using io/resource instead of iterating over all urls

Comment by David Nolen [ 01/Jul/14 9:26 PM ]

Can people chime in whether the patch works for them, thanks.





[CLJS-813] Warn about reserved JS keyword usage in namespace names Created: 11/Jun/14  Updated: 16/Jul/14

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

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


 Description   

If a namespace is identified as foo.long.core it will get munged into foo.long$.core. This is unexpected and a source of confusion when goog.require("foo.long.core") fails.



 Comments   
Comment by Max Veytsman [ 15/Jul/14 4:50 PM ]

I'm starting to take a look at this.

Would it be most appropriate to add this check into compiler.cljs where the munging actually happens, or into analyzer.cljs where most of the warnings of this type live?

Comment by Mike Fikes [ 15/Jul/14 5:34 PM ]

If a solution is identified that eliminates “overly aggressive” munging for certain cases, then CLJS-689 could benefit.

Comment by Max Veytsman [ 16/Jul/14 2:44 PM ]

Currently, when munging "foo.bar.baz", we map over ["foo", "bar", "baz"] and check if each is a reserved word (https://github.com/clojure/clojurescript/blob/master/src/clj/cljs/compiler.clj#L94-L95)

According to my understanding of https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Lexical_grammar#Reserved_word_usage and https://es5.github.io/#x7.6 reserved words are only disallowed in Identifiers. MemberExpressions and CallExpressions (the things with "."s in them) do not ban reserved words except in the first Identifier.

For our purposes, it could be enough to check if the entire token and the first period-seperated element is reserved. I.e. long becomes long$, long.night.ahead becomes long$.night.ahead, but foo.long.core remains foo.long.core.

Mike, this unfortunately won't affect CLJS-689

Does that sound like a good approach?





[CLJS-808] Warning from `find-classpath-lib` mistakenly included in generated source Created: 02/Jun/14  Updated: 02/Jun/14

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

Type: Defect Priority: Major
Reporter: Joshua Ballanco Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

clojurescript 0.0-r2227
target: node


Attachments: Text File warn-with-out-str-fix.patch    
Patch: Code

 Description   

When compiling for node.js, `find-classpath-lib` generates a warning because `cljs/nodejs.js` doesn't contain a `goog.provide` declaration. However, this warning ends up being emitted into the JS files being generated (caused by a call to `with-out-str`), later causing a critical failure in the Closure compiler.

To reproduce:

  • `lein new cljs-node buggy`
  • Update clojurescript to r2227, lein-cljsbuild to 1.0.3
  • `lein cljsbuild once`

Result:

  • Multiple warnings
  • Generated "buggy.js" file contains only node.js shebang

Fix:

  • The simplest patch (attached) is to rebind `out` to `err` when emitting a warning
  • A better fix would be to write a function for error printing that wraps this idiom
  • Ideally, cljs/nodejs.js should have a `goog.provides` line added to prevent the warning in the first place


 Comments   
Comment by David Nolen [ 02/Jun/14 7:45 AM ]

A patch that adds a goog.provide to the file is preferred, thanks!





[CLJS-806] support ^:const Created: 09/May/14  Updated: 17/Dec/14

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

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

Attachments: Text File cljs_806.patch    
Patch: Code and Test

 Description   

Currently def's do not support ^:const annotations, this is useful in conjunction with case.



 Comments   
Comment by Peter Schuck [ 17/Dec/14 4:53 PM ]

def's marked with ^:const are now treated as constants, they can't be redefined or set to a different value. Currently only case takes advantage of this. Additionally the emitted var is annotated as a constant for the closure compiler.





[CLJS-773] Use unchecked-*-int functions for real 32-bit math Created: 26/Feb/14  Updated: 02/Dec/14

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

Type: Enhancement Priority: Major
Reporter: Francis Avila Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: numerics
Environment:

r2173



 Description   

Currently the unchecked-* functions and macros simply alias the primitive js operators. It would be nice if the unchecked-*-int family of functions and macros implemented C/Java-like signed int operations with silent overflows (just like in Clojure) using asm.js coersion idioms. This should also allow us to share such code between clojure and clojurescript without worrying about their different numerics.

A use case is that porting hash algorithms from java to clojurescript is trickier and more verbose than it needs to be.



 Comments   
Comment by David Nolen [ 08/May/14 6:43 PM ]

This sounds interesting, would like to see more thoughts on approach, benchmarks etc.

Comment by David Nolen [ 02/Dec/14 5:46 AM ]

Bump, this enhancements sound simple & fine.

Comment by Francis Avila [ 02/Dec/14 1:26 PM ]

I'll have time to do this in about a week. The implementation is straightforward (basically use xor 0 everywhere). The goal is correctness, but I expect performance to be as good as or better than it is now on most platforms. I'm not sure if advanced mode will drop intermediate truncations or what impact this has on performance.

Some higher-level numeric analysis using the asm.js type system is possible but I doubt it's worth it.





[CLJS-719] this-as behaves incorrectly in "scoping function" Created: 07/Dec/13  Updated: 02/Dec/14

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

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


 Description   

When a this-as expression gets put in a "scoping function", e.g. in a let-binding, the value bound via this-as refers to the scoping function, and not to the outer scope.

Example:

(def foo
  (js-obj
    "bar" "baz"
    "getBarRight" (fn [] (this-as self (.-bar self)))
    "getBarWrong" (fn []
                    (let [bar (this-as self (.-bar self))]
                      bar))))
     
(.log js/console (.getBarRight foo)) ;; => "baz"
(.log js/console (.getBarWrong foo)) ;; => undefined

Whereas foo.getBarRight expands to something like

function() {
  var self = this; // this refers to foo
  return self.bar; // returns "bar"
}

foo.getBarWrong on the other hand expands to

function() {
  var bar = function() {
    var self = this; // this refers to enclosing function
    return self.bar; // returns undefined
  }();
  return bar; // returns undefined
}





[CLJS-713] optimized case Created: 04/Dec/13  Updated: 23/Jun/14

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

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

Attachments: Text File 0001-CLJS-713-Allow-test-expressions-for-case-to-be-chars.patch     Text File 0001-CLJS-713-first-cut-at-compiling-case-to-switch.patch    

 Description   

With the advent of asm.js many engines will like compile switch statements over integers into jump tables. We should provide a real `case*` ast node that compiles to JS `switch` when possible - i.e. numbers, strings, keywords etc.



 Comments   
Comment by Michał Marczyk [ 18/Feb/14 5:56 PM ]

First cut impl also available here:

https://github.com/michalmarczyk/clojurescript/tree/713-compile-case-to-switch

With this patch applied, case expressions are compiled to switch + some extra bits when all tests are numbers or strings, otherwise old logic is used.

For example, {{(fn [] (let [x 1] (case x 1 :foo (2 3) :bar :quux)))}} gets compiled to

function () {
    var x = 1;
    var G__6469 = x;
    var caseval__6470;
    switch (G__6469) {
      case 1:
        caseval__6470 = new cljs.core.Keyword(null, "foo", "foo", 1014005816);
        break;
      case 2:
      case 3:
        caseval__6470 = new cljs.core.Keyword(null, "bar", "bar", 1014001541);
        break;
      default:
        caseval__6470 = new cljs.core.Keyword(null, "quux", "quux", 1017386809);
    }
    return caseval__6470;
}

The existing test suite passes, but I suppose it wouldn't hurt to add some tests with case in all possible contexts.

Comment by Michał Marczyk [ 18/Feb/14 6:05 PM ]

As a next step, I was planning to arrange things so that numbers/strings are fished out from among the tests and compiled to switch always, with any leftover tests put in an old-style nested-ifs-based case under default:. Does this sound good?

It seems to me that to deal with symbols and keywords in a similar manner we'd have to do one of two things:

1. check symbol? and keyword? at the top, then compile separate switches (the one for keywords would extract the name from the given keyword and use strings in the switch);

2. use hashes for dispatch.

Which one sounds better? Or is there a third way?

Comment by Michał Marczyk [ 18/Feb/14 6:11 PM ]

Of course we'd need to compute hashes statically to go with 2. I'd kind of like it if it were impossible (randomized seed / universal hashing), but currently it isn't.

Comment by Francis Avila [ 19/Feb/14 12:22 AM ]

At least on v8, there are surprisingly few cases where a switch statement will be optimized to a jump table. Basically the type of the switched-over value must always (across calls) match the type of every case, and there must be fewer than 128 cases, and integer cases must be 31-bit ints (v8's smi type). So mixing string and number cases in the same switch guarantees the statement will never be compiled. In many cases an equivalent if-else will end up being significantly faster on v8 just because the optimizing jit recognizes them better. There's an oldish bug filed against v8 switch performance. Looking at the many jsperfs of switch statements, it doesn't seem that v8 has improved. Relevant jsperf

Firefox is much better at optimizing switch statements (maybe because of their asm.js/emscripten work) but I don't know what conditions trigger (de)optimization.

I suspect the best approach is probably going to be your option one: if-else dispatch on type if any case is not a number, and then a switch statement covering the values for each of the keyword/string/symbol types present (no nested switch statements, and outlining the nested switches might be necessary). Even with a good hash, to guarantee v8 optimizing-compilation you would need to truncate the hashes into an smi (signed-left-shift once?) inside the case*.

Comment by David Nolen [ 19/Feb/14 12:50 AM ]

There's no need for invention here. We should follow the strategy that Clojure adopts - compile time hash calculation.

Comment by Francis Avila [ 19/Feb/14 3:09 PM ]

The problem, as Michal alluded to, is that the hash functions in cljs's runtime environment are not available at compile-time (unlike in Clojure). This might be a good opportunity to clean up that situation or even use identical hash values across Clojure and Clojurescript (i.e. CLJS-754), but that's a much bigger project. Especially considering it will probably not bring much of a speedup over an if-else-if implementation except in very narrow circumstances.

Comment by David Nolen [ 19/Feb/14 4:38 PM ]

Francis Avila I would make no such assumptions about performance without benchmarks. One of the critical uses for case is over keywords. Keyword hashes are computed at compile time, so that's one function call and a jump on some JavaScript engines. This is particularly useful for the performance of records where you want to lookup a field via keyword before checking the extension map.

This ticket should probably wait for CLJS-754 before proceeding.

Comment by Francis Avila [ 22/Feb/14 4:44 AM ]

Record field lookup is a good narrow use case to test. I put together a jsperf to compare if-else (current) vs switch with string cases vs switch with int cases (i.e., hash-compares, assuming perfect hashing).

Comment by David Nolen [ 10/May/14 3:59 PM ]

I've merged the case* analyzer and emitter bits by hand into master.

Comment by David Nolen [ 10/May/14 4:42 PM ]

I'v merged the rest of the patch into master. If any further optimizations are done it will be around dispatching on hash code a la Clojure.

Comment by Francis Avila [ 11/May/14 12:53 AM ]

Your keyword-test optimization has a bug: https://github.com/clojure/clojurescript/commit/9872788b3caa86f639633ff14dc0db49f16d3e2a

Test case:

(let [x "a"] (case x :a 1 "a"))
;=> 1
;;; Should be "a"

Github comment suggests two possible fixes.

Comment by David Nolen [ 11/May/14 10:50 AM ]

Thanks fix in master.

Comment by Christoffer Sawicki [ 23/Jun/14 3:41 PM ]

case over "chars" is currently not being optimized to switch (in other words: (case c (\a) :a :other) uses if instead of switch).

Given that ClojureScript chars are just strings of length 1, could this perhaps simply be fixed by tweaking https://github.com/clojure/clojurescript/blob/master/src/clj/cljs/core.clj#L1187 ?

Comment by Christoffer Sawicki [ 23/Jun/14 4:11 PM ]

OK, I couldn't resist trying and it seems to be that easy. Would be great if somebody more knowledgeable could look at it and say if it has any side-effects. (See patch with name 0001-CLJS-713-Allow-test-expressions-for-case-to-be-chars.patch.)

Comment by David Nolen [ 23/Jun/14 4:15 PM ]

The patch looks good I would have applied it if I hadn't already gone and done it master myself just now

Comment by Christoffer Sawicki [ 23/Jun/14 4:22 PM ]

Hehe. Thanks! Don't forget to update the "case* tests must be numbers or strings" message on line 496 too.

Comment by David Nolen [ 23/Jun/14 4:48 PM ]

The existing docstring is inaccurate - case supports all compile time literals.





[CLJS-712] resolve-var for symbol with dot still wrong Created: 03/Dec/13  Updated: 03/Dec/13

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

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


 Description   

We need to recur on the first segment passing an new additional argument to resolve-var indicating that we should not try to resolve in the current namespace and instead warn.






[CLJS-710] port clojure.pprint Created: 02/Dec/13  Updated: 27/Oct/14

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

Type: Task Priority: Major
Reporter: David Nolen Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None


 Comments   
Comment by Shaun LeBron [ 15/Aug/14 1:29 PM ]

working on this here: https://github.com/shaunlebron/cljs-pprint

Comment by Shaun LeBron [ 05/Oct/14 12:40 PM ]

I'm ceasing development on the port.

fipp should be ported in place of pprint in cljs. pprint is slow and complex, whereas fipp is fast, simple, and very customizable.

Brandon Bloom is awaiting a go-ahead from the cljs community on whether the fipp port should be completed:
https://github.com/brandonbloom/fipp/issues/7

Comment by David Nolen [ 05/Oct/14 12:55 PM ]

fipp only covers a very small (though important part) of clojure.pprint's functionality. I think it's fine that fipp is a standalone library that user's can adopt in their own projects if they desire. This doesn't change the desire for a full port of clojure.pprint.

Comment by Shaun LeBron [ 27/Oct/14 1:13 PM ]

k, resuming.





[CLJS-705] locals clearing Created: 27/Nov/13  Updated: 29/Nov/13

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

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


 Description   

Without locals clearing ClojureScript is likely to suffer from the same cases as Clojure did for common uses of lazy sequences.



 Comments   
Comment by David Nolen [ 29/Nov/13 3:03 PM ]

For this we'll need to introduce some special private way to set a local to nil, i.e. (_clear_local sym)





[CLJS-689] js/-Infinity munges to _Infinity Created: 20/Nov/13  Updated: 02/Dec/14

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

Type: Defect Priority: Major
Reporter: Chas Emerick Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

A dumb special-casing of js/ munging should fix.






[CLJS-667] validate extend-type and extend-protocol shape Created: 07/Nov/13  Updated: 02/Dec/14

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

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


 Description   

deftype and defrecord are sugar over the extend-type grouping, this is a source of confusion we should signal an error/warning if extend-type / extend-protocol malformed.






[CLJS-666] :require-macros should throw a sensible error if no macro file exists Created: 06/Nov/13  Updated: 25/Apr/14

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

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


 Comments   
Comment by Jonas Enlund [ 25/Apr/14 1:29 PM ]

What would you consider to be a sensible error? The resulting stacktrace after a failed (:require [cljs.reader :include-macros true]) looks like this:

Exception in thread "main" clojure.lang.ExceptionInfo: failed compiling file:test/cljs/test_runner.cljs {:file #<File test/cljs/test_runner.cljs>}
at clojure.core$ex_info.invoke(core.clj:4327)
at cljs.compiler$compile_file.invoke(compiler.clj:1006)
at cljs.compiler$compile_root.invoke(compiler.clj:1059)
at cljs.closure$compile_dir.invoke(closure.clj:337)
at cljs.closure$eval2820$fn__2821.invoke(closure.clj:377)
at cljs.closure$eval2757$fn_2758$G2748_2765.invoke(closure.clj:288)
at cljs.closure$eval2807$fn__2808.invoke(closure.clj:391)
at cljs.closure$eval2757$fn_2758$G2748_2765.invoke(closure.clj:288)
at cljs.closure$build.invoke(closure.clj:940)
at cljs.closure$build.invoke(closure.clj:909)
at user$eval2998.invoke(cljsc.clj:21)
at clojure.lang.Compiler.eval(Compiler.java:6619)
at clojure.lang.Compiler.load(Compiler.java:7064)
at clojure.lang.Compiler.loadFile(Compiler.java:7020)
at clojure.main$load_script.invoke(main.clj:294)
at clojure.main$script_opt.invoke(main.clj:356)
at clojure.main$main.doInvoke(main.clj:440)
at clojure.lang.RestFn.invoke(RestFn.java:436)
at clojure.lang.Var.invoke(Var.java:423)
at clojure.lang.AFn.applyToHelper(AFn.java:167)
at clojure.lang.Var.applyTo(Var.java:532)
at clojure.main.main(main.java:37)
Caused by: clojure.lang.ExceptionInfo: Could not locate cljs/reader__init.class or cljs/reader.clj on classpath: at line 1 /home/jonas/dev/clojure/clojurescript/test/cljs/foo/ns_shadow_test.cljs {:tag :cljs/analysis-error, :file "/home/jonas/dev/clojure/clojurescript/test/cljs/foo/ns_shadow_test.cljs", :line 1, :column 1}
at clojure.core$ex_info.invoke(core.clj:4327)
at cljs.analyzer$error.invoke(analyzer.clj:267)
at cljs.analyzer$analyze_seq.invoke(analyzer.clj:1415)
at cljs.analyzer$analyze$fn__1517.invoke(analyzer.clj:1506)
at cljs.analyzer$analyze.invoke(analyzer.clj:1499)
at cljs.analyzer$analyze.invoke(analyzer.clj:1494)
at cljs.analyzer$analyze_file$fn__1534.invoke(analyzer.clj:1575)
at cljs.analyzer$analyze_file.invoke(analyzer.clj:1569)
at cljs.analyzer$analyze_deps.invoke(analyzer.clj:963)
at cljs.analyzer$eval1306$fn__1308.invoke(analyzer.clj:1131)
at clojure.lang.MultiFn.invoke(MultiFn.java:241)
at cljs.analyzer$analyze_seq.invoke(analyzer.clj:1417)
at cljs.analyzer$analyze$fn__1517.invoke(analyzer.clj:1506)
at cljs.analyzer$analyze.invoke(analyzer.clj:1499)
at cljs.analyzer$analyze.invoke(analyzer.clj:1494)
at cljs.compiler$compile_file_STAR_.invoke(compiler.clj:885)
at cljs.compiler$compile_file.invoke(compiler.clj:999)
... 20 more
Caused by: java.io.FileNotFoundException: Could not locate cljs/reader__init.class or cljs/reader.clj on classpath:
at clojure.lang.RT.load(RT.java:443)
at clojure.lang.RT.load(RT.java:411)
at clojure.core$load$fn__5018.invoke(core.clj:5530)
at clojure.core$load.doInvoke(core.clj:5529)
at clojure.lang.RestFn.invoke(RestFn.java:408)
at clojure.core$load_one.invoke(core.clj:5336)
at clojure.core$load_lib$fn__4967.invoke(core.clj:5375)
at clojure.core$load_lib.doInvoke(core.clj:5374)
at clojure.lang.RestFn.applyTo(RestFn.java:142)
at clojure.core$apply.invoke(core.clj:619)
at clojure.core$load_libs.doInvoke(core.clj:5413)
at clojure.lang.RestFn.applyTo(RestFn.java:137)
at clojure.core$apply.invoke(core.clj:619)
at clojure.core$require.doInvoke(core.clj:5496)
at clojure.lang.RestFn.invoke(RestFn.java:408)
at cljs.analyzer$eval1306$fn__1308.invoke(analyzer.clj:1137)
at clojure.lang.MultiFn.invoke(MultiFn.java:241)
at cljs.analyzer$analyze_seq.invoke(analyzer.clj:1417)
... 34 more





[CLJS-575] cljsc.bat emit FileNotFoundException when compile samples in windows Created: 25/Aug/13  Updated: 19/Jun/14

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

Type: Defect Priority: Major
Reporter: Park Sang Kyu Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: Compiler, bug, patch
Environment:

in windows 7


Attachments: File cljsc.bat.diff     File cljsc-path.bat    
Patch: Code

 Description   

cljsc.bat emit FileNotFoundException when it compile samples of the ClojureScript project in windows like below.

------------------------------------------------
Exception in thread "main" java.io.FileNotFoundException: Could not locate cljs/closure__init.class
or cljs/closure.clj on classpath:
------------------------------------------------

It is caused by lack of a backslash in the end of path of the system environment variable, %CLOJURESCRIPT_HOME% set by a user.
In the case CLASSPATH is set to "C:\\clojure\clojurescriptsrc\clj;C:\\clojure\clojurescriptsrc\cljs" and this make it impossible for javac to find cljs/clojure.clj file.

So it can be solved by adding a backslash to the path of %CLOJURESCRIPT_HOME%.

I attached the patched file, "cljsc-path.bat"



 Comments   
Comment by David Nolen [ 04/Sep/13 11:04 PM ]

Can we please get a proper git diff thanks (and please send in your CA)! Also would be nice to get Windows users to check this out.

Comment by Park Sang Kyu [ 15/Sep/13 3:16 AM ]

git diff

Comment by David Nolen [ 05/Oct/13 11:55 AM ]

Thank you! Have you sent in your CA? http://clojure.org/contributing

Comment by Park Sang Kyu [ 19/Jun/14 10:24 AM ]

Yes i have sent my CA.

Comment by David Nolen [ 19/Jun/14 10:27 AM ]

Excellent, the patch is not correctly formatted. Can we get a new patch that conforms to http://github.com/clojure/clojurescript/wiki/Patches





[CLJS-525] Allow hashtable lookup used for numbers and strings to be extended to other built-in types Created: 17/Jun/13  Updated: 17/Jun/13

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

Type: Enhancement Priority: Major
Reporter: Chas Emerick Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

...which would enable safe extension of key cljs protocols to types without modifying their prototypes, e.g. CLJS-523.



 Comments   
Comment by David Nolen [ 17/Jun/13 2:56 PM ]

Date is the only JS native case that I'm aware of that we don't handle. One tricky bit is that goog.typeOf won't give us the information we need, but I think instanceof should cover us here?

Comment by Fogus [ 17/Jun/13 3:05 PM ]

instanceof or the ever-gruesome toString.call(aDate) == '[object Date]' will work.





[CLJS-497] Constant literal optimization Created: 25/Apr/13  Updated: 27/Aug/13

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

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


 Description   

We should optimize constant literals, in particular keywords. This optimization means that we will have to decide whether to make identical? slower by testing for keywords (this means it's probably a bad idea to continue to inline it) or to provide a special keyword-identical? that does the right thing.



 Comments   
Comment by Sean Grove [ 26/Aug/13 11:03 PM ]

This is related to the reified keywords in cljs, see http://dev.clojure.org/jira/browse/CLJS-576

Comment by Sean Grove [ 27/Aug/13 4:24 PM ]

There's another interesting twist while using piggieback + brepl that relates to a missing constants_table.js. Not sure what causes it (haven't found a way to repro), but only happens in a few circumstances, so the repl still mainly works.

The runtime part continues to works fine however.





[CLJS-449] stack traces for ex-info Created: 23/Dec/12  Updated: 03/Jan/15

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

Type: Enhancement Priority: Major
Reporter: Tom Jack Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File 0001-hack-ex-info-stacktraces.patch    

 Description   

Currently, ex-info exceptions have no stack traces, which makes them.. much less useful.

Attached patch (0001-hack-ex-info-stacktraces.patch) is one hackish way of getting stack traces back when Error.captureStackTrace is available (at least in Chrome, presumably in Node as well).

But this seems unacceptable — all the stuff emitted for the deftype (making the constructor print as a type) gets overwritten.

Is there a better way?

Will this maybe be made irrelevant by source maps?



 Comments   
Comment by David Nolen [ 23/Dec/12 11:51 AM ]

Isn't the stack trace recoverable from the original exception?

Comment by Tom Jack [ 23/Dec/12 4:01 PM ]

I hadn't considered that. If a cause is provided, that seems reasonable. The only trouble is that you still have to know where the ExceptionInfo is being thrown, so that you can catch it and throw the cause. Chrome's debugger would make it easy to at least find where the ExceptionInfo is thrown, but it's still more trouble than having the appropriate stack trace directly on the ExceptionInfo.

If no cause is provided, of course that won't work. But I suppose one could write (ex-info msg data (js/Error.)) instead of (ex-info msg data)? Personally, I don't expect to pass a cause very often — I expect the binary arity to be much more common in my cljs libraries.

If we rely on the cause for the stack trace, maybe we could have the cause default to a new Error if not supplied? It seems sort of conceivable that we could also wire up the ExceptionInfo to proxy .stack to that Error so that the stacktrace will come through, without needing to override the ExceptionInfo constructor.

Comment by David Nolen [ 03/Jan/15 6:12 PM ]

Sorry for letting this one languish, it appears stack traces just work under Node.js. However that doesn't seem to be the case in browsers. I would take a rebased patch, thanks.





[CLJS-404] Automate Browser REPL testing Created: 23/Oct/12  Updated: 26/Dec/14

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

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


 Description   

It's worth investigating Selenium, PhantomJS, etc. as solutions to sanity check the Browser REPL when we run the other tests.



 Comments   
Comment by Robert Krahn [ 22/Dec/14 1:22 PM ]

An attempt: https://github.com/clojure/clojurescript/pull/42

Comment by David Nolen [ 24/Dec/14 8:57 AM ]

This looks like an interesting patch, thanks!

Comment by Robert Krahn [ 26/Dec/14 10:57 AM ]

I'll post a patch here, first I'll investigate the load-file issue, though.





[CLJS-374] satisfies? produces strange code when the protocol is not in the fast-path list Created: 06/Sep/12  Updated: 19/Nov/13

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





[CLJS-365] apply needs to put all args after the 20th into an array seq Created: 29/Aug/12  Updated: 29/Aug/12

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

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


 Description   

This ticket is related to CLJS-359






[CLJS-364] compiler needs to put all args of an invocation after 20 into an array-seq Created: 29/Aug/12  Updated: 29/Aug/12

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

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


 Description   

This ticket is related to CLJS-359






[CLJS-109] Compiler errors/warnings should be displayed when cljs namespace 'package' names start with an unacceptable javascript symbol. Created: 29/Nov/11  Updated: 31/Aug/12

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

Type: Defect Priority: Major
Reporter: Benjamin Conlan Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None
Environment:

OSX 10.7



 Description   

Clojurescript namespaces are extremely flexible. The generated javascript symbol names are not. ie:

cljs:
[code]
(ns canvas.context.2d)

(defn ^:export create-image-data
([canvas image-data] (.createImageData image-data))
([canvas sw sh] (.createImageData canvas sw sh)))
[/code]

generated js:
[code]
goog.provide('canvas.context.2d');
goog.require('cljs.core');
canvas.context.2d.create_image_data = (function() {
var create_image_data = null;
var create_image_data__2432 = (function (canvas,image_data){ return image_data.createImageData(); });
var create_image_data__2433 = (function (canvas,sw,sh){ return canvas.createImageData(sw,sh); });
create_image_data = function(canvas,sw,sh){
switch(arguments.length){ case 2 : return create_image_data__2432.call(this,canvas,sw); case 3 : return create_image_data__2433.call(this,canvas,sw,sh); }
throw('Invalid arity: ' + arguments.length);
};
return create_image_data;
})()
;
goog.exportSymbol('canvas.context.2d.create_image_data', canvas.context.2d.create_image_data);[/code]

Note the symbol name "canvas.context.2d.create_image_data". Because of the "2d" namespace name, the javascript generated is invalid.

This can simply be resolved by renaming the namespace but some notification to the developer during the compilation stage should help avoid unnecessary problems.



 Comments   
Comment by Jozef Wagner [ 09/Jan/12 2:59 PM ]

I had a similar problem when my namespace contained a word class, e.g. in namespace:

(ns foo.bar.class)

Advanced closure compiler produced an error treating class as a JS keyword.





[CLJS-27] Conditional compilation (or reading) Created: 22/Jul/11  Updated: 21/Jan/15

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

Type: Enhancement Priority: Major
Reporter: Rich Hickey Assignee: Unassigned
Resolution: Unresolved Votes: 12
Labels: None

Attachments: File cljs-27.diff     File cljs-27-v2.diff     File cljs-27-v3.diff     File cljs-27-v4.diff     File cljs-27-v5.diff     File cljs-27-v6.diff     File conditional-compilation-clojure.diff     File conditional-compilation-clojurescript.diff    
Patch: Code and Test
Approval: Vetted

 Description   

As people start trying to write libs that are portable between Clojure and ClojureScript they might need to have a bit of branching on target. N.B. supporting this means a change to Clojure, although it has general utility there as well.

Consider CL #+ #- reader macros - http://www.lispworks.com/documentation/lw50/CLHS/Body/02_dhq.htm

Patch: cljs-27-v6.diff

Related: CLJ-1424, TRDR-14



 Comments   
Comment by Roman Scherer [ 19/Jul/12 8:52 AM ]

The following patches include an implementation of Common Lisp's #+
and #- reader macros to allow conditional compilation/reading for
Clojure and ClojureScript.

The patches add a dynamic variable called features to the
clojure.core and cljs.core namespaces, that should contain the
supported features of the platform in question as keywords.

Unlike in Common Lisp, the variable is a Clojure set and not a list.
In Clojure the set contains at the moment the :clojure keyword, and in
ClojureScript the :clojurescript keyword.

I would like to get feedback on the names that are added to this
variable. Are those ok? Is :jvm for Clojure and :js for ClojureScript
better? Should ClojureScript add something like :rhino, :v8 or
:browser as well?

To run the ClojureScript tests, drop a JAR named "clojure.jar" that
has the Clojure patch applied into ClojureScript's lib directory.

Comment by David Nolen [ 19/Jul/12 12:18 PM ]

This is an enhancement so it probably requires a design page and extended discussion before it will go anywhere. Until that happens I'm marking this is as low priority.

Comment by Roman Scherer [ 19/Jul/12 1:45 PM ]

Ok. If someone could give me write access to the Clojure Dev Wiki I would be happy to start such a design page.

Comment by David Nolen [ 19/Jul/12 5:50 PM ]

If you've sent in your CA request permissions on the clojure-dev mailing list.

Comment by Roman Scherer [ 21/Jul/12 5:45 AM ]

I started a design page for this ticket in the Clojure Dev wiki:
http://dev.clojure.org/display/design/Feature+Expressions

Comment by Stuart Halloway [ 27/Jul/12 1:48 PM ]

Posting my comments over on the design page...

Comment by Alex Miller [ 06/Aug/14 7:42 AM ]

Latest patch updates into current ClojureScript and use of tools.reader/tools.analyzer etc. The reader changes are all in the accompanying tools.reader patch in TRDR-14. This patch adds support to allow a new option "features" which is expected to be a set of keywords. build will inject :cljs into this set. The feature set is maintained in clojure.tools.reader/*features*. set! is enhanced to special-case an update to *features* in the code (presumably a rarely-used feature).

Because tools.reader needs the supporting patch, I left in several changes that pull in a new version of tools.reader - I don't actually expect those to be the correct versions but they are there as a reminder to update all of the proper places.

Comment by Alex Miller [ 11/Sep/14 9:04 AM ]

cljs-27-v2.diff adds ability to load .clj files as well as .cljs files when compiling.

Comment by Alex Miller [ 07/Nov/14 10:55 AM ]

Fresh patch, switch to take .cljc files.

Comment by Alex Miller [ 06/Jan/15 3:04 PM ]

Freshened patch for current CLJS.

Comment by David Nolen [ 06/Jan/15 9:55 PM ]

Alex the freshened patch includes modifications to the compiler version dynamic var and compiler version fn. Can we remove these? Thanks!

Comment by Alex Miller [ 07/Jan/15 12:28 PM ]

Yep, updated to -v5 patch.

Comment by Alex Miller [ 21/Jan/15 4:36 PM ]

Updated to use new tools.reader patch and to remove the ability to dynamically set the features set. The active set of features can be set on startup and are held in the features var in the analyzer.





Generated at Mon Jan 26 12:33:14 CST 2015 using JIRA 4.4#649-r158309.