<< Back to previous view

[CLJS-824] Unsigned hash for keywords produced with (keyword s) Created: 06/Jul/14  Updated: 16/Jul/14  Resolved: 16/Jul/14

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

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

Attachments: Text File cljs_824-2.patch     Text File cljs_824.patch    

 Description   

ClojureScript will produce unsigned hash values at times (in particular for keywords produced with the keyword fn), and this is inconsistent with hashes produced on literal keywords and inconsistent with Clojure. The hash values will have the same 32-bit hex value, so this is perhaps not that critical for many uses.

Examples:

cljs.user=> (hash :op)
;=> -1882987955

cljs.user=> (hash (keyword "op"))
2411979341

This unsigned value is produced on Safari (shipping and WebKit nightly), and thus is in theory produced via both variants of the imul implementation (see CLJS-823). The unsigned value is also produced by Firefox.

In either case the value is equivalent to 0x8fc3e24d and works for many use cases, but it fails an = test.

user=> (= (hash :op) (hash (keyword "op")))
;=> true

cljs.user=> (= (hash :op) (hash (keyword "op")))
;=> false



 Comments   
Comment by Mike Fikes [ 06/Jul/14 8:27 PM ]

Not sure of the perf, but the attached cljs_824.patch appears to resolve the issue.

Comment by Mike Fikes [ 06/Jul/14 10:04 PM ]

Attached a simpler cljs_824-2.patch:

The JavaScript idiom to coerce to signed 32-bit is x|0. The revised patch does this, but by simply invoking the cljs core int fn (which in turn does a bit-or with 0).

Comment by David Nolen [ 16/Jul/14 4:32 PM ]

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





[CLJS-825] Conflict between cljs/nodejs.cljs & cljs/nodejs.js Created: 09/Jul/14  Updated: 16/Jul/14  Resolved: 16/Jul/14

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

Type: Defect Priority: Major
Reporter: Boris Kourtoukov Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: nodejs


 Description   

This is the gist with errors and bare minimal example: https://gist.github.com/BorisKourt/7b1434ec490cfa6c8397



 Comments   
Comment by Boris Kourtoukov [ 09/Jul/14 2:15 PM ]

Noticed this while working with the main.cljs file linked in the above gist.

As described by David Nolen: cljs/nodejs.cljs cljs/nodejs.js are getting mixed up by the compiler even though they are completely different.

This explains why the error is intermittent, but the warning is consistent.

This is reproducible with an even more minimal example as well. Just wanted to link a basic but functional one.

Comment by John Wang [ 12/Jul/14 12:21 AM ]

As a more convenient test case, this also happens with samples/nodehello.cljs when following the build instructions given in the comments of that file.

Comment by David Nolen [ 16/Jul/14 4:27 PM ]

fixed https://github.com/clojure/clojurescript/commit/95122e1206d4a64f392cff03bfd73712ab7f3a33





[CLJS-827] wrap macro expansion in try/catch Created: 14/Jul/14  Updated: 16/Jul/14  Resolved: 16/Jul/14

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 cljs_827.patch    

 Description   

This would allow us to report more accurate error file/line location when macroexpansions fails in Clojure, i.e. in the case of `defn`.



 Comments   
Comment by Max Veytsman [ 15/Jul/14 12:28 AM ]

It turned out that there was already a `wrapping-errors` macro that did what we needed. I used it within the body of `macroexpand-1` in order to annotate macro expansion errors with line numbers.

Comment by David Nolen [ 16/Jul/14 7:53 AM ]

fixed https://github.com/clojure/clojurescript/commit/13501318b4967eb5d0e486907e6cb35da81d60b9





[CLJS-826] Closure goog/base.js in third party closure release Created: 09/Jul/14  Updated: 16/Jul/14  Resolved: 16/Jul/14

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

Type: Defect Priority: Trivial
Reporter: Thomas Heller Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None
Environment:

Recent google/closure-library checkout


Attachments: Text File cljs-closure-release-script-fix.patch    
Patch: Code

 Description   

Not sure when this started but the more recent "org.clojure/google-closure-library-third-party" releases started containing a dummy "goog/base.js" which break build using my shadow-build library. The "make-closure-library-jars.sh" script used to delete that base.js but I guess the directory changed (maybe when the repo moved to github).

Patch just deletes the new location. Could possibly just forget about the old location, happy to provide a patch that replaces the paths instead of adding them.

Proof:

jar -tvf org/clojure/google-closure-library-third-party/0.0-20140226-71326067/google-closure-library-third-party-0.0-20140226-71326067.jar 
     0 Fri Mar 07 15:18:58 CET 2014 META-INF/
    68 Fri Mar 07 15:18:58 CET 2014 META-INF/MANIFEST.MF
   533 Fri Mar 07 15:18:56 CET 2014 AUTHORS
 10173 Fri Mar 07 15:18:56 CET 2014 LICENSE
   293 Fri Mar 07 15:18:56 CET 2014 README
     0 Fri Mar 07 15:18:56 CET 2014 goog/
   101 Fri Mar 07 15:18:56 CET 2014 goog/base.js
   822 Fri Mar 07 15:18:56 CET 2014 goog/deps.js


 Comments   
Comment by Thomas Heller [ 09/Jul/14 7:11 PM ]

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

This commit is to blame I guess, so its fine to just change the paths instead of keeping the old ones arround (since they never existed).

Comment by David Nolen [ 16/Jul/14 7:50 AM ]

fixed https://github.com/clojure/clojurescript/commit/13501318b4967eb5d0e486907e6cb35da81d60b9





Generated at Tue Jul 22 22:03:37 CDT 2014 using JIRA 4.4#649-r158309.