<< Back to previous view

[CLJ-1220] instance? should either verify all operands or throw if more than one passed Created: 19/Jun/13  Updated: 19/Jun/13

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

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


 Description   

(instance? Number 1) ;; => true
(instance? Number "a") ;; => false
(instance? Number 1 "a") ;; => true

I find behavior of the last expression very surprising, I would
expect it to either desugar to logical "and" over all operands:

(and (instance? Number 1) (instance? Number "a") ...)

Or throw "Wrong number of args (3) passed to instance?" exception.



 Comments   
Comment by Andy Fingerhut [ 19/Jun/13 5:32 PM ]

Irakli, one of the patches for CLJ-1171 addresses this issue by causing (instance? Number 1 "a") to throw a Wrong number of args (3) passed to core$instance-QMARK- ArityException.





[CLJ-1219] Make identical? variadic Created: 19/Jun/13  Updated: 19/Jun/13

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

Type: Enhancement Priority: Major
Reporter: Irakli Gozalishvili Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

(= 1 1 1) ;; => true
(= 1 1 2) ;; => false
(== 1 1 1) ;; => true
(== 1 1 2) ;; => false
(identical? 1 1 1) ;; ArityException Wrong number of args (3) passed to: core$identical-QMARK- clojure.lang.AFn.throwArity (AFn.java:437)

I think it would make far more sense to make identical? consistent with all other comparison operators
and allow it to take variadic number of arguments.






[CLJ-1218] mapcat is not very lazy Created: 16/Jun/13  Updated: 17/Jun/13

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

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


 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 (concat (first s) (concats (rest s)))))


 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.





[CLJ-1217] for consumes sequence argument more eagerly than necessary Created: 14/Jun/13  Updated: 14/Jun/13

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

Type: Defect Priority: Major
Reporter: Alan Malloy Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

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: 09/Jun/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: None

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-1215] Mention where to position docstring when using pre/postconditions on the Special Forms page Created: 07/Jun/13  Updated: 07/Jun/13

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

Type: Enhancement Priority: Minor
Reporter: Jakub Holy Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: documentation


 Description   

The description of pre- and post-conditions doesn't mention where docstring should be when using them. Placing it wrong will lead to the conditions to be ignored.

At http://clojure.org/Special%20Forms--(fn%20name?%20[params*%20]%20condition-map?%20exprs*), change

(fn name? [params* ] condition-map? exprs*)

to

(fn name? [params* ] condition-map? docstring? exprs*)

or at least mention it in the description or use it in the example.

Thank you!






[CLJ-1214] Compiler runs out of memory on a small snippet of code Created: 31/May/13  Updated: 02/Jun/13

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

Type: Defect Priority: Major
Reporter: Praki Prakash Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Linux 3.2.0-39-generic


Attachments: File fubar.clj    

 Description   

Clojure compiler runs out of memory when loading the attached file. Transcript below.

$ java -cp ~/.m2/repository/org/clojure/clojure/1.5.1/clojure-1.5.1.jar:. clojure.main
Clojure 1.5.1
user=> (load "fubar")
OutOfMemoryError GC overhead limit exceeded  [trace missing]
user=> 

The file contents are:

  (ns fu.bar)

  (defn foo[l] (concat (drop-last l) (repeat (last l))))

  (def ^:const bar (foo [#(print "") #(println ";")]))

  bar

If I remove the metadata on bar, it works. Removal of namespace seems to fix it as well. Pretty strange.

Although I realize this code is not quite kosher, it would be nice to have the compiler deal with it.



 Comments   
Comment by Andy Fingerhut [ 01/Jun/13 7:40 PM ]

If you really do have 'bar' on a line by itself at the end of file fubar.clj, it seems you are asking it to evaluate the value of bar, which by the definitions is an infinite list, and will of course exhaust all available memory if you attempt to evaluate it.

It seems to me the more odd thing is not that it runs out of memory as shown, but that it does not run out of memory when you remove the metadata on bar.

What is the purpose of having 'bar' on a line by itself at the end of the file?

If I try this but remove 'bar' as the last line of the file, loading the file causes no errors regardless of whether there is metadata on bar's definition or not. It is strange that doing (load "fubar") followed by (first fu.bar/bar) seems to go into an infinite loop if the :const is there on bar, but quickly returns the correct answer if the metadata is removed.

Comment by Praki Prakash [ 01/Jun/13 8:40 PM ]

This code snippet is a minimal test case to show that the compiler runs out of memory. What I meant by "it works" was that the compiler doesn't run out of memory and successfully loads the file (or in my real code base, the namespace is compiled).

In my code, I use bar (or whatever the real thing is) as source of sequence of functions. The sole reference to bar is needed to trigger this issue. I believe that bar is not being fully evaluated here and thus no infinite loop. If I try to print it, yes, it will ultimately fail.

Comment by Praki Prakash [ 01/Jun/13 9:04 PM ]

Having thought about this a bit more, it seems to me that when bar is annotated with const, the compiler is trying to evaluate the associated expression which exhausts the heap? I have never looked at the compiler source and thus not sure if this is what is happening. If it is, then it seems like one should be really careful when adding metadata.

That still leaves the other question about the namespace requirement to cause memory exhaustion. I quite distinctly recall having to add the namespace when trying to come up with minimal test case to reproduce the bug.

If you think this is really user error, I would accept it

Comment by Andy Fingerhut [ 02/Jun/13 4:56 AM ]

It is not just any old metadata that causes this issue, only :const metadata. I tried testing with :const replaced with :dynamic and :private, and there was no problem.

This might shed some light on the issue: https://github.com/clojure/clojure/blob/master/changes.md#215-const-defs

It appears that ^:const is causing the compiler to evaluate the value at compile time. The value in your example is unbounded, so that can never complete.





[CLJ-1213] consistency with def and Unbound Created: 29/May/13  Updated: 29/May/13

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

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


 Description   

In this example I'd expect `b` to return `Unbound` for consistency.

Clojure 1.5.1
user=> (def a "MyA")
#'user/a
user=> (def a "MyA2")
#'user/a
user=> (def b "MyB")
#'user/b
user=> (def b) ;; unbound b
#'user/b
user=> (def c) ;; unbound c
#'user/c
user=> a
"MyA2"
user=> b
"MyB"
user=> c
#<Unbound Unbound: #'user/c>





[CLJ-1212] Silent truncation/downcasting of primitive type on reflection call to overloaded method (Math/abs) Created: 28/May/13  Updated: 29/May/13

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: None
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: 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: Trevor Wennblom Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 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.






[CLJ-1209] Teach clojure.test reporting about ex-info/ex-data Created: 11/May/13  Updated: 11/May/13

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

Type: Enhancement Priority: Trivial
Reporter: Thomas Heller Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: File clj-test-print-ex-data.diff    
Patch: Code

 Description   

When clojure.test/deftest does error reports on unexpected exceptions it currently ignores ExceptionInfo and the valuable ex-data it carries. So this patch simple prints this data, it might be helpful to pprint or format it in another way but this was good enough for me.

See example from my tests: https://gist.github.com/thheller/5559391






[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: 0
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: 26/May/13

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

Type: Defect Priority: Major
Reporter: Howard Lewis Ship Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: feedback
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-1205] Update Maven build for Nexus 2.4 Created: 22/Apr/13  Updated: 19/Jun/13

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

Type: Task Priority: Major
Reporter: Stuart Sierra Assignee: Stuart Sierra
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File 0001-nexus-2.4-releases.patch    
Approval: Ok

 Description   

These additions to the build configuration are necessary to support changes to the Sonatype Nexus server at oss.sonatype.org, which we use to promote our build artifacts into the Maven Central Repository.

See Sonatype's announcement at https://groups.google.com/d/msg/clojure-dev/lBpfII2u6vM/LQvr_rO5UGgJ



 Comments   
Comment by Stuart Halloway [ 19/Jun/13 10:07 AM ]

Am I right in assuming that the only way we will know this works is by trying it in a release?

Comment by Stuart Sierra [ 19/Jun/13 10:51 AM ]

Yes.





[CLJ-1204] hash is inconsistent with = for many BigInteger values Created: 18/Apr/13  Updated: 25/Apr/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: Andy Fingerhut Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File clj-1204-make-hash-consistent-with-equal-for-bigintegers-v1.txt    
Patch: Code and Test

 Description   

The function hash is documented to be consistent with =. For many BigInteger values, hash is inconsistent with =. This leads to incorrect behavior for data structures like hash maps that use hash.

user> (apply = [-1 -1N (biginteger -1)])
true
user> (map hash [-1 -1N (biginteger -1)])
(0 0 -1)

;; Incorrect return value with multiple keys = to each other
user> (assoc (hash-map -1N :should-be-replaced) (biginteger -1) :new-val)
{-1N :should-be-replaced, -1 :new-val}

;; array-map gives correct value, since it uses =, not hash
user> (assoc (array-map -1N :should-be-replaced) (biginteger -1) :new-val)
{-1N :new-val}


 Comments   
Comment by Andy Fingerhut [ 18/Apr/13 8:10 PM ]

Patch clj-1204-make-hash-consistent-with-equal-for-bigintegers-v1.txt dated Apr 18 2013 takes the following approach to the issue:

Change the behavior of hasheq so that when given a BigInteger value that could fit into a long, returns the same hash code as that long value.

hasheq continues to return x.hashCode() if the BigInteger value does not fit into a long. This is consistent with the hash value returned by a BigInt value that does not fit into a long.

New tests are included, some of which fail without the change to hasheq, but pass with the change.

Comment by Andy Fingerhut [ 18/Apr/13 8:19 PM ]

Overwrite patch with one that leaves out some unnecessary code.

Comment by Andy Fingerhut [ 25/Apr/13 6:42 PM ]

Changing priority to minor, since I suppose one could work around this issue, if you were diligent about it, by converting all BigIntegers to BigInts before they are ever used in a place where they are hashed.





[CLJ-1202] protocol fns with dashes may get compiled into property access when used higher order Created: 16/Apr/13  Updated: 10/May/13

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

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

Attachments: Text File CLJ-1202.patch    
Patch: Code and Test
Approval: Vetted

 Description   
user=> (defprotocol Foo (-foo [x]))
Foo
user=> (deftype Bar [] Foo (-foo [_] :baz))
user.Bar
user=> (map -foo [(Bar.)])
IllegalArgumentException No matching field found: foo for class user.Bar  
clojure.lang.Reflector.getInstanceField (Reflector.java:271)

I would have expected to see (:baz). The full stack is:

IllegalArgumentException No matching field found: foo for class user.Bar
	clojure.lang.Reflector.getInstanceField (Reflector.java:271)
	clojure.lang.Reflector.invokeNoArgInstanceMember (Reflector.java:300)
	user/eval79/fn--80/G--71--82 (NO_SOURCE_FILE:11)
	user/eval79/fn--80/G--70--85 (NO_SOURCE_FILE:11)
	clojure.core/map/fn--4207 (core.clj:2485)
	clojure.lang.LazySeq.sval (LazySeq.java:42)
	clojure.lang.LazySeq.seq (LazySeq.java:60)
	clojure.lang.RT.seq (RT.java:484)
	clojure.core/seq (core.clj:133)
	clojure.core/print-sequential (core_print.clj:46)
	clojure.core/fn--5406 (core_print.clj:143)
	clojure.lang.MultiFn.invoke (MultiFn.java:231)
nil

I suspect this is somehow related to the property access changes to make Clojure/ClojureScript compatible. I was in fact prepping core.logic for a unified code base and was adopting the ClojureScript protocol naming convention when I encountered this issue.

CLJ-872 added dash property support to Clojure.



 Comments   
Comment by Alan Malloy [ 18/Apr/13 7:18 PM ]

Attached patch fixes the issue, and adds regression test for it.

Comment by Gabriel Horner [ 10/May/13 3:19 PM ]

Verified patch works





[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: 0
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-1200] RestFn & ArraySeq performance Created: 14/Apr/13  Updated: 07/Jun/13

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

Type: Enhancement Priority: Minor
Reporter: Brandon Bloom Assignee: Unassigned
Resolution: Unresolved Votes: 3
Labels: patch

Attachments: Text File no-getComponentType--v001.patch    
Patch: Code

 Description   

I was profiling one of my projects and noticed a hotspot inside functions using rest arguments.

Most overloads of RestFn.invoke will construct an ArraySeq object. The ArraySeq constructor calls java.lang.Class.getComponentType, which seems to be pretty slow. The array's component type is cached in a field on the ArraySeq object for the sole purpose of passing it to Reflector.prepRet. I'm not entirely sure of the total utility of prepRet, but it's clearly a no-op when passed Object.class, such as the case with the non-specialized base class of ArraySeq: the component type of object[] is Object.class.

The attached patch eliminates the component type field from ArraySeq and passes Object.class to prepRet directly. It may be possible to eliminate calls to prepRet all together, but I'll assume that's a different ticket. With this patch, ArraySeq initialization no longer shows up as a hotspot when profiling.

Before the patch:

user=> (dotimes [n 10] (time (dotimes [i 5000000] ((fn [& args]) 1 2 3 4))))
"Elapsed time: 874.742 msecs"
"Elapsed time: 900.277 msecs"
"Elapsed time: 911.164 msecs"
"Elapsed time: 872.132 msecs"
"Elapsed time: 885.495 msecs"
"Elapsed time: 897.537 msecs"
"Elapsed time: 879.691 msecs"
"Elapsed time: 888.52 msecs"
"Elapsed time: 871.556 msecs"
"Elapsed time: 1088.682 msecs"

After the patch:

user=> (dotimes [n 10] (time (dotimes [i 5000000] ((fn [& args]) 1 2 3 4))))
"Elapsed time: 628.144 msecs"
"Elapsed time: 634.163 msecs"
"Elapsed time: 617.397 msecs"
"Elapsed time: 622.714 msecs"
"Elapsed time: 646.743 msecs"
"Elapsed time: 648.708 msecs"
"Elapsed time: 629.223 msecs"
"Elapsed time: 638.058 msecs"
"Elapsed time: 725.473 msecs"
"Elapsed time: 636.909 msecs"

That's about a 30% reduction in execution time.

Granted it only represents a change of 52 nanoseconds per RestFn invoke (181 ns to 129 ns), but this actually was a pretty decent win for a library that makes makes almost exclusively variadic function calls in a tight loop.



 Comments   
Comment by Zach Tellman [ 07/Jun/13 5:40 PM ]

As a data point, I was recently profiling a Hadoop job where the code made heavy use of 'partial', which apparently unlike 'comp' and 'juxt', always uses apply [1]. As a result, .getComponentType accounted for 41% of the overall compute time. This might be as much a comment on the implementation of partial as anything else, but it certainly shows that this can have a significant effect on performance.

[1] https://github.com/clojure/clojure/blob/master/src/clj/clojure/core.clj#L2388

Comment by Kevin Downey [ 07/Jun/13 5:46 PM ]

prepRet usage appears to be all about enforcing canonical Boolean values, so I think using Object.class is not the best. Maybe splitting ArraySeq in to ArraySeq and ArraySeqBoolean would be better. ArraySeq would no longer do a prepRet and ArraySeqBoolean would

Comment by Kevin Downey [ 07/Jun/13 7:19 PM ]

ArraySeq actually already has specialized implementations, and interestingly the specialized boolean implementation doesn't call prepRet

Comment by Kevin Downey [ 07/Jun/13 7:35 PM ]

The ArraySeq split I proposed above will fail if you have an array of Objects that happen to be Booleans, it seems like the best bet would be something like preRet that did a instanceof Boolean check without the requirement of passing in a class





[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-1197] Allow fold to parallelize over lazy sequences Created: 10/Apr/13  Updated: 10/Apr/13

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

Type: Enhancement Priority: Major
Reporter: Paul Butcher Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: File foldable-seq.diff    
Patch: Code

 Description   

This patch implements foldable-seq, which allows fold to parallelize over a lazy sequence. See this conversation on the Clojure mailing list:

https://groups.google.com/forum/#!msg/clojure/8RKCjF00ukQ/b5mmmOB5Uh4J

The patch is code only, sadly. No tests because I've not been able to find any existing tests for fold:

https://groups.google.com/d/msg/clojure-dev/plQ16L1_FC0/CIyMVIgSZkkJ

However, I have tested it in a separate project successfully.






[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-1193] bigint, biginteger throw on double values outside of long range Created: 07/Apr/13  Updated: 24/May/13

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

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

Attachments: Text File clj-1197-make-bigint-work-on-all-doubles-v1.txt    
Patch: Code and Test
Approval: Screened

 Description   

This works fine:

user> (bigint (* 0.99 Long/MAX_VALUE))
9131138316486227968N

but this and any other Float or Double values outside the range of a
long throw an exception:

user> (bigint (* 1.01 Long/MAX_VALUE))
IllegalArgumentException Value out of range for long: 9.315605757223324E18 clojure.lang.RT.longCast (RT.java:1178)

Similarly for biginteger



 Comments   
Comment by Andy Fingerhut [ 07/Apr/13 4:38 PM ]

Patch clj-1197-make-bigint-work-on-all-doubles-v1.txt dated Apr 7 2013 changes bigint and biginteger so that they return the correct value for all float and double values, and no longer throw an exception.

Comment by Gabriel Horner [ 17/May/13 10:52 AM ]

Looks good. Tests pass and the failing example now converts correctly





[CLJ-1192] vec function is substantially slower than into function Created: 06/Apr/13  Updated: 14/Jun/13

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

Type: Defect Priority: Trivial
Reporter: Luke VanderHart Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

(vec coll) and (into [] coll) do exactly the same thing. However, due to into using transients, it is substantially faster. On my machine:

(time (dotimes [_ 100] (vec (range 100000))))
"Elapsed time: 732.56 msecs"

(time (dotimes [_ 100] (into [] (range 100000))))
"Elapsed time: 491.411 msecs"

This is consistently repeatable.

Since vec's sole purpose is to transform collections into vectors, it should do so at the maximum speed available.



 Comments   
Comment by Andy Fingerhut [ 07/Apr/13 5:50 PM ]

I am pretty sure that Clojure 1.5.1 also uses transient vectors for (vec (range n)) (probably also some earlier versions of Clojure, too).

Look at vec in core.clj. It checks whether its arg is a java.util.Collection, which lazy seqs are, so calls (clojure.lang.LazilyPersistentVector/create coll).

LazilyPersistentVector's create method checks whether its argument is an ISeq, which lazy seqs are, so it calls PersistentVector.create(RT.seq(coll)).

All 3 of PersistentVector's create() methods use transient vectors to build up the result.

I suspect the difference in run times are not because of transients or not, but because of the way into uses reduce, and perhaps may also have something to do with the perhaps-unnecessary call to RT.seq in LazilyPersistentVector's create method (in this case, at least – it is likely needed for other types of arguments).

Comment by Alan Malloy [ 14/Jun/13 2:17 PM ]

I'm pretty sure the difference is that into uses reduce: since reducers were added in 1.5, chunked sequences know how to reduce themselves without creating unnecessary cons cells. PersistentVector/create doesn't use reduce, so it has to allocate a cons cell for each item in the sequence.





[CLJ-1191] Improve apropos to show some indication of namespace of symbols found Created: 04/Apr/13  Updated: 05/Apr/13

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: Andy Fingerhut Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None

Attachments: Text File clj-1191-patch-v1.txt    
Patch: Code

 Description   

apropos does find all symbols in all namespaces that match the argument, but the return value gives no clue as to which namespace the found symbols are in. It can even return multiple occurrences of the same symbol, which only gives a clue that the symbol exists in more than one namespace, but not which ones. For example:

user=> (apropos "replace")
(postwalk-replace prewalk-replace replace re-quote-replacement replace replace-first)

It would be nice if the returned symbols could indicate the namespace, either always, or if the found symbol is not in the current namespace.



 Comments   
Comment by Andy Fingerhut [ 04/Apr/13 8:25 PM ]

Path clj-1191-patch-v1.txt enhances apropos to put a namespace/ qualifier before every symbol found that is not in the current namespace ns. It also finds the shortest namespace alias if there is more than one. Examples of output with patch:

user=> (apropos "replace")
(replace clojure.string/re-quote-replacement clojure.string/replace clojure.string/replace-first clojure.walk/postwalk-replace clojure.walk/prewalk-replace)

user=> (require '[clojure.string :as str])
nil
user=> (apropos "replace")
(replace clojure.walk/postwalk-replace clojure.walk/prewalk-replace str/re-quote-replacement str/replace str/replace-first)

user=> (in-ns 'clojure.string)
#<Namespace clojure.string>
clojure.string=> (clojure.repl/apropos "replace")
(re-quote-replacement replace replace-by replace-first replace-first-by replace-first-char replace-first-str clojure.core/replace clojure.walk/postwalk-replace clojure.walk/prewalk-replace)

Comment by Colin Jones [ 05/Apr/13 1:34 PM ]

+1

apropos as it already stands is quite helpful for already-referred vars, but not for vars that are only in other nses.

This update includes the information someone would need to further investigate the output.





[CLJ-1190] Javadoc for public Java API Created: 03/Apr/13  Updated: 03/Apr/13

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

Type: Enhancement Priority: Minor
Reporter: Stuart Halloway Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

We should publish javadoc for http://dev.clojure.org/jira/browse/CLJ-1188.

  • make sure to publish on the the public API classes and interfaces
  • if IFn remains part of public interface, find javadoc control knob to disable including the nested interfaces dealing with primitives
  • automate publishing the docs somewhere (javadoc.clojure.org?)

This ticket should wait until CLJ-1188 is ok'ed.






[CLJ-1189] Map-destructuring :or fumble needs compiler warning Created: 31/Mar/13  Updated: 28/May/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: None

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-1187] Clojure loses quoted metadata on empty literals Created: 22/Mar/13  Updated: 24/May/13

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

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

Attachments: Text File 001-CLJ-1187.patch    
Patch: Code and Test
Approval: Screened

 Description   

user=> (meta '^:foo [])
nil

while
user=> (meta '^:foo [1])
{:foo true}

This bug propagates to ^:const vars:
user=> (def ^:const foo ^:foo [])
#'user/foo
user=> (meta foo)
nil
user=> (meta @#'foo)
{:foo true}



 Comments   
Comment by Nicola Mometto [ 22/Mar/13 2:12 PM ]

After patch:

user=> (meta '^:foo [])
{:foo true}
user=> (meta '^:foo [:a])
{:foo true}
user=> (def ^:const foo ^:foo [])
#'user/foo
user=> (meta foo)
{:foo true}

Comment by Stuart Halloway [ 29/Mar/13 6:13 AM ]

I believe the title should read "Clojure loses quoted metdata on empty literals".

Comment by Stuart Halloway [ 29/Mar/13 6:16 AM ]

At first glance, the implementation looks wrong in that it blocks non-IObjs ever getting to EmptyExpr. Probably all persistent collections are IObjs, but would not want to bake this in.

Comment by Nicola Mometto [ 29/Mar/13 7:00 AM ]

You're right, I've updated my patch, it should work as expected now

Comment by Andy Fingerhut [ 04/Apr/13 9:44 PM ]

Nicola: Your updated patch 001-CLJ-1187.patch dated Mar 29, 2013 gives syntax errors when I try to compile it.

Comment by Nicola Mometto [ 05/Apr/13 9:06 AM ]

Oh, the irony, I messed up some parentheses writing java.

Sorry for it, here's the correct patch, it applies on upstream/master.

Comment by Gabriel Horner [ 24/May/13 9:14 AM ]

Looks good





[CLJ-1185] `reductions should respect `reduced Created: 16/Mar/13  Updated: 10/Apr/13

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

Type: Enhancement Priority: Minor
Reporter: Brandon Bloom Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJ-1181-v001.patch    
Patch: Code

 Description   

This returns 16:

(reduce (fn [acc x]
          (let [x' (* x x)]
            (if (> x' 10)
              (reduced x')
              x')))
        (range))

But replacing `reduce with `reductions will never terminate:

(reductions (fn [acc x]
              (let [x' (* x x)]
                (if (> x' 10)
                  (reduced x')
                  x')))
            (range))

This is because `reductions ignores 'clojure.lang.Reduced, it never tests for `reduced?

I know that I only just discovered the `reduced, function, but `reductions is a big part of my debugging process, so it's unfortunate that they don't work together.



 Comments   
Comment by Brandon Bloom [ 16/Mar/13 6:10 PM ]

Attaching patch





[CLJ-1184] Evaling #{do ...} or [do ...] is treated as the do special form Created: 16/Mar/13  Updated: 09/Jun/13

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

Type: Defect Priority: Trivial
Reporter: Jiří Maršík Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: Compiler, bug

Attachments: Text File CLJ-1184-p1.patch    
Patch: Code and Test
Approval: Vetted

 Description   

Evaluating a persistent collection for which the function 'first' returns the symbol 'do' leads to that collection being treated as the do special form, even though it may be a vector or even a set. IMHO, the expected result would be to report that 'do' cannot be resolved. For the cause, see the if condition on line 6604 of Compiler.java in clojure-1.5.1.

E.g.:
[do 1 2]
;=> 2

#{"hello" "goodbye" do}
;=> "hello"
; Wat?



 Comments   
Comment by Gary Fredericks [ 26/May/13 2:13 PM ]

Attached a patch that changes IPersistentCollection to ISeq on the referenced line, and a regression test.

Comment by Nicola Mometto [ 09/Jun/13 2:52 PM ]

As found out on #clojure, there are still some cases where the symbol "do" behaves in unexpected ways that this patch doesn't address.

user=> ((fn [do] do) 1)
nil
user=> (let [do 1] do)
nil

Comment by Gary Fredericks [ 09/Jun/13 3:00 PM ]

Presumably the same issue is the difference between

(let [x 5] do x)

which returns 5 and

(let [x 5] do do x)

which gives a compile error.

Comment by Nicola Mometto [ 09/Jun/13 4:31 PM ]

This is actually a different bug.
I created another ticket with patch+test see http://dev.clojure.org/jira/browse/CLJ-1216





[CLJ-1183] java interop - cannot call a final method on non-public superclass Created: 13/Mar/13  Updated: 18/May/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: Shlomi Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None

Attachments: GZip Archive call-test.tar.gz    

 Description   

when trying to call a method on a concrete class that is defined as final on its super class that is not public, the runtime throws:

"java.lang.IllegalArgumentException: Can't call public method of non-public class"

even when fully annotated, Reflection is still used and the call fails.

you can read the full description here https://groups.google.com/d/msg/clojure/p2tBMT-BIYc/mDQB8cSponMJ

I included a sample project that demonstrate the problem



 Comments   
Comment by Shlomi [ 13/Mar/13 6:51 AM ]

in my sample project, i used a nested class, but i didnt have to (as pointed by Marko Topolnik). changing the java code to:

abstract class AbstractParent{
final public int x() {return 6;}
}

public class test extends AbstractParent {}

and the clojure to:

(ns call-test.core (:gen-class))
(defn -main [& args](.x ^AbstractParent (test.)))

would produce the same error,

java.lang.IllegalArgumentException: Can't call public method of non-public class: public final int AbstractParent.x()
at clojure.lang.Reflector.invokeMatchingMethod(Reflector.java:88)

Comment by zoowar [ 16/May/13 12:05 PM ]

This issue affects the upcoming netty-4.0 release in which the public modifier of AbstractBootstrap was removed.

Comment by Matthew Phillips [ 18/May/13 3:48 AM ]

To get Netty 4 working with Clojure I had to create a set of public static Java methods for the various inaccessible Netty calls, which I then call from Clojure. A PITA, but works fine. Happy to post code if anyone would find it useful.

Comment by Shlomi [ 18/May/13 4:31 AM ]

Matthew, i kinda left that project after running to these and other troubles (focused on previous Netty until version 4 will become ready and be properly documented), but i'd still like to see your code. you have a github account or a gist with it?

Clojure devs - are there any plans of checking this problem out? it came up from Netty, but the problem is pretty generic

Comment by Matthew Phillips [ 18/May/13 7:22 PM ]

Shlomi: here's a gist with the code I'm using in it. It's not comprehensive, just the bits I needed.

https://gist.github.com/scramjet/5606195





[CLJ-1182] Regexp are never equal Created: 12/Mar/13  Updated: 13/Apr/13

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

Type: Enhancement Priority: Minor
Reporter: Christian Fortin Assignee: Omer Iqbal
Resolution: Unresolved Votes: 0
Labels: bug

Attachments: File fix-CLJ-1182.diff    
Patch: Code
Approval: Triaged

 Description   

The following (= #"asd" #"asd") will return false in CLJ, even if they are the same.

Consequently, everything with a regexp in it (lists, vectors, maps...) will never be equal.

It seems to have been fixed for CLJS, but not for CLJ.
https://github.com/clojure/clojurescript/commit/e35c3a57472fa62ae41591418a73794dc8ac6dde



 Comments   
Comment by Omer Iqbal [ 12/Mar/13 4:08 PM ]

added an implementation for the equiv method if both args are java.util.regex.Pattern

Comment by Andy Fingerhut [ 12/Mar/13 5:54 PM ]

Omer, could you also include in your patch a few test cases? At least one that validates that two regex's that should be equal, are equal, and another that two regex's that should be different, are non-equal. Preferably the first of those tests should fail with the existing Clojure code, and pass with your changes.

Comment by Omer Iqbal [ 13/Mar/13 5:39 AM ]

I updated the patch with some tests. Hope I added them in the correct file. I also changed the implementation a bit, by instead of adding another implementation of equiv with a different signature, checking the type of the Object in the equiv method with signature (Object k1, Object k2).
For the sake of consistency I also added an EquivPred implementation, though I'm not entirely sure when its used.

Comment by Andy Fingerhut [ 13/Mar/13 1:04 PM ]

All comments here refer to the patch named fix-CLJ-1182.diff dated Mar 13, 2013.

The location for the new tests looks reasonable. However, note that your new patch has your old changes plus some new ones, not just the new ones. In particular, the new signature for equiv is still in your latest patch. You should start from a clean pull of the latest Clojure master and make only the changes you want when creating a patch, not build on top of previous changes you have made.

Also, there are several whitespace-only changes in your patch that should not be included.

Comment by Omer Iqbal [ 13/Mar/13 1:39 PM ]

I uploaded a clean patch, removing the whitespace diff and only adding the latest changes. Thanks for clarifying the workflow!
Just to clarify, this refers to the patch named fix-CLJ-1182.diff dated Mar 13 1:34 PM

Comment by Stuart Halloway [ 29/Mar/13 5:46 AM ]

I am recategorizing this as an enhancement, because if this is a bug it is a bug in Java – the Java Patterns class documents being immutable, but apparently does not implement .equals.

Other recent "make Clojure more complicated to work around problems in Java" patches have been rejected, and I suspect this one will be too, because it might impact the performance of equality everywhere.

Comment by Stuart Halloway [ 12/Apr/13 9:04 AM ]

At first pass, Rich and I both believe that, as regex equality is undecidable, that Java made the right choice in using identity for equality, that this ticket should be declined, and the ClojureScript should revert to match.

But leaving this ticket open for now so that ClojureScript dev can weigh in.

Comment by Michael Drogalis [ 12/Apr/13 9:32 AM ]

What do you mean when you say "undecidable"?

Comment by Alexander Redington [ 12/Apr/13 10:03 AM ]

If Regex instances were interned by the reader, but still used identity for equality, then code like

(= #"abc" #"abc")

would return true, but it wouldn't diverge in the definition of equality for regular expressions between Java and Clojure.

Comment by Fogus [ 12/Apr/13 10:13 AM ]

Undecidable means that for any given regular expression, there is no single way to write it. For example #"[a]" #"a" both match the same strings, but are they the same? Maybe. But how can we decide if /any/ given regular expression matches all of the same strings as any other? The answer is, you can't. Java does provide a Pattern#pattern method that returns the string that was used to build it, but I'm not sure if that could/should be used as a basis for equality given the potential perf hit.

Comment by Herwig Hochleitner [ 12/Apr/13 10:31 AM ]

I posted in Stu's thread: https://groups.google.com/d/topic/clojure-dev/OTPNJQbPtds/discussion
TL;DR: Disagree with undecidability, agree with reverting to identity based equality

Comment by Michael Drogalis [ 12/Apr/13 10:32 AM ]

That makes sense to me. Thanks Fogus.

Comment by Herwig Hochleitner [ 12/Apr/13 9:42 PM ]

From my post to the ml thread, it might not be entirely clear, why I don't think we should implement equality for host regexes:

It would involve parsing and would leave a lot of room for errors and platform-idiosycracies to leak. And it would be different for every platform.

As Alexander said, I also think this ticket could be resolved by interning regex literals, akin to keywords. That, however, would warrant a design page first, because there are tradeoffs to be made about how far the interning should go.

Comment by Rich Hickey [ 13/Apr/13 8:51 AM ]

Why are we spending time on this? Where is the problem statement? Does someone actually need this for a real world purpose not solved by using regex strings as keys?

Comment by Michael Drogalis [ 13/Apr/13 9:13 PM ]

It was prompted as a matter of consistency, which I think is valid. I can't think of a good reason to use regex's as keys though.





[CLJ-1181] clojure.pprint/code-dispatch breaks on certain types of anonymous functions Created: 10/Mar/13  Updated: 18/Mar/13

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

Type: Defect Priority: Major
Reporter: Devin Walters Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None


 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: 10/Mar/13

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: None

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)






[CLJ-1177] clojure.java.io/resource and non-ASCII characters Created: 07/Mar/13  Updated: 10/Mar/13

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: 1
Labels: bug, enhancement

Attachments: Text File clj-1177-patch-v1.txt    
Patch: Code

 Description   

clojure.java.io/resource corrupts path containing UTF-8 characters without issuing warning.

user=> (System/getProperty "java.runtime.version")
"1.8.0-ea-b79"
user=> (clojure-version)
"1.5.0"
user=> (System/getProperty "user.dir")
"/dir/déf"
user=> (clojure.java.io/resource "myfile.txt")
#<URL file:/dir/d%c3%a9f/resources/myfile.txt>
user=> (slurp (clojure.java.io/resource "myfile.txt") :encoding "UTF-8")
FileNotFoundException /dir/déf/resources/myfile.txt (No such file or directory)  java.io.FileInputStream.open (FileInputStream.java:-2)


 Comments   
Comment by Andy Fingerhut [ 08/Mar/13 12:10 AM ]

Below is a workaround, at least. I don't know, but perhaps the as-file method for URLs in io.clj of Clojure, the part that converts %hh sequences to a character with code point in the range 0 through 255, is at least partly at fault here. I don't know right now if it is possible to modify that code to handle the general case of whatever character encoding munging is going on here to when .getResource creates the URL object.

clojure.java.io/resource is documented to return a Java object of type java.net.URL, which seems like it does %hh escaping of many characters. Reference [1] to a Java bug from 2001 where a Java user was surprised by the then-recent change in behavior of the getResource method [2].

Doing a little searching I found this StackOverflow question [3], which has what might be a workaround. I tried it on my Mac OS X 10.6 system running JDK 1.6 and it seemed to work:

(slurp (.getContent (clojure.java.io/resource "abcíd/foo.txt")))

That getContent is a method for class java.net.URL [4]

[1] http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4466485
[2] http://docs.oracle.com/javase/1.5.0/docs/api/java/lang/Class.html#getResource%28java.lang.String%29
[3] http://stackoverflow.com/questions/13013629/best-international-alternative-to-javas-getclass-getresource
[4] http://docs.oracle.com/javase/1.5.0/docs/api/java/net/URL.html#getContent%28%29

Comment by Trevor Wennblom [ 08/Mar/13 9:56 AM ]

Hi Andy,

Thanks for the background and suggestions, that's very helpful.

I'm gradually learning Clojure with no Java experience. In this case I was searching for the preferred Clojure way to access items in directories declared under :resource-paths in a Leiningen project.clj file. Perhaps clojure.java.io/resource isn't the best way to do this as it's possibly too tied to the expectation for a URI instead of a more general IRI.

You're suggested workaround did work for my use case:

(slurp (.getContent (clojure.java.io/resource "abcíd/foo.txt")))

but hopefully there would be more native/direct Clojure way to accomplish the same eventually.

I don't know if java.net.IDN would be useful internally as a fix in clojure.java.io/resource — I'm assuming not since it wasn't added until Java 6.[1]

user=> (import 'java.net.IDN)
java.net.IDN
user=> (java.net.IDN/toASCII "/dir/déf")
"xn--/dir/df-gya"
user=> (java.net.IDN/toUnicode "xn--/dir/df-gya")
"/dir/déf"

[1]: http://docs.oracle.com/javase/6/docs/api/java/net/IDN.html

Comment by Andy Fingerhut [ 08/Mar/13 1:30 PM ]

Patch clj-1177-patch-v1.txt dated Mar 8 2013 is an attempt to solve this issue, in what I think may be a correct way. As specified in RFC 3986, when taking a Unicode string and making a URL of it, it should be encoded in UTF-8 and then each individual byte is subject to the %HH hex encoding. This patch reverses that to turn URLs into file names.

Tested on Mac OS X 10.6 with a command line like this (it doesn't work without the -Dfile.encoding=UTF-8 option on my Mac, probably because the default encoding is MacRoman):

% java -cp clojure.jar:path/to/resource -Dfile.encoding=UTF-8 clojure.main
user=> (require '[clojure.java.io :as io])
nil
user=> (io/resource "abcíd/foo.txt")
#<URL file:/Users/jafinger/clj/clj-ns-browser/resource/abc%c3%add/foo.txt>
user=> (slurp (io/resource "abcíd/foo.txt"))
"The quick brown fox jumped over the lázy dög!\n"





[CLJ-1176] clojure.repl/source fails when *read-eval* bound to :unknown Created: 06/Mar/13  Updated: 24/May/13

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

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

Attachments: Text File 0001-CLJ-1176-Bind-read-eval-true-in-clojure.repl-source-.patch    
Patch: Code
Approval: Screened

 Description   

clojure.repl/source is broken in Clojure 1.5.0 when *read-eval* is bound to :unknown, since source-fn reads without binding.

Reproduce:

  1. Either set
    :jvm-opts ["-Dclojure.read.eval=unknown"]
    in Leiningen or eval at REPL:
    (alter-var-root #'*read-eval* (constantly :unknown))
  2. (use 'clojure.repl)
  3. (source drop-last)

Expected:

Source of drop-last.

Actual:

RuntimeException Reading disallowed - *read-eval* bound to :unknown



 Comments   
Comment by Tim McCormack [ 06/Mar/13 4:04 PM ]

The attached patch just binds *read-eval* to true inside source-fn.

Comment by Stuart Halloway [ 29/Mar/13 6:24 AM ]

Note: Allowing this implies that you trust the data on your classpath. If there are reasons somebody might not, we should reject this patch and people will have to be explicit when calling source.

Comment by Tim McCormack [ 29/Mar/13 6:37 AM ]

Ugh, that's a fair point when it comes to sandboxing. I'll check with the owners of clojurebot and lazybot.

Comment by Tim McCormack [ 04/May/13 10:40 PM ]

I haven't come up with any scenarios where this is problematic, and I haven't heard anything back from the bot owners. As for sandboxing, clojure.repl can easily be excluded.

Comment by Gabriel Horner [ 24/May/13 9:42 AM ]

Looks good





[CLJ-1175] NPE in clojure.lang.Delay/deref Created: 06/Mar/13  Updated: 24/May/13

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

Type: Defect Priority: Minor
Reporter: Alan Malloy Assignee: Stuart Halloway
Resolution: Unresolved Votes: 3
Labels: None

Attachments: Text File delayed-exceptions.patch    
Patch: Code and Test
Approval: Screened

 Description   

Summary: Delays get into a corrupt state if an exception thrown, allowing deref to behave differently on back-to-back calls. Patch causes Delay to rethrow the original exception, includes tests.

If a Delay wraps a function which throws an exception, then forcing that delay multiple times causes behavior which is, to me at least, surprising and unexpected: the first time it is forced, the expected exception is thrown; but after that it will behave as if all locals the expression refers to are nil. This can manifest in multiple ways, depending on the expression being delayed:

;; calling a local as a function causes an NPE inside clojure.lang.Delay
(let [f #(/ 1 0) d (delay (f))]
  [(try (force d) 
        (catch Exception e e))
   (try (force d)
        (catch Exception e e))])
[#<ArithmeticException java.lang.ArithmeticException: Divide by zero> 
 #<NullPointerException java.lang.NullPointerException>]
;; if nil is a valid value, this can cause subsequent forces to "succeed"
;; even though the first fails as it should
(let [x (java.util.Date.) d (delay (seq x))]
  [(try (force d) 
        (catch Exception e e))
   (try (force d)
        (catch Exception e e))])
[#<IllegalArgumentException java.lang.IllegalArgumentException: Don't know how to create ISeq from: java.util.Date> 
 nil]

The reason for this is that clojure.core/delay creates a ^:once function, promising to call it only once, so that the compiler can do locals-clearing on its lexical closure. However, when an exception is thrown the Delay object is left thinking it has never called the function, so when it is forced again it calls the function again, breaking its promise to the compiler and causing the function to run after its locals have been cleared.

In fact, once we realize that locals-clearing is involved, we can make the delay behave differently the first N times it is forced, instead of only the first time, by constructing an expression which throws an exception before using all of its locals:

(let [x (java.util.Date.) 
            y (Long. 1)
            d (delay (let [y (seq y)]
                       (cons (seq x) y)))]
  [(try (force d) 
        (catch Exception e e))
   (try (force d)
        (catch Exception e e))
   (try (force d)
        (catch Exception e e))])
[#<IllegalArgumentException java.lang.IllegalArgumentException: Don't know how to create ISeq from: java.lang.Long> 
 #<IllegalArgumentException java.lang.IllegalArgumentException: Don't know how to create ISeq from: java.util.Date>
 (nil)]

I'm not sure what the right fix for this issue is: perhaps the best choice is even to leave it alone, and just document that delay's behavior is undefined if the expression throws an exception. However, I propose making the Delay remember the first exception that was thrown, and rethrow it on subsequent force attempts. This makes sense, in a way: the "result" of the expression was this exception e being thrown, and so this should happen every time. It might be preferable to have the Delay retry the expression until it succeeds, but I don't believe this is possible without abandoning locals-clearing, which would cause perfectly-valid delays to now run out of memory, holding onto no-longer-needed locals just in case the expression needs to retry at some later date.






[CLJ-1172] Cross-linking between clojure.lang.Compiler and clojure.lang.RT Created: 28/Feb/13  Updated: 23/Mar/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()





[CLJ-1171] Compiler macro for clojure.core/instance? disregards lexical shadows on class names Created: 27/Feb/13  Updated: 24/May/13

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

Type: Defect Priority: Major
Reporter: Herwig Hochleitner Assignee: Stuart Halloway
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File 0001-CLJ-1171-Tests-for-clojure.core-instance-compiler-ma.patch     Text File 0002-CLJ-1171-Obey-lexical-scope-for-class-argument-in-in.patch     Text File 0003-CLJ-1171-Check-arity-in-instance-compiler-macro.patch    
Patch: Code and Test
Approval: Screened

 Description   

Summary

The compiler tries to emit jvm native instanceof expressions for direct clojure.core/instance? calls.
For that, it tries to resolve its first argument as a class name. However, it disregards lexical bindings when doing that.
This is incongruent to the default implementation in core.clj

Patches

[Stu] All three patches should be applied IMO.

  • 0002 makes instance? respect lexical bindings
  • 0003 makes instance?'s compiled form check arity, consistent with higher-order behavior
  • 0001 has a minimal test for both 0002 and 0003.

Data

Test case

user=> (let [Long String] (instance? Long "abc"))
false
;; expected true as in
user=> (let [Long String] (apply instance? [Long "abc"]))
true

Culprit method

https://github.com/clojure/clojure/blob/4ccb10edbe66eae81336a4c0972050d9759b0bf7/src/jvm/clojure/lang/Compiler.java#L3578

List Discussion

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

Tangent

This was discovered because the same compiler macro also omits the arity check implicit in the default definition. This could also conveniently be fixed when touching that method:

user=> (instance? String)
false
;; expected
user=> (apply instance? [String])
ArityException Wrong number of args (1) passed to: core$instance-QMARK-  clojure.lang.AFn.throwArity (AFn.java:437)

EDIT elaborated on ticket title and description; added tangent



 Comments   
Comment by Herwig Hochleitner [ 27/Feb/13 8:11 PM ]

Attached patches test and fix issue + tangent

Comment by Herwig Hochleitner [ 04/Mar/13 3:51 PM ]

Note: Patch 0003 just adds the arity check, hence is optional, but if it's omitted from the patchset, the corresponding test from patch 0001 will fail.

Comment by Stuart Halloway [ 29/Mar/13 7:31 AM ]

Summarizing the decisions in these patches:

  • instance? and apply instance? should be consistent
  • they should check arity (matching apply instance? existing behavior)
  • they should allow lexical shadowing of the class argument (matching apply instance? existing behavior)

It is possible (although unlikely) that existing code relies on the current eccentric behavior of instance?. I think it would be fair to categorize programs relying on this behavior as buggy, but that is easy for me to say.





[CLJ-1169] Add filename and line number to defn parameter declaration error Created: 22/Feb/13  Updated: 10/Mar/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: Andrei Kleschinski Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None
Environment:

Windows


Attachments: Text File 0001-CLJ-1169-proposed-patch.patch     Text File 0002-CLJ-1169-fix-unit-tests.patch     File defn_error_message.clj    
Patch: Code

 Description   

When mistyping parameter list in defn declaration, e.g.

(defn test
(some-error))

error message shows name of parameter (without quotes), but not function name, filename or line number:

Exception in thread "main" java.lang.IllegalArgumentException: Parameter declaration some-error should be a vector
at clojure.core$assert_valid_fdecl.invoke(core.clj:6567)
at clojure.core$sigs.invoke(core.clj:220)
at clojure.core$defn.doInvoke(core.clj:294)
at clojure.lang.RestFn.invoke(RestFn.java:467)
at clojure.lang.Var.invoke(Var.java:427)
at clojure.lang.AFn.applyToHelper(AFn.java:172)
at clojure.lang.Var.applyTo(Var.java:532)
at clojure.lang.Compiler.macroexpand1(Compiler.java:6366)
at clojure.lang.Compiler.macroexpand(Compiler.java:6427)
at clojure.lang.Compiler.eval(Compiler.java:6495)
at clojure.lang.Compiler.load(Compiler.java:6952)
at clojure.lang.Compiler.loadFile(Compiler.java:6912)
at clojure.main$load_script.invoke(main.clj:283)
at clojure.main$init_opt.invoke(main.clj:288)
at clojure.main$initialize.invoke(main.clj:316)
at clojure.main$null_opt.invoke(main.clj:349)
at clojure.main$main.doInvoke(main.clj:427)
at clojure.lang.RestFn.invoke(RestFn.java:421)
at clojure.lang.Var.invoke(Var.java:419)
at clojure.lang.AFn.applyToHelper(AFn.java:163)
at clojure.lang.Var.applyTo(Var.java:532)
at clojure.main.main(main.java:37)



 Comments   
Comment by Andrei Kleschinski [ 22/Feb/13 5:39 AM ]

Proposed patch for issue
Process exceptions in macroexpand1 and wraps them in CompilerException with source,line,column information.

Also patch adds quotes around invalid symbol name in error message to make them more distinguishable from rest of message.

Comment by Andy Fingerhut [ 01/Mar/13 9:32 AM ]

Patch 0001-CLJ-1169-proposed-patch.patch dated Feb 22 2013 causes several tests to fail. Run "./antsetup.sh" then "ant" to see which ones (or "mvn package").

Comment by Andrei Kleschinski [ 01/Mar/13 10:25 AM ]

Fix for failed unit-tests





[CLJ-1167] repl value of *file* is "NO_SOURCE_PATH", of *source-path* is "NO_SOURCE_FILE" Created: 19/Feb/13  Updated: 19/Feb/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: None


 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-1165] Forbid varargs defprotocol/definterface method declarations because those cannot be defined anyway Created: 15/Feb/13  Updated: 04/Apr/13

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

Type: Enhancement Priority: Major
Reporter: Tassilo Horn Assignee: Stuart Halloway
Resolution: Unresolved Votes: 0
Labels: enhancement, patch

Attachments: Text File 0001-Protocol-interface-method-declarations-don-t-allow-f.patch    
Patch: Code

 Description   

Protocol, interface method declarations don't allow for varags. Currently, for example

  (defprotocol FooBar
    (foo [this & more]))

compiles just fine, and & is interpreted as a usual argument that happens to be
named & without special meaning. But clearly, the user wanted to specify a
varags parameter here. The same applies to definterface.

Similarly, providing method implementations via defrecord, deftype, and reify
don't allow for varags (but dynamic extensions via extend do).

So this patch makes defprotocol and definterface throw an
IllegalArgumentException if a user tries to use varargs in method signatures.

Similarly, defrecord, deftype, and reify throw an IllegalArgumentException if
any method implementation arglist contains a varargs argument.

This patch is a cut-down variant of my patch to http://dev.clojure.org/jira/browse/CLJ-1024
which has been reverted shortly before Clojure 1.5 was released. The CLJ-1024 patch
was the same as this one, but it has also forbidden destructuring in defprotocol and
definterface. This was a bit too much, because although destructuring has no
semantic meaning with method declarations, it still can serve a documentation purpose.

This has been discussed on the list: https://groups.google.com/d/topic/clojure-dev/qjkW-cv8nog/discussion



 Comments   
Comment by Stuart Halloway [ 29/Mar/13 5:27 AM ]

I think that this patch would be much more helpful to users if it reported the problem form (both name and params).

(And I wonder if we should be using ex-info for all errors going forward.)

Comment by Tassilo Horn [ 31/Mar/13 5:17 AM ]

New version of the patch that mentions both method name and argument vector, and uses ex-info as Stu suggested.

Comment by Andy Fingerhut [ 04/Apr/13 7:24 PM ]

Presumuptuously changing Approval from Incomplete back to None, since the reason for marking it Incomplete seems to have been addressed with a new patch.





[CLJ-1162] Error Message when calling deref on a non-IDeref is unhelpful Created: 12/Feb/13  Updated: 28/May/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: None
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-1161] sources jar has bad versions.properties resource Created: 11/Feb/13  Updated: 24/May/13

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

Type: Defect Priority: Minor
Reporter: Steve Miner Assignee: Stuart Halloway
Resolution: Unresolved Votes: 0
Labels: patch

Attachments: Text File 0001-CLJ-1161-Remove-version.properties-from-sources-JAR.patch    
Patch: Code
Approval: Screened

 Description   

Summary: patch leaves version.properties file out of sources JAR, where it causes problems for tools.

The "sources" jar (at least since Clojure 1.4 and including 1.5 RC) has a bad version.properties file in it. The resource clojure/version.properties is literally:

version=${version}

The regular Clojure jar has the correct version string in that resource.

I came across a problem when I was experimenting with the sources jar (as used by IDEs). I naively added the sources jar to my classpath, and Clojure died on start up. The bad clojure/versions.properties file was found first, which led to a parse error as the clojure version was being set.



 Comments   
Comment by Steve Miner [ 11/Feb/13 10:04 AM ]

Notes from the dev mailing list:

The "sources" JAR is generated by another Maven plugin, configured here:
https://github.com/clojure/clojure/blob/clojure-1.5.0-RC15/pom.xml#L169-L181

The simplest solution might be to just exclude the file from the sources jar. It looks like maven-source-plugin has an excludes option which would do the trick:

http://maven.apache.org/plugins/maven-source-plugin/jar-mojo.html#excludes





[CLJ-1160] reducers/mapcat ignores Reduced Created: 11/Feb/13  Updated: 01/Mar/13

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

Type: Defect Priority: Major
Reporter: Christophe Grand Assignee: Christophe Grand
Resolution: Unresolved Votes: 1
Labels: None

Attachments: File lazy-rmapcat.diff    
Patch: Code and Test

 Description   

The following code throws an exception:

(->> (concat (range 100) (lazy-seq (throw (Exception. "Too eager"))))
(r/mapcat (juxt inc str))
(r/take 5)
(into []))

This is because r/mapcat introduces an intermediate reduce which swallows the reduced value coming from r/take.






[CLJ-1159] clojure.java.io/delete-file doesn't return the status of the deletion(true/false) Created: 10/Feb/13  Updated: 10/Feb/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: delete-file, evil, or
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-1157] Classes generated by gen-class aren't loadable from remote codebase for mis-implementation of static-initializer Created: 04/Feb/13  Updated: 12/Apr/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: Tsutomu Yano Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Tested on Mac OS X 10.8 and Oracle JVM 1.7.0 update 13.


Attachments: File 20130204_fix_classloader.diff    
Patch: Code
Approval: Vetted

 Description   

When a client program uses a remote service which uses RMI, and the service returns a object which created with gen-class with clojure as the return value, the return value is not loadable at client side.

At client side, a following exeption will be thrown.

Exception in thread "main" java.lang.ExceptionInInitializerError
        at java.io.ObjectStreamClass.hasStaticInitializer(Native Method)
        at java.io.ObjectStreamClass.computeDefaultSUID(ObjectStreamClass.java:1723)
        at java.io.ObjectStreamClass.access$100(ObjectStreamClass.java:69)
        at java.io.ObjectStreamClass$1.run(ObjectStreamClass.java:247)
        at java.io.ObjectStreamClass$1.run(ObjectStreamClass.java:245)
        at java.security.AccessController.doPrivileged(Native Method)
        at java.io.ObjectStreamClass.getSerialVersionUID(ObjectStreamClass.java:244)
        at java.io.ObjectStreamClass.initNonProxy(ObjectStreamClass.java:600)
        at java.io.ObjectInputStream.readNonProxyDesc(ObjectInputStream.java:1601)
        at java.io.ObjectInputStream.readClassDesc(ObjectInputStream.java:1514)
        at java.io.ObjectInputStream.readOrdinaryObject(ObjectInputStream.java:1750)
        at java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1347)
        at java.io.ObjectInputStream.readObject(ObjectInputStream.java:369)
        at sun.rmi.server.UnicastRef.unmarshalValue(UnicastRef.java:324)
        at sun.rmi.server.UnicastRef.invoke(UnicastRef.java:173)
        at java.rmi.server.RemoteObjectInvocationHandler.invokeRemoteMethod(RemoteObjectInvocationHandler.java:194)
        at java.rmi.server.RemoteObjectInvocationHandler.invoke(RemoteObjectInvocationHandler.java:148)
        at $Proxy0.getResult(Unknown Source)
        at client.SampleClient$_main.doInvoke(SampleClient.clj:12)
        at clojure.lang.RestFn.invoke(RestFn.java:397)
        at clojure.lang.AFn.applyToHelper(AFn.java:159)
        at clojure.lang.RestFn.applyTo(RestFn.java:132)
        at client.SampleClient.main(Unknown Source)
 Caused by: java.io.FileNotFoundException: Could not locate remoteserver/SampleInterfaceImpl__init.class or remoteserver/SampleInterfaceImpl.clj on classpath: 
        at clojure.lang.RT.load(RT.java:434)
        at clojure.lang.RT.load(RT.java:402)
        at clojure.core$load$fn__5039.invoke(core.clj:5520)
        at clojure.core$load.doInvoke(core.clj:5519)
        at clojure.lang.RestFn.invoke(RestFn.java:408)
        at clojure.lang.Var.invoke(Var.java:415)
        at remoteserver.SampleInterfaceImpl.<clinit>(Unknown Source)
        ... 23 more

HOW TO REPRODUCT THIS ISSUE

If you want to see this issue at your computer, clone my example project from my github.

git clone git://github.com/tyano/clojure_genclass_fix.git

and build them (You must have installed Leiningen 2):

cd clojure_genclass_fix
sh build.sh

start rmiregistry:

rmiregistry &

start remoteserver:

cd remoteserver
sh start.sh

You will see a message "Server ready. " or "Server ready. (rebind)".

At last, start client program:

cd ../client
sh start.sh

Without my patch, you will see a same Exception described above. But with clojure with my patch, you will see a right response message: "response = this is sample."

THE REASON

The reason of this problem is in bytecodes generated by gen-class. A gen-classed class (in this case, SampleInterfaceImpl.class) uses a static-initializer for loading SampleInterfaceImpl__init.class (which load other classes which implements functions in the class). The static-initializer is like bellow: (the following code is decompiled with JD - http://java.decompiler.free.fr/?q=jdgui )

static
  {
    RT.var("clojure.core", "load").invoke("/remoteserver/SampleInterfaceImpl");
  }

Very simple code. it seems non-problematic. But RT.load changes the classloader for loading __init.class in the processing! RT.load in default uses a context-classloader for loading classes. But all classes depending on a gen-classed class must be loaded a same classloader with a main gen-classed class. In this case, RT.load must use a remote URLClassLoader which load a main class.

So, gen-class must be create bytecodes that is same with the following java code.

static
  {
    Var.pushThreadBindings(RT.map(new Object[] { Compiler.LOADER, SampleInterfaceImpl.class.getClassLoader() }));
    try {
        RT.var("clojure.core", "load").invoke("/remoteserver/SampleInterfaceImpl");
    }
    finally 
    {
        Var.popThreadBindings();
    }
  }

With this code, RT.load will uses a same classloader which load SampleInterfaceImpl.class.
My patch for gen-class will create bytecodes equal to the above example.

You can use an attached patch '20130204_fix_classloader.diff', or pull 'fix_classloader' branch from my github repositry ( git@github.com:tyano/clojure.git ).



 Comments   
Comment by Stuart Halloway [ 01/Mar/13 10:20 AM ]

This sounds reasonable, but anything touching classloaders must be considered very carefully.

Comment by Stuart Halloway [ 01/Mar/13 12:12 PM ]

It seems overly complex to have the patch do so much code generation. Why not implement a method that does this job, and have the generated code call that?





[CLJ-1154] Compile.java closes out preventing java from reporting exceptions Created: 31/Jan/13  Updated: 12/Apr/13

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

Type: Defect Priority: Major
Reporter: Robert (Bobby) Evans Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None

Approval: Vetted

 Description   

I was trying to compile a project that has some native dependencies. I am using clojure-maven-plugin version 1.3.13 with Maven 2.0. I forgot to set java.library.path properly so that the native library could be found, and only got an error of

[INFO] ------------------------------------------------------------------------
[ERROR] BUILD ERROR
[INFO] ------------------------------------------------------------------------
[INFO] Clojure failed.
[INFO] ------------------------------------------------------------------------

I traced this down to Compile.java, where it is flushing and closing

out
in the finally block. The JVM uses out to write out a stack trace for any uncaught exceptions. When out is closed it is unable to write out the stack trace for the UnsatisfiedLinkError that was being thrown. This made it very difficult to debug what was happening.



 Comments   
Comment by Stuart Halloway [ 01/Mar/13 10:45 AM ]

I have encountered this problem as well. Did not verify the explanation, but sounds reasonable.





[CLJ-1152] PermGen leak in multimethods and protocol fns Created: 30/Jan/13  Updated: 30/Jan/13

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

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


 Description   

There is a PermGen memory leak that we have tracked down to protocol methods and multimethods called inside an eval, because of the caches these methods use. The problem only arises when the value being cached is an instance of a class (such as a function or reify) that was defined inside the eval. Thus extending IFn or dispatching a multimethod on an IFn are likely triggers.

My fellow LonoClouder, Jeff Dik describes how to reproduce and work around the problem:

The easiest way that I have found to test this is to set "-XX:MaxPermSize" to a reasonable value so you don't have to wait too long for the PermGen space to fill up, and to use "-XX:+TraceClassLoading" and "-XX:+TraceClassUnloading" to see the classes being loaded and unloaded.

leiningen project.clj
(defproject permgen-scratch "0.1.0-SNAPSHOT"
  :dependencies [[org.clojure/clojure "1.5.0-RC1"]]
  :jvm-opts ["-XX:MaxPermSize=32M"
             "-XX:+TraceClassLoading"
             "-XX:+TraceClassUnloading"])

You can use lein swank 45678 and connect with slime in emacs via M-x slime-connect.

To monitor the PermGen usage, you can find the Java process to watch with "jps -lmvV" and then run "jstat -gcold <PROCESS_ID> 1s". According to the jstat docs, the first column (PC) is the "Current permanent space capacity (KB)" and the second column (PU) is the "Permanent space utilization (KB)". VisualVM is also a nice tool for monitoring this.

Multimethod leak

Evaluating the following code will run a loop that eval's (take* (fn foo [])).

multimethod leak
(defmulti take* (fn [a] (type a)))

(defmethod take* clojure.lang.Fn
  [a]
  '())

(def stop (atom false))
(def sleep-duration (atom 1000))

(defn run-loop []
  (when-not @stop
    (eval '(take* (fn foo [])))
    (Thread/sleep @sleep-duration)
    (recur)))

(future (run-loop))

(reset! sleep-duration 0)

In the lein swank session, you will see many lines like below listing the classes being created and loaded.

[Loaded user$eval15802$foo__15803 from __JVM_DefineClass__]
[Loaded user$eval15802 from __JVM_DefineClass__]

These lines will stop once the PermGen space fills up.

In the jstat monitoring, you'll see the amount of used PermGen space (PU) increase to the max and stay there.

-    PC       PU        OC          OU       YGC    FGC    FGCT     GCT
 31616.0  31552.7    365952.0         0.0      4     0    0.000    0.129
 32000.0  31914.0    365952.0         0.0      4     0    0.000    0.129
 32768.0  32635.5    365952.0         0.0      4     0    0.000    0.129
 32768.0  32767.6    365952.0      1872.0      5     1    0.000    0.177
 32768.0  32108.2    291008.0     23681.8      6     2    0.827    1.006
 32768.0  32470.4    291008.0     23681.8      6     2    0.827    1.006
 32768.0  32767.2    698880.0     24013.8      8     4    1.073    1.258
 32768.0  32767.2    698880.0     24013.8      8     4    1.073    1.258
 32768.0  32767.2    698880.0     24013.8      8     4    1.073    1.258

A workaround is to run prefer-method before the PermGen space is all used up, e.g.

(prefer-method take* clojure.lang.Fn java.lang.Object)

Then, when the used PermGen space is close to the max, in the lein swank session, you will see the classes created by the eval'ing being unloaded.

[Unloading class user$eval5950$foo__5951]
[Unloading class user$eval3814]
[Unloading class user$eval2902$foo__2903]
[Unloading class user$eval13414]

In the jstat monitoring, there will be a long pause when used PermGen space stays close to the max, and then it will drop down, and start increasing again when more eval'ing occurs.

-    PC       PU        OC          OU       YGC    FGC    FGCT     GCT
 32768.0  32767.9    159680.0     24573.4      6     2    0.167    0.391
 32768.0  32767.9    159680.0     24573.4      6     2    0.167    0.391
 32768.0  17891.3    283776.0     17243.9      6     2   50.589   50.813
 32768.0  18254.2    283776.0     17243.9      6     2   50.589   50.813

The defmulti defines a cache that uses the dispatch values as keys. Each eval call in the loop defines a new foo class which is then added to the cache when take* is called, preventing the class from ever being GCed.

The prefer-method workaround works because it calls clojure.lang.MultiFn.preferMethod, which calls the private MultiFn.resetCache method, which completely empties the cache.

Protocol leak

The leak with protocol methods similarly involves a cache. You see essentially the same behavior as the multimethod leak if you run the following code using protocols.

protocol leak
(defprotocol ITake (take* [a]))

(extend-type clojure.lang.Fn
  ITake
  (take* [this] '()))

(def stop (atom false))
(def sleep-duration (atom 1000))

(defn run-loop []
  (when-not @stop
    (eval '(take* (fn foo [])))
    (Thread/sleep @sleep-duration)
    (recur)))

(future (run-loop))

(reset! sleep-duration 0)

Again, the cache is in the take* method itself, using each new foo class as a key.

A workaround is to run -reset-methods on the protocol before the PermGen space is all used up, e.g.

(-reset-methods ITake)

This works because -reset-methods replaces the cache with an empty MethodImplCache.



 Comments   
Comment by Chouser [ 30/Jan/13 9:10 AM ]

I think the most obvious solution would be to constrain the size of the cache. Adding an item to the cache is already not the fastest path, so a bit more work could be done to prevent the cache from growing indefinitely large.

That does raise the question of what criteria to use. Keep the first n entries? Keep the n most recently used (which would require bookkeeping in the fast cache-hit path)? Keep the n most recently added?





[CLJ-1151] Minor Code Cleanup in core.reducers: use required walk, drop this for coll Created: 21/Jan/13  Updated: 21/Jan/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.






[CLJ-1149] Unhelpful error message from :use (and use function) when arguments are malformed Created: 17/Jan/13  Updated: 26/May/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: Sean Corfield Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

the following exception happens when you have something like this(bad):

(ns runtime.util-test
(:use [midje.sweet :reload-all]))

as opposed to any of these(correct):

(ns runtime.util-test
(:use midje.sweet :reload-all))

(ns runtime.util-test
(:use [midje.sweet] :reload-all))

and the exception is:
Exception in thread "main" java.lang.IllegalArgumentException: No value supplied for key: true
at clojure.lang.PersistentHashMap.create(PersistentHashMap.java:77)
at clojure.core$hash_map.doInvoke(core.clj:365)
at clojure.lang.RestFn.applyTo(RestFn.java:137)
at clojure.core$apply.invoke(core.clj:617)
at clojure.core$load_lib.doInvoke(core.clj:5352)
at clojure.lang.RestFn.applyTo(RestFn.java:142)
at clojure.core$apply.invoke(core.clj:619)
at clojure.core$load_libs.doInvoke(core.clj:5403)
at clojure.lang.RestFn.applyTo(RestFn.java:137)
at clojure.core$apply.invoke(core.clj:621)
at clojure.core$use.doInvoke(core.clj:5497)

Note that this is similar to the equally unhelpful message shown in http://dev.clojure.org/jira/browse/CLJ-1140 although that is a different root cause.

Probably best to enhance the `use` function to validate its arguments before trying to apply hash-map?



 Comments   
Comment by Gary Fredericks [ 26/May/13 3:17 PM ]

I believe this applies to require as well.





[CLJ-1148] adds docstring support to defonce, and stops it from stomping existing metadata Created: 17/Jan/13  Updated: 17/Jan/13

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

Type: Enhancement Priority: Minor
Reporter: Joe Gallo Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None

Attachments: Text File 0001-new-defonce-hotness.patch    
Patch: Code

 Description   

Two issues here:

1) defonce doesn't support docstrings, although def does, so it would be nice to bring the two in line with each other

2) defonce can stomp metadata, like this:

(defonce ^:private foo 5)
#'user/foo
(meta #'foo) --> the private is there
(defone foo 10)
foo is still 5
(meta #'foo) --> the private has been lost






[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: 13/Jan/13

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: 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.






[CLJ-1142] Incorrect divide-by-zero error with floating point numbers Created: 08/Jan/13  Updated: 08/Jan/13

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

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


 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: 07/Jan/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: 1
Labels: None
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: 21/Dec/12

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: Compiler, bug
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-1134] star-directive in clojure.pprint/cl-format with an at-prefix ("~n@*") do not obey its specifications Created: 18/Dec/12  Updated: 26/Dec/12

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: Jean Niklas L'orange Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: bug, pprint

Attachments: Text File clj-1134-star-directive-in-cl-format.txt    
Patch: Code and Test

 Description   

The star-directive in clojure.pprint/cl-format with an at-prefix (~n@*) does not obey its specifications according to Common Lisp the Language, 2nd Edition. There are two bugs within ~n@* as of right now:

  1. When ~n@* is supposed to jump forward over more than one argument, it jumps one step backward as if it had seen ~:*. For instance, (cl-format nil "~D ~3@*~D" 0 1 2 3) will return "0 0" and not "0 3" as expected.
  2. When ~@* is seen, the formatter is supposed to jump to the first argument (as n defaults to 0, see specification linked above). However, whenever a ~@*-directive is seen, the formatter jumps to the second argument instead.

What (small set of) steps will reproduce the problem?

Inside a clean Clojure repl, perform these steps:

user=> (require '[clojure.pprint :refer [cl-format]])
nil
user=> (cl-format nil "~D ~3@*~D" 0 1 2 3)
"0 0"                                           ;; Expected: "0 3"
user=> (cl-format nil "~D~D~D~D ~@*~D" 0 1 2 3)
"0123 1"                                        ;; Expected: "0123 0"

What is the expected output? What do you see instead?

The expected output is "0 3" and "0123 0", but is "0 0" and "0123 1" as shown above.

What version are you using?

Tested on both 1.4.0 and 1.5.0-beta2, both have the defect described.

Please provide any additional information below.

The format strings which reproduces the problem has been compared with the format function from the Common Lisp implementations SBCL, CLisp and Clozure. All of them print the expected output.



 Comments   
Comment by Jean Niklas L'orange [ 18/Dec/12 9:28 PM ]

Patch attached.

It may be easier to read the changes the patch does from within JIRA instead from the commit message, so I've added it here:

This solves two issues as specified by #CLJ-1134. Issue #1 is solved by doing a
relative jump forward within absolute-reposition in cl_format.clj, line 114 by
switching (- (:pos navigator) position) with (- position (:pos navigator)).

Issue #2 is handled by changing the default n-parameter to * depending on
whether the @-prefix is placed or not. If it is placed, then n defaults to
0, otherwise it defaults to 1.

In addition, new tests have been appended to test_cl_format.clj to ensure the
correctness of this patch. The tests have been tested on the Common Lisp
implementation GNU CLISP 2.49, which presumably handle the ~n@*
correctly. This patch and GNU CLISP returns the same output for each format
call, sans case for printed symbols; Common Lisp has case-insensitive symbols,
whereas Clojure has not.





[CLJ-1133] Certain actions on mutable fields in deftype lead to very strange error messages Created: 18/Dec/12  Updated: 19/Dec/12

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: None
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: 18/Dec/12

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: constructor, 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: 17/May/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: feedback


 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-1130] Invoking a static method with the wrong number of arguments results in a NoSuchFieldException, rather than a proper message that the arguments could not be matched to the method Created: 17/Dec/12  Updated: 06/Jan/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: feedback


 Description   

My code inadventently invoked a method that expected a single parameter, but passed no parameters.

The exception:

Exception in thread "main" java.lang.NoSuchFieldException: getService, compiling:(com/annadaletech/nexus/services/registry.clj:168)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6462)
	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$LetExpr$Parser.parse(Compiler.java:5883)
....
Caused by: java.lang.NoSuchFieldException: getService
	at java.lang.Class.getField(Class.java:1520)
	at clojure.lang.Compiler$StaticFieldExpr.<init>(Compiler.java:1180)
	at clojure.lang.Compiler$HostExpr$Parser.parse(Compiler.java:923)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6455)
	... 78 more

That threw me for a bit; really, it looks like the compiler is attempting to access a field and invoke it as an IFn. However, a proper message would be "getService() requires 1 parameter, not 0" (or something to that effect). Even "invalid number of parameters for SmashRuntime/getService()" would be better.



 Comments   
Comment by Michael Drogalis [ 06/Jan/13 6:44 PM ]

It looks like it's first trying to resolve a field by name, since field access with / is legal. For example:

user=> (Integer/parseInt)
CompilerException java.lang.NoSuchFieldException: parseInt, compiling:(NO_SOURCE_PATH:1)

user=> (Integer/MAX_VALUE)
2147483647

Would trying to resolve a method before a field fix this?





[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-1125] Clojure can leak memory when used in a servlet container Created: 11/Dec/12  Updated: 14/Jun/13

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

Type: Defect Priority: Major
Reporter: Toby Crawley Assignee: Stuart Halloway
Resolution: Unresolved Votes: 9
Labels: None

Attachments: File threadlocal-removal-tcrawley-2012-12-11.diff     File threadlocal-removal-tcrawley-2013-06-14.diff    
Patch: Code
Approval: Vetted

 Description   

When used within a servlet container
(Jetty/Tomcat/JBossAS/Immutant/etc), the thread locals Var.dvals (used
to store dynamic bindings) and LockingTransaction.transaction (used to
store the currently active transaction(s)) prevent all of the classes
loaded by an application's clojure runtime from being garbage collected,
resulting in a memory leak.

The issue comes from threads living beyond the lifetime of a
deployment - servlet containers use thread pools that are shared
across all applications within the container. Currently, the dvals and
transaction thread locals are not discarded when they are no longer
needed, causing their contents to retain a hard reference to their
classloaders, which, in turn, causes all of the classes loaded under
the application's classloader to be retained until the thread exits
(which is generally at JVM shutdown).

I've attached a patch that does the following:

  • Var.dvals is now removed when the thread bindings are popped
  • Var.dvals no longer has an initialValue, so checking to see if it is
    set will no longer set it to an empty Frame
  • The outer transaction in LockingTransaction.transaction now removes
    the thread local when it is finished

There is still the opportunity for memory leaks if agents or futures
are used, and the executors used for them are not shutdown when the
app is undeployed. That's a solvable problem, but should probably be
solved by the containers themselves (and/or the war generation tools)
instead of in clojure itself.

This patch has a small performance impact: its use of a try/finally
around running transactions to remove the outer transaction adds
4-6 microseconds to each transaction call on my hardware.

Providing an automated test for this patch is difficult - I've tested
it locally with repeated deployments to a container while monitoring
GC and permgen. All of clojure's tests pass with it applied.

The above is a condensation of:
https://groups.google.com/d/topic/clojure-dev/3CXDe8_9G58/discussion

I'm happy to provide whatever feedback/work is needed to get this
applied.



 Comments   
Comment by Colin Jones [ 13/May/13 7:30 PM ]

This patch works great for me to avoid OOM/PermGen crashes from classloaders being retained [mine is a non-servlet use case].

Comment by Stuart Halloway [ 24/May/13 9:43 AM ]

Does Tomcat create warnings for Clojure, as described e.g. here?

If so, does this patch make the warnings go away?

Comment by Toby Crawley [ 24/May/13 9:56 AM ]

Stu: that's a good question. I'll take a look at Tomcat this afternoon.

Comment by Stuart Halloway [ 24/May/13 10:04 AM ]

The code that calls transaction.remove() seems unncessarily subtle. There are two exits from the method, and only one is protected by the finally block.

If the "outer" case was a top-level if, the logic would be more clear, and only the "outer" case would need try/finally, which might reduce the performance penalty in the case of deeply nested dosyncs.

Did your transaction overhead of 4-6 microseconds test only one level of dosync, or many?

Comment by Stuart Halloway [ 24/May/13 10:13 AM ]

Because the unwind code calls remove at the top (as opposed to set(null)), the code should now be safe for use with Clojure-defined ThreadLocal subclasses.

Therefore, Var's use of an initialValue should be irrelevant to this patch, and it should be possible to fix this bug with a patch half the size of the current patch, touching only LockingTransaction.runInTransaction and Var.popThreadBindings.

Comment by Toby Crawley [ 14/Jun/13 7:38 AM ]

re: Tomcat ThreadLocal warnings

With Clojure 1.5.1 using my test app (linked below), I see:

Jun 14, 2013 6:35:22 AM org.apache.catalina.loader.WebappClassLoader checkThreadLocalMapForLeaks
SEVERE: The web application [/leak] created a ThreadLocal with key of type [clojure.lang.Var$1] (value [clojure.lang.Var$1@4902919]) and a value of type [clojure.lang.Var.Frame] (value [clojure.lang.Var$Frame@147a2aa6]) but failed to remove it when the web application was stopped. Threads are going to be renewed over time to try and avoid a probable memory leak.
Jun 14, 2013 6:35:22 AM org.apache.catalina.loader.WebappClassLoader checkThreadLocalMapForLeaks
SEVERE: The web application [/leak] created a ThreadLocal with key of type [java.lang.ThreadLocal] (value [java.lang.ThreadLocal@608602ca]) and a value of type [clojure.lang.LockingTransaction] (value [clojure.lang.LockingTransaction@7e214d47]) but failed to remove it when the web application was stopped. Threads are going to be renewed over time to try and avoid a probable memory leak.

With the original patch (threadlocal-removal-tcrawley-2012-12-11.diff) and the one attached today (threadlocal-removal-tcrawley-2013-06-14.diff), I no longer see these warnings.

re: the LockingTransaction.runInTransaction changes

In today's patch (threadlocal-removal-tcrawley-2013-06-14.diff), I modified runInTransaction to have one exit point, and only wrap a call to run with a try/finally in the outer transaction case. It does introduce two locations where run can be called to preserve the case where an inner transaction has null info:

static public Object runInTransaction(Callable fn) throws Exception{
	LockingTransaction t = transaction.get();
        Object ret;
	if(t == null) {
            transaction.set(t = new LockingTransaction());
            try {
                ret = t.run(fn);
            } finally {
                transaction.remove();
            }
        } else {
            if(t.info != null) {
                ret = fn.call();
            } else {
                ret = t.run(fn);
            }
        }

        return ret;
}

However, this will likely not reduce the speed penalty I observed in my testing, as I was only using a single level of dosync when capturing timing data.

re: removing initialValue from dvals

My original solution kept initialValue, but I then apparently discovered cases where the leak still occurred (see the mailing list thread).

Unfortunately, I can neither recreate that case, nor find in my notes, test code, or the clojure code a reason why keeping initialValue would allow the ThreadLocals to leak when popThreadBindings is patched (assuming one doesn't call Var.getThreadBindings from Java without calling Var.popThreadBindings).

Therefore, I've attached a simpler patch (threadlocal-removal-tcrawley-2013-06-14.diff) that just patches LockingTransaction.runInTransaction and Var.popThreadBindings.

I've also created a project that demonstrates the leak with 1.5.1, and that the leak does not appear with this patch applied to 1.6.0-master. See its README for usage details.

The patched version of 1.6.0-master is available as [org.clojars.tcrawley/clojure "1.6.0-clearthreadlocals"] if anyone wants to give it a try in their own projects. Note that since its group isn't 'org.clojure', you may need to add exclusions to your project to prevent another version of clojure being included.

Comment by Andy Fingerhut [ 14/Jun/13 10:56 AM ]

Presumptuously changing ticket approval from Incomplete back to its former Vetted state, since Toby's comments and new patch seem to address the comments that led Stu to change it to Incomplete.





[CLJ-1124] for-as Created: 10/Dec/12  Updated: 10/Dec/12

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

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


 Description   

A common pattern in programming is building up some data structure step by step:

In Python:

[code]
x = {0: 1}
for item in stuff:
x[item] = item * x.get(item - 1, 0)
[/code]
etc.

In an imperative for loop this is easy since we have easy access to the "current" data structure being built up.

I propose the addition of a function for-as similar to as-> except the value of the last loop iteration is bound to the name.

So we can write the above as:

[code]
(last (for-as [x {0 1}]
[item stuff]
(assoc x item (* item (get x (- item 1) 0)))))
[/code]

An (un-optimized) implementation might be something like:

[code]
(defmacro reduce-for [[res init] for-seq-exprs body-expr]
`(reduce #(%2 %1) ~init
(for ~for-seq-exprs
(fn [~res]
~body-expr))))
[/code]

Note: reduce-for does not return a seq, instead it returns the result of the last loop body iteration.



 Comments   
Comment by Yongqian Li [ 10/Dec/12 11:31 PM ]

(Fixed formatting)

x = {0: 1}
for item in stuff:
    x[item] = item * x.get(item - 1, 0)
(last (for-as [x {0 1}]
        [item stuff]
  (assoc x item (* item (get x (- item 1) 0)))))
(defmacro reduce-for [[res init] for-seq-exprs body-expr]
  `(reduce #(%2 %1) ~init
    (for ~for-seq-exprs
      (fn [~res]
        ~body-expr))))

(Someone please fix the formatting in above and delete this comment.)





[CLJ-1122] Add contributing.md file to github repository (shows clear message on issues/pull request create form) Created: 09/Dec/12  Updated: 24/May/13

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

Type: Enhancement Priority: Minor
Reporter: Max Penet Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File contributing.patch     Text File contributing-v2.patch    
Patch: Code
Approval: Screened

 Description   

This adds a clear message when someone wants to create a pull request/issue and invites the user to read the contribution guidelines: see https://github.com/blog/1184-contributing-guidelines.

The same thing could be done for all the clojure/* repositories.

The content of the file is just a markdown version of http://clojure.org/contributing

Preview here: https://github.com/mpenet/clojure/blob/aef170ca5eca1b71a2eb1ef320223d1277df0e5e/CONTRIBUTING.md



 Comments   
Comment by Stuart Halloway [ 02/Jan/13 6:31 AM ]

Please change this to link to the definitive prose, so we don't have to maintain that in two places.

Comment by Max Penet [ 02/Jan/13 6:51 AM ]

Feel free to correct the wording, I used something simple.

Comment by Gabriel Horner [ 17/May/13 9:04 AM ]

Stu, he linked to clojure.org as you requested so I'm moving this along.





[CLJ-1121] -> and ->> have unexpected behavior when combined with unusual macros Created: 06/Dec/12  Updated: 24/May/13

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

Type: Defect Priority: Minor
Reporter: Gary Fredericks Assignee: Stuart Halloway
Resolution: Unresolved Votes: 4
Labels: None

Attachments: Text File 0001-CLJ-1121-Reimplement-and-without-recursion.patch     Text File CLJ-1121-v002.patch    
Patch: Code and Test
Approval: Screened

 Description   

My intuitive understanding of the classic threading macros is that the meaning of forms like (-> a b c) can be understood syntactically independent of the meaning of the symbols involved or the fact that the two threading macros are defined recursively. However the recursive definition breaks that expectation. After

(macroexpand-1 (macroexpand-1 '(-> a b c)))

=> (c (-> a b))

c is now in control if it is a macro, and is now seeing the argument (-> a b) rather than (b a) as would be the case if we had written (c (b a)) originally.

Admittedly I do not know of a realistic example where this is an important distinction (I noticed this when playing with a rather perverse use of ->> with macros from korma), but at the very least it means that the behavior of the threading macros isn't quite as easy to accurately explain as I thought it was.



 Comments   
Comment by Gary Fredericks [ 07/Dec/12 9:19 AM ]

I just realized that my patch also implements CLJ-1086.

Comment by Stuart Halloway [ 22/Dec/12 9:48 AM ]

Would be nice if tests also demonstrated that metadata is preserved correctly.

Comment by Gary Fredericks [ 26/Dec/12 8:16 PM ]

New patch in response to stuarthalloway feedback.

Comment by Tim McCormack [ 09/Jan/13 6:47 PM ]

This patch also prevents an infinite loop in the macroexpander when fed the following expression:

(->> a b (->> c d))

Edit: Far simpler example.





[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: 16/Dec/12

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.





[CLJ-1118] inconsistent numeric comparison semantics between BigDecimal and other Numerics Created: 30/Nov/12  Updated: 25/Apr/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: Arthur Ulfeldt Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File clj-1118-make-double-equals-true-for-more-bigdecimals-patch-v3.txt    
Patch: Code and Test

 Description   

user> clojure-version
{:major 1, :minor 5, :incremental 0, :qualifier "beta1"}
user> (== 2.0 2.0M)
true
user> (== 2 2.0M)
false <-- this one is not like the others
user> (== 2 2.0)
true
user> (== 2N 2.0)
true
user> (== 2 (double 2.0M))
true

It's not clear if this is a bug or an enhancement request, Should BigDecimal's be special in comparason to their smaller equivalents?



 Comments   
Comment by Arthur Ulfeldt [ 30/Nov/12 1:51 PM ]

I understand that the definition of equality between bigDecimals is dependent on both value and scale as in this case:

user> (== 0.000000M 0.0M)
false

I just want to make sure the decission to propagate that semantic across types is intentional. If this is on purpose than this is not a bug.

Comment by Arthur Ulfeldt [ 30/Nov/12 2:03 PM ]

this could be fixed by calling stripTrailingZeros on bigDecimals before comparing them to Longs or BigInts.

(== 2 (double (. 2.0M stripTrailingZeros)))
true

Edited by Andy Fingerhut: Unfortunately that fails for BigDecimal values equal to 0, unless they happen to have a scale that matches what you are comparing it to.

I think a more complete solution is to use BigDecimal's compareTo method, e.g.:

(zero? (.compareTo 2.0M (bigdec 2)))
true

Comment by Timothy Baldridge [ 03/Dec/12 11:31 AM ]

It seems we need some more eyes on this issue, can you bring this up on clojure-dev and see what they think?

Comment by Andy Fingerhut [ 14/Apr/13 4:03 AM ]

Patch clj-1118-make-double-equals-true-for-more-bigdecimals-patch-v1.txt dated Apr 14 2013 changes equiv for BigDecimals so that instead of using BigDecimal.equals(), it uses BigDecimal.compareTo() and checks the return value is equal to 0.

The Java docs for these methods explicitly state that BigDecimal.equals() will treat values that are otherwise equal numerically, but differ in scale, as not equal.

They also say that BigDecimal.compareTo() will return 0 for such BigDecimals.

I'm not sure if this is the preferred behavior for Clojure, but if it is, this patch should do it.

Comment by Andy Fingerhut [ 15/Apr/13 12:18 AM ]

clj-1118-make-double-equals-true-for-more-bigdecimals-patch-v2.txt dated Apr 14 2013 is same as clj-1118-make-double-equals-true-for-more-bigdecimals-patch-v1.txt described in previous comment, except it also has some new tests included.

Comment by Andy Fingerhut [ 15/Apr/13 9:07 PM ]

clj-1118-make-double-equals-true-for-more-bigdecimals-patch-v3.txt dated Apr 15 2013 is the same as the the previous patch clj-1118-make-double-equals-true-for-more-bigdecimals-patch-v2.txt, except for the following:

By changing == behavior for BigDecimal by modifying the BigDecimalOps.equiv() method, that also changes the behavior of = when comparing BigDecimal values to other numbers. hash should be consistent with =, so now hash should return same value for all numerically equal BigDecimal values. This patch should achieve that.





[CLJ-1117] Partition does not follow docs Created: 29/Nov/12  Updated: 17/May/13

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

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

OS X, 10.8


Attachments: Text File clj-1117.patch    
Patch: Code

 Description   

The doc for partition states "In case there are not enough padding elements, return a partition with less than n items."

However, the behavior of this function is as follows:

user=> (partition 3 (range 10))
((0 1 2) (3 4 5) (6 7 8))
user=> (partition 4 (range 10))
((0 1 2 3) (4 5 6 7))
user=> (partition 5 (range 10))
((0 1 2 3 4) (5 6 7 8 9))

Either the doc should be updated to make it clear that not providing a pad will mean that items are dropped, or the functionality of partition should be fixed to the following:

user=> (partition 3 (range 10))
((0 1 2) (3 4 5) (6 7 8) (9))



 Comments   
Comment by Andy Fingerhut [ 29/Nov/12 2:15 PM ]

That would be a potentially breaking change for some people's code that uses partition. partition-all behaves as you wish.

Also, your concern with the documentation is for when there are padding elements specified as an argument, but your examples don't specify any padding elements.

Comment by Timothy Baldridge [ 29/Nov/12 2:55 PM ]

I agree, but I think the docs should then explicitly state: "if no padding is given, not all input elements may be returned in the output partitions" or something to that line.

Comment by Andy Fingerhut [ 29/Nov/12 4:43 PM ]

More precise documentation of current behavior is always welcome in my opinion.

Comment by Gabriel Horner [ 17/May/13 10:14 AM ]

I've uploaded a patch that calls out when and how partition drops tail elements:
"If a pad collection is not supplied, any tail elements that remain from dividing the input collection length by n will not be included in a partition."





[CLJ-1115] multi arity into Created: 25/Nov/12  Updated: 09/Dec/12

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: 0
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.





[CLJ-1113] `reductions` reducer Created: 23/Nov/12  Updated: 23/Nov/12

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: None

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: 22/Nov/12

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: patch

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-1107] 'get' should throw exception on non-Associative argument Created: 13/Nov/12  Updated: 24/May/13

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: 1
Labels: None

Attachments: Text File 0001-CLJ-1107-Throw-exception-for-get-called-on-unsupport.patch     Text File clj-1107-throw-on-get-for-unsupported-types-patch-v2.txt    
Patch: Code and Test

 Description   

The implementation of clojure.core/get returns null if its argument is not a valid associative collection. However, calling 'get' on something which is neither nil nor an Associative collection is almost certainly a bug, and should be indicated by an exception.

This behavior can obscure common programmer errors such as:

(def a (atom {:a 1 :b 2})

(:foo a)   ; forgot to deref a
;;=> nil

CLJ-932 was accepted as a similar enhancement to 'clojure.core/contains?'

Attached patch 0001 throws an IllegalArgumentException as the fall-through case of RT.getFrom.



 Comments   
Comment by Andy Fingerhut [ 24/May/13 12:31 PM ]

Patch clj-1107-throw-on-get-for-unsupported-types-patch-v2.txt dated May 24 2013 is identical to 0001-CLJ-1107-Throw-exception-for-get-called-on-unsupport.patch dated Nov 13 2012, except it applies cleanly to latest master. A recent commit for CLJ-1099 changed many IllegalArgumentException occurrences to Throwable in the tests, which is the only thing changed in this updated patch.





[CLJ-1105] defrecord classes implement IPersistentCollection but not .empty, clojure.walk assumes collections support empty Created: 08/Nov/12  Updated: 12/Apr/13

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

Type: Defect Priority: Minor
Reporter: Jouni K. Seppänen Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None

Approval: Vetted

 Description   

Using clojure.walk functions fails surprisingly for data containing records defined with defrecord:

user=> (defrecord Foo [x])
user.Foo
user=> (def f (Foo. :x))
#'user/f
user=> (use 'clojure.walk)
nil
user=> (postwalk identity {:foo f})
UnsupportedOperationException Can't create empty: user.Foo user.Foo (NO_SOURCE_FILE:1)

This seems to be because clojure.walk/walk guards a call to (empty form) with a (coll? form) check. The check succeeds because records implement IPersistentCollection, but (empty form) throws an exception. This looks to me like a bug in clojure.walk (it should check records separately and either treat them as atomic or implement a way of walking through them) but perhaps it is a sign of some unclarity in the contract of collections.



 Comments   
Comment by Nicola Mometto [ 08/Nov/12 2:35 PM ]

maybe clojure should follow clojurescript's footsteps and move empty out of IPersistentCollection and create an
interface IEmptyableCollection extends IPersistentCollection { IEmptyableCollection empty(); }

Comment by Stuart Halloway [ 25/Nov/12 6:39 PM ]

Can whoever claims this please consider walk's behavior in the face of all different collection types? I think it also fails with Java collections.

Also, the collection partitioning code in clojure.data may be of use.





[CLJ-1104] Concurrent with-redefs do not unwind properly, leaving a var permanently changed Created: 07/Nov/12  Updated: 21/Dec/12

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

Type: Enhancement Priority: Minor
Reporter: Jason Wolfe Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Mac OS, Java 6


Attachments: Text File clj-1104-doc-unsafety-of-concurrent-with-redefs-v1.txt    
Patch: Code
Approval: Vetted

 Description   

On 1.4 and latest master:

user> (defn ten [] 10)
#'user/ten
user> (doall (pmap #(with-redefs [ten (fn [] %)] (ten)) (range 20 100)))
(20 21 22 23 24 25 34 27 28 29 30 31 32 33 34 35 36 37 38 39 39 35 42 43 44 45 48 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 79 87 88 89 90 91 92 93 94 95 97 92 98 99)
user> (ten)
79

Not sure if this is a bug per se, but the doc doesn't mention lack of concurrency safety and my expectation was that the original value would always be restored after any (arbitrarily interleaved) sequence of with-redefs calls.



 Comments   
Comment by Tim McCormack [ 07/Nov/12 8:50 PM ]

The with-redefs doc (v1.4.0) says "These temporary changes will be visible in all threads." That sounds non-thread-safe to me.

In general, changes to var root bindings are not thread safe.

Comment by Herwig Hochleitner [ 08/Nov/12 9:17 AM ]

As I understand it, with-redefs is mainly used in test suites to mock out vars. It was introduced when vars became static by default and a lot of testsuites had been using binding for mocking. Maybe the docstring should be amended with something along the lines of: When using this you have to ensure that only a single thread is interacting with redef'd vars.

Comment by Stuart Halloway [ 25/Nov/12 6:41 PM ]

Behavior find as is, doc string change would be fine.

Comment by Andy Fingerhut [ 25/Nov/12 6:57 PM ]

Patch clj-1104-doc-unsafety-of-concurrent-with-redefs-v1.txt dated Nov 25 2012 updates doc string of with-redefs to make it clear that concurrent use is unsafe.





[CLJ-1103] Make conj assoc dissoc and transient versions handle args similarly Created: 04/Nov/12  Updated: 09/Dec/12

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: Text File clj-1103-make-conj-assoc-dissoc-handle-args-similarly-v1.txt    
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





[CLJ-1102] Better handling of exceptions with empty stack traces Created: 04/Nov/12  Updated: 30/Nov/12

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: 0
Labels: None

Attachments: Text File clj-1102-improve-empty-stack-trace-handling-v1.txt    
Patch: Code
Approval: Vetted

 Description   

I don't know what I did to cause some exceptions to be thrown while running Clojure tests that return a length 0 array from (.getStackTrace throwable), but according to the Java docs this is legal. I searched all places in the Clojure code that call .getStackTrace and found two that don't handle this correctly, one of which causes an ArrayOutOfBoundsException (that is the one I found during my testing).



 Comments   
Comment by Andy Fingerhut [ 04/Nov/12 5:07 PM ]

clj-1102-improve-empty-stack-trace-handling-v1.txt dated Nov 4 2012 improves the handling of .getStackTrace returning a length 0 array in two places. I checked all other places .getStackTrace was called and they seem to already handle this case gracefully.

Comment by Timothy Baldridge [ 30/Nov/12 2:57 PM ]

Vetting.





[CLJ-1100] Reader literals cannot contain periods Created: 02/Nov/12  Updated: 14/Feb/13

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

Type: Defect Priority: Major
Reporter: Kevin Lynagh Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: reader

Approval: Vetted

 Description   

The LispReader tries to read a record instead of a literal if the tag contains periods:

https://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/LispReader.java#L1171

Which effectively means that reader tags cannot contain periods.
The EDN spec is unclear on this:

edn supports extensibility through a simple mechanism. # followed immediately by a symbol starting with an alphabetic character indicates that that symbol is a tag.

(issue opened: https://github.com/edn-format/edn/issues/39)

If periods are allowed, then the LispReader should first check to see if the tag is in *data-readers* and only then if not try to initialize a Java class.

I'm happy to write the patch if this behavior is what is desired.



 Comments   
Comment by Steve Miner [ 06/Nov/12 9:41 AM ]

The suggested patch (clj-1100-reader-literal-periods.patch) will break reading records when *default-data-reader-fn* is set. Try adding a test like this:

(deftest tags-containing-periods-with-default
      ;; we need a predefined record for this test so we (mis)use clojure.reflect.Field for convenience
      (let [v "#clojure.reflect.Field{:name \"fake\" :type :fake :declaring-class \"Fake\" :flags nil}"]
        (binding [*default-data-reader-fn* nil]
          (is (= (read-string v) #clojure.reflect.Field{:name "fake" :type :fake :declaring-class "Fake" :flags nil})))
        (binding [*default-data-reader-fn* (fn [tag val] (assoc val :meaning 42))]
          (is (= (read-string v) #clojure.reflect.Field{:name "fake" :type :fake :declaring-class "Fake" :flags nil})))))
Comment by Rich Hickey [ 29/Nov/12 9:36 AM ]

The problem assessment is ok, but the resolution approach may not be. What happens should be based not upon what is in data-readers but whether or not the name names a class.

Is the intent here to allow readers to circumvent records? I'm not in favor of that.

Comment by Steve Miner [ 29/Nov/12 4:01 PM ]

New patch following Rich's comments. The decision to read a record is now based on the symbol containing periods and not having a namespace. Otherwise, it is considered a data reader tag. User
defined tags are required to be qualified but they may now have periods in the name. Tests added to show that
data readers cannot override record classes. Note: Clojure-defined data reader tags may be unqualified, but they should not contain periods in order to avoid confusion with record classes.

Comment by Steve Miner [ 29/Nov/12 4:17 PM ]

I deleted my old patch and some comments referring to it to avoid confusion.

In Clojure 1.5 beta 1, # followed by a qualified symbol with a period in the name is considered a record and causes an exception for the missing record class. With the patch, only non-qualified symbols containing periods are considered records. That allows user-defined qualified symbols with periods in their names to be used as data reader tags.

Comment by Andy Fingerhut [ 07/Feb/13 9:05 AM ]

clj-1100-periods-in-data-reader-tags-patch-v2.txt dated Feb 7 2013 is identical to CLJ-1100-periods-in-data-reader-tags.patch dated Nov 29 2012, except it applies cleanly to latest master. The only change appears to be in some white space in the context lines.

Comment by Andy Fingerhut [ 07/Feb/13 12:53 PM ]

I've removed clj-1100-periods-in-data-reader-tags-patch-v2.txt mentioned in the previous comment, because I learned that CLJ-1100-periods-in-data-reader-tags.patch dated Nov 29 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-1100-periods-in-data-reader-tags.patch

I've already updated the JIRA workflow and screening patches wiki pages to mention this --ignore-whitespace option.

Comment by Andy Fingerhut [ 13/Feb/13 11:31 AM ]

Both of the current patches, CLJ-1100-periods-in-data-reader-tags.patch dated Nov 29 2012, and clj-1100-reader-literal-periods.patch dated Nov 6 2012, fail to apply cleanly to latest master (1.5.0-RC15) as of today, although they did last week. Given all of the changes around read / read-string and edn recently, they should probably be evaluated by their authors to see how they should be updated.

Comment by Steve Miner [ 14/Feb/13 12:23 PM ]

I deleted my patch: CLJ-1100-periods-in-data-reader-tags.patch. clj-1100-reader-literal-periods.patch is clearly wrong, but the original author or an administrator has to delete that.

Comment by Kevin Lynagh [ 14/Feb/13 1:28 PM ]

I cannot figure out how to remove my attachment (clj-1100-reader-literal-periods.patch) in JIRA.

Comment by Steve Miner [ 14/Feb/13 1:43 PM ]

Downarrow (popup) menu to the right of the "Attachments" section. Choose "manager attachments".

Comment by Kevin Lynagh [ 14/Feb/13 2:02 PM ]

Great, thanks Steve. Are you going to take another pass at this issue, or should I give it a go?

Comment by Steve Miner [ 14/Feb/13 3:04 PM ]

Kevin, I'm not planning to work on this right now as 1.5 is pretty much done. It might be worthwhile discussing the issue a bit on the dev mailing list before working on a patch, but that's up to you. I think my approach was correct, although now changes would have to be applied to both LispReader and EdnReader.





[CLJ-1097] node-seq for clojure.zip Created: 29/Oct/12  Updated: 29/Oct/12

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: clojure.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 destrucring emit direct keyword lookups Created: 29/Oct/12  Updated: 26/Jan/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: None

Attachments: File desctructure-keyword-lookup.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






[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-1094] Zero-arity versions of every-pred and some-fn Created: 25/Oct/12  Updated: 25/Oct/12

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

Type: Enhancement Priority: Minor
Reporter: Tassilo Horn Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: patch, test

Attachments: Text File 0001-Add-zero-arity-variants-for-every-pred-and-some-fn.patch    
Patch: Code and Test

 Description   

This patch adds zero-arity versions of every-pred and some-fn with these semantics.

(every-pred) === (constantly true)
(some-fn)    === (constantly nil)

These variants are useful in situations like the following:

;; compute-preds-for may return zero or many predicate fns
(let [preds (compute-preds-for something)]
  (filter (apply every-pred preds) some-coll))


 Comments   
Comment by Tassilo Horn [ 25/Oct/12 7:12 AM ]

This is the thread where Max Penet suggested to have 0-arity versions of the two fns:

https://groups.google.com/forum/?fromgroups=#!topic/clojure/IRlN-4LH_U0





[CLJ-1093] Empty record literals gets incorrectly evaluated to array-maps Created: 24/Oct/12  Updated: 13/Mar/13

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

Type: Defect Priority: Major
Reporter: Nicola Mometto Assignee: Timothy Baldridge
Resolution: Unresolved Votes: 2
Labels: None

Attachments: Text File 001-fix-empty-record-literal.patch     Text File clj-1093-fix-empty-record-literal-patch-v2.txt    
Patch: Code and Test

 Description   

before patch:
user=> (defrecord a [b])
user.a
user=> #user.a{}
{}

after patch:
user=> (defrecord a [b])
user.a
user=> #user.a{}
#user.a{:b nil}



 Comments   
Comment by Timothy Baldridge [ 27/Nov/12 11:41 AM ]

Unable to reproduce this bug on latest version of master. Most likely fixed by some of the recent changes to data literal readers.

Marking Not-Approved.

Comment by Timothy Baldridge [ 27/Nov/12 11:41 AM ]

Could not reproduce in master.

Comment by Nicola Mometto [ 01/Mar/13 1:23 PM ]

I just checked, and the problem still exists for records with no arguments:

Clojure 1.6.0-master-SNAPSHOT
user=> (defrecord a [])
user.a
user=> #user.a[]
{}

Admittedly it's an edge case and I see little usage for no-arguments records, but I think it should be addressed aswell since the current behaviour is not what one would expect

Comment by Herwig Hochleitner [ 02/Mar/13 8:14 AM ]

Got the following REPL interaction:

% java -jar ~/.m2/repository/org/clojure/clojure/1.5.0/clojure-1.5.0.jar
user=> (defrecord a [])
user.a
user=> (a.)
#user.a{}
user=> #user.a{}
{}
#user.a[]
{}

This should be reopened or declined for another reason than reproducability.

Comment by Nicola Mometto [ 10/Mar/13 2:18 PM ]

I'm reopening this since the bug is still there.

Comment by Andy Fingerhut [ 13/Mar/13 2:04 PM ]

Patch clj-1093-fix-empty-record-literal-patch-v2.txt dated Mar 13, 2013 is identical to Bronsa's patch 001-fix-empty-record-literal.patch dated Oct 24, 2012, except that it applies cleanly to latest master. I'm not sure why the older patch doesn't but git doesn't like something about it.





[CLJ-1090] Indirect function calls through Var instances fail to clear locals Created: 22/Oct/12  Updated: 30/Nov/12

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

Type: Defect Priority: Minor
Reporter: Spencer Tipping Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: performance
Environment:

Probably all, but observed on Ubuntu 12.04, OpenJDK 6


Attachments: File var-clear-locals.diff    
Patch: Code
Approval: Vetted

 Description   

If you make a function call indirectly by invoking a Var object (which derefs itself and invokes the result), the invocation parameters remain in the thread's local stack for the duration of the function call, even though they are needed only long enough to be passed into the deref'd function. As a result, passing a lazy seq into a function invoked in its Var form may run out of memory if the seq is forced inside that function. For example:

(defn total [xs] (reduce + 0 xs))
(total (range 1000000000)) ; this works, though takes a while
(#'total (range 1000000000)) ; this dies with out of memory error

I can provide a patch if it would be useful. The fix should be trivial, something along the lines of wrapping each argN in clojure/lang/Var.java inside a Util.ret1(argN, argN = null) as is done in RestFn.java.



 Comments   
Comment by Spencer Tipping [ 23/Oct/12 1:37 PM ]

Sorry, I typo'd the example. (defn total ...) should be (defn sum ...).

Comment by Timothy Baldridge [ 27/Nov/12 11:45 AM ]

fixed typeo in example

Comment by Timothy Baldridge [ 27/Nov/12 11:47 AM ]

Couldn't reproduce the exception, but the 2nd example did chew through about 4x the amount of memory. Vetting.

Comment by Timothy Baldridge [ 29/Nov/12 2:57 PM ]

adding a patch. Since most of Clojure ends up running this code in one way or another, I'd assert that tests are included as part of the normal Clojure test process.

Patch simply calls Util.ret1(argx, argx=null) on all invoke calls.

Comment by Timothy Baldridge [ 29/Nov/12 3:17 PM ]

And as a note, both examples in the original report now have extremely similar memory usages.

Comment by Spencer Tipping [ 30/Nov/12 2:22 PM ]

Sounds great, and the patch looks good too. Let me know if I need to do anything else.





[CLJ-1088] repl/source could support protocol functions Created: 21/Oct/12  Updated: 21/Oct/12

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: None

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.





[CLJ-1087] clojure.data/diff uses set union on key seqs Created: 15/Oct/12  Updated: 15/Oct/12

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: bug, 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-1086] Support arity-1 for ->> Created: 12/Oct/12  Updated: 12/Jan/13

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

Type: Enhancement Priority: Minor
Reporter: Shantanu Kumar Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: None
Environment:

Clojure and ClojureScript


Attachments: File thread-last-arity-1.diff    
Patch: Code

 Description   

Currently (as of Clojure 1.4) ->> doesn't support arity-1, though -> does. Arity-1 for ->> would be useful to let somebody comment out forms as follows:

(->> foo
  #_(bar baz)
  #_quux)

Discussion: https://groups.google.com/forum/?fromgroups=#!topic/clojure/_IVl0Tb7Au4

My name on contributor list page http://clojure.org/contributing is: Shantanu Kumar



 Comments   
Comment by Andy Fingerhut [ 08/Nov/12 1:37 PM ]

Shantanu, you give your name on the contributing page, but I don't see it there. Is your CA in transit, perhaps?

Comment by Shantanu Kumar [ 08/Nov/12 1:43 PM ]

@Andy I can see my name on the contributors page when I search for the text "Shantanu". My CA was submitted more than a year ago.

Comment by Andy Fingerhut [ 08/Nov/12 1:53 PM ]

Right you are. I somehow accidentally got my browser to enable case matching, and was expecting it to search case insensitively. Sorry for the noise.

Comment by Víctor M. Valenzuela [ 12/Jan/13 6:48 PM ]

Just for the record, the patch provided at http://dev.clojure.org/jira/browse/CLJ-1121 addresses this issue.





[CLJ-1083] Incorrect ArityException message for function names containing -> Created: 09/Oct/12  Updated: 07/Jun/13

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

Type: Defect Priority: Minor
Reporter: Alex Nixon Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: None

Attachments: File better-throw-arity-messages.diff    
Patch: Code
Approval: Vetted

 Description   

user=> (defn a->b [])
#'user/a->b
user=> (a->b 1)
ArityException Wrong number of args (1) passed to: user$a clojure.lang.AFn.throwArity (AFn.java:437)

Note that the reported function name in the stack trace is "user$a", where it should be "user$a->b" (or some mangled variant thereof?)

See discussion here: https://groups.google.com/d/msg/clojure/PVNoLclhhB0/_NWqyE0cPAUJ



 Comments   
Comment by Timothy Baldridge [ 26/Nov/12 10:27 AM ]

Fix for this defect.

Comment by Timothy Baldridge [ 26/Nov/12 10:30 AM ]

The throwArity now attempts to locate and call clojure.main/demunge. If it finds the function it invokes it and uses the returned string in the error. Otherwise it just throws the actual class name. This results in the following behaviour:

user=> (defn a->b [])
#'user/a->b
user=> (a->b 32)
ArityException Wrong number of args (1) passed to: user/a->b clojure.lang.AFn.throwArity (AFn.java:449)
user=>

Comment by Stuart Halloway [ 24/May/13 11:35 AM ]

Timothy: Why the empty catch block? I don't see anything in the try block whose failure we would want to ignore.

Comment by Timothy Baldridge [ 30/May/13 2:02 PM ]

The demunge function is created inside of a .clj file. So it is possible that we could hit this exception before demunge exists. The try simply says "if we can get a better error message, use it. Otherwise, fall back to the old (half-broken) method of getting method names"

Comment by Andy Fingerhut [ 07/Jun/13 2:01 PM ]

Presumptuously changing approval from Incomplete back to its former value of Vetted after Timothy responded to what I believe is the reason it was marked Incomplete (Stu's question).





[CLJ-1082] Subvecs of primitive vectors cannot be reduced Created: 05/Oct/12  Updated: 26/Jan/13

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
Environment:

1.7.0-08, OS X 10.8


Attachments: Text File clj-1082.patch    
Patch: Code and Test
Approval: Vetted

 Description   

I could reproduce on current 10/05 git.

Reduce doesn't work on subvecs of primitive vectors.
Root cause seems to be that RT/subvec and APersistentVector$SubVector make assumptions about implementation

If reduce on a subvec doesn't work then neither will nice ops like fold.

How to cause:

(let [prim-vec (into (vector-of :long) (range 10000))]
(reduce + (subvec prim-vec 1 500)))

->> ClassCastException clojure.core.Vec cannot be cast to clojure.lang.PersistentVector clojure.lang.APersistentVector$SubVector.iterator (APersistentVector.java:523)



 Comments   
Comment by Timothy Baldridge [ 27/Nov/12 11:52 AM ]

Confirmed to be broken on master. Vetting. Not sure what it's going to take to fix this, however. If this is of intrest for you, you might want to help push it along by providing a patch.

Comment by Andy Fingerhut [ 27/Nov/12 12:09 PM ]

There is no code or ticket for this yet, but I wanted to mention that I've begun working on an implementation of RRB-Tree (Relaxed Radix Balanced Tree) vectors for Clojure (see discussion at https://groups.google.com/forum/?fromgroups=#!topic/clojure-dev/xnbtzTVEK9A). Assuming it is no big deal to get reducers to work on such vectors, subvec could be implemented in O(log n) time in such a way that the result was the same concrete type of vector as you started with, and thus reducers would work on them, too.

It would mean O(log n) time for subvec instead of today's O(1), but this would likely fix other limitations that exist today with subvec's, e.g. CLJ-761.

Comment by Mike Anderson [ 20/Jan/13 5:14 AM ]

I have a fix for this and a regression test in the tree below:

https://github.com/mikera/clojure/tree/winfix

Not sure how best to make this into a patch though - I also have the pprint fix in here (CLJ-1076)

Comment by Mike Anderson [ 20/Jan/13 6:41 PM ]

Attached a patch that I created with:

git format-patch winfix --stdout HEAD~3..HEAD > clj-1082.patch

Does this do the trick? I had to use the HEAD~3..HEAD to just get the most recent 3 commits and exclude the pprint changes that I needed in order to build on Windows.

Comment by Mike Anderson [ 20/Jan/13 6:42 PM ]

Patch for CLJ-1082, containing 3 commits

Comment by Andy Fingerhut [ 21/Jan/13 1:11 AM ]

Mike, your patch clj-1082.patch applies cleanly to latest master for me, so looks like you found one way to do it.

Another would be as follows, and closer to the directions on the JIRA workflow page: http://dev.clojure.org/display/design/JIRA+workflow (but not identical). Note that these commands would work on Mac OS X or Linux. I'm not sure what the correct corresponding command would be on Windows for the "git am" step below, unless that just happens to work because Windows and/or git implement the input redirection with "<" somehow.

  1. Check out a fresh repo
    $ git clone git://github.com/clojure/clojure.git
    $ cd clojure
  1. Apply the patch for CLJ-1076 to the master branch. This step isn't on the JIRA workflow page.
    $ git am --keep-cr -s < clj-1076-fix-tests-on-windows-patch-v1.txt
  1. Create a branch for yourself
    $ git checkout -b fix-clj-1082
  1. After editing to make your changes, commit them to the current fix-clj-1082 branch
    $ git commit -a -m "fixed annoying bug, refs #42"

From there on down it is the same as the instructions on the JIRA workflow page. The "git format-patch master --stdout > file.patch" will create a patch for the changes you have made in the current branch fix-clj-1082 starting from the master branch, which has the CLJ-1076 fix because of the 'git am' command above.





[CLJ-1081] REPL binding not working that works with with-bindings Created: 30/Sep/12  Updated: 01/Oct/12

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

Type: Defect Priority: Major
Reporter: Steven Devijver Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 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: 07/Feb/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: None

Attachments: Text File clj-1080-eliminate-many-reflection-warnings-patch-v3.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.





[CLJ-1079] Don't squash explicit :line and :column metadata in the MetaReader Created: 29/Sep/12  Updated: 07/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: Major
Reporter: Chas Emerick Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None

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-1078] Added queue, queue* and queue? to clojure.core Created: 26/Sep/12  Updated: 31/May/13

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

Type: Enhancement Priority: Trivial
Reporter: Timothy Baldridge Assignee: Unassigned
Resolution: Unresolved Votes: 3
Labels: data-structures, queue

Attachments: File clj-1048-queue-takes-collections.diff     Text File queue.patch    
Patch: Code and Test

 Description   

Original patch added functions for PersistentQueue. queue, queue? and queue* match the list functions of the same naming conventions. Patches include updates to tests.

In the updated patch, "queue" accepts a single collection argument as per Rich's suggestion, and does not provide a "queue*".



 Comments   
Comment by Andy Fingerhut [ 28/Sep/12 8:43 AM ]

Timothy, I tried applying both of these Sep 26, 2012 patches to latest Clojure master as of that date. I had to apply 0001-make-PersistentQueue-ctor-public.patch by hand since it failed to apply using git or patch. It built fine, but failed to pass several of the Clojure tests. Have you looked into those test failures to see if you can find the cause and fix them? I tested on Ubuntu 11.10 with Oracle JDK 1.6 and 1.7, and saw similar failures with both.

Comment by Timothy Baldridge [ 26/Oct/12 5:23 PM ]

Fixed the patch. Tests pass, created the patch, applied it to a different copy of the source and the tests still pass. So this new patch should be good to go.

Comment by Andy Fingerhut [ 26/Oct/12 5:43 PM ]

Timothy, I'm not sure how you are getting successful results when applying this patch. Can you try the steps below and see what happens for you? I get errors trying to apply the patch with latest Clojure master as of Oct 26, 2012. Also please use the steps on the JIRA workflow page to create a git format patch (http://dev.clojure.org/display/design/JIRA+workflow under "Development" heading).

% git clone git://github.com/clojure/clojure.git
% cd clojure
% patch -p1 < queues.patch
patching file src/clj/clojure/core.clj
patching file src/jvm/clojure/lang/PersistentQueue.java
Hunk #1 FAILED at 32.
1 out of 1 hunk FAILED – saving rejects to file src/jvm/clojure/lang/PersistentQueue.java.rej
patching file test/clojure/test_clojure/data_structures.clj
Hunk #1 succeeded at 123 with fuzz 2.
Hunk #2 succeeded at 861 with fuzz 2.
Hunk #3 FAILED at 872.
1 out of 3 hunks FAILED – saving rejects to file test/clojure/test_clojure/data_structures.clj.rej
patching file test/clojure/test_clojure/java_interop.clj

Comment by Timothy Baldridge [ 26/Oct/12 6:08 PM ]

I was using git apply. I tried the method you show above, and now I'm seeing the same issues you show above.

Comment by Andy Fingerhut [ 26/Oct/12 6:26 PM ]

Just so you know, the preferred way to create and apply patches are the "git format-patch master --stdout > patch.txt" to create a patch (after doing the branching commands described on the JIRA workflow page to create a branch for your changes), and the "git am --keep-cr -s < patch.txt" to apply a patch. If a patch was created that way and applies cleanly with that command, then you are definitely good to go.

The "patch -p1 < patch.txt" command is just a secondary method sometimes used to try to apply patches that aren't in the format produced above, or have errors when applying using that method.

Comment by Timothy Baldridge [ 26/Oct/12 9:15 PM ]

Just so you know, the preferred way to create and apply patches are the "git format-patch master --stdout > patch.txt" to create a patch (after doing the branching commands described on the JIRA workflow page to create a branch for your changes), and the "git am --keep-cr -s < patch.txt" to apply a patch. If a patch was created that way and applies cleanly with that command, then you are definitely good to go.

The "patch -p1 < patch.txt" command is just a secondary method sometimes used to try to apply patches that aren't in the format produced above, or have errors when applying using that method.

Comment by Timothy Baldridge [ 26/Oct/12 9:16 PM ]

added patch

Comment by Andy Fingerhut [ 26/Oct/12 9:37 PM ]

That one applies cleanly and passes all tests. It should show up on the next list of prescreened patches. Thanks.

Comment by Rich Hickey [ 29/Nov/12 9:54 AM ]

we don't use the queue* convention elsewhere, e.g. vec and vector. I think queue should take a collection like vec and set. (queue [1 2 3]) could be made to 'adopt' the collection as front.

Comment by Andy Fingerhut [ 11/Dec/12 1:00 PM ]

Patch queue.patch dated Oct 26 2012 no longer applies cleanly after recent CLJ-1000 commits, but only because of one line of changed patch context. It still applies cleanly with "patch -p1 < queue.patch". Not bothering to update the stale patch given Rich's comments suggesting more substantive changes.

Comment by Steve Miner [ 06/Apr/13 8:06 AM ]

See also CLJ-976 (tagged literal support for PersistentQueue)

Comment by John Jacobsen [ 23/May/13 8:54 PM ]

Don't want to step on Timothy B's toes here, but it looks straightforward to adopt his patch to implement Rich's suggestion. I'd offer to give it a whack if nobody else wants the ticket now.

Comment by John Jacobsen [ 26/May/13 9:04 AM ]

Discussion initiated on clojure-dev: https://groups.google.com/forum/?fromgroups#!topic/clojure-dev/2BOqHm24Vc4

Comment by John Jacobsen [ 31/May/13 9:58 AM ]

This patch (if accepted) supersedes Timothy Baldridge's patch; it implements "queue" and "queue?" (but not "queue*"); "queue" accepts a collection rather than being a variadic function, as per Rich's suggestion.





[CLJ-1077] thread-bound? returns true (implying set! should succeed) even for non-binding thread Created: 26/Sep/12  Updated: 01/Oct/12

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: 1
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.



 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.





[CLJ-1076] pprint tests fail on Windows, expecting \n Created: 26/Sep/12  Updated: 02/Mar/13

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

Type: Defect Priority: Major
Reporter: Ivan Kozik Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None
Environment:

Windows 7


Attachments: Text File clj-1076-fix-tests-on-windows-patch-v1.txt     Text File clj-1076-fix-tests-on-windows-patch-v2.txt     Text File pprint_test_failures_01b4cb7156.txt    
Patch: Code and Test

 Description   

New pprint tests were committed recently, but they fail on Windows because the tests check for \n, while pprint seems to output \r\n. A log with the test failures is attached.

The first failing commit is https://github.com/clojure/clojure/commit/4ca0f7ea17888ba7ed56d2fde0bc2d6397e8e1c0



 Comments   
Comment by Andy Fingerhut [ 29/Sep/12 2:27 PM ]

Patch clj-1076-fix-tests-on-windows-patch-v1.txt dated Sep 29 2012 when applied to the particular commit that Ivan mentions causes the tests to pass when I run "ant" on a Windows 7 machine for me, and it continues to pass all tests on Mac OS X 10.6.8, too.

I may be doing something wrong, but when I try to run "ant" to build and test on Windows 7 with latest Clojure master, with or without this patch, it fails right at the beginning of the tests because it can't find clojure.test.generative. I'm probably doing something wrong somewhere. Ivan, would you be able to test this patch on Windows with the latest Clojure master to see if all tests pass for you now?

Comment by Ivan Kozik [ 29/Sep/12 2:59 PM ]

All tests pass on Windows 7 here with the patch.

Ant can't find my test.generative either because it isn't in my "${maven.test.classpath}". I put it in CLASSPATH and modified my build.xml like this:

<java classname="clojure.main" failonerror="false" fork="true">
<classpath>
+ <pathelement path="${java.class.path}"/>
<pathelement path="${maven.test.classpath}"/>

Comment by Andy Fingerhut [ 10/Dec/12 1:33 PM ]

Just as a rough idea of how often people are hitting this issue, CLJ-1123 was opened on Dec 9 2012 and then closed when the ticket creator realized it was a duplicate of this one.

Comment by Mike Anderson [ 18/Jan/13 7:44 PM ]

Hi there is this likely to get fixed soon?

I'd like to help contribute some more patches to Clojure but it's tricky to do when I can't get the build to work

Comment by Andy Fingerhut [ 18/Jan/13 8:39 PM ]

I do not know if or when this patch will be committed to Clojure.

I can tell you that you can apply the patch to your own local copy of the Clojure source code, and then develop new Clojure patches based upon that version. The patch that fixes this problem only affects one test file, so it is unlikely to conflict with any changes you develop and submit.

Comment by Mike Anderson [ 21/Jan/13 6:36 AM ]

I can confirm this patch works fine for me on Windows with Maven/Eclipse

Suggest this patch gets pushed through approval and applied ASAP? It's a pretty obvious fix that is breaking the build....

Comment by Stuart Halloway [ 01/Mar/13 12:44 PM ]

This patch is sloppy – it makes unnecessary whitespace changes in several places.

Would it be better to make the tests trailing whitespace agnostic? Otherwise this feels like poking and prodding until the build box is happy.

Comment by Andy Fingerhut [ 02/Mar/13 2:50 PM ]

Patch clj-1076-fix-tests-on-windows-patch-v2.txt dated Mar 2, 2013 fixes pprint tests on Windows in a different way: Removing all occurrences of carriage return (\r) characters in the output of pprint before comparing it to the expected string.

I tried simply doing str/trim-newline to remove newlines and carriage returns at the end of the string, but that does not make the tests pass. They still fail due to carriage returns in the middle of the string.

Comment by Andy Fingerhut [ 02/Mar/13 2:51 PM ]

Presumptuously changing Approval from Incomplete back to None, since there is a new patch attached that should address the reason it was marked Incomplete.





[CLJ-1074] Read/print round-trip for +/-Infinity and NaN Created: 21/Sep/12  Updated: 25/May/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: patch

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, Double/POSITIVE_INFINITY, Double/NEGATIVE_INFINITY, Double/NaN) are not eval-able after a round-trip via

(read-string (binding [*print-dup* true] (pr-str f))

The two options I see are to provide print-method implementations for these and their Float cousins, or to make Infinity, -Infinity, +Infinity, and NaN readable values. Since it sounds like edn may want to provide a spec for these values (see https://groups.google.com/d/topic/clojure-dev/LeJpOhHxESs/discussion and https://github.com/edn-format/edn/issues/2), I think making these values directly readable as already printed is preferable. Something like Double/POSITIVE_INFINITY seems too low-level from edn's perspective, as it would refer to a Java class and constant.

I'm attaching a patch implementing reader support for Infinity, -Infinity, +Infinity, and NaN.



 Comments   
Comment by Timothy Baldridge [ 03/Dec/12 11:34 AM ]

Please bring this up on clojure-dev. We'll be able to vet this ticket after that.

Comment by Colin Jones [ 03/Dec/12 1:18 PM ]

Should I respond to my original clojure-dev post about this (linked in the issue description above), or start a new one?

Comment by Andy Fingerhut [ 24/May/13 1:11 PM ]

Patch clj-1074-read-infinity-and-nan-patch-v2.txt dated May 24 2013 is identical to 0001-Read-Infinity-and-NaN.patch dated Sep 21 2012, except it applies cleanly to latest master. The older patch conflicts with a recent commit made for CLJ-873.

Comment by Nicola Mometto [ 25/May/13 11:55 AM ]

clj-1074-read-infinity-and-nan-patch-v2-plus-edn-reader.patch is the same as clj-1074-read-infinity-and-nan-patch-v2.txt except it patches EdnReader too, but it must be applied after #CLJ-873 0001-Fix-CLJ-873-for-EdnReader-too.patch get merged





[CLJ-1073] Make print-sequential interruptible Created: 21/Sep/12  Updated: 01/Mar/13

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

Type: Enhancement Priority: Minor
Reporter: Colin Jones Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: patch

Attachments: Text File 0001-Allow-thread-interruption-in-print-sequential.patch     Text File clj-1073-add-print-interruptibly-patch-v2.txt     File perftest-print.clj     File test.sh    
Patch: Code
Approval: Vetted

 Description   

This allows print-sequential to be interrupted by Thread.interrupt(), rather than requiring clients to resort to Thread.stop(). This is especially helpful when printing very large sequences.

See also clojure-dev discussion at https://groups.google.com/d/topic/clojure-dev/vs0RNUQXiYE/discussion



 Comments   
Comment by Andy Fingerhut [ 21/Sep/12 7:04 PM ]

In a quickly hacked up performance test on Mac OS X 10.6.8 + Oracle/Apple JDK 1.6.0_35 which I can attach, I saw about a 9% to 10% slowdown for Colin's patch in printing large vectors.

I have a modified patch that only calls Thread/interrupted after every 20 items are printed, and with that version the slowdown versus the original code was about 3% to 4%.

Colin, would there be any down side to checking Thread/interrupted less often for your purposes?

To run performance test, edit attachment compile.sh to point at your clojure.jar, put file perftest-print.clj in the same directory, and run ./compile.sh It should work on Mac or Linux.

Comment by Andy Fingerhut [ 22/Sep/12 10:47 AM ]

clj-1073-allow-thread-interrupt-in-print-sequential-patch.txt dated Sep 22 2012 is similar to Colin's patch 0001-Allow-thread-interruption-in-print-sequential.patch dated Sep 21 2012, except it only checks interrupted status every 20 (or maybe 21?) times through the loop in print-sequential. It is the one that is 3-4% slower than the current latest master Clojure code in my performance test mentioned above, versus Colin's patch which is about 9-10% slower on that test.

Comment by Stuart Halloway [ 19/Oct/12 3:31 PM ]

Is this primarily intended for dev-time use? I wouldn't want to lose performance for this if there is any way to implement it as a dev-time feature.

Comment by Colin Jones [ 19/Oct/12 4:27 PM ]

Andy: The only caveat I can think of with checking Thread/interrupted less often is in the case of deeply nested collections.

Stu: Dev-time use was the original reason for opening this, yes. But I can imagine it being needed for anytime a thread can be interrupted, whether that's by ctrl-c or other means.

My original thinking, performance-wise, was that once we're already doing IO, we're probably not too concerned with CPU-bound checks like this, so I didn't bother with it. I guess with an SSD that's not as likely to be true.

Locally (w/ my SSD), I'm seeing that Andy's benchmark of printing a million numbers is about a second slower than the baseline with my original patch (12.08s -> 13.10s), and about the same with Andy's patch (12.08s -> 11.75s). Decreasing to a thousand numbers doesn't really show any difference (every version completes in ~1.3s). Bumping to 2 million adds 2 seconds above the baseline with my patch, and Andy's is again just a bit faster than the baseline (somehow). Bumping to 5 million runs me out of heap space

Comment by Andy Fingerhut [ 08/Nov/12 4:16 PM ]

clj-1073-add-print-interruptibly-patch-v1.txt dated Nov 8 2012 is the same idea as Colin's patch 0001-Allow-thread-interruption-in-print-sequential.patch dated Sep 21 2012, except it only checks (Thread/interrupted) if a new var print-interruptibly is true. Its default value is false.

Performance results of the perftest-print.clj program, as driven by the test.sh script, for Clojure 1.5-beta1 and with two different patches. All run times are elapsed, in seconds, and sorted in increasing order for easier comparison.

Executive summary: Performance results when print-interruptibly is left at default false value are pretty much same as today. With print-interruptibly bound to true, performance results are slower, as expected, and about the same as with Colin's patch.

Original 1.5-beta1 unchanged:
13.75 13.80 13.83 13.87 13.93

With this new print-interruptibly patch, with print-interruptibly
at default false value:
13.86 13.91 14.01 14.08 14.14

With this new print-interruptibly patch, with print-interruptibly
bound to true while printing (so a slightly modified version of
perftest-print.clj that does (binding [*print-interruptibly* true]
...) around the heart of the code):
15.29 15.44 15.45 15.62 15.63

With patch 0001-Allow-thread-interruption-in-print-sequential.patch
applied:
15.38 15.46 15.48 15.49 15.50

Comment by Colin Jones [ 12/Nov/12 1:37 PM ]

Andy's latest patch looks good & makes it easy for the REPL and other interruptible scenarios to opt into the "safe" behavior. I would personally have preferred to have the "safe" behavior on by default, but can understand the performance concerns, and this gets me what I really wanted.

Comment by Timothy Baldridge [ 30/Nov/12 3:09 PM ]

Vetting as it sounds like the performance issues are taken care of.

Comment by Andy Fingerhut [ 13/Feb/13 12:28 AM ]

clj-1073-add-print-interruptibly-patch-v2.txt dated Feb 12 2013 is identical to clj-1073-add-print-interruptibly-patch-v1.txt dated Nov 8 2012 (soon to be removed) except that it applies cleanly to latest master.





[CLJ-1063] Missing dissoc-in Created: 07/Sep/12  Updated: 17/Sep/12

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

Type: Enhancement Priority: Minor
Reporter: Gunnar Völkel Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: File 001-dissoc-in.diff     Text File clj-1063-add-dissoc-in-patch-v2.txt    
Patch: Code and Test
Approval: Not Approved

 Description   

There is no clojure.core/dissoc-in although there is an assoc-in.
It is correct that dissoc-in can be build with update-in and dissoc but this is an argument against assoc-in as well.
When a shortcut for assoc-in is provided, there should also be one for dissoc-in for consistency reasons.
Implementation is analogical to assoc-in.



 Comments   
Comment by Andy Fingerhut [ 13/Sep/12 2:17 PM ]

Patch clj-1063-add-dissoc-in-patch-v2.txt dated Sep 13 2012 supersedes 001-dissoc-in.diff dated Sep 7 2012. It fixes a typo (missing final " in doc string), and adds a test case for the new function.

Comment by Nicola Mometto [ 13/Sep/12 2:27 PM ]

Thanks for the fix Andy

Comment by Andy Fingerhut [ 14/Sep/12 8:24 PM ]

This proposed dissoc-in should be compared with the one in clojure.core.incubator which I just happened across. I see they look different, but haven't examined to see if there are any behavior differences.

https://github.com/clojure/core.incubator/blob/master/src/main/clojure/clojure/core/incubator.clj

Comment by Nicola Mometto [ 15/Sep/12 6:43 AM ]

dissoc-in in clojure.core.incubator recursively removes empty maps

user=> (clojure.core.incubator/dissoc-in {:a {:b {:c 1}}} [:a :b :c])
{}

while the one in this patch doesn't (as I would expect)

user=> (dissoc-in {:a {:b {:c 1}}} [:a :b :c])
{:a {:b {}}}

Comment by Stuart Halloway [ 17/Sep/12 7:04 AM ]

Please do this work in the incubator.





[CLJ-1060] 'list*' returns not a list Created: 03/Sep/12  Updated: 18/Jan/13

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

Type: Defect Priority: Trivial
Reporter: Andrei Zhlobich Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: File list-star-fix.diff    
Patch: Code and Test

 Description   

Function 'list?' returns sequence, but not a list.
It is a bit confusing.

user=> (list? (list* 1 '(2 3)))
false


 Comments   
Comment by Stuart Halloway [ 17/Sep/12 6:52 AM ]

should the docstring for list* change to say it returns a seq?

Comment by Timothy Baldridge [ 27/Nov/12 11:58 AM ]

Is there a reason why we can't have list* actually return a list? The cost of creating a list vs a cons is negligible.

Comment by Marek Srank [ 04/Jan/13 2:02 PM ]

The question is what to do with the one-argument case of list*, because in cases like: (list* {:a 1 :b 2}) it doesn't return IPersistentList as well. I propose just applying 'list'.

I added patch 'list-star-fix.diff' (dated 04/Jan/2013) with Cons implementing IPersistentList and doing (apply list args) in one-argument case of list*. To be able to use 'apply' in list* I had to declare it before the definition of list* in the source code. The apply itself also uses list*, but luckily not the one-argument version of list*, so it should be safe... The patch also contains simple unit tests.

Discussion is also here: https://groups.google.com/forum/#!topic/clojure/co8lcKymfi8

Comment by Michał Marczyk [ 04/Jan/13 4:11 PM ]

(apply list args) would make (list* (range)) hang, where currently it returns a partially-realized lazy seq. Also, even for non-lazy seqs – possibly lists themselves – it would always build a new list from scratch, right?

Also, if I'm reading the patch correctly, it would make 2+-ary list* overloads and cons return lists – that is, IPersistentList instances – always (Conses would now be lists), but repeatedly calling next on such a list might eventually produce a non-list. The only way around that would be to make all seqs into IPersistentLists – that doesn't seem desirable at first glance...?

On a side note, for the case where the final "args" argument is in fact a list, we already have a "prepend many items" function, namely conj. list* currently acts as the vararg variant of cons (in line with Lisp tradition); I'm actually quite happy with that.

Comment by Michał Marczyk [ 04/Jan/13 4:19 PM ]

I'm in favour of the "list" -> "seq" tweak to the docstring though, assuming impl remains unchanged.

Comment by Marek Srank [ 04/Jan/13 6:13 PM ]

Yep, these are all valid points, thanks! I see this as a question whether the list* function is a list constructor or not. If yes (and it would be possible to implement it in a satisfactory way), it should probably return a list.

We could avoid building a new list by sth like:

(if (list? args)
  args
  (apply list args))

(btw, 'vec' also creates a new vector even when the argument itself is a vector)

The contract of next seems to be to return a seq, not a list - its docstring reads: "Returns a seq of the items after the first. Calls seq on its argument. If there are no more items, returns nil."

Btw, in some Lisp/Scheme impls I checked, cons seems to be a list as well. E.g. in CLisp (and similar in Guile and Racket):

> (listp (cons 2 '()))
T
> (listp (list* 1 2 '()))
T




[CLJ-1059] PersistentQueue doesn't implement java.util.List, causing nontransitive equality Created: 03/Sep/12  Updated: 11/Dec/12

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

Type: Defect Priority: Major
Reporter: Philip Potter Assignee: Philip Potter
Resolution: Unresolved Votes: 0
Labels: None

Attachments: File 001-clj-1059-make-persistentqueue-implement-list.diff     File 002-clj-1059-asequential-rebased-to-cached-hasheq.diff    
Patch: Code and Test
Approval: Vetted

 Description   

PersistentQueue implements Sequential but doesn't implement java.util.List. Lists form an equality partition, as do Sequentials. This means that you can end up with nontransitive equality:

(def q (conj clojure.lang.PersistentQueue/EMPTY 1 2 3))
;=> #user/q
(def al (doto (java.util.ArrayList.) (.add 1) (.add 2) (.add 3)))
;=> #user/al
(def v [1 2 3])
;=> #user/v
(= al v)
;=> true
(= v q)
;=> true
(not= al q)
;=> true

This happens because PersistentQueue is a Sequential but not a List, ArrayList is a List but not a Sequential, and PersistentVector is both.



 Comments   
Comment by Philip Potter [ 15/Sep/12 3:41 AM ]

Whoops, according to http://dev.clojure.org/display/design/JIRA+workflow I should have emailed clojure-dev before filing this ticket. Here is the discussion:

https://groups.google.com/d/topic/clojure-dev/ME3-Ke-RbNk/discussion

Comment by Philip Potter [ 15/Sep/12 2:37 PM ]

Attached 001-make-PersistentQueue-implement-List.diff, 15/Sep/12

Note that this patch has a minor conflict with the one I added to CLJ-1070, because both add an extra interface to PersistentQueue - List in this case, IHashEq in CLJ-1070.

Comment by Chouser [ 18/Sep/12 1:04 AM ]

Philip, patch looks pretty good – thanks for doing this. A couple notes:

This is only my opinion, but I prefer imports be listed without wildcards, even if it means an extra couple lines at the top of a .java file.

I noticed the "List stuff" code is a copy of what's in ASeq and EmptyList. I suppose this is copied because EmptyList and PersistentQueue extend Obj and therefore can't extend ASeq. Is this the only reason? It seems a shame to duplicate these method definitions, but I don't know of a better solution, do you?

It would also be nice if the test check a couple of the List methods you've implemented.

Comment by Chouser [ 18/Sep/12 1:08 AM ]

oh, also "git am" refused to apply the patch, but I'm not sure why. "patch -p 1" worked perfectly.

Comment by Philip Potter [ 18/Sep/12 1:19 AM ]

did you use the --keep-cr option to git am?

I struggled to know whether I should be adding CRs or not to line endings, because the files I was editing weren't consistent in their usage. If you open them in emacs, half the lines have ^M at the end.

Comment by Philip Potter [ 18/Sep/12 1:21 AM ]

Will submit another patch, with the import changed. I'll have a think about the list implementation and see what ideas I can come up with.

Comment by Philip Potter [ 18/Sep/12 3:17 PM ]

Attached 002-make-PersistentQueue-implement-Asequential.diff

This patch is an alternative to 001-make-PersistentQueue-implement-List.diff

So I took on board what you said about ASeq, but it didn't feel right making PersistentQueue directly implement ISeq, somehow.

So I split ASeq into two parts – ASequential, which implements j.u.{Collection,List} and manages List-equality and hashcodes; and ASeq, which... doesn't seem to be doing much anymore, to be honest.

As a bonus, this patch fixes CLJ-1070 too, so I went and added the tests from that ticket in to demonstrate this fact. It also tidies up PersistentQueue by removing all equals/hashcCode stuff and all Collection stuff.

(It turns out that because ASeq was already implementing Obj, the fact that PersistentQueue was implementing Obj was no barrier to using it.)

Would appreciate comments on this approach, and how it differs from the previous patch here and the patch on CLJ-1070.

Comment by Philip Potter [ 18/Sep/12 3:44 PM ]

Looking at EmptyList's implementation of List, it is a duplicate of the others, but it shouldn't be. I think its implementation of indexOf is the biggest culprit - it should just be 'return -1;' but it has a great big for loop! But this is beyond the scope of this ticket, so I won't patch that here.

Comment by Andy Fingerhut [ 20/Oct/12 12:29 PM ]

Philip, now that the patch for CLJ-1070 has been applied, these patches no longer apply cleanly. Would you be willing to update them? If so, please remove the obsolete patches.

Comment by Philip Potter [ 22/Oct/12 5:10 AM ]

Andy, thanks so much for your efforts to make people aware of these things. I will indeed submit new patches, hopefully later this week.

Comment by Philip Potter [ 03/Nov/12 12:23 PM ]

Replaced existing patches with new ones which apply cleanly to master.

There are two patches:

001-clj-1059-make-persistentqueue-implement-list.diff

This fixes equality by making PersistentQueue implement List directly. I also took the opportunity to remove the wildcard import and to add tests for the List methods, as compared with the previous version of the patch.

002-clj-1059-asequential.diff

This fixes equality by creating a new abstract class ASequential, and making PersistentQueue extend this.

My preferred solution is still the ASequential patch, but I'm leaving both here for comparison.

Comment by Timothy Baldridge [ 30/Nov/12 3:37 PM ]

Vetting.

Comment by Andy Fingerhut [ 11/Dec/12 12:50 PM ]

Philip, this time I think it was patches that were committed for CLJ-1000 that make your patch 002-clj-1059-asequential.diff not apply cleanly. I often fix up stale patches where the change is straightforward and mechanical, but in this case you are moving some methods that CLJ-1000's patch changed the implementation of, so it would be best if someone figured out a way to update this patch in a way that doesn't clobber the CLJ-1000 changes.

Comment by Philip Potter [ 11/Dec/12 1:57 PM ]

Thanks Andy. Submitted a new patch, 002-clj-1059-asequential-rebased-to-cached-hasheq.diff, which supersedes 002-clj-1059-asequential.diff.

The patch 001-clj-1059-make-persistentqueue-implement-list.diff still applies cleanly, and is still an alternative to 002-clj-1059-asequential-rebased-to-cached-hasheq.diff.





[CLJ-1058] StackOverflowError on exception in reducef for PersistentHashMap fold Created: 02/Sep/12  Updated: 12/Apr/13

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

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

Clojure 1.5.0-alpha4, Sun Java 1.6.0_35, with [org.codehaus.jsr166-mirror/jsr166y "1.7.0"]


Approval: Vetted

 Description   

If reducef throws an exception, the PHM fold code can descend into an infinite loop, causing a stack overflow which masks the problem. This situation is commented "aargh" in PersistentHashMap.java line 444 (as of 412a51d).

To reproduce:

user> (require '[clojure.core.reducers :as r])
nil
user> (r/fold (fn ([]) ([ret k v] (+ 3 "foo") ret)) (into {} (map (juxt identity identity) (range 10000))))
;; boom

This results in a stack like: https://raw.github.com/gist/3bab917287a7fd635a84/f38bfe3e270556e467f3fc02062af7ea10781390/gistfile1.txt



 Comments   
Comment by Timothy Baldridge [ 30/Nov/12 3:40 PM ]

Verified as a bug.





[CLJ-1057] Var's .setDynamic does not set :dynamic in metadata Created: 02/Sep/12  Updated: 03/Dec/12

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

Type: Defect Priority: Trivial
Reporter: Brandon Bloom Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   
((juxt (comp :dynamic meta) #(.isDynamic %)) #'*agent*)


 Comments   
Comment by Timothy Baldridge [ 03/Dec/12 9:10 AM ]

This is actually an enhancement as no where in the clojure code is provision made for syncing var's metadata and dynamic state. .isDynamic is the authoritative source, and the calling of .setDynamic is configured by the compiler. If you'd like to see this change, please, feel free to bring it up on clojure-dev for a discussion.





[CLJ-1056] defprotocol: invalid method overload syntax getting accepted Created: 01/Sep/12  Updated: 29/Nov/12

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

Type: Defect Priority: Minor
Reporter: Víctor M. Valenzuela Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Approval: Vetted

 Description   

The compiler accepts both of these erroneous forms which while silly, are not imposible to come up with.

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



 Comments   
Comment by Timothy Baldridge [ 29/Nov/12 4:02 PM ]

Can not reproduce the fist error:

user=> (defprotocol Foo (f ([this]) ([this arg])))
CompilerException java.lang.IllegalArgumentException: Parameter declaration missing, compiling:(NO_SOURCE_PATH:5:1)

But the 2nd one I can reproduce:

user=> (defprotocol Bar (m [this]) (m [this arg]))
Bar
user=> Bar
{:on-interface user.Bar, :on user.Bar, :sigs {:m {:doc nil, :arglists ([this arg]), :name m}}, :var #'user/Bar, :method-map {:m :m}, :method-builders {#'user/m #<user$eval71$fn_72 user$eval71$fn_72@1a2b53fb>}}
user=>

Notice that :arglists only has one entry

Vetting





[CLJ-1053] Locals still cleared too aggressively on delay in specific cases Created: 30/Aug/12  Updated: 09/Apr/13

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

Type: Defect Priority: Minor
Reporter: Jean Niklas L'orange Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

This seems to be an extension of #CLJ-232. Locals are still cleared too aggressively in some specific instances: If the delay throws an exception when realized, and you dereference a local within the delay, the local may or may not be cleared depending on its need in the tail positions (without any obvious pattern). Examples of functions which works as intended and doesn't work as intended follows.

(def d (let [y (delay 0)]
         (delay (if (zero? @y) (/ 0 0) (/ @y 1))))) ; works properly

(def d (let [y (delay 0)]
         (delay (if (zero? @y) (/ 0 0) (/ 1 1))))) ; does not work properly

(def d (let [y (delay 0)]
         (delay (if (zero? @y) (/ @y 0) (/ 1 1))))) ; does not work properly

(def d (let [y (delay 0)]
         (delay (if (zero? @y) (/ @y 0) (/ @y 1))))) ; does not work properly

(def d (let [y (delay 0)]
         (delay @y (/ 0 0)))) ; does not work properly

(def d (let [y (delay 0)]
         (delay @y (/ 0 0) @y))) ; works properly

By "works", d will throw "ArithmeticException Divide by zero" every single time it is dereferenced. By "does not work", d will throw "ArithmeticException Divide by zero" on first dereferencing, but from second and onwards it will throw "NullPointerException [trace missing]".



 Comments   
Comment by Jean Niklas L'orange [ 09/Apr/13 11:55 AM ]

With Clojure 1.5.1, the trace is given and returns NullPointerException clojure.core/deref-future (core.clj:2NullPointerException clojure.core/deref-future (core.clj:2108)108), which refers to a .get call. The stacktrace reveals the following:

NullPointerException 
	clojure.core/deref-future (core.clj:2108)
	clojure.core/deref (core.clj:2129)
	user/fn--1/fn--4 (NO_SOURCE_FILE:2)
	clojure.lang.Delay.deref (Delay.java:33)
	clojure.core/deref (core.clj:2128)
	user/eval13 (NO_SOURCE_FILE:-1)
	clojure.lang.Compiler.eval (Compiler.java:6619)
	clojure.lang.Compiler.eval (Compiler.java:6582)
	clojure.core/eval (core.clj:2852)
	clojure.main/repl/read-eval-print--6588/fn--6591 (main.clj:259)
	clojure.main/repl/read-eval-print--6588 (main.clj:259)
	clojure.main/repl/fn--6597 (main.clj:277)




[CLJ-1049] Make reducer/folder support reduce-kv Created: 22/Aug/12  Updated: 23/Sep/12

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

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

Attachments: File 0001-reduce-kv-transformations.diff    
Patch: Code

 Description   

Currently, although rfn makes an effort to support reduce-kv, it is wasted effort since the return values of reducer and folder don't satisfy IKVReduce.

I have a working patch for this, it's quite small. I have printed a CA but not sent it, so I won't reveal the patch yet...

This also applies to ClojureScript.



 Comments   
Comment by Tom Jack [ 15/Sep/12 12:47 AM ]

Hmm.. it's not quite as simple as I thought. My first patch simply had reducer/folder implement IKVReduce with `(clojure.core.protocols/kv-reduce coll (xf f1) init)`, but for map and mapcat, this results in strange behavior (reduce-kv reducefs being called with only 2 args).

Patch 0001 includes modifications to map and mapcat so that the reduce-kv reducef will always be called with 3 args. The previous implementation of mapcat also had the converse problem: during non-kv reduce, if the mapcat fn returned a map, the reducef would be called with 3 args since maps reduce with reduce-kv.

The patch gives behavior like:

user> (->> {1 {2 3 4 5} 6 {7 8 9 10}}
        (r/mapcat #(r/map + %2))
        (reduce-kv #(conj %1 [%2 %3]) []))
[[2 5] [4 9] [7 15] [9 19]]
user> (->> [{2 3 4 5} {7 8 9 10}]
        (r/mapcat identity)
        (r/reduce conj []))
[[2 3] [4 5] [7 8] [9 10]]
Comment by Tom Jack [ 23/Sep/12 8:34 PM ]

Would like to discuss these changes (and auto-kv for maps) on clojure-dev, but my membership is still pending. My CA has been received. Anyone know who to contact to get my membership approved?





[CLJ-1047] Simplify the process of requiring fj in clojure.core.reducers Created: 21/Aug/12  Updated: 21/Aug/12

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

Type: Enhancement Priority: Minor
Reporter: Nicola Mometto Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File 001-simplify-fj-importing.patch    
Patch: Code

 Description   

this patch removes compile-if in favor of import-if, removing code duplication






[CLJ-1046] Drop-while as a reducer Created: 18/Aug/12  Updated: 18/Aug/12

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

Type: Enhancement Priority: Major
Reporter: Alan Malloy Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File drop-while-reducer.patch    
Patch: Code and Test

 Description   

Implement drop-while as a reducer. Follows the same atom-based strategy as drop and take.

Does not depend on any of my other reducer patches, but there will probably be some minor merge conflicts unless it is merged after CLJ-1045, and before CLJ-992 and CLJ-993.






[CLJ-1045] Generalize/refactor implementation of PersistentVector/coll-fold Created: 18/Aug/12  Updated: 25/Jan/13

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

Type: Enhancement Priority: Major
Reporter: Alan Malloy Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File clj-1045-fold-by-halves-patch-v2.txt     Text File fold-by-halves.patch    
Patch: Code

 Description   

Vector currently contains a specialized implementation of the folding algorithm "split the collection in half until the pieces are small enough". The attached commit lifts out the general strategy so that it can be reused by other collection types amenable to splitting.

CLJ-993 depends on this patch, as it uses the new fold-by-halves function.



 Comments   
Comment by Andy Fingerhut [ 25/Jan/13 2:29 PM ]

clj-1045-fold-by-halves-patch-v2.txt dated Jan 25 2013 is identical to fold-by-halves.patch dated Aug 18 2012, except it updates one line of context changed by a recent commit to Clojure master.





[CLJ-1044] Enable refering to ->type inside deftype Created: 18/Aug/12  Updated: 15/Dec/12

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: 0
Labels: None

Attachments: File 001-enable-factory-ctor-inside-deftype.diff    
Patch: Code

 Description   

Inside defrecord is possible to refer to ->type-ctor, that is not possible inside deftype.
This patch adds an implicit declare, as done in defrecord



 Comments   
Comment by Timothy Baldridge [ 03/Dec/12 11:29 AM ]

Seems valid. Vetting.





[CLJ-1043] Unordered literals does not preserve left-to-right evaluation of arguments Created: 16/Aug/12  Updated: 23/Sep/12

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

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


 Description   

Given: (defn f [x] (println x) x)

#{(f 2) (f 1)}

Prints:

1
2

But expected would be:

2
1

This issue is related to CLJS-288



 Comments   
Comment by Andy Fingerhut [ 23/Sep/12 6:01 PM ]

I have the same question as David Nolen for CLJS-288: Is this a bug, or just behavior you didn't expect?

It seems that vectors preserve the order of evaluation, so if you really want to control evaluation order you could use something like (set [(f 2) (f 1)]) or (set (map f [2 1])).

Comment by Brandon Bloom [ 23/Sep/12 7:38 PM ]

I'd consider the expected default behavior of any syntax or macro to evaluate each sub-form once each, from left to right. Conditional, repeated, or out-of-order evaluation should be documented as deviations from that norm. If you buy that, then this is either a code or a documentation bug. My vote is for code bug.





[CLJ-1039] Using 'def with metadata {:type :anything} throws ClassCastException Created: 09/Aug/12  Updated: 10/Aug/12

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

Type: Defect Priority: Minor
Reporter: Gunnar Völkel Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Ubuntu, lein 1.7.1 - lein repl


Approval: Vetted

 Description   

In lein 1.7.1 (and CCW) the form (def ^{:type :anything} mydef 1) throws "ClassCastException clojure.lang.Var cannot be cast to clojure.lang.IObj clojure.core/with-meta (core.clj:211)".
This seems to be due to setting the :type metadata. With other metadata keys it works well.

If it is intended to forbid setting the :type metadata, then there should be an appropriate error message instead of the ClassCastException

In lein2 repl which is using "REPL-y 0.1.0-beta8" the exception does not occur.

Full stacktrace:
java.lang.ClassCastException: clojure.lang.Var cannot be cast to clojure.lang.IObj
at clojure.core$with_meta.invoke (core.clj:211)
clojure.core$vary_meta.doInvoke (core.clj:617)
clojure.lang.RestFn.invoke (RestFn.java:425)
clojure.core/fn (core_print.clj:76)
clojure.lang.MultiFn.invoke (MultiFn.java:167)
clojure.core$pr_on.invoke (core.clj:3266)
clojure.core$pr.invoke (core.clj:3278)
clojure.lang.AFn.applyToHelper (AFn.java:161)
clojure.lang.RestFn.applyTo (RestFn.java:132)
clojure.core$apply.invoke (core.clj:601)
clojure.core$prn.doInvoke (core.clj:3311)
clojure.lang.RestFn.invoke (RestFn.java:408)
clojure.main$repl$read_eval_print__6405.invoke (main.clj:246)
clojure.main$repl$fn__6410.invoke (main.clj:266)
clojure.main$repl.doInvoke (main.clj:266)
clojure.lang.RestFn.invoke (RestFn.java:512)
user$eval27$acc_3261auto__30$fn_32.invoke (NO_SOURCE_FILE:1)
clojure.lang.AFn.run (AFn.java:24)
java.lang.Thread.run (Thread.java:662)



 Comments   
Comment by Stuart Halloway [ 10/Aug/12 1:40 PM ]

This is caused by the printer dispatch function

(defmulti print-method (fn [x writer]
                         (let [t (get (meta x) :type)]
                           (if (keyword? t) t (class x)))))

which ends up calling the default dispatch, which tries to vary-meta.





[CLJ-1037] Allow doc strings for both interfaces and concrete implementations Created: 04/Aug/12  Updated: 04/Aug/12

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

Type: Enhancement Priority: Major
Reporter: Warren Lynn Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

In this post
http://groups.google.com/group/clojure/browse_thread/thread/84de74740928da76#

I mentioned the rationale (I think) why this is important and needed. Thank you for consideration.






[CLJ-1036] Util/hasheq should be hashing a BigInteger to the same values as Long, and BigInt Created: 02/Aug/12  Updated: 12/Apr/13

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

Type: Defect Priority: Major
Reporter: Paul Stadig Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File clj-1036-hasheq-for-biginteger-patch-v2.txt    
Patch: Code and Test
Approval: Not Approved

 Description   

The doc string for hash states that it defines a hash code function that is consistent with =, but for java.math.BigInteger hash is not consistent with =.

user=> (apply = [(Long. -1) -1N (biginteger -1)])
true
user=> (map hash [(Long. -1) -1N (biginteger -1)])
(0 0 -1)
user=>

It is possible to have a PHM with two key/value pairs where the keys are equal, and the hash codes are different:

user=> (assoc clojure.lang.PersistentHashMap/EMPTY (biginteger -1) :oops! -1N :one)
{-1N :one, -1 :oops!}

The expected behavior is the same as PersistentArrayMap, which does not have this issue, because it does not hash its keys:

user=> (assoc clojure.lang.PersistentArrayMap/EMPTY (biginteger -1) :oops! -1N :one)
{-1 :one}

This same misbehavior also occurs for Doubles and Floats:

thalia.core=> (apply = [(Float. 1e9) (Double. 1e9)])
true
thalia.core=> (map hash [(Float. 1e9) (Double. 1e9)])
(1315859240 1104006501)

That leads to the same difference in array-map and hash-map behavior as above for BigInteger and BigInt.



 Comments   
Comment by Paul Stadig [ 02/Aug/12 9:55 AM ]

Also, the biginteger function has metadata saying that it has been added since 1.0, but it was actually added in 1.3. The bigint function has metadata saying that it has been added since 1.3, but it has been added since 1.0.

I think during the work to implement BigInt someone renamed the existing bigint function (which used to return a BigInteger) to biginteger, and the metadata got carried with it, then a new bigint function was added with :since 1.3 metadata even though that function name has existed since 1.0.

Comment by Andy Fingerhut [ 26/Sep/12 11:59 AM ]

clj-1036-hasheq-for-biginteger-patch-v1.txt dated Sep 26 2012 makes BigInteger's return equal hash values for values considered equal by =.

It does the same for Float and Double types, which before returned different hash values for values considered equal by =

I went ahead and changed the :added metadata on bigint and biginteger, although I can see that without my change, the person who did that may have meant for the :added to go with the behavior of the function, not with the name. Paul's suggested change that I have in the patch is for the :added metadata to go with the name, not the function behavior. It is easy to remove that part of the patch if that change is not desired.

Comment by Rich Hickey [ 13/Nov/12 3:29 PM ]

You can't just consider only the lower long of bigints. Also, what's the rationale for the float stuff?

Comment by Andy Fingerhut [ 13/Nov/12 9:44 PM ]

clj-1036-hasheq-for-biginteger-patch-v2.txt dated Nov 13 2012 is identical to clj-1036-hasheq-for-biginteger-patch-v1.txt except that it addresses Rich's comment that for BigInt's and BigInteger values that don't fit in a long, their entire value must be hashed.

The rationale for the changes to hasheq for Float and Double types is the same as the rationale for the change for BigInteger: without that change, Float and Double types that are = can have different hasheq values.