<< Back to previous view

[CLJS-25] goog.global.Date/String etc don't work in non-browser advanced mode Created: 22/Jul/11  Updated: 23/Jul/11  Resolved: 23/Jul/11

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

Type: Defect Priority: Blocker
Reporter: Rich Hickey Assignee: Rich Hickey
Resolution: Completed Votes: 0
Labels: None


 Description   

goog.global resolves to something undefined in advanced mode when not in the browser. This prevents us from using e.g. goog.global.String

We should experiment with --use_only_custom_externs=true flag for these builds. The book also mentions:
http://code.google.com/p/closure-compiler/source/browse/trunk/externs/es3.js?r=996

Not having this is causing all kinds of ick as people use js* for everything



 Comments   
Comment by Stuart Halloway [ 22/Jul/11 1:38 PM ]

It appears that the disabled warnings in script/compile

--jscomp_off=undefinedVars \
  --jscomp_off=missingProperties \

are disabled because of js* stuff that would go away if this issue was solved. Can these warnings be re-enabled or are they necessary for some other reason?

Comment by Stuart Halloway [ 22/Jul/11 2:36 PM ]

While experimenting with Advanced mode codegen, I found a workaround:

(defn ^:export _ggfn [] [goog.global])
(set! goog.global.String.prototype.call ...

compiles to

ba("cljs.core._ggfn", function() {
  return T([n])
});
n.H.prototype.call = function() {

The important thing above is the n. which refers to goog.global. Without the exported fn mentioning goog.global, the same set! code compiles to an invalid reference to this:

this.H.prototype.call = function() {

Interestingly, an exported def that refers to goog.global does not fix the problem, it has to be a defn.

Comment by Rich Hickey [ 22/Jul/11 3:37 PM ]

I'm taking this. I understand the problem (we were looking at goog.global completely backwards), and a fix will require some language (or at least ns) magic.





[CLJS-558] CLONE - closures act like js not clojure Created: 27/Jul/13  Updated: 27/Jul/13  Resolved: 27/Jul/13

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

Type: Defect Priority: Blocker
Reporter: Anonymous Assignee: Rich Hickey
Resolution: Completed Votes: 0
Labels: None


 Description   

(loop [i 0 j ()]
(if (< i 5)
(recur (inc i) (conj j (fn [] i)))
(map #(%) j)))

In Clojure, this returns: `(4 3 2 1 0)`

In ClojureScript it returns: `(5 5 5 5 5)`

Imported from github issue #85



 Comments   
Comment by David Nolen [ 27/Jul/13 1:45 PM ]

cloned, was resolved





[CLJS-70] Protocols not working in IE8 Created: 10/Sep/11  Updated: 27/Jul/13  Resolved: 23/Sep/11

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

Type: Defect Priority: Blocker
Reporter: Wilkes Joiner Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None
Environment:

IE8


Attachments: Text File fix_for_ie8.patch    
Patch: Code

 Description   

The function generated by defprotocol does not work in IE8. I haven't tried any other versions of IE. The fix is pretty simple, however I don't know the edge cases so consider the patch of a proof of concept.

I'll use IEquiv as an example. The currently generated code is:

cljs.core._equiv = function(a, b) {
return cljs.core.truth_(cljs.core.truth_(a) ? a.cljs$core$IEquiv$_equiv : a) ? a.cljs$core$IEquiv$_equiv(a, b) : function() {
var b = _equiv[goog.typeOf.call(null, a)];
if(cljs.core.truth_(b)) { return b }else {
if(b = equiv., cljs.core.truth_(b)) { return b }else { throw cljs.core.missing_protocol.call(null, "IEquiv.-equiv", a); }
}
}().call(null, a, b)
};

Inside the function the reference to _equiv is unqualified. In IE8, this evaluates to undefined. Using the fully qualified name fixes the issue. So, _equiv[goog.typeOf.call(null, a)] becomes cljs.core._equiv[goog.typeOf.call(null, a)], and equiv. becomes cljs.core.equiv.



 Comments   
Comment by David Nolen [ 23/Sep/11 1:35 AM ]

Fixed, https://github.com/clojure/clojurescript/commit/2c2f49ea9843f4ccc0b440506f8d3fcd93c898bf





[CLJS-42] Function names clash with namespace names Created: 28/Jul/11  Updated: 27/Jul/13  Resolved: 20/Dec/11

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

Type: Defect Priority: Blocker
Reporter: Anthony Simpson Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None


 Description   

Related to this thread: https://groups.google.com/forum/?hl=en#!topic/clojure/AINvE8fUjAM

The issue is that, if you have a namespace called 'foo.core' and a function called 'foo', foo clashes with the namespace inside the generated JavaScript code and results in odd and generally indecipherable errors. Here is some example code that fails:

(def net (node/require "net"))

(defn portal
  [port host] (.createConnection net port host))

(defn -main [& args] ; Note that only one of these -main functions is in the file at any given time, of course.
  (.createConnection net 1337 "localhost"))

(defn -main [& args]
  (portal 1337 "localhost"))

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

The first main is the one that works. The second main is a failwhale. Running the second main results in this error:

TypeError: Cannot read property 'net' of undefined
and this generated JavaScript:

portal.core.net = cljs.nodejs.require.call(null, "net");
portal.core.portal = function portal(b, c) {
  return portal.core.net.createConnection(b, c)
};
portal.core._main = function(a) {
  cljs.core.array_seq(Array.prototype.slice.call(arguments, 0), 0);
  return portal.core.portal.call(null, 1337, "localhost")
};

And here is the generated JS for the working -main:

portal.core._main = function(a) {
  cljs.core.array_seq(Array.prototype.slice.call(arguments, 0), 0);
  return portal.core.net.createConnection(1337, "localhost")
};


 Comments   
Comment by David Nolen [ 23/Sep/11 1:21 AM ]

Is this still a problem? I don't see anything in the generated source that looks problematic. Do you have an testable example of the issue that doesn't involve Node?

Comment by David Nolen [ 20/Dec/11 4:26 PM ]

Already resolved





[CLJS-40] Variadic fns fail to convey this Created: 28/Jul/11  Updated: 27/Jul/13  Resolved: 29/Jul/11

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

Type: Defect Priority: Blocker
Reporter: Rich Hickey Assignee: Fogus
Resolution: Completed Votes: 0
Labels: None


 Description   

When used for call, prevents 'this' getting to the bodies in the generated sub-fns. Currently blocking multimethods



 Comments   
Comment by Fogus [ 29/Jul/11 11:31 AM ]

Pushed to master. Frank mentioned that the fix works for his multimethods work.





[CLJS-753] Too many open files exception Created: 28/Jan/14  Updated: 28/Jan/14  Resolved: 28/Jan/14

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

Type: Defect Priority: Blocker
Reporter: Roman Scherer Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: File too-many-open-files.diff    
Patch: Code

 Description   

This patch uses `io/reader` in conjunction with `with-open` to prevent
"too many open files" errors. This happens in a browser REPL when
evaluating a file that references a couple of other files multiple
times.

From the Google Group discussion at: https://groups.google.com/forum/#!topic/clojurescript/r2iGPh2Lv0U

----------------------------

Hello Clojure Scripters,

with my current REPL setup I get some really annoying "Too many
files open" errors. I'm not sure if it's a problem with
ClojureScript itself, Austin the browser REPL, nREPL or my own
miss-configuration of something.

When I connect to the browser REPL via AUSTIN and eval a whole
ClojureScript file the first time a lot of Ajax requests are sent
over the wire and my main namespace is getting compiled and
shipped to the browser. So far so good, my Java process is at
around 18676 open files. I don't care yet.

Compiling the same file again and again increases the open files

not sure if this is a problem with my setup, but I

18676, 19266, 22750, 21352, 33097, 62913, 64398, 64398, 64398,
64398, 64398 up to 171977, where some ulimit is reached and I get
an exception like this:

java.io.FileNotFoundException: .repl/5614/request/routes.js (Too many open files)

and my ClojureScript hangs up and I have to do a
cider-restart. Ok maybe I shouldn't eval whole files too often
over the XHR connection, but this seems not right.

I used the command "lsof -n | grep java | wc -l" to watch the
above numbers while evaluating the file again and again.

Does someone had a similar problem, knows how to solve that, or
has any ideas how to track this one down?

Thanks for your help, Roman.



 Comments   
Comment by Roman Scherer [ 28/Jan/14 5:56 PM ]

Hi David, can you review this. Thanks.

Comment by David Nolen [ 28/Jan/14 6:19 PM ]

Looks good to me will apply soon and test.

Comment by David Nolen [ 28/Jan/14 10:51 PM ]

fixed, https://github.com/clojure/clojurescript/commit/905c64445faa1c60e21b66fb226982759b0d4001





[CLJS-551] CLONE - Create Sample Application Created: 27/Jul/13  Updated: 27/Jul/13  Resolved: 27/Jul/13

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

Type: Enhancement Priority: Critical
Reporter: Rich Hickey Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None


 Description   

Remaining items:

  • host the application somewhere
  • links to compiled code
  • someone other than Brenton should review the code (is this a good example of how to write ClojureScript?)
  • could use a little more design work

Since I (Brenton) won't be able to do any of these things, I am relinquishing this task to someone who can.



 Comments   
Comment by David Nolen [ 27/Jul/13 1:45 PM ]

cloned, was resolved





[CLJS-26] Create a standard way to reference this Created: 22/Jul/11  Updated: 27/Jul/13  Resolved: 01/Oct/11

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

Type: Enhancement Priority: Critical
Reporter: Rich Hickey Assignee: Fogus
Resolution: Completed Votes: 0
Labels: None


 Description   

Some js frameworks require you to implement 'methods' and those methods need to access js this. Currelty this forces you to js*.

_the following is handled by CLJS-83_

Currently deftype only supports implementing protocol functions. It could be enhanced to support 'method' fns, possibly by using the Object section:

(deftype Foo [a]
Object
(anyArbitraryMethod [a-name-for-this] ...))

Note that the arity of the actual fn would be one less than for protocol 'methods'.

Explore other options before going this way.

Supporting access to 'this' in stand-alone functions may cause gclosure to complain (it only wants to see traditional methods on prototypes), and is a non-objective for now.



 Comments   
Comment by Fogus [ 30/Sep/11 2:17 PM ]

The plan is to start with a pair, this-as and extend-object!. used as such:

;;;;;; GLOBAL THIS

(defn f [] 
  (this-as self 
    (println self)) 
  [self])

(f)
;; #<Object Global>
;;=> [#<undefined>] 

;;;;;; METHOD THIS

(def o (array))

(extend-object! o {:foo f})

(. o (foo))
;; #<Array []>
;;=> [#<undefined>]
Comment by Fogus [ 01/Oct/11 8:56 PM ]

I am going to split out the (deftype T ... Object ...) capability into its own ticket as it's scope is greater than the accessing of this. It's likely that the new ticket will eliminate the need for extend-object!.

Comment by Fogus [ 01/Oct/11 9:19 PM ]

Added note about related ticket. (CLJS-83)

Comment by Fogus [ 01/Oct/11 9:20 PM ]

The this-as macro provides a scoped way to access the implicit this.





[CLJS-442] Lazy seqs fails to close over local vars Created: 14/Dec/12  Updated: 27/Jul/13  Resolved: 18/Dec/12

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

Type: Defect Priority: Critical
Reporter: Jozef Wagner Assignee: Michał Marczyk
Resolution: Completed Votes: 1
Labels: None
Environment:

master clojurescript, commit 9abeb143f6


Attachments: Text File 0001-CLJS-442-fix-a-bug-whereby-fns-might-not-close-over-.patch    

 Description   

When executing following code

(loop [i 10
       v [0]
       v2 [0]]
  (if (pos? i)
    (let [j i
          x (map #(+ j %) v)]
      (recur (dec i) x (map #(+ j %) v2)))
    (concat v v2)))

Clojure produces

(55 55)
while Clojurescript produces
(10 55)

Forcing realization of lazy seqs produces correct output (55 55) on both environments.

(loop [i 10
       v [0]
       v2 [0]]
  (if (pos? i)
    (let [j i
          x (doall (map #(+ j %) v))]
      (recur (dec i) x (doall (map #(+ j %) v2))))
    (concat v v2)))


 Comments   
Comment by Michał Marczyk [ 14/Dec/12 11:28 PM ]

This is actually due to a bug in analyze-let whereby functions involved in init-exprs would not close over earlier bindings. The attached patch fixes this.

Comment by Michał Marczyk [ 14/Dec/12 11:30 PM ]

Incidentally, I wonder whether *loop-lets* is still an appropriate name for this Var...

Comment by David Nolen [ 18/Dec/12 3:04 PM ]

Do you have a better name in mind?

Comment by David Nolen [ 18/Dec/12 3:08 PM ]

fixed http://github.com/clojure/clojurescript/commit/84e9488f49bcfea4b4037a562f8f797c7ddd79f0

Comment by Jozef Wagner [ 19/Dec/12 7:46 AM ]

Thank you, you guys are great!





[CLJS-544] named fn lexical scoping bug Created: 18/Jul/13  Updated: 27/Jul/13  Resolved: 19/Jul/13

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

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


 Description   
(fn [foo] (fn foo [] foo))

generates the following code

function (foo) {
    return (function foo() {
        return foo__$1;
    });
}


 Comments   
Comment by David Nolen [ 19/Jul/13 1:06 AM ]

It looks like the issue is that emit-fn-method in compiler.clj just emits :name from the fn AST, but this :name lack the shadow information present that we pass along via locals in parse fn* star case in analyzer.clj

Comment by David Nolen [ 19/Jul/13 8:06 AM ]

fixed http://github.com/clojure/clojurescript/commit/9ddf847b44ec82070e91038f4afbd8a2baec94ff





[CLJS-333] Keyword self-lookups to return default value when appropriate Created: 03/Jul/12  Updated: 27/Jul/13  Resolved: 03/Jul/12

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

Type: Defect Priority: Critical
Reporter: Michał Marczyk Assignee: Michał Marczyk
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 0001-CLJS-333-add-missing-arity-to-cljs.core.Keyword-s-IF.patch    

 Description   

Originally reported by Evan Mezeske in a comment on CLJS-330.

Due to a missing -invoke arity in cljs.core.Keyword's implementation of IFn, calls like (:foo {} 1) fail to produce the given default value.



 Comments   
Comment by Michał Marczyk [ 03/Jul/12 7:30 PM ]

Note the case with not-found doesn't bother to check if it's dealing with an ObjMap. This is because an ObjMap is capable of holding nil or false as a value, so we would still need to call contains? which on an ObjMap does exactly the same work as -lookup, except it returns true / false thus forcing us to pick the correct return value.

Comment by David Nolen [ 03/Jul/12 7:52 PM ]

fixed, http://github.com/clojure/clojurescript/commit/04e44e444f9bf63d2833d8ff9f87fdb32ac9de48





[CLJS-189] Order-independent hashing of maps Created: 21/Apr/12  Updated: 27/Jul/13  Resolved: 21/Apr/12

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

Type: Defect Priority: Critical
Reporter: Michał Marczyk Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 0001-clojure.lang.APersistentMap-like-hashing-for-maps.patch    

 Description   

This patch introduces a new function – {cljs.core/hash-imap} – which calculates a hash for a map using the algorithm of {clojure.lang.APersistentMap} and wires it into the {-hash} implementations of the various map types. The benefit is that it is order-independent and thus guarantees a stable hash value congruent with {=} without the need for keyset sorting.

This should also be used in sets once {PersistentTreeSet} lands (along with the tree map).



 Comments   
Comment by Michał Marczyk [ 21/Apr/12 11:28 AM ]

Note this calculates the hash modulo 4503599627370496 so it can be represented exactly. Perhaps it would be better to stay within int bounds though?

Comment by David Nolen [ 21/Apr/12 11:37 AM ]

Fixed, https://github.com/clojure/clojurescript/commit/6fd16df50bca7a6118693712e6918c70a64de255





[CLJS-201] Fix bug in PersistentTreeMap, supply some missing proto impls in PTM, PHM auxiliaries Created: 24/Apr/12  Updated: 27/Jul/13  Resolved: 24/Apr/12

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

Type: Defect Priority: Critical
Reporter: Michał Marczyk Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 0001-Fix-bugs-around-PersistentTreeMap-PersistentHashMap.patch    

 Description   

From the commit message:



 Comments   
Comment by Michał Marczyk [ 24/Apr/12 1:08 AM ]

A missing ctor argument is supplied in tree-map-append, missing
protocol method and .toString implementations are supplied in PTM- and
PHM-related auxiliary types.

Comment by Michał Marczyk [ 24/Apr/12 7:17 AM ]

The actual .patch file this time.

Comment by David Nolen [ 24/Apr/12 11:10 AM ]

fixed, https://github.com/clojure/clojurescript/commit/63102bceaea60485ae936047a96c6d66bb7387c3





[CLJS-89] Simplify the dot access special form. Created: 14/Oct/11  Updated: 27/Jul/13  Resolved: 01/Feb/12

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

Type: Enhancement Priority: Critical
Reporter: Fogus Assignee: Fogus
Resolution: Completed Votes: 2
Labels: None


 Description   

Currently ClojureScript provides the following dot access syntaxes:

(.p o)           ;=> a property access

(.m o <args>)    ;=> a method call

(. o p)          ;=> a property access

(. o p <args>)   ;=> a method call

(. o (m))        ;=> a method call

(. o (m <args>)) ;=> a method call

This is potentially confusing. Especially considering that Clojure looks as follows:

(.p o)           ;=> a property access if p is a property

(.p o)           ;=> a method call if p is a method

(.m o <args>)    ;=> a method call

(. o p)          ;=> a property access if p is a property

(. o p)          ;=> a method call if p is a method

(. o p <args>)   ;=> a method call

(. o (m))        ;=> a method call

(. o (m <args>)) ;=> a method call

The divergence between the Clojure and ClojureScript way is due to first-class functions. That is, in JavaScript a property can be a field or a function or a method. There is currently no way to distinguish between the desire to call a method and retrieve a function as a property except through syntax. This ticket would shorten the gap between Clojure and ClojureScript in the following way (ClojureScript proposal below):

(.m o)           ;=> a method call

(.m o <args>)    ;=> a method call

(. o m)          ;=> a method call

(. o m <args>)   ;=> a method call

(. o (m))        ;=> a method call

(. o (m <args>)) ;=> a method call

(.-p o)          ;=> a property access

(. o -p)         ;=> a property access

This would be a breaking change, but would shorten the divergence between Clojure and ClojureScript.



 Comments   
Comment by Fogus [ 16/Nov/11 9:09 AM ]

The implementation on the prop-lookup branch is functional and has associated tests. Ready for inclusion.

Comment by Benjamin Conlan [ 02/Dec/11 12:02 AM ]

I've just given this a quick spin and it works beautifully (at least regarding the property value vs property function syntax).

But after looking at my code I can't help but think that for the non-function case (ie json data) just using the maps symbol operator might be more desirable instead of (.-p o))

so json such as:

var jsonData = {
  "person": {
    "title" : "mr",
    "name" : {
      "firstname" : "Harry",
      "surname" : "Jones"
    }
  }
}

could be accessed like:

(:firstname (:name (:person jsonData)))

But in saying this I haven't put any thought into this statement (and of course the suggested code doesn't work anyway in the master/prop-access branches).

In coming full circle, I'm just appreciative of your work on the prop-access branch.

Comment by Stuart Sierra [ 03/Jan/12 5:05 PM ]

Back in the dark ages before Clojure 1.0, you always had to write this;

(. o (m))  ; method call
(. o f)    ; field access

Later, but before the introduction of (.m o) syntax, this was shortened to (. o x) for both methods and fields. What if we went back to the original dot operator syntax, but assume that (.m o) always means a method call:

(. o f)     ; always field access
(. o (m))   ; always method call
(.m o)      ; always method call

Raw field access is rare in Java, perhaps less so in JavaScript. But this doesn't require adding any new syntax and is consistent with what Clojure programmers are used to.

The common case for field access in ClojureScript is, I believe, interop which uses JavaScript objects as map-like data structures. My preferred solution for that case would be better conversion between JS objects and real ClojureScript maps.

Comment by Thomas Scheiblauer [ 01/Feb/12 12:37 PM ]

I would also prefer Stuart's solution due to the reasons he mentioned and because the currently implemented concatenation of sugar characters (.-somefield obj) didn't really make the matter taste more sweet it feels rather clumsy to me.
By the way, I would also propose to implement 'memfn' for staying consistent with Clojure (if it hasn't already been done so).

Comment by David Nolen [ 01/Feb/12 4:09 PM ]

At this point it's probably water under the bridge. If you're interested in memfn, please submit a patch

Comment by David Nolen [ 01/Feb/12 4:53 PM ]

Done





[CLJS-226] Fix bug in TransientVector impl Created: 01/May/12  Updated: 27/Jul/13  Resolved: 01/May/12

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

Type: Defect Priority: Critical
Reporter: Michał Marczyk Assignee: Michał Marczyk
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 0002-Fix-bug-in-TransientVector-impl.patch    
Patch: Code

 Description   

This is the bug reported by Brian Taylor in CLJS-208. Patch created on top of CLJS-224, but applies cleanly against current master.



 Comments   
Comment by David Nolen [ 01/May/12 5:26 PM ]

fixed, https://github.com/clojure/clojurescript/commit/b97dd8707f7d78d672cdc255a18ff52106f8f481





[CLJS-229] Fix count for non-counted collections, remove ICounted from Cons Created: 01/May/12  Updated: 27/Jul/13  Resolved: 02/May/12

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

Type: Defect Priority: Critical
Reporter: Michał Marczyk Assignee: Michał Marczyk
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 0001-Fix-count-for-non-counted-collections-remove-ICounte.patch    
Patch: Code

 Description   

This caused TwitterBuzz breakage reported by leimy in #clojure: http://clojure-log.n01se.net/date/2012-05-01.html#21:36

Patch created on top of CLJS-228, but should apply cleanly to master.

From the commit message:

Since default no longer gets extended to ICounted, the original
implementation of cljs.core/count no longer works on LazySeq
instances.

This patch takes the approach of checking whether the given collection
is counted and either calling -count or a linear traversal
helper (which rolls over to -count as soon as possible).

The implementation of ICounted on Cons is now unnecessary and has been
removed, so that things for which counted? returns true really do
provide constant-time count.



 Comments   
Comment by Michał Marczyk [ 01/May/12 9:49 PM ]

The actual patch file this time.

Comment by David Nolen [ 02/May/12 10:30 AM ]

fixed, https://github.com/clojure/clojurescript/commit/0ad5556531325d03746f05845dcd26acdc716ef7





[CLJS-204] Bug in sort Created: 25/Apr/12  Updated: 27/Jul/13  Resolved: 18/May/12

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

Type: Defect Priority: Critical
Reporter: Sean Nilan Assignee: Hubert Iwaniuk
Resolution: Completed Votes: 0
Labels: clojure
Environment:

Mac OS X, recently cloned ClojureScript repository


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

 Description   

The sort function (and thus sort-by) do not seem to be behaving as they would in normal Clojure. For example, in my ClojureScript REPL I have:

ClojureScript:cljs.user> (def values (list [-11 10] [-8 12] [-15 -4] [4 5]))
([-11 10] [-8 12] [-15 -4] [4 5])
ClojureScript:cljs.user> (sort values)
([-11 10] [-15 -4] [-8 12] [4 5])

But in the Clojure REPL, you get what one would expect:
user=> (def values (list [-11 10] [-8 12] [-15 -4] [4 5]))
#'setback.core/values
user=> (sort values)
([-15 -4] [-11 10] [-8 12] [4 5])

I have only noticed this bug for sorting vectors, since in Clojure vectors are sorted position by position.



 Comments   
Comment by Sean Nilan [ 25/Apr/12 8:52 AM ]

The bug is actually in the compare function, which uses the google.array.defaultCompare as the compare function for vectors. A simple patch such as below could fix the problem:

(defn vector-compare
[v1 v2]
(cond
(and (empty? v1) (empty? v2))
0
(empty? v1)
-1
(empty? v2)
1
:default
(let [cmp (apply compare (map first [v1 v2]))]
(if (zero? cmp)
(vector-compare (rest v1) (rest v2))
cmp))))

(defn compare
"Comparator. Returns a negative number, zero, or a positive number
when x is logically 'less than', 'equal to', or 'greater than'
y. Uses google.array.defaultCompare for objects of the same type
and special-cases nil to be less than any other object."
[x y]
(cond
(and (vector? x) (vector? y)) (vector-compare x y)
(identical? (type x) (type y)) (garray/defaultCompare x y)
(nil? x) -1
(nil? y) 1
:else (throw (js/Error. "compare on non-nil objects of different types"))))

Comment by David Nolen [ 25/Apr/12 8:54 AM ]

Thanks. We need a IComparable protocol.

Comment by Hubert Iwaniuk [ 16/May/12 3:11 AM ]

IComparable protocol, implementations and tests.

Comment by David Nolen [ 16/May/12 8:15 AM ]

Can we squash the commits here? Thanks!

Comment by Hubert Iwaniuk [ 16/May/12 8:30 AM ]

Commits squashed.

Comment by David Nolen [ 16/May/12 11:31 AM ]

Why do we need to extend IComparable to strings, arrays, boolean, and number?

Comment by Hubert Iwaniuk [ 17/May/12 2:17 PM ]

booleans - are supported in clj
number - special case in clj, not sure if it brings any speed increase, will test with jsperf
arrays - are supported in clj
strings - to support symbols and keywords

Comment by Hubert Iwaniuk [ 18/May/12 7:46 AM ]

Simplified version, just to solve comparing vectors.

Comment by David Nolen [ 18/May/12 7:28 PM ]

fixed, http://github.com/clojure/clojurescript/commit/43ff1c4adea322e6edc1d368ff128f2ad47406a5





[CLJS-589] '({:a 1}) doesn't implement INext Created: 12/Sep/13  Updated: 13/Sep/13  Resolved: 13/Sep/13

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

Type: Defect Priority: Critical
Reporter: George Fraser Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None
Environment:

Clojure 1.5.1
lein-clsbuild 0.3.2



 Description   

Clojurescript:

(map set '({:a 1}))
==> No protocol method INext.-next defined for type cljs.core/NodeSeq: ([:a 1])

(map set (list {:a 1}))
==> (#{[:a 1]})

Clojure:
(map set '({:a 1}))
==> (#{[:a 1]})



 Comments   
Comment by David Nolen [ 13/Sep/13 7:46 AM ]

this bug is not present in the latest release of ClojureScript, 0.0-1878.





[CLJS-751] requiring clojure.core.reducers throws error Created: 15/Jan/14  Updated: 16/Jan/14  Resolved: 16/Jan/14

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

Type: Defect Priority: Critical
Reporter: Francis Avila Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: reducers
Environment:

r2138


Attachments: Text File cljs-751.patch    

 Description   

Requiring the reducers library in any file throws a TypeError at runtime. The generated javascript in reducers.js is this:

cljs.core.IPersistentVector.prototype.clojure$core$reducers$CollFold$ = true;

Error messages is "Uncaught TypeError: Cannot read property 'prototype' of undefined reducers.js:733
(anonymous function)". This stops all execution of JS and basically makes it impossible to use any

IPersistentVector in reducers.cljs should probably be PersistentVector. The specify machinery in extend-type probably exposed this bug.



 Comments   
Comment by Francis Avila [ 15/Jan/14 4:54 PM ]

Note that because of advanced-compilation dead-code elimination, you won't catch this if you advance-compile. Run script/test with whitespace optimizations to see the reducer tests fail.

Comment by Francis Avila [ 15/Jan/14 4:57 PM ]

Patch fixes typo and causes existing reducer tests to pass (tests run with whitespace optimization). Doesn't make sense to write any more tests, although the test runner should probably not use advanced compilation as the default....

Comment by David Nolen [ 16/Jan/14 5:19 PM ]

fixed, https://github.com/clojure/clojurescript/commit/6e10f3d2ca99c58c441de1a1031be2649dae4072





[CLJS-789] Advanced compilation broken with latest closure libra Created: 04/Apr/14  Updated: 22/Apr/14  Resolved: 22/Apr/14

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

Type: Defect Priority: Critical
Reporter: Francis Avila Assignee: Unassigned
Resolution: Duplicate Votes: 0
Labels: None
Environment:

cljs >= 2197



 Description   

CLJS releases >= 2197 are depending on the most recent packaged closure lib [org.clojure/google-closure-library "0.0-20140226-71326067"].

This is bad,



 Comments   
Comment by Francis Avila [ 04/Apr/14 3:50 PM ]

I have no idea how this was submitted both too soon and twice! I'm very sorry for the mess. Please mark duplicate of CLJS-790 and close.





[CLJS-812] Recurring from a case statement emits invalid JavaScript Created: 09/Jun/14  Updated: 13/Jun/14  Resolved: 13/Jun/14

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

Type: Defect Priority: Critical
Reporter: Luke VanderHart Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: bug, compiler


 Description   

In 0.2227, compiling the following form produces syntactically invalid JavaScript:

(defn reproduce
  [value]
  (case value
    :a (recur :b)
    :b 0))

Yields:

bug_repro_test.reproduce = (function reproduce(value) {
    while (true) {
        var G__5832 = (((value instanceof cljs.core.Keyword)) ? value.fqn : null);
        var caseval__5833;
        switch (G__5832) {
            case "b":
                caseval__5833 = 0
                break;
            case "a":
                caseval__5833 = {
                    var G__5834 = cljs.core.constant$keyword$67;
                    value = G__5834;
                    continue;
                }

                break;
            default:
                caseval__5833 = (function () {
                    throw (new Error(("No matching clause: " + cljs.core.str.cljs$core$IFn$_invoke$arity$1(value))))
                })()
        }
        return caseval__5833;
        break;
    }
});

When evaluated in any JavaScript environment (including the Google Closure compiler) environment, this yields a syntax error in this code:

caseval__5833 = {
                    var G__5834 = cljs.core.constant$keyword$67;
                    value = G__5834;
                    continue;
                }


 Comments   
Comment by David Nolen [ 10/Jun/14 7:56 AM ]

Good catch. I suspect that throw may be similarly problematic.

Comment by David Nolen [ 13/Jun/14 3:59 PM ]

fixed in master https://github.com/clojure/clojurescript/commit/cc11c7996ba8522d6767fb45df2f76e20e4c1773





[CLJS-819] cljs.reader cannot handle character classes beginning with slashes in regex literals Created: 20/Jun/14  Updated: 01/Jul/14  Resolved: 01/Jul/14

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

Type: Defect Priority: Critical
Reporter: Ziyang Hu Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: bug, cljs, reader

Attachments: Text File cljs-819.patch    

 Description   

cljs.user> (cljs.reader/read-string "#\"\\s\"")
Compilation error: Error: Unexpected unicode escape \s



 Comments   
Comment by Ziyang Hu [ 20/Jun/14 10:03 AM ]

This in particular means that (cljs.reader/read-string (str [#"\s"])) won't work

Comment by Francis Avila [ 25/Jun/14 11:46 AM ]

Patch and test.

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

fixed https://github.com/clojure/clojurescript/commit/32259c5ff3f86ea086ae3949403df80c2f518c7e





[CLJS-790] Advanced compilation broken with latest closure libra Created: 04/Apr/14  Updated: 03/Jul/14  Resolved: 03/Jul/14

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

Type: Defect Priority: Critical
Reporter: Francis Avila Assignee: Unassigned
Resolution: Completed Votes: 2
Labels: None
Environment:

cljs >= 2197


Attachments: Text File cljs-790.patch     Text File cljs-790-updated.patch    

 Description   

CLJS releases >= 2197 are depending on the most recent packaged closure lib [org.clojure/google-closure-library "0.0-20140226-71326067"].

This is bad,



 Comments   
Comment by Francis Avila [ 04/Apr/14 3:46 PM ]

ARG, submitted too soon by accident.

Rundown is this:

  1. CLJS >= 2197 depends on the latest packaged closure lib (March 17th release, "0.0-20140226-71326067").
  2. This closure lib version is incompatible with the closure compiler closurescript depends on at least because of changes to how goog.base works. See commit 5bdb54d221dbf7c6177ba5ba6901c012981501ec on the closure compiler library (and many more after this one with a similar commit message).
  3. When you advanced-compile a cljs project, goog classes which internally use the goog.base(this, 'superMethod') form will all be munged incorrectly and throw exceptions. Minimal test case: (ns myproj (:require goog.net.XhrIo)) (goog.net.XhrIo/send "http://example.org" #(js/console.log %)). This will fail at a "disposeInternal" call. (Notice that the output js still has the "disposeInternal" string!)
  4. Oddly, the compiler does not emit any warnings about this!
  5. Additionally, the clojurescript project's POM and project.clj specify different versions of the closure library starting at https://github.com/clojure/clojurescript/commit/523a0c5b18138c9a4d23c5104a77b65488bc28c3 The POM specifies the new lib, but the project.clj the older one.

A workaround is to declare an explicit dependency in your project to [org.clojure/google-closure-library "0.0-20130212-95c19e7f0f5f"]. If you do this everything will start working again.

Basically, cljs can't use the new closure lib until its closure compiler has an updated release. (There is some hope that this may be easier to do in the future: https://groups.google.com/d/msg/closure-compiler-discuss/tKZQ1eLUixA/3urgnli84SYJ)

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

Has this been resolved by a new closure compiler release yet? Thanks!

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

Donno, will give v20140407 release a try tonight.

Comment by Francis Avila [ 13/May/14 8:13 AM ]

Seems to work. Ran the test suite, benchmark, and some other projects I had which were failing before. Patch attached with updated dependencies.

Comment by Stephen Nelson [ 17/Jun/14 10:21 PM ]

This bug (or a similar one) is still affecting clojurescript 0.0-2234. Another example:

(ns myproj
(:require goog.net.XhrManager))
(.send (goog.net.XhrManager.) "xhr-request" "myproj.js" "GET" nil nil nil #(.log js/console %))

The workaround suggested above (using the old closure library) works, but versions of clojurescript newer than 2197 cannot be used in production without the workaround. If this bug is going to remain open in new releases, please consider documenting the workaround in the release notes.

Comment by David Nolen [ 18/Jun/14 10:42 AM ]

Can we get a new patch with a more recent release? http://search.maven.org/#artifactdetails%7Ccom.google.javascript%7Cclosure-compiler-parent%7Cv20140508%7Cpom

Comment by Francis Avila [ 24/Jun/14 10:30 AM ]

Bumped version. Note that this patch is unnecessary in the 1.6.0 branch, as its versions are up to date and its pom and project.clj agree. This commit is likely conflict when 1.6.0 is merged into master (the master side should be ignored if that happens).

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

this should be resolved in master

Comment by Francis Avila [ 03/Jul/14 1:55 PM ]

It is. You can close.

Comment by David Nolen [ 03/Jul/14 2:10 PM ]

fixed in master with 1.6.0 work merge





[CLJS-1] Pattern used in path-seq is not portable Created: 21/Jul/11  Updated: 28/Jul/11  Resolved: 28/Jul/11

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

Type: Defect Priority: Major
Reporter: Paul Michael Bauer Assignee: Brenton Ashworth
Resolution: Completed Votes: 1
Labels: None
Environment:

Any environment that uses '\' as its default path separator.


Attachments: File portable-path-regex.diff     File portable-path-regex-properly-annotated.diff    
Patch: Code

 Description   

The pattern used in cljs/compiler.clj to generate a path sequence is not portable.
Every execution of cljsc throws a Pattern error in some environments.



 Comments   
Comment by Brenton Ashworth [ 25/Jul/11 7:54 AM ]

Here is the related conversation on the mailing list.

http://groups.google.com/group/clojure/browse_thread/thread/90e6c22b8224ded6

Comment by Paul Michael Bauer [ 25/Jul/11 10:49 PM ]

Re-uploading patch (portable-path-regex-propertly-annotated.diff).
Previous patch did not have the proper name and email address associated with the CA.

Comment by Marko Kocić [ 26/Jul/11 6:54 AM ]

The attached patch solves only a part of the issue. After applying the patch I was still not able to compile twitterbuzz. Here's the exception I got:

c:\development\cvstree\clojurescript\samples\twitterbuzz>..\..\bin\cljsc src > twitterbuzz.js
Exception in thread "main" java.lang.RuntimeException: java.io.FileNotFoundException: The file C:\development\cvstree\clojurescript\src\cljs\cljs%5ccore.cljs does not exist.
at clojure.lang.Util.runtimeException(Util.java:153)
at clojure.lang.Compiler.eval(Compiler.java:6417)
at clojure.lang.Compiler.load(Compiler.java:6843)
at clojure.lang.Compiler.loadFile(Compiler.java:6804)
at clojure.main$load_script.invoke(main.clj:282)
at clojure.main$script_opt.invoke(main.clj:342)
at clojure.main$main.doInvoke(main.clj:426)
at clojure.lang.RestFn.invoke(RestFn.java:421)
at clojure.lang.Var.invoke(Var.java:405)
at clojure.lang.AFn.applyToHelper(AFn.java:163)
at clojure.lang.Var.applyTo(Var.java:518)
at clojure.main.main(main.java:37)
Caused by: java.io.FileNotFoundException: The file C:\development\cvstree\clojurescript\src\cljs\cljs%5ccore.cljs does not exist.
at cljs.compiler$compile_file.invoke(compiler.clj:1135)
at cljs.closure$compile_file.invoke(closure.clj:224)
at cljs.closure$eval1132$fn__1133.invoke(closure.clj:266)
at cljs.closure$eval1068$fn_1069$G1059_1076.invoke(closure.clj:187)
at cljs.closure$eval1119$fn__1120.invoke(closure.clj:280)
at cljs.closure$eval1068$fn_1069$G1059_1076.invoke(closure.clj:187)
at cljs.closure$eval1127$fn__1128.invoke(closure.clj:271)
at cljs.closure$eval1068$fn_1069$G1059_1076.invoke(closure.clj:187)
at cljs.closure$get_compiled_cljs.invoke(closure.clj:421)
at cljs.closure$cljs_dependencies.invoke(closure.clj:451)
at cljs.closure$add_dependencies.doInvoke(closure.clj:473)
at clojure.lang.RestFn.applyTo(RestFn.java:139)
at clojure.core$apply.invoke(core.clj:602)
at cljs.closure$build.invoke(closure.clj:712)
at user$eval1266.invoke(cljsc.clj:21)
at clojure.lang.Compiler.eval(Compiler.java:6406)
... 10 more

Comment by Brenton Ashworth [ 26/Jul/11 9:04 AM ]

Marko,

I did some work on Windows issues yesterday and found and fixed this problem as well as a few others. I will apply this patch and my fixes today.

Comment by Brenton Ashworth [ 26/Jul/11 12:53 PM ]

The following two commits fix this problem as well as all other problems that I found while trying the compile the sample apps on Windows within the Cygwin environment.

https://github.com/clojure/clojurescript/commit/add1dfcdbb68c2d98ff62f999c094f536c842f78
https://github.com/clojure/clojurescript/commit/d38729e66631bd649dc7c87f70e9b501d58da266

Comment by Marko Kocić [ 27/Jul/11 2:14 AM ]

I can confirm that compile now works correctly on windows.
I tested it in plain windows (no cygwin) using my patch for adding cmdline scripts which is attached to #CLJS-24





[CLJS-34] fns should hash by identity Created: 23/Jul/11  Updated: 29/Jul/11  Resolved: 29/Jul/11

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

Type: Enhancement Priority: Major
Reporter: Stuart Halloway Assignee: Stuart Halloway
Resolution: Completed Votes: 0
Labels: None

Approval: Ok

 Description   

currently hit -hash missing when you try to make them e.g. map keys.

I have committed a fix (https://github.com/clojure/clojurescript/commit/22a64ff17b343b6c61039fcb66fd9acf34d98522) which uses goog.getUid for function objects. While I don't like goog.getUid for arbitrary objects, I think it is a good fit for functions, which need to hash by identity, but only occasionally.



 Comments   
Comment by Rich Hickey [ 29/Jul/11 8:35 AM ]

ok by me





[CLJS-32] Implement range Created: 22/Jul/11  Updated: 23/Jul/11  Resolved: 23/Jul/11

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

Type: Defect Priority: Major
Reporter: Benjamin Teuber Assignee: Unassigned
Resolution: Duplicate Votes: 0
Labels: None


 Description   

I guess range is not yet implemented due to chunking etc. implementation, but a simple version would do for now.



 Comments   
Comment by Benjamin Teuber [ 22/Jul/11 7:29 PM ]

Sorry, duplicate - please delete..

Comment by Devin Walters [ 23/Jul/11 7:41 PM ]

See: http://dev.clojure.org/jira/browse/CLJS-6





[CLJS-49] ClojureScript breaks goog.fx.dom.BgColorTransform Created: 01/Aug/11  Updated: 03/Aug/11  Resolved: 03/Aug/11

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

Type: Defect Priority: Major
Reporter: Sergey Didenko Assignee: Brenton Ashworth
Resolution: Completed Votes: 0
Labels: None

Attachments: File bg-color.cljs     HTML File bg-color.html     GZip Archive test.tar.gz    

 Description   

The following JS snippet in Google Closure works. (It animates the background color of the elem):

var transform = new goog.fx.dom.BgColorTransform(elem, start, end, 2000);
return transform.play();

The following CLJS code:

(ns bgcolor
(:require [goog.fx.dom :as fx-dom]))

(defn ^:export animate [elem start end]
(let [anim (fx-dom/BgColorTransform. elem start end 2000)]
(.play anim)))

gets compiled to (the final part in whitespace mode)

goog.provide("bgcolor");goog.require("cljs.core");goog.require("goog.fx.dom");bgcolor.animate=function animate(elem,start,end){var anim__1960=new goog.fx.dom.BgColorTransform(elem,start,end,2E3);return anim__1960.play};goog.exportSymbol("bgcolor.animate",bgcolor.animate);

and looks ok but does not work neither in whitespace mode nor in advanced mode

See the attached files for full examples.



 Comments   
Comment by Sergey Didenko [ 01/Aug/11 5:33 PM ]

I used the wrong syntax for calling function without args.

Must be (.play anim ()) or (. anim (play))

Comment by Sergey Didenko [ 01/Aug/11 5:34 PM ]

Please close this, I can't find the way to do it.





[CLJS-38] add range function Created: 27/Jul/11  Updated: 28/Jul/11  Resolved: 28/Jul/11

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

Type: Enhancement Priority: Major
Reporter: Arthur Ulfeldt Assignee: Unassigned
Resolution: Duplicate Votes: 0
Labels: None

Attachments: File add-range-function.diff    
Patch: Code and Test

 Description   

implament the range function in core.cljs



 Comments   
Comment by Arthur Ulfeldt [ 27/Jul/11 3:54 AM ]

duplicate of CLJS-6.
moved the patch there.





[CLJS-59] destructured locals in loops are closed over incorrectly Created: 20/Aug/11  Updated: 23/Sep/11  Resolved: 23/Sep/11

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

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


 Description   

This is just like CLJS-22 except requires destructuring in the loop binding to trigger the error:

(loop [[_ i] [:dmy 0] j ()]
  (if (< i 5)
    (recur [:dmy (inc i)]
           (conj j (fn [] i)))
    (map #(%) j)))

ClojureScript incorrectly returns: (5 5 5 5 5)

Clojure correctly returns: (4 3 2 1 0)



 Comments   
Comment by David Nolen [ 23/Sep/11 1:08 AM ]

duplicate of CLJS-39





[CLJS-71] Add reify Created: 11/Sep/11  Updated: 13/Sep/11  Resolved: 13/Sep/11

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

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

Attachments: Text File 71-reify-2.patch     Text File 71-reify.patch    

 Comments   
Comment by David Nolen [ 12/Sep/11 1:54 AM ]

A patch to add reify.

Comment by David Nolen [ 13/Sep/11 12:51 AM ]

A much, much simpler patch that I think has all the desired properties. Leverages deftype since deftype has all the machinery required for dealing with fields, uses the namespace itself as the cache.

Comment by Rich Hickey [ 13/Sep/11 6:48 AM ]

I've made you a member - go ahead and apply this.

Comment by David Nolen [ 13/Sep/11 7:54 AM ]

Done, 791834558b61459ece6c124af6186580a65128a1





[CLJS-73] add :externs and :use-only-custom-externs options Created: 12/Sep/11  Updated: 14/Sep/11  Resolved: 14/Sep/11

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

Type: Task Priority: Major
Reporter: John Li Assignee: Brenton Ashworth
Resolution: Completed Votes: 0
Labels: None

Attachments: File external.js     File externs.diff     File externs.js     File test.cljs     HTML File test.html    

 Description   

One bit I'm not sure of is whether :use-only-custom-externs on nodejs should include "cljs/nodejs_externs.js". In the current patch, it doesn't. You would have to specify it manually.

:use-only-custom-externs is also untested.

I've attached a small test.
external.js defines external.fn(str) (which is just toUpperCase).
test.cljs defines test.main which calls external.fn.
test.html sources external.js and test.js (the advanced-compiled test.cljs) and calls external.fn directly and then via test.main.
externs.js gives an empty definition of external.fn.

Without the patch, (my) advanced-compiled test.js renames external.fn to external.K. When compiled with :externs (./bin/cljsc test.cljs '{:optimizations :advanced :output-to "test.js" :externs ["externs.js"]}'), external.fn is left alone.

Closure reference: https://code.google.com/closure/compiler/docs/api-tutorial3.html#mixed



 Comments   
Comment by John Li [ 12/Sep/11 11:20 PM ]

Adding rest of the test files.

Comment by Brenton Ashworth [ 14/Sep/11 3:39 PM ]

A patch for this was submitted under a different issue: CLJS-10.

I have applied that patch to the externs branch.

Please test your code with those changes to ensure that this works for you or to discover if more work needs to be done. If you run into problems, comment on that issue or on the Clojure dev mailing list.

Thanks for the patch and sorry for the confusion.

Comment by Brenton Ashworth [ 14/Sep/11 3:40 PM ]

Duplicate issue.





[CLJS-90] Browser repl does not work if the JVM working directory is not the CLJS directory Created: 14/Oct/11  Updated: 28/Oct/11  Resolved: 28/Oct/11

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

Type: Defect Priority: Major
Reporter: Luke VanderHart Assignee: Brenton Ashworth
Resolution: Declined Votes: 0
Labels: None
Environment:

MacOS 10.6.8, Java SE Runtime Environment (build 1.6.0_26-b03-384-10M3425)



 Description   

The browser repl hangs if the JVM working directory is not in the CLJS repository ($CLOJURESCRIPT_HOME). This is an odd and nonintuitive restriction.

For example, if my machine has $CLOJURESCRIPT_HOME set to ~/src/clojurescript, and I'm using emacs inferior lisp:

Dired to ~/src/clojurescript, set inferior-lisp-program to "script/repl", everything works fine.

Dired to ~/src/foo, set inferior-lisp-program to "../clojurescript/script/repl", the Clojure repl starts up fine, but the ClojureScript repl hangs.

This is not a classpath error: I have verified that the classpath is identical in both cases. Everything except the ClojureScript repl works in both cases.

Nor is it an error with the 'script/repl' startup script - I was initially using my own startup script in a completely separate directory, which is where I discovered the problem.



 Comments   
Comment by David Nolen [ 16/Oct/11 12:39 PM ]

Does this problem occur at the command line sans Emacs?

Comment by Brenton Ashworth [ 17/Oct/11 9:37 AM ]

I can't reproduce the problem.

Here is what I did:

  • Create directory foo next to clojurescript
  • cd foo & cp -a ../clojurescript/samples/repl/* .
  • In emacs
    • dired to foo
    • M-x set-variable
      • inferior-lisp-program
      • "../clojurescript/script/repl"
    • M-x run-lisp
  • In the REPL
    • (use 'cljs.closure)
    • (def opts {:output-to "main.js" :output-dir "out"})
    • (build "src" opts)
    • (require '[cljs.repl :as repl])
    • (require '[cljs.repl.browser :as browser])
    • (def env (browser/repl-env))
    • (repl/repl env)
  • Open foo/index.html in browser

At this point everything works.

I also tried doing the same from the command line, just running ../clojurescript/script/repl which also worked.

Comment by Luke VanderHart [ 28/Oct/11 1:59 PM ]

I've replicated your steps and it works.

There must be something more subtle and complex going on with my shell environment. I will try to isolate it - I'll let you know if there's anything in ClojureScript that needs to be fixed.

In the meantime, we can close this ticket as "cannot reproduce" since apparently it's only happening on some of my projects, reason TBD.

Thanks!

Comment by Luke VanderHart [ 28/Oct/11 2:00 PM ]

Cannot reproduce, suspect user error. Will re-open if ClojureScript does turn out to be at fault.





[CLJS-309] Bug - Typo in commit "Tagged literals in the CLJS compiler and first blush tests " Created: 08/Jun/12  Updated: 08/Jun/12  Resolved: 08/Jun/12

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

Type: Defect Priority: Major
Reporter: Raphaël AMIARD Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: bug

Attachments: Text File fix-data-readers-typo.patch    
Patch: Code

 Description   

I'm pretty sure there is a typo in this commit,

cljs-data-readers is declared, but then data-readers is bound in compile-file, which makes compilation fail

Patch attached



 Comments   
Comment by Fogus [ 08/Jun/12 8:43 AM ]

I'm unable to reproduce this. Do you mind posting the steps that you ran to see the error?
Thank you.

Comment by Raphaël AMIARD [ 08/Jun/12 10:48 AM ]

Sorry, i failed to follow the implications of CLJ-890, the bug was related to my configuration, and the binding valid.





[CLJS-58] ClojureScript places fully qualified keywords in the user namespace Created: 19/Aug/11  Updated: 05/Dec/11  Resolved: 16/Sep/11

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

Type: Defect Priority: Major
Reporter: Paul Michael Bauer Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None


 Description   

(ns testns)

(pr-str "kw: " ::kwtest " ns: " (namespace ::kwtest))
==> ("kw: " :user/kwtest "ns: " "user")

while in clojure:
(ns testns)

(pr-str "kw: " ::kwtest " ns: " (namespace ::kwtest))
==> ("kw: " :testns/kwtest "ns: " "testns")



 Comments   
Comment by Paul Michael Bauer [ 19/Aug/11 12:55 AM ]

https://groups.google.com/d/topic/clojure/LQYU55mPbHY/discussion

Comment by David Nolen [ 16/Sep/11 8:32 PM ]

duplicate of CLJS-74

Comment by Jeremy Hughes [ 16/Oct/11 6:21 AM ]

This affects more than just the repl.

Comment by Radford Smith [ 05/Dec/11 12:06 AM ]

Yeah, this seems to happen in normally compiled code. Am I missing something or does this need to be reopened?

Comment by David Nolen [ 05/Dec/11 8:21 AM ]

So you see the correct behavior at the REPL but not compiled code?





[CLJS-108] Colon characters in symbols don't look to be interpreted correctly in compiled code Created: 29/Nov/11  Updated: 05/Dec/11  Resolved: 04/Dec/11

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

Type: Defect Priority: Major
Reporter: Benjamin Conlan Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None
Environment:

OSX 10.7



 Description   

Compiled javascript source files are not outputting the colon character correctly. I have a feeling that this might have something to do with closure-latest (bootstrapped as of 28th Nov)

the cljs source

`(def colon-test [:symbol-name])`

outputs

`cljs.user.colon_test = cljs.core.Vector.fromArray(["﷐'symbol-name"]);`



 Comments   
Comment by Benjamin Conlan [ 29/Nov/11 6:03 AM ]

using: `(def colon-test [":symbol-name"])`
outputs: `cljs.user.colon_test = cljs.core.Vector.fromArray([":symbol-name"]);`

which as outlined in the differences from clojure (https://github.com/clojure/clojurescript/wiki/Differences-from-Clojure) stating that they are javascript string is a simple work around.

As such, this issue should be lowered to minor.

Comment by Benjamin Conlan [ 04/Dec/11 8:30 PM ]

I've just noticed on the mailing list this is identified using the

'
character also:

http://groups.google.com/group/clojure/browse_thread/thread/b4f0c335067ca74d/36d34b5d151d2ad0?lnk=gst&q=clojurescript+core#36d34b5d151d2ad0

The solution seems to be to use a page encoding of utf-8 (which is great when using javascript in a browser).

Comment by David Nolen [ 04/Dec/11 9:28 PM ]

Keywords and symbols are just JavaScript strings with a special unicode code character. ClojureScript requires the utf-8 meta tag to function properly in most browsers.

Comment by Benjamin Conlan [ 05/Dec/11 9:53 PM ]

Yes, again I've just found this link http://groups.google.com/group/clojure/browse_thread/thread/6613fcf1a9129c3a which clarifies the logic.

I noticed that closure has a `--charset=[charset-name]` flag which might help me out as i'm not playing with this stuff in a browser (at least for the moment).





[CLJS-99] Allow names from cljs.core to be properly resolved Created: 11/Nov/11  Updated: 15/Dec/11  Resolved: 15/Dec/11

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

Type: Enhancement Priority: Major
Reporter: Chris Gray Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: File cljs-name-resolving.diff     File cljs-name-resolving.diff    
Patch: Code

 Description   

It is not currently possible to redefine functions from cljs.core. For example, the following cljs:

(defn first [x]
1)

(first 2)

compiles to:

cljs.core.first = (function first{ return 1; });
cljs.core.first.call(null,2);

With the attached patch, it compiles to:

voronoi_diagram.rationals.first = (function first{ return 1; }
});
voronoi_diagram.rationals.first.call(null,2);

which is correct.



 Comments   
Comment by Chris Gray [ 11/Nov/11 6:26 PM ]

I have signed and sent the CA, though it was recently and might not be received yet.

Comment by Chris Gray [ 11/Nov/11 7:13 PM ]

It was also necessary to remove some macros that are also defined as functions in cljs.core, since I need to redefine some of these functions in my project.

I have not checked that all the macros that I removed are implemented as functions, but at first glance they appear to be.

Comment by David Nolen [ 20/Nov/11 1:26 PM ]

Thanks, but a patch to add compiler support for :refer-clojure :exclude is preferred. This would allow for the desired behavior w/o penalizing people who want fast arithmetic.

Comment by David Nolen [ 15/Dec/11 8:43 PM ]

This actually already works, you just need to exclude the symbol you want to redefine

code
(ns foo.bar
(:refer-clojure :exclude [first]))
code

Comment by David Nolen [ 15/Dec/11 8:43 PM ]

This actually already works, you just need to exclude the symbol you want to redefine

code
(ns foo.bar
(:refer-clojure :exclude [first]))
code





[CLJS-329] Cannot reference protocols using dot syntax (cljs.core.ILookup). Created: 29/Jun/12  Updated: 30/Jun/12  Resolved: 30/Jun/12

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

Type: Defect Priority: Major
Reporter: Kevin Lynagh Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None


 Description   

One used to be able to reference protocols using dot-separators; e.g.,

(satisifies? cljs.core.ILookup {:a 1}) ;;=> true

however, after CLJS commit 2aeb3d8f this is no longer possible.
These forms work correctly:

(satisfies? cljs.core/ILookup {:a 1})
(satisfies? ILookup {:a 1})

This issue came to my attention because core.match uses the cljs.core.ILookup form, so if that gets depreciated, downstream libraries will have to be updated.



 Comments   
Comment by Michał Marczyk [ 29/Jun/12 7:12 PM ]

Note that extend-protocol in Clojure expects the foo.bar/IQuux form (or IQuux in same namespace / after :use) and refuses to work with foo.bar.Quux. That's because in contrast to types and records, protocols actually expose a Var. The dotted name refers to the interface backing the protocol and seems to be an implementation detail.

So, I think the current behaviour is in line with how things are in Clojure.

Comment by Kevin Lynagh [ 29/Jun/12 8:47 PM ]

Matching the syntax of JVM Clojure sounds reasonable to me.
I take it I should close this issue and open up a ticket/patch core.match to use the correct syntax?

Comment by Kevin Lynagh [ 30/Jun/12 12:11 PM ]

Patch opened on downstream lib:

http://dev.clojure.org/jira/browse/MATCH-62





[CLJS-143] (hash obj-map) inappropriately depends on key order. Created: 03/Feb/12  Updated: 06/Feb/12  Resolved: 03/Feb/12

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

Type: Defect Priority: Major
Reporter: Kevin Lynagh Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 1a70326a25009cdddca56db0584bc157595ce1a9.patch    
Patch: Code and Test

 Description   

Seqing on obj-maps moves in key insertion order, which can lead to different hash values for maps with equal key/value pairs.
This issue is related to CLJS-118, and attached patch fixes it in the same way (i.e., seq in sorted key order).

Commit here

https://github.com/lynaghk/clojurescript/tree/143-hash-obj-map



 Comments   
Comment by David Nolen [ 03/Feb/12 7:05 PM ]

Fixed, https://github.com/clojure/clojurescript/commit/d9190cb4d4145a0cc1f0ebf0632e530a1a8b6023

Comment by Dave Sann [ 06/Feb/12 9:26 PM ]

There appears to still be an issue with this in the following scenario:

(ns test
  (:require [cljs.reader :as reader]))

(defn log [x]
  (.log js/console (pr-str x))
  x)

(def data
 {{:start 143  :end 144}  "data" })

(log {:data data})

(doseq [k (keys data)]  
  (let [ks (pr-str k)
        kr (reader/read-string ks)]
    (log  {:k      k 
           :v      (get data k)
           :k-hash (hash k)
           :read-k kr
           :read-k-hash (hash kr)
           :read-v (get data kr)})))

my output:

{:data {{:end 144, :start 143} "data"}}
{:k {:end 144, :start 143},
 :k-hash 40749209, 
 :read-k {:start 143, :end 144}, 
 :read-k-hash -1503612376, 
 :read-v nil, 
 :v "data"}

I have checked my cljs version and the patch has been applied.
can this be confirmed?

Comment by David Nolen [ 06/Feb/12 10:15 PM ]

Fixed, https://github.com/clojure/clojurescript/commit/e3854ccef19d1ceb427bb2ba7cd0ac3633dc7d0e





[CLJS-535] Update ClojureScript to use the new Google Closure releases from maven central Created: 08/Jul/13  Updated: 27/Jul/13  Resolved: 27/Jul/13

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

Type: Enhancement Priority: Major
Reporter: Stephen Nelson Assignee: Stuart Sierra
Resolution: Completed Votes: 0
Labels: None


 Description   

ClojureScript is currently using 20120710-r2029, which is several versions out of date. The most recent zip version available was released in December 2012 and it seems there won't be any more of that form as Google Closure has migrated to git and is pushing versions to maven central using a new version scheme (https://code.google.com/p/closure-compiler/wiki/Maven). The latest release is v20130603.

I think ClojureScript needs to switch to using the maven central releases, and it would be good to have a process/plan in place for getting more regular library updates.



 Comments   
Comment by Stephen Nelson [ 08/Jul/13 6:37 PM ]

I think I've misunderstood the difference between the Closure compiler and the Closure libraries. The compiler is being regularly release to central (as you are obviously aware - it's used in the build process) but the libraries don't seem to be being released at all since the switch to git. I can't find any references to a release plan for the closure libraries: this seems like something ClosureScript is going to need to deal with. There are several features that have been added to the libraries in the last year that would be 'nice to have'.

Comment by Jonas Enlund [ 09/Jul/13 12:33 AM ]

Here's a list of all closure-library releases: https://code.google.com/p/closure-library/downloads/list. The version ClojureScript is using today is r2029 from July 2012. The latest version is from February 2013.

I suspect they don't ship the closure-library to maven central since it doesn't contain any Java code.

Comment by David Nolen [ 16/Jul/13 6:20 AM ]

There's no process for staying up to date with closure-library beyond having the periodically bundle it and release them ourselves. More than happy to bump provided that someone tests that the latest versions of closure-libary don't break anything.

Comment by David Nolen [ 16/Jul/13 6:21 AM ]

Stuart, mind cutting new releases of the closure library so that people can test?





[CLJS-268] warn on calling type constructor with incorrect number of arguments Created: 23/May/12  Updated: 27/Jul/13  Resolved: 24/May/12

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

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

Attachments: Text File 0001-CLJS-268-warn-on-calling-type-constructor-with-wrong.patch     Text File 0002-fixed-type-constructor-argument-count-warnings.patch    

 Comments   
Comment by Brian Taylor [ 24/May/12 6:36 PM ]

0001

Added warning when type-constructor is invoked with the wrong number of warnings.

0002

The change in 0001 caused existing code to generate the newly created warning. This second patch addresses those new warnings.

Comment by David Nolen [ 24/May/12 7:00 PM ]

Man I love warnings Thank you!

Comment by David Nolen [ 24/May/12 7:02 PM ]

fixed, http://github.com/clojure/clojurescript/commit/fb240abd9a2bf99761d067709be772a8bc4dc0a9





[CLJS-269] (rest ()) returns nil Created: 23/May/12  Updated: 27/Jul/13  Resolved: 24/May/12

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

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

Attachments: Text File 0001-CLJS-269-rest-returns-nil.patch    

 Comments   
Comment by Brian Taylor [ 24/May/12 5:25 PM ]

(rest ()) now returns ().

Test (assert (= () (rest ()))) failed before change and passes now.

Comment by David Nolen [ 24/May/12 7:11 PM ]

Fixed, http://github.com/clojure/clojurescript/commit/0a26ec00a14104593a3731365dd83e6c9af75aa9





[CLJS-263] investigate special case for not in emit :invoke, could eliminate most uses of coercive-not=, coercive-not Created: 23/May/12  Updated: 27/Jul/13  Resolved: 24/May/12

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

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


 Description   

The use of coercive-not=, and coercive-not is getting a bit ugly. We could avoid this by checking for not invokes. If the argument is a bool-expr we can emit "!" directly on the argument. This would make core.cljs considerably more idiomatic.

Need to check that this doesn't affect perf.



 Comments   
Comment by David Nolen [ 24/May/12 8:31 PM ]

fixed, http://github.com/clojure/clojurescript/commit/af5dc7eff95c1e3af269a3fb999d8c4a74f8f147





[CLJS-265] Implement IChunk, ChunkedCons, ArrrayChunk, IChunkedSeq, ChunkBuffer Created: 23/May/12  Updated: 27/Jul/13  Resolved: 24/May/12

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

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


 Comments   
Comment by David Nolen [ 24/May/12 7:12 PM ]

work in progress here, http://github.com/clojure/clojurescript/compare/master...chunks

Comment by David Nolen [ 24/May/12 7:38 PM ]

fixed, http://github.com/clojure/clojurescript/commit/b4459bd8fd122602156aad44ded2e60f113b3853





[CLJS-278] ClojureScript macro eval bug since 0.0-1211? Created: 28/May/12  Updated: 27/Jul/13  Resolved: 31/May/12

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

Type: Defect Priority: Major
Reporter: Roman Scherer Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 0001-CLJS-278-bind-ns-to-a-real-namespace-not-symbol-duri.patch    

 Description   

As discussed on the Mailing List:

--------------------------------------------------------------

Hello ClojureScripters,

I'm trying to update a port of hiccup [1] to the latest
ClojureScript version. During HTML compilation I call at some
point eval (in a Clojure macro) on a datastructure and get a
java.lang.ClassCastException. This used to work in ClojureScript
versions before 0.0-1211.

I traced it down to the following example. This works in
ClojureScript 0.0-1011:

; Define a Clojure macro.
(defmacro eval-test [arg]
(eval arg))

; Use the Clojure macro from ClojureScript.
(eval-test "1")
;=> "1"
(eval-test 1)
;=> 1
(eval-test "div")
;=> "div"
(eval-test :div)
;=> :div
(eval-test [])
;=> []
(eval-test {})
;=> {}
(eval-test #{})
;=> #{}

In ClojureScript 0.0-1211 it works on strings, numbers and
symbols, but fails on vectors, maps and sets. I get the follwoing
results:

(eval-test "1")
;=> "1"
(eval-test 1)
;=> 1
(eval-test "div")
;=> "div"
(eval-test :div)
;=> :div
(eval-test [])

java.lang.ClassCastException: clojure.lang.Symbol cannot be cast to clojure.lang.Namespace, compiling:(NO_SOURCE_PATH:1)
at clojure.lang.Compiler.analyzeSeq(Compiler.java:6462)
at clojure.lang.Compiler.analyze(Compiler.java:6262)
at clojure.lang.Compiler.eval(Compiler.java:6508)
at clojure.lang.Compiler.eval(Compiler.java:6477)
at clojure.core$eval.invoke(core.clj:2797)
at hiccup.core$eval_test.invoke(core.clj:20)
at clojure.lang.AFn.applyToHelper(AFn.java:167)
at clojure.lang.AFn.applyTo(AFn.java:151)
at clojure.core$apply.invoke(core.clj:605)
at cljs.compiler$macroexpand_1.invoke(compiler.clj:1351)
at cljs.compiler$analyze_seq.invoke(compiler.clj:1368)
at cljs.compiler$analyze.invoke(compiler.clj:1425)
at cljs.compiler$analyze.invoke(compiler.clj:1418)
at cljs.repl$evaluate_form.invoke(repl.clj:64)
at cljs.repl$eval_and_print.invoke(repl.clj:124)
at cljs.repl$repl.doInvoke(repl.clj:173)
at clojure.lang.RestFn.invoke(RestFn.java:410)
at cljsbuild.repl.listen$run_repl_listen.invoke(listen.clj:10)
at cljsbuild.repl.listen$run_repl_launch.invoke(listen.clj:31)
at user$eval2490.invoke(NO_SOURCE_FILE:1)
at clojure.lang.Compiler.eval(Compiler.java:6511)
at clojure.lang.Compiler.eval(Compiler.java:6500)
at clojure.lang.Compiler.eval(Compiler.java:6501)
at clojure.lang.Compiler.eval(Compiler.java:6477)
at clojure.core$eval.invoke(core.clj:2797)
at clojure.main$eval_opt.invoke(main.clj:297)
at clojure.main$initialize.invoke(main.clj:316)
at clojure.main$null_opt.invoke(main.clj:349)
at clojure.main$main.doInvoke(main.clj:427)
at clojure.lang.RestFn.invoke(RestFn.java:421)
at clojure.lang.Var.invoke(Var.java:419)
at clojure.lang.AFn.applyToHelper(AFn.java:163)
at clojure.lang.Var.applyTo(Var.java:532)
at clojure.main.main(main.java:37)
Caused by: java.lang.ClassCastException: clojure.lang.Symbol cannot be cast to clojure.lang.Namespace
at clojure.lang.Compiler.currentNS(Compiler.java:6864)
at clojure.lang.Compiler.lookupVar(Compiler.java:6826)
at clojure.lang.Compiler.lookupVar(Compiler.java:6847)
at clojure.lang.Compiler.isInline(Compiler.java:6323)
at clojure.lang.Compiler.analyzeSeq(Compiler.java:6448)
... 33 more
java.lang.ClassCastException: clojure.lang.Symbol cannot be cast to clojure.lang.Namespace, compiling:(NO_SOURCE_PATH:1)

Does anyone have an idea what has changed between those versions?
Could this be a bug?

Thanks for your help, Roman.



 Comments   
Comment by David Nolen [ 30/May/12 8:09 AM ]

Patch welcome. I suspect this may be due to us binding cljs-ns to ns in compiler.clj line 1450. This was done to give file & line numbers for macro errors.

Comment by Brian Taylor [ 30/May/12 3:52 PM ]

macroexpand-1 is binding ns to a symbol but the Clojure compiler assumes ns will be a namespace. We find or create an appropriately named CLJ namespace for the parallel CLJS namespace being compiled.

Comment by Brian Taylor [ 30/May/12 4:15 PM ]

This may also address CLJS-152 (though I suspect we'll need a test case or feedback from the ticket creator to be sure.)

Comment by David Nolen [ 31/May/12 8:18 AM ]

fixed, 6eada144088a373fe5c2b1eed65d6d3a8d75cb37

Comment by Roman Scherer [ 31/May/12 10:14 AM ]

I can confirm that this patch solved my problem. Thanks for fixing this so fast ...





[CLJS-267] warn on undeclared protocols Created: 23/May/12  Updated: 27/Jul/13  Resolved: 25/May/12

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

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

Attachments: Text File 0001-CLJS-267-warn-on-undeclared-protocols.patch     Text File 0002-fixing-un-resolved-protocol-warnings.patch    

 Description   

While working in the chunks branch I could have saved a lot of time if I had a warning that I had written ICount instead of ICounted.



 Comments   
Comment by Brian Taylor [ 24/May/12 8:40 PM ]

Added a warning to extend-type when a referenced protocol's symbol does not exist (or is not actually a protocol.)

The second patch corrects the new warnings in pre-existing code that the first patch revealed.

Comment by Brian Taylor [ 24/May/12 8:54 PM ]

I ran the benchmark and found another new warning that I had missed.

Comment by Brian Taylor [ 24/May/12 9:06 PM ]

If accepted, these changes resolve CLJS-236 as well.

Comment by David Nolen [ 25/May/12 7:09 AM ]

fixed, http://github.com/clojure/clojurescript/commit/56da2c3ca3bde1e15faf6ff372da151cae9343db





[CLJS-236] extend-protocol should throw error for undefined arguments Created: 03/May/12  Updated: 27/Jul/13  Resolved: 25/May/12

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

Type: Defect Priority: Major
Reporter: Robert Nikander Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None


 Description   

This should throw an error:

(extend-protocol undefined js/Text (foo [x] x))

It currently does not, which means if you forget a require or mis-type a name, you get no error until later when you protocol is not there as expected.

(extend-protocol Missspellled js/Text (foo [x] x))



 Comments   
Comment by Brian Taylor [ 24/May/12 9:05 PM ]

This issue should be resolved once CLJS-267 is resolved as they both come down to the same additional checks in extend-type.

Comment by David Nolen [ 25/May/12 7:10 AM ]

fixed, http://github.com/clojure/clojurescript/commit/56da2c3ca3bde1e15faf6ff372da151cae9343db





[CLJS-271] cljs.compiler/resolve-var doesn't look in :use'd namespaces Created: 24/May/12  Updated: 27/Jul/13  Resolved: 25/May/12

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

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

Attachments: Text File 0001-resolve-var-learned-about-use-d-namespaces.patch    
Patch: Code

 Description   

This is the cause of issue CLJS-216.



 Comments   
Comment by David Nolen [ 25/May/12 7:14 AM ]

fixed, http://github.com/clojure/clojurescript/commit/468cf4e1f4bb708b8c7ee114af25c7d66dd41d33





[CLJS-216] Can't (:use) protocol from another namespace Created: 29/Apr/12  Updated: 27/Jul/13  Resolved: 25/May/12

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

Type: Defect Priority: Major
Reporter: Stuart Campbell Assignee: Unassigned
Resolution: Completed Votes: 1
Labels: None


 Description   

The ClojureScript compiler doesn't correctly resolve the namespace of implemented protocol methods when the protocol is referenced via `(:use)`.

E.g.

(ns foo)

(defprotocol SomeProtocol
  (some-function [this]))
(ns bar
  (:use [foo :only (SomeProtocol)]))

(defrecord SomeRecord
  SomeProtocol
  (some-function [_] :quux))

The protocol function is compiled as though SomeProtocol was defined in the "bar" namespace, e.g.:

bar.SomeType.prototype.bar$SomeProtocol$ = true;
bar.SomeType.prototype.bar$SomeProtocol$some_function = function(this$) {
  var this__14211 = this;
  return"\ufdd0'quux"
};

This example works as expected in Clojure.



 Comments   
Comment by Brian Taylor [ 24/May/12 10:03 PM ]

If accepted, the patch associated with CLJS-271 resolves this issue.

Comment by David Nolen [ 25/May/12 7:14 AM ]

fixed, http://github.com/clojure/clojurescript/commit/468cf4e1f4bb708b8c7ee114af25c7d66dd41d33





[CLJS-394] PersistentTreeSet lookup bug Created: 15/Oct/12  Updated: 27/Jul/13  Resolved: 15/Oct/12

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

Type: Defect Priority: Major
Reporter: Erik Ouchterlony Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: bug, patch

Attachments: File bugfix-PersistentTreeSet-lookup.diff    
Patch: Code and Test

 Description   

The lookup function in PersistentTreeSet behaves differently in clojurescript than in clojure when a custom comparison function is used. If two elements are evaluated as equal, one of then is in the set and we do a lookup on the other, clojure returns the element in the set, whereas clojurescript returns the lookup element.

(let [compare-quot-2 #(compare (quot %1 2) (quot %2 2))
s (sorted-set-by compare-quot-2 1)]
(get s 0))

Should return: 1
Returns: 0



 Comments   
Comment by David Nolen [ 15/Oct/12 10:02 PM ]

fixed, http://github.com/clojure/clojurescript/commit/409fcc9a824668207953b2bc47d40aaab85191d3





[CLJS-389] Compiler emits throw string Created: 07/Oct/12  Updated: 27/Jul/13  Resolved: 15/Oct/12

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

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


 Description   

http://github.com/clojure/clojurescript/blob/master/src/clj/cljs/compiler.clj#L468



 Comments   
Comment by David Nolen [ 15/Oct/12 9:59 PM ]

fixed http://github.com/clojure/clojurescript/commit/ddcb192be4c34a548cf669625fd6d1a5bf81aa9f





[CLJS-388] expose :output-wrapper compile option Created: 07/Oct/12  Updated: 27/Jul/13  Resolved: 15/Oct/12

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

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


 Comments   
Comment by Daniel Pittman [ 07/Oct/12 7:15 PM ]

I went looking for exactly this feature two days ago, as part of investigating ClojureScript as part of the "component model" for our single page application. It uses the "Asynchronous Module Definition" pattern to isolate code, as defined here: https://github.com/amdjs/amdjs-api/wiki/AMD

While it probably isn't the only headache, being able to easily produce an appropriate function definition around the ClojureScript code would make it much easier to get started here.

Comment by David Nolen [ 15/Oct/12 10:26 PM ]

fixed, http://github.com/clojure/clojurescript/commit/e83cf518ef4d3d6aa9a1cca18dc889efb0eae57f





[CLJS-275] self calls do not optimize Created: 25/May/12  Updated: 27/Jul/13  Resolved: 28/May/12

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

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

Attachments: Text File 0001-CLJS-275-optimize-self-calls.patch    

 Description   

.call is emitted for self calls. This should be addressed before tackling CLJS-273



 Comments   
Comment by Brian Taylor [ 27/May/12 8:39 PM ]

Added a second analysis pass over the method bodies for named fn's that includes the fn-var flag and the arity info. Self calls get optimized in this pass.

Comment by David Nolen [ 28/May/12 12:27 PM ]

fixed, http://github.com/clojure/clojurescript/commit/d07f2e1e777591338e6f2a3d1fb9a73e1e81edfd





[CLJS-444] def issues in REPL Created: 18/Dec/12  Updated: 27/Jul/13  Resolved: 18/Dec/12

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

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


 Description   

(def x) now fails in the REPL, this is very likely a result of CLJS-397 and CLJS-403



 Comments   
Comment by David Nolen [ 18/Dec/12 1:06 PM ]

This may be related to dead code elimination optimizations introduced in CLJS-397

Comment by David Nolen [ 18/Dec/12 2:49 PM ]

http://github.com/clojure/clojurescript/commit/ab868f2121b64e31c7206e3d17f209f63e8e6a51

Issue was not related to other mentioned tickets. No code was emitted if init not provided was the issue.





[CLJS-244] optimize list fn Created: 08/May/12  Updated: 27/Jul/13  Resolved: 28/May/12

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

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

Attachments: Text File 0001-CLJS-244-optimize-list-fn.patch    

 Comments   
Comment by Brian Taylor [ 27/May/12 10:39 PM ]

Before:

[], (list), 1000000 runs, 1247 msecs
[], (list 1 2 3), 1000000 runs, 4984 msecs

After:

[], (list), 1000000 runs, 19 msecs
[], (list 1 2 3), 1000000 runs, 797 msecs

After (with CLJS-275 fix):

[], (list), 1000000 runs, 18 msecs
[], (list 1 2 3), 1000000 runs, 645 msecs

Comment by David Nolen [ 28/May/12 1:06 PM ]

fixed, http://github.com/clojure/clojurescript/commit/b46995281acacd6eecd9c232ab2318ad34d1ac9b





[CLJS-274] type-hint the return value of seq, rest, next Created: 25/May/12  Updated: 27/Jul/13  Resolved: 28/May/12

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

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


 Description   

If we tag the return value of seq then we can avoid a lot of truth tests. It's simply too common in Clojure code to use if, when etc after a call to seq.



 Comments   
Comment by David Nolen [ 28/May/12 3:40 PM ]

fixed, http://github.com/clojure/clojurescript/commit/af3ec4b3acbd020979f70ed473014c0bcdd8cd99





[CLJS-30] Remove use of js* from demo app code Created: 22/Jul/11  Updated: 27/Jul/13  Resolved: 22/Jul/11

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

Type: Enhancement Priority: Major
Reporter: Alan Dipert Assignee: Alan Dipert
Resolution: Completed Votes: 0
Labels: None


 Description   

Twitterbuzz uses js* in several places to access objects in the browser, like Math and Regexp. js* shouldn't be used like this. Use goog.global.Math form instead.

goog.global works because gclosure compiler is loaded with dom/browser externs, and knows about these objects. They just need to be referred to the right way.



 Comments   
Comment by Alan Dipert [ 22/Jul/11 4:26 PM ]

goog.global.Math and goog.global.RegExp are provided by gclosure's default externs. They can be accessed with goog.global.x, where x is a global object in an extern'd environment.

RegExp, for reasons unknown, cannot be reliably accessed this way from cljs source. As a result, re-pattern in core is implemented on top of js*. For the possibly wrong but working solution, see https://github.com/clojure/clojurescript/blob/329fb01483452288c158195c94c95f94d9bd5cfd/src/cljs/cljs/core.cljs#L2375





[CLJS-39] doseq bindings not properly scoped Created: 28/Jul/11  Updated: 27/Jul/13  Resolved: 23/Sep/11

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

Type: Defect Priority: Major
Reporter: Remco van 't Veer Assignee: David Nolen
Resolution: Completed Votes: 1
Labels: None
Environment:

Ubuntu, Sun JDK 1.6, master at ba94ed7


Attachments: Text File cljs-39.patch    

 Description   

Example:

(def q (atom []))
(doseq [v [1 2 3]] (swap! q conj (fn [] v)))
(map #(%) @q))

Clojurescript: (3 3 3)
Clojure: (1 2 3)

Probably related to CLJS-22.



 Comments   
Comment by Rich Hickey [ 28/Jul/11 8:03 AM ]

Please recheck this now that CLJS-22 is fixed.

Comment by Remco van 't Veer [ 29/Jul/11 4:59 AM ]

Problem still exists:

$ git pull
Already up-to-date.
$ git log -1
commit 5b32f81c3ffc1fabb3cbd4376690118ffa9a8d75
Author: fogus <mefogus@gmail.com>
Date: Thu Jul 28 16:21:00 2011 -0400

Fixed FF's error related to #40. More testing needed.
$ ./script/repljs
#'user/jse
"Type: " :cljs/quit " to quit"
ClojureScript:cljs.user> (let [q (atom [])] (doseq [v [1 2 3]] (swap! q conj (fn [] v))) (map #(%) @q))
(3 3 3)

Comment by Remco van 't Veer [ 29/Jul/11 5:45 AM ]

Also offered as pull request on github: https://github.com/clojure/clojurescript/pull/7

Comment by John Li [ 14/Sep/11 12:57 AM ]

I was pretty confused when I ran into this.

Unfortunately I can't get the patch won't apply, even when I update to the revision you mention (ba94ed7). I'm rusty with git, so maybe I'm just doing something wrong. However, when I manually made the changes, my problem went away.

Comment by David Nolen [ 22/Sep/11 8:09 AM ]

I think the patch needs further work. doseq maps to loop/recur which is an efficient looping construct. By just wrapping the body of the loop in a function to preserve bindings is inefficient for many use cases. Better would be to analyze the body of loop to see if the bindings are involved in closures and only then wrap the body of the loop in a function.

Comment by David Nolen [ 22/Sep/11 8:49 AM ]

Another interesting approach would be to wrap the fns in a closure at their creation site. Probably much simpler to implement as well.

Comment by David Nolen [ 23/Sep/11 2:54 PM ]

Fixed, https://github.com/clojure/clojurescript/commit/17df9703f8933fa2f0037a2d8ad5185f620414a1

The issue was not loop/recur which communicates loop bindings to fns. The issue was that doseq wasn't keeping those bindings inside the loop binding list.





[CLJS-45] Browser-connected REPL Created: 29/Jul/11  Updated: 27/Jul/13  Resolved: 09/Sep/11

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

Type: Enhancement Priority: Major
Reporter: Brenton Ashworth Assignee: Brenton Ashworth
Resolution: Completed Votes: 0
Labels: None


 Description   

Create a REPL that uses the browser as the JavaScript runtime instead of Rhino.

  • send compiled JavaScript strings to the browser for evaluation
  • side effects happen in the browser, return values are sent back to the REPL
  • abstract over communication leveraging goog library
  • stub code
  • launch browser


 Comments   
Comment by Brenton Ashworth [ 09/Sep/11 4:38 PM ]

REPL code has been removed from the compiler. There is now a protocol for evaluation and implementations exist for Rhino and the browser.





[CLJS-57] namespace function in core.cljs returns :else instead of nil for a symbol without namespace Created: 10/Aug/11  Updated: 27/Jul/13  Resolved: 16/Sep/11

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

Type: Defect Priority: Major
Reporter: Arthur Edelstein Assignee: Brenton Ashworth
Resolution: Completed Votes: 0
Labels: None

Approval: Screened

 Description   

For example,

ClojureScript:cljs.user> (namespace pr)
:else



 Comments   
Comment by Brenton Ashworth [ 16/Sep/11 9:05 AM ]

This function works properly when passed a keyword or symbol. The current implementation is incorrect when passing something other than a keyword or symbol. It is weird that the (if) function allows three arguments without complaining.

In Clojure an exception is thrown if (namespace) is passed something other than a keyword or symbol. I think we should do the same here. I'm not sure why the thrown exception was commented out and nil is returned.

Comment by Brenton Ashworth [ 16/Sep/11 10:09 AM ]

The namespace function has been updated to return nil (instead of :else) when passed an argument of the incorrect type.





[CLJS-53] Implement defrecord Created: 05/Aug/11  Updated: 27/Jul/13  Resolved: 23/Sep/11

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

Type: Enhancement Priority: Major
Reporter: Tom Hickey Assignee: Tom Hickey
Resolution: Completed Votes: 0
Labels: None

Attachments: File 53-defrecord.diff    
Approval: Screened

 Description   
  • Protocols to support
    • IRecord
    • IHash
    • IEquiv
    • IMeta
    • IWithMeta
    • ILookup
    • ICounted
    • ICollection
    • IAssociative
    • IMap
    • ISeq
    • IPrintable


 Comments   
Comment by Tom Hickey [ 05/Aug/11 10:01 PM ]

The initial pass of this is up on remote branch 53-defrecord.

All supported protocols have been implemented. The main piece that is missing is defaulting __meta and __extmap to nil when the record ctor is called without those params (currently you get a warning when calling without those params). This breaks assoc and conj, and causes meta to return nothing, as opposed to nil.

Comment by Tom Hickey [ 25/Aug/11 2:59 PM ]

I think I've been working on this in a vacuum too long, and could really use a review of the work I've done. I've included a patch to make it easier for whomever takes a look. I had a few questions about my approach:

Is creating a new emit method for :defrecord* in the compiler kosher, or should I have worked this into the :deftype* method?

The quoting of the impls for extend-type in the emit-defrecord fn seem awkward to me. Feels like I'm doing something wrong.

I used gclosure @param jsdocs to avoid warnings from the 2 different ctor "arities", is that okay?

Comment by David Nolen [ 23/Sep/11 1:27 AM ]

Fixed, https://github.com/clojure/clojurescript/commit/e380f80c4fd34629d519e3aa5ed0ab33b5dfb0a3





[CLJS-43] Targeting nodejs fails after 954e8529b1ec814f40af77d6035f90e93a9126ea Created: 28/Jul/11  Updated: 27/Jul/13  Resolved: 30/Aug/11

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

Type: Defect Priority: Major
Reporter: Anthony Simpson Assignee: Christopher Redinger
Resolution: Completed Votes: 3
Labels: None


 Description   

I first noticed this morning after I deleted the out/ folder in the project I was working on, forcing new compilation (I hadn't done this since before the relevant commit). I did a little research, and apparently this commit has broken the nodejs target: https://github.com/clojure/clojurescript/commit/954e8529b1ec814f40af77d6035f90e93a9126ea

After that commit, any attempt at using nodejs (even the hello world sample that ships with ClojureScript) results in this error:

TypeError: Cannot read property 'prototype' of undefined
    at Object.<anonymous> (/Users/anthony/code/clojurescript/nodeh.js:1685:17)
    at Module._compile (module.js:402:26)
    at Object..js (module.js:408:10)
    at Module.load (module.js:334:31)
    at Function._load (module.js:293:12)
    at Array.<anonymous> (module.js:421:10)
    at EventEmitter._tickCallback (node.js:126:26)

The relevant JavaScript generated is this line:

goog.global.Date.prototype.cljs$core$IEquiv$ = !0;
on line 1685. It seems to be related to this issue: http://dev.clojure.org/jira/browse/CLJS-25 and it only happens after the commit linked above.

I don't know if this is happening on Linux systems, but the reports appear to be from Windows and OS X users. I'm running OS X 10.6.8 myself. Probably not relevant, but I figured I'd throw that out there anyway.



 Comments   
Comment by Ricardo da Silva Lima [ 04/Aug/11 9:11 AM ]

I found the same error on Linux. It's an Ubuntu 11/04 with node.js v 0.4.10 and Clojurescript commit 7a179189f6114b0620875d3d521789472ec04aa3.





[CLJS-139] Internet Explorer treats \uFDD0 up to \uFDEF as equal so some keyword and symbol related things do not work Created: 31/Jan/12  Updated: 27/Jul/13  Resolved: 25/Feb/12

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

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

Clojurescript HEAD, tested ond IE7 and IE8


Attachments: Text File 139_fix_unicode_emit.patch     Text File cljs-139+133_symbol+keyword-fix.patch     Text File CLJS_139.patch     Text File general-escaping-emit-constant.patch     Zip Archive unicodetest.zip    

 Description   

Internet Explorer treats \uFDD0 up to \uFDEF as equal. e.g. '(= \uFDD0 \uFDD1)' returns true.
Therefore e.g. '(symbol? :whatever)', '(symbol? (keyword "whatever"))' and so on return true which obviously shouldn't happen.
Further on read-string does not correctly unmarshal keywords because of that issue.
Using pr-str on that unicode range returns ":" for all codes.
All other non IE Browsers I've tested (Firefox, Chrome/Chromium, Opera, iOS BRowser, etc...) do not expose this problem and work as expected.

Some context: http://stackoverflow.com/questions/5188679/whats-the-purpose-of-the-noncharacters-ufdd0-to-ufdef



 Comments   
Comment by David Nolen [ 31/Jan/12 10:24 AM ]

Do you have a proposed solution?

Comment by Thomas Scheiblauer [ 01/Feb/12 3:17 AM ]

Maybe change the keyword and symbol identifiers to \uFDF0 and \uFDF1 (or any other cross browser accepted value) respectively?
I will try that and report back any success or failure.

Comment by Thomas Scheiblauer [ 01/Feb/12 5:37 AM ]

My proposed solution, replacing \uFDD0 and \uFDD1 with \uFDF0 and \uFDF1 respectively works on all tested Browsers (Firefox9, Chromium17, iPAD/iOS-5.0.1, Epiphany AND IE7 + IE8).
I used this bash command line inside the "src" folder to do the replacement (after checking that the replacements will be appropriate):
for src in $(grep -R -i -l 'fdd' .); do sed -i -e 's/FDD/FDF/g' $src; done

Comment by David Nolen [ 01/Feb/12 10:53 AM ]

Thanks, will look into this.

Comment by David Nolen [ 03/Feb/12 7:14 PM ]

Please apply this patch and confirm that it works for you. Thanks!

Comment by Thomas Scheiblauer [ 06/Feb/12 8:49 AM ]

It works, thank you!

Comment by David Powell [ 06/Feb/12 12:04 PM ]

Can anyone confirm this problem?

In IE8, in Javascript:

('\ufdd0' === '\ufdd1')

returns false.

Comment by David Nolen [ 06/Feb/12 5:08 PM ]

David Powell, so you're not seeing the same issue?

Comment by Thomas Scheiblauer [ 07/Feb/12 8:49 AM ]

I just did another test where I compared the two values using Javascript directly and one using compiled Clojurescript with only an alert outputting the result of the same test. These two tests produced the correct results (as David Powell observed) even in the IEs. So it seems that there happens something in the context of my application, maybe it's caused by one of the included js libs (I'm also using jquery, jquery.mobile and ckeditor... though I've already included these in the testing environment, uhmmm...). I will have to investigate that further. I'll post another comment when I've found out something.

Comment by Thomas Scheiblauer [ 08/Feb/12 4:29 AM ]

Intermediate result of my investigations:
It only happens when I turn Clojurescript optimizations OFF!!!
As soon as I activate any compiler optimizations even if it's only set to "simple", the problem will not manifest itself on IE.
It also seems not to depend on any of my included third party libs.

Comment by David Nolen [ 08/Feb/12 7:59 AM ]

Are you using your own code or one of the examples that ships with ClojureScript?

Comment by Thomas Scheiblauer [ 08/Feb/12 11:50 AM ]

I'm using my own code.

I attached a test that illustrates the described problem with IE (at least on IE7 and IE8, I still couldn't test more recent IE versions).
The result of the equations should always be "false" which isn't the case with non optimized, compiled ClojureScript within the unicode range \uFDD0 up to \uFDEF while it works e.g. for \uFDF0 and \uFDF1.
"unicodetest.html" uses the the non optimized code which exposes the bug; it was compiled using: cljsc unicodetest.cljs > unicodetest.js
"unicodetest_simpleopt.html" uses the "simple" optimized code which does not expose the bug; it was compiled using: cljsc unicodetest.cljs '{:output-dir "simpleopt_out" :optimizations :simple}' > unicodetest_simpleopt.js
Using advanced optimization also produces correct results even on IE.

The code was compiled using the following ClojureScript version and dependencies:
ClojureScript from git (commit 1028ca12f169322e31d7967d8c385165cf773665)
Google closure library r1487
Google closure compiler r1592
guava 10.0.1
rhino 1.7R3

running on:
Gentoo Linux x86_64, Linux kernel 3.2.5
Java SE 1.6.0_30
Clojure 1.3.0

The problem was observed on:
Windows XP (SP 2)
using IE7 and IE8
(it did not occur on any other non-IE browser I've tested: Firefox, Chromium, iPad Browser, Epiphany and Opera regardless of the OS they were running on)

Comment by Thomas Scheiblauer [ 08/Feb/12 4:18 PM ]

Looking at the compiled code I see that in the version without optimizations the Unicode characters are converted from the UTF-16 \u notation to their UTF-8 byte values (3 bytes); look here: http://www.fileformat.info/info/unicode/char/fdd0/index.htm
... while in the optimized version the \u notation stays intact.
That of course does not explain why IE treats strings containing the byte sequences 0xEFB790 up to 0xEFB7AF as equal but it explains why there is a difference between the optimized and non optimized compiler output and also why it works in IE when coding the equality test directly in Javascript using the \u notation.

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

I suspect this because of improperly escaped Java strings - looking into it.

Comment by David Nolen [ 16/Feb/12 9:19 AM ]

This one avoids emitting unicode chars, please test

Comment by Thomas Scheiblauer [ 22/Feb/12 8:27 AM ]

I've just tested your new fix which prevents the compiler from converting unicode literals to their respective UTF-8 representations but now I encounter the bug when using read-string as described here: CLJS-133
Again only on IE and only when not using any optimization during compilation.

Comment by Thomas Scheiblauer [ 23/Feb/12 8:22 AM ]

I have attached a new patch which should fix all problems related to this issue (including CLJS-133 which is related) so far (works for me). It includes David's partial fix so only this one is required.
However, to prevent possible related future issues it should probably be considered to escape every non-ascii character in emitted strings and characters because IE obviously HAS severe problems with handling unescaped unicode.

Comment by David Nolen [ 23/Feb/12 10:44 AM ]

This seems less ideal than just solving this once and for all. I'm assuming many people will want to emit unicode chars and want them to emit properly and work under development in IE. When we emit strings we should rebuild the string replacing unicode chars. This should be done efficiently as possible.

Comment by Thomas Scheiblauer [ 23/Feb/12 11:07 AM ]

Sure, that's what I wanted to express with my last sentence. I'm currently exploring possible solutions to find the most efficient one.

Comment by Thomas Scheiblauer [ 23/Feb/12 12:48 PM ]

I have just attached my first stab at a general non-ascii escape patch for emit-constant. It should already be pretty efficient. I basically took the core from Stuart Sierra's clojure/data.json write-json-string function (to not having to reinvent the wheel), optimized it a bit (hopefully) and integrated it properly into compiler.cljs.

Comment by Thomas Scheiblauer [ 24/Feb/12 4:11 AM ]

Just to nip questions regarding the emittance of regular expressions in the bud, I've just tested this and it behaves correctly without "manual" intervention. More precisely, escaped unicode in regular expressions does apparently not get converted to utf-8 (or whatever character set) when applying ".toString" or "str" to them.

Comment by David Nolen [ 24/Feb/12 9:24 PM ]

This patch is good but could you please create a patch with git that includes proper attribution information. This is a good guide - http://ariejan.net/2009/10/26/how-to-create-and-apply-a-patch-with-git

Comment by Thomas Scheiblauer [ 25/Feb/12 2:54 AM ]

Ok, I have replaced the patch with one created according to the instructions in the guide you pointed me at.

Comment by David Nolen [ 25/Feb/12 10:25 AM ]

Fixed, https://github.com/clojure/clojurescript/commit/965dc505229652558adcb526ecb5a9f91ce31ce2





[CLJS-63] reader/read-string doesn't like strings with escaped quotes Created: 29/Aug/11  Updated: 27/Jul/13  Resolved: 02/Sep/11

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

Type: Defect Priority: Major
Reporter: Matthew Gilliard Assignee: Christopher Redinger
Resolution: Completed Votes: 0
Labels: None

Attachments: File escaped-chars-fix.diff    
Patch: Code and Test
Approval: Ok

 Description   

1/ think of a string, eg: say "ahh"
2/ now make it into a clojure string: "say \"ahh\""
3/ now call pr-str on it: "\"say \\\"ahh\\\"\""

I would expect to be able to use read-string to turn it back into the string in step 2:

(reader/read-string "\"say \\\"ahh\\\"\"")

But this chokes. The stack-trace in chrome is:

Uncaught TypeError: Object " has no method 'append'
cljs.reader.read_string hello.js:495
cljs.reader.read hello.js:506
cljs.reader.read_string hello.js:506
(anonymous function)



 Comments   
Comment by Matthew Gilliard [ 29/Aug/11 5:22 PM ]

This works as expected using clojure-core read-string on the JVM.

Comment by Christopher Redinger [ 02/Sep/11 10:14 PM ]

This patch was contributed by John Li: https://groups.google.com/d/topic/clojure-dev/l5222NuBmyc/discussion





[CLJS-281] Cache hash on records, use hash-imap Created: 28/May/12  Updated: 27/Jul/13  Resolved: 28/May/12

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

Type: Enhancement Priority: Major
Reporter: Michał Marczyk Assignee: Michał Marczyk
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 0001-CLJS-281-cache-hash-on-records-use-hash-imap.patch    

 Description   

Records don't currently cache their hash values, nor do they use the map-specific hashing algorithm (in contrast to Clojure's records which share their hashing algo with Clojure's maps). The patch to be attached in a moment implements hash caching and wires in hash-imap.



 Comments   
Comment by David Nolen [ 28/May/12 7:24 PM ]

fixed, http://github.com/clojure/clojurescript/commit/8fb130b1627688c1cc4860c7db6e137475623738





[CLJS-280] ci-reduce and array-reduce repeatedly find count of coll Created: 28/May/12  Updated: 27/Jul/13  Resolved: 28/May/12

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

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

Attachments: Text File 0001-ci-reduce-array-reduce-call-count-once.patch    
Patch: Code

 Description   

BEFORE

[coll (range 1000000)], (reduce + 0 coll), 1 runs, 367 msecs
[coll (into [] (range 1000000))], (reduce + 0 coll), 1 runs, 1284 msecs

AFTER

[coll (range 1000000)], (reduce + 0 coll), 1 runs, 269 msecs
[coll (into [] (range 1000000))], (reduce + 0 coll), 1 runs, 739 msecs



 Comments   
Comment by Brian Taylor [ 28/May/12 6:56 PM ]

Note: This is not related to CLJS-279.

Comment by David Nolen [ 28/May/12 7:22 PM ]

fixed, http://github.com/clojure/clojurescript/commit/3e41ee0d14732958c5b845378f8b955d6fcc5093





[CLJS-445] declare stopped working in the REPL Created: 18/Dec/12  Updated: 27/Jul/13  Resolved: 18/Dec/12

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

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


 Description   

Typing a declare statement in the REPL gives an error



 Comments   
Comment by David Nolen [ 18/Dec/12 12:36 PM ]

duplicate





[CLJS-62] memoize is not working, function arguments cannot be used as a key in hashmaps Created: 29/Aug/11  Updated: 27/Jul/13  Resolved: 12/Oct/11

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

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

ubuntu



 Description   

memoize is not working and the reason is that function arguments throws an exception if used as a key in a map.

Following example throws an exception in clojurescript, but works in clojure:

(assoc {} ((fn [& args] args) 1) 2)

using clojurescript from 2011/08/23, commit 91be59bb891b3635899e



 Comments   
Comment by David Nolen [ 24/Sep/11 11:19 AM ]

Fixed, https://github.com/clojure/clojurescript/commit/fe1dcdcf14d3dbbf0eca55f3e795ea289abf88d5

Comment by David Nolen [ 12/Oct/11 1:22 PM ]

(assoc {} ((fn [& args] args) 1 2) 3)

Throws the error

Comment by David Nolen [ 12/Oct/11 10:10 PM ]

Fixed, https://github.com/clojure/clojurescript/commit/d95353182860e1345b6a52933cb431b3eedade2c





[CLJS-76] satisfies? macro should double check that the property is not on the object itself Created: 18/Sep/11  Updated: 27/Jul/13  Resolved: 23/Sep/11

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

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

Attachments: Text File 76-fix-satisfies.patch    
Approval: Screened

 Description   

The current behavior means that prototypes are not printable at the REPL and the resulting error is quite confusing.

(satisfies? IPrintable (.prototype Vector)) ; => true



 Comments   
Comment by David Nolen [ 23/Sep/11 1:28 AM ]

Fixed, https://github.com/clojure/clojurescript/commit/f1df6bcd9891f71d2884a3a79ecac1680ecafc3e





[CLJS-72] goog-depenencies* parsing fails on compiler generated goog/deps.js Created: 12/Sep/11  Updated: 27/Jul/13  Resolved: 14/Sep/11

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

Type: Defect Priority: Major
Reporter: Hugo Duncan Assignee: Brenton Ashworth
Resolution: Completed Votes: 0
Labels: None

Attachments: File fix-goog-dependencies.diff    
Patch: Code

 Description   

When using a closure compiler generated goog/deps.clj, goog-dependencies* fails to parse anything from the file. This is due to the compiler writing the dependencies using double quotes for the first argument to addDepenendency.

The attached patch modifies the regex used to match either single or double quotes.






[CLJS-75] Throw Error objects instead of string literals in core.cljs Created: 16/Sep/11  Updated: 27/Jul/13  Resolved: 27/Sep/11

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

Type: Defect Priority: Major
Reporter: Brenton Ashworth Assignee: Brenton Ashworth
Resolution: Completed Votes: 0
Labels: None


 Description   

For historical reasons, most of the exceptions thrown in core.cljs are throwing string literals. Update all of these to throw Error objects:

(throw (js/Error. "message"))

If an exception is thrown as a string when using the browser-connected REPL, it will not be caught and returned and will cause the REPL to freeze.






[CLJS-74] Namespaced keywords always resolve to user Created: 15/Sep/11  Updated: 27/Jul/13  Resolved: 16/Sep/11

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

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

Approval: Screened

 Description   

If we're in the namespace foo.bar at the REPL and we enter ::cool we get :user/cool, instead of :foo.bar/cool



 Comments   
Comment by Brenton Ashworth [ 16/Sep/11 10:46 AM ]

ClojureScript uses the Clojure reader. Updated the REPL to bind the current namespace to ns while reading.

Comment by Fogus [ 22/Sep/11 12:57 PM ]

Patch occurred at https://github.com/clojure/clojurescript/commit/a4918d63200e83f1799939799bf11827d2e3b3b7





[CLJS-283] Fix protocols & protocol method names in reducers/Cat Created: 29/May/12  Updated: 27/Jul/13  Resolved: 29/May/12

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

Type: Defect Priority: Major
Reporter: Michał Marczyk Assignee: Michał Marczyk
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 0001-CLJS-283-fix-protocol-protocol-method-names-in-reduc.patch    

 Description   

Patch coming in a moment.



 Comments   
Comment by David Nolen [ 29/May/12 4:40 PM ]

fixed, http://github.com/clojure/clojurescript/commit/f454d4a5ab3924744b583604d01360ba9baf4efe





[CLJS-419] Exclude cljs source file from compilation Created: 17/Nov/12  Updated: 27/Jul/13  Resolved: 18/Dec/12

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

Type: Enhancement Priority: Major
Reporter: Mimmo Cosenza Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: enhancement
Environment:

every environment


Attachments: Text File 0001-CLJS-419-exclude-file-or-dir-from-compilation.patch    

 Description   

Scenario:

  1. you have a :dev build and a :prod build
  2. In the :dev build you want to have a brepl connected with the browser and you coded that connection in a isolated cljs source file.
  3. In the :prod build you do not want that connection active, meaning you don't want source cljs file enabling the connection to be included in the compilation.

Given this scenario, you need to duplicate all the cljs source files but the one that enable the connection. This mean you have to manually maintain two code bases.

It could be useful to have a way to :exclude some files from :source-path.



 Comments   
Comment by David Nolen [ 18/Nov/12 3:16 PM ]

This could easily be done by adding support for this in closure.clj - patch welcome!

Comment by Mimmo Cosenza [ 20/Nov/12 6:11 AM ]

I propose to add a new keyword/value in the optimization option map, namely :exclude-path. the value of this option should be a subdir of the source-dir.

Top level scenario:

1. you use lein-cljsbuild to define your cljs project
2. you define more builds, e.g. a :dev build and a :prod build.
(defproject ...
:cljsbuild {:builds
{:dev {:source-path "src/cljs" {:compiler {:output-to "resources/public/js/main_dbg.js"
:optimizations :whitespace
:pretty-print true}}}
:prod {:source-path "src/cljs" {:compiler {:output-to "resources/public/js/main_dbg.js"
:optimizations :advanced
:exclude-path "some-path"}}}})

3. lein-cljsbuild plugin will instruct CLJ compiler by passing it the soruce-dir (e.g. "src/cljs") and the options map which now can contain also an optional :exclude-path keyword.

4. During compilation the compiler will exclude from source-dir any cljs source which is contained in the named excluded directory.

I'll start bottom-up from 4. then I'll try to patch lein-cljsbuild too.

Mimmo

Comment by Mimmo Cosenza [ 07/Dec/12 10:30 AM ]

Hi David, here is the flattened patch relative. The two guys which worked on the patch under my direction are interns in my own company. Next monday we'll send their signed CA.

My best
Mimmo

Comment by Brenton Ashworth [ 07/Dec/12 11:13 AM ]

In general, we should not complicate the compiler with additional options when functionality can be provided by external tools.

I think this feature can be provided by external tools. The compiler will only automatically pull in things that are actually dependencies of the sources that we provide to the compiler. External tools should provide ways to limit what is handed to the compiler.

I would first attempt to modify lein-cljsbuild to do what you want.

When using the compiler directly, you can provide your own implementation of Compilable which, when given a directory, will filter out sources based on some criteria you provide. In my projects I have custom implementations of Compilable that do just this. You should be able to do the same thing in lein-cljsbuild.

-Brenton

Comment by Mimmo Cosenza [ 07/Dec/12 12:30 PM ]

Thanks for the advice Brenton. I'll try to understand from the maintainer of `lein-cljsbuild` where to start from. I agree with you about keeping the compiler clean from options that can be implemented by the tools. But I'm no so sure that patching lein-cljsbuild we'll be as easy as adding `:exclude` option to the compiler.

Mimmo

Comment by Brenton Ashworth [ 07/Dec/12 1:04 PM ]

It doesn't matter which one is easier to do. Every new option and special case that we add to the compiler makes it more difficult to understand how changes will impact users.

Comment by David Nolen [ 07/Dec/12 5:40 PM ]

I agree that anything that can be solved at higher level tools is better - it wasn't clear to me from the implementation that Compilable could be used to control this - but I see now.

Mimmo, cljs.closure/build takes a Compilable and a map of options. So lein-cljsbuild could construct the custom Compilable that understands :excludes and pass it along.

Comment by Evan Mezeske [ 09/Dec/12 9:48 PM ]

FWIW, I agree with Brenton that this should be in lein-cljsbuild.

I didn't know that cljs.closure/build was flexible enough to do this already. I always thought that it needed to be extended so that a vector of files could be passed in, or something, but it sounds like the Compilable approach should work just fine.

I will happily accept a patch for this. One thing to keep in mind, though, is that the :exclude entry should not be in the :compiler map if lein-cljsbuild is handling it. The :compiler map is passed straight through as options to cljs.closure/build. So, the :exclude entry should be a neighbor of the :compiler entry.

Comment by Mimmo Cosenza [ 10/Dec/12 4:20 AM ]

Hi all,
we all agree with Brenton and yes, the :exclude option has to be at the same hierarchical level of :source-path. I'll verify if we can also extend :source-path from taking a String to taking a vector of String, in such a way that the user could ask to compile more cljs src-dir.

Mimmo

Comment by Mimmo Cosenza [ 10/Dec/12 4:37 AM ]

Hi all,
just a small add on to the previous comment. I don't think we're going to update cljsc/cljsc.clj, which I consider a kind of tool, much more fragile and limited than cljsbuild plugin, to interoperate with CLJS compiler.

My best

Mimmo

Comment by David Nolen [ 18/Dec/12 3:09 PM ]

Should be resolved by tools that use the compiler.





[CLJS-446] Rhino REPL emits warnings twice Created: 18/Dec/12  Updated: 27/Jul/13  Resolved: 18/Dec/12

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

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


 Description   

This only affects functions - functions get a second pass from the compiler in order to leverage arity information for self call optimization.



 Comments   
Comment by David Nolen [ 18/Dec/12 3:38 PM ]

fixed http://github.com/clojure/clojurescript/commit/cd89461082c74c9b7d3429dfa34ee9344531690d





[CLJS-36] Implement defmulti, defmethod Created: 25/Jul/11  Updated: 27/Jul/13  Resolved: 29/Jul/11

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

Type: Enhancement Priority: Major
Reporter: Frank Failla Assignee: Frank Failla
Resolution: Completed Votes: 0
Labels: None


 Comments   
Comment by Frank Failla [ 29/Jul/11 3:07 PM ]

Fogus is reviewing this, will close issue after his review





[CLJS-87] Inconsistent select-keys Created: 13/Oct/11  Updated: 27/Jul/13  Resolved: 13/Oct/11

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

Type: Defect Priority: Major
Reporter: Murphy McMahon Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None
Environment:

Linux



 Description   

In the Clojure REPL:
(select-keys {:foo 100 :bar false :baz true} [:foo :bar])
=> {:foo 100 :bar false}

In the ClojureScript REPL:
(select-keys {:foo 100 :bar false :baz true} [:foo :bar])
=> {:foo 100}



 Comments   
Comment by David Nolen [ 13/Oct/11 10:25 PM ]

Fixed, https://github.com/clojure/clojurescript/commit/3ab435beca34eac0c5ce239f384bd29f31180224





[CLJS-432] Include line and file information in error messages Created: 28/Nov/12  Updated: 27/Jul/13  Resolved: 30/Nov/12

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

Type: Enhancement Priority: Major
Reporter: Brandon Bloom Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: patch, patch,

Attachments: Text File CLJS-432-v001.patch     Text File CLJS-432-v002.patch     Text File CLJS-432-v003.patch     Text File CLJS-432-v004.patch     Text File CLJS-432-v005.patch     Text File CLJS-432-v006.patch    
Patch: Code

 Description   

Just as the ana/warning function did, now errors and assertions include line and file source information. I needed this sorely.



 Comments   
Comment by Brandon Bloom [ 28/Nov/12 6:13 PM ]

Added v2 of patch that will handle any exception during parsing. Similar could be done during code generation. This approach seems much more robust and less invasive.

Comment by Brandon Bloom [ 28/Nov/12 11:15 PM ]

That v2 was made hastily. Upon further thought, it seems broken in the face of recursive analysis. I think the right thing, if we prefer the exception catching approach, is to rethrow as a custom exception type, which would be allowed through un-re-wrapped.

For example:

(try
  (parse-form ...)
  (catch AnalysisError e
    throw e)
  (catch Throwable e
    (throw (AnalysisError. env e))))
Comment by David Nolen [ 29/Nov/12 11:32 AM ]

I don't have a problem with the approach in the first patch. I don't really see how a less invasive patch is even possible - you still need to pass the environment to some assertion check if you are going to throw a custom exception as well.

Comment by Brandon Bloom [ 29/Nov/12 2:28 PM ]

v3 of patch fixes issue with v2

Comment by Brandon Bloom [ 29/Nov/12 2:50 PM ]

v4 of patch as discussed in irc

Comment by Brandon Bloom [ 29/Nov/12 2:57 PM ]

v5 also catches error from parse-invoke

Comment by Brandon Bloom [ 29/Nov/12 5:05 PM ]

v6 improves errors in repl as discussed in IRC

Comment by David Nolen [ 30/Nov/12 11:25 AM ]

fixed, http://github.com/clojure/clojurescript/commit/af4ab91754d30f082b117b40c07ea94d0063c0d6





[CLJS-85] cljs.reader/read-string does not handle ":a:b:c" correctly Created: 11/Oct/11  Updated: 27/Jul/13  Resolved: 12/Oct/11

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

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


 Comments   
Comment by David Nolen [ 12/Oct/11 7:58 PM ]

Fixed, https://github.com/clojure/clojurescript/commit/d84bd9ae57a164d83e76911c10a88fad9c1d3666





[CLJS-84] Behavior of (str 'foo) differs between Clojure and ClojureScript Created: 11/Oct/11  Updated: 27/Jul/13  Resolved: 12/Oct/11

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

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

google closure / chrome
clojure 1.3 / jdk 1.6 / osx


Attachments: Text File 84-str-on-symbol-keyword.patch    
Patch: Code

 Description   

In ClojureScript (str 'foo) evaluates to a symbol, whereas in Clojure (str 'foo) evaluates to a string.

ClojureScript:cljs.user> (str 'foo)
foo

user> (str 'foo)
"foo"



 Comments   
Comment by David Nolen [ 11/Oct/11 1:46 AM ]

Fixed, https://github.com/clojure/clojurescript/commit/d3cba99b7741f8fc445f21b4cddf0ea4ee96a1cb

Comment by David Nolen [ 12/Oct/11 5:14 PM ]

Real fix, https://github.com/clojure/clojurescript/commit/f451ce29730881d247b15aaee214ba7a99f9726a





[CLJS-88] bug in javascript generated by condp Created: 14/Oct/11  Updated: 27/Jul/13  Resolved: 14/Oct/11

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

Type: Defect Priority: Major
Reporter: Eric Harris-Braun Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None


 Description   

The javascript generated by calls to condp includes a line like:

{throw (new java.lang.IllegalArgumentException(cljs.core.str.call(null,"No matching clause: ",expr__2417__2420)));}

Which gets called when there are no matches, and fails with an error because there doesn't seem to be "java" object.

Occurs in 415a5fc6a.



 Comments   
Comment by David Nolen [ 14/Oct/11 8:37 AM ]

Fixed, https://github.com/clojure/clojurescript/commit/cf84fa2cc1ce0e6f13249baef6cd71a635568bcc





[CLJS-92] Cannot interact with Browser REPL running in iOS/Webkit Mobile devices Created: 21/Oct/11  Updated: 27/Jul/13  Resolved: 21/Oct/11

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

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


 Comments   
Comment by David Nolen [ 21/Oct/11 4:51 PM ]

Works just fine.





[CLJS-98] (instance? js/String "foo") returns false Created: 04/Nov/11  Updated: 27/Jul/13  Resolved: 13/Nov/11

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

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


 Comments   
Comment by David Nolen [ 08/Nov/11 10:29 AM ]

https://github.com/clojure/clojurescript/compare/master...98-fix-instance

In this branch the following hold:

(instance? js/Number 1) ; => true
(instance? js/String "foo") ; => true
(instance? js/Object 1) ; => true
(instance? js/Object true) ; => true

Only undefined and null return false.

One issue is that instance? cannot be used to check whether something satisfies a protocol. This must be done with satisfies? which is a macro. Are people ok with this dichotomy?

Should try/catch check instance? and satisfies?

Comment by David Nolen [ 13/Nov/11 5:50 PM ]

Fixed, https://github.com/clojure/clojurescript/compare/11e8bfe75d...53e5d19370





[CLJS-383] Empty variadic argument initialised with (nil) when using arity overloading Created: 23/Sep/12  Updated: 27/Jul/13  Resolved: 15/Oct/12

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

Type: Defect Priority: Major
Reporter: Daniel Skarda Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None
Environment:

Fresh CLJS checkout from github


Attachments: Text File CLJS-383-gfredericks.patch     Text File CLJS-383.patch    

 Description   

Empty variadic argument (& rest) is incorrectly initialised with (nil), when variadic arguments are used together with overloading.
See following example:

(defn foo
[& TEST]
TEST)

(foo)
=> nil ;; correct

(foo 1 2)
=> (1 2) ;; correct

(defn bar
([]
:empty)
([A & B]
B))

(bar)
=> :empty ;; correct

(bar 1 2)
=> (2) ;; correct

(bar 1)
=> (nil) ;; error - should be nil



 Comments   
Comment by Daniel Skarda [ 29/Sep/12 8:32 AM ]

I prepared a patch for this issue.

See fix and tests in https://github.com/orfelyus/clojurescript/commit/16373968e8d0a5069dc4b43827d2d4b37e14d152

Comment by David Nolen [ 29/Sep/12 12:25 PM ]

Excellent can you attach the patch here? Thanks!

Comment by Daniel Skarda [ 30/Sep/12 3:12 AM ]

See CLJS-383.patch

Comment by Gary Fredericks [ 02/Oct/12 10:55 PM ]

I stumbled on this independently and noticed the related fact that (array-seq (array 1) 1) returns a non-nil IndexedSeq (which will return itself if passed to cljs.core/seq, and will present itself as length 0 or length 1 depending on how exactly you ask). I wasn't sure if this was a bug or not. If array-seq is not considered a public method then it's probably fine, but otherwise it's obviously bad.

Incidentally, this bug manifested itself to me not as a non-nil return value but as the wrong arity of another function being called.

Comment by Daniel Skarda [ 03/Oct/12 6:56 AM ]

Thanks! You are right!

(defn bar
([] :empty)
([A & B] B))

Compiled into

function (A, var_args) {
var B = var_args;
switch (arguments.length) { case 0: return bar__0.call(this); default: return bar__2.cljs$lang$arity$variadic(A, cljs.core.array_seq(arguments, 1)); }
throw ("Invalid arity: " + arguments.length);
}

Compared to variant without arity overloading, there was missing isDef and call to slice.
But I missed original problem, which was:

(array-seq (js* "[1]") 1)

evaluates to (nil). Call to array-seq translates to prim-seq translates to IndexedSeq. I digged deeper and found two potential issues:

1) Call to first without check for i and length

(deftype IndexedSeq [a i]
....
ISeq
(-first [_] (aget a i))

ie. if i == a.length () then first on such sequence returns undefined (instead of nil). Which might cause other issues.

2) seq:

(defn ^seq seq
[coll]
(when-not (nil? coll)
(if (satisfies? ASeq coll)
coll
(-seq coll))))

IndexedSeq is not nil and (-seq IndexedSeq) returns itself.
So we have a sequence, which is empty, but it is not nil. I think it can cause zillions of issues.

@svannodette: I think we can forget my original patch The correct solution is to fix prim-seq directly, not its effects on other parts of clojurescript (variadic functions)

prim-seq is

(defn prim-seq
([prim]
(prim-seq prim 0))
([prim i]
(when-not (zero? (alength prim))
(IndexedSeq. prim i))))

while it should be

(defn prim-seq
([prim]
(prim-seq prim 0))
([prim i]
(when (< i (alength prim))
(IndexedSeq. prim i))))

In addition, we could avoid call to slice and have variadic functions calls a little little bit faster
What do you think? I will prepare a patch tomorrow.

Comment by David Nolen [ 03/Oct/12 10:57 AM ]

One first glance yes this sounds good.

Comment by Gary Fredericks [ 05/Oct/12 7:07 PM ]

Submitting a patch since Skarda hasn't yet. The fix is the same as his second suggestion, and three tests are provided.

I've only tested this with V8 since that's all I could get installed. Disturbingly, the tests crash in advanced mode only as provided, but if I take the second of the three tests and repeat it after them (ABCB), then everything passes. I'm hoping it's some crazy quirk of my installation.

Comment by David Nolen [ 15/Oct/12 10:51 PM ]

fixed, http://github.com/clojure/clojurescript/commit/62aca8f7a03c0d3cfad4e48665832b2b0e520680

Comment by Tom Jack [ 16/Oct/12 1:32 AM ]

Was the mysterious test failure solved?

Comment by David Nolen [ 17/Oct/12 10:48 AM ]

I never encountered it.





[CLJS-369] gensyms break re-analyze; prevents variable shadowing analysis Created: 01/Sep/12  Updated: 27/Jul/13  Resolved: 15/Oct/12

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

Type: Defect Priority: Major
Reporter: Brandon Bloom Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: patch, patch,

Attachments: Text File CLJS-369-v2.patch     Text File shadowing.patch    
Patch: Code

 Description   

From my email discussion with dnolon before making this patch:

The current analyzer does some trickery to prepare for emitting to JavaScript. Among this trickery is gensyms for locals (including "this"), the new $ prefix on some namespaces, uniqify on parameters, and more. This must be mildly annoying for people writing alternate compiler backends, but for the most part non-blocking because fewer symbol collisions should never be an additional problem for a target language with different symbol resolution rules.

[snip lots more text]

Consider what an ANF transform would look like for the :let form:

(defmethod anf :let
  [{:keys [env bindings statements ret form] :as ast}]
  (let [bindings (mapcat (fn [{:keys [sym init] :as binding}]
                           [name (anf init)])
                         bindings)
        body-env (-> bindings last :env)]
    (ana/analyze env `(~(first form) [~@(map :form bindings)]
                          ~@(anf-block (assoc ast :env body-env))))))

Simple enough, right? This walks each binding, ANF transforms the init expression, gets the environment in which to analyze the body, and then analyzes a new let (or loop) statement with the modified bindings.

Unfortunately, this doesn't work. When the ana/analyze call happens, body-env contains gensymed locals. The result is that body-env is invalidated by the outer analyze call, which is re-generating the symbols for the local variables. If you take the gensyms out of analyze-let, then analyze becomes pure (modulo def forms) and this code magically becomes correct. I've run into this same problem anywhere gensyms are used in analyze.

Commit message on the patch:

AST Changes
    
    * Anywhere a binding was introduced for a local used to be a symbol,
      now it is a map with a :name key and potentially a :shadow key.
    
    * Bindings vectors are no longer alternating symbols, then init maps.
      Instead, the are a vector of maps of the shape described for locals
      plus an :init key.
    
    * The :gthis key for functions has been replaced with :type, which
      is the symbol describing the type name of the enclosing deftype form.
    
    * recur frames now expose :params as binding maps, instead of :names
    
    Benefits:
    
    * Shadowed variables are now visible to downstream AST transforms.
    
    * :tag, :mutable, and other metadata are now uniform across ops
    
    * Eliminates usages of gensym inside the analyzer, which was a source
      of state that made the analyzer impossible to use for some
      transformations of let, letfn, etc which require re-analyzing forms.
    
    * Removes JavaScript shadowing semantics from the analyze phase.


 Comments   
Comment by David Nolen [ 03/Sep/12 1:13 PM ]

Can we please get the ticket number in the first line of the patch? If you look at the ClojureScript commit history you can see the convention that's been adopted. Thanks!

Comment by Brandon Bloom [ 03/Sep/12 2:02 PM ]

Reworded commit message to meet new convention.

Comment by David Nolen [ 28/Sep/12 5:55 PM ]

Can we put the shadow patch in another ticket with the patch referencing the new ticket #. Thanks!

Comment by Brandon Bloom [ 28/Sep/12 6:11 PM ]

Not sure what good a separate ticket would do. How about this new title?

Comment by David Nolen [ 29/Sep/12 12:39 PM ]

They are separate patches. One is a enhancement to the compiler. The other one is an enhancement to my simplistic shadowing solution using the improvements to the compiler in the other enhancement. Thanks!

Comment by Brandon Bloom [ 30/Sep/12 12:34 PM ]

Are you talking about the namespace shadowing? This patch affects all variable shadowing. For example, (fn [x x] x) will produce `function (x x__$1) { return x__$1; }` instead of `function (x_G12 x_G13) { return x__G13; }` or something like that.

I'm not sure how I can break this patch down into smaller pieces. All of the gensyms were there before to eliminate potential shadowing; the two issues are tightly related. If you eliminated all the gensyms, the compiler would work fine... unless you shadowed a variable (which several tests cover).

Have you studied the patch? Can you suggest a concrete way to break it up into smaller patches? I'm not sure it's worth the trouble.

Comment by David Nolen [ 03/Oct/12 10:59 AM ]

Right sorry, makes sense now. I will review both patches more closely. Thanks again.

Comment by David Nolen [ 15/Oct/12 10:43 PM ]

I've finally found some time go through this patch. It's great but it no longer applies to master. If I get an updated patch I';; apply promptly. Thank you very much and sorry for the delay.

Comment by David Nolen [ 15/Oct/12 11:01 PM ]

Went ahead and applied it, only needed a minor change.

Comment by David Nolen [ 15/Oct/12 11:02 PM ]

fixed, http://github.com/clojure/clojurescript/commit/19afb31a52504293ba2182c584b1867917316662

Comment by Brandon Bloom [ 15/Oct/12 11:45 PM ]

Awesome. Glad to see this applied.

Off topic: I like to keep tabs on my open source contributions semi-automatically using the author information stored in git. Sadly, your minor change switched the author to you. If it's not too much to ask, could you please preserve the author info in the future? Either via a separate fixup commit, or by using the --author flag when editing commits. Thanks!

Comment by David Nolen [ 17/Oct/12 10:53 AM ]

Brandon, yep of course! I didn't know about the --author flag otherwise I would have used it. Thanks again.





[CLJS-284] do not emit flags on the prototype for fast path protocols Created: 29/May/12  Updated: 27/Jul/13  Resolved: 30/May/12

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

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

Attachments: Text File 0001-CLJS-284-do-not-emit-flags-on-the-prototype-for-fast.patch     Text File 0001-CLJS-284-do-not-emit-flags-on-the-prototype-for-fast.patch    

 Description   

Brian Taylor's sweet graphs seem to indicate a performance regression for PersistentVectors after the introduction of IReversible http://www.50ply.com/cljs-bench/.

One hunch is that deoptimization is kicking in on V8 because we've set too many non-method properties on the prototype. We should not emit the boolean protocol flag if we have a fast path protocol.



 Comments   
Comment by Michał Marczyk [ 30/May/12 1:28 AM ]

Here's a first cut at a patch. Certain vector ops are indeed much faster than on master, but most of the other benchmarks seem to get slower (unfortunately first/rest/next on all sorts of inputs seem to suffer particularly badly).

The commit message includes a summary of the approach and some issues (with String/Number/Boolean and truth_) worked around in the patch. NB. js/String actually is extended in core.cljs.

Comment by David Nolen [ 30/May/12 8:03 AM ]

The satisfies? part of this patch is not going to work. It emits a function call as well as a call to truth_. type inference won't help you here since cond expands into if's which return null in the false branch defeating boolean inference. As far as the function call, I suspect the emitting code is complex enough that Closure won't optimize it. satisfies? at this point is finely crafted to emit the most optimal code both for Closure and JS engines I strongly suggest against any attempts at refactoring. Simply make the most minimal change to make it work.

Comment by Michał Marczyk [ 30/May/12 8:44 AM ]

There is no call to truth_ that I can see and still only a single function call in the test part of an 'if (and of about the same complexity as the one generated for the 'or on master). With the patch #(satisfies? IVector []) compiles to the following JavaScript (notice the outer function is the one introduced by #(...); that just makes it convenient to print from Rhino and I'm copying and pasting it here with no change so as not to introduce any mistakes):

function () {
    var G__9752__9753 = cljs.core.PersistentVector.fromArray([]);
    if (G__9752__9753) {
        if ((G__9752__9753.cljs$lang$protocol_mask$partition0$ & 16384)) {
            return true;
        } else {
            if ((function () {
                var and__3822__auto____9754 = G__9752__9753.constructor;
                if (and__3822__auto____9754) {
                    return (G__9752__9753.constructor.prototype.cljs$lang$protocol_mask$partition0$ & 16384);
                } else {
                    return and__3822__auto____9754;
                }
            })()) {
                return true;
            } else {
                if (G__9752__9753.cljs$core$IVector$) {
                    return true;
                } else {
                    if ((!G__9752__9753.cljs$lang$protocol_mask$partition0$)) {
                        return (cljs.core.type_satisfies_.cljs$lang$arity$2 ? cljs.core.type_satisfies_.cljs$lang$arity$2(cljs.core.IVector, G__9752__9753) : cljs.core.type_satisfies_.call(null, cljs.core.IVector, G__9752__9753));
                    } else {
                        if (true) {
                            return false;
                        } else {
                            return null;
                        }
                    }
                }
            }
        }
    } else {
        return (cljs.core.type_satisfies_.cljs$lang$arity$2 ? cljs.core.type_satisfies_.cljs$lang$arity$2(cljs.core.IVector, G__9752__9753) : cljs.core.type_satisfies_.call(null, cljs.core.IVector, G__9752__9753));
    }
}

I tried making small modifications to the main Boolean expression (test bit mask, fall back to flag) w/o introducing any conds, but these all resulted in truth_ appearing in the compiled output. I'm not making any claims as to whether I've tried all possible small modifications, but I've attempted a fair number with and without bool-expr hints.

As for Closure's optimizations – I'll have to check that. As mentioned above though, there is still only one function generated in the test part of an 'if expression (in this version for an 'and form, since the refactoring into a 'cond gets rid of the 'or form), is about the same size as the one generated for the 'or previously and occurs in a test which resides fully inside a single curly-delimited block.

Not that I think this patch should be applied – quite the contrary, it makes things slower for the most part; my point is just that there's no truth_ to lay the blame on and I'm not sure how to arrive at the same semantics in the same amount of generated JS code while still attaching the masks to the instances for inline protocol impls.

This does give me an idea for another possible approach, though... I'll be back with a different patch if it works.

Comment by David Nolen [ 30/May/12 8:52 AM ]

Code emitted for first prior to the patch under simple optimizations (no more fn unwrapping occurs after simple):

cljs.core.first = function(a) {
  if(null == a) {
    return null
  }
  var b;
  a ? (b = (b = a.cljs$lang$protocol_mask$partition0$ & 64) ? b : a.cljs$core$ISeq$, b = b ? !0 : a.cljs$lang$protocol_mask$partition0$ ? !1 : cljs.core.type_satisfies_(cljs.core.ISeq, a)) : b = cljs.core.type_satisfies_(cljs.core.ISeq, a);
  if(b) {
    return cljs.core._first(a)
  }
  a = cljs.core.seq(a);
  return null == a ? null : cljs.core._first(a)
};

After:

cljs.core.first = function(a) {
  if(null == a) {
    return null
  }
  if(cljs.core.truth_(function() {
    if(a) {
      if(a.cljs$lang$protocol_mask$partition0$ & 64) {
        return!0
      }
      var b;
      b = (b = a.constructor) ? a.constructor.prototype.cljs$lang$protocol_mask$partition0$ & 64 : b;
      return b || a.cljs$core$ISeq$ ? !0 : a.cljs$lang$protocol_mask$partition0$ ? !1 : cljs.core.type_satisfies_(cljs.core.ISeq, a)
    }
    return cljs.core.type_satisfies_(cljs.core.ISeq, a)
  }())) {
    return cljs.core._first(a)
  }
  var b = cljs.core.seq(a);
  return null == b ? null : cljs.core._first(b)
};

There is a truth_ test and a function call now.

Comment by David Nolen [ 30/May/12 8:55 AM ]

Again my suggestion is to not attempt any sort of refactor at all. Just emit different code for non-fast path protocols.

To make my point about truth_ a bit clearer. With the current patch the result of satisfies? can longer be inferred as boolean if you use cond.

Comment by Michał Marczyk [ 30/May/12 9:13 AM ]

Well, thanks for setting me straight re: truth_.

NB. fast path protocols are implemented both inline and with a standalone extend-type; however, I think we can skip the flag only in the case where the impl occurs inline in a deftype/defrecord (this includes the implicit impls provided by defrecord) and emit a flag otherwise. I'll write a patch along these lines.

Comment by Michał Marczyk [ 30/May/12 10:22 AM ]

New patch taking the previously outlined approach.

There's a little something extra in there – I just discovered that the fast path bits were not being set for the protocols implicitly implemented by records. With this patch, they are.

Using metadata to communicate with extend-type seemed like the simplest approach allowing deftype and defrecord to use the same extend-type that everybody else does.

Comment by David Nolen [ 30/May/12 10:51 AM ]

Why do we need to enumerate the implicit protocols for defrecords? Aren't these captured by the expansion of the defrecord macro?

Comment by Michał Marczyk [ 30/May/12 11:06 AM ]

Ah, right; they're generated by emit-defrecord, so we don't need to enumerate them if we prepare-protocol-masks once they're in place. New patch does this.

Comment by David Nolen [ 30/May/12 11:12 AM ]

fixed, http://github.com/clojure/clojurescript/commit/dc4b87d2f6fb35d25ebfa38a53cb641420052b08





[CLJS-279] Vector reduce performance regression Created: 28/May/12  Updated: 27/Jul/13  Resolved: 30/May/12

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

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


 Description   

d07f2e1e777591338e6f2a3d1fb9a73e1e81edfd
[coll (into [] (range 1000000))], (reduce + 0 coll), 1 runs, 595 msecs
[coll (into [] (range 1000000))], (reduce + 0 coll), 1 runs, 614 msecs

(the next commit is...)

944264365091ccc7071e9b5302d95bc58af089db
[coll (into [] (range 1000000))], (reduce + 0 coll), 1 runs, 1419 msecs
[coll (into [] (range 1000000))], (reduce + 0 coll), 1 runs, 1635 msecs

(the problem persists in...)

0708e91dcaa117bb7b7a5a4a44c391b6a57c62c6 origin/master
[coll (into [] (range 1000000))], (reduce + 0 coll), 1 runs, 1294 msecs
[coll (into [] (range 1000000))], (reduce + 0 coll), 1 runs, 1234 msecs



 Comments   
Comment by David Nolen [ 28/May/12 7:19 PM ]

We have chunked seqs and we're not leveraging them enough. The previous performance was horrendous too We need to find out why it's taking longer than 2X of Clojure on JVM.

Comment by David Nolen [ 30/May/12 11:15 AM ]

addressed in CLJS-284





[CLJS-152] Compilation fails on code expanded by macros which create code using current name space *ns* Created: 20/Feb/12  Updated: 27/Jul/13  Resolved: 17/Jun/12

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

Type: Defect Priority: Major
Reporter: Daniel Kwiecinski Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None


 Description   

When macro uses current namespace for its expansion the current namespace is bound to 'user' one rather to whatever ns points to. Even though fixing it, clojurescript compiler fails due to assertion '(assert (not (namespace sym)) "Can't def ns-qualified name")' in compiler.clj:663

PATCH: https://github.com/lambder/clojurescript/commit/612d013eb8d61d7aa61b1247a18745f6a19f5788



 Comments   
Comment by David Nolen [ 20/Feb/12 7:05 AM ]

Do you have a simple example illustrating the problem?

Comment by Brian Taylor [ 30/May/12 4:14 PM ]

I'm not totally clear on how to replicate your test scenario but I suspect this issue may be related to CLJS-278 and may be resolved if my patch is accepted.

Comment by David Nolen [ 17/Jun/12 12:06 PM ]

resolving unless we hear otherwise.





[CLJS-97] defrecords of different types are equal Created: 30/Oct/11  Updated: 27/Jul/13  Resolved: 20/Nov/11

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

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


 Description   

In clojure, defrecords of different types are not considered to be equal:

user=> (defrecord A [x])
user.A
user=> (defrecord B [x])
user.B
user=> (= (>A 1) (>B 1))
false

But in clojurescript they are:

ClojureScript:cljs.user> (defrecord A [x])

ClojureScript:cljs.user> (defrecord B [x])

ClojureScript:cljs.user> (= (>A 1) (>B 1))
true

[
A related note, that I've raised as a Clojure issue:

In Clojure records are not considered to be equal if they have different types, but the record type is not included in the hashcode - this will lead to excessive hash collisions if a map contains heterogenous record keys that don't have distinct data.

ClojureScript is consistent with Clojure in this area, but I think it should be changed.
]



 Comments   
Comment by David Nolen [ 20/Nov/11 1:47 PM ]

Fixed, https://github.com/clojure/clojurescript/commit/bb2e10f20beaa7b5d3afe01c3544384d1c40ef1c





[CLJS-393] sebseq and sorted-set-by Created: 13/Oct/12  Updated: 27/Jul/13  Resolved: 18/Oct/12

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

Type: Defect Priority: Major
Reporter: Erik Ouchterlony Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: bug

Attachments: File bugfix-PersistentTreeMap.diff    

 Description   

ClojureScript:cljs.user> (subseq (sorted-set-by <= 1 2 3 4 5) >= 2 <= 4)
"Error evaluating:" (subseq (sorted-set-by <= 1 2 3 4 5) >= 2 <= 4) :as "cljs.core.subseq.call(null,cljs.core.sorted_set_by.call(null,cljs.core.LTEQ,1,2,3,4,5),cljs.core.GTEQ,2,cljs.core.LTEQ,4);\n"
org.mozilla.javascript.JavaScriptException: Error: No protocol method IMapEntry.-key defined for type null: (cljs/core.cljs#211)
at cljs/core.cljs:211 (anonymous)
at cljs/core.cljs:203 (_key)
at cljs/core.cljs:5535 (key)
at cljs/core.cljs:2479 (anonymous)
at cljs/core.cljs:1791 (lazy_seq_value)
at cljs/core.cljs:1840 (anonymous)
at cljs/core.cljs:238 (_seq)
at cljs/core.cljs:343 (seq)
at cljs/core.cljs:817 (anonymous)
at cljs/core.cljs:854 (anonymous)
at cljs/core.cljs:857 (anonymous)
at cljs/core.cljs:868 (anonymous)
at cljs/core.cljs:5925 (anonymous)
at cljs/core.cljs:5937 (anonymous)
at <cljs repl>:1 (anonymous)
at <cljs repl>:1



 Comments   
Comment by Erik Ouchterlony [ 17/Oct/12 5:23 PM ]

I've done some further analysis on this problem and found three different issues:

  1. The sorted-set-by in cljs doesn't support ordinary boolean operators, only comparison functions with values -1,0,1.
  2. Bug in PersistentTreeSet lookup. Resolved in CLJS-394
  3. Bug in PersistentTreeMap -sorted-seq-from. I have attached a patch that seems to resolve the issue.
Comment by David Nolen [ 17/Oct/12 9:25 PM ]

So does the patch address both 1 & 3 or only 3?

Comment by Erik Ouchterlony [ 18/Oct/12 2:48 AM ]

Only 3.

Comment by Erik Ouchterlony [ 18/Oct/12 6:04 AM ]

Here is a patch for the third issue.

Comment by Erik Ouchterlony [ 18/Oct/12 4:21 PM ]

I found a small bug in the last patch, so I attached a new one. This patch handles both the remaining issues.

Comment by David Nolen [ 18/Oct/12 5:44 PM ]

fixed, http://github.com/clojure/clojurescript/commit/e3ed0e7b69f9522e8759d0a6567afabb2a98d949





[CLJS-285] type-hinted fields for TransientHashMap do not work Created: 30/May/12  Updated: 27/Jul/13  Resolved: 30/May/12

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

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

Attachments: Text File 0001-CLJS-285-fix-type-hints-on-TransientHashMap-s-fields.patch    

 Description   

The boolean type hints for TransientHashMap fields don't seem to do anything. We still see calls to truth_.



 Comments   
Comment by Michał Marczyk [ 30/May/12 6:04 PM ]

The problem was due to metadata being implicitly quoted: ^{:tag 'boolean} foo produces a tag of (quote boolean) rather than boolean. The attached patch fixes this and now truth_ steers clear of THM.

Comment by David Nolen [ 30/May/12 8:08 PM ]

fixed, http://github.com/clojure/clojurescript/commit/8e7e10fd06d71159e880f1b8473068839e89a3d3





[CLJS-533] Tests for :import should actually get run and pass Created: 03/Jul/13  Updated: 27/Jul/13  Resolved: 03/Jul/13

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

Type: Defect Priority: Major
Reporter: Michał Marczyk Assignee: Michał Marczyk
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 0001-CLJS-533-call-tests-for-import-in-the-test-runner.patch    

 Description   

There are tests written for :import in ns forms, but the test runner never calls them. The forthcoming patch fixes that.

Also, one of them is actually wrong (missing an str call) – the patch includes a fix for that.



 Comments   
Comment by David Nolen [ 03/Jul/13 7:51 PM ]

fixed, http://github.com/clojure/clojurescript/commit/178e6b28d71325a0466b9c34aa973d9a7ea97922





[CLJS-106] match-int fails in IE Created: 23/Nov/11  Updated: 27/Jul/13  Resolved: 24/Nov/11

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

Type: Defect Priority: Major
Reporter: Wilkes Joiner Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None
Environment:

Internet Explorer 8


Attachments: Text File fix-match-int.patch    
Patch: Code

 Description   

match-int returns 0 in IE.

This is caused by the use of re-find, a regex.exec behaving differently in IE.



 Comments   
Comment by David Nolen [ 24/Nov/11 1:50 PM ]

Fixed, https://github.com/clojure/clojurescript/commit/1e063940ffa6f350dc1691a78da6f12d894fa4a3





[CLJS-531] Supply missing argument to repl/load-stream in rhino.clj Created: 01/Jul/13  Updated: 27/Jul/13  Resolved: 03/Jul/13

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

Type: Defect Priority: Major
Reporter: Michał Marczyk Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 0001-CLJS-531-Supply-missing-argument-to-repl-load-stream.patch    

 Description   

I'm actually not sure if the exact argument supplied in the forthcoming patch is the correct one, but I had to add something to get my Rhino REPL to work as expected.



 Comments   
Comment by David Nolen [ 03/Jul/13 7:54 PM ]

fixed http://github.com/clojure/clojurescript/commit/06810b52466467f076519ca590c51833904a6bf7





[CLJS-290] Use transient vector in vec Created: 31/May/12  Updated: 27/Jul/13  Resolved: 01/Jun/12

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

Type: Enhancement Priority: Major
Reporter: Michał Marczyk Assignee: Michał Marczyk
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 0001-CLJS-290-use-transient-vector-in-vec.patch    
Patch: Code

 Description   

The impl on master uses conj and a regular PV. The attached patch switches to a TV using -as-transient, -conj!, -persistent! directly.



 Comments   
Comment by Michał Marczyk [ 01/Jun/12 8:42 AM ]

Same patch against current master.

Comment by Michał Marczyk [ 01/Jun/12 8:48 AM ]

Same patch against current master, no typos this time.

Comment by David Nolen [ 01/Jun/12 8:51 AM ]

fixed, http://github.com/clojure/clojurescript/commit/37f4914e7fb7d9d76607d5e5ee965a01d05d4818





[CLJS-293] move field metadata to fn form Created: 01/Jun/12  Updated: 27/Jul/13  Resolved: 01/Jun/12

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

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

Attachments: Text File 0001-CLJS-293-field-metadata-attached-to-fn.patch    

 Description   

The metadata that tells about fields (from defrecord or deftype) that are accessible to a fn should be attached to the fn* form instead of to the params vector.



 Comments   
Comment by Brian Taylor [ 01/Jun/12 7:15 PM ]

Moved field metadata to fn form. Added test to verify CLJS-174 is fixed as well. Removed code to passthrough metadata attached to fn arglist vector.

Comment by David Nolen [ 01/Jun/12 9:19 PM ]

fixed, http://github.com/clojure/clojurescript/commit/f58bee6dfab05e9b2e972cd737fd744f19a92149





[CLJS-493] get should accept any type the same way Clojure JVM does Created: 03/Apr/13  Updated: 27/Jul/13  Resolved: 05/Apr/13

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

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

Attachments: Text File 0001-CLJS-493-get-should-accept-any-type.patch    

 Description   

The following throws instead of returning nil:

(get 42 :anything)


 Comments   
Comment by Michał Marczyk [ 03/Apr/13 4:50 PM ]

clojure.lang.RT-style fix.

From the commit message:

The fix involves changes to both core.cljs and core.clj, since get is backed by a macro. Tests are introduced for both direct and higher-order calls.

Comment by Michał Marczyk [ 03/Apr/13 4:52 PM ]

Incidentally, I've prepared the patch on top of patches for CLJS-491 and CLJS-492, it should still apply to master though.

Comment by David Nolen [ 04/Apr/13 6:57 PM ]

I'd prefer that the macro not emit a conditional check. I think it's probably OK if we just provide a default case for ILookup. Thanks.

Comment by Michał Marczyk [ 04/Apr/13 7:14 PM ]

Sure, here's a new patch, with a default case provided for ILookup and the tests from the previous patch. Both macro and function unchanged, since with this change everything satisfies? ILookup.

Out of curiosity, why the preference? Do you think this sort of expansion jumping in at the wrong place could pose problems for GClosure?

Comment by David Nolen [ 04/Apr/13 7:17 PM ]

Thanks get is just extremely common and likely to appear in expression cases - where the CLJS compiler will emit a function. If these get too nested GClosure won't handle them and it's a pretty big perf hit.

Comment by Michał Marczyk [ 04/Apr/13 7:27 PM ]

Right, thanks.

Comment by David Nolen [ 05/Apr/13 9:30 PM ]

fixed, http://github.com/clojure/clojurescript/commit/7b9fccd9a2502c1984c1880176a6b0d310151520





[CLJS-492] avoid producing unnecessary calls to next in emit-apply-to Created: 01/Apr/13  Updated: 27/Jul/13  Resolved: 05/Apr/13

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

Type: Enhancement Priority: Major
Reporter: Michał Marczyk Assignee: Michał Marczyk
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 0001-CLJS-492-avoid-producing-unnecessary-calls-to-next-i.patch    

 Description   

A function like

(defn bar [a b c d e f g h i j & more])

currently receives a cljs$lang$applyTo property of the following form:

function (arglist__4466) {
    var a = cljs.core.first(arglist__4466);
    var b = cljs.core.first(cljs.core.next(arglist__4466));
    var c = cljs.core.first(cljs.core.next(cljs.core.next(arglist__4466)));
    var d = cljs.core.first(cljs.core.next(cljs.core.next(cljs.core.next(arglist__4466))));
    var e = cljs.core.first(cljs.core.next(cljs.core.next(cljs.core.next(cljs.core.next(arglist__4466)))));
    var f = cljs.core.first(cljs.core.next(cljs.core.next(cljs.core.next(cljs.core.next(cljs.core.next(arglist__4466))))));
    var g = cljs.core.first(cljs.core.next(cljs.core.next(cljs.core.next(cljs.core.next(cljs.core.next(cljs.core.next(arglist__4466)))))));
    var h = cljs.core.first(cljs.core.next(cljs.core.next(cljs.core.next(cljs.core.next(cljs.core.next(cljs.core.next(cljs.core.next(arglist__4466))))))));
    var i = cljs.core.first(cljs.core.next(cljs.core.next(cljs.core.next(cljs.core.next(cljs.core.next(cljs.core.next(cljs.core.next(cljs.core.next(arglist__4466)))))))));
    var j = cljs.core.first(cljs.core.next(cljs.core.next(cljs.core.next(cljs.core.next(cljs.core.next(cljs.core.next(cljs.core.next(cljs.core.next(cljs.core.next(arglist__4466))))))))));
    var more = cljs.core.rest(cljs.core.next(cljs.core.next(cljs.core.next(cljs.core.next(cljs.core.next(cljs.core.next(cljs.core.next(cljs.core.next(cljs.core.next(arglist__4466))))))))));
    return bar__delegate(a, b, c, d, e, f, g, h, i, j, more);
}

The forthcoming patch changes that to

function (arglist__4460) {
    var a = cljs.core.first(arglist__4460);
    arglist__4460 = cljs.core.next(arglist__4460);
    var b = cljs.core.first(arglist__4460);
    arglist__4460 = cljs.core.next(arglist__4460);
    var c = cljs.core.first(arglist__4460);
    arglist__4460 = cljs.core.next(arglist__4460);
    var d = cljs.core.first(arglist__4460);
    arglist__4460 = cljs.core.next(arglist__4460);
    var e = cljs.core.first(arglist__4460);
    arglist__4460 = cljs.core.next(arglist__4460);
    var f = cljs.core.first(arglist__4460);
    arglist__4460 = cljs.core.next(arglist__4460);
    var g = cljs.core.first(arglist__4460);
    arglist__4460 = cljs.core.next(arglist__4460);
    var h = cljs.core.first(arglist__4460);
    arglist__4460 = cljs.core.next(arglist__4460);
    var i = cljs.core.first(arglist__4460);
    arglist__4460 = cljs.core.next(arglist__4460);
    var j = cljs.core.first(arglist__4460);
    var more = cljs.core.rest(arglist__4460);
    return bar__delegate(a, b, c, d, e, f, g, h, i, j, more);
}

The included benchmark seems to demonstrate a substantial reduction in {apply} overhead on V8 for a function with 10 positional arguments preceding the rest arg and a less substantial reduction for a function with 2 positional arguments, as one would expect. The other benchmark entries for {apply} remain in the same ballpark ({list} actually produces the same compilation output with or without the patch; {+} is changed, but only slightly).



 Comments   
Comment by David Nolen [ 05/Apr/13 9:30 PM ]

fixed, http://github.com/clojure/clojurescript/commit/1a094e41ba482de1bc2e363874cdb29414bdac46





[CLJS-174] Destructuring within method definition breaks field scoping Created: 30/Mar/12  Updated: 27/Jul/13  Resolved: 01/Jun/12

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

Type: Defect Priority: Major
Reporter: Kevin Lynagh Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None


 Description   

Destructuring arguments within a method definition breaks references to record fields.
Here are some examples:

;;;;;;;;;;;;
;;Don't work

(deftype Thing [a]
IFn
(invoke [_ [x y]]
[x y a]))

(deftype Thing2 [a])
(extend-type Thing2
IFn
(invoke
([_ [x y]]
[x y a])))

;;;;;;;;;;;;
;;Work

(deftype thing [a]
IFn
(invoke [_ m]
(let [[x y] m]
[x y a])))

(let [a 5]
((reify IFn
(invoke [_ [x y]]
[x y a]))))



 Comments   
Comment by Kevin Lynagh [ 30/Mar/12 10:27 AM ]

Here's some complied JS:

https://gist.github.com/79e7469953e231c75b70

red's broken, green works.

Comment by Brian Taylor [ 30/May/12 10:03 PM ]

The problem was that the metadata that tells the compiler what fields are available was being conveyed through the argument list. This meta-data was being destroyed during the expansion of fn when the argument list required destructuring.

I've pulled in and altered the Clojure fn macro so that it conveys the meta-data even if it has to destructure.

Comment by David Nolen [ 31/May/12 8:13 AM ]

I wonder if adorning the arguments was the best idea. Why not put the fields on the (fn ...) form itself and pull that out in parse 'fn*? Then fields can be added to the environment which makes more sense to me anyhow and lot of duplicated computation can be avoided.

Comment by Brian Taylor [ 01/Jun/12 7:16 PM ]

That was a simpler solution. If accepted, the patch attached to CLJS-293 resolves this issue as well.

Comment by David Nolen [ 01/Jun/12 9:19 PM ]

fixed, http://github.com/clojure/clojurescript/commit/f58bee6dfab05e9b2e972cd737fd744f19a92149





[CLJS-153] ClojureScript wiki QuickStart "Running ClojureScript on Node.js" is broken (between r971 and 335a1f61976d39c209e81e17cb95ce247424a656) Created: 20/Feb/12  Updated: 27/Jul/13  Resolved: 14/Mar/12

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

Type: Defect Priority: Major
Reporter: Adrien Chauve Assignee: Unassigned
Resolution: Completed Votes: 1
Labels: None
Environment:

ubuntu 11.10
git clone of clojurescript (master, 335a1f61976d39c209e81e17cb95ce247424a656)
git clone of nodejs



 Description   

When I compile nodehello.cljs, I get:

GRAVE: ERROR - Duplicate extern input: /home/XXX/dev/clojurescript/src/cljs/cljs/nodejs_externs.js

Note: if I use r971 (git checkout r971 in clojurescript repo), it works fine.



 Comments   
Comment by David Nolen [ 22/Feb/12 12:31 AM ]

Looks like the nodejs extern is getting included twice, not sure yet why.

Comment by Alex E. [ 13/Mar/12 10:02 PM ]

Every revision after e8c55da3148357ed64233991f669501a22dcb399 has this problem.. Unfortunately, I don't know enough (clojure) to pinpoint exactly where, but it's def. in this merge..

Comment by Yoshito Komatsu [ 14/Mar/12 10:44 AM ]

It seems that add-target runs twice in load-externs.
I think that the following patch fixes this problem.

https://github.com/ykomatsu/clojurescript/commit/11356f585e04e047f69ba497cd1a242a7fef6121

Comment by David Nolen [ 14/Mar/12 7:15 PM ]

Fixed, https://github.com/clojure/clojurescript/commit/837d9323106fd2f0f7ce38778dce2645a19ca00e





[CLJS-159] Suggest removing UNIX shell executable permissions from bin/cljsc.bat Created: 06/Mar/12  Updated: 27/Jul/13  Resolved: 14/Mar/12

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

Type: Defect Priority: Major
Reporter: Yesudeep Mangalapilly Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: command, executable, linux, permissions, script, shell, unix, windows
Environment:

Any UNIX platform.



 Description   

Bash completion shows `bin/cljsc.bat` as an executable on UNIX systems
making it very easy to accidentally execute it. However, since
Windows command scripts do not work with UNIX shells, it is not
advisable to allow executable permissions to Windows batch scripts
as accidentally executing them may cause undefined behavior on such
systems.

I strongly suggest stripping UNIX shell executable permissions from
`bin/cljsc.bat`.

Thank you.

Cheers,
Yesudeep.



 Comments   
Comment by Yesudeep Mangalapilly [ 07/Mar/12 12:39 AM ]

The issue also affects:

bin/cljsc.bat
script/repljs.bat
script/repl.bat

Comment by Craig Andera [ 12/Mar/12 1:34 PM ]

I've verified (on both Ubuntu Linux and Windows 7) that stripping the x bit from those files has the desired effect.

The patch is almost too trivial to bother with listing here, but here it is regardless:

diff --git a/bin/cljsc.bat b/bin/cljsc.bat
old mode 100755
new mode 100644
diff --git a/script/repl.bat b/script/repl.bat
old mode 100755
new mode 100644
diff --git a/script/repljs.bat b/script/repljs.bat
old mode 100755
new mode 100644
Comment by David Nolen [ 14/Mar/12 7:22 PM ]

Fixed, https://github.com/clojure/clojurescript/commit/aa51a01141131736871e791918df63f185155421





[CLJS-294] AST contains munged symbols Created: 02/Jun/12  Updated: 27/Jul/13  Resolved: 03/Jun/12

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

Type: Defect Priority: Major
Reporter: Brandon Bloom Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: patch, patch,

Attachments: Text File emit-munge.patch    
Patch: Code

 Description   

All of the symbols in the abstract syntax tree are munged according for use in JavaScript. This is a problem because it means the AST is lossy and new compiler backends may have differing munging rules.

The attached patch removes all munging from the analysis phase and moves it into the emit phase. The emit phase is backend specific, so it's an appropriate place for munging.



 Comments   
Comment by David Nolen [ 03/Jun/12 9:55 AM ]

It may be better to formalize munging as there are some other bits different backend must do, Raphael Amiard has taken some notes on this here: http://github.com/raph-amiard/clojurescript/wiki/How-to-modularize-parse-analyze-phases-in-the-compiler

Comment by Brandon Bloom [ 03/Jun/12 11:33 AM ]

I agree that we'll eventually want to formalize munging. However, whenever that happens, it should still happen after the AST is constructed. Munging shouldn't happen at all if you're doing refactoring or the like, where you query the AST. Munging is code generation concern.

Prior to my patch, the AST needs to support both :name and :name-sym keys to get around the fact that the :name key loses fidelity of the original symbol. This complects the Clojure-specific parse phase with target-specific needs. The patch renames :name-sym to replace :name.

I read Raphael's notes and he and I have a email thread going on too. With that in mind, this is a step towards untangling the architectural layers of the compiler. By better segregating parse from emit, it will be easier to break the analyzer out from the code generation backend.

Comment by David Nolen [ 03/Jun/12 5:12 PM ]

fixed, http://github.com/clojure/clojurescript/commit/a9d1dc6a043e8a6367492a2cf8622f73f2a947f9





[CLJS-435] Stack overflow error when adding large numerical keys to maps Created: 05/Dec/12  Updated: 27/Jul/13  Resolved: 22/Dec/12

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

Type: Defect Priority: Major
Reporter: Praki Prakash Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None
Environment:

Linux 2.6.32-44-generic #98-Ubuntu SMP i686 GNU/Linux, java version "1.6.0_26", just bootstrapped clojurescript.



 Description   

The following code causes a stack overflow in Google Chrome browser and repljs. Transcript follows.

~/tools/clojurescript$  script/repljs
"Type: " :cljs/quit " to quit"
ClojureScript:cljs.user> (assoc {}
  154618822656 1
  261993005056 1)
"Error evaluating:" (assoc {} 154618822656 1 261993005056 1) :as "cljs.core.assoc.call(null,cljs.core.ObjMap.EMPTY,154618822656,1,261993005056,1);\n"
java.lang.StackOverflowError
        org.mozilla.javascript.NativeCall.<init>(NativeCall.java:65)
        org.mozilla.javascript.ScriptRuntime.createFunctionActivation(ScriptRuntime.java:3273)
        org.mozilla.javascript.gen.cljs_core_cljs_834._c__hash_1(cljs/core.cljs)
        org.mozilla.javascript.gen.cljs_core_cljs_834.call(cljs/core.cljs)
        org.mozilla.javascript.ScriptRuntime.applyOrCall(ScriptRuntime.java:2521)
        org.mozilla.javascript.BaseFunction.execIdCall(BaseFunction.java:300)
        org.mozilla.javascript.IdFunctionObject.call(IdFunctionObject.java:129)
        org.mozilla.javascript.optimizer.OptRuntime.call2(OptRuntime.java:76)
        org.mozilla.javascript.gen.cljs_core_cljs_909._c_anonymous_3(cljs/core.cljs:905)
        org.mozilla.javascript.gen.cljs_core_cljs_909.call(cljs/core.cljs)
        org.mozilla.javascript.ScriptRuntime.applyOrCall(ScriptRuntime.java:2521)
        org.mozilla.javascript.BaseFunction.execIdCall(BaseFunction.java:300)
        org.mozilla.javascript.IdFunctionObject.call(IdFunctionObject.java:129)
        org.mozilla.javascript.optimizer.OptRuntime.callN(OptRuntime.java:86)
        org.mozilla.javascript.gen.cljs_core_cljs_909._c_anonymous_5(cljs/core.cljs:913)
        org.mozilla.javascript.gen.cljs_core_cljs_909.call(cljs/core.cljs)
        org.mozilla.javascript.ScriptRuntime.applyOrCall(ScriptRuntime.java:2521)
        org.mozilla.javascript.BaseFunction.execIdCall(BaseFunction.java:300)
        org.mozilla.javascript.IdFunctionObject.call(IdFunctionObject.java:129)
        org.mozilla.javascript.optimizer.OptRuntime.callN(OptRuntime.java:86)
        org.mozilla.javascript.gen.cljs_core_cljs_909._c_anonymous_2(cljs/core.cljs:893)
        org.mozilla.javascript.gen.cljs_core_cljs_909.call(cljs/core.cljs)
        org.mozilla.javascript.ScriptRuntime.applyOrCall(ScriptRuntime.java:2521)
        org.mozilla.javascript.BaseFunction.execIdCall(BaseFunction.java:300)
        org.mozilla.javascript.IdFunctionObject.call(IdFunctionObject.java:129)
        org.mozilla.javascript.optimizer.OptRuntime.call2(OptRuntime.java:76)
        org.mozilla.javascript.gen.cljs_core_cljs_909._c_anonymous_5(cljs/core.cljs:911)
        org.mozilla.javascript.gen.cljs_core_cljs_909.call(cljs/core.cljs)
        org.mozilla.javascript.ScriptRuntime.applyOrCall(ScriptRuntime.java:2521)
        org.mozilla.javascript.BaseFunction.execIdCall(BaseFunction.java:300)
        org.mozilla.javascript.IdFunctionObject.call(IdFunctionObject.java:129)
        org.mozilla.javascript.optimizer.OptRuntime.call2(OptRuntime.java:76)
        org.mozilla.javascript.gen.cljs_core_cljs_1175._c_anonymous_2(cljs/core.cljs:4481)
        org.mozilla.javascript.gen.cljs_core_cljs_1175.call(cljs/core.cljs)
        org.mozilla.javascript.ScriptRuntime.applyOrCall(ScriptRuntime.java:2521)
        org.mozilla.javascript.BaseFunction.execIdCall(BaseFunction.java:300)
        org.mozilla.javascript.IdFunctionObject.call(IdFunctionObject.java:129)
        org.mozilla.javascript.optimizer.OptRuntime.callN(OptRuntime.java:86)
        org.mozilla.javascript.gen.cljs_core_cljs_1175._c_anonymous_4(cljs/core.cljs:4501)
        org.mozilla.javascript.gen.cljs_core_cljs_1175.call(cljs/core.cljs)
        org.mozilla.javascript.ScriptRuntime.applyOrCall(ScriptRuntime.java:2521)
        org.mozilla.javascript.BaseFunction.execIdCall(BaseFunction.java:300)
        org.mozilla.javascript.IdFunctionObject.call(IdFunctionObject.java:129)
        org.mozilla.javascript.optimizer.OptRuntime.callN(OptRuntime.java:86)
        org.mozilla.javascript.gen.cljs_core_cljs_1169._c_anonymous_12(cljs/core.cljs:4388)
        org.mozilla.javascript.gen.cljs_core_cljs_1169.call(cljs/core.cljs)
        org.mozilla.javascript.optimizer.OptRuntime.callN(OptRuntime.java:86)
        org.mozilla.javascript.gen.cljs_core_cljs_1175._c_anonymous_2(cljs/core.cljs:4486)
        org.mozilla.javascript.gen.cljs_core_cljs_1175.call(cljs/core.cljs)
        org.mozilla.javascript.ScriptRuntime.applyOrCall(ScriptRuntime.java:2521)
        org.mozilla.javascript.BaseFunction.execIdCall(BaseFunction.java:300)
        org.mozilla.javascript.IdFunctionObject.call(IdFunctionObject.java:129)


 Comments   
Comment by David Nolen [ 22/Dec/12 2:01 PM ]

fixed, http://github.com/clojure/clojurescript/commit/61455e64f58a07706c9b5ecebc9247bf085f7df1





[CLJS-103] read-keyword breaks in IE8 due to re-matches Created: 19/Nov/11  Updated: 27/Jul/13  Resolved: 21/May/12

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

Type: Defect Priority: Major
Reporter: Wilkes Joiner Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None
Environment:

Internet Explorer 8


Attachments: Text File 0001-handle-empty-string-for-re-exec-in-IE8.patch     Text File fix-keyword-no-ns.patch     Text File fix-reader-read-keyword.patch     Text File fix-read-keyword.patch    
Patch: Code

 Description   

read-keyword breaks in IE8. (read-keyword ":foo") => "\uFDD0'/foo" rather than "\uFDD0'foo"

The root cause is that (re-matches symbol-pattern token) returns [":foo" "" "foo"] instead of [":foo" undefined "foo"].



 Comments   
Comment by Wilkes Joiner [ 19/Nov/11 9:18 AM ]

The fix patches read-keyword, but I'm wondering what the expected behavior of re-matches should be. Should re-matches always return undefined for unmatched groups?

Another fix would be for (keyword "" "foo") to return :foo rather than :/foo.

Comment by Wilkes Joiner [ 19/Nov/11 9:54 AM ]

alternative fix for (keyword "" "foo")

Comment by David Nolen [ 20/Nov/11 12:15 PM ]

The reader patch is better. We should strive for performance in the reader. Please remove the comment about IE and change the test to:

(and ns (> (.length ns) 0))

I'll gladly apply the patch once this is done.

Comment by Wilkes Joiner [ 20/Nov/11 2:59 PM ]

Attached fix-reader-read-keyword.patch that uses .length rather than empty?.

Comment by David Nolen [ 21/Nov/11 9:28 AM ]

Fixed, https://github.com/clojure/clojurescript/commit/3966abb10f71764b0c92487b7f165e9bafe93eb6

Comment by Wilkes Joiner [ 21/May/12 10:24 AM ]

The empty string check was removed during some of the reader work. This is needed for IE8 and below.

Comment by David Nolen [ 21/May/12 11:02 AM ]

Do you have a new patch? Thanks!

Comment by Wilkes Joiner [ 21/May/12 11:13 AM ]

It's 0001-handle-empty-string-for-re-exec-in-IE8.patch

Thanks!

Comment by David Nolen [ 21/May/12 11:17 AM ]

Fixed, http://github.com/clojure/clojurescript/commit/c7525fe1a691745f2288144daa1d6092942cf465





[CLJS-398] new macro cljs.core/extend-instance Created: 19/Oct/12  Updated: 27/Jul/13  Resolved: 19/Oct/12

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

Type: Enhancement Priority: Major
Reporter: Herwig Hochleitner Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: enhancement, patch

Attachments: Text File 0001-new-macro-clojure.core-extend-instance-can-be-used-t.patch     Text File 0001-Test-for-extend-instance.patch    
Patch: Code and Test

 Description   

Patch introduces an extend-instance macro, similar to extend-type.
It doesn't extend the .prototype property, but assigns the prototype impl members to the instance itself.

This is useful to let existing instances, e.g. parsed JSON directly implement a protocol.



 Comments   
Comment by David Nolen [ 19/Oct/12 5:13 PM ]

Rich Hickey already had a good name for an operation like this - specify.

Comment by Herwig Hochleitner [ 19/Oct/12 6:10 PM ]

OK, I've declined the ticket, since not only the name but also the patches should be disregarded.

The problem with above impl is that the generated implementing fns get instantiated, every time extend-instance is evaluated. That is not acceptable on a per-instance basis.

I will work on a new macro called specify, which allows passing implementing fns.

Is a syntax similar to clojure.core/extend right for specify?
Should I create a new ticket?

Comment by David Nolen [ 19/Oct/12 7:51 PM ]

As far as I understand it the interface for specify should be the same as extend-type.





[CLJS-298] CLJS-294 broke ^:export Created: 03/Jun/12  Updated: 27/Jul/13  Resolved: 03/Jun/12

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

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

Attachments: Text File export-munge.patch    

 Description   

the fix for munging broke http://dev.clojure.org/jira/browse/CLJS-294 ^:export.



 Comments   
Comment by Brandon Bloom [ 03/Jun/12 9:55 PM ]

Whoops! I'll look into that a little later tonight (hopefully). I'll also see if I can get an export test case added to the suite.

Comment by David Nolen [ 03/Jun/12 10:59 PM ]

fixed, http://github.com/clojure/clojurescript/commit/2371c1cfcf1df15384f4eed337aeab0ab1d30689





[CLJS-291] Improve PHM performance, fix inode-find cases Created: 01/Jun/12  Updated: 27/Jul/13  Resolved: 03/Jun/12

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

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

Attachments: Text File 0001-CLJS-291-use-inode-lookup-instead-of-inode-find-supp.patch    

 Description   

inode-find implementations are under Object and thus are not subject to various optimizations - in particular multi-arity dispatch. local functions are create and returned - this is a big performance hit on all engines.



 Comments   
Comment by David Nolen [ 03/Jun/12 6:17 AM ]

I was going to take a look at this, but feel free to tackle it. I see no reason to support different arities at all in the Object case as they can't be optimized as far as I can tell.

Comment by Michał Marczyk [ 03/Jun/12 8:10 AM ]

Cool. Agreed on multiple arities on Object, will take that into account.

Comment by Michał Marczyk [ 03/Jun/12 6:49 PM ]

The attached patch wires in a new PHM method, inode-lookup, which always takes a not-found argument and returns just the value found under the given key, rather than a map entry / vector of [key val].

I also experimented with removing one arity of inode-find (passing in nil for not-found instead of calling the smaller arity overload); that resulted in a much smaller speedup.

This patch leaves inode-find implementations intact, as mentioned in the commit message. They can be removed by a subsequent patch or used in what I think would be a more Clojure-like find implementation.

I should say, given the present experience with inode-lookup I think I could write a much faster impl of inode-find anyway if that seems worthwhile. What do you think?

Comment by David Nolen [ 03/Jun/12 7:38 PM ]

Looks good, but it's also important to remove the extra inode-finds - having two of them causes production of non-optimal code. If you have a faster inode-find that sounds good too!

Comment by Michał Marczyk [ 03/Jun/12 7:55 PM ]

Ah, you're right, we only (potentially) need inode-find w/ not-found. Removed the default-to-nil overload in this patch, in addition to doing the same thing with inode-lookup the old one did.

I've also tested an approach w/ single-arity inode-find; that was faster than master, but slower than inode-lookup (i-f took about 1.5x the time on the relevant benchmarks on V8).

I'll play around with inode-find – can't make it faster than inode-lookup, but it just might be close. Guess that might be a separate ticket, though?

Comment by David Nolen [ 03/Jun/12 8:09 PM ]

fixed, http://github.com/clojure/clojurescript/commit/94b2f4aa626f76f807b110b8aa8c77b68041207b

Comment by David Nolen [ 03/Jun/12 9:30 PM ]

further improvements here: http://github.com/clojure/clojurescript/commit/4586ceb68922148607a8555b3caae968970163d1





[CLJS-292] type hints on fn arg list does not seem to work Created: 01/Jun/12  Updated: 27/Jul/13  Resolved: 04/Jun/12

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

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


 Comments   
Comment by David Nolen [ 04/Jun/12 7:33 PM ]

fixed, http://github.com/clojure/clojurescript/commit/f9d2c3b3917ea3e3754e68085a36fd7bc6df9c6c





[CLJS-379] optimize tail call in linear-traversal-nth Created: 19/Sep/12  Updated: 27/Jul/13  Resolved: 28/Sep/12

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

Type: Defect Priority: Major
Reporter: Tom Jack Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File cljs-379.patch    
Patch: Code and Test

 Description   

linear-traversal-nth is tail-recursive but doesn't call recur, so e.g. `(nth (iterate inc 0) 1e5)` can overflow the stack.

Attached patch cljs-379.patch (17 Sep 2012) fixes this.



 Comments   
Comment by David Nolen [ 28/Sep/12 5:49 PM ]

fixed, http://github.com/clojure/clojurescript/commit/0023c7a7b3a6c00826c9972e4417ee98163bada8





[CLJS-289] Represent ast :children as a vector of keys Created: 31/May/12  Updated: 27/Jul/13  Resolved: 05/Jun/12

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

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

Attachments: Text File children-keys.patch    
Patch: Code

 Description   

See discussion here: https://groups.google.com/d/topic/clojure-dev/vZLVKmKX0oc/discussion

Summary: Duplication of ast nodes in various keys and in :children is problematic for some users. In particular, it doesn't play nicely with printing. A solution is needed which preserves "The AST is data. Period."

The attached patch makes no changes to how the sub nodes are currently stored outside of the :children key. Instead, it replaces :children values with a vector of keys into the main ast node. This preserves child ordering and allows a children function to be defined as:

(defn children [ast]
(mapcat #(if (coll? %) % [%]) ((apply juxt (:children ast)) ast)))

The attached patch has two caveats: 1) Many (but not all?) blocks will now be present in the children hierarchy as 'do forms. 2) Interleaved forms are intentionally bugged with respect to ordering. The :children vector for those forms match the actual behavior, not the expected behavior. This can be fixed along with http://dev.clojure.org/jira/browse/CLJS-288



 Comments   
Comment by David Nolen [ 03/Jun/12 10:00 AM ]

the result of the discussion did not end with agreement on representing :children as a vector of keys

Comment by Brandon Bloom [ 03/Jun/12 11:24 AM ]

I realize that, but it was one approach suggested and seemed entirely viable, so I tried it. From what I can tell, it seems like a solution that meets all of the criteria that came up during the discussion.

Did I overlook a requirement?

Who is currently using :children and could weigh in on whether or not this still meets their needs?

Comment by Jonas Enlund [ 03/Jun/12 11:52 AM ]

I'm using :children and I don't think this patch (currently) meets my needs. At least {:op :let ...} is broken IMO. (something like `(map :init bes)` is missing from the patch)

Comment by Brandon Bloom [ 03/Jun/12 12:09 PM ]

Ah OK, you're right. It's the same problem there that I ran into with :statements and analyze-block: There is a pseudo AST node that needs to be traversed down into.

The easy fix for that here would be to merge {:children [:init]} into each binding, but the binding hash will be missing :op and :form. For analyze-block, I was able to use :op :do and :form (list 'do ...), but there isn't an obvious analogy here without inventing a :binding op or relaxing the requirement for :op and :form.

Interestingly, each successive binding can be treated as a single nested binding. What if we defined let as a macro which expands to a series of let1 forms? (let [x 1 y 2] (* x y)) --> (let1 [x 1] (let1 [y 2] (* x y))

That may increase the depth of the AST, but it would really simplify traversal into binding clauses. Each let op would have :name, :init, and children as [:init].

In general, it may be worth while to consider doing the same thing with the various analyze-blocks situations, such that 'do is the only place multiple statements can occur.

Comment by Brandon Bloom [ 03/Jun/12 12:16 PM ]

Er, I mean: a hypothetical let1 op would have children [:init :statements :ret]

However, if you also expanded the implicit do blocks, you'd be able to simplify to [:init :body] or something like that.

Comment by Brandon Bloom [ 03/Jun/12 12:34 PM ]

Forgive me for repeatedly commenting, but it also occurs to me that you can change 'do to a macro which expands to a series of 'do2 forms:

(do x y z) --> (do2 x (do2 y z)) or (do2 (do2 x y) z)

That do2 op could have children [:left :right] and all other :statements and :ret children could be simplified down to 'do2 expansions.

With that in mind, the children fn becomes simply (defn children [ast] ((apply juxt (:children ast)) ast)) and a lot of analyze and emit code gets much simpler by not having to map over many collections all over the place.

Kinda a wild idea and probably not the right forum for it, but worth suggesting. Thoughts?

Comment by David Nolen [ 04/Jun/12 11:35 AM ]

I see little benefit to changing core macros. I also see no problem with creating a :binding ast node.

Comment by Jonas Enlund [ 05/Jun/12 12:44 AM ]

Every node (i.e. an map with :op, :form and :env keys, called an "Expression Object" in the docstring for analyze) in ClojureScript corresponds to a self-contained expression. A binding form is not self-contained, it is a part of the let expression.

Another similar example is functions. There is a node {:op :fn ...} but there is no {:op :fn-method} even thou there are maps under the :methods key describing function methods. The same argument goes for this: A function method is not self-contained, it is part of the function and should not be a node.

The nodes (expression objects) which make up the ClojureScript AST seems to be really well thought out and I would be careful in making the proposed change.

Comment by David Nolen [ 05/Jun/12 8:09 AM ]

Agreed I think it's far too early to have a ticket for this. Confluence page first.

Comment by Brandon Bloom [ 29/Aug/12 12:09 AM ]

Design page here: http://dev.clojure.org/display/design/AST+children





[CLJS-448] Support for runtime reading of tagged elements Created: 21/Dec/12  Updated: 27/Jul/13  Resolved: 22/Dec/12

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

Type: Enhancement Priority: Major
Reporter: Tom Hickey Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: File prefixed-tags-20121221.diff    
Patch: Code and Test

 Description   

We ned the abilty to supply custom and default tag parsers when using cljs.reader/read-string (found in src/cljs/cljs/reader.cljs) in running ClojureScript applications.

ns prefixed tags

As per the edn specification "user tags must contain a prefix component". Currently, because (name tag) is being called, the ns prefix component is dropped when looking up the tag in *tag-table*

default reader fn

Currently, there is no support for supplying a default reader fn (akin to *default-data-reader-fn* in Clojure proper).

2 issues rolled into one?

I know this may be bad practice rolling 2 issues into one, but I do not know how to formulate the separate patches that affect the same lines of code and have the apply correctly regardless of order. If I need to correct this, please advise.

Already reported?

I know this issue was already raised in CLJS-335, but from my experience with making these modifications for a running ClojureScript app, I did not come across the fns in the files mentioned in that task. I assume this is because that had more to do with compiling cljs code, but please correct me if I'm wrong.

Along those lines, is this patch not acceptable if it doesn't fix both places at once?

Approach & Naming

I followed the approach that was already in place for tag parsers, supplying register and deregister helper fns. I was a bit torn on naming because in Clojure *default-data-reader-fn* is used, but in the existing ClojureScript code they are called tag-parser. Should the "reader" vs "parser" nomenclature be unified between Clojure and ClojureScript, or are they named differently because they are fundamentally different in some way that I am missing?



 Comments   
Comment by David Nolen [ 21/Dec/12 5:03 PM ]

I don't think we can have a unified solution without a reader that is shared both at compile time and run time and I think the benefits today of run time support trumps the possible future convenience of compile time support. I don't think there is any good reason for the difference in naming - I'm inclined to keep things in line with Clojure as much as possible.

Happy to move forward with this one unless there are objections from others.

Comment by David Nolen [ 22/Dec/12 1:40 PM ]

fixed, http://github.com/clojure/clojurescript/commit/19d265dfc262ae93b9d6b1be5bf13905ef5445c6

Comment by kovas boguta [ 25/Dec/12 1:42 PM ]

My problem in CLJS-335 was that you couldn't compile cljs code with embedded user-defined tagged literals.

The code in this ticket is also great. But it would be even better if there was a generic function for reading tagged literals, known or unknown, that had the exact same argument structure as the the tag reader functions.

My solution for CLJS-335 is to read the TL into code that resolves and invokes the tag reader function on the CLJS side.

However it will fail for unknown tags, and will not fall back on the default reader functionality defined in the solution for this ticket. It would be trivial to fix this, if there was a single function that my solution can use that handled both known and unknown tags.

Otherwise, my solution can be extended with more code, to first check for a specific reader, and then fall back on the default reader.





[CLJS-308] Support types & protocols as type hints Created: 07/Jun/12  Updated: 27/Jul/13  Resolved: 08/Jun/12

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

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


 Description   

If we have a type / protocol type hint we can direct dispatch in regular fns too.



 Comments   
Comment by David Nolen [ 08/Jun/12 6:12 PM ]

fixed, http://github.com/clojure/clojurescript/commit/8e944175668044eebc3912b2831515e99f524c9a





[CLJS-296] empty PersistentQueue seems to have one null element Created: 03/Jun/12  Updated: 27/Jul/13  Resolved: 05/Jun/12

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

Type: Defect Priority: Major
Reporter: Tom Jack Assignee: Michał Marczyk
Resolution: Completed Votes: 0
Labels: None
Environment:

clojurescript @ a9d1dc6a


Attachments: Text File 0001-CLJS-296-seq-should-return-nil-on-empty-PersistentQu.patch    

 Description   

The following code prints "nil" once:

(doseq [x cljs.core.PersistentQueue/EMPTY] (println x))



 Comments   
Comment by Michał Marczyk [ 05/Jun/12 1:07 AM ]

This is because PQ implements -seq to return () on empty instances. The attached patch fixes this.

Comment by David Nolen [ 05/Jun/12 8:02 PM ]

fixed, http://github.com/clojure/clojurescript/commit/281811f67e7c3e722e1572db3c824e4465ac74aa

Comment by Tom Jack [ 05/Jun/12 8:05 PM ]

Thanks!





[CLJS-473] cljs.closure/add-dep-string calls wrong munge Created: 18/Feb/13  Updated: 27/Jul/13  Resolved: 19/Feb/13

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

Type: Defect Priority: Major
Reporter: Jozef Wagner Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None
Environment:

latest CLJS


Attachments: File cljs-473.diff    

 Description   

cljs.closure/add-dep-string calls wrong munge (Clojure one), while it should be calling cljs.compiler/munge.

The problem shows up when namespace contains a reserved js name and incremental dev build is performed.



 Comments   
Comment by David Nolen [ 19/Feb/13 8:50 AM ]

fixed, http://github.com/clojure/clojurescript/commit/896a1421ed720628f8d1a58c9a97050f2885d4bb





[CLJS-166] Strange compiler error under compilation mode - affects lein-cljsbuild Created: 23/Mar/12  Updated: 27/Jul/13  Resolved: 24/Mar/12

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

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


 Description   

Under advanced compilation, the compiler seems to emit strange error especially when a CLJS source file depends on another.

http://github.com/emezeske/lein-cljsbuild/issues/54

I encountered this while trying to create a test suite for the core.logic CLJS port.



 Comments   
Comment by David Nolen [ 24/Mar/12 4:51 PM ]

not a bug, just a user classpath error.





[CLJS-168] -lookup on Records merges base fields with ext-map every time Created: 25/Mar/12  Updated: 27/Jul/13  Resolved: 26/Mar/12

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

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


 Description   

Switch to a cond/p + lookup in ext-map in else clause



 Comments   
Comment by David Nolen [ 26/Mar/12 11:12 AM ]

Fixed, https://github.com/clojure/clojurescript/commit/d49fb7db8b959de105d9cd2528bc542cd81d5ac1





[CLJS-303] (case e ...) evaluates e multiple times Created: 05/Jun/12  Updated: 27/Jul/13  Resolved: 06/Jun/12

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

Type: Defect Priority: Major
Reporter: Brandon Bloom Assignee: Michał Marczyk
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 0001-CLJS-303-fix-case-so-it-only-evaluates-its-first-arg.patch    

 Description   

(case (do (pr 1) 2) 3 4 5 6 7) ; CLJS prints 117, CLJ prints 17



 Comments   
Comment by Michał Marczyk [ 05/Jun/12 11:09 PM ]

The attached patch fixes the case macro so that its first arg is only evaluated once.

Comment by David Nolen [ 06/Jun/12 11:22 AM ]

fixed, http://github.com/clojure/clojurescript/commit/77d29466c7c8598a04d2f7d0ca0f05b5f3e9f154





[CLJS-501] add boolean coercion fn Created: 28/Apr/13  Updated: 27/Jul/13  Resolved: 28/Apr/13

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

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


 Description   

we often type-hint return values as boolean when they are not, we should instead provide a boolean coercion fn like Clojure has.



 Comments   
Comment by David Nolen [ 28/Apr/13 10:59 PM ]

this exists





[CLJS-534] chunked-seq? hard-wired for concrete types is problematic for RRB trees Created: 04/Jul/13  Updated: 27/Jul/13  Resolved: 12/Jul/13

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

Type: Defect Priority: Major
Reporter: Michał Marczyk Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 0001-CLJS-534-use-ternary-satisfies-in-chunked-seq.patch    

 Description   

RRB trees can support chunked seqs without a problem, but they need their own implementation, as the one in cljs.core uses PersistentVector's array-for. Since chunked-seq? is currently hard-wired to check for the built-in chunked seq types as an optimization for for and doseq, it returns false for such custom chunked seqs.

This can be solved by using satisfies? with false passed in as the extra check-native argument. Happily the performance difference is negligible.

Patch forthcoming.



 Comments   
Comment by Michał Marczyk [ 04/Jul/13 1:02 AM ]

Did I mention that ternary satisfies? is awesome?

Comment by David Nolen [ 12/Jul/13 8:31 AM ]

Heh, I've been thinking that we need another test, perhaps called implements? which does what satisfies? false does now - I think it be less of an eye sore and communicate the difference.

Comment by David Nolen [ 12/Jul/13 8:36 AM ]

fixed, http://github.com/clojure/clojurescript/commit/1b9eae2df729cdfc73e883edaddfdd88cd46e75a





[CLJS-305] direct protocol dispatch from within deftype / defrecord / extend-type Created: 07/Jun/12  Updated: 27/Jul/13  Resolved: 08/Jun/12

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

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


 Description   

We actually have a lot of information around deftype / defrecord / extend-type. We could use this information to emit direct dispatches to protocol implementations and avoid going through protocol fns.



 Comments   
Comment by David Nolen [ 07/Jun/12 9:32 AM ]

We needs to store all the protocols a type implements. We then should add type meta to the this argument of the protocol fn implementations. This should actually result in a reasonable performance boost across the board.

Comment by David Nolen [ 08/Jun/12 6:11 PM ]

fixed, http://github.com/clojure/clojurescript/commit/8e944175668044eebc3912b2831515e99f524c9a





[CLJS-363] `format` %s behavior is incorrect for keyword, symbol etc. Created: 29/Aug/12  Updated: 27/Jul/13  Resolved: 29/Aug/12

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

Type: Defect Priority: Major
Reporter: Shantanu Kumar Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: bug
Environment:

ClojureScript Rhino REPL



 Description   

On the Clojure REPL, `format` works fine:

user=> (format "foo%s" :s) 
"foo:s" 
user=> (format "foo%s" 's) 
"foos"

However, on the CLJS REPL (Rhino), the output is different:

ClojureScript:cljs.user> (format "foo%s" :s) 
"foo 's" 
ClojureScript:cljs.user> (format "foo%s" 's) 
"foo 's"

Reference: http://groups.google.com/group/clojure/browse_thread/thread/b253d810536a4046



 Comments   
Comment by David Nolen [ 29/Aug/12 7:54 PM ]

fixed, http://github.com/clojure/clojurescript/commit/bf0622a594473c9a6de57fe3b5d10e0419fc7a2a





[CLJS-310] Use backend agnostic munging function for namespace resolution Created: 09/Jun/12  Updated: 27/Jul/13  Resolved: 11/Jun/12

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

Type: Enhancement Priority: Major
Reporter: Raphaël AMIARD Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File munge-path.patch    
Patch: Code

 Description   

ns->relpath uses the js specific munge function for path resolution.

A patch is attached that uses a backend agnostic munge function.



 Comments   
Comment by David Nolen [ 09/Jun/12 8:09 PM ]

Excellent! Been a bit busy will try to apply this tomorrow!

Comment by David Nolen [ 10/Jun/12 1:18 PM ]

Patch looks good. Did you verify that analysis still follows namespace dependencies w/ this patch applied?

Comment by Raphaël AMIARD [ 11/Jun/12 5:46 AM ]

Yes i did, i think it works just the same as before

Comment by David Nolen [ 11/Jun/12 7:23 AM ]

fixed, http://github.com/clojure/clojurescript/commit/3db771065ebda9d23ba5f52994b6e4f99c28e11f





[CLJS-170] Implement Transient PersistentVectors Created: 28/Mar/12  Updated: 27/Jul/13  Resolved: 26/Apr/12

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

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

Attachments: Text File 0001-TransientVector-ported-from-Clojure.patch    

 Comments   
Comment by Michał Marczyk [ 25/Apr/12 9:48 PM ]

Applies cleanly on top of the current master.

Branch @

https://github.com/michalmarczyk/clojurescript/tree/transient-vector

This patch passes all currently existing tests and doesn't break the reader through the use of transient vectors in cljs.core.PersistentVector/fromArray, but includes no new tests for parts of the transient functionality other than simple conj!-ing followed by persistent!; I'll write those sometime (very) soon, but since unfortunately I need to take a break for now, here's the patch for anybody interested in taking it for a spin.

Comment by Michał Marczyk [ 26/Apr/12 5:20 AM ]

New patch against current master, branch @

https://github.com/michalmarczyk/clojurescript/tree/transient-vector-2

Includes some bug fixes and a test suite.

Comment by David Nolen [ 26/Apr/12 7:51 AM ]

fixed, https://github.com/clojure/clojurescript/commit/788b9eb451d1dbbef2e0cf4536ff6d178997ede5





[CLJS-481] doseq bindings with :let not properly scoped Created: 03/Mar/13  Updated: 27/Jul/13  Resolved: 09/Apr/13

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

Type: Defect Priority: Major
Reporter: Gary Fredericks Assignee: David Nolen
Resolution: Completed Votes: 1
Labels: None


 Description   

This seems to be a subtler version of CLJS-39:

(let [fs (atom [])]
  (doseq [x (range 4)
          :let [y (inc x)
                f (fn [] y)]]
    (swap! fs conj f))
  (map #(%) @fs))

;; => (4 4 4 4)
;; should be (1 2 3 4)

I think this example is minimal. The bug is not present if we use a proper let.



 Comments   
Comment by Michał Marczyk [ 06/Apr/13 4:49 PM ]

It appears that this works correctly on current master. In fact, it worked correctly on r1576 – but not on r1552. Also, splitting the :let into two ("adjacent") :let's fixes this, whereas moving both the y and f bindings into a single regular let inside the doseq does not. That means it's probably an instance of CLJS-442 (fns defined in local init exprs not closing over earlier locals), fixed in https://github.com/clojure/clojurescript/commit/84e9488f49bcfea4b4037a562f8f797c7ddd79f0 (included in releases since r1576).

Comment by David Nolen [ 08/Apr/13 4:53 PM ]

So I'm assuming we can just close this one?

Comment by Michał Marczyk [ 08/Apr/13 7:50 PM ]

Yeah, seems so.

Comment by David Nolen [ 09/Apr/13 3:07 AM ]

added test, this test passes on master, http://github.com/clojure/clojurescript/commit/a1b41bed8921dbd9ef9ee7d735b158ae71ed6132

Comment by Gary Fredericks [ 11/Apr/13 9:34 PM ]

Sounds good to me. Thanks!





[CLJS-272] support :require without :as Created: 25/May/12  Updated: 27/Jul/13  Resolved: 12/Jun/12

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

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

Attachments: Text File 0001-CLJS-272-support-refer-and-skipping-as-in-require.patch     Text File 0001-CLJS-272-support-require-without-as.patch    

 Comments   
Comment by Michał Marczyk [ 11/Jun/12 8:31 PM ]

To be applied on top of CLJS-312. Adds support for [lib.ns] and lib.ns specs in :require.

Comment by David Nolen [ 12/Jun/12 7:55 AM ]

It would be preferable if this patch could be applied alone as there's less to consider.

Comment by Michał Marczyk [ 12/Jun/12 9:35 PM ]

Hm, that's true. New patch attached, but it's different, see below.

While writing this patch I thought it would be nice also to provide support for :refer – since I'm already working on :require... The patch thus ended up being a bit of an overhaul of :require and :use handling. Here's the commit message:

CLJS-272: support :refer and skipping :as in :require

The :as alias part of a :require spec can now be omitted.
Alternatively, a symbol naming the namespace to be required without an
alias can be specified without wrapping it in a vector.

Additionally, :require accepts a :refer option with :use-like effect.

(ns foo.core
  (:require
    ;; bar-fn will be available unqualified
    [lib.bar :as bar :refer [bar-fn]]
    ;; :as lib.baz is implicit
    [lib.baz]
    ;; likewise
    lib.quux))

Either, both or none of :as, :refer may be specified in each :require
spec.

:require-macros supports the same options.

:use / :use-macros is now supported by rewriting the specs as :require
/ :require-macros specs with :refer.

:require / :require-macros now produce problem-specific error
messages which include the offending spec.

Tests are provided for :refer and the new handling of :use.

I wonder if parse-require-spec should be a factored out of parse 'ns (something appropriate would need to be done about error-msg – not sure which possibility is best).

Comment by Michał Marczyk [ 12/Jun/12 9:56 PM ]

Just in case, here's a patch which introduces pretty much the smallest change necessary to support :require without :as.

I very much prefer the previous patch, though, not least because of the error messages.

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

fixed, http://github.com/clojure/clojurescript/commit/9179a355a87021a8011ff9f0b32a4db8ee22dfcd





[CLJS-495] New undefined? macro that safely handles undeclared variables Created: 16/Apr/13  Updated: 27/Jul/13  Resolved: 18/Apr/13

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

Type: Enhancement Priority: Major
Reporter: Kevin Lynagh Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None


 Description   

The current `undefined?` function will blow up with a JavaScript reference error if you pass it an undeclared variable.

It'd be nice to have a way to test if a JavaScript variable has been declared.
My use case is writing a ClojureScript library that extends cljs protocols to JavaScript things.
I'd like to write

(when-not (undefined? js/jQuery)

...)

to extend-type jQuery when jQuery is around.



 Comments   
Comment by David Nolen [ 18/Apr/13 10:53 AM ]

What about bound? as a name for this macro?

Comment by Kevin Lynagh [ 18/Apr/13 11:59 AM ]

Yeah, `bound?` sounds like an appropriate name to me.

Comment by David Nolen [ 18/Apr/13 2:36 PM ]

Stuart Sierra pointed out that `bound?` may have too many connotations already in Clojure. Maybe `exists?`?

Comment by David Nolen [ 18/Apr/13 5:15 PM ]

fixed http://github.com/clojure/clojurescript/commit/3ee148acac436dd489396fc6783ba72bbd7ff79f





[CLJS-400] Stop using goog.string.quote to escape strings for printing Created: 19/Oct/12  Updated: 27/Jul/13  Resolved: 22/Oct/12

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

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

Attachments: File CLJS-400.diff    
Patch: Code and Test

 Description   

The contract of goog.string.quote is to produce a "valid JavaScript string", which can end up being quite different from what is expected by e.g. the Clojure reader and other edn-compatible readers. This has been discussed in a variety of threads/issues:

The most significant issue is that characters > 127 and < 256 are outputted with an \x.. escape sequence, which no other reader supports. Further, other Unicode characters are always escaped using the \u.... notation; this is not a functional difference, but can be a stumbling block, e.g. when viewing data that had been pr-str'd that contains high codepoint characters.

The most straightforward fix is to take control of printed string escaping to ensure the results conform first to the expectations of existing readers (primarily Clojure's and ClojureScript's), and to the formal [edn specification](https://github.com/edn-format/edn/) as that matures.

This ticket supersedes http://dev.clojure.org/jira/browse/CLJ-1025.



 Comments   
Comment by Chas Emerick [ 19/Oct/12 7:50 PM ]

Patch attached

Comment by David Nolen [ 19/Oct/12 8:13 PM ]

Thanks. Performance things - can we avoid instantiating the RegExp everytime, can we switch char-escapes to a JS object and use aget?

Comment by Chas Emerick [ 19/Oct/12 9:11 PM ]

Patch CLJS-400-fast.diff added with suggested perf tweaks. Please excuse my (still) absolutely shoddy cljs instincts.

Comment by David Nolen [ 20/Oct/12 9:42 AM ]

Using the let won't work here. That will be emit top-levels that are not namespaced. Edge case in the compiler that should be sorted out on another ticket. Just define top levels for these thanks.

Comment by Chas Emerick [ 22/Oct/12 2:03 PM ]

New patch attached with requested changes, CLJS-400.diff.

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

fixed, http://github.com/clojure/clojurescript/commit/3b32345966a65f66e49563a3a40a2877d0435c5d





[CLJS-357] Large PersistentVector benchmarks Created: 17/Aug/12  Updated: 27/Jul/13  Resolved: 29/Aug/12

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

Type: Enhancement Priority: Major
Reporter: Michał Marczyk Assignee: Michał Marczyk
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 0001-CLJS-357-large-PersistentVector-benchmarks.patch    

 Description   

We need benchmarks for large data structures, e.g. large PVs with shifts greater than 5 (a shift of 20 would perhaps be problematic due to the necessary size of the vector, so probably 10 or 15).

My first cut at a patch (to be attached in a moment) provides benchmarks for building a large vector without using transients (to get an idea of conj perf), conjing onto a large vector in a situation where this involves a push-tail operation, associng in non-tail position in a large vector and popping a large vector where this involves a pop-tail op.

The push-tail / pop-tail benchmarks, as written here, are dependent on the exact position of the tail inside the vector; as an alternative, we could benchmark conjing / popping 32 times (max tail length is probably going to be pretty stable). Thoughts?



 Comments   
Comment by Michał Marczyk [ 17/Aug/12 4:25 AM ]

As mentioned in the discussion on CLJS-299, these benchmarks could use some scrutiny in case they're not testing what they should be. Also, I wonder if the iteration counts should be greater.

Comment by David Nolen [ 21/Aug/12 5:04 PM ]

Will take a look and report back.

Comment by David Nolen [ 29/Aug/12 7:56 PM ]

Looks good to me.

Comment by David Nolen [ 29/Aug/12 8:00 PM ]

fixed, http://github.com/clojure/clojurescript/commit/52820a1ececb3a66660551102fc456ba43ccafb6





[CLJS-334] (new (some-complex-expr) ...) results in incomprehensible error Created: 03/Jul/12  Updated: 27/Jul/13  Resolved: 29/Aug/12

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

Type: Defect Priority: Major
Reporter: Michał Marczyk Assignee: Michał Marczyk
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 0001-CLJS-334-produce-sane-error-message-when-ctor-arg-to.patch    

 Description   

Here's a link to a conversation with kovasb on #clojure (can't seem to find it in Chouser's logs for some reason):

http://rotty.xx.vu/irclogs/freenode/%23clojure/2012-06-30/#e598

The problem is that new always fails if given a complex expression (= not a symbol) as the ctor argument. This used not to be the case in earlier ClojureScript releases; the reason it fails now is due to number-of-fields validation not guarding against the non-symbol-ctor-argument case.

I see two possible solutions: (1) restore the old behaviour while preserving the number-of-fields validation for symbolic ctor args; (2) mandate symbolic-ctor-only usage of new and throw an error with a useful message in parse 'new. I'm leaning towards (1), so the forthcoming patch takes this approach. Either approach is easy enough to implement. (I think the way parse 'new is written suggests that allowing complex ctor expressions was the original intention; I could be wrong about this though.)



 Comments   
Comment by Michał Marczyk [ 03/Jul/12 11:08 PM ]

Allow non-symbolic-ctor args to new while still providing field count warnings when given a symbolic arg.

Comment by Michał Marczyk [ 03/Jul/12 11:11 PM ]

Improved commit message.

Comment by David Nolen [ 10/Jul/12 11:16 AM ]

This doesn't work in Clojure, you have to refer to a real class. Unless we hear otherwise I see no reason to diverge in this case. Let's get a better error message.

Comment by Michał Marczyk [ 15/Aug/12 5:04 AM ]

Here's a patch taking this approach (that is with an assertion requiring that ctor arg to new be a symbol).

Comment by David Nolen [ 29/Aug/12 8:39 PM ]

fixed, http://github.com/clojure/clojurescript/commit/dae155a34cd30dabba94f90e306afac930345dcf





[CLJS-312] Support :import Created: 11/Jun/12  Updated: 27/Jul/13  Resolved: 30/Aug/12

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

Type: Enhancement Priority: Major
Reporter: