<< Back to previous view

[CLJ-1409] Add support for marking gen-class methods as native Created: 21/Apr/14  Updated: 21/Apr/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.6
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Alex Miller Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: gen-class, interop


 Description   

As far as I know, there is no support for creating a Java instance in Clojure with native methods. Everything else needed exists, but there is no way to get the right annotation on the method right now (similar to static).

Here's an example (http://benchmarksgame.alioth.debian.org/u64q/program.php?test=pidigits&lang=clojure&id=4) from Alioth perf tests where ASM is being used directly to generate a class with native methods where gen-class would have been perfectly adequate with this enhancement. (Equivalent Java: http://benchmarksgame.alioth.debian.org/u64q/program.php?test=pidigits&lang=java&id=2).

Suggested implementation is to mark ^{:native true} on a method and omit the body.






[CLJ-1408] Add transient keyword to cached toString() value in _str Created: 19/Apr/14  Updated: 20/Apr/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.6
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Tomasz Nurkiewicz Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJ-1408.patch    

 Description   

_str field in Keyword and Symbol classes lazily caches result of toString(). Because this field is not transient, serializing (using Java serialization) any keyword before and after calling toString() for the first time yields different results:

(import (java.io ByteArrayOutputStream ObjectOutputStream
                 ByteArrayInputStream  ObjectInputStream))

(defn- serialize [obj]
  (with-open [bos (ByteArrayOutputStream.)
              stream (ObjectOutputStream. bos)]
    (.writeObject stream obj)
    (-> bos .toByteArray seq)))

(def s1 (serialize :k))
;(println :k)
(def s2 (serialize :k))
(println (= s1 s2))

Uncomment (println :k) and suddenly s1 and s2 are no longer equal.

This issue came up when I was trying to use keywords as key in [Hazelcast](https://github.com/hazelcast/hazelcast) map. Hazelcast uses serialized keys in various scenarios, thus if I first put something to map under key :k and then print :k, I can no longer find such key.

Attaching patch against current master, will sign CLA if you agree with this issue.



 Comments   
Comment by Alex Miller [ 19/Apr/14 7:28 AM ]

Hi Tomasz, it would be good to fix this, can you sign the CLA?

Comment by Tomasz Nurkiewicz [ 20/Apr/14 7:26 AM ]

Thanks, I'll sign and send CLA ASAP.





[CLJ-1407] Recur mismatch might cause multiple evaluation Created: 17/Apr/14  Updated: 17/Apr/14

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

Type: Defect Priority: Minor
Reporter: Nicola Mometto Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: compiler, macro


 Description   

Since mismatching recurs cause the loop body to be re-analyzed, macroexpansion in the loop body might happen more than once, causing any side effects that happen during macroexpansion to be evaluated potentially multiple times

Clojure 1.7.0-master-SNAPSHOT
user=> (defmacro x [] (println "foo"))
#'user/x
user=> (fn [] (loop [y 1] (x) (recur (Integer. 1))))
foo
foo
#<user$eval6$fn__7 user$eval6$fn__7@71687585>


 Comments   
Comment by Andy Fingerhut [ 17/Apr/14 6:59 PM ]

This is not a question about whether the behavior in the description is a bug or not, but rather a curiosity about how often people write macros that have side effects at macroexpansion time. I think the following in Clojure itself do, but there may be others:

  • gen-class, and also ns because it uses gen-class
  • gen-interface, and also definterface because it uses gen-interface
  • clojure.core/compile-if (private) calls eval on its expr arg, but as used now doesn't cause macroexpansion-time side effects
  • doc seems to have one case that prints at macroexpansion time
  • I am not sure whether defprotocol or deftype have macroexpansion time side effects, or whether they are limited to run time
Comment by Nicola Mometto [ 17/Apr/14 9:20 PM ]

Andy, I don't think there are that many macros that side-effect at macroexpansion time and I haven't discovered this bug in real code but while thinking about how loop locals invalidation was implemented in Compiler.java.

Because there are a really a small number of side-effecting macros, this is unlikely to cause problems in real code, so I changed the priority to minor.





[CLJ-1406] Libs are blindly added into loaded-libs even if an error occurs during loading Created: 17/Apr/14  Updated: 17/Apr/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5, Release 1.6
Fix Version/s: None

Type: Defect Priority: Major
Reporter: OHTA Shogo Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File 0001-modify-clojure.core-load-lib-so-that-it-removes-the-.patch    
Patch: Code

 Description   

Suppose you have a lib that causes some errors during loading, like the following:

(ns broken-lib)

(} ; this line will cause a reader error

And then, if you require the lib, it would be added into loaded-libs in spite of the reader error, which makes require succeed silently after that.

user=> (contains? (loaded-libs) 'broken-lib)
false
user=> (require 'broken-lib)
CompilerException java.lang.RuntimeException: Unmatched delimiter: }, compiling:(broken_lib.clj:3:3) 
user=> (contains? (loaded-libs) 'broken-lib)
true
user=> (require 'broken-lib)
nil
user=>

Cause:
The patch for CLJ-1116 made the ns macro blindly add the lib being defined into loaded-libs even if an error occurs during loading.

Approach:
Modify clojure.core/load-lib so that it removes the lib from loaded-libs on error.



 Comments   
Comment by Alex Miller [ 17/Apr/14 9:07 AM ]

This patch seems somewhat removed from the cause - is there some way to instead prevent the lib from being added to loaded-libs in the first place?

Comment by OHTA Shogo [ 17/Apr/14 9:21 AM ]

To do so, I think we need to revert CLJ-1116.





[CLJ-1405] clojure runtime does not work with onejar-maven-plugin Created: 16/Apr/14  Updated: 16/Apr/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5, Release 1.6, Release 1.7
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Sean Shubin Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

I was able to repeat this problem on windows, linux, and osx


Attachments: Text File onejar-maven-plugin-fix.patch    
Patch: Code

 Description   

I have created a sample project, steps to repeat the problem, and a proposed patch here:
https://github.com/SeanShubin/clojure-one-jar

I have also attached my proposed patch to this ticket, and pasted the relevant portion of README.md below:

clojure-one-jar
===============

Sample project to demonstrate problem with combining clojure runtime with onejar-maven-plugin

Steps to repeat the problem
===========================

  • mvn package
  • java -jar target/greeter.jar

Behavior before patch is applied
================================
Exception in thread "main" java.lang.reflect.InvocationTargetException
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:483)
at com.simontuffs.onejar.Boot.run(Boot.java:340)
at com.simontuffs.onejar.Boot.main(Boot.java:166)
Caused by: java.lang.ExceptionInInitializerError
at com.seanshubin.clojure_one_jar.Greeter.main(Greeter.java:12)
... 6 more
Caused by: java.lang.NullPointerException
at clojure.lang.RT.lastModified(RT.java:387)
at clojure.lang.RT.load(RT.java:421)
at clojure.lang.RT.load(RT.java:411)
at clojure.lang.RT.doInit(RT.java:447)
at clojure.lang.RT.<clinit>(RT.java:329)
... 7 more

Behavior after patch is applied
===============================
No need to call RT.init() anymore
Hello, world!

Patch
=====
I applied this patch to my local copy of clojure 1.7.0-master-SNAPSHOT
https://github.com/SeanShubin/clojure-one-jar/blob/master/onejar-maven-plugin-fix.patch



 Comments   
Comment by Alex Miller [ 16/Apr/14 9:45 PM ]

I think this is a dupe of CLJ-971 - can you verify?

Comment by Sean Shubin [ 16/Apr/14 9:49 PM ]

Yes, I looked over CLJ-971 and can confirm this matches my observations regarding why this is happening.

Comment by Sean Shubin [ 16/Apr/14 9:54 PM ]

My solution is a bit different in that it still tries to get the date of the file rather than defaulting to the date of the entire jar. I don't have a solid enough understanding of the calling code to know if this is important.

Comment by Alex Miller [ 16/Apr/14 10:17 PM ]

FYI, to consider your patch, I would need you to sign a Contributor Agreement - see http://clojure.org/contributing for details. I do think that is an area that something is needed, will need to assess the options a bit more.

Comment by Sean Shubin [ 16/Apr/14 10:56 PM ]

Sure thing, I just signed and sent a Contributor Agreement, per the instructions at http://clojure.org/contributing





[CLJ-1403] ns-resolve might throw ClassNotFoundException but should return nil Created: 14/Apr/14  Updated: 14/Apr/14

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

Type: Defect Priority: Minor
Reporter: Nicola Mometto Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None


 Description   

The doc of ns-resolve states that in case the symbol cannot be resolved, it should return nil.

user=> (doc ns-resolve)
-------------------------
clojure.core/ns-resolve
([ns sym] [ns env sym])
  Returns the var or Class to which a symbol will be resolved in the
  namespace (unless found in the environment), else nil.  Note that
  if the symbol is fully qualified, the var/Class to which it resolves
  need not be present in the namespace.
nil

However if the symbol contains dots and is not a resolvable Class, a ClassNotFoundException is thrown

user=> (ns-resolve *ns* 'foo.bar)
ClassNotFoundException foo.bar  java.net.URLClassLoader$1.run (URLClassLoader.java:372)
user=> (pst *e)
ClassNotFoundException foo.bar
	java.net.URLClassLoader$1.run (URLClassLoader.java:372)
	java.net.URLClassLoader$1.run (URLClassLoader.java:361)
	java.security.AccessController.doPrivileged (AccessController.java:-2)
	java.net.URLClassLoader.findClass (URLClassLoader.java:360)
	clojure.lang.DynamicClassLoader.findClass (DynamicClassLoader.java:61)
	java.lang.ClassLoader.loadClass (ClassLoader.java:424)
	java.lang.ClassLoader.loadClass (ClassLoader.java:357)
	java.lang.Class.forName0 (Class.java:-2)
	java.lang.Class.forName (Class.java:340)
	clojure.lang.RT.classForName (RT.java:2065)
	clojure.lang.Compiler.maybeResolveIn (Compiler.java:6963)
	clojure.core/ns-resolve (core.clj:4026)
nil


 Comments   
Comment by Alex Miller [ 14/Apr/14 2:07 PM ]

Can you include the (pst *e) ?

Comment by Nicola Mometto [ 14/Apr/14 2:10 PM ]

Added result of (pst *e) in the description





[CLJ-1402] sort-by calls keyfn more times than is necessary Created: 11/Apr/14  Updated: 11/Apr/14

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

Type: Enhancement Priority: Minor
Reporter: Steve Kim Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: performance


 Description   

clojure.core/sort-by evaluates keyfn for every pairwise comparison. This is wasteful when keyfn is expensive to compute.

user=> (def keyfn-calls (atom 0))
#'user/keyfn-calls
user=> (defn keyfn [x] (do (swap! keyfn-calls inc) x))
#'user/keyfn
user=> @keyfn-calls
0
user=> (sort-by keyfn (repeatedly 10 rand))
(0.1647483850582695 0.2836687590331822 0.3222305842748623 0.3850390922996001 0.41965440953966326 0.4777580378736771 0.6051704988802923 0.659376178201709 0.8459820304223701 0.938863131161208)
user=> @keyfn-calls
44


 Comments   
Comment by Steve Kim [ 11/Apr/14 11:46 AM ]

CLJ-99 is a similar issue





[CLJ-1401] CompilerException / IllegalStateException when reloading namespaces Created: 10/Apr/14  Updated: 12/Apr/14

Status: Reopened
Project: Clojure
Component/s: None
Affects Version/s: Release 1.6
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Mike Anderson Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: compiler, errormsgs


 Description   
user> (ns op)
nil
op> (defn * [a b] (clojure.core/* a b))
WARNING: * already refers to: #'clojure.core/* in namespace: op, being replaced by: #'op/*
#'op/*
op> (ns use-op (:require [op :refer :all]))
WARNING: * already refers to: #'clojure.core/* in namespace: use-op, being replaced by: #'op/*
nil
use-op> (ns use-op (:require [op :refer :all]))
IllegalStateException * already refers to: #'op/* in namespace: use-op  clojure.lang.Namespace.warnOrFailOnReplace (Namespace.java:88)
use-op> (clojure.repl/pst *e)
IllegalStateException * already refers to: #'op/* in namespace: use-op
	clojure.lang.Namespace.warnOrFailOnReplace (Namespace.java:88)
	clojure.lang.Namespace.reference (Namespace.java:110)
	clojure.lang.Namespace.refer (Namespace.java:168)
	clojure.core/refer (core.clj:3920)
	use-op/eval2402/loading--4958--auto----2403 (NO_SOURCE_FILE:1)
	use-op/eval2402 (NO_SOURCE_FILE:1)
	clojure.lang.Compiler.eval (Compiler.java:6703)
	clojure.lang.Compiler.eval (Compiler.java:6692)
	clojure.lang.Compiler.eval (Compiler.java:6666)
	clojure.core/eval (core.clj:2927)
	clojure.main/repl/read-eval-print--6625/fn--6628 (main.clj:239)
	clojure.main/repl/read-eval-print--6625 (main.clj:239)

I would expect (at worst) a similar warning to the initial namespace loading, rather than an exception here.



 Comments   
Comment by Alex Miller [ 11/Apr/14 8:26 AM ]

Could you put together a better reproducible test case for this that does not depend on core.matrix? Also, please include the (pst *e) when it occurs.

Comment by Andy Fingerhut [ 11/Apr/14 10:19 AM ]

I have tried the smallest possible Leiningen project I could think of that would cause the warnings about redefinitions, to see if I could get the exception to occur. 'lein new try1' to create the skeleton project, then edit src/try1/core.clj to contain only the following function definitions:

(defn merge
  "This definition of merge replaces clojure.core/merge"
  [x y]
  (- x y))

(defn *
  [x y]
  (* x y))

Then start a REPL with 'lein repl', and I see this behavior:

user=> (require '[try1.core :as c])
WARNING: merge already refers to: #'clojure.core/merge in namespace: try1.core, being replaced by: #'try1.core/merge
WARNING: * already refers to: #'clojure.core/* in namespace: try1.core, being replaced by: #'try1.core/*
nil
user=> (require '[try1.core :as c] )
nil
user=> (require '[try1.core :as c] :reload)
WARNING: merge already refers to: #'clojure.core/merge in namespace: try1.core, being replaced by: #'try1.core/merge
WARNING: * already refers to: #'clojure.core/* in namespace: try1.core, being replaced by: #'try1.core/*
nil

Ths all looks like behavior as I would expect, and I did not see the exception that Mike reports.

It seems that either Ctrl+Alt+L in Counterclockwise does something different than (require ... :reload), or there is something different about Mike's namespace in addition to redefining names in clojure.core that is causing the problem.

Comment by Alex Miller [ 11/Apr/14 11:17 AM ]

Marking this as NR for now - would be happy to see it reopened with an easily reproducible test case.

Comment by Mike Anderson [ 12/Apr/14 12:41 AM ]

To reproduce:

(ns op)
(defn * [a b] (clojure.core/* a b)) ;; gives warning
(ns use-op (:require [op :refer :all])) ;; gives warning
(ns use-op (:require [op :refer :all])) ;; gives error!

I believe Counterclockwise is simply loading the namespace again with CTRL-Alt+L, which is causing the ns form to be re-executed.

The docstring implies that ns can be used multiple times ("Sets ns to the namespace named by name (unevaluated), creating it if needed") so I would certainly expect multiple invocations of ns to be a no-op





[CLJ-1399] missing field munging when recreating deftypes serialized in to byte code Created: 02/Apr/14  Updated: 02/Apr/14

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

Type: Defect Priority: Major
Reporter: Kevin Downey Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None

Attachments: File clj-1399.diff    
Patch: Code

 Description   

to embed deftypes in the bytecode the compiler emits the value of each field, then emits a call to the deftypes underlying class's constructor.

to get a list of fields the compiler calls .getBasis.

the getBasis fields are the "clojure" level field names of the deftype, which the actual "jvm" level field names have been munged (replacing - with _, etc), so the compiler tries to generate code to set values on non-existent fields

https://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/Compiler.java#L4579

https://www.refheap.com/70731

you can work around this by using field names that don't require munging. a solution might be just calling munge in the emission of field sets of ITypes



 Comments   
Comment by Kevin Downey [ 02/Apr/14 4:26 PM ]

reproducing case

$ rlwrap java -server -Xmx1G -Xms1G -jar /Users/hiredman/src/clojure/target/clojure-1.6.0-master-SNAPSHOT.jar
Clojure 1.6.0-master-SNAPSHOT
user=> (deftype Foo [hello-world])
user.Foo
user=> (alter-var-root #'default-data-readers assoc 'foo (fn [x] (Foo. x)))
{foo #<user$eval6$fn__7 user$eval6$fn__7@2f953efd>, inst #'clojure.instant/read-instant-date, uuid #'clojure.uuid/default-uuid-reader}
user=> #foo "1"
CompilerException java.lang.IllegalArgumentException: No matching field found: hello-world for class user.Foo, compiling:(NO_SOURCE_PATH:0:0)
user=>
Comment by Kevin Downey [ 02/Apr/14 4:39 PM ]

this patch fixes the issue on the latest master for me

Comment by Chas Emerick [ 02/Apr/14 4:57 PM ]

FWIW, this was precipitated by real experience (I think I created the refheap paste). The workaround is easy (don't use dashes in field names of deftypes you want to return from data reader functions), but I wouldn't expect anyone to guess that that wasn't already oversensitized to munging edge cases.





[CLJ-1398] Update URLs in javadoc.clj Created: 02/Apr/14  Updated: 04/Apr/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.7
Fix Version/s: None

Type: Enhancement Priority: Trivial
Reporter: Eli Lindsey Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File 0001-update-apache-commons-javadoc-location.patch     Text File 0002-add-javadoc-lookup-for-guava-and-apache-commons-lang.patch     Text File 0003-add-javadoc-lookup-for-jdk8.patch    
Patch: Code

 Description   

Three minor fixes/enhancements to javadoc.clj:

0001 corrects the URLs for apache commons javadoc (the ones used in javadoc.clj no longer resolve).
0002 adds javadoc lookup for guava and apache commons lang3.
0003 adds javadoc lookup for jdk8.

(Note: contributor agreement is in the mail)



 Comments   
Comment by Andy Fingerhut [ 04/Apr/14 11:22 AM ]

Eli, thanks for the patches. It appears that you are not currently on the list of Clojure contributors here: http://clojure.org/contributing

It is the policy of the Clojure team only to incorporate patches submitted by people who have signed and submitted a Clojure CA. Were you interested in doing that?

Comment by Eli Lindsey [ 04/Apr/14 11:27 AM ]

> It is the policy of the Clojure team only to incorporate patches submitted by people who have signed and submitted a Clojure CA. Were you interested in doing that?

Yup! I mailed off the CA to Rich on Wednesday when this was filed; should be arriving shortly.





[CLJ-1394] Print multi method dispatch values in the exception messages. Created: 31/Mar/14  Updated: 01/Apr/14

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

Type: Enhancement Priority: Trivial
Reporter: Roman Scherer Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: errormsgs

Attachments: File pr-str-dispatch-value-safe.diff    
Patch: Code and Test

 Description   

The error messages of defmulti are at the moment not as helpful as they could be under certain circumstances. Calling this multi method with a lazy seq as it's dispatch argument raises the following exception:

(defmulti test-multi identity)
(defmethod test-multi 1 [arg] true)

(test-multi (map identity [:x]))
;=> java.lang.IllegalArgumentException: No method in multimethod 'test-multi' for dispatch value: clojure.lang.LazySeq@3c6f1187

Sometimes it would be useful to actually see which values are in the lazy seq being dispatched on. A better error message could look like
this for example:

(test-multi (map identity [:x]))
;=> java.lang.IllegalArgumentException: No method in multimethod 'test-multi' for dispatch value (:x) of class clojure.lang.LazySeq

This patch addresses this issue by formatting the dispatch value via `pr-str` and printing the class before it is passed to the exception constructor. The same is also done for the methods in MultiFn.java that throw a dispatch value as part of their exception message.



 Comments   
Comment by Nicola Mometto [ 31/Mar/14 8:22 PM ]

What if the value is infinite lazy-seq?

Comment by Roman Scherer [ 01/Apr/14 2:50 AM ]

Nicola, I forgot those. But I think infinite sequences could be handled with:

(set! *print-length* 10)

I'll try it out and will update the patch later.

Any other edge cases in mind?

Comment by Roman Scherer [ 01/Apr/14 2:28 PM ]

After having read "Controlling run-away trains, onions, and exercise
bikes" [1] I now bind print-length and print-size when building
the error message. This helps when not being able to dispatch on this
for example:

(test-multi (let [x (atom 0)] (reset! x {:deeper x})))

However I'm not sure if this helps in the following case, where
dispatching would fail on an infinite seq.

(test-multi (iterate inc 0))

The above doesn't terminate in Clojure 1.6.0, nor does it when binding
print-length like the attached patch does.

[1] http://blog.n01se.net/blog-n01se-net-p-85.html





[CLJ-1391] Allow logical operators on assert expressions Created: 26/Mar/14  Updated: 26/Mar/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Sanel Zukan Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: clojure.test


 Description   

With current code, it is not possible to express logical operators on some clojure.test assert expressions. For example, this will work:

(is (thrown? Exception <some-expression>))

however, here will fail:

(is (not (thrown? Exception <some-expression>)))

since '(thrown?)' is not an ordinary function, but looks like. This also adds confusion which is hard to explain to others unless '(is)' code was shown first.

Also, if the one would like to implement macro (e.g. 'is-not-thrown?') in form:

(defmacro is-not-thrown? [e expr]
  `(is (not ('thrown? ~e ~expr))))

which could be even more confusing for a person not knowing how 'thrown?' is implemented.






[CLJ-1390] pprint a GregorianCalendar results in Arity exception Created: 25/Mar/14  Updated: 10/Apr/14

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

Type: Defect Priority: Minor
Reporter: Steve Suehs Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: print

Attachments: Text File 0001-CLJ-1390.patch     Text File 0002-CLJ-1390-test2.patch     Text File 0002-CLJ-1390-test.patch     Text File CLJ-1390-pprint-GregorianCalendar.patch    
Patch: Code

 Description   

What I was doing: attempting to pretty-print nested structures from Things mac app, which include instances of java.util.GregorianCalendar.
What I expected to happen: output should have an #inst very much like printing java.util.Date.
What happened instead: ArityException Wrong number of args (4) passed to: pprint$pretty-writer$fn

thingsplay.core=> (def nowish (java.util.GregorianCalendar.))
#'thingsplay.core/nowish
thingsplay.core=> nowish
#inst "2014-03-25T22:43:29.240-05:00"
thingsplay.core=> (require 'clojure.pprint)
nil
thingsplay.core=> (pprint nowish)
ArityException Wrong number of args (4) passed to: pprint$pretty-writer$fn  clojure.lang.AFn.throwArity (AFn.java:437)
#inst "
thingsplay.core=> (simple-dispatch nowish)
#inst "2014-03-25T22:43:29.240-05:00"nil
thingsplay.core=> nowish
#inst "2014-03-25T22:43:29.240-05:00"
thingsplay.core=> (write nowish)
ArityException Wrong number of args (4) passed to: pprint$pretty-writer$fn  clojure.lang.AFn.throwArity (AFn.java:437)
#inst "


 Comments   
Comment by Norman Richards [ 29/Mar/14 4:15 PM ]

The print-calendar function introduced in CLJ-928 doesn't work with clojure.pprint/pretty-writer since pretty-writer does not correctly implement the java.io.Writer interface. It only implements write(String) but print-calendar wants write(String,int,int) among others. Although pretty-writer should probably correctly implement java.io.Writer, it's pretty easy to rewrite print-calendar to use the supported subset of java.io.Writer that is implemented.

Comment by Steve Suehs [ 29/Mar/14 4:20 PM ]

Thank you, "random person at the Austin Clojure Hack Day" "that I don't know" that has a CA in place. You are awesome!

See you at the next Austin Clojure Meetup.

-s

Comment by Andy Fingerhut [ 04/Apr/14 2:51 PM ]

Norman, it would be good if the patch included a few test cases, especially ones that fail without the patch, and succeed with the patch.

Comment by Norman Richards [ 04/Apr/14 3:09 PM ]

Absolutely. I have no idea how test cases work on Clojure core, but I assume it should be easy enough to do.

Comment by Andy Fingerhut [ 04/Apr/14 3:15 PM ]

I would recommend looking at the following file in the Clojure repo, which already contains other pprint tests. It shouldn't be too difficult to get an idea from one or more of the tests there. Actually those might be slightly unusual in that many of them use a simple-tests macro defined in file test_helper.clj that most of the Clojure tests do not use, but ask questions if you have trouble, e.g. on the Clojure Google group or IRC channel.

test/clojure/test_clojure/pprint/test_pretty.clj

Comment by Steve Suehs [ 04/Apr/14 7:50 PM ]

Alright...you two are inspiring me to go work on getting my CA in.

Comment by Norman Richards [ 04/Apr/14 8:31 PM ]

Test case attached. Apply the test patch, "mvn test" fails. Apply the fix, test passes.

Comment by Andy Fingerhut [ 06/Apr/14 3:32 PM ]

It would be better if the "is" were of the form:

(is (= calculated-value "constant string to compare against") "string to show if test fails")

rather than just (is calculated-value "string to show if test fails"). The second form will fail if calculating the value throws an exception, but only the first form will calculate it, and then verify that the value is the expected one (and fail if it is not the expected one).

Comment by Norman Richards [ 07/Apr/14 10:49 AM ]

Ok - here's an alternative test case. I'm less happy with this test case, since I have to add the TimeZone and make assumptions about how the specifics of how the pretty printer formats. But, it does test the fix adequately, so if you like the test2 patch better, that's perfectly fine with me.

Comment by Steve Miner [ 10/Apr/14 4:23 PM ]

I would rather fix the actual bug in pretty_writer.clj. The proxy needs to support more of the java.io.Writer interface. I think adding another arity to the write method would work. Something like:

(write 
   ...
  ([x off len]
      (.write this (subs (str x) off (+ off len)))))
Comment by Steve Miner [ 10/Apr/14 4:38 PM ]

CLJ-1390-pprint-GregorianCalendar.patch fixes the pretty_writer.clj proxy to support the missing version of the write method. Includes the same test as the previous patch.





[CLJ-1389] Re-loading a namespace ignores metadata specified for the namespace Created: 20/Mar/14  Updated: 20/Mar/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Howard Lewis Ship Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: metadata, namespace, repl


 Description   

Using the REPL I added some metadata to a namespace and reloaded it.

(ns io.aviso.rook-test5)

to

(ns io.aviso.rook-test5
  "A testing namespace"
  {:inherted   :namespace
   :overridden :namespace})

But requesting the meta data yields nil:

(-> 'io.aviso.rook-test5 find-ns meta)
=> nil

I have tested a few variations, such as putting the metadata on the symbol instead of providing an attribute map. In all cases, the metadata from before the load persists.

Using remove-ns before re-loading the namespace does the right thing ... the metadata shows up as expected.






[CLJ-1385] Docstrings for `conj!` and `assoc!` should suggest using the return value; effect not always in-place Created: 16/Mar/14  Updated: 06/Apr/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.6
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Pyry Jahkola Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: collections, docstring

Attachments: Text File CLJ-1385-reword-docstrings-on-transient-update-funct.patch    

 Description   

The docstrings of both `assoc!` and `conj!` say "Returns coll.", suggesting the transient edit happens always in-place, `coll` being the first argument.

However, the fact that the following example omits the key `8` in its result proves that in-place edits aren't always the case:

(let [a (transient {})]
      (dotimes [x 9]
        (assoc! a x :ok))
      (persistent! a))
    ;;=> {0 :ok, 1 :ok, 2 :ok, 3 :ok, 4 :ok, 5 :ok, 6 :ok, 7 :ok}

Instead, programmers should be guided towards using constructs like `reduce` with transients:

(persistent! (reduce #(assoc! %1 %2 :ok)
                 (transient {})
                 (range 9)))
    ;;=> {0 :ok, 1 :ok, 2 :ok, 3 :ok, 4 :ok, 5 :ok, 6 :ok, 7 :ok, 8 :ok}

The easiest way to achieve this is by changing the docstrings of (at least) `conj!` and `assoc!` to not read "Returns coll." but instead tell that the change is destructive.



 Comments   
Comment by Alex Miller [ 16/Mar/14 8:49 AM ]

When modifying transient collections, it is required to use the collection returned from functions like assoc!. The ! here indicates its destructive nature. The transients page (http://clojure.org/transients) describes the calling pattern pretty explicitly: "You must capture and use the return value in the next call."

I do not agree that we should be guiding programmers away from using functions like assoc! – transients are used as a performance optimization and using assoc! or conj! in a loop is often the fastest version of that. However I do think it would be helpful to make the docstring more explicit.

Comment by Gary Fredericks [ 05/Apr/14 10:23 AM ]

Alex I think you must have misread the ticket – the OP is suggesting guiding toward using the return value of assoc!, not avoiding assoc! altogether.

And the docstring is not simply inexplicit, it's actually incorrect specifically in the case that the OP pointed out. conj! and assoc do not return coll at the point where array-maps transition to hash-maps, and the fact that they do otherwise is supposed to be an implementation detail as far as I understand it.

Comment by Alex Miller [ 05/Apr/14 11:55 AM ]

@Gary - you're right, I did misread that.

assoc and conj both explicitly say "return a new collection" whereas assoc! and conj! say "Returns coll." I read that as "returns the modified collection" without regard to whether it's the identical instance, but I can read it your way too.

Would saying "Returns updated collection." transmit the right idea? Using "collection" instead of "coll" removes the concrete tie to the variable and "updated" hints more strongly that you should use the return value.

Comment by Pyry Jahkola [ 05/Apr/14 12:47 PM ]

@Alex, that update makes it sound right to me, FWIW.

Comment by Gary Fredericks [ 05/Apr/14 2:37 PM ]

Yeah, I think that's better. Thanks Alex. I'd be happy to submit a patch for that but I'm assuming patches are too heavy for this kind of change?

Comment by Andy Fingerhut [ 06/Apr/14 3:35 PM ]

Patches are exactly what has been done in the past for this kind of change, if it is in a doc string and not on the clojure.org web page.

Comment by Alex Miller [ 06/Apr/14 4:13 PM ]

Yup, patch desired.

Comment by Gary Fredericks [ 06/Apr/14 5:32 PM ]

Glad I asked.

Patch is attached that also updates the docstring for pop! which had the same issue, though arguably it's less important since afaik pop! does always return the identical collection (but I don't think this is part of the contract).





[CLJ-1383] Should name throw on nil? Created: 14/Mar/14  Updated: 15/Mar/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.6
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: John Chijioke Assignee: Jozef Wagner
Resolution: Unresolved Votes: 0
Labels: None

Attachments: File clj-1383.diff    
Patch: Code and Test

 Description   

The name function throws NullPointerException on nil. Since the name function is about obtaining the string form of a specific object it should not throw on nil. It should just return the nil object as the str fn does.



 Comments   
Comment by Jozef Wagner [ 15/Mar/14 3:23 AM ]

added patch with test





[CLJ-1381] Improve support for extending protocols to primitive arrays Created: 13/Mar/14  Updated: 13/Mar/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5, Release 1.6
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Alex Miller Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: protocols


 Description   

It is possible to extend protocols to primitive arrays but specifying the class for the type is a little tricky:

(defprotocol P (p [_]))
(extend-protocol P (Class/forName "[B") (p [_] "bytes"))
(p (byte-array 0))   ;; => "bytes"

However, things go bad if you try to do more than one of these:

(extend-protocol P 
  (Class/forName "[B") (p [_] "bytes")
  (Class/forName "[I") (p [_] "ints"))
CompilerException java.lang.UnsupportedOperationException: nth not supported on this type: Character, compiling:(NO_SOURCE_PATH:1:1)
	clojure.lang.Compiler.analyze (Compiler.java:6380)
	clojure.lang.Compiler.analyze (Compiler.java:6322)
	clojure.lang.Compiler$MapExpr.parse (Compiler.java:2879)
	clojure.lang.Compiler.analyze (Compiler.java:6369)
	clojure.lang.Compiler.analyze (Compiler.java:6322)
	clojure.lang.Compiler$InvokeExpr.parse (Compiler.java:3624)
	clojure.lang.Compiler.analyzeSeq (Compiler.java:6562)
	clojure.lang.Compiler.analyze (Compiler.java:6361)
	clojure.lang.Compiler.analyze (Compiler.java:6322)
	clojure.lang.Compiler$BodyExpr$Parser.parse (Compiler.java:5708)
	clojure.lang.Compiler$FnMethod.parse (Compiler.java:5139)
	clojure.lang.Compiler$FnExpr.parse (Compiler.java:3751)
Caused by:
UnsupportedOperationException nth not supported on this type: Character
	clojure.lang.RT.nthFrom (RT.java:857)
	clojure.lang.RT.nth (RT.java:807)
	clojure.core/emit-hinted-impl/hint--5951/fn--5953 (core_deftype.clj:758)
	clojure.core/map/fn--4207 (core.clj:2487)
	clojure.lang.LazySeq.sval (LazySeq.java:42)
	clojure.lang.LazySeq.seq (LazySeq.java:60)
	clojure.lang.RT.seq (RT.java:484)
	clojure.lang.RT.countFrom (RT.java:537)
	clojure.lang.RT.count (RT.java:530)
	clojure.lang.Cons.count (Cons.java:49)
	clojure.lang.Compiler.analyze (Compiler.java:6352)
	clojure.lang.Compiler.analyze (Compiler.java:6322)

The code in {parse-impls} is seeing the second {(Class/forName "[I")} as a function, not as a new type. One workaround for this is to only extend the protocol to one type at a time.

It would be even better (moving into enhancement area) if there was a syntax here to specify primitive array types - we already have the syntax of {bytes, ints, longs}, etc for type hints and those seem perfectly good to me.






[CLJ-1380] Three-arg ExceptionInfo constructor permits nil data Created: 13/Mar/14  Updated: 25/Mar/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Gordon Syme Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File clj-1380.patch    
Patch: Code and Test

 Description   

The argument check in the two-arg clojure.lang.ExceptionInfo constructor isn't present in the three-arg constructor so it's possible to create an ExceptionInfo with arbitrary (or nil) data.

E.g.:

user=> (clojure-version)
"1.5.1"

user=> (ex-info "hi" nil)
IllegalArgumentException Additional data must be a persistent map: null  clojure.lang.ExceptionInfo.<init> (ExceptionInfo.java:26)

user=> (ex-info "hi" nil (Throwable.))
NullPointerException   clojure.lang.ExceptionInfo.toString (ExceptionInfo.java:40)


 Comments   
Comment by Gordon Syme [ 13/Mar/14 10:47 AM ]

Sorry, didn't meant to classify as "major" and I don't have permissions to edit.

Comment by Gordon Syme [ 13/Mar/14 11:11 AM ]

Patch + tests

I'm not at all familiar with the project so may have put tests in the wrong language and/or wrong place.

The ex-info-works test is a bit dorky but shows that both constructors are equivalent (and passes without the patch to ExceptionInfo).

Comment by Alex Miller [ 13/Mar/14 12:18 PM ]

No worries on the classification - I adjust most incoming tickets in some way or another.

Thanks for the patch, however it cannot be considered unless you complete the Clojure Contributor's Agreement - http://clojure.org/contributing. This is an important step in the process that keeps the Clojure codebase on a sound legal basis.

Someone else could develop a clean room patch implementation for this ticket later, but of course it would be ideal if you could become a contributor!

Comment by Gordon Syme [ 13/Mar/14 1:15 PM ]

Hi Alex,

sure, that makes sense. I'll get the contributor's agreement in the post. It may take a while to arrive since I'm based in Europe.

Comment by Gordon Syme [ 25/Mar/14 10:03 AM ]

I just checked http://clojure.org/contributing, looks like my CCA made it through





[CLJ-1379] Quoting of :actual form is incorrect in clojure.test :pass type maps Created: 12/Mar/14  Updated: 12/Mar/14

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

Type: Defect Priority: Minor
Reporter: Hugo Duncan Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: test
Environment:

All clojure versions


Attachments: File fix-quoting-in-pass-case.diff    
Patch: Code

 Description   

The function symbol is not correctly quoted in the construction of the :actual value in a :pass type map for clojure.test.

It currently produces (list = 1 1) instead of (list '= 1 1) for an (is (= 1 1)) test.

I haven't been able to come up with a workaround for this.






[CLJ-1376] Initialize internal maps to more efficient version Created: 11/Mar/14  Updated: 11/Mar/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.6
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Alex Miller Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: performance


 Description   

In reviewing some hashing stuff, I noticed that there are many places internal to Clojure that use maps initialized with PersistentHashMap.EMPTY. Many of these maps are likely to have a small number of entries such that a PersistentArrayMap might be more efficient.

These are the candidates:

src/jvm/clojure/lang/ARef.java
19:private volatile IPersistentMap watches = PersistentHashMap.EMPTY;

src/jvm/clojure/lang/Compiler.java
3009:				IPersistentMap m = PersistentHashMap.EMPTY;
3819:					       KEYWORDS, PersistentHashMap.EMPTY,
3820:					       VARS, PersistentHashMap.EMPTY,
3964:	IPersistentMap closes = PersistentHashMap.EMPTY;
3977:	IPersistentMap keywords = PersistentHashMap.EMPTY;
3978:	IPersistentMap vars = PersistentHashMap.EMPTY;
5121:                            ,CLEAR_SITES, PersistentHashMap.EMPTY
7259:			       KEYWORDS, PersistentHashMap.EMPTY,
7260:			       VARS, PersistentHashMap.EMPTY
7418:			IPersistentMap opts = PersistentHashMap.EMPTY;
7475:			IPersistentMap fmap = PersistentHashMap.EMPTY;
7522:					       KEYWORDS, PersistentHashMap.EMPTY,
7523:					       VARS, PersistentHashMap.EMPTY,
7912:                            ,CLEAR_SITES, PersistentHashMap.EMPTY

src/jvm/clojure/lang/LispReader.java
755:					RT.map(GENSYM_ENV, PersistentHashMap.EMPTY));

src/jvm/clojure/lang/MultiFn.java
39:	this.methodTable = PersistentHashMap.EMPTY;
41:	this.preferTable = PersistentHashMap.EMPTY;
49:		methodTable = methodCache = preferTable = PersistentHashMap.EMPTY;

src/jvm/clojure/lang/Var.java
48:	final static Frame TOP = new Frame(PersistentHashMap.EMPTY, null);
175:	setMeta(PersistentHashMap.EMPTY);
341:	IPersistentMap ret = PersistentHashMap.EMPTY;

Approach: Two possible approaches - initialize to PersistentArrayMap.EMPTY or call RT.map(). The latter requires function invocation so is slightly slower, but has the benefit of localizing map construction into a single place.






[CLJ-1375] Remove Util.pcequiv() and stop pretending Java colls are equiv to Clojure colls Created: 11/Mar/14  Updated: 11/Mar/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Alex Miller Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: collections

Attachments: Text File clj-1375-v1.patch    
Patch: Code

 Description   

The Util.pcequiv() method

static public boolean pcequiv(Object k1, Object k2){
	if(k1 instanceof IPersistentCollection)
		return ((IPersistentCollection)k1).equiv(k2);
	return ((IPersistentCollection)k2).equiv(k1);
}

tries to get equiv semantics (cross-class number equality) for cases of mixed Clojure/Java collection comparison. However, this is not a sustainable direction and we would like to stop doing this.

Attached patch removes this and changes calling code to only call equiv when both collections are IPersistentCollection.






[CLJ-1372] Inconsistent hash with java collections Created: 09/Mar/14  Updated: 20/Mar/14

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

Type: Defect Priority: Major
Reporter: Jozef Wagner Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: collections
Environment:

1.6.0 master


Attachments: File clj-1372-2.diff     File clj-1372.diff    
Patch: Code and Test

 Description   

c.c/hash always use hashCode for java collections, which is incompatible when comparing with Clojure collections, which use Murmur3.

user=> (== (hash (java.util.ArrayList. [1 2 3])) (hash [1 2 3]))
false
user=> (= (java.util.ArrayList. [1 2 3]) [1 2 3])
true

One way to fix it is to add a special case in Util/hasheq for java.util.Collections, as it is now for Strings.



 Comments   
Comment by Jozef Wagner [ 09/Mar/14 8:02 AM ]

Same problem for maps, so hasheq should have a special case for java.util.Map too.

Comment by Jozef Wagner [ 09/Mar/14 9:21 AM ]

Added patch with fix for j.u. Map, Set and List.

Comment by Andy Fingerhut [ 09/Mar/14 6:02 PM ]

Add patch clj-1372-2.diff that is identical to Jozef Wagner's clj-1372.diff, except it also adds some new tests that fail without his changes, and pass with them.

Comment by Alex Miller [ 10/Mar/14 9:31 AM ]

I think the contract on equiv/hasheq is more narrowly scoped than this and only applies if both collections are IPersistentCollection. In other words, I don't think this is wanted or required.

Note that the Java .equals/.hashCode contract is maintained here - these collections will compare as .equals() and do have the same .hashCode().

Comment by Jozef Wagner [ 10/Mar/14 9:38 AM ]

Without the patch the following statement is not valid: "If two objects are equal with c.c/=, than their hash returned by c.c/hash is the same number". We can say that this is valid only iff both objects are 'clojure' objects, but this goes against clojures interop principles (interop is easy, fast, no surprises).

Comment by Jozef Wagner [ 10/Mar/14 9:54 AM ]

Manifestation of this bug

user=> (assoc (hash-map [1 2 3] :foo) (java.util.ArrayList. [1 2 3]) :bar)
{[1 2 3] :bar, [1 2 3] :foo}
user=> (get (hash-map [1 2 3] :foo) (java.util.ArrayList. [1 2 3]))
nil
Comment by Alex Miller [ 10/Mar/14 10:58 AM ]

I agree that would be a nice thing to say without qualification.

There is a real cost to adding more branches in hasheq - adding those collection checks affects every hasheq. Running a full Clojure build, I see the following set of classes with >100 occurences where this happens (note that exactly 0 of these are the Java collections - this case doesn't exist in the Clojure build itself):

clojure.lang.Var 107001502
java.lang.Class 2651389
java.lang.Character 2076322 
java.util.UUID 435235 
java.util.Date 430956
clojure.lang.Compiler$LocalBinding 116830
java.lang.Boolean 112361
java.util.regex.Pattern 325

We'd be adding 4 more instanceof checks in the path of every one of those hasheqs. This would also likely blow any JVM inlining.

Rich says "all bets should be off for hasheq/equiv of non-values" where Java collections obviously fall into the class of "non-values".

Comment by Andy Fingerhut [ 10/Mar/14 11:04 AM ]

Would a doc patch be considered? Say one that modified the doc of clojure.core/hash to include a phrase indicating that it is only promised to be consistent with clojure.core/= for immutable values? It could even perhaps mention that Floats are out, too: see CLJ-1036

Comment by Alex Miller [ 10/Mar/14 12:00 PM ]

I think it would be preferred to do any detailed docs about hash at http://clojure.org/data_structures rather than in the docstring. Although the docstring on hash probably could use an update and a pointer to the web site after the latest changes.

Comment by Jozef Wagner [ 10/Mar/14 12:14 PM ]

Neverthless it is a breaking change from 1.5, and it should be mentioned in changelog. What still bugs me is that c.c/= is supported in such cases but the c.c/hash is not. If supporting c.c/hash is expensive, isn't it better to drop support for c.c/= in such cases? It will eliminate surprises such as:

user=> (apply distinct? (hash-set [1 2 3] (java.util.Collections/unmodifiableList [1 2 3])))
false
Comment by Alex Miller [ 10/Mar/14 2:05 PM ]

I'm not sure it's a "breaking" change if something not considered to be guaranteed changes. But I take your point.

I don't think it's feasible to drop = support for Clojure and Java collections - that seems important and useful. And if it were free to do so, I would like to be able to say without qualification that if equiv=true, then hasheq is the same.

It's unclear to me that the examples listed on this ticket are actually real problems people are likely to encounter. The main users of hasheq are hash map and hash set. So to manifest, you would need to be putting a mixture of Clojure and Java collections into one of those, in particular a mixture of collections that compare as equal.

Still thinking about it.

Comment by Jozef Wagner [ 10/Mar/14 3:27 PM ]

Sorry for spamming but there may be another option, to not fallback into hashCode in hasheq, but to instead throw in cases where hasheq is requested for non-values. This will lead to a cleaner separation of hash types. Of course it will prevent putting non-values into hash-set.

Comment by Alex Miller [ 10/Mar/14 3:34 PM ]

There is no simple check for "valueness" though?

Comment by Andy Fingerhut [ 10/Mar/14 3:37 PM ]

An idea, for what it might be worth: Add one test for instance of java.util.Collection in Util.hasheq method instead of 3 separate tests for Set, List, and Map. It doesn't cover Map.Entry.

Comment by Alex Miller [ 10/Mar/14 3:38 PM ]

Map doesn't extend Collection either.

Comment by Stuart Halloway [ 11/Mar/14 10:44 AM ]

I think this needs more consideration and should not hold up 1.6.

Comment by Andy Fingerhut [ 20/Mar/14 2:01 PM ]

Both patches clj-1372.diff and clj-1372-2.diff fail to apply cleanly as of latest Clojure master on Mar 20 2014. They did apply cleanly before the Mar 19 2014 commit, I believe, and the only issue appears to be a changed line of diff context. Given the discussion about whether such a change is desired, it sounds like more thought is needed before deciding what change should be made, if any.





[CLJ-1371] divide(Object, Object) with (NaN, 0) does not return NaN Created: 07/Mar/14  Updated: 07/Mar/14

Status: Reopened
Project: Clojure
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Trivial
Reporter: Yongqian Li Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: math


 Description   

user=> (def x Double/NaN)
#'user/x
user=> (/ x 0)

ArithmeticException Divide by zero clojure.lang.Numbers.divide (Numbers.java:156)
user=> (/ Double/NaN 0)
Double/NaN



 Comments   
Comment by Alex Miller [ 07/Mar/14 7:50 AM ]

As per the Java Language Specification (http://docs.oracle.com/javase/specs/jls/se7/html/jls-4.html#jls-4.2.4),

"All numeric operations with NaN as an operand produce NaN as a result."

Comment by Yongqian Li [ 07/Mar/14 7:54 AM ]

But in the first example it produces an ArithmeticException.

Comment by Alex Miller [ 07/Mar/14 9:27 AM ]

Ah, I see the question now.

Here we are dividing a double by a long. In the first case, this is parsed as divide(Object, long) which then calls divide(Object, Object), which throws ArithmeticException if the second arg is 0 (regardless of the first arg).

In the second case it's parsed as divide(double, long) which just relies on Java to properly upcast the primitive long to a double to do the divide.

Note that making this call with 2 doubles does return NaN:

user=> (def x Double/NaN)
#'user/x
user=> (/ x 0.0)
NaN

or type hinting x to a double works as well:

user=> (def x Double/NaN)
#'user/x
user=> (/ ^double x 0.0)
NaN

I think one option to "fix" this behavior would be to add checks in divide(Object, Object) to check whether x is NaN and instead return NaN.





[CLJ-1368] Document usage for case with non-readable constants Created: 02/Mar/14  Updated: 02/Mar/14

Status: Reopened
Project: Clojure
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Herwig Hochleitner Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docs, interop


 Description   

Problem

It is pretty obscure how to get constant-time dispatch for e.g. Enums, even if user knows about case.

Proposal

The possibility to dispatch to arbitrary constants with case, by wrapper macro, should be documented.

Wording

  • Should it warn against doing that with unstable values?
  • Should it mention anything else than java Enums?

Case Techniques

Case is documented for accepting all readable forms as test-constants. However, it can also be made to use any compile-time-known constants as test-constants, by wrapping it in another macro.

Sometimes this is appropriate, e.g. when dispatching on a java Enum.
Other times, less so, e.g. when dispatching on objects whose hash changes when the vm is restarted (breaks AOT).

Implications

This technique is an application of a more general technique: Passing non-literals to a macro from another macro.
Are there other macros that have use cases like this?

References

https://groups.google.com/d/topic/clojure/3yGjDO2YnjQ/discussion



 Comments   
Comment by Herwig Hochleitner [ 02/Mar/14 11:25 AM ]

This is a duplicate of http://dev.clojure.org/jira/browse/CLJ-1367

Actually, it's an alternate solution





[CLJ-1367] Allow case statement to compare java constants Created: 02/Mar/14  Updated: 02/Mar/14

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

Type: Enhancement Priority: Minor
Reporter: Adam Clements Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: interop


 Description   

As raised on the mailing list: https://groups.google.com/forum/#!topic/clojure/3yGjDO2YnjQ

It's not possible to use java constants in a case statement. condp = could be used in this case but these are things which could be used in a java switch statement and so it's annoying to give up constant time dispatch. For example:

(case (.getActionMasked event)
MotionEvent/ACTION_POINTER_DOWN :down
MotionEvent/ACTION_UP :up
MotionEvent/ACTION_POINTER_UP :up
MotionEvent/ACTION_MOVE :move
MotionEvent/ACTION_CANCEL :cancel
MotionEvent/ACTION_OUTSIDE :outside
:none))

Doesn't work, but there is no reason this couldn't be resolved at compile time and dispatched in constant time.



 Comments   
Comment by Herwig Hochleitner [ 02/Mar/14 11:32 AM ]

Another solution for this problem: http://dev.clojure.org/jira/browse/CLJ-1368





[CLJ-1366] The empty map literal is read as a different map each time Created: 01/Mar/14  Updated: 02/Mar/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5, Release 1.6
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: OHTA Shogo Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: memory, reader

Attachments: Text File 0001-make-the-reader-return-the-same-empty-map-when-it-re.patch     Text File 0002-make-the-reader-return-the-same-empty-map-when-it-re.patch    
Patch: Code

 Description   

As reported here (https://groups.google.com/forum/?hl=en#!topic/clojure-dev/n83hlRFsfHg), the empty map literal is read as a different map each time.

user=> (identical? (read-string "{}") (read-string "{}"))
false

Making the reader return the same empty map when it reads an empty map is expected to improve some memory efficiency, and also lead to consistency with the way other collection literals are read in.

user=> (identical? (read-string "()") (read-string "()"))
true
user=> (identical? (read-string "[]") (read-string "[]"))
true
user=> (identical? (read-string "#{}") (read-string "#{}"))
true

Cause: LispReader calls RT.map() with an empty array when it reads an empty map, and RT.map() in turn makes a new map unless its argument given is null.

Approach: make RT.map() return the same empty map when the argument is an empty array as well, not only when null



 Comments   
Comment by OHTA Shogo [ 01/Mar/14 2:59 AM ]

Sorry, the patch 0001-make-the-reader-return-the-same-empty-map-when-it-re.patch didn't work.

The updated patch 0002-make-the-reader-return-the-same-empty-map-when-it-re.patch works, but I'm afraid it'd be beyond the scope of this ticket since it modifies RT.map() behavior a bit.





[CLJ-1364] Primitive VecSeq does not implement equals or hashing methods Created: 19/Feb/14  Updated: 19/Feb/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Alex Miller Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: collections


 Description   

VecSeq (as produced by (seq (vector-of :int 1 2 3))) does not implements equals, hashCode, or hasheq and does not play with any other Clojure collections or sequences appropriately in this regard.

user=> (def rs (range 3))
user=> (def vs (seq (vector-of :int 0 1 2)))
user=> rs
(0 1 2)
user=> vs
(0 1 2)
user=> (.equals rs vs)
true
user=> (.equals vs rs)    ;; expect: true
false
user=> (.equiv rs vs)
true
user=> (.equiv vs rs)
true
user=> (.hashCode rs)
29824
user=> (.hashCode vs)     ;; expect to match (.hashCode rs)
2081327893
user=> (System/identityHashCode vs)  ;; show that we're just getting Object hashCode
2081327893
user=> (.hasheq rs)
29824
user=> (.hasheq vs)       ;; expect same as (.hasheq rs) but not implemented at all
IllegalArgumentException No matching field found: hasheq for class clojure.core.VecSeq  clojure.lang.Reflector.getInstanceField (Reflector.java:271)





[CLJ-1360] clojure.string/split strips trailing delimiters Created: 18/Feb/14  Updated: 18/Feb/14

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

Type: Defect Priority: Major
Reporter: Tim McCormack Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None


 Description   

clojure.string/split and clojure.string/split-lines inherit the bizarre default behavior of java.lang.String#split(String,int) in stripping trailing consecutive delimiters:

(clojure.string/split "banana" #"an")
⇒ ["b" "" "a"]
(clojure.string/split "banana" #"na")
⇒ ["ba"]
(clojure.string/split "nanabanana" #"na")
⇒ ["" "" "ba"]

In the case of split-lines, processing a file line by line and rejoining results in truncation of trailing newlines in the file. In both cases, the behavior is surprising and cannot be inferred from the docstrings.

This behavior should either be fixed or documented.



 Comments   
Comment by Andy Fingerhut [ 18/Feb/14 10:51 AM ]

Probably documenting would be safer than changing the behavior at this point, given that some people may actually rely on the current behavior after testing, deploying, etc.

I don't currently have a suggestion for a modified doc string, but note that there are examples of this behavior and how one can use an extra "-1" limit argument at the end to get all split strings: http://clojuredocs.org/clojure_core/clojure.string/split





[CLJ-1358] doc macro does not expand special cases properly Created: 17/Feb/14  Updated: 17/Feb/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.6
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Chad Taylor Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: repl

Attachments: Text File CLJ-1358.patch    
Patch: Code and Test

 Description   

The doc macro supports three special cases, mapping & to fn, catch to try, and finally to try. However, the macro does not currently expand these cases - it executes them like a function instead. This is evident if you use the following at a REPL:

user> (macroexpand '(doc try))   ;; ok
((var clojure.repl/print-doc) ((var clojure.repl/special-doc) (quote try)))

user> (macroexpand '(doc catch)) ;; broken
;; -- unexpectedly prints try doc -- ;;
nil

user> (= (with-out-str (doc catch)) (with-out-str (doc try))) ;; broken, expect true
;; -- unexpectedly prints try doc -- ;;
false

Workaround: Call doc with the symbol to which the special case is mapped, fn or try.

Cause: Incorrect quoting when handling special cases in doc macro

Solution: Update special case quoting approach to match the other cases.

Patch: CLJ-1358.patch



 Comments   
Comment by Chad Taylor [ 17/Feb/14 10:41 PM ]

Adding a patch with code and test.





[CLJ-1347] finalize won't work in reified objects - document Created: 10/Feb/14  Updated: 01/Mar/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5, Release 1.6
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Jozef Wagner Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

java 7



 Description   

Finalize is called for reified objects even when they are still reachable. It gets called second time at proper time.

user=> (def x (reify Object (finalize [o] (println "OH MY!"))))
#'user/x
user=> (System/gc)
nil
OH MY!
user=> x
#<user$reify__1496 user$reify__1496@53fb35af>
user=> (System/gc)
nil
user=> (def x nil)
#'user/x
user=> (System/gc)
nilOH MY!

Deftype seems to work fine

user=> (deftype T [] Object (finalize [o] (println "great success")))
user.T
user=> (def y (->T))
#'user/y
user=> (System/gc)
nil
user=> (def y nil)
#'user/y
user=> (System/gc)
great success


 Comments   
Comment by Alex Miller [ 10/Feb/14 8:38 AM ]

Just a note: the calls to System/gc don't necessarily cause finalizers to run on the first try - sometimes it took more than one for that to succeed for me. You'd think System/runFinalizers would do it but I had no luck at all with that.

Comment by Gary Fredericks [ 13/Feb/14 10:01 PM ]

reify actually creates two objects – the first is created by reify*, and then reify immediately calls with-meta on it, creating a copy.

The docstring sort of describes this behavior: "reify always implements clojure.lang.IObj and transfers meta data of the form to the created object."

Comment by Jozef Wagner [ 14/Feb/14 5:01 AM ]

Oh, so finalizer is a no-no in reify. Should be mentioned in docs IMO.

Comment by Gary Fredericks [ 14/Feb/14 6:28 AM ]

Just for fun you could do something tricksy like:

^::second-object
(reify Object
  (finalize [self]
    (when (::second-object (meta self))
      ...)))

(have not actually run this)

Comment by Gary Fredericks [ 01/Mar/14 1:36 PM ]

It looks like the class generated by reify always has a constructor that takes a metadata argument, so it doesn't seem out of the question to eliminate the extra object altogether.

I'll try to keep digging on this.





[CLJ-1346] clojure.core.VecSeq does not implement method equals Created: 09/Feb/14  Updated: 09/Feb/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5, Release 1.6
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Andy Fingerhut Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

.equals is asymmetric for seq's of primitive vectors and PersistentVectors, because clojure.core.VecSeq does not implement Java's equals method. It implements equiv, so clojure.core/= is fine:

user=> (def v1 [1 2 3])
#'user/v1
user=> (def v2 (vector-of :long 1 2 3))
#'user/v2
user=> (= v1 v2)
true
user=> (.equals v1 v2)
true
user=> (= (seq v1) (seq v2))
true
user=> (.equals (seq v1) (seq v2))
true
user=> (= v2 v1)
true
user=> (.equals v2 v1)
true
user=> (= (seq v2) (seq v1))
true
;; This is the one that is not like the others, and a symptom of the problem
user=> (.equals (seq v2) (seq v1))
false





[CLJ-1342] Byte comparison boxes both bytes and converts to longs to compare (which is slow) Created: 06/Feb/14  Updated: 06/Feb/14

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

Type: Enhancement Priority: Minor
Reporter: Alex Miller Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: compiler

Attachments: File bytebox.clj    

 Description   

This came up in a much more complicated example but consider a case like this:

(defn simple []
  (let [b (byte-array [(byte 0)])
        m (byte 0)]
    (= m (aget b 0))))

In the compiled bytecode, both m and (aget b 0) are known to be bytes, but both are boxed using Byte.valueOf(), then cast using RT.uncheckedLongCast() and finally compared as longs:

26: iload_2
  27: invokestatic  #69  // Method java/lang/Byte.valueOf:(B)Ljava/lang/Byte;
  30: checkcast     #81  // class java/lang/Number
  33: invokestatic  #85  // Method clojure/lang/RT.uncheckedLongCast:(Ljava/lang/Object;)J

In a tight loop manipulating and matching against byte arrays, this boxing is significant for performance.

Attached is a test that demonstrates the performance difference between the byte[] and long[] performance to get an idea of the difference.



 Comments   
Comment by Nicola Mometto [ 06/Feb/14 9:10 PM ]

The description states that Util.equiv() has a byte/byte comparison variant but it doesn't look like it actually exists.

Comment by Nicola Mometto [ 06/Feb/14 9:17 PM ]

By the way, tools.emitter.jvm uses i2l to cast the byte to a long instead of boxing && unboxing to a long

Comment by Alex Miller [ 06/Feb/14 9:39 PM ]

Thanks Nicola - I must have confused it with the boolean/boolean version.





[CLJ-1341] keyword function returns nil on bad input Created: 05/Feb/14  Updated: 14/Feb/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Alex Miller Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: errormsgs

Attachments: Text File keyword-1341-2014-02-12.2.patch     Text File keyword-1341-2014-02-12.patch    
Patch: Code and Test

 Description   

The keyword function should throw an exception on bad input rather than return nil.

user=> (keyword 5)
nil
user=> (keyword [])
nil

Cause: The keyword function is defined as a cond with cases for keywords, symbols, and strings. There is no :else so all other cases return nil.

Proposal: Add an :else branch and throw an exception in keyword.

Patch:



 Comments   
Comment by Eric Normand [ 12/Feb/14 7:17 PM ]

The keyword function should throw an IllegalArgumentException on wrong argument type rather than return nil. For consistency, the two-argument case should throw an IllegalArgumentException if not both arguments are strings.

The find-keyword function should behave similarly to maintain the same signature.

Current behavior:

user=> (keyword 5)
nil
user=> (keyword [])
nil
user=> (keyword 1 1)
java.lang.ClassCastException: java.lang.Long cannot be cast to java.lang.String

Cause: The keyword function is defined as a cond with cases for keywords, symbols, and strings. There is no :else so all other cases return nil. The same goes for find-keyword, which should behave in the same way. The two-argument case does throw an exception, but the message is not very helpful.

Proposal: I have added an :else branch to the cond that throws an IllegalArgumentException with a message that indicates the acceptable types and prints the actual argument. I made the same change to find-keyword. There are also simple tests.

Patch: keyword-1341-2014-02-12.patch

Note: This change does not check for all bad input, just the type. For instance, it is still possible to pass in a string with "illegal" keyword characters.

Comment by Alex Miller [ 12/Feb/14 9:20 PM ]

Hey Eric, thanks for the patch! The 1 arg change looks good.

On the 2 arg change I have a concern - I'm worried that we are adding new checks into a pretty hot code path (keyword creation). The 2 arg path is not a silent failure as you'll get a ClassCastException so I do not think adding these checks here is worth it. In the 1-arg case you've already fallen through the else, so there's no additional cost.

Comment by Eric Normand [ 12/Feb/14 9:35 PM ]

Understood. I'll remove the two-argument case.

Comment by Eric Normand [ 12/Feb/14 9:51 PM ]

The keyword function should throw an IllegalArgumentException on wrong argument type rather than return nil. The two-argument case already throws an exception.

The find-keyword function should behave similarly to maintain the same signature.

Current behavior:

user=> (keyword 5)
nil
user=> (keyword [])
nil
user=> (keyword 1 1)
java.lang.ClassCastException: java.lang.Long cannot be cast to java.lang.String

Cause: The keyword function is defined as a cond with cases for keywords, symbols, and strings. There is no :else so all other cases return nil. The same goes for find-keyword, which should behave in the same way.

Proposal: I have added an :else branch to the cond that throws an IllegalArgumentException with a message that indicates the acceptable types and prints the actual argument. I made the same change to find-keyword. There are also simple tests.

Alternatives: Adding checks for the two-argument case was considered but it was feared that adding the extra overhead was not worth it since it already threw an exception. No significant overhead is added in the single-argument case since it will only affect erroneous input.

Patch: keyword-1341-2014-02-12.2.patch

Note: This change does not check for all bad input, just the type. For instance, it is still possible to pass in a string with "illegal" keyword characters.





[CLJ-1340] Emit unboxed cohercions from int/long to float/double Created: 05/Feb/14  Updated: 05/Feb/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Christophe Grand Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: enhancement, performance

Attachments: File primitive-cohercion.diff    
Patch: Code

 Description   

Currently when an int or long is used where a float or double is expected, boxed conversion happens instead of emitting [IL]2[FD] instructions.






[CLJ-1337] defprotocol's docstring is out of date Created: 04/Feb/14  Updated: 04/Feb/14

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

Type: Defect Priority: Trivial
Reporter: OHTA Shogo Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring

Attachments: Text File 0001-Update-defprotocol-s-docstring-to-remove-an-out-of-d.patch    
Patch: Code

 Description   

The docstring of defprotocol has an out-of-date description as follows:

defprotocol is dynamic, has no special compile-time effect, and defines no new types or classes.

Indeed, it used to have no compile-time effect. Today, however, it does generate a corresponding interface via gen-interface at compile time.

The patch just removes this description.






[CLJ-1333] Documentation for "=" is misleading Created: 30/Jan/14  Updated: 14/Feb/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: George Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docs
Environment:

linux 2.6.32-431.el6.x86_64



 Description   

Document for clojure.core/= says it compares numbers in a type-independent manner. In reality the comparission is made in a type dependent manner. If the above statement was true then (= 1 1.0) would eval to true not false;

clojure.core/=
([x] [x y] [x y & more])
Equality. Returns true if x equals y, false if not. Same as
Java x.equals except it also works for nil, and compares
numbers and collections in a type-independent manner. Clojure's immutable data
structures define equals() (and thus =) as a value, not an identity,
comparison.



 Comments   
Comment by Kevin Downey [ 02/Feb/14 4:58 PM ]

I think this is a little more complex than described.

= does compare things in a jvm type independent manner, but it does use what people have taken to calling "equality classes"

(= [1 2] '(1 2))

(= {:a 1} (doto (java.util.HashMap.) (.put :a 1)))

etc.

now for numbers, it seems logical to me, to have floating point and precise numbers in distinct equality classes

in which case, 1.0 and 1 are in distinct equality classes, so not equal.

Comment by Gary Fredericks [ 13/Feb/14 10:16 PM ]

The docstring is definitely misleading for people unfamiliar with this sort of thing though. Numbers are probably the first thing that the words "type independent manner" bring to mind. A brief pointer to the == function might be useful.

Comment by George [ 14/Feb/14 5:47 AM ]

I find == function to be confusing
For example
(== 1 1.0) => true
(== 1 1.0M) => false ; what is wrong with this comparison?

and doc says:
Returns non-nil if nums all have the equivalent value (type-independent)

Comment by Alex Miller [ 14/Feb/14 7:56 AM ]

@George - that last example (== 1 1.0M) is actually a bug that is fixed in 1.6 where it will return true.





[CLJ-1332] Exceptions are not cached in lazy seqs Created: 29/Jan/14  Updated: 13/Feb/14

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

Type: Enhancement Priority: Minor
Reporter: Yongqian Li Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None


 Description   

It is confusing that exceptions will only be thrown once when it is possible to iterate over a seq many times.

user=> (def a (for [i (reverse (range 2))] (/ 1 i)))
#'user/a
user=> (println a)

ArithmeticException Divide by zero clojure.lang.Numbers.divide
(Numbers.java:156)
(user=> (println a)
(1)
nil
user=> (println a)
(1)
nil



 Comments   
Comment by Gary Fredericks [ 13/Feb/14 10:29 PM ]

The cause of this is the lazy-seq macro which uses :once metadata to signal to the compiler that the thunk it creates will only be called once.

When the evaluation of a lazy seq throws an exception, trying to walk the seq again causes the function to be called a second time. Since its closed over values have likely been cleared by that point, you get different behavior.

Glancing at LazySeq.java made me pretty convinced you can't cache exceptions without adding an extra check somewhere in the standard codepath for lazy seq traversal.

Comment by Yongqian Li [ 13/Feb/14 11:38 PM ]

Btw, I ran into this issue while trying to evaluate a lazy-seq in a future in order to do some processing concurrently in the background. Any suggestions for workarounds?





[CLJ-1327] Clojure Primitives extend Serializable without serialVersionUID Created: 20/Jan/14  Updated: 20/Jan/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

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

Linux



 Description   

Clojure keywords for instance are serializable but do not define a serialVersionUID.






[CLJ-1326] Inconsistent reflection warnings when target is a literal Created: 19/Jan/14  Updated: 05/Feb/14

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

Type: Enhancement Priority: Trivial
Reporter: Nicola Mometto Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: errormsgs


 Description   
user=> (set! *warn-on-reflection* true)
true
user=> (.get {} 0)
Reflection warning, NO_SOURCE_PATH:2:1 - call to get can't be resolved.
nil
user=> (.get {1 1} 0)
nil
user=> (.get ^:foo {1 1} 0)
Reflection warning, NO_SOURCE_PATH:4:1 - call to get can't be resolved.
nil
user=> (.get {1 (inc 0)} 0)
Reflection warning, NO_SOURCE_PATH:5:1 - call to get can't be resolved.
nil

Similar issues apply to other literals (vector literals, list literals)






[CLJ-1324] Allow leading slashes in unqualified symbol names Created: 15/Jan/14  Updated: 10/Feb/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Mike Anderson Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: reader
Environment:

All


Attachments: Text File clj-1324-1.patch    
Patch: Code and Test

 Description   

The proposal is to allow the reader to accept symbol names with leading slashes.

Problem: Leading slashes are frequently useful, e.g. in mathematical operators like "/" or "/="

Currently, only "/" is allowed as a special case, and is used for the division operator in clojure.core

This could be extended to allow all symbols to have names starting with a leading slash.

There should be no ambiguity with namespace-qualified symbols:
1) In the case of a leading slash, a symbol should be interpreted as an unqualified symbol e.g. "/="
2) In the case of a slash anywhere except in leading position, it should considered as namespace qualified, e.g. "clojure.core/+"
3) In the case of multiple non-leading slashes, the first slash is the namespace separator, e.g. "clojure.core.matrix.operators//="

Optionally, it also would be possible to allow multiple slashes after the leading slash in a name. This would allow symbols such as "/src/main/clojure" to become valid.



 Comments   
Comment by Paavo Parkkinen [ 10/Feb/14 7:32 AM ]

Attached patch to allow leading slashes in symbol names.

The patch changes the regexp pattern used to match symbols to accept characters after a slash in symbol names.

Tests pass, and the patch also adds a couple of new special cases to the symbol tests.





[CLJ-1323] AsmReflector throws exceptions on JDK8 Created: 13/Jan/14  Updated: 23/Mar/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.6
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Andy Fingerhut Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: File clj-1323-disable.diff    

 Description   

After the commit of the updated ASM library for CLJ-713, Clojure builds and passes all tests except for one, compare-reflect-and-asm in reflect.clj.

This can be narrowed down somewhat to a difference in behavior of the following 2 forms evaluated with the latest Clojure and JDK8:

;; The following two lines work with the latest (Jan 11 2014) Clojure 1.6.0-master-SNAPSHOT
;; if run on JDK 6 or JDK 7, but throw an exception with JDK 8.

(import '[clojure.asm ClassReader ClassVisitor Type Opcodes])
(def r (ClassReader. "java.lang.Object"))

I am not certain, but from a bit of Google searching it appears that this may be a limitation of the ASM library version 4 – it throws exceptions when attempting to read class files produced by JDK 8, because of a newer classfile version number. Links that seem to support this conclusion:

http://mail-archive.ow2.org/asm/2013-02/msg00000.html

http://forge.ow2.org/tracker/index.php?func=detail&aid=316375&group_id=23&atid=350023

A couple of alternatives are:

(1) update ASM again to one that supports JDK 8 class files

(2) disable the compare-reflect-and-asm test. Clojure itself does not use the AsmReflector for anything except this unit test. The Java reflector is the default one.



 Comments   
Comment by Alex Miller [ 13/Jan/14 8:16 AM ]

1) There is no released ASM that supports JDK 8 yet. ASM 5 will but it will not be final till JDK 8 is in the final stages of release.

2) Probably more likely.

Comment by Nicola Mometto [ 19/Mar/14 9:01 AM ]

As of now, both JDK8 and ASM5 are out.
I just tried compiling clojure on JDK8 with ASM5 and all compiles fine

Comment by Alex Miller [ 19/Mar/14 9:07 AM ]

How are you running this test?

Comment by Nicola Mometto [ 19/Mar/14 9:11 AM ]

I downloades ASM5, replaced the bundled ASM that comes with clojure with that one after changing che package name to "clojure.asm" and run `mvn install`, all the tests pass.

Comment by Alex Miller [ 19/Mar/14 9:24 AM ]

I was actually talking about the JDK 8 change - was curious about exactly what was being changed?

Comment by Alex Miller [ 19/Mar/14 10:04 AM ]

In particular, I'm assuming that you're not altering the build.xml to change the compilation -source or -target and running with JAVA_HOME / path set to JDK 8.

We don't have any plans to actually build Clojure with JDK 8 any time soon, so I'm not overly concerned about that. But it does appear that the embedded ASM 4 cannot read newer class files from JDK 8. Afaik, the only place that happens is in clojure.reflect.java in the AsmReflector, which is not the default reflector. The JavaReflector will properly reflect the Java 8 classes.

ASM 5 has only been out a couple days and already has at least one serious bug reported - I'd like that to see more use before we switch to it, so maybe this is a good target for the release after Clojure 1.6.

Comment by Alex Miller [ 23/Mar/14 12:17 PM ]

Patch to temporarily disable the failing test until we have an ASM that supports JDK 8.





[CLJ-1321] Documentation improvement for clojure.walk, to note use of recursion that can easily blow the JVM stack Created: 09/Jan/14  Updated: 09/Jan/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Lee Spector Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: documentation
Environment:

JVM



 Description   

prewalk and postwalk are use recursion in ways that will blow the stack for sufficiently deep nested structures.

I suggest that this be noted in various clojure.walk documentation strings since this kind of recursion/limit seems to be rare in Clojure, and hence will be unexpected.

(It'd be even better to remove the recursion/limit via something like zippers and iteration, but this issue is just a suggestion for improvement of the documentation.)






[CLJ-1319] array-map fails lazily if passed an odd number of arguments Created: 08/Jan/14  Updated: 30/Jan/14

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

Type: Enhancement Priority: Minor
Reporter: Stuart Sierra Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: errormsgs

Attachments: Text File 0001-CLJ-1319-Throw-on-odd-arguments-to-PersistentArrayMa.patch    
Patch: Code and Test

 Description   

If called with an odd number of arguments, array-map does not throw an exception until the map is realized, when it throws the confusing ArrayIndexOutOfBoundsException.

Example, in 1.5.1 and 1.6.0-alpha3:

user=> (def m (array-map :a 1 :b))
#'user/m

user=> (prn m)
ArrayIndexOutOfBoundsException 3
  clojure.lang.PersistentArrayMap$Seq.first
  (PersistentArrayMap.java:313)


 Comments   
Comment by Alex Miller [ 08/Jan/14 11:01 AM ]

PersistentArrayMap.createAsIfByAssoc could check length is even to catch this

Comment by Jason Felice [ 27/Jan/14 1:01 PM ]

A better error message would be nice... this is the best I could think of.





[CLJ-1317] clojure.zip/seq-zip returns spurious nils during traversal Created: 31/Dec/13  Updated: 05/Feb/14

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

Type: Defect Priority: Minor
Reporter: Michał Marczyk Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: zip

Attachments: Text File 0001-CLJ-1317-fix-seq-zip-to-avoid-spurious-nils.patch    
Patch: Code

 Description   

Problem reported by Lee Spector on the mailing list:

https://groups.google.com/d/msg/clojure/8TL7IGmE7N0/u1xfgTOLDRgJ

Here's a quote from Lee's post describing the problem:

Here's an illustration, stepping through '(() 0) with next and printing the node at each step: 

(loop [z (zip/seq-zip '(() 0))] 
  (if (zip/end? z) 
    :done 
    (do (println (zip/node z)) 
      (recur (zip/next z))))) 

That produces: 

(() 0) 
() 
nil 
0 
:done 

I don't expect the nil to be there. 

The underlying cause is that seq-zip passes identity as the children argument to zipper. Applied to (), this returns (), which is truthy, leading zipper to descend into a non-existent subtree.

One natural solution would be to use seq in place of identity:

(defn seq-zip [root]
  (zipper seq?
          seq  ;; changed
          (fn [node children] (with-meta children (meta node)))
          root))

With this change, no nil is produced in the example above. Patch with this change forthcoming.



 Comments   
Comment by Michał Marczyk [ 31/Dec/13 5:52 PM ]

Note that the docstring of clojure.zip/zipper asks that the children argument return a seq of children. The rest of clojure.zip, however, expects nil to be returned when there are no children, as evidenced by this problem.

One could argue that this behaviour of the rest of clojure.zip should be fixed, but I think it makes sense and is convenient. Perhaps the docstring should be adjusted, though.





[CLJ-1314] Correct placement of doc string for function bubble-max-key Created: 23/Dec/13  Updated: 05/Feb/14

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

Type: Enhancement Priority: Trivial
Reporter: Andy Fingerhut Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring

Attachments: File clj-1314-v2.diff    
Patch: Code

 Description   

It is private, defined with defn-, so perhaps the doc string is superfluous, but someone wrote one, and it is in the wrong place relative to the args:

(defn- bubble-max-key [k coll]
  "Move a maximal element of coll according to fn k (which returns a number) 
   to the front of coll."
  (let [max (apply max-key k coll)]
    (cons max (remove #(identical? max %) coll))))

Found using a pre-release version of the Eastwood Clojure lint tool.



 Comments   
Comment by Andy Fingerhut [ 23/Dec/13 4:11 PM ]

Patch clj-1314-v1.diff simply corrects the location of the doc string for bubble-max-key.

Comment by Andy Fingerhut [ 02/Jan/14 11:05 AM ]

Patch clj-1314-v2.diff is same as the previous clj-1314-v1.diff, except it leaves out some unintended changes.





[CLJ-1311] gen-interface uses DynamicClassLoader when not compiling, gen-class doesn't Created: 20/Dec/13  Updated: 05/Feb/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Joel Kaasinen Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring, gen-class, interop


 Description   

The documentation for both gen-class and gen-interface says: "When not compiling, does nothing."

However, gen-interface does the right thing and uses DynamicClassLoader.defineClass when not compiling. This means e.g. that gen-interface works from the repl.

I don't see a reason why gen-class couldn't do the same. Obviously, the docstrings would need to be updated too.






[CLJ-1309] Bindings after :as in list destructuring should throw error Created: 19/Dec/13  Updated: 05/Feb/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.4, Release 1.5
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: ben wolfson Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: Compiler, errormsgs


 Description   

If you try to define a vector binding with anything at all after an :as parameter, you do not get a compiler error, and the binding is silently swallowed:

user> ((fn [[:as y z]] y) [1 2])
[1 2]

If you try to actually use the binding, there will be a compiler error (the compiler will complain that there's no binding for the symbol), but the actual error has already happened, and should be reported earlier.






[CLJ-1308] extend-type doesn't type-hint correctly as promised by the doc when the class is determined at run-time Created: 15/Dec/13  Updated: 05/Feb/14

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

Type: Defect Priority: Minor
Reporter: Nicola Mometto Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: typehints


 Description   

extend-type works with non-constant expressions as its type:

(extend-type (class 1) proto (protof [this]))

However, in this case this will get tagged with `(class 1)`, which is clearly wrong; the doc explicitely states that the args will be proberly type-hinted: "[..] Propagates the class as a type hint on the first argument of all fns."

I don't know if extend-type is not supposed to work with non-constant Classes, in which case it should be stated in the doc or if the current behaviour is wrong.






[CLJ-1305] Add optional not-found argument when invoking vectors or sets as functions Created: 12/Dec/13  Updated: 05/Feb/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Dave Tenny Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None


 Description   

Maps, keywords, and symbols when used as operators allow optional second arguments for 'default-not-found' values is if to 'get'.

({:a 1} :b 'b) => b

However sets don't support this behavior (though they do with 'get') and vectors don't allow the optional default-not-found in their pseudo 'nth' semantics.

user=> (#{:a  :b} :b 'notfound)
ArityException Wrong number of args (2) passed to: PersistentHashSet  clojure.lang.AFn.throwArity (AFn.java:437)





[CLJ-1300] take-while with n<1 behaves like (repeat 1) Created: 22/Nov/13  Updated: 22/Nov/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.1, Release 1.2, Release 1.3, Release 1.4, Release 1.5, Release 1.6
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Yaron Peleg Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   
user=> (take-nth -1 [1 3 4])
; hangs
user=> (take 5 (take-nth -1 [5 9 14]))
(1 1 1 1 1)

I understand this behavior may be intentionally undefined,
but raising the issue on IRC didn't yield an answer on whether
this is a bug or grey area.






[CLJ-1298] Add more type predicate fns to core Created: 21/Nov/13  Updated: 05/Feb/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5, Release 1.6
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Alex Fowler Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None


 Description   

Add more built-in type predicates:

1) Definitely missing: (atom? x), (ref? x), (deref? x), (named? x), (map-entry? x), (lazy-seq? x).
2) Very good to have: (throwable? x), (exception? x), (pattern? x).

The first group is especially important for writing cleaner code with core Clojure.



 Comments   
Comment by Alex Miller [ 21/Nov/13 8:42 AM ]

In general many of the existing predicates map to interfaces. I'm guessing these would map to checks on the following types:

atom? = Atom (final class)
ref? = IRef (interface)
deref? = IDeref (interface)
named? = Named (interface, despite no I prefix)
map-entry? = IMapEntry (interface)
lazy-seq? = LazySeq (final class)

throwable? = Throwable
exception? = Exception, but this seems less useful as it feels like the right answer when you likely actually want throwable?
pattern? = java.util.regex.Pattern

Comment by Alex Fowler [ 21/Nov/13 9:02 AM ]

Yes, they do, and sometimes the code has many checks like (instance? clojure.lang.Atom x). Ok, you can write a little function (atom? x) but it has either to be written in all relevant namespaces or required/referred there from some extra namespace. All this is just a burden. For example, we have predicates like (var? x) or (future? x) which too map to Java classes, but having them abbreviated often makes possible to write a cleaner code.

I feel the first group to be especially significant for it being about core Clojure concepts like atom and ref. Having to fall to manual Java classes check to work with them feels inorganic. Of course we can, but why then do we have (var? x), (fn? x) and other? Imagine, for example:

(cond
(var? x) (...)
(fn? x) (...)
(instance? clojure.lang.Atom x) (...)
(or (instance? clojure.lang.Named x) (instance? clojure.lang.LazySeq x)) (...))

vs

(cond
(var? x) (...)
(fn? x) (...)
(atom? x) (...)
(or (named? x) (lazy-seq? x)) (...))

The second group is too, essential since these concepts are fundamental for the platform (but you're right with the (exception? x) one).

Comment by Alex Fowler [ 22/Nov/13 6:35 AM ]

Also, obviously I missed the (boolean? x) predicate in the original post. Did not even guess it is absent too until I occasionally got into it today. Currently the most clean way we have is to do (or (true? x) (false? x)). Needles to say, it looks weird next to the present (integer? x) or (float? x).





[CLJ-1296] locking expressions cause vars to be dereferenced, even if not executed, unless wrapped in let Created: 17/Nov/13  Updated: 18/Apr/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Andy Fingerhut Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: performance


 Description   

Description of one example with poor performance discovered by Michał Marczyk in the discussion thread linked below.

The difference between the compiled versions of:

(defn foo [x]
  (if (> x 0)
    (inc x)
    (locking o
      (dec x))))

and

(defn bar [x]
  (if (> x 0)
    (inc x)
    (let [res (locking o
                (dec x))]
      res)))

is quite significant. foo gets compiled to a single class, with invocations handled by a single invoke method; bar gets compiled to a class for bar + an extra class for an inner function which handles the (locking o (dec x)) part – probably very similar to the output for the version with the hand-coded locking-part (although I haven't really looked at that yet). The inner function is a closure, so calling it involves an allocation of a closure object; its ctor receives the closed-over locals as arguments and stores them in two fields (lockee and x). Then they get loaded from the fields in the body of the closure's invoke method etc.

Note: The summary line may be too narrow a description of the root cause, and simply the first example of a case where this issue was noticed and examined. Please make the summary and this description more accurate if you diagnose this issue.

See discussion thread on Clojure group here: https://groups.google.com/forum/#!topic/clojure/x86VygZYf4Y



 Comments   
Comment by Kevin Downey [ 18/Apr/14 2:17 AM ]

maybe it is already clear to others, but this was not immediately clear to me:

the reason

(defn bar [x]
  (if (> x 0)
    (inc x)
    (let [res (locking o
                (dec x))]
      res)))

generates a second class is locking is a macro that contains a try/finally form in it's expansion.

binding the result of a try/finally form to a result (as in the let) would require some real tricky code gen without adding the extra function, so of course the clojure compile adds the extra function.





[CLJ-1292] print docstring should specify nil return value Created: 01/Nov/13  Updated: 05/Feb/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.6
Fix Version/s: None

Type: Enhancement Priority: Trivial
Reporter: Phill Wolf Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring, print


 Description   

The docstring for print does not mention its return value. The docstring should clarify whether print dependably returns nil or shouldn't be depended on to (lest, for example, something leak out as the inadvertent return value of print's caller).






[CLJ-1291] struct-map docstring incomplete, inconsistent Created: 01/Nov/13  Updated: 05/Feb/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.6
Fix Version/s: None

Type: Enhancement Priority: Trivial
Reporter: Phill Wolf Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring


 Description   

The docstring for struct-map refers to "structure-basis" and "keyvals" while the parameters are "s" and "inits". The docstring says "keyvals can also contain keys not in the basis" but does not say what happens in that case.






[CLJ-1290] clojure.xml parse docstring omits InputSource Created: 01/Nov/13  Updated: 05/Feb/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.6
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Phill Wolf Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring, xml


 Description   

The clojure.xml parse docstring mentions that parameter s "can be a File, InputStream or String naming a URI." But those choices do not cover a common case, parsing the value of a String. Actually, parse also allows InputSource, which solves the problem. The docstring should mention InputSource (or clarify its omission, if not inadvertent).

user> (use '[clojure.xml :as xml])
nil
user> (import '[java.io StringReader])
java.io.StringReader
user> (import '[org.xml.sax InputSource])
org.xml.sax.InputSource
user> (xml/parse (InputSource. (StringReader. "<egg>green</egg>")))
{:tag :egg, :attrs nil, :content ["green"]}





[CLJ-1286] Fix reader spec and regex to match code for keywords starting with digits Created: 31/Oct/13  Updated: 05/Feb/14

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

Type: Defect Priority: Minor
Reporter: Alex Miller Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: reader


 Description   

The reader page at http://clojure.org/reader states that symbols (and keywords) cannot start with a number and the regex used in LispReader (and EdnReader) also has this intention. CLJ-1252 addressed this by fixing the broken reader regex to match the spec. However, that broke some existing code so we rolled back the change. There is still a disconnect here and this ticket serves to decide what to do instead.

I presume that we are effectively deciding that keywords like :5 are ok to read. If so, we should alter the regex to more accurately capture that intent - right now it allows these purely by accident due to backtracking. A secondary question is whether the Clojure and EDN reader spec should also explicitly allow these as valid. My preference would be to have the reader and the spec match, so I would lobby to loosen the reader spec.



 Comments   
Comment by Nicola Mometto [ 12/Nov/13 4:50 PM ]

what about keywords like :1/1 or :1/a? Clojure currently accepts the latter but not the former.





[CLJ-1284] Clojure functions and reified objects should expose a public static field to identify their proper Clojure name Created: 24/Oct/13  Updated: 25/Oct/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Howard Lewis Ship Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: errormsgs


 Description   

There are several examples of frameworks that attempt to de-mangle a Java class name into a Clojure symbol (including namespace); this is useful for writing out an improved, Clojure-specific stack trace when reporting exceptions.

Existing libraries are based on regular expression matching and guesswork, and can occasionally give incorrect results, such as when a namespace or function name actually contains an underscore.

It would be helpful for authors of such frameworks if Clojure would expose a static final field on such classes with the proper name that should appear in the stack trace; libraries would then be able to use reflection to access the proper name of the field, without the current guesswork.

I would suggest CLOJURE_SOURCE_NAME as a reasonable name for such a field.

Other Clojure class constructs beyond functions, such as reified types and protocol implementations, would also benefit, though it is less obvious what exact string value would properly and unambiguously identify what purpose the class plays.



 Comments   
Comment by Alex Miller [ 24/Oct/13 8:31 PM ]

FYI, there is a patch on the way in for 1.6 that contains a new demunge function in Compiler. However, the munged name is not always reversible so having the original around is a good idea.

Comment by Andy Fingerhut [ 25/Oct/13 11:10 AM ]

The patch Alex is referring to is attached to CLJ-1083.

Comment by Andy Fingerhut [ 25/Oct/13 11:13 AM ]

Howard, there seems to be some overlap in the intent between this ticket and CLJ-1278. I guess either of them could be done without the other, but wanted to check.





[CLJ-1283] Method clojure.lang.RT.lastModified(URL, String) fails if 'jar' protocol is not handled by JarURLConnector. Created: 24/Oct/13  Updated: 24/Oct/13

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

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

Attachments: Text File fix.patch    
Patch: Code

 Description   

Method clojure.lang.RT.lastModified(URL, String) throws a ClassCastException if it is called in environment, where 'jar' protocol is not handled by standard JarURLConnector. The provided patch just adds a type check for that case. I ran into this problem when I tried to add a Clojure REPL to existing Java application.



 Comments   
Comment by Alex Miller [ 24/Oct/13 8:26 AM ]

Hi TV, thanks for the report! Just a note, that we can't accept your patch unless you have signed a Contributor's Agreement as per http://clojure.org/contributing.

Just out of curiosity, is this an internal environment where you've customized the jar protocol or some kind of app server someone else would be likely to encounter?

Comment by TV [ 24/Oct/13 8:44 AM ]

It is an internal environment and so I set the priority to minor.





[CLJ-1280] Create reusable exception that can carry file/line/col info Created: 18/Oct/13  Updated: 18/Apr/14

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

Type: Enhancement Priority: Major
Reporter: Alex Miller Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None


 Description   

This concept already exists in multiple places in Clojure - Compiler$CompilerException and the Exception classes buried in EdnReader and LispReader. It would also be useful in other places where IllegalArgument or other other exceptions are thrown.

For example, this protocol exception throws an IllegalArgumentException and could transmit the file, line, and column info at the location of the error but it seems weird to use any of the existing exceptions for this purpose.

(defprotocol Bar (m [this]) (m [this arg]))


 Comments   
Comment by Kevin Downey [ 18/Apr/14 12:09 AM ]

seems like ExceptionInfo can do this





[CLJ-1279] Fix confusing macroexpand1 ArityException handling Created: 16/Oct/13  Updated: 05/Feb/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.6
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Alex Coventry Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: Compiler, errormsgs, macro

Attachments: Text File 0001-Edit-macro-ArityException-in-AFn.patch     Text File 0001-Fix-macroexpand1-s-handling-of-ArityException.patch    
Patch: Code and Test

 Description   

macros can give very confusing error messages when they execute a form which generates an ArityException. clojure.lang.Compiler.macroexpand1 assumes that any ArityException comes from the call to the macro itself, which need not be the case. For instance:

user> (do (defmacro f [] (assoc)) (f))
ArityException Wrong number of args (-2) passed to: core$assoc clojure.lang.Compiler.macroexpand1 (Compiler.java:6488)
user> (use 'clojure.repl) (pst)
nil
ArityException Wrong number of args (-2) passed to: core$assoc
clojure.lang.Compiler.macroexpand1 (Compiler.java:6488)
clojure.lang.Compiler.macroexpand (Compiler.java:6544)
clojure.lang.Compiler.eval (Compiler.java:6618)
clojure.lang.Compiler.eval (Compiler.java:6624)
clojure.lang.Compiler.eval (Compiler.java:6597)
clojure.core/eval (core.clj:2864)
clojure.main/repl/read-eval-print-6596/fn-6599 (main.clj:260)
clojure.main/repl/read-eval-print--6596 (main.clj:260)
clojure.main/repl/fn--6605 (main.clj:278)
clojure.main/repl (main.clj:278)
clojure.tools.nrepl.middleware.interruptible-eval/evaluate/fn--1251 (interruptible_eval.clj:56)
clojure.core/apply (core.clj:617)
nil

Easy enough to see the source of the problem in this case, but because both the number of arguments actually passed is off by two, and the stacktrace element for the call to assoc has been dropped, this shortcut by macroexpand1 can get super confusing.

The attached patch corrects this behavior. E.g.

user=> (do (defmacro f [] (assoc)) (f))
ArityException Wrong number of args (0) passed to: core$assoc clojure.lang.AFn.throwArity (AFn.java:437)
user=> (use 'clojure.repl) (pst)
nil
ArityException Wrong number of args (0) passed to: core$assoc
user/f (NO_SOURCE_FILE:1)
clojure.lang.Var.invoke (Var.java:419)
clojure.lang.Var.applyTo (Var.java:532)
clojure.lang.Compiler.macroexpand1 (Compiler.java:6507)
clojure.lang.Compiler.macroexpand (Compiler.java:6580)
clojure.lang.Compiler.eval (Compiler.java:6654)
clojure.lang.Compiler.eval (Compiler.java:6660)
clojure.lang.Compiler.eval (Compiler.java:6633)
clojure.core/eval (core.clj:2864)
clojure.main/repl/read-eval-print-6594/fn-6597 (main.clj:260)
clojure.main/repl/read-eval-print--6594 (main.clj:260)
clojure.main/repl/fn--6603 (main.clj:278)
nil



 Comments   
Comment by Alex Coventry [ 17/Oct/13 11:01 AM ]

Patch with test

Comment by Alex Coventry [ 23/Oct/13 11:42 PM ]

Amended patch to deal more gracefully with unexpected stack trace structure.

Comment by Alex Miller [ 24/Oct/13 12:09 AM ]

Also see CLJ-397 and CLJ-383.

Comment by Alex Coventry [ 24/Oct/13 2:46 PM ]

Thanks, Alex. It would be easy enough to move most of the logic into ArityException, which would be a compromise between Stu's[1] options 1 and 2. Is that worth doing?

Amending clojure.lang.AFn.throwArity to check whether "this" is a macro and adjust the arg count there accordingly might be the simplest way. I can see why Rich prefers all the logic to go into ArityException, but since ArityExceptions are used for things other than macros, I don't see a way to make an honest error message there without groveling the stack trace.

[1] http://dev.clojure.org/jira/browse/CLJ-397?focusedCommentId=24090&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-24090

Comment by Alex Miller [ 24/Oct/13 2:57 PM ]

I would have to take more time than I have to make an informed opinion but I can say that from a general point of view inspecting StackTraceElements does not seem like the right solution to (almost) any problem.

Comment by Alex Coventry [ 24/Oct/13 10:26 PM ]

This patch causes Var.setMacro to set instance attribute AFn.macrop to true, so that AFn.throwArity can reduce the number of arguments reported.

I'm not used to negotiating java class hierarchies, so it's possible there's a cleaner way. Since Var.fn() returns an IFn, I added macrop handling methods IFn.setMacro and IFn.isMacro. These then needed to be implemented in Ref and Keyword, as well as AFn (where I wanted them) because they implement the IFn interface but don't inherit from AFn.

The real drawback I see with this approach is the duplicated state, though: ^{:macro true} vs AFn.macrop==true.

Comment by Andy Fingerhut [ 25/Oct/13 6:33 PM ]

I have not investigated the reason yet, but neither patch applies cleanly after the latest commits to Clojure master on Oct 25 2013. Given that what kinds of solution methods would be acceptable for this issue, it sounds like more thinking and code changes are probably needed anyway before worrying too much about that.





[CLJ-1278] Provide a useful implementation of toString() for Clojure functions Created: 10/Oct/13  Updated: 14/Oct/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Howard Lewis Ship Assignee: Unassigned
Resolution: Unresolved Votes: 6
Labels: errormsgs, interop

Attachments: Text File CLJ-1278-2.patch     Text File CLJ-1528--function-tostring.patch    
Patch: Code and Test

 Description   

Clojure functions do not provide a useful override of the default Java Object.toString() method.

Because of this, any time a Clojure function is printed out, it places a load on the develop to mentally reverse the necessary name mangling to get back to the namespace and function name.

I would vastly prefer better information here: the non-mangled name of the function, including namespace and, ideally, a little bit of the available meta-data: the file name and line number.

In other words, instead of novate.core.processing.async$locate_destination@2690d691 something more like novate.core.processing.async/locate-destination(async.clj:231).

Ideally, anything that is code generated as a class should implement a useful toString(). It would be nice if reified types could identify the containing namespace and function (and file/line number) as part of their default toString().

I suspect this would need to be controlled by a compiler option as it would slightly increase the generated bytecode size, and (imperceptably?) affect compilation speed.



 Comments   
Comment by Howard Lewis Ship [ 10/Oct/13 8:39 PM ]

Contains changes and updated tests. I don't have any details on if this affects compiler performance or generated code size in any significant or even measurable way.

Comment by Andy Fingerhut [ 11/Oct/13 4:06 PM ]

Howard, sorry I do not have more useful comments on the changes you make in your patch. Right now I only have a couple of minor comments on its form. The preferred format for patches is that created using the instructions shown on this wiki page: http://dev.clojure.org/display/community/Developing+Patches

Also, there are several parts of your patch that appear to only make changes in the whitespace of lines. It would be best to leave such changes out of a proposed patch.

Comment by Howard Lewis Ship [ 11/Oct/13 5:00 PM ]

Yes, I didn't notice the whitespace changes until after; I must have hit reformat at some point, despite my best efforts. I'll put together a new patch shortly.

Comment by Howard Lewis Ship [ 11/Oct/13 6:26 PM ]

Clean patch





[CLJ-1275] print-dup's handling of metadata typehint is unreadable in some circumstances Created: 02/Oct/13  Updated: 05/Feb/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5, Release 1.6
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Alex Coventry Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: metadata, print

Attachments: Text File 0001-Don-t-use-shorthand-for-typehints-when-print-dup.patch    
Patch: Code and Test

 Description   

With print-dup true, if an object being printed has a metadata map with only a :tag key, the printer renders it as "^value". This can cause an IllegalArgumentException if you try to read the printed string back in, in some circumstances. E.g.

user=> (read-string (let [ge (with-meta (gensym) {:tag Object})] (binding [*print-dup* true] (pr-str ge))))
  IllegalArgumentException Metadata must be Symbol,Keyword,String or Map  clojure.lang.LispReader$MetaReader.invoke (LispReader.java:732)

This is causing problems with sleight/riddley's [1] handling of the (case) macro, which drops a type-hint on a gensym it incorporates in the form it returns. When sleight tries to reserialize a macroexpanded (case) form from riddley, it fails as demonstrated above. E.g.

user=> (read-string (binding [*print-dup* true] (pr-str (macroexpand '(case 1 1 1)))))
  user=> IllegalArgumentException Metadata must be Symbol,Keyword,String or Map  clojure.lang.LispReader$MetaReader.invoke (LispReader.java:732)

The attached patch corrects this by making core_print.clj's print-meta always print out the full metadata map if print-dup is true. The patch also contains a test for this case.

[1] https://github.com/ztellman/sleight https://github.com/ztellman/riddley



 Comments   
Comment by Alex Coventry [ 02/Oct/13 10:28 PM ]

Corresponding bug on sleight: https://github.com/ztellman/sleight/issues/5





[CLJ-1272] Agent thread executors do not use the global uncaught exception handler Created: 01/Oct/13  Updated: 05/Feb/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: David Greenberg Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: agents


 Description   

If you use Thread.setDefaultUncaughtExceptionHandler to catch all application exceptions, and then throw an exception in a future, that exception will get swallowed up in deployment environments that don't watch stdout. It seems that the agent's executors ought to delegate to the global handler.

This issue bit us, in that we deploy and monitor our system only through its logs and metrics, and never actually saw that exceptions were being thrown in futures.






[CLJ-1266] Better primitive support for floats Created: 26/Sep/13  Updated: 05/Feb/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Christophe Grand Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: performance

Attachments: File floats.diff     File floats-intrinsics.diff    
Patch: Code

 Description   

Clojure offers optimized arithmetic functions for long, int and doubles but none for floats.
Plus converting from integers (ints or longs) to floating point numbers (float or double) doesn't use the specialized bytecode.
This patch adds float-add/subtract/multiply/divide and more efficient coversion from integers to floating points numbers.



 Comments   
Comment by Alex Miller [ 05/Feb/14 12:29 PM ]

I think it's unlikely the arithmetic float ops will be accepted.
However, the intrinsics changes could be useful - could you split those into a new ticket?

Comment by Christophe Grand [ 05/Feb/14 4:20 PM ]

I attached a new patch with only intrinsics and more comprehensive primitive coercion. Is it the split you expected?

Comment by Alex Miller [ 05/Feb/14 5:22 PM ]

No, but totally my fault for saying the wrong words.

I think the changes in Compiler to get access to I2D, L2D, I2F, and L2F are potentially useful (most particularly L2D) - these would make sense in a new ticket.

The other changes in Intrinsics and Numbers to support float math are unlikely to be accepted.

Comment by Christophe Grand [ 05/Feb/14 5:40 PM ]

Godd thing I had already split coercions in a sparate commit then

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





[CLJ-1263] Allow static compilation of function invocations Created: 14/Sep/13  Updated: 07/Nov/13

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

Type: Enhancement Priority: Major
Reporter: Mike Anderson Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: compiler


 Description   

This proposal is to allow metadata on functions to prevent a fully dynamic var deref to be used whenever the function is called.

When the function is invoked, JVM "invokevirtual" instruction will be used, which is faster than the current implementation (var deref + IFn cast + invokinterface) and has less restrictions (no need to predefine interfaces to match the function parameters). The JVM is generally able to compile such invokevirtual instructions into extremely efficient code - effectively as fast as pure Java.

This is intended to pave the way to better support for statically compiled, high performance code. In particular, it allow:

  • Supporting arbitrary JVM primitives (float, int, byte, char etc.) as well as just double/long.
  • Supporting typed return values e.g. "String". This could eliminate many casts and type checks.
  • Supporting typed reference arguments (e.g. String).

Suggested usage:

(defn ^:static foo ^int [^String a ^String b]
(+ (count a) (Integer/parseInt b)))

Existing code / semantics should not be affected



 Comments   
Comment by Alex Fowler [ 18/Sep/13 5:08 AM ]

Very nice! That is what would really improve experience with certain tasks. I think it will also make possible to work with primitive arrays without the conversions?

Comment by Mike Anderson [ 19/Sep/13 5:44 PM ]

Hi Alex - which aspect of "work with primitive arrays" are you referring to? This feature would certainly help with passing primitive arguments to/from functions that use primitive arrays. It would also potentially help avoid some casts of primitive array arguments themselves. I don't think it helps in any other way - perhaps a separate issue would be appropriate if there is another thing you are trying to do?

Comment by Kevin Downey [ 29/Oct/13 11:50 AM ]

this issue is confusing, because there was/is a :static feature in clojure(which seems to be disabled in the compiler at the moment) and this proposal doesn't mention the existing work at all.

I also think this proposal is begging the question, there is no discussion of other possible solutions to the performance problem (whatever that is) that this is trying to solve.

the (var.deref()(IFn)).invoke(...) is pretty fundamental to the feel of clojure, in fact the existing :static keyword seems to be disabled in the compiler exactly because it complicates those semantics. so we should have a very clear proposal (not a wishlist) if we want to change that with some very clear wins.

maybe an optimizing clojure compiler would be a better approach.

Comment by Mike Anderson [ 30/Oct/13 11:01 PM ]

Hi Kevin,

This is partly in response to this discussion on Clojure-Dev, where we discussed there are quite a lot of performance issues around the way that Clojure passes arguments currently:
https://groups.google.com/d/topic/clojure/H5P25eYKBj4/discussion

Also I believe it reinstates the original intention of "^:static": I can't find where this is/was officially documented, but Arthur's answer in this SO question suggests that this was the case:
http://stackoverflow.com/questions/7552632/what-does-static-do-in-clojure

I think the proposal is relatively clear: it's probably the minimal change required to get static/direct (i.e. not via an indirect var reference / IFn) function invocations without affecting any of the semantics of current code.

This is sufficiently important for me that it's preventing me from shifting some performance-critical code to Clojure (even with primitive type hints). e.g. here's a simple case with a small primitive function:

(defn ^long foo [^long x]
(inc x))

(c/quick-bench (dotimes [i 100] (foo i))) ;; c = criterium
=> Execution time mean : 194.334293 ns

(c/quick-bench (dotimes [i 100] (inc i)))
=> Execution time mean : 71.539048 ns

i.e. the indirect function invocation is costing us nearly 170% overhead. In Java the equivalent functions perform identically: the overhead is zero because with static function invocation the JVM JIT is able to eliminate all the function call overhead.

In the long term, I agree that a proper optimising compiler would be the best way forward (perhaps Clojure 2.0/CinC can give us this?) but in the meantime I think this is a pragmatic way to improve performance with minimal impact on existing code. Even with an optimising compiler, I think we' would need some way to specifiy the "optimised" semantics rather than the indirect var deref behaviour, and "^:static" seems like a reasonable way to do so (unless anyone has a better idea?)

Comment by Kevin Downey [ 04/Nov/13 3:58 PM ]

have you looked at the definition of int and how it uses :inline/definline to avoid the call overhead?

Comment by Mike Anderson [ 05/Nov/13 4:27 AM ]

Good point Kevin - :inline and definline seem like a good approach in many cases (although it's marked as "experimental" - does that mean we can't rely on it to work in future releases?).

This proposal is still somewhat different: the inline solutions and its variants are effectively doing macro expansion to generate code without a function call on the Clojure side. The approach in this proposal would still emit a function call in bytecode, but do so in a way that the JVM can subsequently inline and optimise much more efficiently. Both have their uses, I think?

Commented edited Nov 7 2013 by Andy Fingerhut: Regarding definline marked as experimental, it has been so marked since Clojure 1.0's release, and the plan is to keep it marked that way in the pending Clojure 1.6 release. See discussion thread on CLJ-1281. No plans to remove it that I am aware of.

Comment by Kevin Downey [ 06/Nov/13 2:06 PM ]

my point is your benchmark above is not a comparison of clojure's current deref + cast + invoke vs. invokevirtual, inc is being inlined in to a static method call there

Comment by Kevin Downey [ 06/Nov/13 2:32 PM ]

I've been noodling around this, and it is entirely possible to generate and invoke code in clojure right now without paying the extra deref() cost:

 (defn ^long fib [^long n]
   (case n
     0 0
     1 1
     (+ (fib (dec n))
        (fib  (dec (dec n))))))

can be written as

(declare TheR1798)

(definterface I1797
  (^long fib_Invoke_1 [^long n]))

(deftype R1798 []
  I1797
  (^long fib_Invoke_1
    [this1799 ^long n]
    (case n
      0 0
      1 1
      (+ (.fib_Invoke_1 this1799 (dec n))
         (.fib_Invoke_1 this1799 (dec (dec n)))))))

(def TheR1798 (new R1798))

(defn ^long fib [^long n]
  (.fib_Invoke_1 TheR1798  n)))

now the recursive calls are invokeinterfaces, and the resulting function seems to have mean execution time about 5 times smaller using criterium to bench mark

it is entirely possible to write a macro that translates one in to other, and the weird names in the above are because I have a little proof of concept that does that.

the body of the bytecode for the regular fib function first shown looks something like:

  public final java.lang.Object invokePrim(long);
    Code:
       0: lload_1       
       1: lstore_3      
       2: lload_3       
       3: l2i           
       4: tableswitch   { // 0 to 1
                     0: 28
                     1: 40
               default: 52
          }
      28: lconst_0      
      29: lload_3       
      30: lcmp          
      31: ifne          52
      34: getstatic     #33                 // Field const__1:Ljava/lang/Object;
      37: goto          94
      40: lconst_1      
      41: lload_3       
      42: lcmp          
      43: ifne          52
      46: getstatic     #37                 // Field const__3:Ljava/lang/Object;
      49: goto          94
      52: getstatic     #57                 // Field const__5:Lclojure/lang/Var;
      55: invokevirtual #70                 // Method clojure/lang/Var.getRawRoot:()Ljava/lang/Object;
      58: checkcast     #6                  // class clojure/lang/IFn$LO
      61: lload_1       
      62: invokestatic  #75                 // Method clojure/lang/Numbers.dec:(J)J
      65: invokeinterface #77,  3           // InterfaceMethod clojure/lang/IFn$LO.invokePrim:(J)Ljava/lang/Object;
      70: getstatic     #57                 // Field const__5:Lclojure/lang/Var;
      73: invokevirtual #70                 // Method clojure/lang/Var.getRawRoot:()Ljava/lang/Object;
      76: checkcast     #6                  // class clojure/lang/IFn$LO
      79: lload_1       
      80: invokestatic  #75                 // Method clojure/lang/Numbers.dec:(J)J
      83: invokestatic  #75                 // Method clojure/lang/Numbers.dec:(J)J
      86: invokeinterface #77,  3           // InterfaceMethod clojure/lang/IFn$LO.invokePrim:(J)Ljava/lang/Object;
      91: invokestatic  #81                 // Method clojure/lang/Numbers.add:(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Number;
      94: areturn       
    LineNumberTable:
      line 243: 0
      line 244: 2
      line 247: 52
      line 247: 52
      line 247: 61
      line 248: 70
      line 248: 79
      line 248: 79
    LocalVariableTable:
      Start  Length  Slot  Name   Signature
             2      92     3 G__301   J
             0      94     0  this   Ljava/lang/Object;
             0      94     1     n   J

  public java.lang.Object invoke(java.lang.Object);
    Code:
       0: aload_0       
       1: aload_1       
       2: checkcast     #89                 // class java/lang/Number
       5: invokestatic  #93                 // Method clojure/lang/RT.longCast:(Ljava/lang/Object;)J
       8: invokeinterface #77,  3           // InterfaceMethod clojure/lang/IFn$LO.invokePrim:(J)Ljava/lang/Object;
      13: areturn       

the body of the "optimized" version looks like:

  public long fib_Invoke_1(long);
    Code:
       0: lload_1       
       1: lstore_3      
       2: lload_3       
       3: l2i           
       4: tableswitch   { // 0 to 1
                     0: 28
                     1: 38
               default: 48
          }
      28: lconst_0      
      29: lload_3       
      30: lcmp          
      31: ifne          48
      34: lconst_0      
      35: goto          80
      38: lconst_1      
      39: lload_3       
      40: lcmp          
      41: ifne          48
      44: lconst_1      
      45: goto          80
      48: aload_0       
      49: checkcast     #6                  // class com/thelastcitadel/kernel/I2364
      52: lload_1       
      53: invokestatic  #53                 // Method clojure/lang/Numbers.dec:(J)J
      56: invokeinterface #55,  3           // InterfaceMethod com/thelastcitadel/kernel/I2364.fib_Invoke_1:(J)J
      61: aload_0       
      62: checkcast     #6                  // class com/thelastcitadel/kernel/I2364
      65: lload_1       
      66: invokestatic  #53                 // Method clojure/lang/Numbers.dec:(J)J
      69: invokestatic  #53                 // Method clojure/lang/Numbers.dec:(J)J
      72: invokeinterface #55,  3           // InterfaceMethod com/thelastcitadel/kernel/I2364.fib_Invoke_1:(J)J
      77: invokestatic  #59                 // Method clojure/lang/Numbers.add:(JJ)J
      80: lreturn       
    LineNumberTable:
      line 251: 0
      line 251: 2
      line 251: 48
      line 251: 48
      line 251: 52
      line 251: 61
      line 251: 65
      line 251: 65
    LocalVariableTable:
      Start  Length  Slot  Name   Signature
             2      78     3 G__2363   J
             0      80     0  this   Lcom/thelastcitadel/kernel/R2365;
             0      80     1     n   J

so the calls are not invokevirtual (due to the way clojure compiles stuff, you cannot type anything inside a record as being that record's type), but the interface is unique and only has one instance, so I think the jvm's class hierarchy analysis makes short work of that.

if I have time I may try and complete my macro and release it as a library, but given tools.analyzer.jvm someone should be able to do better than my little proof of concept very quickly.

Comment by Andy Fingerhut [ 07/Nov/13 12:48 PM ]

I don't know if my editing of Mike Anderson's Nov 5 2013 comment is notified to people watching this ticket, so adding a new comment so those interested in definline's experimental status can know to go back and re-read it.





[CLJ-1257] Suppress warnings when clojure.core symbols are explicitly replaced with "use" or "refer" Created: 06/Sep/13  Updated: 05/Feb/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Mike Anderson Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: namespace

Attachments: File clj-1257-2.diff     File clj-1257.diff    
Patch: Code

 Description   

Problem: Libraries that provide DSLs (such as core.matrix) often replace or extend functions in core (such as "+", "==", "zero?"), since it is desirable to use the best / most idiomatic names.

Currently importing such libraries with "use" causes unwanted warnings like "WARNING: + already refers to: #'clojure.core/+ in namespace: test.blank, being replaced by: #'clojure.core.matrix/+".

Avoiding these warnings requires extra user effort and boilerplate code, which is frustrating for users since they have already explicitly asked for the full library to be imported into the current namespace (i.e. via "use" or ":refer :all").

Proposed solution is to introduce a new var warn-on-replace similar to warn-on-reflection which allows this warning to be controlled.



 Comments   
Comment by Mike Anderson [ 06/Sep/13 10:40 PM ]

Basic patch and test attached.

Comment by Andy Fingerhut [ 07/Sep/13 3:22 PM ]

I have no idea whether this idea will be vetted or not, but if it is, I have some comments on the proposed patch.

The new symbol warn-on-replace should have doc and metadata on it. See add-doc-and-meta for warn-on-reflection in core.clj for an example to copy and edit.

You check WARN_ON_REFLECTION before issuing the warning in Namespace.java, instead of WARN_ON_REPLACE.

Possible typo in the test description in ns_libs.clj: Maybe "symbol in clojure.core" instead of "symbol is clojure.core"?

If someone wants warnings from :use statements in ns forms, it seems the only way to do that with patch clj-1257.diff would be to do (set! warn-on-replace true) in a file before the ns form. That would not work well with the current version of tools.namespace, which assumes that if there is an ns form, it is the first form in the file. One can argue that tools.namespace should not make such an assumption, but it does right now.

Perhaps there should be a command line option clojure.compile.warn-on-replace like there is for clojure.compile.warn-on-reflection (search for warn-on-replace in Compile.java)?

Comment by Mike Anderson [ 07/Sep/13 11:09 PM ]

Thanks Andy for the feedback! I'll post an updated patch shortly.

It occurs to me that we should probably implement a more general approach to warnings in Clojure. Adding new vars and command line options for each warning doesn't seem like a good long-term strategy. I think that's beyond the scope of this patch though.

Comment by Andy Fingerhut [ 08/Sep/13 12:49 AM ]

Actually, there is something called compiler-options (search for the variations compiler-options, COMPILER_OPTIONS, and compiler_options for all related occurrences) that is a map where each key/value pair is a different option. That might be preferable for warn-on-replace, if it is in fact desired.

Comment by Mike Anderson [ 08/Sep/13 1:47 AM ]

Updated patch attached.

Compiler-options looks like it may indeed be a better place for this, if that is the preferred strategy for controlling warnings. But I'll wait for more feedback / confirmation on the approach before making that change.

Comment by Alex Miller [ 09/Sep/13 1:43 PM ]

Is (:refer-clojure :exclude [+ = zero?]) a valid workaround? Or are you really concerned with the consumers of the library?

Comment by Mike Anderson [ 09/Sep/13 7:18 PM ]

I'm mainly concerned with consumers of the library.

So while (:refer-clojure :exclude [+ = zero?]) is possible as a temporary workaround, it's very inconvenient for users. We should really fix this in Clojure itself. Users have enough trouble with ns forms already without adding to their woes

As an alternative solution: I personally wouldn't mind it if the library author could add some metadata to symbols (e.g. "^:replace-symbol") to signal that a library function is intended to replace something in core. That's a slightly different approach (and I think a bit trickier to implement) but it should also work.





[CLJ-1256] Support type-hinted overrides of function parameters Created: 06/Sep/13  Updated: 09/Sep/13

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

Type: Enhancement Priority: Major
Reporter: Mike Anderson Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: compiler, interop, typehints


 Description   

Problem: in many cases, the Clojure compiler has enough information about the type of a function argument to statically emit maximally efficient code on the JVM (i.e. without instance? checks, type casts or other forms of dynamic polymorphic dispatch). We are currently unable to do so in Clojure, which pushes developers with strong performance requirements to use some unidiomatic or convoluted workarounds.

Proposal is simply to allow functions to take type-hinted overloads of function arguments, e.g.

(defn foo
([^double x] (Math/floor x))
([^float x] (Math/floor (double x)))
([^String s] (count s)))

An "Object" version of the code with the correct arity will always be emitted, which will maintain compatibility with the IFn interface and ensure that the function can still be used in dynamic / interactive contexts. If the "Object" version is not explicitly provided, then it will be generated to use instance? checks that subsequently delegate to the appropriate typed version of the function (or throw an InvalidArgumentException if no match is found).

Matching rules would be the same as Java.

This will be backwards compatible with all existing uses of defn. In particular, it should extend / enhance / supercede the existing handling of primitive functions.

In the future, this technique might be used alongside core.typed to ensure that the most efficient function version is chosen based on type inference.






[CLJ-1255] Support Abstract Base Classes with "reify" Created: 06/Sep/13  Updated: 06/Sep/13

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

Type: Enhancement Priority: Major
Reporter: Mike Anderson Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: interop


 Description   

Problem:

  • Various Java APIs depend on extension of abstract base classes rather than interfaces
  • "proxy" has limitations (no access to protected fields or super)
  • "proxy" has performance overhead because of an extra layer of functions / parameter boxing etc.
  • "gen-class" is complex and is complected with compilation / bytecode generation

In summary, Clojure does not currently have a good / convenient way to extend a Java abstract base class dynamically.

Proposal is to extend "reify" to allow a single abstract base class as well as interfaces/protocols. Code generation would occur as if the abstract base class had been directly extended in Java (i.e. with full access to protected members and with fully type-hinted fields). This change would be backwards-compatible with all existing uses of "reify".






[CLJ-1251] The update function: like update-in but allows multiple (shallow) updates at once. Created: 03/Sep/13  Updated: 14/Feb/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.6
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Michael O. Church Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

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

 Description   

Like update-in, update allows functions to be applied to specified keys in an associative data structure. It does not support updates deeper than 1 level, and does not require a seq or vector of keys to be specified.

Unlike update-in, update allows multiple fields to be updated in one call, e.g.:

(def heal-spell [creature] (update creature :HP #(+ % 100) :MP #(- % 8) :spells-cast inc))

This is usually one of the first things I find myself adding to the language on my own projects, so I thought it'd be worth submitting. Apologies if this has already been discussed before.



 Comments   
Comment by Alex Miller [ 06/Sep/13 9:56 AM ]

I like this - kind of halfway between assoc and update-in.

Comment by Michael O. Church [ 07/Sep/13 12:41 PM ]

It's very useful. I assumed that its non-inclusion was for a reason (hence was hesitant to submit the patch) but it comes in handy a lot. One project I'd like to do with some free time is a library for turn-based strategy games, which use update frequently to express game-state changes.

The downside of this change is that 'update is probably a defined function in a good number of modules written by other people. IMO the strongest reason not to include it is that it's such a common name; but the benefits (in my view) outweigh the downsides.

Comment by Andy Fingerhut [ 14/Feb/14 11:50 AM ]

Patch update.patch dated Sep 3 2013 no longer applies cleanly to latest Clojure master as of Feb 14 2014. It did on Feb 7 2014. I haven't checked in detail, but this is probably simply due to some tests recently added to a test file that require updating some diff context lines.





[CLJ-1243] Cannot resolve public generic method from package-private base class Created: 01/Aug/13  Updated: 17/Nov/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.3, Release 1.4, Release 1.5
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Stuart Sierra Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: interop

Attachments: GZip Archive clj-1243-demo1.tar.gz    

 Description   

The Clojure compiler cannot resolve a public generic method inherited from a package-private base class.

Instructions to reproduce:

  • In package P1
    • Define a package-private class A with generic type parameters
    • Define a public method M in A using generic types in either its arguments or return value
    • Define a public class B which extends A
  • In package P2
    • Construct an instance of B
    • Invoke B.M()

This is valid in Java. In Clojure, invoking B.M produces a reflection warning, followed by the error "java.lang.IllegalArgumentException: Can't call public method of non-public class." No amount of type-hinting prevents the warning or the error.

Attachment clj-1243-demo1.tar.gz contains sample code and script to demonstrate the problem.

Examples of Java projects which use public methods in package-private classes:



 Comments   
Comment by Stuart Sierra [ 01/Aug/13 5:11 PM ]

It is also not possible to call the method reflectively from Java.

This may be a bug in Java reflection: JDK-4283544

But why does it only happen on generic methods?

Comment by Stuart Sierra [ 08/Aug/13 11:59 AM ]

According to Rich Hickey, the presence of bridge methods is unspecified and inconsistent across JDK versions.

A possible solution is to use ASM to examine the bytecode of third-party Java classes, instead of the reflection API. That way the Clojure compiler would have access to the same information as the Java compiler.

Comment by Andy Fingerhut [ 17/Nov/13 11:01 PM ]

CLJ-1183 was closed as a duplicate of this one. Mentioning it here in case anyone working on this ticket wants to follow the link to it and read discussion or test cases described there.





[CLJ-1242] get/= on sorted collections when types don't match result in a ClassCastException Created: 31/Jul/13  Updated: 31/Jul/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5, Release 1.6
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Nicola Mometto Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: None

Attachments: Text File 0001-fix-for-CLJ-1242-tests.patch    
Patch: Code and Test

 Description   

user=> (= (sorted-set 1) #{:a})
ClassCastException java.lang.Long cannot be cast to clojure.lang.Keyword clojure.lang.Keyword.compareTo (Keyword.java:109)

but

user=> (= (sorted-set 1) :a)
false

also

user=> (get (sorted-set 1) :a 2)
ClassCastException java.lang.Long cannot be cast to clojure.lang.Keyword clojure.lang.Keyword.compareTo (Keyword.java:109)



 Comments   
Comment by OHTA Shogo [ 31/Jul/13 8:02 PM ]

PersistentVector also has the same problem.

user=> (compare [1] [:a])
java.lang.ClassCastException: clojure.lang.Keyword cannot be cast to java.lang.Number

The cause of this problem is that Util.compare() casts the second argument
to Number without checking its type when the first argument is a Number.

Comment by OHTA Shogo [ 31/Jul/13 8:26 PM ]

Umm, my brain was not working right.
Util.compare() should raise an Exception when the arguments' type are different.





[CLJ-1240] Clarify limits of clojure.walk/macroexpand-all in docstring Created: 29/Jul/13  Updated: 03/Sep/13

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

Type: Enhancement Priority: Trivial
Reporter: Stuart Sierra Assignee: Stuart Sierra
Resolution: Unresolved Votes: 0
Labels: walk

Attachments: Text File 0001-CLJ-1240-Note-limits-of-clojure.walk-macroexpand-all.patch    
Patch: Code

 Description   

The clojure.walk/macroexpand-all function appears to be a general recursive macroexpansion, but it is not because it doesn't understand special forms such as let.

Current patch: 0001-CLJ-1240-Note-limits-of-clojure.walk-macroexpand-all.patch

The modified docstring in this patch notes that clojure.walk/macroexpand-all is not identical to macroexpansion by the Clojure compiler and should be used for development only.






[CLJ-1239] faster, more flexible dispatch for clojure.walk Created: 29/Jul/13  Updated: 27/Feb/14

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

Type: Enhancement Priority: Minor
Reporter: Stuart Sierra Assignee: Stuart Sierra
Resolution: Unresolved Votes: 4
Labels: walk

Attachments: Text File 0001-CLJ-1239-protocol-dispatch-for-clojure.walk.patch     Text File 0002-CLJ-1239-protocol-dispatch-for-clojure.walk.patch    
Patch: Code

 Description   

The conditional dispatch in clojure.walk is slow and not open to extension. Prior to CLJ-1105 it did not support records.

Approach: Reimplement clojure.walk using protocols. The public API does not change. Users can extend the walk protocol to other types (for example, Java collections) if desired. As in CLJ-1105, this version of clojure.walk supports records.

Patch: 0002-CLJ-1239-protocol-dispatch-for-clojure.walk.patch

Performance: My tests indicate this is 1.5x-2x the speed of the original clojure.walk. See https://github.com/stuartsierra/clojure.walk2 for benchmarks.

Risks: This approach carries some risk of breaking user code that relied on type-specific behavior of the old clojure.walk. When running the full Clojure test suite, I discovered (and fixed) some breakages that did not show up in clojure.walk's unit tests. See, for example, commit 730eb75 in clojure.walk2



 Comments   
Comment by Vjeran Marcinko [ 19/Oct/13 12:32 PM ]

It looks, as it is now, that walking the tree and replacing forms doesn't preserve original meta-data contained in data structures.

Comment by Andy Fingerhut [ 23/Nov/13 1:11 AM ]

Patch 0001-CLJ-1239-protocol-dispatch-for-clojure.walk.patch no longer applies cleanly to latest Clojure master since the patch for CLJ-1105 was committed on Nov 22, 2013. From the description, it appears the intent was either that patch or this one, not both, so I am not sure what should happen with this patch, or even this ticket.

Comment by Alex Miller [ 23/Nov/13 1:52 AM ]

This patch and ticket are still candidates for future release.

Comment by Stuart Sierra [ 20/Dec/13 9:14 AM ]

Added new patch that applies on latest master after CLJ-1105.

Comment by Chouser [ 27/Feb/14 10:26 AM ]

The way this patch behaves can be surprising compared to regular maps:

(clojure.walk/prewalk-replace {[:a 1] nil} {:a 1, :b 2})
;=> {:b 2}

(defrecord Foo [a b])
(clojure.walk/prewalk-replace {[:a 1] nil} (map->Foo {:a 1, :b 2}))
;=> #user.Foo{:a 1, :b 2}

Note how the [:a 1] entry is removed from the map, but not from the record.

Here's an implementation that doesn't suffer from that problem, though it does scary class name munging instead: https://github.com/LonoCloud/synthread/blob/a315f861e04fd33ba5398adf6b5e75579d18ce4c/src/lonocloud/synthread/impl.clj#L66

Perhaps we could add to the defrecord abstraction to support well the kind of things that synthread code is doing clumsily, and then walk could take advantage of that.

Comment by Alex Miller [ 27/Feb/14 2:11 PM ]

@Chouser, can you file a new ticket related to this? It's hard to manage work on something from comments on a closed ticket.

Comment by Alex Miller [ 27/Feb/14 3:54 PM ]

@Chouser - Never mind! I was thinking this was the change that went into 1.6. Carry on.

Comment by Nicola Mometto [ 27/Feb/14 5:17 PM ]

Alex, for what it matters clojure-1.6.0 after CLJ-1105 exibits the same behaviour as described by Chouser for this patch





[CLJ-1237] reduce gives a SO for pathological seqs Created: 27/Jul/13  Updated: 25/Aug/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Gary Fredericks Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: None
Environment:

1.5.1


Attachments: Text File CLJ-1237c.patch    
Patch: Code and Test

 Description   

reduce gives a StackOverflowError on long sequences that contain many transitions between chunked and unchunked:

(->> (repeat 50000 (cons :x [:y]))
     (apply concat)
     (reduce (constantly nil)))
;; throws StackOverflowError

Such a sequence is well behaved under most other sequence operations, and its underlying structure can even be masked such that reduce succeeds:

(->> (repeat 50000 (cons :x [:y]))
     (apply concat)
     (take 10000000)
     (reduce (constantly nil)))
;; => nil

I don't think Clojure developers normally worry about mixing chunked and unchunked seqs, so the existence of such a sequence is not at all unreasonable (and indeed this happened to me at work and was very difficult to debug).

It seems obvious what causes this given the implementation of reduce – it bounces back and forth between the chunked impl and the unchunked impl, consuming more and more stack as it goes. Without proper tail call optimization, it's not obvious to me what a good fix would be.

Presumed bad solutions

Degrade to naive impl after first chunk

In the IChunkedSeq implementation, instead of calling internal-reduce when the
sequence stops being chunked, it could have an (inlined?) unoptimized implementation,
ensuring that no further stack space is taken up. This retains the behavior that a
generic seq with a chunked tail will still run in an optimized fashion, but a seq with
two chunked portions would only be optimized the first time.

Use clojure.core/trampoline

This would presumably work, but requires wrapping the normal return values from all
implementations of internal-reduce.

Proposed Solution

(attached as CLJ-1237c.patch)

Similar to using trampoline, but create a special type (Unreduced) that signals
an implementation change. The two implementation-change points in internal-reduce
(in the IChunkedSeq impl and the Object impl) are converted to return an instance
of Unreduced instead of a direct call to internal-reduce.

Then seq-reduce is converted to check for instances of Unreduced before returning,
and recurs if it finds one.

Pros

  • Only requires one additional check in most cases
  • Reduces stack usage for existing heterogeneous reductions that weren't extreme enough to crash
  • Should be compatible with 3rd-party implementations of internal-reduce, which can still use the old style (direct recursive calls to internal-reduce) or the optimized style if desired.

Cons

  • internal-reduce is slightly more complicated
  • There's an extra check at the end of seq-reduce – is that a performance worry?


 Comments   
Comment by Gary Fredericks [ 25/Aug/13 4:13 PM ]

Added patch.





[CLJ-1232] Functions with non-qualified return type hints force import of hinted classes when called from other namespace Created: 18/Jul/13  Updated: 17/Apr/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Tassilo Horn Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: typehints


 Description   

You can add a type hint to function arglists to indicate the return type of a function like so.

user> (import '(java.util List))
java.util.List
user> (defn linkedlist ^List [] (java.util.LinkedList.))
#'user/linkedlist
user> (.size (linkedlist))
0

The problem is that now when I call `linkedlist` exactly as above from another namespace, I'll get an exception because java.util.List is not imported in there.

user> (in-ns 'user2)
#<Namespace user2>
user2> (refer 'user)
nil
user2> (.size (linkedlist))
CompilerException java.lang.IllegalArgumentException: Unable to resolve classname: List, compiling:(NO_SOURCE_PATH:1:1)
user2> (import '(java.util List)) ;; Too bad, need to import List here, too.
java.util.List
user2> (.size (linkedlist))
0

There are two workarounds: You can import the hinted type also in the calling namespace, or you always use fully qualified class names for return type hints. Clearly, the latter is preferable.

But clearly, that's a bug that should be fixed. It's not in analogy to type hints on function parameters which may be simple-named without having any consequences for callers from other namespaces.



 Comments   
Comment by Andy Fingerhut [ 16/Apr/14 3:47 PM ]

To make sure I understand, Nicola, in this ticket you are asking that the Clojure compiler change behavior so that the sample code works correctly with no exceptions, the same way as it would work correctly without exceptions if one of the workarounds were used?

Comment by Tassilo Horn [ 17/Apr/14 12:18 AM ]

Hi Andy. Tassilo here, not Nicola. But yes, the example should work as-is. When I'm allowed to use type hints with simple imported class names for arguments, then doing so for return values should work, too.





[CLJ-1231] fn and letfn don't support hinting the function's return type Created: 17/Jul/13  Updated: 03/Sep/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Tassilo Horn Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: typehints


 Description   

You can add type hints to the argument vector(s) of `defn` to declare the return type of a function like so (with warn-on-reflection being true):

user> (defn foo ^String [s] s)
#'user/foo
user> (.substring (foo "hallo") 1 2)
"a"

But sadly, the same doesn't work with `fn` and `letfn`:

user> (.substring ((fn ^String [s] s) "hallo") 1 2)
Reflection warning, NO_SOURCE_PATH:1:1 - call to substring can't be resolved.
"a"
user> (letfn [(foo ^String [s] s)]
	(.substring (foo "hallo") 1 2))
Reflection warning, NO_SOURCE_PATH:2:7 - call to substring can't be resolved.
"a"

I don't see why this feature is available to `defn` but not to `fn` and `letfn`. I even consider it a kind of defect, because anything else including :pre/:post are also supported by the latter two, so the support for hinting the return type should be there simply because of analogy.






[CLJ-1230] print-table displays tables incorrectly when one of the cells is a string that has newlines Created: 09/Jul/13  Updated: 31/Jan/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Ben Booth Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: print
Environment:

Mac OS X, Clojure 1.5.1



 Description   

When using print-table to print an ASCII table to stdout, the table display breaks if any of the values is a string with any new lines in it. For example:

user=> (print-table [{:a "test" :b "test\ntest2"}])

|   :a |         :b |
|------+------------|
| test | test
test2 |
nil

I would expect the output to look something like this:

user=> (print-table [{:a "test" :b "test\ntest2"}])

|   :a |         :b |
|------+------------|
| test | test       +
|      | test2      |
nil

The + symbol on the right border means that the row continues over multiple lines. This is similar to how the PostgreSQL psql tool displays table with multi-line rows:

user=# select 'test' col1, E'test\ntest2\ntest3' col2;
 col1 | col2  
------+-------
 test | test +
      | test2+
      | test3
(1 row)

Time: 0.776 ms


 Comments   
Comment by Ben Booth [ 09/Jul/13 2:10 PM ]

JIRA destroyed my formatting, and looks like I can't edit it to fix it. Here is what I meant to say:

When using print-table to print an ASCII table to stdout, the table display breaks if any of the values is a string with any new lines in it. For example:

user=> (print-table [{:a "test" :b "test\ntest2"}])

|   :a |         :b |
|------+------------|
| test | test
test2 |

I would expect the output to look something like this:

user=> (print-table [{:a "test" :b "test\ntest2"}])
|   :a |         :b |
|------+------------|
| test | test       +
|      | test2      |

The + symbol on the right border means that the row continues over multiple lines. This is similar to how the PostgreSQL psql tool displays table with multi-line rows:

labtrack=# select 'test' col1, E'test\ntest2' col2;
 col1 | col2  
------+-------
 test | test +
      | test2
(1 row)
Comment by Andy Fingerhut [ 22/Jul/13 2:39 AM ]

I have no direct knowledge of this, but my guess would be that the Clojure team would consider this an enhancement request rather than a defect.

You are likely to get what you want faster either by writing your own version of print-table that works as you wish, or see whether these projects already behave as desired, or the authors are willing to enhance them: https://github.com/cldwalker/table or https://github.com/joegallo/doric





[CLJ-1229] count silently overflows for sequences with more than Integer/MAX_VALUE elements Created: 09/Jul/13  Updated: 03/Sep/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Andy Fingerhut Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: math

Attachments: Text File clj-1229-count-overflow-patch-v1.txt    
Patch: Code

 Description   

Found by John Jacobsen: https://mail.google.com/mail/?shva=1#label/clojure/13fbba0c3e4ba6b7

user> (time (count (range (*' 1000 1000 1000 3))))
"Elapsed time: 375225.663 msecs"
-1294967296



 Comments   
Comment by Andy Fingerhut [ 09/Jul/13 1:41 AM ]

Patch clj-1229-count-overflow-patch-v1.txt dated Jul 8 2013 modifies count to throw an ArithmeticException for sequences with more than Integer/MAX_VALUE elements.

Performance experiments with this expression show no significant time differences before and after the change. I verified with -XX:+PrintCompilation, and only paid attention to timing results after no further JIT compilation was performed when executing the expression:

(defn foo [n m] (doall (for [i (range n)] (count (range m)))))
(time (foo 100 1000000))

No tests are added. A test verifying that an exception is thrown for such a long sequence would be prohibitively slow.

Comment by Mike Anderson [ 09/Jul/13 3:08 AM ]

I think a better strategy might be to convert all count-like functions from int to long. int overflow exceptions aren't a particularly nice result, especially since they probably won't get picked up in tests for the reasons Andy mentioned.

That buys us a quite a few years more future proofing...

Comment by Andy Fingerhut [ 09/Jul/13 10:15 AM ]

Mike, unless I am missing something, that would require changing the method count() in the Counted interface to return a long, and that in turn requires little changes throughout the code base wherever Counted is used. It could be done, of course, but it is not a small change.

Comment by John Jacobsen [ 09/Jul/13 12:35 PM ]

I agree that Mike's approach is nicer overall, but think Andy's patch is an immediate improvement over what we have now, and could be implemented until someone takes the time to correctly make all the detailed changes Mike is suggesting.

Comment by John Jacobsen [ 09/Jul/13 12:52 PM ]

FWIW I did apply the patch, build, and test manually:

user=> (count (range (* 1000 1000 1000 3)))
ArithmeticException integer overflow clojure.lang.RT.countFrom (RT.java:549)
user=>

Comment by Alex Miller [ 11/Jul/13 3:26 PM ]

Perhaps of interest, Java's Collection.size() returns Integer.MAX_VALUE if the size of the collection > MAX_VALUE. I can't say that either that behavior or overflow is particularly helpful in practice of course.





[CLJ-1226] set! of InstanceFieldExpr results in wrong type emission Created: 26/Jun/13  Updated: 03/Sep/13

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

Type: Defect Priority: Minor
Reporter: Nicola Mometto Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: deftype

Attachments: Text File 0001-fix-CLJ-1226.patch    
Patch: Code

 Description   

Clojure 1.6.0-master-SNAPSHOT

user=> (defprotocol p (f [_]))
p
user=> (deftype t [^:unsynchronized-mutable x] p (f [_] (set! (.x _) 1)))
user.t
user=> (f (t. 1))
ClassCastException user.t cannot be cast to compile__stub.user.t user.t (NO_SOURCE_FILE:1

After patch:
Clojure 1.6.0-master-SNAPSHOT
user=> (defprotocol p (f [_]))
p
user=> (deftype t [^:unsynchronized-mutable x] p (f [_] (set! (.x _) 1)))
user.t
user=> (f (t. 1))
1



 Comments   
Comment by Nicola Mometto [ 27/Jun/13 5:30 AM ]

This patch offers a better workaround for CLJ-1075, making it possible to write
(deftype foo [^:unsynchronized-mutable x] MutableX (set-x [this v] (try (set! (.x this) v)) v))





[CLJ-1223] Improve App Engine Support by Providing an Option to Use the App Engine ThreadManger Created: 22/Jun/13  Updated: 03/Sep/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: James Thornton Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None
Environment:

Google App Engine



 Description   

Clojure support for App Engine can be improved now that App Engine supports sockets and threads.

Sockets
https://developers.google.com/appengine/docs/java/sockets/overview

Threads
https://developers.google.com/appengine/docs/java/javadoc/com/google/appengine/api/ThreadManager

The new sockets API uses java.net.Socket so it should just work without much/any modification to core. The Sockets API is an important addition because it enables you to connect to Compute Engine servers from App Engine without having to use the HTTP client.

Presently Clojure's agents and futures do not work on App Engine, but the new ThreadManager API now allows you to execute short-lived threads; however, you must use the ThreadManger API to create threads – "you cannot invoke new Thread() yourself or use the default thread factory" and "each request is limited to 50 concurrent request threads" – so adding support for threads will require a build option and some modifications.

Supporting agents on App Engine could be done by adding a Clojure build option for App Engine that includes the App Engine JDK dependency and injects the App Engine TreadManager factory into jvm/clojure/lang/Agent.java .

Google App Engine SDK for Java
https://developers.google.com/appengine/downloads#Google_App_Engine_SDK_for_Java

See also https://developers.google.com/appengine/docs/java/runtime#The_Sandbox ...

A Java application can create a new thread, but there are some restrictions on how to do it. These threads can't "outlive" the request that creates them. (On a backend server, an application can spawn a background thread, a thread that can "outlive" the request that creates it.)

An application can...

Implement java.lang.Runnable; and
Create a thread factory by calling com.google.appengine.api.ThreadManager.currentRequestThreadFactory()
call the factory's newRequestThread method, passing in the Runnable, newRequestThread(runnable)
or use the factory object returned by com.google.appengine.api.ThreadManager.currentRequestThreadFactory() with an ExecutorService (e.g., callExecutors.newCachedThreadPool(factory)).

However, you must use one of the methods on ThreadManager to create your threads. You cannot invoke new Thread() yourself or use the default thread factory.

An application can perform operations against the current thread, such as thread.interrupt().

Each request is limited to 50 concurrent request threads.

App Engine JRE Whitelist:
https://developers.google.com/appengine/docs/java/jrewhitelist

From the current App Engine Magic README (https://github.com/gcv/appengine-magic)...

"Google App Engine maintains a whitelist of permitted classes in Java's standard library. Other classes will cause your application to fail to deploy. Examples include threads and sockets. If you use those in your application, it will not work. This means that you cannot use Clojure's agents or futures. In addition, if one of your dependencies uses those, your application will also not work. For example, clojure.java.io (and its fore-runner, duck-streams from clojure-contrib), uses java.net.Socket, a forbidden class."






[CLJ-1221] Should repackage jsr166 and include known version with Clojure Created: 20/Jun/13  Updated: 03/Sep/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Alex Miller Assignee: Alex Miller
Resolution: Unresolved Votes: 0
Labels: reducers


 Description   

Clojure 1.5 reducers work with either the JDK version of forkjoin (JDK 1.7+) or with an external jsr166 jar. This causes complexity for users and complexity in the build to deal with the two options.

jsr166 code is public domain and it is common for other projects to repackage the handful of files and ship it with the project (similar to what we do with asm). This would allow us just use a known existing version of jsr166 across all jdks and we could get rid of the custom build wrangling we introduced in Clojure 1.5.

jsr166y is compatible with JDK 1.6+ and is the version that (for example) Scala currently repackages. That's the best choice for JDK 1.6 and 1.7. In JDK 1.8, the best choice will (temporarily) be the built-in version in java.util.concurrent which tracks jsr166e but then as soon as there are updates will become jsr166e. Many fork/join fixes are ported to both y and e right now.

Some choices here for JDK 1.8:

  • go for maximal compatibility just use repackaged jsr166y regardless of JDK (simplest)
  • check for jdk version # and use java.util.concurrent instead
  • check for jdk version # and repackage jsr166e and use it instead

Not sure yet which of these is best choice right now.






[CLJ-1218] mapcat is too eager Created: 16/Jun/13  Updated: 05/Feb/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Gary Fredericks Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: lazy


 Description   

The following expression prints 1234 and returns 1:

(first (mapcat #(do (print %) [%]) '(1 2 3 4 5 6 7)))

The reason is that (apply concat args) is not maximally lazy in its arguments, and indeed will realize the first four before returning the first item. This in turn is essentially unavoidable for a variadic concat.

This could either be fixed just in mapcat, or by adding a new function (to clojure.core?) that is a non-variadic equivalent to concat, and reimplementing mapcat with it:

(defn join
  "Lazily concatenates a sequence-of-sequences into a flat sequence."
  [s]
  (lazy-seq (when-let [[x & xs] (seq s)] (concat x (join xs)))))


 Comments   
Comment by Gary Fredericks [ 17/Jun/13 7:54 AM ]

I realized that concat could actually be made lazier without changing its semantics, if it had a single [& args] clause that was then implemented similarly to join above.

Comment by John Jacobsen [ 27/Jul/13 8:08 AM ]

I lost several hours understanding this issue last month [1, 2] before seeing this ticket in Jira today... +1.

[1] http://eigenhombre.com/2013/07/13/updating-the-genome-decoder-resulting-consequences/

[2] http://clojurian.blogspot.com/2012/11/beware-of-mapcat.html

Comment by Gary Fredericks [ 05/Feb/14 1:35 PM ]

Updated join code to be actually valid.





[CLJ-1217] for consumes sequence argument more eagerly than necessary Created: 14/Jun/13  Updated: 05/Feb/14

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

Type: Enhancement Priority: Minor
Reporter: Alan Malloy Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: lazy

Attachments: Text File 0001-Don-t-realize-seq-exprs-in-for-unless-necessary.patch    
Patch: Code

 Description   

In a call like (do (for [x (do (println "realized") nil)] x) nil), no elements of the for comprehension are ever requested, and so it is not actually necessary to evaluate the inner do-block. However, this expression causes "realized" to be printed, because the first sequence-expression in for is evaluated even if no items are ever requested from the output lazy-seq.

It's not documented whether this is intended or unintentional, but I was surprised by this behavior, and a brief unscientific survey on #clojure suggests that other users, even "old hands" who've been using clojure for years, don't expect this either.

I've attached a patch that wraps the problematic expression in a lazy-seq call. This is not quite ideal, because it means that the first iteration is "lazied" twice, as in ((fn step [s] (lazy-seq ...)) (lazy-seq xs)), but a change to make this not happen would be much broader in scope, and this seemed the least dangerous.






[CLJ-1216] Evaling ((fn [do] do) 1) returns nil while ((fn [do] do do) 1) returns 1 Created: 09/Jun/13  Updated: 03/Sep/13

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

Type: Defect Priority: Minor
Reporter: Nicola Mometto Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: compiler

Attachments: Text File 0001-Create-a-DoExpr.Parser-class-that-delegates-to-BodyE.patch    
Patch: Code and Test

 Description   

user=> ((fn [do] do) 1)
nil

user=> ((fn [do] (do do)) 1)
1

user=> ((fn [] do))
nil

user=> ((fn [] do do))
CompilerException java.lang.RuntimeException: Unable to resolve symbol: do in this context, compiling:(NO_SOURCE_PATH:0:0)



 Comments   
Comment by Nicola Mometto [ 09/Jun/13 4:31 PM ]

This patch creates a DoExpr class and makes DoExpr.Parser the DO special form parser.

DoExpr.Parser simply removes the 'do' symbol and delegates to BodyExpr, that was previously done by BodyExpr incorrectly.





[CLJ-1212] Silent truncation/downcasting of primitive type on reflection call to overloaded method (Math/abs) Created: 28/May/13  Updated: 05/Feb/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Matthew Willson Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: primitives, typehints
Environment:

Clojure 1.5.1
OpenJDK Runtime Environment (IcedTea6 1.12.5) (6b27-1.12.5-0ubuntu0.12.04.1)



 Description   

I realise relying on reflection when calling these kinds of methods isn't a great idea performance-wise, but it shouldn't lead to incorrect or dangerous behaviour.

Here it seems to trigger a silent downcast of the input longs, giving a truncated integer output:

user> (defn f [a b] (Math/abs (- a b)))
Reflection warning, NO_SOURCE_PATH:1:15 - call to abs can't be resolved.
#'user/f
user> (f 1000000000000 2000000000000)
727379968
user> (class (f 1000000000000 2000000000000))
java.lang.Integer
user> (defn f [^long a ^long b] (Math/abs (- a b)))
#'user/f
user> (f 1000000000000 2000000000000)
1000000000000
user> (class (f 1000000000000 2000000000000))
java.lang.Long



 Comments   
Comment by Matthew Willson [ 28/May/13 12:50 PM ]

For an even simpler way to replicate the issue:

user> (#(Math/abs %) 1000000000000)
Reflection warning, NO_SOURCE_PATH:1:3 - call to abs can't be resolved.
727379968

Comment by Andy Fingerhut [ 28/May/13 1:36 PM ]

I was able to reproduce the behavior you see with these Java 6 JVMs on Ubuntu 12.04.2:

java version "1.6.0_27"
OpenJDK Runtime Environment (IcedTea6 1.12.5) (6b27-1.12.5-0ubuntu0.12.04.1)
OpenJDK 64-Bit Server VM (build 20.0-b12, mixed mode)

java version "1.6.0_45"
Java(TM) SE Runtime Environment (build 1.6.0_45-b06)
Java HotSpot(TM) 64-Bit Server VM (build 20.45-b01, mixed mode)

However, I tried two Java 7 JVMs, and it gave the following behavior which looks closer to what you would hope for. I do not know what is the precise difference between Java 6 and Java 7 that leads to this behavior difference, but this is some evidence that this has something to do with Java 6 vs. Java 7.

user=> (set! warn-on-reflection true)
true
user=> (defn f [a b] (Math/abs (- a b)))
Reflection warning, NO_SOURCE_PATH:1:15 - call to abs can't be resolved.
#'user/f
user=> (f 1000000000000 2000000000000)
1000000000000
user=> (class (f 1000000000000 2000000000000))
java.lang.Long

Above behavior observed with Clojure 1.5.1 on these JVMs:

Ubuntu 12.04.2 plus this JVM:
java version "1.7.0_21"
Java(TM) SE Runtime Environment (build 1.7.0_21-b11)
Java HotSpot(TM) 64-Bit Server VM (build 23.21-b01, mixed mode)

Mac OS X 10.8.3 plus this JVM:
java version "1.7.0_15"
Java(TM) SE Runtime Environment (build 1.7.0_15-b03)
Java HotSpot(TM) 64-Bit Server VM (build 23.7-b01, mixed mode)

Comment by Matthew Willson [ 29/May/13 5:17 AM ]

Ah, interesting.
Maybe it's a difference in the way the reflection API works in java 7?

Here's the bytecode generated incase anyone's curious:

public java.lang.Object invoke(java.lang.Object);
Code:
0: ldc #14; //String java.lang.Math
2: invokestatic #20; //Method java/lang/Class.forName:(Ljava/lang/String;)Ljava/lang/Class;
5: ldc #22; //String abs
7: iconst_1
8: anewarray #24; //class java/lang/Object
11: dup
12: iconst_0
13: aload_1
14: aconst_null
15: astore_1
16: aastore
17: invokestatic #30; //Method clojure/lang/Reflector.invokeStaticMethod:(Ljava/lang/Class;Ljava/lang/String;[Ljava/lang/Object;)Ljava/lang/Object;
20: areturn

Comment by Matthew Willson [ 29/May/13 5:20 AM ]

Just an idea (and maybe this is what's happening under java 7?) but given it's a static method and all available overloaded variants are presumably known at compile time, perhaps it could generate code along the lines of:

(cond
(instance? Long x) (Math/abs (long x))
(instance? Integer x) (Math/abs (int x))
;; ...
)

Comment by Andy Fingerhut [ 29/May/13 3:19 PM ]

In Reflector.java method invokeStaticMethod(Class c, String methodName, Object[] args) there is a call to getMethods() followed by a call to invokeMatchingMethod(). getMethods() returns the 4 java.lang.Math/abs methods in different orders on Java 6 and 7, causing invokeMatchingMethod() to pick a different one on the two JVMs:

java version "1.6.0_39"
Java(TM) SE Runtime Environment (build 1.6.0_39-b04)
Java HotSpot(TM) 64-Bit Server VM (build 20.14-b01, mixed mode)

user=> (pprint (seq (clojure.lang.Reflector/getMethods java.lang.Math 1 "abs" true)))
(#<Method public static int java.lang.Math.abs(int)>
#<Method public static long java.lang.Math.abs(long)>
#<Method public static float java.lang.Math.abs(float)>
#<Method public static double java.lang.Math.abs(double)>)
nil

java version "1.7.0_21"
Java(TM) SE Runtime Environment (build 1.7.0_21-b11)
Java HotSpot(TM) 64-Bit Server VM (build 23.21-b01, mixed mode)

user=> (pprint (seq (clojure.lang.Reflector/getMethods java.lang.Math 1 "abs" true)))
(#<Method public static double java.lang.Math.abs(double)>
#<Method public static float java.lang.Math.abs(float)>
#<Method public static long java.lang.Math.abs(long)>
#<Method public static int java.lang.Math.abs(int)>)
nil

That might be a sign of undesirable behavior in invokeMatchingMethod() that is too dependent upon the order of methods given to it.

As you mention, type hinting is good for avoiding the significant performance hit of reflection.





[CLJ-1210] error message for (clojure.java.io/reader nil) — consistency for use with io/resource Created: 23/May/13  Updated: 15/Feb/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Trevor Wennblom Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: errormsgs, io

Attachments: File extend-io-factory-to-nil.diff    
Patch: Code and Test

 Description   

This seems to a common idiom:

(clojure.java.io/reader (clojure.java.io/resource "myfile"))

When a file is available these are the behaviors:

=> (clojure.java.io/reader "resources/myfile")
#<BufferedReader java.io.BufferedReader@1f291df0>

=> (clojure.java.io/resource "myfile")
#<URL file:/project/resources/myfile>

=> (clojure.java.io/reader (clojure.java.io/resource "myfile"))
#<BufferedReader java.io.BufferedReader@1db04f7c>

If the file (resource) is unavailable:

=> (clojure.java.io/reader "resources/nofile")
FileNotFoundException resources/nofile (No such file or directory) java.io.FileInputStream.open (FileInputStream.java:-2)

=> (clojure.java.io/resource "nofile")
nil

=> (clojure.java.io/reader (clojure.java.io/resource "nofile"))
IllegalArgumentException No implementation of method: :make-reader of protocol: #'clojure.java.io/IOFactory found for class: nil clojure.core/-cache-protocol-fn (core_deftype.clj:541)

The main enhancement request is to have a better error message from `(clojure.java.io/reader nil)`. I'm not sure if io/resource should return something like 'resource "nofile" not found' or if io/reader could add a more helpful suggestion.



 Comments   
Comment by Alexander Redington [ 14/Feb/14 3:13 PM ]

This patch extends IOFactory to nil, providing error messages consistent with the default error messages provided for Object.

Comment by Benjamin Peter [ 15/Feb/14 1:31 PM ]

Looks like a good solution to me as a user. Thanks for the effort!





[CLJ-1208] Namespace is not loaded on defrecord class init Created: 03/May/13  Updated: 03/May/13

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

Type: Defect Priority: Major
Reporter: Tim McCormack Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None


 Description   

As a user of Clojure interop from Java, I want defrecords (and deftypes?) to load their namespaces upon class initialization so that I can simply construct and use AOT'd record classes without manually requiring their namespaces first.

Calling the defrecord's constructor may or may not result in "Attempting to call unbound fn" exceptions, depending on what code has already been run.

This issue has been raised several times over the years, but I could not find an existing ticket for it:






[CLJ-1207] Importing a class that does not exist fails to report the name of the class that did not exist Created: 29/Apr/13  Updated: 05/Feb/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Howard Lewis Ship Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: errormsgs
Environment:

1.5.1, OS X


Waiting On: Howard Lewis Ship

 Description   

Pop quiz: What Java class is missing from the classpath?

java.lang.NoClassDefFoundError: Could not initialize class com.annadaletech.nexus.util.logging__init
 at java.lang.Class.forName0 (Class.java:-2)
    java.lang.Class.forName (Class.java:264)
    clojure.lang.RT.loadClassForName (RT.java:2098)
    clojure.lang.RT.load (RT.java:430)
    clojure.lang.RT.load (RT.java:411)
    clojure.core$load$fn__5018.invoke (core.clj:5530)
    clojure.core$load.doInvoke (core.clj:5529)
    clojure.lang.RestFn.invoke (RestFn.java:408)
    clojure.core$load_one.invoke (core.clj:5336)
    clojure.core$load_lib$fn__4967.invoke (core.clj:5375)
    clojure.core$load_lib.doInvoke (core.clj:5374)
    clojure.lang.RestFn.applyTo (RestFn.java:142)
    clojure.core$apply.invoke (core.clj:619)
    clojure.core$load_libs.doInvoke (core.clj:5413)
    clojure.lang.RestFn.applyTo (RestFn.java:137)
    clojure.core$apply.invoke (core.clj:619)
    clojure.core$require.doInvoke (core.clj:5496)
    clojure.lang.RestFn.invoke (RestFn.java:512)
    novate.console.app$eval1736$loading__4910__auto____1737.invoke (app.clj:1)
    novate.console.app$eval1736.invoke (app.clj:1)
    clojure.lang.Compiler.eval (Compiler.java:6619)
    clojure.lang.Compiler.eval (Compiler.java:6608)
    clojure.lang.Compiler.load (Compiler.java:7064)
    user$eval1732.invoke (NO_SOURCE_FILE:1)
    clojure.lang.Compiler.eval (Compiler.java:6619)
    clojure.lang.Compiler.eval (Compiler.java:6582)
    clojure.core$eval.invoke (core.clj:2852)
    clojure.main$repl$read_eval_print__6588$fn__6591.invoke (main.clj:259)
    clojure.main$repl$read_eval_print__6588.invoke (main.clj:259)
    clojure.main$repl$fn__6597.invoke (main.clj:277)
    clojure.main$repl.doInvoke (main.clj:277)
    clojure.lang.RestFn.invoke (RestFn.java:1096)
    clojure.tools.nrepl.middleware.interruptible_eval$evaluate$fn__584.invoke (interruptible_eval.clj:56)
    clojure.lang.AFn.applyToHelper (AFn.java:159)
    clojure.lang.AFn.applyTo (AFn.java:151)
    clojure.core$apply.invoke (core.clj:617)
    clojure.core$with_bindings_STAR_.doInvoke (core.clj:1788)
    clojure.lang.RestFn.invoke (RestFn.java:425)
    clojure.tools.nrepl.middleware.interruptible_eval$evaluate.invoke (interruptible_eval.clj:41)
    clojure.tools.nrepl.middleware.interruptible_eval$interruptible_eval$fn__625$fn__628.invoke (interruptible_eval.clj:171)
    clojure.core$comp$fn__4154.invoke (core.clj:2330)
    clojure.tools.nrepl.middleware.interruptible_eval$run_next$fn__618.invoke (interruptible_eval.clj:138)
    clojure.lang.AFn.run (AFn.java:24)
    java.util.concurrent.ThreadPoolExecutor.runWorker (ThreadPoolExecutor.java:1110)
    java.util.concurrent.ThreadPoolExecutor$Worker.run (ThreadPoolExecutor.java:603)
    java.lang.Thread.run (Thread.java:722)

If you guess "com.annadaletech.nexus.util.logging__init" you are wrong!

Wait, I'll give you a hint:

(ns com.annadaletech.nexus.util.logging
  (:use [clojure.string :only [trim-newline]]
        [clojure.pprint :only [code-dispatch pprint with-pprint-dispatch *print-right-margin*]])
  (:import [java.io StringWriter]
           [org.slf4j MDC MarkerFactory Marker LoggerFactory]
           [java.util.concurrent.locks ReentrantLock]))

Oh, sorry, did that not help?

The correct answer is "org.slf4j.MDC".

Having that information in the stack trace would have saved me nearly an hour. I think it is worth the effort to get that reported correctly.



 Comments   
Comment by Gabriel Horner [ 10/May/13 1:56 PM ]

When I try this on a fresh project, I get this error:
"ClassNotFoundException org.slf4j.MDC
java.net.URLClassLoader$1.run (URLClassLoader.java:202)
java.security.AccessController.doPrivileged (AccessController.java:-2)"

Howard, could you give us a project.clj or better yet a github repository that recreates this issue?

Comment by Howard Lewis Ship [ 10/May/13 4:51 PM ]

I'll see what I can do. Probably be next week. Thanks for looking at this.

Comment by Gary Fredericks [ 26/May/13 8:20 AM ]

This reminds me of an issue with `lein run` that resulted from it trying to figure out whether you wanted to run a namespace or a java class:

https://github.com/technomancy/leiningen/issues/1182





[CLJ-1206] 'eval' of closures or fns with runtime metadata within a call expr yields "No matching ctor found" exceptions Created: 28/Apr/13  Updated: 28/Apr/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Jason Wolfe Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

I ran into some issues with 'eval' when writing compilation strategies for Graph. It seems these may have been known for some time [1], but I couldn't find a ticket for them, so here we are.

Clojure docs [2] say "If the operator is not a special form or macro, the call is considered a function call. Both the operator and the operands (if any) are evaluated, from left to right," and "Any object other than those discussed above will evaluate to itself." While bare fns do seem to evaluate to themselves in all cases, when in a call expression, the evaluation of the operator fails on fn objects that are closures or have run-time metadata applied:

;; raw non-closures are fine
user> (eval (fn [x] (inc x)))
#<user$eval30559$fn_30560 user$eval30559$fn_30560@354ee11c>

;; raw closures are fine
user> (eval (let [y 1] (fn [x] (+ x y))))
#<user$eval30511$fn_30512 user$eval30511$fn_30512@3bac3a34>

;; non-closures in exprs are fine
user> (eval `(~(fn [x] (inc x)) 1))
2

;; but closures in exprs cause an error
user> (eval `(~(let [y 1] (fn [x] (+ x y))) 1))
IllegalArgumentException No matching ctor found for class user$eval30535$fn__30536 clojure.lang.Reflector.invokeConstructor (Reflector.java:163)

;; as do fns with metadata in exprs
user> (eval `(~(with-meta (fn [x] (inc x)) {:x 1}) 1))
IllegalArgumentException No matching ctor found for class clojure.lang.AFunction$1 clojure.lang.Reflector.invokeConstructor (Reflector.java:163)

[1] http://stackoverflow.com/a/11287181
[2] http://clojure.org/evaluation






[CLJ-1201] There should also be writing in clojure.edn Created: 15/Apr/13  Updated: 23/May/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Vitaly Shukela Assignee: Unassigned
Resolution: Unresolved Votes: 3
Labels: edn


 Description   

In clojure.edn I see only "read" and "read-string".

For symmetry I expect "write" and "write-string" to be nearby. At first it could be just alias for "pr" and "pr-str", but in furure they may limited version of "pr" which only produces valid input for clojure.edn/read.



 Comments   
Comment by Andy Fingerhut [ 23/May/13 5:56 PM ]

Related clojure-dev message: https://groups.google.com/forum/?fromgroups#!topic/clojure-dev/fLJWh9A3OuA

and enhancement proposal wiki page: http://dev.clojure.org/display/design/Representing+EDN





[CLJ-1199] Record values are not 'eval'uated, unlike values of PersistentMap: Created: 13/Apr/13  Updated: 13/Apr/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Jason Wolfe Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

I'm not sure if this is by design, but it caught me off guard.

user> (defrecord A [x])
user.A

user> (eval (hash-map :x `long))
{:x #<core$long clojure.core$long@5de54eb7>}
user> (eval (->A `long))
#user.A{:x clojure.core/long}
user> (eval (map->A (hash-map :x `long)))
#user.A{:x clojure.core/long}

and in case it matters, here's a simplified version of the real use case where this came up, with no eval – just a macro:

user> (defmacro munge-meta1 [x] (assoc x :schema (->A (:schema (meta x)))))
#'user/munge-meta1
user> (munge-meta1 ^{:schema long} {})
{:schema #user.A{:x long}}

user> (defmacro munge-meta2 [x] (assoc x :schema (hash-map :x (:schema (meta x)))))
#'user/munge-meta2
user> (munge-meta2 ^{:schema long} {})
{:schema {:x #<core$long clojure.core$long@5de54eb7>}}

This seems to be fixed by moving the record creation post-evaluation, so it's not a big deal, just surprising (plus I haven't yet convinced myself that this will always work if the user's schema itself contains record-creating forms, although it seems to work OK):

user> (defmacro munge-meta1 [x] (assoc x :schema `(->A ~(:schema (meta x)))))
#'user/munge-meta1
user> (munge-meta1 ^{:schema long} {})
{:schema #user.A{:x #<core$long clojure.core$long@5de54eb7>}}

I brought this up on the mailing list here:

https://groups.google.com/forum/?fromgroups=#!topic/clojure-dev/UgD35E1RQTo






[CLJ-1198] Apply metadata to primitive fns causes them to lose their primitive-ness Created: 13/Apr/13  Updated: 13/Apr/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Jason Wolfe Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

user> (def f (fn [^long x] x))
#'user/f
user> (.invokePrim (with-meta f {}) 1)
IllegalArgumentException No matching method found: invokePrim for class clojure.lang.AFunction$1 clojure.lang.Reflector.invokeMatchingMethod (Reflector.java:53)
user> (contains? (ancestors (class f)) clojure.lang.IFn$LO)
true
user> (contains? (ancestors (class (with-meta f {}))) clojure.lang.IFn$LO)
false

We're working on libraries that use metadata on functions to track information about their arguments (schemata, etc), and this currently blocks us from fully supporting primitive fns.






[CLJ-1195] emit-hinted-impl expands to non-ns-qualified invocation of 'fn' Created: 09/Apr/13  Updated: 09/Apr/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Jason Wolfe Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Mac os X Clojure 1.5.1



 Description   

(ns plumbing.tmp
(:refer-clojure :exclude [fn]))

(defprotocol Foo
(foo [this]))

(extend-protocol Foo
Object
(foo [this]))

yields

CompilerException java.lang.RuntimeException: Unable to resolve symbol: fn in this context, compiling/Users/w01fe/prismatic/prismatic/plumbing/src/plumbing/tmp.clj:7:1)

This makes it difficult to construct a namespace that provides a replacement def for fn.






[CLJ-1189] Map-destructuring :or fumble needs compiler warning Created: 31/Mar/13  Updated: 03/Sep/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Phill Wolf Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: errormsgs

Attachments: Text File CLJ-1189-p1.patch    
Patch: Code and Test

 Description   

Here is a map-destructuring blunder that I wish the compiler warned about:

(defn server
[{servlet ::servlet
type ::type
:or {::type :jetty}
:as service-map}]

It would be splendid to get a warning that :or keys that are not symbols being bound have no effect.

The incomplete code snippet above comes from Pedestal.service 0.1.0.

Here is a complete one-line example with the coding error:

user> (defn picnic [{botulism :botulism :or {:botulism 6}}] botulism)
#'user/picnic
user> (picnic {})
nil
user> ;; I intended 6.



 Comments   
Comment by Gary Fredericks [ 26/May/13 8:25 AM ]

Should this be a warning or an exception? I don't know of any similar example of the compiler giving a warning, so I would expect the latter.

Comment by Gary Fredericks [ 26/May/13 9:54 AM ]

Added a patch that throws an exception when :or is not a map or its keys are not symbols. Also some tests.





[CLJ-1181] clojure.pprint/code-dispatch breaks on certain types of anonymous functions Created: 10/Mar/13  Updated: 05/Feb/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Devin Walters Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: print


 Description   
(with-out-str 
  (with-pprint-dispatch code-dispatch 
                        (pp/pprint (read-string "(fn* [x] x)"))))

breaks because the format string here: https://github.com/clojure/clojure/blob/master/src/clj/clojure/pprint/dispatch.clj#L378 expects a sequence. In the case of (fn* [x] x) it is passed a symbol.



 Comments   
Comment by Jean Niklas L'orange [ 18/Mar/13 5:40 PM ]

I think the main "issue" here resides within the undocumented functionality of fn*. (fn* [x] x) is a semantically working function, but (fn [x] x) expands into (fn* ([x] x)). Anonymous function literals expand into (fn* [gensyms] (...)), and as such, it also accepts expressions like (fn* [x] x). Should pprint pretty print expressions which has used fn* directly, or should it "just" ignore it?





[CLJ-1180] defprotocol doesn't resolve tag classnames Created: 10/Mar/13  Updated: 05/Feb/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5, Release 1.6
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Nicola Mometto Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: protocols

Attachments: Text File 001-CLJ-1180.patch    
Patch: Code and Test

 Description   

defprotocol doesn't resolve tag classnames, this results in exceptions being thrown when the declared protocol uses as a tag an imported class that is not imported in the namespace that uses it.

user=> (import 'clojure.lang.ISeq)
clojure.lang.ISeq
user=> (defprotocol p (^ISeq f [_]))
p
user=> (ns x)
nil
x=> (defn x [y] (let [z (user/f y)] (inc z)))
CompilerException java.lang.IllegalArgumentException: Unable to resolve classname: ISeq, compiling:(NO_SOURCE_PATH:4:33)



 Comments   
Comment by Alex Miller [ 03/Sep/13 9:38 AM ]

Similer to CLJ-1232.





[CLJ-1172] Cross-linking between clojure.lang.Compiler and clojure.lang.RT Created: 28/Feb/13  Updated: 21/Jun/13

Status: Reopened
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Yegor Bugayenko Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

version 1.5.0-RC17



 Description   

This is my code (an example):

import clojure.lang.Compiler;
import clojure.lang.RT;
import clojure.lang.Var;

Compiler.load("(+ 5 %)");
Var foo = RT.var("bar", "foo");
Object result = foo.invoke(10);
assert result.toString().equals("15");

This is what I'm getting:

ava.lang.ExceptionInInitializerError
	at clojure.lang.Compiler.<clinit>(Compiler.java:47)
	at foo.main(Main.java:75)
Caused by: java.lang.NullPointerException
	at clojure.lang.RT.baseLoader(RT.java:2043)
	at clojure.lang.RT.load(RT.java:417)
	at clojure.lang.RT.load(RT.java:411)
	at clojure.lang.RT.doInit(RT.java:447)
	at clojure.lang.RT.<clinit>(RT.java:329)
	... 36 more

The same code worked just fine with version 1.4. Looks like Compiler is using RT and RT is using Compiler, both statically.



 Comments   
Comment by Yegor Bugayenko [ 04/Mar/13 11:40 AM ]

I cross-posted this question to SO: http://stackoverflow.com/questions/15207596

Comment by Yegor Bugayenko [ 05/Mar/13 12:04 AM ]

calling RT.init() before Compiler.load() solves the problem

Comment by Andy Fingerhut [ 05/Mar/13 4:17 AM ]

Yegor, do you consider it OK to close this ticket as not being a problem, or at least one with a reasonable workaround?

Comment by Yegor Bugayenko [ 05/Mar/13 1:11 PM ]

Yes, of course. Let's close it.

Comment by Andy Fingerhut [ 05/Mar/13 6:14 PM ]

Ticket submitter agrees that this is not an issue, or that there is a reasonable workaround.

Comment by Andy Fingerhut [ 13/Mar/13 12:58 AM ]

This issue came up again on the Clojure group. https://groups.google.com/forum/?hl=en_US&fromgroups=#!topic/clojure/2xdLNMb9yyQ

I did some testing, and the issue did not exist in Clojure 1.5.0-RC3 and before, and it has existed since 1.5.0-RC4. There was only one commit between those two points:

https://github.com/clojure/clojure/commit/9b80a552fdabeabdd93951a625b55ae49c2f8d83

Maybe this new behavior is an intended consequence of that change. I don't know. In any case, it seems like perhaps the "No need to call RT.init() anymore" message might be outdated?

Comment by Andy Fingerhut [ 13/Mar/13 12:59 AM ]

Reopening since it came up again, and there is some more info known about the issue. I'll let someone who knows more about the issue decide whether to close it.

Comment by Edward [ 23/Mar/13 10:31 AM ]

Doing this RT.load("clojure/core"); at the top works avoids the message from RT.init()

Comment by Jean Niklas L'orange [ 21/Jun/13 10:36 AM ]

It seems like RT.load("clojure/core") does not hide the message anymore - at least not from 1.5.1.





[CLJ-1167] repl value of *file* is "NO_SOURCE_PATH", of *source-path* is "NO_SOURCE_FILE" Created: 19/Feb/13  Updated: 03/Sep/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Defect Priority: Trivial
Reporter: Brian Marick Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: repl


 Comments   
Comment by Brian Marick [ 19/Feb/13 4:22 PM ]

Forgot to mention: I think it's intended to be the other way around, given the names.





[CLJ-1162] Error Message when calling deref on a non-IDeref is unhelpful Created: 12/Feb/13  Updated: 03/Sep/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Julian Birch Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: errormsgs
Environment:

Found this on ubuntu, lein 2, Clojure 1.5 snapshot, but it's obvious from inspection


Attachments: Text File CLJ-1162-p1.patch    
Patch: Code

 Description   

If you just type "@1" is the repl, on previous versions you'll get an error that Long cannot be cast to IDeref. In 1.5, the error message is that it cannot be cast to java.util.concurrent.Future. This is because it assumes that anything that isn't an IDeref is automatically a Future. The deref method should generate a custom error stating that the type you've passed in is neither an IDeref nor a Future.



 Comments   
Comment by Gary Fredericks [ 26/May/13 3:00 PM ]

Attached a patch that implements the old behavior (can't cast to IDeref), which strikes me as good enough considering the support for j.u.c.Future seems rather an edge case (being that clojure's futures are themselves IDeref).

The weirdest thing I did was to use clojure.core/cast to unconditionally throw a ClassCastException. Let me know if that's weird and I'll do something different.





[CLJ-1159] clojure.java.io/delete-file doesn't return the status of the deletion(true/false) Created: 10/Feb/13  Updated: 03/Sep/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: AtKaaZ Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: errormsgs, io
Environment:

any



 Description   

initially reported it here(somehow):
https://groups.google.com/d/msg/clojure/T9Kvr0IL0kg/wcKBfR9w_1sJ

Basically clojure.java.io/delete-file doesn't ever return false (even when silently is true, it returns the value of silently), it's due to how it's implemented - but it's obvious from the code, so I'll stop here.

Thanks.

PS: this is what I'm using as my current workaround:
(defn delete-file
"
an implementation that returns the true/false status
which clojure.java.io/delete-file doesn't do(tested in 1.5.0-RC14)
"
[f & [silently]]
(let [ret (.delete (clojure.java.io/file f))]
(cond (or ret silently)
ret
:else
(throw (java.io.IOException. (str "Couldn't delete " f)))
)
)
)

I'm sure you guys can find a better way, but as a clojure newbie(really!) that's what I have.



 Comments   
Comment by AtKaaZ [ 10/Feb/13 2:01 PM ]

I kinda just realized it affects all versions since and including 1.2, because it appears that its implementation was the same since then.

If it's not meant to return the result of the delete, maybe it should specifically return nil and/or the doc say something?

Comment by Sean Corfield [ 10/Feb/13 2:21 PM ]

As noted in a thread on the Clojure ML, you can pass a known value in the second argument position to detect a delete that failed:

(clojure.java.io/delete-file some-file :not-deleted)

This returns true on success and :not-deleted on failure.

However the docstring could be better worded to make that intention clear. Perhaps:

Delete file f. Return true if it succeeds. If silently is nil or false, raise an exception if it fails, else return the value of silently.
This allows you to detect whether the delete succeeded without catching an exception by passing a non-true truthy value as the second argument.





[CLJ-1151] Minor Code Cleanup in core.reducers: use required walk, drop this for coll Created: 21/Jan/13  Updated: 03/Sep/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Enhancement Priority: Trivial
Reporter: Stefan Kamphausen Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: File tiny-reducers-cleanup.diff    
Patch: Code

 Description   

First, core.reducers requires clojure.walk :as walk, but does not use the alias.
Second, the two arity implementation of coll-reduce in function reducer uses 'this', whereas similar implementations in that file use 'coll'. AFAICT it makes no difference to use 'coll' (all tests pass, no change in performance) and it is more in line with the rest of the code.

The two things seem small enough to be put into one cleanup case.



 Comments   
Comment by Jozef Wagner [ 03/Sep/13 6:47 AM ]

your patch is wrong. If you want to replace this with coll, you have to allso call xf on f1.

(clojure.core.protocols/coll-reduce this f1 (f1))

becomes

(clojure.core.protocols/coll-reduce coll (xf f1) (f1))
Comment by Stefan Kamphausen [ 03/Sep/13 2:45 PM ]

Scary, that the test suite did not detect that.

Comment by Andy Fingerhut [ 03/Sep/13 2:59 PM ]

Is it straightforward to add a test that would have detected that?

Comment by Stefan Kamphausen [ 03/Sep/13 4:09 PM ]

I will happily look into that. It may take a few days, before I'll find the time, though.





[CLJ-1147] Threading macro (->) does not permit inline function declarations Created: 14/Jan/13  Updated: 26/May/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Stephen Nelson Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

(-> [1 2 3] (fn [args] apply + args))

CompilerException java.lang.Exception: Unsupported binding form: 1, compiling:(NO_SOURCE_PATH:1:13)

The expression is expanded to:

(fn [1 2 3] [args] apply + args)

If this is intended behaviour then at the least the compiler error message is confusing. It would be preferable if the -> macro checked for (fn..) before treating a form as a sequence and injecting the argument.



 Comments   
Comment by Andy Fingerhut [ 15/Jan/13 12:56 AM ]

Note that this works as you might have hoped:

(-> [1 2 3] ((fn [args] (apply + args))))

because it expands into:

((fn [args] (apply + args)) [1 2 3])

Your suggestion that -> check for (fn ...) before treating it as a sequence and injecting the argument leaves open the question: Why only (fn ...) should be treated specially? Why not (let ...), (for ...), (doseq ...), etc? And if you go that far, how do you decide what should be allowed and what not?

Comment by Gabriel Horner [ 17/May/13 2:56 PM ]

I agree with Andy, that it's not realistic suggestion to check for fn,let,etc. Perhaps a doc fix would help here but I'm not sure if we just want to call out (fn ...). I'd recommend closing this unless Stephen speaks up.

Comment by Stephen Nelson [ 19/May/13 10:29 PM ]

I'm happy with Andy's synopsis of the problem, and it's reasonable not to change the behaviour of the threading macro specifically for (fn..).

However, this is a mistake that I'm sure many others make/have made and it's hard to diagnose what is going wrong without dumping interpreted form – hardly a reasonable expectation for a novice user.

Before closing this issue, I'd like to see improved failure reporting, such as causing the threading macro to throw a compile error or warning if passed a raw (unwrapped) function declaration (are there legitimate use cases this would affect?).

Comment by Gary Fredericks [ 26/May/13 2:21 PM ]

Throwing an error on (-> [1 2 3] (fn ...)) would certainly affect any perverse individual using a local redefinition of 'fn.

I think the best that can be done here is a mention in the docstring.





[CLJ-1146] Symbol name starting with digits to defn throws "Unmatched delimiter )" Created: 13/Jan/13  Updated: 18/Apr/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Enhancement Priority: Trivial
Reporter: Linus Ericsson Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: errormsgs, reader
Environment:

$java -jar clojure-1.5.0-RC2.jar

$java -version
java version "1.6.0_37"
Java(TM) SE Runtime Environment (build 1.6.0_37-b06-434-10M3909)
Java HotSpot(TM) 64-Bit Server VM (build 20.12-b01-434, mixed mode)
Mac OS X:
System Version: Mac OS X 10.6.8 (10K549)
Kernel Version: Darwin 10.8.0



 Description   

When trying to use an invalid symbol name when defining a function, the error message thrown is a confusing and wrong one. The error message is "RuntimeException Unmatched delimiter: ) clojure.lang.Util.runtimeException (Util.java:219)", which unfortunately is the only message seen in nrepled emacs.

$ java -jar clojure-1.5.0-RC2.jar
Clojure 1.5.0-RC2
user=> (defn 45fn [] nil)
NumberFormatException Invalid number: 45fn clojure.lang.LispReader.readNumber (LispReader.java:255)
[]
nil
RuntimeException Unmatched delimiter: ) clojure.lang.Util.runtimeException (Util.java:219)

Expected:
When trying to (defn or (def a thing with a non valid symbol name, the last thrown error message should be one stating that the given symbol name is not a valid one.



 Comments   
Comment by Kevin Downey [ 18/Apr/14 2:27 AM ]

this is an artifact of how streams and repls work.

when you type (defn 45fn [] nil) and hit enter, the inputstream flushes and "(defn 45fn [] nil)" is made available to the reader, the reader reads up to 45fn, throws an error back to the main repl loop, which prints out the error, then calls read, which still has the unread parts available to it "[] nil)"

changing this behavior would require significant changes to clojure's repl.

checkout https://github.com/trptcolin/reply instead





[CLJ-1142] Incorrect divide-by-zero error with floating point numbers Created: 08/Jan/13  Updated: 05/Feb/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.4
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Tim McCormack Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: math


 Description   

The unary call for clojure.core// treats a dividend of 0.0 differently than the binary call, likely due to inlining.

(/ 0.0) ;; java.lang.ArithmeticException: Divide by zero
(/ 1 0.0) ;;= Infinity
(/ 1 (identity 0.0)) ;; java.lang.ArithmeticException: Divide by zero


 Comments   
Comment by Tim McCormack [ 08/Jan/13 11:22 PM ]

The relevant code seems to be this in clojure.lang.Numbers/divide:

if(yops.isZero((Number)y))
  throw new ArithmeticException("Divide by zero");

Making Numbers/divide be more restrictive than double arithmetic seems like a bug; explicitly throwing an ArithmeticException instead of letting the JVM figure it just seems like more work than necessary.





[CLJ-1141] Allow pre and post-conditions in defprotocol and deftype macros Created: 02/Jan/13  Updated: 04/Sep/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.4
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Alexander Kiel Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: deftype, protocols
Environment:

Dos not matter.



 Description   

The fn special form and the defn macro allow pre- and post-conditions. It would be nice if one could use that conditions also in method declarations of the defprotocol and deftype macro.

Currently I use the extend function as workaround where one can specify the methods using a map of keyword-name and fn special form.



 Comments   
Comment by Michael Drogalis [ 06/Jan/13 6:22 PM ]

Using :pre and :post, IMO, isn't a good idea. Handling assertions is a two part game. The mechanism needs to account for both detection and reaction, and the latter is missing.

This isn't a perfect work-around, as it's a little verbose, but using Dire might work better than using extend. In addition, you get the "reaction" functionality that's missing from :pre and :post

Example for protocol preconditions: https://gist.github.com/4471276

Comment by Alexander Kiel [ 07/Jan/13 11:52 AM ]

@Michael I read your gist and the README of Dire. I think the supervision concept of Erlang has it's places but I don't like it for pre- and post-conditions. For me, such conditions have two proposes:

  1. they should document the code and
  2. they should fail fast to detect failures early.

To support my first point, your pre- and post-conditions are just lexical too far away from the actual function definition. For the second point: I think in the case of violations the program should just crash. One could maybe wrap some part of the program with one of your exception supervisors handling an AssertionError. But I don't think that handling pre- and post-condition violations for individual functions is a good thing.

Comment by Michael Drogalis [ 07/Jan/13 5:28 PM ]

@Alexander Indeed, your points are correct. Dire is meant to be exactly what you described. Lexically removed from application logic, and opportunity to recover from crashing. That was my best shot at aiding your needs quickly, anyway.





[CLJ-1138] data-reader returning nil causes exception Created: 22/Dec/12  Updated: 14/Feb/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.4, Release 1.5
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Steve Miner Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: reader
Environment:

clojure 1.5 beta2, Mac OS X 10.8.2, java version "1.6.0_37"



 Description   

If a data-reader returns nil, the reader throws java.lang.RuntimeException: No dispatch macro... The error message implies that there is no dispatch macro for whatever the first character of the tag happens to be.

Here's a simple example:

(binding [*data-readers* {'f/ignore (constantly nil)}] (read-string "#f/ignore 42 10"))

RuntimeException No dispatch macro for: f clojure.lang.Util.runtimeException (Util.java:219)



 Comments   
Comment by Steve Miner [ 22/Dec/12 9:43 AM ]

clj-1138-allow-data-reader-to-return-nil-instead-of-throwing.patch allows a data-reader to return nil instead of throwing. Does sanity check that possible tag or record isJavaIdentifierStart(). Gives better error message for special characters that might actually be dispatch macros (rather than assuming it's a tagged literal).

Comment by Steve Miner [ 22/Dec/12 10:06 AM ]

clj-1138-data-reader-return-nil-for-no-op.patch allows a data-reader returning nil to be treated as a no-op by the reader (like #_). nil is not normally a useful value (actually it causes an exception in Clojure 1.4 through 1.5 beta2) for a data-reader to return. With this patch, one could get something like a conditional feature reader using data-readers.

Comment by Steve Miner [ 22/Dec/12 10:26 AM ]

clj-1138-allow-data-reader-to-return-nil-instead-of-throwing.patch is the first patch to consider. It merely allows nil as a value from a data-reader and returns nil as the final value. I think it does what was originally intended for dispatch macros, and gives a better error message in many cases (mostly typos).

The second patch, clj-1138-data-reader-return-nil-for-no-op.patch, depends on the other being applied first. It takes an extra step to treat a nil value returned from a data-reader as a no-op for the reader (like #_).

Comment by Steve Miner [ 23/Dec/12 11:52 AM ]

It turns out that you can work around the original problem by having your data-reader return '(quote nil) instead of plain nil. That expression conveniently evaluates to nil so you can get a nil if necessary. This also works after applying the patches so there's still a way to return nil if you really want it.

(binding [*data-readers* {'x/nil (constantly '(quote nil))}] (read-string "#x/nil 42"))
;=> (quote nil)

Comment by Andy Fingerhut [ 07/Feb/13 9:20 AM ]

Patch clj-1138-allow-data-reader-to-return-nil-instead-of-throwing.patch dated Dec 22 2012 still applies cleanly to latest master if you use the following command:

% git am --keep-cr -s --ignore-whitespace < clj-1138-allow-data-reader-to-return-nil-instead-of-throwing.patch

Without the --ignore-whitespace option, the patch fails only because some whitespace was changed in Clojure master recently.

Comment by Andy Fingerhut [ 13/Feb/13 11:24 AM ]

OK, now with latest master (1.5.0-RC15 at this time), patch clj-1138-allow-data-reader-to-return-nil-instead-of-throwing.patch no longer applies cleanly, not even using --ignore-whitespace in the 'git am' command given above. Steve, if you could see what needs to be updated, that would be great. Using the patch command as suggested in the "Updating stale patches" section of http://dev.clojure.org/display/design/JIRA+workflow wasn't enough, so it should probably be carefully examined by hand to see what needs updating.

Comment by Steve Miner [ 14/Feb/13 12:21 PM ]

I removed my patches. Things have changes recently with the LispReader and new EdnReader.





[CLJ-1137] Metadata on a def gets evaluated twice Created: 21/Dec/12  Updated: 21/Dec/12

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

Type: Defect Priority: Minor
Reporter: Ghadi Shayban Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: File CLJ-1137-eval-metadata-once.diff    
Patch: Code

 Description   

Metadata on the symbol of a def special form is evaluated twice.

(def ^{:foo (println "HA")} a [])

prints out HA HA. Offending line is in Compiler$DefExpr, fixed.






[CLJ-1136] Type hinting for array classes does not work in binding forms Created: 20/Dec/12  Updated: 03/Sep/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.4, Release 1.5
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Luke VanderHart Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: interop, typehints
Environment:

replicated on OpenJDK 7u9 on Ubuntu 12.04, and Hotspot 1.6.0_37 on OSX Lion



 Description   

Type hints don't work as expected in binding forms.

The following form results in a reflection warning:

(let [^{:tag (Class/forName "[Ljava.lang.Object;")} a (make-array Object 2)]
(aget a 0))

However, hinting does appear to work correctly on vars:

(def ^{:tag (Class/forName "[Ljava.lang.Object;")} a (make-array Object 2))
(aget a 0) ;; no reflection warning



 Comments   
Comment by Ghadi Shayban [ 20/Dec/12 10:51 PM ]

It's a little more insidious than type hinting: the compiler doesn't evaluate metadata in the binding vec.

This doesn't throw the necessary exception...

(let [^{:foo (Class/forName "not real")} bar 42]
bar)

neither this...

(let [^{gyorgy ligeti} a 42]
a)

Gyorgy Ligeti never resolves.

These two equivalent examples don't reflect:
(let [^objects a (make-array Object 2)]
(aget a 0))

(let [a ^objects (make-array Object 2)]
(aget a 0))

Comment by Ghadi Shayban [ 21/Dec/12 11:09 AM ]

On only the left-hand side of a local binding, metadata on a symbol is not analyzed or evaluated.





[CLJ-1133] Certain actions on mutable fields in deftype lead to very strange error messages Created: 18/Dec/12  Updated: 03/Sep/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.4
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Vladimir Matveev Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: deftype
Environment:

Archlinux x86_64, Windows 7 x86_64



 Description   

Consider the following code:

(definterface Test
(^void fail []))

(deftype TestImpl
[^{:unsynchronized-mutable true :tag int} x]
Test
(fail [this]
(set! x (dec x))))

Its compilation fails with the following message:
CompilerException java.lang.VerifyError: (class: test/TestImpl, method: fail signature: ()V) Expecting to find integer on stack, compiling.../test.clj:27)

The following code works:

(definterface Test
(^void fail []))

(deftype TestImpl
[^{:unsynchronized-mutable true :tag int} x]
Test
(fail [this]
(set! x (int (dec x)))))

The only change here is that I have wrapped (dec x) form into (int) call.

I understand that in fact the former code should not work anyway (or at least should not work as I have expected) because (dec) is defined as a call to clojure.lang.Numbers.dec(), which is overloaded for double, long and Object only (in fact, changing :tag int to :tag long in the first example allows the program to compile). However, the error message is completely uninformative and misleading; it also looks like that it is a consequence of compiler error. It is also not a problem of this concrete example; I found this error in more complex interface method implementation where (set!) call was right in the middle of its body.

I'm using Clojure 1.4.0 and have experienced this problem on Archlinux x86_64 and Windows 7 x86_64.

Full stack trace of the error, in case it would be helpful:

java.lang.VerifyError: (class: test/TestImpl, method: fail signature: ()V) Expecting to find integer on stack, compiling.../test.clj:27)
at clojure.lang.Compiler.analyzeSeq(Compiler.java:6462)
at clojure.lang.Compiler.analyze(Compiler.java:6262)
at clojure.lang.Compiler.analyze(Compiler.java:6223)
at clojure.lang.Compiler$BodyExpr$Parser.parse(Compiler.java:5618)
at clojure.lang.Compiler$FnMethod.parse(Compiler.java:5054)
at clojure.lang.Compiler$FnExpr.parse(Compiler.java:3674)
at clojure.lang.Compiler.analyzeSeq(Compiler.java:6453)
at clojure.lang.Compiler.analyze(Compiler.java:6262)
at clojure.lang.Compiler.analyzeSeq(Compiler.java:6443)
at clojure.lang.Compiler.analyze(Compiler.java:6262)
at clojure.lang.Compiler.access$100(Compiler.java:37)
at clojure.lang.Compiler$DefExpr$Parser.parse(Compiler.java:518)
at clojure.lang.Compiler.analyzeSeq(Compiler.java:6455)
at clojure.lang.Compiler.analyze(Compiler.java:6262)
at clojure.lang.Compiler.analyzeSeq(Compiler.java:6443)
at clojure.lang.Compiler.analyze(Compiler.java:6262)
at clojure.lang.Compiler.analyze(Compiler.java:6223)
at clojure.lang.Compiler$BodyExpr$Parser.parse(Compiler.java:5618)
at clojure.lang.Compiler$LetExpr$Parser.parse(Compiler.java:5919)
at clojure.lang.Compiler.analyzeSeq(Compiler.java:6455)
at clojure.lang.Compiler.analyze(Compiler.java:6262)
at clojure.lang.Compiler.analyze(Compiler.java:6223)
at clojure.lang.Compiler$BodyExpr$Parser.parse(Compiler.java:5618)
at clojure.lang.Compiler$FnMethod.parse(Compiler.java:5054)
at clojure.lang.Compiler$FnExpr.parse(Compiler.java:3674)
at clojure.lang.Compiler.analyzeSeq(Compiler.java:6453)
at clojure.lang.Compiler.analyze(Compiler.java:6262)
at clojure.lang.Compiler.eval(Compiler.java:6508)
at clojure.lang.Compiler.load(Compiler.java:6952)
at clojure.lang.Compiler.loadFile(Compiler.java:6912)
at clojure.lang.RT$3.invoke(RT.java:307)
at test$eval3224.invoke(NO_SOURCE_FILE:43)
at clojure.lang.Compiler.eval(Compiler.java:6511)
at clojure.lang.Compiler.eval(Compiler.java:6477)
at clojure.core$eval.invoke(core.clj:2797)
at clojure.main$repl$read_eval_print__6405.invoke(main.clj:245)
at clojure.main$repl$fn__6410.invoke(main.clj:266)
at clojure.main$repl.doInvoke(main.clj:266)
at clojure.lang.RestFn.invoke(RestFn.java:421)
at clojure.main$repl_opt.invoke(main.clj:332)
at clojure.main$main.doInvoke(main.clj:428)
at clojure.lang.RestFn.invoke(RestFn.java:397)
at clojure.lang.Var.invoke(Var.java:411)
at clojure.lang.AFn.applyToHelper(AFn.java:159)
at clojure.lang.Var.applyTo(Var.java:532)
at clojure.main.main(main.java:37)
Caused by: java.lang.VerifyError: (class: test/TestImpl, method: fail signature: ()V) Expecting to find integer on stack
at java.lang.Class.forName0(Native Method)
at java.lang.Class.forName(Class.java:264)
at clojure.lang.RT.classForName(RT.java:2039)
at clojure.lang.Compiler$HostExpr.maybeClass(Compiler.java:957)
at clojure.lang.Compiler$HostExpr.access$400(Compiler.java:736)
at clojure.lang.Compiler$NewExpr$Parser.parse(Compiler.java:2473)
at clojure.lang.Compiler.analyzeSeq(Compiler.java:6455)
... 45 more



 Comments   
Comment by Vladimir Matveev [ 18/Dec/12 1:51 PM ]

Shouldn't have set major priority; but I cannot edit issue again

Comment by Andy Fingerhut [ 19/Dec/12 1:20 AM ]

Reduced priority to minor, since ticket creator could not do so themselves.





[CLJ-1132] For record type Rec, (instance? Rec (map->Rec {...})) need not return true, though (instance? Rec (Rec. ...)) does. Created: 18/Dec/12  Updated: 03/Sep/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.4
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Christopher Genovese Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: defrecord
Environment:

Apache Tomcat/6.0.24 JVM/1.6.0_26-b03 Linux 2.6.32-279.el6.x86_64

Clojure 1.4.0, Ring 1.1.6, Compojure 1.1.3, Lein-Ring Plugin 0.7.5 (for lein ring uberwar)


Attachments: File recordbug.tgz    

 Description   

(defrecord Rec ...)

(instance? Rec (Rec. ...)) ;=> true
(instance? Rec (map->Rec {...})) ;=> can be false

This occurs when the code is wrapped in a tomcat servlet by `lein ring
uberwar`, but not when run at the REPL or under Jetty, say.

Although produced under ring, this seems to me to be a general problem
with the map-> constructor, as (true? (instance? Rec (map->Rec {...})))
should be an invariant, regardless of the environment or context.
The problem seems to arise in some aspect of the class loading process:

(.getClassLoader Rec) ;=> WebappClassLoader (delegate: false, repositories: /WEB-INF/classes/, parent: org.apache.catalina.loader.StandardClassLoader@790bc49d)
(.getClassLoader (class (Rec. ...))) ;=> WebappClassLoader (same as the previous line)
(.getClassLoader (class (map->Rec ...))) ;=> clojure.lang.DynamicClassLoader@2e7227a8

The map->Rec delegates to the create method, which seems to be where the problem lies.

The record namespace is AOT compiled, properly I think/hope, and the requisite classes
imported as they should be.

I have attached a minimal web app that reproduces the problem and shows
the configuration. As a sanity check, I have also created a trivial
workaround defrecord* that creates a clojure-native map constructor,
for which the problem does not occur. See the README.md in the tar
file for usage details, and the project.clj for my configuration.

Again, I've only been able to reproduce the problem under Tomcat,
not under Jetty or at the REPL.






[CLJ-1131] Importing a non-existent class causes an exception that does not fully identify the source file Created: 17/Dec/12  Updated: 03/Sep/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.4
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Howard Lewis Ship Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: errormsgs


 Description   

I'm in the process of stripping out some OSGi support, and I missed one import.

The exception identifies "init.clj", but I'd prefer to see the full path there, as I have a few different "init.clj" files in my overall project.

:core-services:compileClojure
Reflection warning, com/annadaletech/nexus/services/registry.clj:37 - call to unregisterAll can't be resolved.
Reflection warning, com/annadaletech/nexus/services/registry.clj:131 - call to getConfiguration can't be resolved.
Reflection warning, com/annadaletech/nexus/services/registry.clj:150 - call to getConfiguration can't be resolved.
Exception in thread "main" java.lang.ClassNotFoundException: org.osgi.framework.ServiceRegistration, compiling:(init.clj:1)
	at clojure.lang.Compiler$InvokeExpr.eval(Compiler.java:3387)
	at clojure.lang.Compiler.compile1(Compiler.java:7035)
	at clojure.lang.Compiler.compile1(Compiler.java:7025)
	at clojure.lang.Compiler.compile(Compiler.java:7097)
	at clojure.lang.RT.compile(RT.java:387)
	at clojure.lang.RT.load(RT.java:427)
	at clojure.lang.RT.load(RT.java:400)
	at clojure.core$load$fn__4890.invoke(core.clj:5415)
	at clojure.core$load.doInvoke(core.clj:5414)
	at clojure.lang.RestFn.invoke(RestFn.java:408)
	at clojure.core$load_one.invoke(core.clj:5227)
	at clojure.core$compile$fn__4895.invoke(core.clj:5426)
	at clojure.core$compile.invoke(core.clj:5425)
	at clojuresque.tasks.compile$main$fn__64.invoke(compile.clj:23)
	at clojuresque.cli$with_command_line_STAR_.invoke(cli.clj:92)
	at clojuresque.tasks.compile$main.doInvoke(compile.clj:6)
	at clojure.lang.RestFn.applyTo(RestFn.java:137)
	at clojure.core$apply.invoke(core.clj:601)
	at clojure.lang.Var.invoke(Var.java:419)
	at clojuresque.Driver.main(Driver.java:39)
Caused by: java.lang.ClassNotFoundException: org.osgi.framework.ServiceRegistration


 Comments   
Comment by Gabriel Horner [ 17/May/13 3:56 PM ]

While it's reasonable to want this for your case, having long path names in a stacktrace could be inconvenient for others. I'd recommend posting your desired change on the dev list - https://groups.google.com/forum/?fromgroups#!forum/clojure-dev . If they're ok with it, then I'd recommend submitting a patch.





[CLJ-1128] Improve merge-with Created: 13/Dec/12  Updated: 13/Dec/12

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

Type: Enhancement Priority: Minor
Reporter: Edward Tsech Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File 0001-Improve-merge-with.patch     Text File 0002-Improve-merge-with.patch    
Patch: Code

 Description   

Set a first map as an initial value for reduce to
avoid merge-entry (series of contains? calls and etc) call on the first map.



 Comments   
Comment by Edward Tsech [ 13/Dec/12 1:36 PM ]

Tests pass.

Comment by Andy Fingerhut [ 13/Dec/12 2:32 PM ]

Edward, your patch replaces the expression (or m1 {}) with m1. It was changed from m1 to (or m1 {}) in a commit on Oct 16, 2008 with descriptive text "improved nil handling in merge, merge-with", so I am pretty sure it would be best to leave it as (or m1 {}). I believe the intent is to allow all but one of the map arguments to merge-with be nil, and everything will still work.

The rest of the patch for avoiding one merge call seems sound to me.

Your change would be even better at preserving any metadata on the first non-nil map in the list, if instead of calling with the first map, it called it with the first non-nil item of the list, and then the rest of the list after that.

Comment by Edward Tsech [ 13/Dec/12 5:41 PM ]

I figured out that `reduce1` did pass a head of the list for me. (https://github.com/clojure/clojure/blob/master/src/clj/clojure/core.clj#L887)
But case with first nil argument is still valid. Correct me, please, if i'm wrong.

I'm not sure about `(or m1 {})`. I don't see any problems which can happen. Probably behaviour of functions which are used internally was changed since 2008.

(contains? nil :a) ;=> false
(assoc nil :a 1) ;=> {:a 1}
(get nil :a) ;=> nil

I could write some tests for that.





[CLJ-1120] Introduce ex-message and ex-cause to abstract away the platform in dealing with ExceptionInfo Created: 06/Dec/12  Updated: 21/Dec/12

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

Type: Enhancement Priority: Minor
Reporter: Michał Marczyk Assignee: Michał Marczyk
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File 0001-CLJ-1120-ex-message-ex-cause.patch    
Patch: Code

 Description   

As described in the title. See CLJS-429.



 Comments   
Comment by Michał Marczyk [ 06/Dec/12 6:23 AM ]

The attached patch implements ex-message and ex-cause to work on arbitrary Throwables.





[CLJ-1119] inconsistent behavior of lazy-seq w/ macro & closure on excptions Created: 03/Dec/12  Updated: 08/Aug/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.4
Fix Version/s: None

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


 Description   

lazy-seq seems to evaluate inconsistently when body includes a macro and throws and exception. 1st evalutation throws the exceptions, subsequent ones return empty sequence.

demo code:

(defn gen-lazy []
(let [coll [1 2 3]]
(lazy-seq
(when-let [s (seq coll)]
(throw (Exception.))))))

(def lazy (gen-lazy))

(try
(println "lazy:" lazy)
(catch Exception ex
(println ex)))

(try
(println "lazy, again:" lazy)
(catch Exception ex
(println ex)))

It should throw an exception both times but only does on 1st. Generally speaking an expression shouldn't evaluate to different things depending on whether it's been evaluated before.

When removing the closure ...

(defn gen-lazy []
(lazy-seq
(when-let [s (seq [1 2 3])]
(throw (Exception.)))))

... or removing the when-let macro ...

(defn gen-lazy []
(let [coll [1 2 3]]
(lazy-seq
(seq coll)
(throw (Exception.)))))

It works i.e. consistently throws the exception, so seems to be some interaction between the closure and the macro at work here. This particular combination is used in the 'map' function.

See also: https://groups.google.com/forum/?fromgroups=#!topic/clojure/Z3EiBUQ7Inc



 Comments   
Comment by Hank [ 03/Dec/12 4:26 AM ]

N.B. The primary use case I have for this, in case it matters, is interrupting the evaluation of a 'map' expression in which the mapped fn is slow to evaluate (throwing an InterruptedException), because I am not interested in its result any more. Then later I re-evaluate it because I am interested in the result after all, however with above bug the lazy sequence terminates instead of recommencing where it left off.

(UPDATE: This use case is similar to the kind of ersatz continuations that Jetty does (RetryRequest runtime exception) or even Clojure itself when barging STM transactions with the RetryEx exception.)

Comment by Hank [ 03/Dec/12 8:45 AM ]

Related to CLJ-457 according to Christophe. His patch fixes this,too.

Comment by Hank [ 04/Dec/12 5:02 AM ]

Sorry Christophe's patch doesn't work for me here. It avoids evaluating the LazySeq a second time by prematurely throwing an exception. However a LazySeq might evaluate properly the second time around b/c the situation causing the exception was transient. As per comment above an evaluation might get interrupted, throwing InterruptedException the first time around but not the second time.

Also the observation with the closure and macro need explanation IMHO.

Comment by Hank [ 08/Dec/12 3:51 AM ]

further insight: 'delay' exhibits the same behavior and is a more simple case to examine. the macro suspicion is a red herring: as demoed below it is actually the closed over variable magically turns to nil, the when-let macro simply turned that into a nil for the whole expression.

(def delayed
  (let [a true]
    (delay
      (print "a=" a)
      (throw (Exception.)))))

(try
  (print "delayed 1:")
  (force delayed)
  (catch Exception ex (println ex)))

(try
  (print "delayed 2:")
  (force delayed)
  (catch Exception ex (println ex)))

prints:

delayed 1:a= true#<Exception java.lang.Exception>
delayed 2:a= nil#<Exception java.lang.Exception>

Comment by Hank [ 09/Dec/12 1:31 AM ]

The above leads to dodgy outcomes such as: The following expression leads to an Exception on 1st evaluation and to "w00t!" on subsequent ones.

(def delayed
  (let [a true]
    (delay
      (if a
        (throw (Exception.))
        "w00t!"))))

Try it like this:

(try
  (print "delayed 1:" )
  (println (force delayed))
  (catch Exception ex (println ex)))

(try
  (print "delayed 2:")
  (println (force delayed))
  (catch Exception ex (println ex)))

Results in:
delayed 1:#<Exception java.lang.Exception>
delayed 2:w00t!

This code shows that the problem is tied to the :once flag as suspected.

(def test-once
  (let [a true]
    (^{:once true} fn* foo []
	    (println "a=" a)
	    (throw (Exception.)))))

Invoking the fn twice will show 'a' turning from 'true' to 'nil', try it like this:

(try
  (print "test-once 1:")
  (test-once)
  (catch Exception ex (println ex)))

(try
  (print "test-once 2:")
  (test-once)
  (catch Exception ex (println ex)))

Results in:
test-once 1:a= true
#<Exception java.lang.Exception>
test-once 2:a= nil
#<Exception java.lang.Exception>

That doesn't happen when the ^{:once true} is removed. Now one could argue that above fn is invoked twice, which is exactly what one is not supposed to do when decorated with the :once flag, but I argue that an unsuccessful call doesn't count as invocation towards the :once flag. The delay and lazy-seq macros agree with me there as the resulting objects are not considered realized (as per realized? function) if the evaluation of the body throws an exception, and realization/evaluation of the body is therefore repeated on re-evaluation of the delay/lazy-seq.

Try this using

(realized? delayed)
after the first evaluation in the code above. In the implementation this can be seen e.g. here for clojure.lang.Delay (similarly for LazySeq), the body-fn is set to null (meaning realized) after the invocation returns successfully only.

The :once flag affects this part of the compiler only. Some field is set to nil there in the course of a function invocation, for the good reason of letting the garbage compiler collect objects, however this should to be done after the function successfully completes only. Can this be changed?

Comment by Hank [ 16/Dec/12 4:02 AM ]

A workaround for the case of the 'map' function as described in the 1st comment, works as this: The original map function, if you take out the cases for several colls, the performance enhancements for chunked seqs and forcing the coll argument to a seq, looks like this:

(defn map [f s]
  (lazy-seq
    (cons (f (first s)) (map f (rest s)))))

In my workaround I evaluate f twice:

(defn map [f s]
  (lazy-seq
    (f (first s))
    (cons (f (first s)) (map f (rest s)))))

Because the downstream functions that are slow to evaluate are all of the deref kind that cache their result (more lazy-seqs, delays, futures, promises), the InterruptedException can only happen during the 1st evaluation, while the tail call optimization that sets closed-over variables to nil (it pays to read this here: http://clojure.org/lazy) only happens on the second call. The first still creates an fn that captures the head of the sequence 's', however this is not being held onto as it is not returned.

I use this special version of map (and other, similarly rewritten functions based on lazy-seq such as iterate) when I want interruptible, restartable seq evaluations.

Comment by Hank [ 06/Aug/13 9:06 AM ]

Instead of above hack, implementing map and all other combinators using a lazy Y-combinator, and removing the :once meta-data tag in the lazy-seq definition fixes things properly. Since the compiler sort-of-hack that clears closed-over variables that's triggered by the :once meta-data tag is basically only for cases of recursion that can be implemented using the Y-combinator, that won't be needed anymore either.

Comment by Alex Miller [ 08/Aug/13 3:20 AM ]

Re the Delay reference above, this is covered (and addressed) in CLJ-1175 which is waiting for a final screen.





[CLJ-1115] multi arity into Created: 25/Nov/12  Updated: 05/Feb/14

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

Type: Enhancement Priority: Trivial
Reporter: Yongqian Li Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None

Attachments: File multi-arity-into.diff    
Patch: Code and Test

 Description   

Any reason why into isn't multi arity?

(into to & froms) => (reduce into to froms)

(into #{} [3 3 4] [2 1] ["a"]) looks better than (reduce into #{} [[3 3 4] [2 1] ["a"]])



 Comments   
Comment by Timothy Baldridge [ 27/Nov/12 11:25 AM ]

Seems to be a valid enhancement. I can't see any issues we'd have with it. Vetted.

Comment by Timothy Baldridge [ 29/Nov/12 2:06 PM ]

Added patch & test. This patch retains the old performance characteristics of into in the case that there is only one collection argument. For example: (into [] [1 2 3]) .

Since the multi-arity version will be slightly slower, I opted to provide it as a second body instead of unifying both into a single body. If someone has a problem with this, I can rewrite the patch. At least this way, into won't get slower.

Comment by Rich Hickey [ 09/Dec/12 7:39 AM ]

This is a good example of an idea for an enhancement I haven't approved, and thus is not yet vetted.

Comment by Yongqian Li [ 05/Feb/14 1:16 AM ]

What's the plan for this bug?

Comment by Alex Miller [ 05/Feb/14 11:27 AM ]

@Yongqian This ticket is one of several hundred open enhancements right now. It is hard to predict when it will be considered. Voting (or encouraging others to vote) on an issue will raise it's priority. The list at http://dev.clojure.org/display/community/Screening+Tickets indicates how we prioritize triage.





[CLJ-1113] `reductions` reducer Created: 23/Nov/12  Updated: 03/Sep/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Marshall T. Vandegrift Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: reducers

Attachments: File reductions-reducer.diff    
Patch: Code and Test

 Description   

It would be nice to have a reducers implementation of the core `reductions` function.

Initial implementation attempt included. This version requires an initial reduction value parameter (vs using (f)) and includes that initial value in the final reduction. I'm not certain either of these decisions is optimal, but results in a function which most closely mimics `clojure.core/reductions`.






[CLJ-1112] Var *loading-verbosely* should initialize from a JVM system property Created: 21/Nov/12  Updated: 03/Sep/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.4
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Howard Lewis Ship Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None

Attachments: Text File 0001-Allow-setting-loading-verbosely-by-system-property.patch    
Patch: Code

 Description   

I often find myself adding :verbose to a (require) or (use) clause of my (ns) in order to debug problems (especially macros, or bad namespace declarations). It would be very nice if I could define a JVM system property (say -Dclojure.load-verbosely=true) to default loading-verbosely to true for a REPL session, or as part of a build.

Sometimes I just like to see that namespaces load as a measure of progress, when starting an application, or when running a set of tests.



 Comments   
Comment by Tassilo Horn [ 22/Nov/12 2:12 AM ]

This patch implements the suggested feature.

The new system property is called clojure.core.loading-verbosely in analogy to the existing clojure.compile.warn-on-reflection.





[CLJ-1108] Allow to specify an Executor instance to be used with future-call Created: 18/Nov/12  Updated: 27/Dec/12

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

Type: Enhancement Priority: Major
Reporter: Max Penet Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File bac37b91230d8e4ab3a1e6042a6e8c4b7e9cbf53.patch     Text File clj-1108-enhance-future-call-patch-v2.txt    
Patch: Code

 Description   

This adds an arity to future-call that expects a java.util.concurrent/ExecutorService instance to be used instead of clojure.lang.Agent/soloExecutor.



 Comments   
Comment by Andy Fingerhut [ 26/Dec/12 4:50 PM ]

Rich Hickey committed a change on Dec 21, 2012 to the future-call function that made the patch bac37b91230d8e4ab3a1e6042a6e8c4b7e9cbf53.patch dated Nov 18 2012 no longer apply cleanly.

clj-1108-enhance-future-call-patch-v2.txt dated Dec 26 2012 is identical to that earlier patch, except it has been updated to apply cleanly to the latest master.

It would be best if Max Penet, author of the earlier patch, could verify I've merged his patch to the latest Clojure master correctly.

Comment by Max Penet [ 27/Dec/12 2:25 AM ]

It's verified, it applies correctly to the latest master 00978c76edfe4796bd6ebff3a82182e235211ed0 .

Thanks Andy.





[CLJ-1103] Make conj assoc dissoc and transient versions handle args similarly Created: 04/Nov/12  Updated: 14/Feb/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.4, Release 1.5
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Andy Fingerhut Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: File clj-1103-4.diff    
Patch: Code and Test

 Description   

A discussion came up in the Clojure Google group about conj giving an error when taking only a coll as an argument, as opposed to disj which works for this case:

https://groups.google.com/forum/?fromgroups=#!topic/clojure/Z9mFxsTYTqQ

I looked through the rest of the code for similar cases, and found that conj! assoc assoc! and disj! also had the same property, and there were some other differences between them in how different numbers of arguments were handled, such as:

conj handles an arbitrary number of arguments, but conj! does not.
assoc checks for a final key with no value specified (CLJ-1052), but assoc! did not.



 Comments   
Comment by Andy Fingerhut [ 04/Nov/12 6:04 PM ]

clj-1103-make-conj-assoc-dissoc-handle-args-similarly-v1.txt dated Nov 4 2012 makes conj conj! assoc assoc! dissoc dissoc! handle args similarly to each other.

Comment by Brandon Bloom [ 09/Dec/12 5:30 PM ]

I too ran into this and started an additional discussion here: https://groups.google.com/d/topic/clojure-dev/wL5hllfhw4M/discussion

In particular, I don't buy the argument that (into coll xs) is sufficient, since into implies conj and there isn't an terse and idiomatic way to write (into map (parition 2 keyvals))

So +1 from me

Comment by Andy Fingerhut [ 08/Nov/13 10:41 AM ]

Patch clj-1103-2.diff is identical to the previous patch clj-1103-make-conj-assoc-dissoc-handle-args-similarly-v1.txt except it applies cleanly to latest master. The only changes were some changed context lines in a test file.

Comment by Andy Fingerhut [ 23/Nov/13 12:52 AM ]

Patch clj-1103-3.diff is identical to the patch clj-1103-2.diff, except it applies cleanly to latest master. The only changes were some doc strings for assoc! and dissoc! in the context lines of the patch.

Comment by Andy Fingerhut [ 14/Feb/14 12:04 PM ]

Patch clj-1103-4.diff is identical to the previous clj-1103-3.diff, except it updates some context lines so that it applies cleanly to latest Clojure master as of today.





[CLJ-1097] node-seq for clojure.zip Created: 29/Oct/12  Updated: 03/Sep/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Timothy Baldridge Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: zip

Attachments: File node-seq.diff    
Patch: Code and Test

 Description   

Many times it's easier to get to a zipper node via (first (filter pred ...)) instead of manually walking the tree via next/up/down. Other times it's easier to process certain nodes via filter & map instead of again, walking the tree. This patch provides a single function called node-seq that uses zip/next to generate a lazy-seq of nodes. Tests provide two examples.






[CLJ-1096] Make destructuring emit direct keyword lookups Created: 29/Oct/12  Updated: 04/Sep/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.4
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Christophe Grand Assignee: Christophe Grand
Resolution: Unresolved Votes: 0
Labels: performance

Attachments: File desctructure-keyword-lookup.diff     File inline-get-keyword.diff    
Patch: Code

 Description   

Currently associative destructuring emits calls to get. The attached patch modify desctruture to emit direct keyword lookups when possible.

Approved here https://groups.google.com/d/msg/clojure-dev/MaYcHQck8VA/nauMus4mzPgJ



 Comments   
Comment by Christophe Grand [ 04/Sep/13 3:40 AM ]

Rethinking about this patch now, it may be too specific: get's inline expansion should be modified when the key is a literal keyword.

Comment by Christophe Grand [ 04/Sep/13 3:41 AM ]

More generic patch (inline-get-keyword.diff): all get calls with literal keywords as keys are inlined to direct keyword lookup.





[CLJ-1095] Allow map-indexed to accept multiple collections (a la map) Created: 25/Oct/12  Updated: 08/Nov/12

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

Type: Enhancement Priority: Trivial
Reporter: Bo Jeanes Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File 0001-map-indexed-accepts-multiple-collections.patch     Text File 0002-Add-test-for-multi-collection-map-indexed-fn.patch    
Patch: Code and Test

 Description   

Bring external interface of map-indexed in line with map. Existing usages of map-indexed unchanged both in implementation and interface.

examples
(map vector (range 10 20) (range 30 35)) ;=> ([10 30] [11 31] [12 32] [13 33] [14 34])
(map-indexed vector (range 10 20) (range 30 35)) ;=> ([0 10 30] [1 11 31] [2 12 32] [3 13 33] [4 14 34])

The attached patch is not necessarily the best implementation (I haven't benchmarked it or tried any alternatives yet) but hopefully enough to start a conversation about whether this is an addition that is warranted. I know I wished for this behavior a few weeks ago though I ended up finding another way.

(I haven't sent my CA yet, but I have it signed and ready to send in the next few days)



 Comments   
Comment by Aaron Bedra [ 25/Oct/12 5:11 PM ]

Can you add a test for the improved functionality?

Comment by Bo Jeanes [ 25/Oct/12 5:20 PM ]

You bet. I tried to before submitting this but found no existing tests for map-indexed to expand upon. Given that, I decided to just start the conversation first. If you think this is a good addition, I'll find a place to stick the tests and add a new patch file.

Comment by Bo Jeanes [ 25/Oct/12 8:05 PM ]

Add two unit tests for map-indexed. One tests old behavior (single collection) and the second tests mapping across 3 collections.

There were no existing tests for map-indexed that I could see to expand upon (using git grep map-indexed src/clojure)





[CLJ-1088] repl/source could support protocol functions Created: 21/Oct/12  Updated: 31/Jan/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Enhancement Priority: Trivial
Reporter: Chouser Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: repl

Attachments: Text File 0001-Add-support-for-protocol-fns-to-repl-source.-CLJ-1088.patch    
Patch: Code

 Description   
user=> (source clojure.core.protocols/coll-reduce)
Source not found

But since the protocol fn's var's metadata points to the protocol var, and the protocol var knows the file and line where it was defined, it would be trivial to improve 'source' to look like this:

user=> (source clojure.core.protocols/coll-reduce)
(defprotocol CollReduce
  "Protocol for collection types that can implement reduce faster than
  first/next recursion. Called by clojure.core/reduce. Baseline
  implementation defined in terms of Iterable."
  (coll-reduce [coll f] [coll f val]))


 Comments   
Comment by Chouser [ 21/Oct/12 10:00 AM ]

Add one-line patch to clojure.repl/source so that it will find the protocol definition for a given protocol function.

Comment by Andy Fingerhut [ 31/Jan/14 6:25 PM ]

Patch 0001-Add-support-for-protocol-fns-to-repl-source.-CLJ-1088.patch no longer applies cleanly as of commits made to Clojure master on Jan 31 2014, probably due to the patch for CLJ-1176. I have not investigated how easy or difficult it would be to update.





[CLJ-1087] clojure.data/diff uses set union on key seqs Created: 15/Oct/12  Updated: 03/Sep/13

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

Type: Enhancement Priority: Minor
Reporter: Tom Jack Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: performance

Attachments: Text File clj-1087-diff-perf-enhance-patch-v1.txt    
Patch: Code

 Description   

clojure.data/diff, on line 118, defines:

java.util.Map
(diff-similar [a b]
  (diff-associative a b (set/union (keys a) (keys b))))

Since keys returns a key seq, this seems like an error. clojure.set/union has strange and inconsistent behavior with regard to non-sets, and in this case the two key seqs are concatenated. Based on a cursory benchmark, it seems that this bug is a slight performance gain when the maps have no common keys, and a significant performance loss when the maps have the same keys. The results are still correct because of the merging reduce in diff-associative.

The patch is easy (just call set on each key seq).



 Comments   
Comment by Andy Fingerhut [ 15/Oct/12 2:52 PM ]

clj-1087-diff-perf-enhance-patch-v1.txt dated Oct 15 2012 implements Tom's suggested performance enhancement, although not exactly in the way he suggested. It does calculate the union of the two key sequences.





[CLJ-1081] REPL binding not working that works with with-bindings Created: 30/Sep/12  Updated: 05/Feb/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.4
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Steven Devijver Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: repl


 Description   

This works as expected:

java -jar clojure-1.4.0.jar -e "(do (require 'clojure.repl) (.setDynamic #'clojure.repl/print-doc) (with-bindings {#'clojure.repl/print-doc str} (eval '(clojure.repl/doc println))))"

Output:

"{:ns #<Namespace clojure.core>, :name println, :arglists ([& more]), :added \"1.0\", :static true, :doc \"Same as print followed by (newline)\", :line 3325, :file \"clojure/core.clj\"}"

But the same thing does not work in the REPL:

java -jar clojure-1.4.0.jar -e "(do (require 'clojure.repl) (.setDynamic #'clojure.repl/print-doc) (clojure.main/repl :init (fn [] {#'clojure.repl/print-doc str}))))"

Output for Output of {{(doc println)}}:

user=> (doc println)
-------------------------
clojure.core/println
([& more])
Same as print followed by (newline)
nil
user=>




 Comments   
Comment by Steven Devijver [ 01/Oct/12 5:51 AM ]

Found a work-around:

java -jar clojure-1.4.0.jar -e "(do (require 'clojure.repl) (.setDynamic #'clojure.repl/print-doc) (with-bindings {#'clojure.repl/print-doc str} (clojure.main/repl)))))"

I'm still not sure whether the method above using :init should or should not work.





[CLJ-1080] Eliminate many uses of reflection in Clojure code Created: 30/Sep/12  Updated: 10/Oct/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.4, Release 1.5
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Andy Fingerhut Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: performance, typehints

Attachments: Text File clj-1080-v5.txt    
Patch: Code

 Description   

There are dozens of uses of reflection in Clojure code that can be eliminated by adding suitable type hints. This patch adds the necessary type hints for most of those.



 Comments   
Comment by Andy Fingerhut [ 30/Sep/12 11:39 AM ]

Patch clj-1080-eliminate-many-reflection-warnings-patch-v1.txt dated Sep 30 2012 adds type hints to eliminate many uses of reflection in Clojure core code.

Comment by Andy Fingerhut [ 14/Nov/12 1:26 PM ]

clj-1080-eliminate-many-reflection-warnings-patch-v2.txt dated Nov 14 2012 is identical to the previous patch (to be removed soon), except it applies cleanly to latest master.

Comment by Andy Fingerhut [ 07/Feb/13 8:54 AM ]

clj-1080-eliminate-many-reflection-warnings-patch-v3.txt dated Feb 7 2013 is identical to the previous patch (to be removed soon), except it applies cleanly to latest master. One type hint in the patch was added due to a different change, and was no longer needed in the patch.

Comment by Andy Fingerhut [ 09/Sep/13 11:37 PM ]

Patch clj-1080-v4.txt eliminates many, but not all, uses of reflection. To avoid overlap with CLJ-1259, it does not touch pprint or any of the files loaded from pprint.clj. See CLJ-1259 for those.

Comment by Andy Fingerhut [ 10/Oct/13 12:22 AM ]

Patch clj-1080-v5.txt eliminates many, but not all, uses of reflection. It does not touch pprint or any of the files loaded from pprint.clj – see CLJ-1259 for those. Similarly see CLJ-1277 for elimination of reflection in instant.clj





[CLJ-1079] Don't squash explicit :line and :column metadata in the MetaReader Created: 29/Sep/12  Updated: 03/Sep/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.4, Release 1.5
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Chas Emerick Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: reader

Attachments: File CLJ-1079.diff    
Patch: Code and Test

 Description   

I have been experimenting with using cljx to produce Clojure and ClojureScript source from a single file. This has gone well so far, with the exception that, due to the way the source transformation works, all of the linebreaks and other formatting is gone from the output. There is an option to include the original :line metadata in the output though, like so:

;;This file autogenerated from 
;;
;;  src/cljx/com/foo/hosty.cljx
;;
^{:line 1} (ns com.foo.hosty)
^{:line 3} (defn ^{:clj true} system-hash [x] ^{:line 5} (System/identityHashCode x))

(Hopefully, such hackery won't be necessary in the future with sjacket or something like it...)

Unfortunately, when read in using a LineNumberingPushbackReader, code like this has its :line metadata squashed by the line numbers coming from that. A REPL-friendly example would be:

=> (meta (read (clojure.lang.LineNumberingPushbackReader.
                 (java.io.StringReader. "^{:line 66} ()"))))
{:line 1}
=> (meta (read (java.io.PushbackReader.
                 (java.io.StringReader. "^{:line 66} ()"))))
{:line 66}

The latter seems more correct to me (and is equivalent to read-string).



 Comments   
Comment by Chas Emerick [ 29/Sep/12 7:07 PM ]

{{CLJ-1097.diff}} contains a fix for this issue, as well as a separate commit that eliminates a series of casts in order to improve readability in the area.

Comment by Andy Fingerhut [ 05/Oct/12 8:23 AM ]

Chas, your patch doesn't apply cleanly to latest Clojure master as of Oct 5 2012. I'm not sure, but I think some recent commits to Clojure on Oct 4 2012 caused that. I also take it as evidence of your awesomeness that you can write patches for tickets that haven't been filed yet

Comment by Chas Emerick [ 05/Oct/12 9:24 AM ]

"patches for tickets that haven't been filed yet?"

Anyway, tweaking up this patch is a small price to pay for having column meta. New {{CLJ-1097.diff}} patch attached, applies clean on master as of now. Otherwise same contents as in the original patch, except:

  • the same dynamic is also applied to :column metadata, now that it's available
  • the changes have been rebased into a single commit (including the elimination of the casts in MetaReader, which were becoming so numerous that the code was less readable than most
Comment by Nicola Mometto [ 05/Oct/12 9:39 AM ]

"patches for tickets that haven't been filed yet?"

He was referring to the fact that you uploaded "CLJ-1097.diff" while the ticket is #1079

Comment by Chas Emerick [ 05/Oct/12 9:42 AM ]

Oh, hah! Twice now, even! One more data point recommending my having slight dyslexia or somesuch. :-P

I've replaced the attached patch with one that is named properly to avoid any later confusion.

Comment by Chas Emerick [ 07/Oct/12 3:57 PM ]

Refreshed patch to apply cleanly to master after the recent off by one patch for :column metadata.

Comment by Stuart Halloway [ 19/Oct/12 3:12 PM ]

This feels backwards to me. If a special purpose tool wants to convey information via metadata, why does it use names that collide with those used by LispReader?

Comment by Chas Emerick [ 19/Oct/12 7:36 PM ]

The information being conveyed is the same :line and :column metadata conveyed by LispReader — in fact, that's where it comes from in the first place.

Kibit (and cljx) is essentially an out-of-band source transformation tool. Given an input like this:

(ns com.foo.hosty)

(defn ^:clj system-hash
  [x]
 (System/identityHashCode x))

(defn ^:cljs system-hash
  [x]
  (goog/getUid x))

…it produces two files, a .clj for Clojure, and a .cljs for ClojureScript. (The first code listing in the ticket description is the former.)

However, because there's no way to transform Clojure code/data without losing formatting, anything that depends on line/column numbers (stack traces, stepping debuggers) is significantly degraded. If LispReader were to defer to :line and :column metadata already available on the loaded forms (there when the two generated files are spit out with *print-meta* on), this would not be the case.

Comment by Andy Fingerhut [ 07/Feb/13 8:47 AM ]

clj-1079-patch-v2.txt dated Feb 7 2013 is identical to Chas's CLJ-1079.diff dated Oct 7 2012, except it applies cleanly to latest master. I believe the only difference is that some white space in the context lines is updated.

Comment by Andy Fingerhut [ 07/Feb/13 12:35 PM ]

Sorry for the noise. I've removed clj-1079-patch-v2.txt mentioned in the previous comment, because I learned that CLJ-1079.diff dated Oct 7 2012 applies cleanly to latest master and passes all tests if you use this command to apply it.

% git am --keep-cr -s --ignore-whitespace < CLJ-1079.diff

I will update the JIRA workflow page instructions for applying patches to mention this, too, because there are multiple patches that fail without --ignore-whitespace, but apply cleanly with that option. That will eliminate the need to update patches merely for whitespace changes.





[CLJ-1077] thread-bound? returns true (implying set! should succeed) even for non-binding thread Created: 26/Sep/12  Updated: 20/Aug/13

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

Type: Defect Priority: Major
Reporter: Paul Stadig Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: None

Attachments: File thread-bound.diff    
Patch: Code

 Description   

thread-bound? returns true for a non-binding thread, this result (according to the docstring) implies that set! should succeed. However, thread-bound? does not check that any binding that might exist was created by the current thread, and calling set! fails with an exception when it is called from a non-binding thread, even though thread-bound? returns true.

thread-bound? should return false if there is a binding, and that binding was not established by the current thread.

Here is an example REPL session where a thread establishes a binding, those bindings are conveyed to a second thread, the second thread checks thread-bound? to see if it can set the binding, thread-bound? returns true indicating that the binding can be set, the second thread tries to set the binding, and the second thread gets an IllegalStateException:

    Clojure 1.5.1
    user=> (def ^:dynamic *set-me* nil)
    #'user/*set-me*
    user=> (defn try-to-set [] (binding [*set-me* 1] (doall (pcalls #(if (thread-bound? #'*set-me*) (set! *set-me* (inc *set-me*)))))))
    #'user/try-to-set
    user=> (try-to-set)
    IllegalStateException Can't set!: *set-me* from non-binding thread  clojure.lang.Var.set (Var.java:230)
    user=> 


 Comments   
Comment by Paul Stadig [ 01/Oct/12 10:07 AM ]

I have attached a patch that changes clojure.lang.Var and clojure.core/thread-bound? to only return true if a Var is set!-able.

Comment by Alex Miller [ 19/Aug/13 12:16 PM ]

REPL example?

Comment by Joe Gallo [ 20/Aug/13 7:55 AM ]

Sure thing, Alex – here's a repl example I just ran this morning.

; nREPL 0.1.7
user> (def ^:dynamic *set-me* nil)
#'user/*set-me*
user> (defn try-to-set [] (binding [*set-me* 1] (doall (pcalls #(if (thread-bound? #'*set-me*) (set! *set-me* (inc *set-me*)))))))
#'user/try-to-set
user> (try-to-set)
IllegalStateException Can't set!: *set-me* from non-binding thread  clojure.lang.Var.set (Var.java:230)
user>




[CLJ-1074] Read/print round-trip for +/-Infinity and NaN Created: 21/Sep/12  Updated: 03/Sep/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.4
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Colin Jones Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: print, reader

Attachments: Text File 0001-Read-Infinity-and-NaN.patch     Text File clj-1074-read-infinity-and-nan-patch-v2-plus-edn-reader.patch    
Patch: Code and Test

 Description   

A few float-related forms (namely, Doubl