<< Back to previous view

[CLJ-1890] enhance pprint to print names of defrecord types Created: 05/Feb/16  Updated: 06/Feb/16

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

Type: Enhancement Priority: Minor
Reporter: Daniel Compton Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: pprint, print

Attachments: Text File CLJ-1890-pprint-records.patch    
Patch: Code and Test
Approval: Triaged

 Description   

pprint currently doesn't print the names of defrecord types, instead printing just the underlying map. This is in contrast to pr-str/println. This ticket proposes that the behaviour of pprint is changed to match pr-str and println's.

More discussion at https://groups.google.com/forum/#!topic/clojure-dev/lRDG6a5eE-s

user=> (defrecord myrec [a b])
user.myrec
user=> (->myrec 1 2)
#user.myrec{:a 1, :b 2}
user=> (pr-str (->myrec 1 2))
"#user.myrec{:a 1, :b 2}"
user=> (println (->myrec 1 2))
#user.myrec{:a 1, :b 2}
nil
user=> (pprint (->myrec 1 2))
{:a 1, :b 2}
nil


 Comments   
Comment by Daniel Compton [ 05/Feb/16 1:51 PM ]

The fix for this will needed to be ported to ClojureScript too.

Comment by Steve Miner [ 06/Feb/16 11:55 AM ]

Added patch to pprint records with classname.

Comment by Steve Miner [ 06/Feb/16 12:03 PM ]

Open question: How should pprint handle a record that has a print-method defined for it? Should the print-method be used instead of the pprint default?

The current release and my patch do not consider the print-method when calling pprint.





[CLJ-1889] Add optional predicate to string trim functions that determines if a character should be trimmed Created: 27/Jan/16  Updated: 28/Jan/16

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

Type: Enhancement Priority: Minor
Reporter: Tamas Szabo Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: string

Attachments: Text File CLJ-1889-trim-enhancement.patch    
Patch: Code and Test
Approval: Triaged

 Description   

The proposal is that the trim functions (trim, triml, and trimr) would get a second arity with a function trim?:

[trim? ^CharSequence s]

trim? comes first to support partial.

New doc string would be:

"Removes characters from both ends of string. 
 If trim? is omitted white space is removed. When supplied it accepts 
 a character and returns true if the character should be removed."

Example test:

(deftest t-trim 
  (is (= "foo" (s/trim "  foo  \r\n"))) 
  (is (= "bar" (s/trim "\u2000bar\t \u2002"))) 

  ;; Additional test 
  (is (= "bar" (s/trim "$%#\u2000bar\t \u2002%$#" 
                       #(or (Character/isWhitespace %) ((set "$#%") %))))))

Similar to Python's strip - https://docs.python.org/2/library/stdtypes.html#str.strip

Approach: The proposed solution isn't very DRY but it follows the design guidelines at the top of the file, more exactly point 3:

"3. Functions take advantage of String implementation details to
write high-performing loop/recurs instead of using higher-order
functions. (This is not idiomatic in general-purpose application
code.)"

First I had a solution in which I replaced Character/isWhitespace from the current implementation by calling pred. pred was defaulted to an is-whitespace? function.
That code is of course nicer, even trim-newline could just call into trimr, removing a lot of duplication, but it adds the overhead of always calling a function, instead of calling Character/isWhitespace directly.

The only way I can see to have optimised and DRYer code is to use macros, but I don't think that it will necessary lead to nicer code.

Given the existing design style of the other functions in string.clj I felt that the best solution would be to just simply duplicate in favour of optimised code.



 Comments   
Comment by Tamas Szabo [ 27/Jan/16 2:44 PM ]

Proposed solution. Code + tests.

Comment by Tamas Szabo [ 27/Jan/16 3:42 PM ]

Added new patch that renames pred to trim?

Comment by Andy Fingerhut [ 27/Jan/16 6:27 PM ]

Note that Java, and thus Clojure/Java, uses UTF-16 encoding for strings in memory. Thus if you wanted to trim a set of Unicode code points from the beginning and/or end of a string, the API of trim? taking a single 16-bit Java character is not enough information to determine whether it should be trimmed or not.

If you want to handle that generality, it would require a more complex implementation, which checks whether the first/last character is one half of a code point that is encoded as 2 16-bit Java characters, and pass a 32-bit int to trim?, or something similar to that.

I have no objections if these API enhancements are made without enabling testing against an arbitrary Unicode code point. In the past, similar suggestions have been rejected in Clojure's built-in lib, e.g. CLJ-945

Comment by Tamas Szabo [ 28/Jan/16 1:56 AM ]

Yes, the UTF-16 encoding and Character representing either a codepoint or a half-codepoint is a bit of a mess, isn't it?

In the Java String and Character API's the methods that accept char, handle only characters in the Basic Multilingual Plane.
trim? accepts a character, so following the same behavior it will work only for removing characters in the Basic Multilingual Plane.

I think even this would be fine, but additionally because the high/low surrogates and the BMP characters are disjoint, you could actually use the same implementation to remove Unicode code points that aren't in the BMP. You can just say that both the high and low code unit of the codepoint are "unwanted".

Ex:
𝄞 is "\uD834\uDD1E"

user=> (trimr (set " \uD834\uDD1E") "example string  𝄞  ")
"example string"
Comment by Andy Fingerhut [ 28/Jan/16 5:11 AM ]

Agreed, but probably better to anti-recommend such an implementation of trimr for removing such things, because it would also remove only one UTF-16 Java character out of 2 high/low surrogates if it matched a member of the set, even if the other surrogate didn't match anything in the set, which would leave behind a malformed UTF-16 string.

Again, probably best to either not include this in the implementation at all, and at most warn about it in the docs, or to handle it in the implementation by checking for high/low surrogates in the loop(s).

Comment by Tamas Szabo [ 28/Jan/16 6:00 AM ]

Yes, you're right. That solution won't work in all cases, so it can't be recommended.

I am slightly inclined towards having trim? accept chars and work only for removing BMP characters. This will arguably be enough for the majority of the use cases.
The other solution can be used for all use cases, but then trim? will have to accept int, or 2 chars, or a string, so trim? would be less intuitive (although closer to the real world ), and writing those trim? functions would be less user friendly.

That being said, I am happy to change the implementation to do that if it is required.

Currently, I'm not even sure if the enhancement will be accepted or rejected or what the process for that is.





[CLJ-1888] AReference#meta() is synchronized Created: 26/Jan/16  Updated: 27/Jan/16

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

Type: Enhancement Priority: Major
Reporter: Roger Kapsi Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: performance

Attachments: PNG File aref-meta-after.png     PNG File aref-meta.png     Text File clj-1888-2.patch     Text File clj-1888.patch    
Patch: Code
Approval: Triaged

 Description   

We use Clojure for a "rules engine". Each function represents a rule and metadata describes the rule and provides some static configuration for the rule itself. The system is immutable and concurrent.

If two or more Threads invoke the same Var concurrently they end up blocking each other because AReference#meta() is synchronized (see attached screenshot, the red dots).

(defn 
  ^{:rule {:remote-address "127.0.0.1"}}
  example
  [request]
  (let [rule (:rule (meta #'example))]
    (= (:remote-address rule) (:remote-address request))))

Approach: Replace synchronized block with a rwlock for greater read concurrency. This approach removes meta read contention (see real world results in comments). However, it comes with the downsides of:

  • extra field for every AReference (all namespaces, vars, atoms, refs, and agents)
  • adds construction of lock into construction of AReference (affects perf and startup time)

Patch: clj-1888-2.patch replaces synchronized with a rwlock for greater read concurrency

Alternatives:

  • Use volatile for _meta and synchronized for alter/reset. Allow read of _meta just under the volatile - would this be safe enough?
  • Extend AReference from ReentrantReadWriteLock instead of holding one - this is pretty weird but would have a different (potentially better) footprint for memory/construction.


 Comments   
Comment by Alex Miller [ 26/Jan/16 10:19 PM ]

A volatile is not sufficient in alterMeta as you need to read/update/write atomically.

You could however use a ReadWriteLock instead of synchronized. I've attached a patch that does this - if you have a reproducible case I'd be interested to see how it affects what you see in the profiler.

There are potential issues that would need to be looked at - this will increase memory per reference (the lock instance) and slow down construction (lock construction) at the benefit of more concurrent reads.

Comment by Roger Kapsi [ 27/Jan/16 8:34 AM ]

Hey Alex,

I do have a reproducible case. The blocking has certainly disappeared after applying your patch (see attached picture). The remaining blocking code on these "WorkerThreads" is sun.nio.ch.SelectorImpl.select(long) (i.e. not clojure related).

You can repro it yourself by executing something like the code below concurrently in an infinite loop.

(defn 
  ^{:rule {:remote-address "127.0.0.1"}}
  example
  [request]
  (let [rule (:rule (meta #'example))]
    (= (:remote-address rule) (:remote-address request))))

Suggestions for the patch: Make the meta lock a final field and maybe pull the read/write locks into local variables to avoid the double methods calls.

alterMeta(...)
  Lock w = _metaLock.writeLock();
  w.lock();
  try {
    // ...
  } finally {
    w.unlock();
  }
}




[CLJ-1887] clojure.core.Vec does not fully implement clojure.lang.IPersistentVector Created: 26/Jan/16  Updated: 26/Jan/16

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

Type: Defect Priority: Major
Reporter: Steffen Dienst Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: collections
Environment:

Windows 7, Ubuntu Linux 14.04


Attachments: Text File CLJ-1887.patch    
Patch: Code and Test
Approval: Prescreened

 Description   

The implementation of `vector-of` in gvec.clj implements the interface clojure.lang.IPersistentVector, but skips the method `int length()`(see https://github.com/clojure/clojure/blob/bc186508ab98514780efbbddb002bf6fd2938aee/src/clj/clojure/gvec.clj#L240).

user=> (.length [1 2 3])
3
user=> (.length (vector-of :long 1 2 3))
AbstractMethodError Method clojure/core/Vec.length()I is abstract  clojure.core.Vec (gvec.clj:-1)

This was encountered while trying to use core.matrix -https://github.com/mikera/core.matrix/issues/266

Approach: Implement length in gvec

Patch: CLJ-1887.patch

Screened by: Alex Miller



 Comments   
Comment by Steffen Dienst [ 26/Jan/16 3:47 AM ]

The attached patch adds a .length method for primitive type vectors. Now it fully satisfies the interface clojure.lang.IPersistentVector

Comment by Alex Miller [ 26/Jan/16 8:50 AM ]

Good find and good fix.





[CLJ-1886] AOT compilation can cause java.lang.NoSuchFieldError: __thunk__0__ Created: 25/Jan/16  Updated: 26/Jan/16

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

Type: Defect Priority: Minor
Reporter: Ryan Fowler Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: aot


 Description   

In some very specific situation that I don't understand, the aot compiler can create class files with an inconsistent idea of a field called _thunk0_.

I've created a project at https://github.com/ryfow/weird-aot that reproduces the problem with `lein run`.

The ingredients for reproduction seem to be slf4j-timbre, tools.analyzer, and core.async.

I suspect that slf4j-timbre being aot compiled but not directly loaded by clojure code is a factor.

Note that the weird-aot timbre version differs from the version compiled in slf4j-timbre.

It's unclear to me why tools.analyzer and core.async are required to exhibit the problem.

Here's the stacktrace I get when I run `lein run` on the weird-aot project.

Exception.txt
Exception in thread "main" java.lang.NoSuchFieldError: __thunk__0__, compiling:(/private/var/folders/2q/tk7cywk93217_d4pxn_5kft40000gn/T/form-init7490372454812250103.clj:1:125)
        at clojure.lang.Compiler.load(Compiler.java:7239)
        at clojure.lang.Compiler.loadFile(Compiler.java:7165)
        at clojure.main$load_script.invoke(main.clj:275)
        at clojure.main$init_opt.invoke(main.clj:280)
        at clojure.main$initialize.invoke(main.clj:308)
        at clojure.main$null_opt.invoke(main.clj:343)
        at clojure.main$main.doInvoke(main.clj:421)
        at clojure.lang.RestFn.invoke(RestFn.java:421)
        at clojure.lang.Var.invoke(Var.java:383)
        at clojure.lang.AFn.applyToHelper(AFn.java:156)
        at clojure.lang.Var.applyTo(Var.java:700)
        at clojure.main.main(main.java:37)
Caused by: java.lang.NoSuchFieldError: __thunk__0__
        at clojure.tools.analyzer.jvm.utils__init.load(Unknown Source)
        at clojure.tools.analyzer.jvm.utils__init.<clinit>(Unknown Source)
        at java.lang.Class.forName0(Native Method)
        at java.lang.Class.forName(Class.java:340)
        at clojure.lang.RT.classForName(RT.java:2154)
        at clojure.lang.RT.classForName(RT.java:2163)
        at clojure.lang.RT.loadClassForName(RT.java:2182)
        at clojure.lang.RT.load(RT.java:436)
        at clojure.lang.RT.load(RT.java:412)
        at clojure.core$load$fn__5448.invoke(core.clj:5866)
        at clojure.core$load.doInvoke(core.clj:5865)
        at clojure.lang.RestFn.invoke(RestFn.java:408)
        at clojure.core$load_one.invoke(core.clj:5671)
        at clojure.core$load_lib$fn__5397.invoke(core.clj:5711)
        at clojure.core$load_lib.doInvoke(core.clj:5710)
        at clojure.lang.RestFn.applyTo(RestFn.java:142)
        at clojure.core$apply.invoke(core.clj:632)
        at clojure.core$load_libs.doInvoke(core.clj:5749)
        at clojure.lang.RestFn.applyTo(RestFn.java:137)
        at clojure.core$apply.invoke(core.clj:632)
        at clojure.core$require.doInvoke(core.clj:5832)
        at clojure.lang.RestFn.invoke(RestFn.java:703)
        at clojure.tools.analyzer.jvm$loading__5340__auto____1677.invoke(jvm.clj:9)
        at clojure.tools.analyzer.jvm__init.load(Unknown Source)
        at clojure.tools.analyzer.jvm__init.<clinit>(Unknown Source)
        at java.lang.Class.forName0(Native Method)
        at java.lang.Class.forName(Class.java:340)
        at clojure.lang.RT.classForName(RT.java:2154)
        at clojure.lang.RT.classForName(RT.java:2163)
        at clojure.lang.RT.loadClassForName(RT.java:2182)
        at clojure.lang.RT.load(RT.java:436)
        at clojure.lang.RT.load(RT.java:412)
        at clojure.core$load$fn__5448.invoke(core.clj:5866)
        at clojure.core$load.doInvoke(core.clj:5865)
        at clojure.lang.RestFn.invoke(RestFn.java:408)
        at clojure.core$load_one.invoke(core.clj:5671)
        at clojure.core$load_lib$fn__5397.invoke(core.clj:5711)
        at clojure.core$load_lib.doInvoke(core.clj:5710)
        at clojure.lang.RestFn.applyTo(RestFn.java:142)
        at clojure.core$apply.invoke(core.clj:632)
        at clojure.core$load_libs.doInvoke(core.clj:5749)
        at clojure.lang.RestFn.applyTo(RestFn.java:137)
        at clojure.core$apply.invoke(core.clj:632)
        at clojure.core$require.doInvoke(core.clj:5832)
        at clojure.lang.RestFn.invoke(RestFn.java:421)
        at weird_aot.core$loading__5340__auto____81.invoke(core.clj:1)
        at weird_aot.core__init.load(Unknown Source)
        at weird_aot.core__init.<clinit>(Unknown Source)
        at java.lang.Class.forName0(Native Method)
        at java.lang.Class.forName(Class.java:340)
        at clojure.lang.RT.classForName(RT.java:2154)
        at clojure.lang.RT.classForName(RT.java:2163)
        at clojure.lang.RT.loadClassForName(RT.java:2182)
        at clojure.lang.RT.load(RT.java:436)
        at clojure.lang.RT.load(RT.java:412)
        at clojure.core$load$fn__5448.invoke(core.clj:5866)
        at clojure.core$load.doInvoke(core.clj:5865)
        at clojure.lang.RestFn.invoke(RestFn.java:408)
        at clojure.core$load_one.invoke(core.clj:5671)
        at clojure.core$load_lib$fn__5397.invoke(core.clj:5711)
        at clojure.core$load_lib.doInvoke(core.clj:5710)
        at clojure.lang.RestFn.applyTo(RestFn.java:142)
        at clojure.core$apply.invoke(core.clj:632)
        at clojure.core$load_libs.doInvoke(core.clj:5749)
        at clojure.lang.RestFn.applyTo(RestFn.java:137)
        at clojure.core$apply.invoke(core.clj:632)
        at clojure.core$require.doInvoke(core.clj:5832)
        at clojure.lang.RestFn.invoke(RestFn.java:408)
        at user$eval65$fn__67.invoke(form-init7490372454812250103.clj:1)
        at user$eval65.invoke(form-init7490372454812250103.clj:1)
        at clojure.lang.Compiler.eval(Compiler.java:6782)
        at clojure.lang.Compiler.eval(Compiler.java:6772)
        at clojure.lang.Compiler.load(Compiler.java:7227)
        ... 11 more


 Comments   
Comment by Kevin Downey [ 26/Jan/16 1:58 AM ]

run.sh in the linked github repo throws:

Exception in thread "main" java.lang.RuntimeException: Method code too large!, compiling:(weird_aot/jetty.clj:4:1)

and fails to compile the required java source

EDIT it does compile the java source, but doesn't create the default compiler output directory for clojure or create it

Comment by Kevin Downey [ 26/Jan/16 2:03 AM ]

`lein compile` with a checkout of the linked github project completes without error for me

Comment by Kevin Downey [ 26/Jan/16 2:20 AM ]

fiddling a little, a number of deps, and their transient dependencies seem to be AOT compiled, likely with different versions of Clojure, which is not intended to work as far as I am aware. Code aot compiled with Clojure version A will fail to link with code being compiled with Clojure version B

Comment by Nicola Mometto [ 26/Jan/16 2:55 AM ]

I agree with Kevin here. The issue is highly likely caused by dependencies being distributed AOT and a dependency clash.

Comment by Kevin Downey [ 26/Jan/16 3:01 AM ]

com.fzakaria/slf4j-timbre "0.2.2" is the issue. the library is aot compiled, which transitively aot compiles its dependencies, which are older versions of a bunch of timbre libraries, which in turn depend on an old version of tools.reader, so the jar for com.fzakaria/slf4j-timbre "0.2.2" contains an old compiled version of `tools.reader`. org.clojure/tools.analyzer.jvm "0.6.9" was aot compiled against a newer version of `tools.reader` so everything explodes

Comment by Alex Miller [ 26/Jan/16 8:54 AM ]

Publishing a jar with AOT'ed dependencies is for sure a problem. I realize this is a bit painful due to CLJ-322 (which I'm hoping to actually make some headway on this year).

Is there something else that should be done on this ticket?

Comment by Nicola Mometto [ 26/Jan/16 8:56 AM ]

I don't think there's anything that we can do other than pushing CLJ-322 and discouraging users to publish AOT compiled libs

Comment by Ryan Fowler [ 26/Jan/16 9:11 AM ]

The problem for me is the error message. It's fine that I can't depend on AOT compiled libraries. It doesn't seem ok that the error message when I do this is "java.lang.NoSuchFieldError: _thunk0_" or "java.lang.RuntimeException: Method code too large!"

Comment by Alex Miller [ 26/Jan/16 9:28 AM ]

I hear you. Unfortuantely, I'm not sure there's any way to detect this is what is happening in a generic way and produce a better error. The same kinds of weirdness can happen in Java as well when using a mixture of library versions.





[CLJ-1885] data/diff does not return a tuple when comparing different maps Created: 16/Jan/16  Updated: 16/Jan/16

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

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

all


Attachments: Text File CLJ-1885.patch     Text File CLJ-1885-tests.patch    
Approval: Triaged

 Description   

Problem: clojure.data/diff inconsistently returns a lazy seq when comparing different maps, but a vector otherwise.

user> (data/diff {:a 1 :b 2} {:a 1})
({:b 2} nil {:a 1})

This is inconsistent with doc and normal behavior :

user> (data/diff {:a 1 :b 2} {:a 1 :b 2})
[nil nil {:a 1, :b 2}]
user> (data/diff #{1 2 3} #{1 2 3})
[nil nil #{1 3 2}]
user> (data/diff #{1 2 3} #{1 2})
[#{3} nil #{1 2}]

The docstring states: "Recursively compares a and b, returning a tuple of [things-only-in-a things-only-in-b things-in-both]", implying that it should always return a vector.



 Comments   
Comment by Eric Dvorsak [ 16/Jan/16 10:02 AM ]

Fixing it just requires to vectorize diff-associative output like this :

(defn- diff-associative
  "Diff associative things a and b, comparing only keys in ks."
  [a b ks]
  (vec (reduce
   (fn [diff1 diff2]
     (doall (map merge diff1 diff2)))
   [nil nil nil]
   (map
    (partial diff-associative-key a b)
    ks))))
Comment by Alex Miller [ 16/Jan/16 10:10 AM ]

There are other potential ways to address this, such as by using transducers instead. Not sure if that's worth doing, but seems reasonable to consider while we're making changes.

Comment by Eric Dvorsak [ 16/Jan/16 10:15 AM ]

Maybe this could be done as an improvement and proposed in an other ticket.

Vec is already used to vectorize the lists in diff-sequential. I would suggest to just fix the bug and add the test cases that should have screen it.

Comment by Eric Dvorsak [ 16/Jan/16 10:20 AM ]

There is a test case that should already fail :

[{:a #{2}} {:a #{4}} {:a #{3}}] {:a #{2 3}} {:a #{3 4}}

I get

({:a #{2}} {:a #{4}} {:a #{3}})
Comment by Alex Miller [ 16/Jan/16 10:33 AM ]

The test may need to be made more strict, checking not just for sequential equality but also for a returned vector.

Just curious - was this issue causing a problem in your code or did you just notice it and find it surprising?

Comment by Eric Dvorsak [ 16/Jan/16 11:05 AM ]

Simple patch that just does for maps what is done for lists : Creates a new vector with the vec function.

Comment by Eric Dvorsak [ 16/Jan/16 11:08 AM ]

@Alex Miller : I noticed a bug in my program behavior and traced it down to a (get diff 2) instead of (nth diff 2), but I realized that it was only buggy in some cases so I looked further and found out if was coming from diff.

Comment by Eric Dvorsak [ 16/Jan/16 11:27 AM ]

More strict tests checking for a returned vector.





[CLJ-1884] Add support for two parameters to rand and rand-int Created: 14/Jan/16  Updated: 14/Jan/16

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

Type: Enhancement Priority: Minor
Reporter: Juan José Conti Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

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

 Description   

I'd like to propose a change for rand and rand-int. I'd like to add the possibility to call them with two parameters having the result is in that range.

I think it would be a useful change as now, when you want to get a random number in a range you have to google how to do it because you don't remember it and end up with something that is not obvious what it's doing:

(+ a (rand (- b a))

The change is simple (I've done it locally, build and tested it). Patch attached.

https://groups.google.com/forum/#!topic/clojure-dev/hl2XtXNGb8w






[CLJ-1883] references to a symbol ending with ? that came from a foreign namespace confuses the compiler inside a :cljc block Created: 13/Jan/16  Updated: 13/Jan/16  Resolved: 13/Jan/16

Status: Closed
Project: Clojure
Component/s: None
Affects Version/s: Release 1.7, Release 1.8
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Geraldo Lopes de Souza Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None

Attachments: File bug.cljc     File bug.tar.bz2    

 Description   

I've found a situation where I'm requiring a om.tempid on a .cljc namespace. The require is
[om.tempid :as tempid :refer [tempid?]]
The reference of tempid? on line 24 of bug.cljc generates
java.lang.IllegalArgumentException: Unable to resolve classname: clojure.core$boolean@7521bffd, compiling/home/geraldo/bug/src/bug/bug.cljc:14:5)

To circumvent I've created a local alias of the tempid? function (line 26). It seems that the ? at the end of the name is causing the trouble because if I reference om.tempid/tempid it does not trigger. Note that the local alias was ended with ? to show that only when is a foreign ns that the problem is present.

Regards and forgive my english.

Geraldo Lopes de Souza



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

tempid? is:

(defn ^boolean tempid? [x]
  (instance? TempId x))

I suspect the ? has nothing to do with it and it's the ^boolean type hint that's the issue.

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

This is a bug in Om in using a type hint in non-conditional code that is cljs-specific. David Nolen is fixing in Om.

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

https://github.com/omcljs/om/commit/cc39f37561455a54153aaef7e5ca36782839aa38

Comment by Geraldo Lopes de Souza [ 13/Jan/16 4:38 PM ]

Alex,

I saw the other issue referencing the ^boolean but I didn't relate with this.

Thank you,

Geraldo Lopes de Souza





[CLJ-1882] Use transients in merge-with Created: 11/Jan/16  Updated: 11/Jan/16

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

Type: Enhancement Priority: Major
Reporter: Ghadi Shayban Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: performance, transient


 Description   

This ticket has been broken away from CLJ-1458 for tracking.






[CLJ-1881] Can :or destructuring refer to previous sequential bindings? Created: 11/Jan/16  Updated: 11/Jan/16

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

Type: Enhancement Priority: Minor
Reporter: Alex Miller Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: destructuring, docs

Approval: Triaged

 Description   

The following code works, but it is unspecified in the docs whether `(inc a)` can rely on `a` being bound.

user=> (defn foo [a {:keys [b] :or {b (inc a)}}]
  [a b])
user=> (foo 1 {:b 99})
[1 99] ;; :or not needed
user=> (foo 1 {})
[1 2]  ;; :or binds b to (inc a)

In sequential destructuring, are bindings bound in order such that subsequent :or value expressions can rely on prior sequential bindings?

This is true based on the current implementation of destructure, but looking for a statement to this effect in the docs and/or tests.






[CLJ-1880] IKVReduce impl for records Created: 09/Jan/16  Updated: 11/Jan/16

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

Type: Enhancement Priority: Major
Reporter: Ghadi Shayban Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: defrecord

Attachments: Text File CLJ-1880.patch    
Approval: Triaged

 Description   

Records don't implement IKVReduce, which could help with efficient merging (CLJ-1458)



 Comments   
Comment by Ghadi Shayban [ 11/Jan/16 2:49 PM ]

simple implementation attached





[CLJ-1879] reduce-kv on a PHMs doesn't consistently execute the intended fastpath Created: 09/Jan/16  Updated: 11/Jan/16

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

Type: Defect Priority: Major
Reporter: Ghadi Shayban Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: collections

Attachments: Text File CLJ-1879.patch    
Approval: Triaged

 Description   

https://github.com/clojure/clojure/blob/010864f/src/clj/clojure/core.clj#L6553-L6562

Because PHMs implement clojure.lang.IKVReduce and IPersistentMap, they have nondeterministic dispatch through the protocol that backs reduce-kv (clojure.core.protocols/IKVReduce).

A potential way to solve this is to add an instance check for clojure.lang.IKVReduce inside `reduce-kv` (This is similar to how reduce checks for IReduceInit)



 Comments   
Comment by Nicola Mometto [ 11/Jan/16 9:23 AM ]

CLJ-1807 offers a generic solution for this class of problems





[CLJ-1878] destructuring doesn't work on a sorted set Created: 08/Jan/16  Updated: 08/Jan/16  Resolved: 08/Jan/16

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

Type: Defect Priority: Major
Reporter: Anton Gladyshevskiy Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: collections


 Description   

Destructuring doesn't work on a sorted set:

(let [[head :as demoset] (sorted-set 1 2 3)] head)

UnsupportedOperationException nth not supported on this type: PersistentTreeSet clojure.lang.RT.nthFrom (RT.java:933)



 Comments   
Comment by Alex Miller [ 08/Jan/16 4:40 PM ]

Sequential destructuring works on sequential collections - sets (even sorted sets) are unordered (not sequential).

You can make this work with something like (which defines a sequential collection from the set):

(let [[head :as demoset] (seq (sorted-set 1 2 3))] head)
Comment by Anton Gladyshevskiy [ 08/Jan/16 4:57 PM ]

Thank you. It's a pity, because in this case 'demoset' becomes a sequence, not a sorted set.





[CLJ-1877] operation on the sorted-set fails under certain conditions Created: 08/Jan/16  Updated: 08/Jan/16  Resolved: 08/Jan/16

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

Type: Defect Priority: Major
Reporter: Anton Gladyshevskiy Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: collections
Environment:

Windows 8.1



 Description   

Function generates a sequence of prime numbers. Uses a priority queue implemented as a sorted set of vectors [priority val].

On the iteration (x = 10) fails with message:

(defn sieve [[x & t] pq]
  (lazy-seq
    (if (or (empty? pq) (< x (ffirst pq)))
      (cons x (sieve t (conj pq [(* x x) (next (iterate (partial + x) (* x x)))])))
      (sieve t
        (loop [pq pq] (let [[key val :as head] (first pq)]
          (if (= x key) (recur (conj (disj pq head) [(first val) (next val)])) pq) ))))))

(def primes (sieve (iterate inc 2) (sorted-set)))

(take 4 primes)
;; => (2 3 5 7)

(take 5 primes)
ClassCastException clojure.lang.Iterate cannot be cast to java.lang.Comparable  clojure.lang.Util.compare (Util.java:153)

When x = 10 it goes to the (loop ...) section and fails while trying to recur.



 Comments   
Comment by Alex Miller [ 08/Jan/16 4:45 PM ]

Sets are unordered (not sequential) and cannot be used with sequential destructuring. You can wrap (seq ) around the conj in the recur if you wish to make it sequential.

Comment by Anton Gladyshevskiy [ 08/Jan/16 4:55 PM ]

Destructuring is not a subject of this issue. The problem is that the code that have been successfully evaluated several times before fails on the subsequent iteration. I.e. when x is 4, 6, 8, 9 it works fine, but when x = 10 it fails.

Comment by Alex Miller [ 08/Jan/16 8:18 PM ]

Oh, yeah! I totally misread this one after reading the last one. The issue here is that you're trying to put something into a sorted set, but the type of the item (here the sequence produced by `iterate`) is not comparable to the existing items in the set (vectors). Vectors are comparable, but not general sequences.

A simpler example of the same issue is: `(conj (sorted-set) [1] '(2))`

There is a ticket for making lists comparable (CLJ-1467), but I'm not sure it would be a good idea to generically extend this to all sequential data types.





[CLJ-1876] calling require from java is not thread safe Created: 07/Jan/16  Updated: 07/Jan/16

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

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

Crappy Linux VM running RHEL6

java version "1.8.0_60"
Java(TM) SE Runtime Environment (build 1.8.0_60-b27)
Java HotSpot(TM) 64-Bit Server VM (build 25.60-b23, mixed mode)



 Description   

As a part of Apache Storm we have some code that can load a clojure function from java using the following code.

public static IFn loadClojureFn(String namespace, String name) {
        try {
            clojure.lang.Compiler.eval(RT.readString("(require '" + namespace + ")"));
        } catch (Exception e) {
            //if playing from the repl and defining functions, file won't exist
        }
        return (IFn) RT.var(namespace, name).deref();
    }

If this function is called from multiple different threads at the same time, trying to import the same namespace, I will occasionally get some very odd errors. NOTE: I had to modify the catch to actually print out the error message it was getting (We should not be eating exceptions either way).

{verbatim}
2016-01-07 16:26:09.305 b.s.u.Utils [WARN] Loading namespace failed
clojure.lang.Compiler$CompilerException: java.lang.RuntimeException: Unable to resolve symbol: sentence-spout in this context, compiling:(storm/starter/clj/word_count.clj:21:1)
at clojure.lang.Compiler.analyze(Compiler.java:6543) ~[clojure-1.7.0.jar:?]
at clojure.lang.Compiler.analyze(Compiler.java:6485) ~[clojure-1.7.0.jar:?]
at clojure.lang.Compiler$InvokeExpr.parse(Compiler.java:3791) ~[clojure-1.7.0.jar:?]
at clojure.lang.Compiler.analyzeSeq(Compiler.java:6725) ~[clojure-1.7.0.jar:?]
at clojure.lang.Compiler.analyze(Compiler.java:6524) ~[clojure-1.7.0.jar:?]
at clojure.lang.Compiler.analyze(Compiler.java:6485) ~[clojure-1.7.0.jar:?]
at clojure.lang.Compiler.eval(Compiler.java:6786) ~[clojure-1.7.0.jar:?]
at clojure.lang.Compiler.load(Compiler.java:7227) ~[clojure-1.7.0.jar:?]
at clojure.lang.RT.loadResourceScript(RT.java:371) ~[clojure-1.7.0.jar:?]
at clojure.lang.RT.loadResourceScript(RT.java:362) ~[clojure-1.7.0.jar:?]
at clojure.lang.RT.load(RT.java:446) ~[clojure-1.7.0.jar:?]
at clojure.lang.RT.load(RT.java:412) ~[clojure-1.7.0.jar:?]
at clojure.core$load$fn__5448.invoke(core.clj:5866) ~[clojure-1.7.0.jar:?]
at clojure.core$load.doInvoke(core.clj:5865) ~[clojure-1.7.0.jar:?]
at clojure.lang.RestFn.invoke(RestFn.java:408) ~[clojure-1.7.0.jar:?]
at clojure.core$load_one.invoke(core.clj:5671) ~[clojure-1.7.0.jar:?]
at clojure.core$load_lib$fn__5397.invoke(core.clj:5711) ~[clojure-1.7.0.jar:?]
at clojure.core$load_lib.doInvoke(core.clj:5710) ~[clojure-1.7.0.jar:?]
at clojure.lang.RestFn.applyTo(RestFn.java:142) ~[clojure-1.7.0.jar:?]
at clojure.core$apply.invoke(core.clj:632) ~[clojure-1.7.0.jar:?]
at clojure.core$load_libs.doInvoke(core.clj:5749) ~[clojure-1.7.0.jar:?]
at clojure.lang.RestFn.applyTo(RestFn.java:137) ~[clojure-1.7.0.jar:?]
at clojure.core$apply.invoke(core.clj:632) ~[clojure-1.7.0.jar:?]
at clojure.core$require.doInvoke(core.clj:5832) ~[clojure-1.7.0.jar:?]
at clojure.lang.RestFn.invoke(RestFn.java:408) ~[clojure-1.7.0.jar:?]
at clojure.core$eval114.invoke(NO_SOURCE_FILE:0) ~[?:?]
at clojure.lang.Compiler.eval(Compiler.java:6782) ~[clojure-1.7.0.jar:?]
at clojure.lang.Compiler.eval(Compiler.java:6745) ~[clojure-1.7.0.jar:?]
at backtype.storm.utils.Utils.loadClojureFn(Utils.java:602) [storm-core-0.11.0-SNAPSHOT.jar:0.11.0-SNAPSHOT]
at backtype.storm.clojure.ClojureBolt.prepare(ClojureBolt.java:57) [storm-core-0.11.0-SNAPSHOT.jar:0.11.0-SNAPSHOT]
at backtype.storm.daemon.executor$fn_8297$fn_8310.invoke(executor.clj:785) [storm-core-0.11.0-SNAPSHOT.jar:0.11.0-SNAPSHOT]
at backtype.storm.util$async_loop$fn__556.invoke(util.clj:482) [storm-core-0.11.0-SNAPSHOT.jar:0.11.0-SNAPSHOT]
at clojure.lang.AFn.run(AFn.java:22) [clojure-1.7.0.jar:?]
at java.lang.Thread.run(Thread.java:745) [?:1.8.0_60]
Caused by: java.lang.RuntimeException: Unable to resolve symbol: sentence-spout in this context
at clojure.lang.Util.runtimeException(Util.java:221) ~[clojure-1.7.0.jar:?]
at clojure.lang.Compiler.resolveIn(Compiler.java:7019) ~[clojure-1.7.0.jar:?]
at clojure.lang.Compiler.resolve(Compiler.java:6963) ~[clojure-1.7.0.jar:?]
at clojure.lang.Compiler.analyzeSymbol(Compiler.java:6924) ~[clojure-1.7.0.jar:?]
at clojure.lang.Compiler.analyze(Compiler.java:6506) ~[clojure-1.7.0.jar:?]
... 33 more{verbatim}

If I make the static java function synchronized the issue goes away. It always seems to blow up when parsing a few specific macros getting confused that a specific symbol cannot be resolved.

The namespace trying to be loaded.
https://github.com/apache/storm/blob/a99d9c11be005ade7c308bebdda71c7fb0111acc/examples/storm-starter/src/clj/storm/starter/clj/word_count.clj

The macros that we seem to get exceptions on.
https://github.com/apache/storm/blob/a99d9c11be005ade7c308bebdda71c7fb0111acc/storm-core/src/clj/backtype/storm/clojure.clj#L77-L138

And like I said it look like it is a threading issue of some sort. When I added the synchronized keyword everything works.






[CLJ-1875] Parameter names in docstring for `into` Created: 06/Jan/16  Updated: 20/Jan/16

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

Type: Enhancement Priority: Minor
Reporter: Marc O'Morain Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring

Attachments: Text File CLJ-1875.patch    
Patch: Code
Approval: Triaged

 Description   

The docstring for into does not have the correct names for the parameters. The parameter names in the arglist are to, from and xform, but the doctring refers to them as to-coll from-coll, and the docstring does not refer to the optional transducer, xform by name.

Approach: update docstring to reflect param names
Patch: CLJ-1875.patch
Screened by:



 Comments   
Comment by Alex Miller [ 20/Jan/16 3:58 PM ]

The additional phrase at the end doesn't make sense to me. How about instead:

"A transducer, xform, may be supplied as an optional second argument."





[CLJ-1874] Var redefinition breaks in AOT-compiled code Created: 05/Jan/16  Updated: 05/Jan/16

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

Type: Defect Priority: Major
Reporter: William Parker Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: aot, compiler

Attachments: Text File 0001-CLJ-1874-ensure-vars-get-interned-when-AOT-compiled.patch    
Patch: Code

 Description   

This is basically a copy of my post from https://groups.google.com/forum/#!topic/clojure/Ozt5HQyM36I on the mailing list. Based on the replies there I'm not sure whether this should be logged as an enhancement or a defect. Please change the designation to whatever is appropriate.

I have found what appears to be a bug in AOT-compiled Clojure when ns-unmap is used; the root cause of this probably impacts other code that redefines Vars as well. I have the following reduced case:

(ns unmap-test.core)

(def a :test-1)

(ns-unmap 'unmap-test.core 'a)

(def a :test-2)

It turns out that a is not resolvable when this namespace is loaded. When I looked at the compiled bytecode,
it appears that the following operations occur:

1. A call to RT.var withe 'unmap-test.core and 'a returns a Var, which is bound to a constant.
This var is added to the namespaces's mapping during this call.
2. Same as 1.
3. The var from 1 is bound to :test-1.
4. ns-unmap is called.
5. The var from 2 is bound to :test-2.

Disclaimer: This is the first time I have had occasion to look directly at bytecode and I could be missing something.

The basic problem here is that the var is accessible from the load method, but when step 5 executes the var is no longer
accessible from the namespace mappings. Thus, the root of the Var is set to :test-2, but that Var is not mapped from the namespace.
This works when there is no AOT compilation, as well as when I use

(ns unmap-test.core)

(def a :test-1)

(ns-unmap 'unmap-test.core 'a)

(intern 'unmap-test.core 'a :test-2)

I realize that creating defs, unmapping them, and then recreating them is generally poor practice in Clojure.
We have an odd case in that we need to have an interface and a Var of the same name in the same namespace. Simply
doing definterface and then def causes a compilation failure:

user=> (definterface abc)
user.abc
user=> (def abc 1)
CompilerException java.lang.RuntimeException: Expecting var, but abc is mapped to interface user.abc, compiling/private/var/folders/3m/tvc28b5d7p50v5_8q5ntj0pmflbdh9/T/form-init4734176956271823921.clj:1:1)

Without going into too much detail, this is basically a hack to allow us to refactor our internal framework code
without immediately changing a very large amount of downstream consumer code. We get rid of the usage of the interface during macroexpansion,
but it still needs to exist on the classpath so it can be imported by downstream namespaces.
There are a number of other ways to accomplish this, so it isn't a particularly big problem for us, but I thought the issue was worth raising.
This was just the first thing I tried and I was surprised when it didn't work.

Note that I used the 1.8.0 RC4 version of Clojure in my sample project, but I had the same behavior on 1.7.0.

Relevant links:

1. Bytecode for the load method of the init class: https://gist.github.com/WilliamParker/d8ef4c0555a30135f35a
2. Bytecode for the init0 method: https://gist.github.com/WilliamParker/dc606ad086670915efd9
3. Decompiled Java code for the init class. Note that this does not completely line up with the bytecode as far as I can tell,
but it is a quicker way to get a general idea of what is happening than the bytecode.
https://gist.github.com/WilliamParker/4cc47939f613d4595d94
4. A simple project containing the code above: https://github.com/WilliamParker/unmap-test
Note that if you try it without AOT compilation the target folder with any previously compiled classes should be removed.



 Comments   
Comment by Nicola Mometto [ 05/Jan/16 9:44 AM ]

The issue is similar to the one in CLJ-1604, the proposed patch extends that fix to all vars rather than just for clojure.core ones.





[CLJ-1873] Docstrings for require and *data-readers* do not mention cljc files Created: 01/Jan/16  Updated: 18/Jan/16

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

Type: Enhancement Priority: Minor
Reporter: Andy Fingerhut Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring, ft

Attachments: Text File clj-1873-v1.patch    
Patch: Code
Approval: Prescreened

 Description   

The behavior of require and *data-readers* was modified in Clojure 1.7.0 to look for files ending with .cljc as well as files ending with .clj, but their doc strings do not mention this change.

Patch: clj-1873-v1.patch

Screened by: Alex Miller



 Comments   
Comment by Andy Fingerhut [ 01/Jan/16 4:30 AM ]

Patch clj-1873-v1.patch dated Jan 1 2016 adds mentions of .cljc files to the doc strings of require and *data-readers*





[CLJ-1872] empty? is broken for transient collections Created: 26/Dec/15  Updated: 12/Jan/16

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

Type: Defect Priority: Critical
Reporter: Leonid Bogdanov Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: collections

Approval: Triaged

 Description   

Couldn't find whether it was brought up earlier, but it seems that empty? predicate is broken for transient collections

user=> (empty? (transient []))
IllegalArgumentException Don't know how to create ISeq from: clojure.lang.PersistentVector$TransientVector  clojure.lang.RT.seqFrom (RT.java:528)

user=> (empty? (transient ()))
ClassCastException clojure.lang.PersistentList$EmptyList cannot be cast to clojure.lang.IEditableCollection  clojure.core/transient (core.clj:3209)

user=> (empty? (transient {}))
IllegalArgumentException Don't know how to create ISeq from: clojure.lang.PersistentArrayMap$TransientArrayMap  clojure.lang.RT.seqFrom (RT.java:528)

user=> (empty? (transient #{}))
IllegalArgumentException Don't know how to create ISeq from: clojure.lang.PersistentHashSet$TransientHashSet  clojure.lang.RT.seqFrom (RT.java:528)

The workaround is to use (zero? (count (transient ...))) check instead.



 Comments   
Comment by Alex Miller [ 26/Dec/15 9:58 PM ]

Probably similar to CLJ-700.





[CLJ-1871] Add the ability to rename columns in clojure.pprint/print-table Created: 24/Dec/15  Updated: 24/Dec/15

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

Type: Enhancement Priority: Minor
Reporter: Aviad Reich Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: print

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

 Description   

I suggest adding the ability to rename columns in clojure.pprint/print-table, with the following interface:

(print-table [[:b "column b"] [:a "a"]]
                 [{:a 1 :b {:a 'is-a} :c ["hi" "there"]}
                  {:b 5 :a 7 :c "dog" :d -700}])

|  column b |  a |
|-----------+----|
| {:a is-a} |  1 |
|         5 |  7 |


 Comments   
Comment by Aviad Reich [ 24/Dec/15 12:28 AM ]

patch





[CLJ-1870] Reloading a defmulti nukes metadata on the var Created: 22/Dec/15  Updated: 18/Jan/16

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

Type: Defect Priority: Critical
Reporter: Nicola Mometto Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: ft, metadata, multimethods

Attachments: Text File 0001-CLJ-1870-don-t-destroy-defmulti-metadata-on-reload.patch    
Patch: Code
Approval: Prescreened

 Description   

Reloading a defmulti expression destroys any existing (or new) metadata on that multimethod's var:

user=> (defmulti foo "docstring" :op)
#'user/foo
user=> (-> #'foo meta :doc)
"docstring"
user=> (defmulti foo "docstring" :op)
nil
user=> (-> #'foo meta :doc)
nil

This is highly problematic for tools.analyzer, since it relies on such metadata to convey information to the pass scheduler about pass dependencies.

This means that any code that uses core.async cannot be reloaded using `require :reload-all`, since it will cause tools.analyzer to reload and the passes to scheduled in a random order. See ASYNC-154 for one example.

Cause: defmulti has defonce semantics and the first def does not re-apply meta.

Approach: Re-apply meta before first def.

After patch:

user=> (defmulti foo "docstring" :op)
#'user/foo
user=> (-> #'foo meta :doc)
"docstring"
user=> (defmulti foo "docstring" :op)
nil
user=> (-> #'foo meta :doc)
"docstring"

Patch: 0001-CLJ-1870-don-t-destroy-defmulti-metadata-on-reload.patch

Screened by: Alex Miller



 Comments   
Comment by Alex Miller [ 23/Dec/15 10:16 AM ]

Related: CLJ-900 CLJ-1446

Comment by Alex Miller [ 23/Dec/15 10:42 AM ]

This patch doesn't compile for me: RuntimeException: Unable to resolve symbol: mm in this context, compiling:(clojure/core.clj:1679:17)

Also, please build the patch with more context - use -U10 with git format-patch to expand it.

Comment by Nicola Mometto [ 23/Dec/15 11:06 AM ]

Updated patch fixing the typo & using -U10

Comment by Alex Miller [ 23/Dec/15 11:17 AM ]

Now:

java.lang.RuntimeException: Too many arguments to def, compiling:(clojure/core.clj:3561:1)

Comment by Nicola Mometto [ 23/Dec/15 11:22 AM ]

whoops. Sorry for this, here's the updated (and working) patch





[CLJ-1869] EdnReader allows keywords starting with number Created: 18/Dec/15  Updated: 18/Dec/15  Resolved: 18/Dec/15

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

Type: Defect Priority: Minor
Reporter: Greg Chapman Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None


 Description   

CLJ-1252 pointed out this problem with the LispReader, but the patch there was rejected because it broke too many things. The same problem exists with the EdnReader, but I would guess that patching it would not cause as much breakage. I suggest applying the CLJ-1252 patch to EdnReader.



 Comments   
Comment by Greg Chapman [ 18/Dec/15 2:35 PM ]

I apologize: for some reason I didn't notice that CLJ-1252 patched EdnReader as well as LispReader. So maybe changing EdnReader did cause breakage. If so, please ignore this report.

Comment by Alex Miller [ 18/Dec/15 2:53 PM ]

At this point, I believe we are likely to continue allowing keywords that start with a number. There is another ticket to sync up the spec with code (and actually it would be nice to fix the regex so it worked intentionally rather than accidentally).





[CLJ-1868] Unknown return type class throws NPE instead of useful exception Created: 17/Dec/15  Updated: 08/Jan/16  Resolved: 08/Jan/16

Status: Closed
Project: Clojure
Component/s: None
Affects Version/s: Release 1.8
Fix Version/s: Release 1.8

Type: Defect Priority: Major
Reporter: Alex Miller Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: compiler, errormsgs, regression

Attachments: Text File clj-1868.patch    
Patch: Code and Test
Approval: Ok

 Description   

This is a regression from CLJ-1232 - if you specify a return type class that is not fully-qualified or imported, you now get an NPE instead of a useful error message.

;; 1.7
CompilerException java.lang.IllegalArgumentException: Unable to resolve classname: Closeable, compiling:(NO_SOURCE_PATH:4:1)

;; 1.8
(defn foo ^Closeable [])
NullPointerException   clojure.core/sigs/resolve-tag--4375 (core.clj:247)

Cause: The new code that resolves classes does not handle the possible null return value of Compiler$HostExpr/maybeClass.

Solution: Check for null and fall back to the original argvecs, which will result in the original message.

Patch: clj-1868.patch



 Comments   
Comment by Nicola Mometto [ 17/Dec/15 5:10 PM ]

Dupe of CLJ-1863





[CLJ-1867] with-redefs used on a macro permanently changes it to a function Created: 10/Dec/15  Updated: 10/Dec/15

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

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

Approval: Triaged

 Description   

If you use with-redefs to redefine a macro (which is likely a mistake), the macro loses its macro status after the with-redefs call completes.

Presumably the fix depends on whether we think there is a valid use of with-redefs on a macro (which would only work if you're calling eval or equivalent in the body, and would require knowing enough about what you're doing to add the two extra macro args to your function) – if so, we would keep it from losing the macro status; if not, we might also have it throw an exception if you accidentally use it on a macro.

Demonstration of the effect:

user> (defmacro kwote [arg] `(quote ~arg))
#'user/kwote
user> (kwote hello)
hello
user> kwote
CompilerException java.lang.RuntimeException: Can't take value of a macro: #'user/kwote, compiling:(/tmp/form-init6222001939841513290.clj:1:18983)

;; Everything above is as expected

user> (with-redefs [kwote (constantly :in-with-redefs)] (kwote with-redefs-body))
with-redefs-body
user> (kwote hello)
CompilerException java.lang.RuntimeException: Unable to resolve symbol: hello in this context, compiling:(/tmp/form-init6222001939841513290.clj:1:1) 
user> (kwote :arg-1)
ArityException Wrong number of args (1) passed to: user/kwote  clojure.lang.AFn.throwArity (AFn.java:429)
user> (kwote :arg-1 :arg-2 :arg-3)
(quote :arg-3)
user> kwote
#object[user$kwote 0x37e32ff6 "user$kwote@37e32ff6"]


 Comments   
Comment by Gary Fredericks [ 10/Dec/15 12:04 PM ]

Looks like the root cause is that with-redefs uses Var#bindRoot which intentionally clears the macro flag: https://github.com/clojure/clojure/blob/5cfe5111ccb5afec4f9c73b46bba29ecab6a5899/src/jvm/clojure/lang/Var.java#L270





[CLJ-1866] Optimise argument boxing during reflection Created: 08/Dec/15  Updated: 11/Dec/15

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

Type: Enhancement Priority: Minor
Reporter: Mike Anderson Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: performance, reflection

Attachments: Text File clj-1866.patch    

 Description   

Currently argument boxing is clojure.lang.Reflector is inefficient for the following two reasons:
1. It makes an unnecessary call to Class.cast(..) when the parameter type is non-primitive
2. It allocates an unnecessary extra Object[] array when boxing arguments

This patch fixes these issues, without otherwise changing behaviour. All tests pass.



 Comments   
Comment by Alex Miller [ 09/Dec/15 7:23 AM ]

Example code where this is an issue?

Benchmark before/after ?

Comment by Mike Anderson [ 09/Dec/15 9:08 PM ]

Hi alex, I'm trying to improve the fast path for reflection, this will be a bunch of changes, together with clj-1784 and a few other ideas I have.

I don't think it will be productive to have an extensive debate / benchmarking over every single change. Would it be better to make a new ticket for all changes together with benchmarking for the overall impact?

Happy to do this, but it would be helpful if you could give an indication that this stuff will be accepted on the assumption that an overall improvement is demonstrated. I don't want to waste effort on Clojure dev if you guys are not interested in performance improvements in this space. And I don't have time to have an extensive debate over every individual change.

Comment by Alex Miller [ 10/Dec/15 7:51 AM ]

Each ticket should identify a specific problem and propose a solution with evidence that it helps. If there are multiple issues it is quite likely that they may move at different rates.

If you identify a hot spot, then we are happy to look at it. But we have to start from a problem to solve not just: "here are some changes". If the change makes things better, then you should be able to demonstrate that.

Comment by Alex Miller [ 10/Dec/15 7:53 AM ]

I would say that I am still dubious that the right answer to a problem with reflection is not just removing the reflection. But I can't evaluate that until you provide a problem.

Comment by Mike Anderson [ 10/Dec/15 7:10 PM ]

The problem is that it is inefficient (and therefore slow for users of reflection), as stated in the description. If you have an alternative way that you would like to see it worded, can you suggest?

Whether or not people should be using reflection is orthogonal to making reflection itself faster. I agree people should use type hints where they can but plenty of people don't have time to figure it out / don't know how to do properly / don't realise it is happening so surely any improvement for these cases should be welcome?

Also you haven't answered my question: would you like everything rolled into a single reflection performance improvement ticket, or not?

Comment by Alex Miller [ 11/Dec/15 10:11 AM ]

The title of this ticket is "Optimise argument boxing during reflection". That is a solution, not a problem. What I'm looking for is a title like "Reflection with boxed args is slow" and a description that starts with some example code demonstrating the problem. (That example code often makes for a particularly good template for a test that should be in the patch as well.)

I am then looking for evidence that the change you are suggesting improves the problem. For a performance issue, I am specifically looking for a before/after benchmark, preferably using a testing tool like criterium that gives me some confidence that the gains are real.

From a prioritization standpoint, I do not consider reflection performance to be a high priority because the best answer is probably: don't use reflection. That said, I'm willing to consider it, particularly if there is a compelling example where it may be difficult to remove the reflection or where it is particularly non-obvious that the reflection is happening.

Regarding your final question, we prefer to consider individual problems rather "a big bunch of changes", so separate would be better.





[CLJ-1865] Direct linking doesn't work on recursive calls Created: 08/Dec/15  Updated: 11/Dec/15

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

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


 Description   

It looks like self-recursive calls aren't optimized by direct linking, but if we redefine the same function twice, the Compiler is tricked into thinking that the call is not recursive and (rightfully) optimizes it into an invokeStatic.

I haven't investigated the cause but I suspect (and I might be wrong) it has to do with :arglist metadata potentially having different values when the Var is undefined vs when it's already bound.

[~]> cat test.clj
(ns test)

(defn a [x]
  (a x))
[~]> clj
Clojure 1.8.0-master-SNAPSHOT
user=> (compile 'test)
test
user=> ^D
[~]> cd classes
[~/classes]> javap -c test\$a
Compiled from "test.clj"
public final class test$a extends clojure.lang.AFunction {
  public static final clojure.lang.Var const__0;

  public static {};
    Code:
       0: ldc           #11                 // String test
       2: ldc           #13                 // String a
       4: invokestatic  #19                 // Method clojure/lang/RT.var:(Ljava/lang/String;Ljava/lang/String;)Lclojure/lang/Var;
       7: checkcast     #21                 // class clojure/lang/Var
      10: putstatic     #23                 // Field const__0:Lclojure/lang/Var;
      13: return

  public test$a();
    Code:
       0: aload_0
       1: invokespecial #26                 // Method clojure/lang/AFunction."<init>":()V
       4: return

  public static java.lang.Object invokeStatic(java.lang.Object);
    Code:
       0: getstatic     #23                 // Field const__0:Lclojure/lang/Var;
       3: invokevirtual #32                 // Method clojure/lang/Var.getRawRoot:()Ljava/lang/Object;
       6: checkcast     #34                 // class clojure/lang/IFn
       9: aload_0
      10: aconst_null
      11: astore_0
      12: invokeinterface #37,  2           // InterfaceMethod clojure/lang/IFn.invoke:(Ljava/lang/Object;)Ljava/lang/Object;
      17: areturn

  public java.lang.Object invoke(java.lang.Object);
    Code:
       0: aload_1
       1: aconst_null
       2: astore_1
       3: invokestatic  #41                 // Method invokeStatic:(Ljava/lang/Object;)Ljava/lang/Object;
       6: areturn
}

Redefining the same function twice makes it work.

[~]> cat test.clj
(ns test)

(defn a [x]
  (a x))

(defn a [x]
  (a x))
[~]> clj
Clojure 1.8.0-master-SNAPSHOT
user=> (compile 'test)
test
user=> ^D
[~]> cd classes
[~/classes]> javap -c test\$a
Compiled from "test.clj"
public final class test$a extends clojure.lang.AFunction {
  public static final clojure.lang.Var const__0;

  public static {};
    Code:
       0: ldc           #11                 // String test
       2: ldc           #13                 // String a
       4: invokestatic  #19                 // Method clojure/lang/RT.var:(Ljava/lang/String;Ljava/lang/String;)Lclojure/lang/Var;
       7: checkcast     #21                 // class clojure/lang/Var
      10: putstatic     #23                 // Field const__0:Lclojure/lang/Var;
      13: return

  public test$a();
    Code:
       0: aload_0
       1: invokespecial #26                 // Method clojure/lang/AFunction."<init>":()V
       4: return

  public static java.lang.Object invokeStatic(java.lang.Object);
    Code:
       0: aload_0
       1: aconst_null
       2: astore_0
       3: invokestatic  #30                 // Method invokeStatic:(Ljava/lang/Object;)Ljava/lang/Object;
       6: areturn

  public java.lang.Object invoke(java.lang.Object);
    Code:
       0: aload_1
       1: aconst_null
       2: astore_1
       3: invokestatic  #30                 // Method invokeStatic:(Ljava/lang/Object;)Ljava/lang/Object;
       6: areturn
}





[CLJ-1864] clojure.core/proxy does not work when reloading namespaces Created: 06/Dec/15  Updated: 08/Dec/15

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

Type: Defect Priority: Major
Reporter: Ralf Schmitt Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: protocols, proxy
Environment:

tested on 64 bit linux, oracle jdk 1.8


Attachments: Text File clj-1864.patch    
Patch: Code
Approval: Triaged

 Description   

clojure.core/proxy does not work when one reloads namespace containing defprotocol.

E.g. one can't reload the following file without triggering an error:

(ns foo.baz)

(defprotocol Hello
  (hello [this]))

(def hello-proxy
  (proxy [foo.baz.Hello] []
    (hello []
      (println "hello world"))))

(hello hello-proxy)

Saving the above as foo/baz.clj, I get the following error:

$ rlwrap java -cp target/clojure-1.8.0-master-SNAPSHOT.jar:. clojure.main
Clojure 1.8.0-master-SNAPSHOT
user=> (require 'foo.baz :reload)
hello world
nil
user=> (require 'foo.baz :reload)
CompilerException java.lang.IllegalArgumentException: No implementation of method: :hello of protocol: #'foo.baz/Hello found for class: foo.baz.proxy$java.lang.Object$Hello$6f95b989, compiling:(foo/baz.clj:11:1) 

I'm using the current git master (commit 5cfe5111ccb5afec4f9c73), but clojure 1.7 has the same problem.

The problem is that proxy-name only uses the interface names as a key. These names do not change when reloading the namespace, but the interfaces themself are new.

I'm going to attach a short patch which fixes that issue for me.



 Comments   
Comment by Ralf Schmitt [ 06/Dec/15 11:45 AM ]

I'm not sure how this interacts with AOT compilation.





[CLJ-1863] Bad type hints on a defn cause the compiler to throw a NPE Created: 04/Dec/15  Updated: 18/Dec/15

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

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


 Description   

After CLJ-1232 was committed to master, it is possible for the Clojure compiler to throw a NPE if a defn is type hinted with a invalid type. This surfaces in CLJS where the defn macro is re-used by the ClojureScript compiler, but I think it raises the question: "Should a bad type hint result in a compiler exception?"

The offending line can be found here on GitHub: https://github.com/clojure/clojure/blob/master/src/clj/clojure/core.clj#L247



 Comments   
Comment by Alex Miller [ 18/Dec/15 8:12 AM ]

This is basically the same as CLJ-1868, but I think what you are asking here is whether bad type hints should be ignored or throw any exception, right?

(Whereas CLJ-1868 is about which exception/message is thrown)

Comment by Timothy Baldridge [ 18/Dec/15 8:22 AM ]

Agreed. I think another possible solution would be to update CLJS to not use the CLJ defn, but I still think that a bad type hint should just be ignored.

Comment by Nicola Mometto [ 18/Dec/15 8:29 AM ]

I don't agree that we shoud ignore bad type hints.
If the compiler knows that something is wrong, it should tell the user immediately rather than silently ignoring and potentially failing at runtime later





[CLJ-1862] Release both a direct linked and a non direct linked clojure Created: 02/Dec/15  Updated: 02/Dec/15

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

Type: Task Priority: Major
Reporter: Nicola Mometto Assignee: Unassigned
Resolution: Unresolved Votes: 3
Labels: release


 Description   

Currently all new clojure releases will have the core library direct linked.
We should distribute both a direct linked and non direct linked alternatives, using a different classifier for the release.






[CLJ-1861] Remove unnecessary var interning Created: 02/Dec/15  Updated: 16/Dec/15  Resolved: 16/Dec/15

Status: Closed
Project: Clojure
Component/s: None
Affects Version/s: Release 1.8
Fix Version/s: Release 1.8

Type: Enhancement Priority: Major
Reporter: Alex Miller Assignee: Rich Hickey
Resolution: Completed Votes: 1
Labels: compiler

Approval: Vetted

 Description   

Remove unused var interns in static initializer (see https://groups.google.com/d/msg/clojure/731p1NBy2wk/lx98zp9oAAAJ ):

L1 {
             ldc "clojure.core" (java.lang.String)
             ldc "float?" (java.lang.String)
             invokestatic clojure/lang/RT var((Ljava/lang/String;Ljava/lang/String;)Lclojure/lang/Var;);
             checkcast clojure/lang/Var
             putstatic malt/utils$string_to_double.const__0:clojure.lang.Var
             ...
}


 Comments   
Comment by Nicola Mometto [ 02/Dec/15 9:49 AM ]

the compiler emits a lot of unused bytecode in its static initializer, not only vars.
The constant table is also full of unused numbers/keywords

Comment by Alex Miller [ 16/Dec/15 3:10 PM ]

resolved by Rich directly in https://github.com/clojure/clojure/commit/bfe14aec1c223abc3253358bac34b503284467d9

Comment by Alex Miller [ 16/Dec/15 3:33 PM ]

1.8.0-RC4





[CLJ-1860] 0.0 and -0.0 compare equal but have different hash values Created: 01/Dec/15  Updated: 12/Jan/16

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

Type: Defect Priority: Major
Reporter: Patrick O'Brien Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: math

Approval: Triaged

 Description   

0.0 and -0.0 compare as equal but have different hash values:

user=> (= 0.0 -0.0)
true
user=> (hash -0.0)
-2147483648
user=> (hash 0.0)
0

This causes problems as the equality/hashing assumption is violated.

user=> #{[1 2 0.0] [1 2 -0.0]}
#{[1 2 -0.0] [1 2 0.0]}

user=> (hash-map 0.0 1 -0.0 2)
{0.0 2}

user=> (hash-map [0.0] 1 [-0.0] 2)
{[0.0] 1, [-0.0] 2}

user=> (array-map [0.0] 1 [-0.0] 2)
{[0.0] 2}

user=> (hash-set [0.0] [-0.0])
#{[0.0] [-0.0]}

Cause: Apparently there is some lurking Java weirdness here. In Java, primitive positive and negative doubles are == but the boxed form is NOT .equals(). See also: http://docs.oracle.com/javase/7/docs/api/java/lang/Double.html#equals%28java.lang.Object%29

Double equality is checked with == in Clojure, which will report true. Hashing falls through to .hashCode(), which returns different values (but is consistent with the .equals() result on the boxed form).

Approach: There are two approaches - make 0.0 != -0.0, or make their hashes the same. Java takes the former approach in the boxed Double so that seems like what we should do.

Patch:






[CLJ-1859] Update parameter name to reflect docstring Created: 30/Nov/15  Updated: 30/Nov/15

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

Type: Enhancement Priority: Trivial
Reporter: Matthew Boston Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring

Attachments: Text File CLJ-1859-Update-parameter-name-to-reflect-docstring.patch    
Approval: Triaged

 Description   

The docstrings for `zero?`, `pos?`, and `neg?` reference `num` but the parameter is named `x`. This issue is to update the name of the parameter to `num` to reflect the docstring.



 Comments   
Comment by Alex Miller [ 30/Nov/15 1:14 PM ]

The inline fns should be updated too.

Comment by Matthew Boston [ 30/Nov/15 1:22 PM ]

Thanks, Alex. I was trying to follow the existing pattern that the inline functions have shorter parameter names. New patch attached.





[CLJ-1858] Transducer for partition-all with step Created: 28/Nov/15  Updated: 28/Nov/15

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

Type: Enhancement Priority: Minor
Reporter: Jeremy Apthorp Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: transducers

Patch: Code

 Description   

The docs for partition-all[1] mention that when a coll is not provided, it returns a transducer. This is true for the form (partition-all n), but not true for (partition-all n step). There's no clear way that I can see to combine transducers from core to produce this sort of "sliding window" transducer, i.e.

user=> (into [] (partition-all 2 1) [1 2 3 4 5])
((1 2) (2 3) (3 4) (4 5) (5))

Of course, there's an arity collision between the above hypothesized (partition-all n step) and the concrete, non-transducer-producing (partition-all n coll), which could be resolved by switching on the type of the second argument, or less hackily by providing the functionality in a separate function, e.g. (sliding window-size step-size), or perhaps by some other means.

I implemented this function with reference to the existing (partition-all n) transducer here: https://gist.github.com/nornagon/03b85fbc22b3613087f6

Would it make sense to work on getting something like this into core?

[1]: http://clojuredocs.org/clojure.core/partition-all






[CLJ-1857] clojure.string/split docstring does not match the behavior of parameter "limit" Created: 27/Nov/15  Updated: 29/Nov/15

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

Type: Enhancement Priority: Trivial
Reporter: Miikka Koskinen Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: string


 Description   
clojure.string/split
([s re] [s re limit])
  Splits string on a regular expression.  Optional argument limit is
  the maximum number of splits. Not lazy. Returns vector of the splits.

What happens is that limit is the maximum number of parts returned, not the number of splits done. If limit is 1, no splits are done, while I'd expect at most one split to be done. It's a bit of a matter of terminology, but I think that the text could be clarified. Based on ClojureDocs examples, I'm not the only one who was confused.

user=> (str/split "1 2 3" #" ")
["1" "2" "3"]
user=> (str/split "1 2 3" #" " 1)
["1 2 3"]
user=> (str/split "1 2 3" #" " 2)
["1" "2 3"]


 Comments   
Comment by Alex Miller [ 29/Nov/15 2:52 PM ]

To me, the last sentence indicates that "split" (as a noun) is being used to refer to the parts resulting from splitting but there is some ambiguity in the prior sentence. Would "parts" be better?

I don't get your point on the clojuredocs examples - those make sense to me.





[CLJ-1856] Direct-linking breaks clojure.test do-report stack-depth assumptions for failure reporting. Created: 24/Nov/15  Updated: 05/Dec/15  Resolved: 05/Dec/15

Status: Closed
Project: Clojure
Component/s: None
Affects Version/s: Release 1.8
Fix Version/s: Release 1.8

Type: Defect Priority: Major
Reporter: Gary Trakhman Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: regression, test

Attachments: Text File clj-1856.patch    
Patch: Code
Approval: Ok

 Description   

Test failure locations are being mis-reported (wrong class/line number).

Given a test ns:

(ns test1856.core-test
  (:require [clojure.test :refer :all]
            [test1856.core :refer :all]))

(deftest fail-test
  ;; 6
  ;; 7
  (is (= nil true)))  ;; 8

The results with 1.8-RC2 (with CLJ-1854 patch included) are:

FAIL in (fail-test) (test.clj:342)    ;; WRONG, expected: (core_test.clj:8)
expected: (= nil true)
  actual: (not (= nil true))

Cause: The location of the error is calculated in test.clj by constructing a Throwable in do-report and then dropping the top 1 frame (which is do-report itself) to find the user frame where the assertion failed. However, with direct linking there are now 2 frames on top of of the failure in user code, so the same (incorrect) location in test.clj is reported every time.

Approach: Change to a different strategy to filter the top frames based on content, not hard-coded depth. This strategy will work regardless of whether direct linking is involved or not.

1. Instead of constructing an exception and using it's stack trace, instead call Thread.getStackTrace() on the current thread. Create a new function that works on stack traces rather than exceptions and no longer needs a depth check.
2. Drop top frames while their class name starts with java.lang (this filters the call to java.lang.Thread.getStackTrace()) or clojure.test. The top frame will then be in the user's code.
3. Deprecated old file-and-line function (not sure if other clojure test reporting frameworks use this, despite it being private).
4. Updated tests that check that these functions work with an empty stack trace, as the JVM may elide it.

Patch: clj-1856.patch



 Comments   
Comment by Gary Trakhman [ 24/Nov/15 4:35 PM ]

'2' works in the standard case of direct-linked clojure with non-direct-linked app code. I'm exploring if there's a way to get the line info via macro-expansion of 'try-expr' and passing the line value into do-report for those cases.

Comment by Gary Trakhman [ 24/Nov/15 4:54 PM ]

I altered a local clojure build to print the stacktrace of the Throwable created by do-report, showing the additional invokeStatic frames during a repl session, showing the first user function frame is at offset 2.

user> (use 'clojure.test)
nil
user> (deftest a []
        (is false))
#'user/a
user> (run-tests)

Testing user

FAIL in (a) (test.clj:342)
expected: false
  actual: false

Ran 1 tests containing 1 assertions.
1 failures, 0 errors.
{:test 1, :pass 0, :fail 1, :error 0, :type :summary}


java.lang.Throwable
    at clojure.test$do_report.invokeStatic(test.clj:355)
    at clojure.test$do_report.invoke(test.clj:342)
    at user$fn__16867.invokeStatic(NO_SOURCE_FILE:74)
    at user$fn__16867.invoke(NO_SOURCE_FILE:73)
    at clojure.test$test_var$fn__7973.invoke(test.clj:706)
    at clojure.test$test_var.invokeStatic(test.clj:706)
    at clojure.test$test_var.invoke(test.clj:697)
    at clojure.test$test_vars$fn__7995$fn__8000.invoke(test.clj:724)
    at clojure.test$default_fixture.invokeStatic(test.clj:676)
    at clojure.test$default_fixture.invoke(test.clj:672)
    at clojure.test$test_vars$fn__7995.invoke(test.clj:724)
    at clojure.test$default_fixture.invokeStatic(test.clj:676)
    at clojure.test$default_fixture.invoke(test.clj:672)
    at clojure.test$test_vars.invokeStatic(test.clj:720)
    at clojure.test$test_all_vars.invokeStatic(test.clj:726)
    at clojure.test$test_ns.invokeStatic(test.clj:747)
    at clojure.test$test_ns.invoke(test.clj:732)
    at clojure.core$map$fn__4781.invoke(core.clj:2648)
    at clojure.lang.LazySeq.sval(LazySeq.java:40)
    at clojure.lang.LazySeq.seq(LazySeq.java:49)
    at clojure.lang.Cons.next(Cons.java:39)
    at clojure.lang.RT.boundedLength(RT.java:1749)
    at clojure.lang.RestFn.applyTo(RestFn.java:130)
    at clojure.core$apply.invokeStatic(core.clj:647)
    at clojure.test$run_tests.invokeStatic(test.clj:757)
    at clojure.test$run_tests.doInvoke(test.clj:757)
    at clojure.lang.RestFn.invoke(RestFn.java:408)
    at clojure.test$run_tests.invokeStatic(test.clj:762)
    at clojure.test$run_tests.invoke(test.clj:757)
    at user$eval16871.invokeStatic(NO_SOURCE_FILE:76)
    at user$eval16871.invoke(NO_SOURCE_FILE:76)
    at clojure.lang.Compiler.eval(Compiler.java:6915)
    at clojure.lang.Compiler.eval(Compiler.java:6878)
    at clojure.core$eval.invokeStatic(core.clj:3107)
    at clojure.core$eval.invoke(core.clj:3103)
    at clojure.main$repl$read_eval_print__7402$fn__7405.invoke(main.clj:240)
    at clojure.main$repl$read_eval_print__7402.invoke(main.clj:240)
    at clojure.main$repl$fn__7411.invoke(main.clj:258)
    at clojure.main$repl.invokeStatic(main.clj:258)
    at clojure.main$repl.doInvoke(main.clj:174)
    at clojure.lang.RestFn.invoke(RestFn.java:1523)
    at clojure.tools.nrepl.middleware.interruptible_eval$evaluate$fn__10251.invoke(interruptible_eval.clj:87)
    at clojure.lang.AFn.applyToHelper(AFn.java:152)
    at clojure.lang.AFn.applyTo(AFn.java:144)
    at clojure.core$apply.invokeStatic(core.clj:645)
    at clojure.core$with_bindings_STAR_.invokeStatic(core.clj:1883)
    at clojure.core$with_bindings_STAR_.doInvoke(core.clj:1883)
    at clojure.lang.RestFn.invoke(RestFn.java:425)
    at clojure.tools.nrepl.middleware.interruptible_eval$evaluate.invokeStatic(interruptible_eval.clj:85)
    at clojure.tools.nrepl.middleware.interruptible_eval$evaluate.invoke(interruptible_eval.clj:55)
    at clojure.tools.nrepl.middleware.interruptible_eval$interruptible_eval$fn__10296$fn__10299.invoke(interruptible_eval.clj:222)
    at clojure.tools.nrepl.middleware.interruptible_eval$run_next$fn__10291.invoke(interruptible_eval.clj:190)
    at clojure.lang.AFn.run(AFn.java:22)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
    at java.lang.Thread.run(Thread.java:745)

This is also with the patch from http://dev.clojure.org/jira/browse/CLJ-1854 applied

Comment by Alex Miller [ 25/Nov/15 1:26 PM ]
(ns test1856.core-test
  (:require [clojure.test :refer :all]
            [test1856.core :refer :all]))

(deftest throw-test
  ;; 6
  ;; 7
  (is (= nil (throw (Exception. "abc")))))  ;; 8

(deftest fail-test
  ;; 11
  ;; 12
  (is (= nil true)))  ;; 13

If I lein test (or run from repl), I see:

;; 1.7
FAIL in (fail-test) (core_test.clj:13)
expected: (= nil true)
  actual: (not (= nil true))

ERROR in (throw-test) (core_test.clj:8)
expected: (= nil (throw (Exception. "abc")))
  actual: java.lang.Exception: abc
 at test1856.core_test/fn (core_test.clj:8)
    clojure.test$test_var$fn__7670.invoke (test.clj:704)
    clojure.test$test_var.invoke (test.clj:704)
    clojure.test$test_vars$fn__7692$fn__7697.invoke (test.clj:722)
    ...
;; 1.8.0-RC2 + CLJ-1854 patch
FAIL in (fail-test) (test.clj:342)    ;; ERROR
expected: (= nil true)
  actual: (not (= nil true))

ERROR in (throw-test) (core_test.clj:8)
expected: (= nil (throw (Exception. "abc")))
  actual: java.lang.Exception: abc
 at test1856.core_test$fn__201.invokeStatic (core_test.clj:8)  ;; OK
    test1856.core_test/fn (core_test.clj:5)
    clojure.test$test_var$fn__7972.invoke (test.clj:703)
    clojure.test$test_var.invokeStatic (test.clj:703)




[CLJ-1855] Add a boolean? function Created: 24/Nov/15  Updated: 24/Nov/15  Resolved: 24/Nov/15

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

Type: Enhancement Priority: Minor
Reporter: Daniel Compton Assignee: Unassigned
Resolution: Duplicate Votes: 0
Labels: None


 Description   

It could be handy to have a boolean? function in core to match the checks for most other primitive Clojure types. Is this something that a patch would be considered for



 Comments   
Comment by Alex Miller [ 24/Nov/15 3:47 PM ]

Yes, although there is a bigger ticket (CLJ-1298) with suggestions for a number of type-related predicates. I would prefer to pass that list by Rich and have him yes/no things on it first prior to getting many individual patches.

boolean? is mentioned in the comments, but feel free to add it to the description to make that more prominent.

I'm going to dupe this to that one.

Comment by Daniel Compton [ 24/Nov/15 3:54 PM ]

Sorry about that. I searched for boolean? in JIRA and it didn't match any tickets so I thought this was a new request.





[CLJ-1854] Direct-linking changes lose line-number on invoke() Created: 24/Nov/15  Updated: 01/Dec/15  Resolved: 01/Dec/15

Status: Closed
Project: Clojure
Component/s: None
Affects Version/s: Release 1.8
Fix Version/s: Release 1.8

Type: Defect Priority: Major
Reporter: Gary Trakhman Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: compiler, regression
Environment:

Clojure 1.8RC2, leiningen 2.5.1


Attachments: Text File CLJ-1854-more-context.patch     Text File CLJ-1854.patch    
Patch: Code
Approval: Ok

 Description   
(ns foo)  ;; 1
;; 2
;; 3
;; 4
;; 5
;; 6
;; 7
(defn callstack []                       ;; 8
  [1 2 3]                                ;; 9
  (throw (Exception. "whoopsie!")))      ;; 10

Stack Trace comparison. Only the first two lines of each trace are relevant, the rest is all REPL fluff.

;;; 1.7.0
{:cause "whoopsie!",
 :via
 [{:type java.lang.Exception,
   :message "whoopsie!",
   :at [foo$callstack invoke "foo.clj" 8]}],
 :trace
 [[foo$callstack invoke "foo.clj" 8]
  [user$eval7675 invoke "form-init3342294504880003721.clj" 1]
  [clojure.lang.Compiler eval "Compiler.java" 6782]
  [clojure.lang.Compiler eval "Compiler.java" 6745]
	...

;;; 1.8 RC2
{:cause "whoopsie!",
 :via
 [{:type java.lang.Exception,
   :message "whoopsie!",
   :at [foo$callstack invokeStatic "foo.clj" 8]}],
 :trace
 [[foo$callstack invokeStatic "foo.clj" 8]
  [foo$callstack invoke "foo.clj" -1]    ;; Unexpected: -1
  [user$eval4 invokeStatic "NO_SOURCE_FILE" 28]
  [user$eval4 invoke "NO_SOURCE_FILE" -1]    ;; Unexpected: -1
  [clojure.lang.Compiler eval "Compiler.java" 6913]
  [clojure.lang.Compiler eval "Compiler.java" 6876]
	...

;;; 1.8 RC2 with patch
{:cause "whoopsie!",
 :via
 [{:type java.lang.Exception,
   :message "whoopsie!",
   :at [foo$callstack invokeStatic "foo.clj" 8]}],
 :trace
 [[foo$callstack invokeStatic "foo.clj" 8]
  [foo$callstack invoke "foo.clj" 8]    ;; Fixed
  [user$eval4 invokeStatic "NO_SOURCE_FILE" 3]
  [user$eval4 invoke "NO_SOURCE_FILE" 3]   ;; Fixed
  [clojure.lang.Compiler eval "Compiler.java" 6917]
  [clojure.lang.Compiler eval "Compiler.java" 6880]
	...

Cause: Non-direct linking now calls from invoke() through to invokeStatic(). In invoke(), Compiler does not visitLineNumber() before invoke() calls invokeStatic(), meaning that stack traces end up with -1 instead of a useful line number.

Patch: CLJ-1854-more-context.patch

Screened by: Alex Miller



 Comments   
Comment by Andy Fingerhut [ 24/Nov/15 1:18 PM ]

I have tested with Clojure 1.8.0-RC2 plus patch CLJ-1854.patch, and it does cause all of the -1 line numbers I have seen in stack traces to be filled in with actual source code line numbers.

For an example see this output after the patch: https://github.com/jafingerhut/st/blob/master/out-clj18-rc2-plus-clj1854-patch.txt

compared to this output with unmodified Clojure 1.8.0-RC2: https://github.com/jafingerhut/st/blob/master/out-clj18-rc2.txt

Comment by Alex Miller [ 24/Nov/15 1:30 PM ]

Ghadi, can you re-make the patch with more lines of diff context (use -U15 on format-patch)?

Comment by Ghadi Shayban [ 24/Nov/15 1:54 PM ]

np. -U15 wasn't enough, used -U30

Comment by Alex Miller [ 24/Nov/15 2:18 PM ]

Does it make sense for the two frames for the invoke and invokeStatic to refer to different line numbers in the source?

Comment by Ghadi Shayban [ 24/Nov/15 3:44 PM ]

Example has recursion through walk and is not minimal. Editing the ticket for reproducibility.

Comment by Gary Trakhman [ 24/Nov/15 3:47 PM ]

The current CLJ-1854-more-context.patch just gives me the same line number for all test cases, in my case it's test.clj:342 instead of -1 from before. I think perhaps clojure.test might need an adjustment as well, in particular the hardcoded '1' magic number here: https://github.com/clojure/clojure/blob/master/src/clj/clojure/test.clj#L351

test.clj:342 is do-report: https://github.com/clojure/clojure/blob/master/src/clj/clojure/test.clj#L342

Comment by Alex Miller [ 24/Nov/15 3:56 PM ]

Gary Trakhman I think that issue should be a separate ticket if so.

Comment by Gary Trakhman [ 24/Nov/15 4:03 PM ]

I will make a separate ticket for the potential clojure.test change.

Comment by Gary Trakhman [ 24/Nov/15 5:20 PM ]

Comparison of line numbers between 1.7 and 1.8 with patch here applied, clojure.test/do-report was modified to print stacktraces. It's weird that the numbers are different between parallel invoke/invokeStatic pairs.

diff --git a/src/clj/clojure/test.clj b/src/clj/clojure/test.clj
index 55e00f7..318ef20 100644
--- a/src/clj/clojure/test.clj
+++ b/src/clj/clojure/test.clj
@@ -349,7 +349,10 @@
   (report
    (case
     (:type m)
-    :fail (merge (file-and-line (new java.lang.Throwable) 1) m)
+     :fail (merge (file-and-line (doto (new java.lang.Throwable)
+                                   (.printStackTrace))
+                                 1)
+                  m)
     :error (merge (file-and-line (:actual m) 0) m) 
     m)))

1.7

gary@gary-dell:~/.m2/repository/org/clojure/clojure/1.7.0$ java -jar clojure-1.7.0.jar -r
Clojure 1.7.0
user=> (use 'clojure.test)
nil
user=> (deftest a []
(is false))
#'user/a
user=> (run-tests)

Testing user
java.lang.Throwable
    at clojure.test$do_report.invoke(test.clj:352)
    at user$fn__3.invoke(NO_SOURCE_FILE:3)
    at clojure.test$test_var$fn__7671.invoke(test.clj:707)
    at clojure.test$test_var.invoke(test.clj:707)
    at clojure.test$test_vars$fn__7693$fn__7698.invoke(test.clj:725)
    at clojure.test$default_fixture.invoke(test.clj:677)
    at clojure.test$test_vars$fn__7693.invoke(test.clj:725)
    at clojure.test$default_fixture.invoke(test.clj:677)
    at clojure.test$test_vars.invoke(test.clj:721)
    at clojure.test$test_all_vars.invoke(test.clj:731)
    at clojure.test$test_ns.invoke(test.clj:750)
    at clojure.core$map$fn__4553.invoke(core.clj:2624)
    at clojure.lang.LazySeq.sval(LazySeq.java:40)
    at clojure.lang.LazySeq.seq(LazySeq.java:49)
    at clojure.lang.Cons.next(Cons.java:39)
    at clojure.lang.RT.boundedLength(RT.java:1735)
    at clojure.lang.RestFn.applyTo(RestFn.java:130)
    at clojure.core$apply.invoke(core.clj:632)
    at clojure.test$run_tests.doInvoke(test.clj:765)
    at clojure.lang.RestFn.invoke(RestFn.java:408)
    at clojure.test$run_tests.invoke(test.clj:763)
    at user$eval7.invoke(NO_SOURCE_FILE:4)
    at clojure.lang.Compiler.eval(Compiler.java:6782)
    at clojure.lang.Compiler.eval(Compiler.java:6745)
    at clojure.core$eval.invoke(core.clj:3081)
    at clojure.main$repl$read_eval_print__7099$fn__7102.invoke(main.clj:240)
    at clojure.main$repl$read_eval_print__7099.invoke(main.clj:240)
    at clojure.main$repl$fn__7108.invoke(main.clj:258)
    at clojure.main$repl.doInvoke(main.clj:258)
    at clojure.lang.RestFn.invoke(RestFn.java:421)
    at clojure.main$repl_opt.invoke(main.clj:324)
    at clojure.main$main.doInvoke(main.clj:421)
    at clojure.lang.RestFn.invoke(RestFn.java:408)
    at clojure.lang.Var.invoke(Var.java:379)
    at clojure.lang.AFn.applyToHelper(AFn.java:154)
    at clojure.lang.Var.applyTo(Var.java:700)
    at clojure.main.main(main.java:37)

FAIL in (a) (NO_SOURCE_FILE:3)
expected: false
  actual: false

Ran 1 tests containing 1 assertions.
1 failures, 0 errors.
{:test 1, :pass 0, :fail 1, :error 0, :type :summary}

1.8 with patch here applied

gary@gary-dell:~/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT$ java -jar clojure-1.8.0-master-SNAPSHOT.jar -r
Clojure 1.8.0-master-SNAPSHOT
user=> (use 'clojure.test)
nil
user=> (deftest a []
(is false))
#'user/a
user=> (run-tests)

Testing user
java.lang.Throwable
    at clojure.test$do_report.invokeStatic(test.clj:355)
    at clojure.test$do_report.invoke(test.clj:342)
    at user$fn__3.invokeStatic(NO_SOURCE_FILE:3)
    at user$fn__3.invoke(NO_SOURCE_FILE:2)
    at clojure.test$test_var$fn__7973.invoke(test.clj:706)
    at clojure.test$test_var.invokeStatic(test.clj:706)
    at clojure.test$test_var.invoke(test.clj:697)
    at clojure.test$test_vars$fn__7995$fn__8000.invoke(test.clj:724)
    at clojure.test$default_fixture.invokeStatic(test.clj:676)
    at clojure.test$default_fixture.invoke(test.clj:672)
    at clojure.test$test_vars$fn__7995.invoke(test.clj:724)
    at clojure.test$default_fixture.invokeStatic(test.clj:676)
    at clojure.test$default_fixture.invoke(test.clj:672)
    at clojure.test$test_vars.invokeStatic(test.clj:720)
    at clojure.test$test_all_vars.invokeStatic(test.clj:726)
    at clojure.test$test_ns.invokeStatic(test.clj:747)
    at clojure.test$test_ns.invoke(test.clj:732)
    at clojure.core$map$fn__4781.invoke(core.clj:2648)
    at clojure.lang.LazySeq.sval(LazySeq.java:40)
    at clojure.lang.LazySeq.seq(LazySeq.java:49)
    at clojure.lang.Cons.next(Cons.java:39)
    at clojure.lang.RT.boundedLength(RT.java:1749)
    at clojure.lang.RestFn.applyTo(RestFn.java:130)
    at clojure.core$apply.invokeStatic(core.clj:647)
    at clojure.test$run_tests.invokeStatic(test.clj:757)
    at clojure.test$run_tests.doInvoke(test.clj:757)
    at clojure.lang.RestFn.invoke(RestFn.java:408)
    at clojure.test$run_tests.invokeStatic(test.clj:762)
    at clojure.test$run_tests.invoke(test.clj:757)
    at user$eval7.invokeStatic(NO_SOURCE_FILE:4)
    at user$eval7.invoke(NO_SOURCE_FILE:4)
    at clojure.lang.Compiler.eval(Compiler.java:6915)
    at clojure.lang.Compiler.eval(Compiler.java:6878)
    at clojure.core$eval.invokeStatic(core.clj:3107)
    at clojure.core$eval.invoke(core.clj:3103)
    at clojure.main$repl$read_eval_print__7402$fn__7405.invoke(main.clj:240)
    at clojure.main$repl$read_eval_print__7402.invoke(main.clj:240)
    at clojure.main$repl$fn__7411.invoke(main.clj:258)
    at clojure.main$repl.invokeStatic(main.clj:258)
    at clojure.main$repl_opt.invokeStatic(main.clj:322)
    at clojure.main$repl_opt.invoke(main.clj:318)
    at clojure.main$main.invokeStatic(main.clj:421)
    at clojure.main$main.doInvoke(main.clj:384)
    at clojure.lang.RestFn.invoke(RestFn.java:408)
    at clojure.lang.Var.invoke(Var.java:379)
    at clojure.lang.AFn.applyToHelper(AFn.java:154)
    at clojure.lang.Var.applyTo(Var.java:700)
    at clojure.main.main(main.java:37)

FAIL in (a) (test.clj:342)
expected: false
  actual: false

Ran 1 tests containing 1 assertions.
1 failures, 0 errors.
{:test 1, :pass 0, :fail 1, :error 0, :type :summary}
user=>




[CLJ-1853] Socket server can't use user-defined accept-fns Created: 24/Nov/15  Updated: 01/Dec/15  Resolved: 01/Dec/15

Status: Closed
Project: Clojure
Component/s: None
Affects Version/s: Release 1.8
Fix Version/s: Release 1.8

Type: Defect Priority: Major
Reporter: OHTA Shogo Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: server

Attachments: Text File 0001-CLJ-1853-Require-the-ns-before-resolving-the-accept-.patch    
Patch: Code
Approval: Ok

 Description   

In 1.8.0 RC2, if you start a socket server with a user-defined accept-fn like the following (clojure.test/testing-contexts-str is just a 0-arity fn used as an example accept fn here):

$ java -cp clojure-1.8.0-RC2.jar -Dclojure.server.foo='{:port 5555 :accept clojure.test/testing-contexts-str}' clojure.main

And then, if you connect to it with a command like "telnet 127.0.0.1 5555", you'll get an NPE.

Clojure 1.8.0-RC2
user=> Exception in thread "Clojure Connection repl 1" java.lang.NullPointerException
        at clojure.core$apply.invokeStatic(core.clj:645)
        at clojure.core.server$accept_connection.invokeStatic(server.clj:60)
        at clojure.core.server$start_server$fn__7327$fn__7328$fn__7330.invoke(server.clj:104)
        at clojure.lang.AFn.run(AFn.java:22)
        at java.lang.Thread.run(Thread.java:745)

This doesn't happen when starting the server with a pre-defined accept-fn, such as clojure.core.server/repl.

Cause: At the moment, clojure.core.server/accept-connection will require the namespace in which the accept-fn is defined after resolving the accept-fn. However, in order to resolve the accept-fn successfully, requiring the ns should be done prior to it.

Approach: Requiring the ns before resolving the accept-fn.

Patch: 0001-CLJ-1853-Require-the-ns-before-resolving-the-accept-.patch

Screened by: Alex Miller






[CLJ-1852] Clojure-generated class names length exceed file-system limit Created: 20/Nov/15  Updated: 18/Jan/16

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

Type: Enhancement Priority: Minor
Reporter: Martin Raison Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: compiler
Environment:

tested on CentOS 6



 Description   

Class names generated by the Clojure compiler can be arbitrarily long, exceeding the file system's maximum allowed file name length. For example it happens when you nest functions a bit too deeply:

(defmacro nestfn [n & body]
  (if (> n 0)
    `(fn [] (nestfn ~(- n 1) ~@body))
    body))

(def myf (nestfn 100 "body"))

Compiling this produces a java.io.IOException: File name too long exception.



 Comments   
Comment by Martin Raison [ 20/Nov/15 9:32 PM ]

The Scala community found this issue a while ago, and now the compiler has a max-classfile-name parameter (defaulting to 255). Hashing is used when the limit is exceeded. Maybe we should consider something similar?





[CLJ-1851] Add :redef key for vars to avoid being direct linked Created: 20/Nov/15  Updated: 05/Dec/15  Resolved: 05/Dec/15

Status: Closed
Project: Clojure
Component/s: None
Affects Version/s: Release 1.8
Fix Version/s: Release 1.8

Type: Enhancement Priority: Major
Reporter: Alex Miller Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File clj-1851.patch    
Patch: Code
Approval: Ok

 Description   

It is useful in some cases to indicate that calls to a var should never be direct linked. That is possible with ^:dynamic but that has additional semantics (and cost). Add a new ^:redef meta for vars that prevents direct invocations but does not have the ^:dynamic semantics.

From CLJ-1845, load was marked dynamic for this reason, now change to redef instead.

Patch: clj-1851.patch (also changes load to be :redef rather than :dynamic)






[CLJ-1850] doseq expansion causes boxed math warning. Created: 13/Nov/15  Updated: 13/Nov/15  Resolved: 13/Nov/15

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

Type: Defect Priority: Trivial
Reporter: Michael Nygard Assignee: Unassigned
Resolution: Not Reproducible Votes: 0
Labels: math


 Description   

When boxed math warnings are enabled, doseq causes a warning.



 Comments   
Comment by Alex Miller [ 13/Nov/15 2:27 PM ]

Example?

Comment by Michael Nygard [ 13/Nov/15 2:58 PM ]

Working on isolating a minimal example.

Comment by Michael Nygard [ 13/Nov/15 4:46 PM ]

With this source:

src/doseq_warning/core.clj
(ns doseq-warning.core
  (:require [clojure.core.async :as async]))

(set! *unchecked-math* :warn-on-boxed)

(defn example
  []
  (async/go-loop [actives []]
    (let [vch (async/alts! actives)]
      (doseq [c (second vch)]
        (async/close! c)))))

Using the following project.clj:

project.clj
(defproject doseq-warning "0.1.0-SNAPSHOT"
  :dependencies [[org.clojure/clojure "1.8.0-beta1"]
                 [org.clojure/core.async "0.1.346.0-17112a-alpha"]]
  :global-vars {*unchecked-math* :warn-on-boxed})

Then executing (load-file "src/doseq_warning/core.clj") at a REPL results in:

Boxed math warning, /Users/mtnygard/tmp/doseq-warning/src/doseq_warning/core.clj:8:3 - call: public static boolean clojure.lang.Numbers.lt(java.lang.Object,java.lang.Object). 
Boxed math warning, /Users/mtnygard/tmp/doseq-warning/src/doseq_warning/core.clj:8:3 - call: public static java.lang.Number clojure.lang.Numbers.unchecked_inc(java.lang.Object). 
#'doseq-warning.core/example
Comment by Alex Miller [ 13/Nov/15 4:49 PM ]

I think it's probably unlikely that this is an error with the boxed math warnings and more likely an artifact of using an older core.async with older tools.analyzer in the go block transformation.

Not reproducible with latest core.async, so closing.





[CLJ-1849] Add tests for CLJ-1846 and CLJ-1825 Created: 13/Nov/15  Updated: 16/Nov/15  Resolved: 16/Nov/15

Status: Closed
Project: Clojure
Component/s: None
Affects Version/s: Release 1.8
Fix Version/s: Release 1.8

Type: Enhancement Priority: Major
Reporter: Alex Miller Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File clj-1849.patch    
Patch: Code and Test
Approval: Ok

 Description   

Add tests to reproduce CLJ-1846 and CLJ-1825 errors for future testing.






[CLJ-1848] update! for transients Created: 13/Nov/15  Updated: 13/Nov/15

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

Type: Enhancement Priority: Minor
Reporter: A. R Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

-



 Description   

Now that we have `update` we should possibly also think of having `update!` for transients for consistency.

Thoughts?






[CLJ-1847] clojure.walk/walk returns a PersistentVector when the input is of type IMapEntry Created: 13/Nov/15  Updated: 15/Nov/15  Resolved: 15/Nov/15

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

Type: Defect Priority: Minor
Reporter: Yehonathan Sharvit Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: walk

Attachments: Text File fix_walk.patch    
Patch: Code

 Description   

`walk` is supposed to build up a data structure of the same type as its input.
With `clojure.lang.IMapEntry`, it doesn't work as expected.
The output is of type: `clojure.lang.PersisentVector`

For instance,
(class (walk/walk identity identity (first {:a 1})) ); clojure.lang.PersisentVector



 Comments   
Comment by Alex Miller [ 15/Nov/15 12:54 PM ]

Thanks for filing this. I spent some time looking at it and I don't think there is anything that needs to be done at this time.

Re your description, "`walk` is supposed to build up a data structure of the same type as its input." Regarding IMapEntry.... prior to 1.8, IMapEntry concrete types were descendants of AMapEntry, in particular the most common concrete type was MapEntry, which were used in PersistentHashMap.

In 1.8, PHM now returns PersistentVectors, which now implement IMapEntry. So, walk now takes a map entry (which is a PV) and returns a PV, which is an IMapEntry. So I believe the contract is still satisfied.

The new map-entry? predicate can be used to catch only 2-element PVs (not other counts) as map entries, however it is important to still consider whether you will incorrectly snare 2 element vectors that aren't in a map in this particular case. The current clojure.walk code however will have the same effective result in either case so the clojure.walk code does not need to change (and in fact non-PV entries still exist in sorted maps).





[CLJ-1846] VerifyError in Clojure 1.8.0-(beta1..RC1) Created: 11/Nov/15  Updated: 11/Nov/15  Resolved: 11/Nov/15

Status: Closed
Project: Clojure
Component/s: None
Affects Version/s: Release 1.8
Fix Version/s: Release 1.8

Type: Defect Priority: Major
Reporter: Shantanu Kumar Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: compiler, regression
Environment:

Oracle JDK 1.7, Oracle JDK 1.8 on Mac OS X


Approval: Vetted

 Description   

Nicola Mometto provided the below minimal repro case:

user=> (defn foo ^long [] 1)
#'user/foo
user=> (Integer/bitCount ^int (foo))
VerifyError (class: user$eval13, method: invokeStatic signature: ()Ljava/lang/Object;) Expecting to find integer on stack  java.lang.Class.getDeclaredConstructors0 (Class.java:-2)

Full stack trace as found with https://github.com/kumarshantanu/asphalt:

$ lein do clean, with-profile dev,c18 test
Exception in thread "main" java.lang.VerifyError: (class: asphalt/core$invoke_with_transaction, method: invokeStatic signature: (Ljava/lang/Object;Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;) Expecting to find integer on stack, compiling:(core.clj:201:1)
	at clojure.lang.Compiler$DefExpr.eval(Compiler.java:463)
	at clojure.lang.Compiler.eval(Compiler.java:6918)
	at clojure.lang.Compiler.load(Compiler.java:7360)
	at clojure.lang.RT.loadResourceScript(RT.java:372)
	at clojure.lang.RT.loadResourceScript(RT.java:363)
	at clojure.lang.RT.load(RT.java:453)
	at clojure.lang.RT.load(RT.java:419)
	at clojure.core$load$fn__5673.invoke(core.clj:5895)
	at clojure.core$load.invokeStatic(core.clj:5894)
	at clojure.core$load_one.invokeStatic(core.clj:5694)
	at clojure.core$load_one.invoke(core.clj)
	at clojure.core$load_lib$fn__5622.invoke(core.clj:5739)
	at clojure.core$load_lib.invokeStatic(core.clj:5738)
	at clojure.core$load_lib.doInvoke(core.clj)
	at clojure.lang.RestFn.applyTo(RestFn.java:142)
	at clojure.core$apply.invokeStatic(core.clj:647)
	at clojure.core$load_libs.invokeStatic(core.clj:5776)
	at clojure.core$load_libs.doInvoke(core.clj)
	at clojure.lang.RestFn.applyTo(RestFn.java:137)
	at clojure.core$apply.invokeStatic(core.clj:647)
	at clojure.core$require.invokeStatic(core.clj:5798)
	at clojure.core$require.doInvoke(core.clj)
	at clojure.lang.RestFn.invoke(RestFn.java:457)
	at asphalt.test_util$eval198$loading__5565__auto____199.invoke(test_util.clj:1)
	at asphalt.test_util$eval198.invokeStatic(test_util.clj:1)
	at asphalt.test_util$eval198.invoke(test_util.clj)
	at clojure.lang.Compiler.eval(Compiler.java:6913)
	at clojure.lang.Compiler.eval(Compiler.java:6902)
	at clojure.lang.Compiler.load(Compiler.java:7360)
	at clojure.lang.RT.loadResourceScript(RT.java:372)
	at clojure.lang.RT.loadResourceScript(RT.java:363)
	at clojure.lang.RT.load(RT.java:453)
	at clojure.lang.RT.load(RT.java:419)
	at clojure.core$load$fn__5673.invoke(core.clj:5895)
	at clojure.core$load.invokeStatic(core.clj:5894)
	at clojure.core$load_one.invokeStatic(core.clj:5694)
	at clojure.core$load_one.invoke(core.clj)
	at clojure.core$load_lib$fn__5622.invoke(core.clj:5739)
	at clojure.core$load_lib.invokeStatic(core.clj:5738)
	at clojure.core$load_lib.doInvoke(core.clj)
	at clojure.lang.RestFn.applyTo(RestFn.java:142)
	at clojure.core$apply.invokeStatic(core.clj:647)
	at clojure.core$load_libs.invokeStatic(core.clj:5776)
	at clojure.core$load_libs.doInvoke(core.clj)
	at clojure.lang.RestFn.applyTo(RestFn.java:137)
	at clojure.core$apply.invokeStatic(core.clj:647)
	at clojure.core$require.invokeStatic(core.clj:5798)
	at clojure.core$require.doInvoke(core.clj)
	at clojure.lang.RestFn.invoke(RestFn.java:457)
	at asphalt.core_test$eval192$loading__5565__auto____193.invoke(core_test.clj:1)
	at asphalt.core_test$eval192.invokeStatic(core_test.clj:1)
	at asphalt.core_test$eval192.invoke(core_test.clj)
	at clojure.lang.Compiler.eval(Compiler.java:6913)
	at clojure.lang.Compiler.eval(Compiler.java:6902)
	at clojure.lang.Compiler.load(Compiler.java:7360)
	at clojure.lang.RT.loadResourceScript(RT.java:372)
	at clojure.lang.RT.loadResourceScript(RT.java:363)
	at clojure.lang.RT.load(RT.java:453)
	at clojure.lang.RT.load(RT.java:419)
	at clojure.core$load$fn__5673.invoke(core.clj:5895)
	at clojure.core$load.invokeStatic(core.clj:5894)
	at clojure.core$load_one.invokeStatic(core.clj:5694)
	at clojure.core$load_one.invoke(core.clj)
	at clojure.core$load_lib$fn__5622.invoke(core.clj:5739)
	at clojure.core$load_lib.invokeStatic(core.clj:5738)
	at clojure.core$load_lib.doInvoke(core.clj)
	at clojure.lang.RestFn.applyTo(RestFn.java:142)
	at clojure.core$apply.invokeStatic(core.clj:647)
	at clojure.core$load_libs.invokeStatic(core.clj:5776)
	at clojure.core$load_libs.doInvoke(core.clj)
	at clojure.lang.RestFn.applyTo(RestFn.java:137)
	at clojure.core$apply.invokeStatic(core.clj:647)
	at clojure.core$require.invokeStatic(core.clj:5798)
	at clojure.core$require.doInvoke(core.clj)
	at clojure.lang.RestFn.applyTo(RestFn.java:137)
	at clojure.core$apply.invokeStatic(core.clj:647)
	at clojure.core$apply.invoke(core.clj)
	at user$eval91.invokeStatic(form-init7505432955041312280.clj:1)
	at user$eval91.invoke(form-init7505432955041312280.clj)
	at clojure.lang.Compiler.eval(Compiler.java:6913)
	at clojure.lang.Compiler.eval(Compiler.java:6903)
	at clojure.lang.Compiler.load(Compiler.java:7360)
	at clojure.lang.Compiler.loadFile(Compiler.java:7298)
	at clojure.main$load_script.invokeStatic(main.clj:275)
	at clojure.main$init_opt.invokeStatic(main.clj:277)
	at clojure.main$init_opt.invoke(main.clj)
	at clojure.main$initialize.invokeStatic(main.clj:308)
	at clojure.main$null_opt.invokeStatic(main.clj:342)
	at clojure.main$null_opt.invoke(main.clj)
	at clojure.main$main.invokeStatic(main.clj:421)
	at clojure.main$main.doInvoke(main.clj)
	at clojure.lang.RestFn.invoke(RestFn.java:421)
	at clojure.lang.Var.invoke(Var.java:383)
	at clojure.lang.AFn.applyToHelper(AFn.java:156)
	at clojure.lang.Var.applyTo(Var.java:700)
	at clojure.main.main(main.java:37)
Caused by: java.lang.VerifyError: (class: asphalt/core$invoke_with_transaction, method: invokeStatic signature: (Ljava/lang/Object;Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;) Expecting to find integer on stack
	at java.lang.Class.getDeclaredConstructors0(Native Method)
	at java.lang.Class.privateGetDeclaredConstructors(Class.java:2671)
	at java.lang.Class.getConstructor0(Class.java:3075)
	at java.lang.Class.newInstance(Class.java:412)
	at clojure.lang.Compiler$ObjExpr.eval(Compiler.java:4902)
	at clojure.lang.Compiler$DefExpr.eval(Compiler.java:450)
	... 95 more


 Comments   
Comment by Nicola Mometto [ 11/Nov/15 8:23 AM ]

Copying a comment I posted on the ML regarding this bug:

To be honest I'm not sure this should even be a valid use of type hints.
You're telling the compiler that the result of (foo) is an int, when it is infact a long.

The correct way to do this should be:

(Integer/bitCount (int (foo))

Again, lack of specification on what the correct type hinting semantics should be make it hard to evaluate if this should be considered a bug or just an user error that previously just got ignored.

Comment by Alex Miller [ 11/Nov/15 3:18 PM ]

The example Nicola gave in the description worked in 1.6 and 1.7 and 1.8 up to 1.8.0-alpha2. It started failing as of https://github.com/clojure/clojure/commit/8c9580cb6706f2dc40bb31bbdb96a6aefe341bd5 for CLJ-1533.

Comment by Alex Miller [ 11/Nov/15 3:20 PM ]

Rich pushed a new commit https://github.com/clojure/clojure/commit/9448d627e091bc010e68e05a5669c134cd715a98 for this in master.

Comment by Alex Miller [ 11/Nov/15 3:20 PM ]

The commit makes this kind of incorrect type hint (previously a no op) now a compile error.

Comment by Shantanu Kumar [ 11/Nov/15 8:59 PM ]

I tested with the latest master and it correctly reports the "Caused by: java.lang.UnsupportedOperationException: Cannot coerce long to int, use a cast instead" error now. However, the reported line number in the exception is that of the defn (first line of the fn) instead of where the coercion was attempted in the fn body.





[CLJ-1845] Allow load to be redefined Created: 10/Nov/15  Updated: 05/Dec/15  Resolved: 05/Dec/15

Status: Closed
Project: Clojure
Component/s: None
Affects Version/s: Release 1.8
Fix Version/s: Release 1.8

Type: Enhancement Priority: Major
Reporter: Alex Miller Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File clj-1845.patch     Text File clj-1845-test.patch     Text File ctyp1845-direct-linking-test.patch    
Patch: Code
Approval: Ok

 Description   

With direct linking of core, we have lost the ability to easily redef load (as calls to load inside Clojure are direct linked).

Proposed: make load dynamic (dynamic vars are not direct linked)

Patch: clj-1845.patch
See: https://groups.google.com/d/msg/clojure/_AGdLHSg41Q/q8LeplkrBQAJ

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

Re-opened because the initial declare of load is not declared as ^:dynamic and thus functions that use load between the initial forward declare and the later actual declaration were still allowing direct linking.

Because we are adding ^:redef, I rolled the changes into CLJ-1851 instead. The only thing here is a new test (which will fail till CLJ-1851 is applied).

Test patch: clj-1845-test.patch (NEW)
See also: CLJ-1851
Screened by: Alex Miller



 Comments   
Comment by Alex Miller [ 20/Nov/15 9:18 AM ]

Reopening...

Comment by Alex Miller [ 20/Nov/15 9:19 AM ]

Test (that doesn't work):

user=> (alter-var-root #'load (fn [f] (fn [& args] (prn "patched") (apply f args))))
#object[user$eval1241$fn__1242$fn__1243 0x1c857e6 "user$eval1241$fn__1242$fn__1243@1c857e6"]
user=> (load)
"patched"
nil
user=> (require 'clojure.core :reload)   ;; expect to see "patched"
nil
user=> (require 'clojure.tools.analyzer :reload)   ;; expect to see "patched"
nil
user=> (require 'clojure.tools.analyzer :reload-all)   ;; expect to see "patched"
nil
Comment by Alex Miller [ 20/Nov/15 9:20 AM ]

The issue is that load is forward-declared and the forward declaration does not declare dynamic. Replacing (declare load) with (def ^:declared ^:dynamic load) will fix it.

Comment by Ambrose Bonnaire-Sergeant [ 20/Nov/15 9:47 AM ]

Interested in a patch with a test?

Comment by Ambrose Bonnaire-Sergeant [ 20/Nov/15 9:52 AM ]

Confirmed that (declare ^:dynamic load) fixes the problem

Comment by Alex Miller [ 20/Nov/15 9:55 AM ]

No patch - this interacts with another change and I may just roll them together.

Comment by Alex Miller [ 20/Nov/15 9:56 AM ]

Actually, if you wanted to make a patch for a test, that would be useful.

Comment by Ambrose Bonnaire-Sergeant [ 20/Nov/15 10:12 AM ]

Attached direct linking test.

Comment by Alex Miller [ 20/Nov/15 11:00 AM ]

New test patch that applies to master

Comment by Andy Fingerhut [ 05/Dec/15 3:13 PM ]

It appears that CLJ-1845, CLJ-1851, and CLJ-1856 are committed now, and can be closed as complete?





[CLJ-1843] Add =to function exposing Util/equivPred Created: 06/Nov/15  Updated: 18/Dec/15

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

Attachments: Text File 0001-CLJ-1843-add-to-for-faster-equality-check-against-kn.patch    
Patch: Code
Approval: Triaged

 Description   

Description:
It is sometimes useful to compare a collection of values against one value, clojure internally defines a predicate for this exact purpose which has some nice performance improvements over just a partial applied =.

Prior discussion with Rich: https://groups.google.com/forum/#!topic/clojure-dev/0c-VNhEKVkI

Example usage:

;;before:
(map (partial = 3) coll)
;;after:
(map (=to 3) coll)

Benchmarks:

test (partial = 'foo) #(= 'foo %) (=to 'foo)
small homogeneous coll 217ns 165ns 39ns
small eterogeneous coll, 192ns 167ns 41ns
big homogeneous coll 66us 52us 8us
big eterogeneous coll 82us 66us 27us

Full benchmarks output:

(use 'criterium.core)

(defn benchmark-f [f]
  (let [colls [['foo 'foo 'foo]
               [1 :foo 'foo]
               (doall (repeat 1e3 'foo))
               (doall (take 1e3 (cycle [1 :foo 'foo])))]]
    (doseq [c colls]
      (quick-bench (run! f c)))))

(benchmark-f (partial = 'foo))
ARNING: Final GC required 1.405293826432628 % of runtime
WARNING: Final GC required 10.202923149112559 % of runtime
Evaluation count : 3116130 in 6 samples of 519355 calls.
Execution time mean : 217.723199 ns
Execution time std-deviation : 29.425291 ns
Execution time lower quantile : 189.944710 ns ( 2.5%)
Execution time upper quantile : 261.717351 ns (97.5%)
Overhead used : 1.863362 ns
WARNING: Final GC required 4.2579397621583315 % of runtime
Evaluation count : 3138636 in 6 samples of 523106 calls.
Execution time mean : 198.985418 ns
Execution time std-deviation : 12.691848 ns
Execution time lower quantile : 182.441245 ns ( 2.5%)
Execution time upper quantile : 207.839280 ns (97.5%)
Overhead used : 1.863362 ns
WARNING: Final GC required 6.631646134523004 % of runtime
Evaluation count : 10038 in 6 samples of 1673 calls.
Execution time mean : 66.977712 µs
Execution time std-deviation : 10.411821 µs
Execution time lower quantile : 59.620690 µs ( 2.5%)
Execution time upper quantile : 84.483254 µs (97.5%)
Overhead used : 1.863362 ns

Found 1 outliers in 6 samples (16.6667 %)
low-severe  1 (16.6667 %)
Variance from outliers : 47.3059 % Variance is moderately inflated by outliers
WARNING: Final GC required 5.272721959665122 % of runtime
Evaluation count : 7908 in 6 samples of 1318 calls.
Execution time mean : 82.588512 µs
Execution time std-deviation : 5.215537 µs
Execution time lower quantile : 75.977936 µs ( 2.5%)
Execution time upper quantile : 86.849982 µs (97.5%)
Overhead used : 1.863362 ns


(benchmark-f #(= 'foo %))
WARNING: Final GC required 1.284421364203217 % of runtime
WARNING: Final GC required 10.04376144830405 % of runtime
Evaluation count : 3643032 in 6 samples of 607172 calls.
             Execution time mean : 165.393131 ns
    Execution time std-deviation : 1.041355 ns
   Execution time lower quantile : 164.277060 ns ( 2.5%)
   Execution time upper quantile : 166.849951 ns (97.5%)
                   Overhead used : 1.605524 ns
WARNING: Final GC required 6.258680973295933 % of runtime
Evaluation count : 3584574 in 6 samples of 597429 calls.
             Execution time mean : 167.659014 ns
    Execution time std-deviation : 3.821817 ns
   Execution time lower quantile : 164.175156 ns ( 2.5%)
   Execution time upper quantile : 173.210781 ns (97.5%)
                   Overhead used : 1.605524 ns

Found 1 outliers in 6 samples (16.6667 %)
	low-severe	 1 (16.6667 %)
 Variance from outliers : 13.8889 % Variance is moderately inflated by outliers
WARNING: Final GC required 6.914389197005716 % of runtime
Evaluation count : 11196 in 6 samples of 1866 calls.
             Execution time mean : 52.593759 µs
    Execution time std-deviation : 834.220092 ns



(benchmark-f (=to 'foo))
WARNING: Final GC required 7.40391654943877 % of runtime
Evaluation count : 15169068 in 6 samples of 2528178 calls.
Execution time mean : 39.937424 ns
Execution time std-deviation : 2.782661 ns
Execution time lower quantile : 37.393937 ns ( 2.5%)
Execution time upper quantile : 42.780432 ns (97.5%)
Overhead used : 1.863362 ns
WARNING: Final GC required 5.986859953402835 % of runtime
Evaluation count : 15199992 in 6 samples of 2533332 calls.
Execution time mean : 41.229082 ns
Execution time std-deviation : 2.815533 ns
Execution time lower quantile : 37.371527 ns ( 2.5%)
Execution time upper quantile : 43.208673 ns (97.5%)
Overhead used : 1.863362 ns
WARNING: Final GC required 5.039484046472016 % of runtime
Evaluation count : 69462 in 6 samples of 11577 calls.
Execution time mean : 8.976972 µs
Execution time std-deviation : 587.089991 ns
Execution time lower quantile : 8.505317 µs ( 2.5%)
Execution time upper quantile : 9.744296 µs (97.5%)
Overhead used : 1.863362 ns
WARNING: Final GC required 5.773010947849351 % of runtime
Evaluation count : 23352 in 6 samples of 3892 calls.
Execution time mean : 27.277376 µs
Execution time std-deviation : 2.115666 µs
Execution time lower quantile : 25.719322 µs ( 2.5%)
Execution time upper quantile : 30.123547 µs (97.5%)
Overhead used : 1.863362 ns

Patch: 0001-CLJ-1843-add-to-for-faster-equality-check-against-kn.patch



 Comments   
Comment by Alex Miller [ 06/Nov/15 9:15 AM ]

Did you look at (apply = 3 coll) ? Just curious.

Comment by Nicola Mometto [ 06/Nov/15 9:19 AM ]

The advantage of Util/equivPred over Util/equiv is that it can assume the type of the provided argument, avoiding the runtime cost of doing the multiple instance checks that Util/equiv has to do to figure out what comparator to use internally

Comment by Alex Miller [ 06/Nov/15 10:08 AM ]

Could you quantify the difference between these approaches on 2-3 collection sizes?

Comment by Nicola Mometto [ 06/Nov/15 2:53 PM ]

With the following setup:

(use 'criterium.core)

(defn =to [x]
  (let [pred (clojure.lang.Util/equivPred x)]
    (fn [y]
      (.equiv pred x y))))

(defn benchmark-f [f]
  (let [colls [['foo 'foo 'foo]
               [1 :foo 'foo]
               (doall (repeat 1e3 'foo))
               (doall (take 1e3 (cycle [1 :foo 'foo])))]]
    (doseq [c colls]
      (quick-bench (run! f c)))))

The results for (benchmark-f (partial = 'foo) are:

WARNING: Final GC required 1.405293826432628 % of runtime
WARNING: Final GC required 10.202923149112559 % of runtime
Evaluation count : 3116130 in 6 samples of 519355 calls.
Execution time mean : 217.723199 ns
Execution time std-deviation : 29.425291 ns
Execution time lower quantile : 189.944710 ns ( 2.5%)
Execution time upper quantile : 261.717351 ns (97.5%)
Overhead used : 1.863362 ns
WARNING: Final GC required 4.2579397621583315 % of runtime
Evaluation count : 3138636 in 6 samples of 523106 calls.
Execution time mean : 198.985418 ns
Execution time std-deviation : 12.691848 ns
Execution time lower quantile : 182.441245 ns ( 2.5%)
Execution time upper quantile : 207.839280 ns (97.5%)
Overhead used : 1.863362 ns
WARNING: Final GC required 6.631646134523004 % of runtime
Evaluation count : 10038 in 6 samples of 1673 calls.
Execution time mean : 66.977712 µs
Execution time std-deviation : 10.411821 µs
Execution time lower quantile : 59.620690 µs ( 2.5%)
Execution time upper quantile : 84.483254 µs (97.5%)
Overhead used : 1.863362 ns

Found 1 outliers in 6 samples (16.6667 %)
low-severe  1 (16.6667 %)
Variance from outliers : 47.3059 % Variance is moderately inflated by outliers
WARNING: Final GC required 5.272721959665122 % of runtime
Evaluation count : 7908 in 6 samples of 1318 calls.
Execution time mean : 82.588512 µs
Execution time std-deviation : 5.215537 µs
Execution time lower quantile : 75.977936 µs ( 2.5%)
Execution time upper quantile : 86.849982 µs (97.5%)
Overhead used : 1.863362 ns

The results fore (benchmark-f (=to 'foo)) are:

WARNING: Final GC required 7.40391654943877 % of runtime
Evaluation count : 15169068 in 6 samples of 2528178 calls.
Execution time mean : 39.937424 ns
Execution time std-deviation : 2.782661 ns
Execution time lower quantile : 37.393937 ns ( 2.5%)
Execution time upper quantile : 42.780432 ns (97.5%)
Overhead used : 1.863362 ns
WARNING: Final GC required 5.986859953402835 % of runtime
Evaluation count : 15199992 in 6 samples of 2533332 calls.
Execution time mean : 41.229082 ns
Execution time std-deviation : 2.815533 ns
Execution time lower quantile : 37.371527 ns ( 2.5%)
Execution time upper quantile : 43.208673 ns (97.5%)
Overhead used : 1.863362 ns
WARNING: Final GC required 5.039484046472016 % of runtime
Evaluation count : 69462 in 6 samples of 11577 calls.
Execution time mean : 8.976972 µs
Execution time std-deviation : 587.089991 ns
Execution time lower quantile : 8.505317 µs ( 2.5%)
Execution time upper quantile : 9.744296 µs (97.5%)
Overhead used : 1.863362 ns
WARNING: Final GC required 5.773010947849351 % of runtime
Evaluation count : 23352 in 6 samples of 3892 calls.
Execution time mean : 27.277376 µs
Execution time std-deviation : 2.115666 µs
Execution time lower quantile : 25.719322 µs ( 2.5%)
Execution time upper quantile : 30.123547 µs (97.5%)
Overhead used : 1.863362 ns
Comment by Nicola Mometto [ 06/Nov/15 3:07 PM ]

Using #(= 'foo %) rather than (partial = 'foo) allows for inlining of = and makes performance a bit better, but =to still wins noticeably

WARNING: Final GC required 1.284421364203217 % of runtime
WARNING: Final GC required 10.04376144830405 % of runtime
Evaluation count : 3643032 in 6 samples of 607172 calls.
             Execution time mean : 165.393131 ns
    Execution time std-deviation : 1.041355 ns
   Execution time lower quantile : 164.277060 ns ( 2.5%)
   Execution time upper quantile : 166.849951 ns (97.5%)
                   Overhead used : 1.605524 ns
WARNING: Final GC required 6.258680973295933 % of runtime
Evaluation count : 3584574 in 6 samples of 597429 calls.
             Execution time mean : 167.659014 ns
    Execution time std-deviation : 3.821817 ns
   Execution time lower quantile : 164.175156 ns ( 2.5%)
   Execution time upper quantile : 173.210781 ns (97.5%)
                   Overhead used : 1.605524 ns

Found 1 outliers in 6 samples (16.6667 %)
	low-severe	 1 (16.6667 %)
 Variance from outliers : 13.8889 % Variance is moderately inflated by outliers
WARNING: Final GC required 6.914389197005716 % of runtime
Evaluation count : 11196 in 6 samples of 1866 calls.
             Execution time mean : 52.593759 µs
    Execution time std-deviation : 834.220092 ns
   Execution time lower quantile : 51.510161 µs ( 2.5%)
   Execution time upper quantile : 53.367649 µs (97.5%)
                   Overhead used : 1.605524 ns
WARNING: Final GC required 6.179040224498723 % of runtime
Evaluation count : 9162 in 6 samples of 1527 calls.
             Execution time mean : 66.527357 µs
    Execution time std-deviation : 2.119652 µs
   Execution time lower quantile : 65.308835 µs ( 2.5%)
   Execution time upper quantile : 70.201570 µs (97.5%)
                   Overhead used : 1.605524 ns

small homogeneous coll, (partial = 'foo): 217ns, #(= 'foo %): 165ns, (=to 'foo): 39ns
small eterogeneous coll, (partial = 'foo): 192ns, #(= 'foo %): 167ns, (=to 'foo): 41ns
big homogeneous coll, (partial = 'foo): 66us, #(= 'foo %): 52us, (=to 'foo): 8us
big eterogeneous coll, (partial = 'foo: 82us, #(= 'foo %): 66us, (=to 'foo): 27us

Comment by Nicola Mometto [ 17/Dec/15 5:13 PM ]

Apparently this was something that was already discussed a couple of years ago and Rich seemed ok with this https://groups.google.com/forum/#!topic/clojure-dev/0c-VNhEKVkI





[CLJ-1842] Use code generation to support more than 4 primitive arguments in function calls Created: 02/Nov/15  Updated: 02/Nov/15  Resolved: 02/Nov/15

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

Type: Enhancement Priority: Minor
Reporter: Richard Davies Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: compiler
Environment:

All



 Description   

The current restriction that "fns taking primitives support only 4 or fewer args" is apparently to prevent an explosion of possible interfaces https://groups.google.com/forum/#!topic/clojure/MI7iakMqEXo . Could the same code generation approach to "Unrolled small vectors" (http://dev.clojure.org/jira/browse/CLJ-1517) be used to increase the supported arities of primitive functions in IFn.java? I have bumped into this restriction a few times when trying to tune my code for high performance.

Each additional arity would require (1 + arg-arity)^3 interfaces to be generated. I understand that it is possible to embed primitives within deftypes instead of passing more parameters. However, the main use case for type-hinting to generate primitive interfaces is when an increase in performance is required so the deftype work-around is not optimal. Likewise, it is possible to drop out to Java to implement the primitive functions but this complicates the development cycle/tools/etc.



 Comments   
Comment by Alex Miller [ 02/Nov/15 7:53 PM ]

The issue is not generating the interfaces (the existing interfaces were themselves generated), but whether the result is manageable in terms of code size etc. There are other ways to approach it though and we may do something in the future. Declining the ticket as we would not work it off of here.





[CLJ-1841] core/bean iterator is broken Created: 02/Nov/15  Updated: 22/Jan/16

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

Type: Defect Priority: Minor
Reporter: Timur Sung Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: interop
Environment:

java version "1.8.0_66"
Java(TM) SE Runtime Environment (build 1.8.0_66-b17)
Java HotSpot(TM) 64-Bit Server VM (build 25.66-b17, mixed mode)


Attachments: Text File clj-1841.patch    
Patch: Code and Test
Approval: Prescreened

 Description   

The bean iterator implementation is returning the wrong data. One place the iterator is used is with transduce, which is under into:

user> (bean "test")  ;; as expected
{:bytes #object["[B" 0x546fe214 "[B@546fe214"], :class java.lang.String, :empty false}
user> (into [] (seq (bean "test")))  ;; seq works as expected
[[:bytes #object["[B" 0x6576fe71 "[B@6576fe71"]] [:class java.lang.String] [:empty false]]
user> (into [] (bean "test"))  ;; BROKEN - should match prior
[[:bytes #object[clojure.core$bean$fn__5742$fn__5743 0x4cdc53ad "clojure.core$bean$fn__5742$fn__5743@4cdc53ad"]] [:class #object[clojure.core$bean$fn__5742$fn__5743 0x55008929 "clojure.core$bean$fn__5742$fn__5743@55008929"]] [:empty #object[clojure.core$bean$fn__5742$fn__5743 0x118e7d04 "clojure.core$bean$fn__5742$fn__5743@118e7d04"]]]

Cause: The iterator impl in bean is incorrectly implemented and exposing some of the inner details instead.

Approach: Pull an iterator from the seq impl. The seq impl uses an embedded fn - the patch pulls that up and invokes from both seq and iterator.

Patch: clj-1841.patch






[CLJ-1837] Improve wording of index-of and last-index-of doc strings Created: 29/Oct/15  Updated: 18/Jan/16

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

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

Attachments: Text File clj-1837-v1.patch    
Patch: Code
Approval: Prescreened

 Description   

Feel free to decline this if it is too trivial. I was reviewing doc strings for new functions in Clojure 1.8.0, and those for clojure.string/index-of and clojure.string/last-index-of seem like they could be worded in a way that is much less subject to confusion.

Current doc string for clojure.string/index-of:

Return index of value (string or char) in s, optionally searching
  forward from from-index or nil if not found.

Issue: the lack of punctuation in the phrase "optionally searching forward from from-index or nil if not found" makes it appear that the "or" connects the first and last part of that phrase.

Suggested clarification:

Return index of value (string or char) in s, optionally searching
  forward from from-index.  Returns nil if value not found.

Screened by: Alex Miller



 Comments   
Comment by Andy Fingerhut [ 01/Jan/16 5:01 AM ]

Patch clj-1837-v1.patch dated Jan 1 2016 modifies the doc strings of index-of and last-index-of as suggested.

Comment by Alex Miller [ 04/Jan/16 4:39 PM ]

I retracted my last comment, I think it's fine as is.





[CLJ-1836] Expose clojure.repl/doc as a function call Created: 28/Oct/15  Updated: 21/Nov/15

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

Type: Enhancement Priority: Minor
Reporter: Terje Dahl Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: repl

Attachments: Text File doc-fn-1.patch    
Patch: Code
Approval: Triaged

 Description   

Restructure the printing function clojure.repl/doc so that it calls a fuction clojure.repl/doc-fn for its data - in the same way as dir calls dir-fn. Make doc-fn public so that it can be called directly and allow developers to parse and display the data as needed.

Use case: I am making a namespace inspector (using JavaFX) (somewhat like the Swing-based tree-inspector in Clojure), and when getting a function, I would like to display the same meta-information as "doc" prints in the REPL - including the special forms data coded in a private var/map in Clojure.

Patch: doc-fn.patch



 Comments   
Comment by Alex Miller [ 28/Oct/15 12:34 PM ]

A few comments:

1) Patch authors must sign the contributor's agreement, see http://clojure.org/contributing

2) The patch is not in the correct format - see http://dev.clojure.org/display/community/Developing+Patches for more info.

3) Patch should include a test for the new doc-fn.

Comment by Terje Dahl [ 21/Nov/15 6:29 AM ]

1. Agreement signed.

2. Tests added.
(That was useful! I had to fix a couple of things.)

3. Patch created as per instructions and attached:
"doc-fn-1.patch"





[CLJ-1835] Fix minor typos in documentation Created: 28/Oct/15  Updated: 28/Oct/15  Resolved: 28/Oct/15

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

Type: Enhancement Priority: Trivial
Reporter: Artur Cygan Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: documentation
Environment:

Not relevant


Attachments: Text File 0001-PATCH-Fix-typos-iff-if-in-docstrings-and-comment.patch    

 Description   

iff -> if



 Comments   
Comment by Nicola Mometto [ 28/Oct/15 6:07 AM ]

Those are not typos, iff means "if and only if"

Comment by Artur Cygan [ 28/Oct/15 6:11 AM ]

Ah okay, didn't know that.





[CLJ-1834] Support for test matrix build of direct linking on / off Created: 26/Oct/15  Updated: 27/Oct/15  Resolved: 27/Oct/15

Status: Closed
Project: Clojure
Component/s: None
Affects Version/s: Release 1.8
Fix Version/s: Release 1.8

Type: Enhancement Priority: Major
Reporter: Alex Miller Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: build

Attachments: Text File clj-1834-2.patch     Text File clj-1834-3.patch     Text File clj-1834.patch    
Patch: Code
Approval: Ok

 Description   

Enable build box test matrix build of direct linking on and off.

Modified build to do the following:

  • Maven build - add user property "directlinking" with default value "true"
  • Maven build - add two profiles: "direct" and "nodirect" which force this property to either "true" or "false"
  • Ant build - defines new property "directlinking"
  • Ant build - inherits this property value from Maven automatically
  • Ant build - echoes the setting of the property during test compilation so you can tell which is activated
  • Ant build - compiles and runs tests with clojure.compiler.direct-linking set to the value of ${directlinking}

The Maven build can be invoked with one of these as follows:

mvn clean test -Ptest-direct
mvn clean test -Ptest-no-direct

The Hudson clojure-test-matrix will have a new axis with values "test-direct" and "test-no-direct" and a modified command line that will set the profile with -P based on the axis value.

Patch: clj-1834-3.patch



 Comments   
Comment by Alex Miller [ 26/Oct/15 5:10 PM ]

I may have broken the default ant build behavior with this patch, need to check on that still but taking a break for now...

Comment by Alex Miller [ 27/Oct/15 9:03 AM ]

Ant behavior fixed in -2 patch





[CLJ-1833] pretty-print fix needs type hint Created: 26/Oct/15  Updated: 26/Oct/15  Resolved: 26/Oct/15

Status: Closed
Project: Clojure
Component/s: None
Affects Version/s: Release 1.8
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Steve Miner Assignee: Unassigned
Resolution: Duplicate Votes: 0
Labels: pprint

Attachments: Text File CLJ-1833-type-hint-in-pretty-writer.patch    

 Description   

A recent fix to the pretty-print code is missing a type hint. Other recent fixes turned on reflection warnings so now you get this warning when building Clojure:

Reflection warning, clojure/pprint/pretty_writer.clj:419:12 - call to method write can't be resolved (target class is unknown).



 Comments   
Comment by Steve Miner [ 26/Oct/15 9:46 AM ]

The original fix was CLJ-1390. It was missing a type hint. (My fault.)

Comment by Steve Miner [ 26/Oct/15 9:47 AM ]

added type hint

Comment by Alex Miller [ 26/Oct/15 9:47 AM ]

Dupe of CLJ-1827





[CLJ-1832] Enhance either documentation or implementation of unchecked-* functions Created: 26/Oct/15  Updated: 27/Oct/15

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

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


 Description   

The behavior of unchecked-* functions such as unchecked-add, unchecked-subtract, and unchecked-multiply are at least somewhat surprising, and perhaps desirable to change, when given arguments that are boxed Long values rather than primitive long values. For example:

user=> *clojure-version*
{:major 1, :minor 7, :incremental 0, :qualifier nil}
user=> (doc unchecked-multiply)
-------------------------
clojure.core/unchecked-multiply
([x y])
  Returns the product of x and y, both long.
  Note - uses a primitive operator subject to overflow.
nil
user=> (unchecked-multiply 2432902008176640000 21)
-4249290049419214848
user=> (unchecked-multiply 2432902008176640000 (Long. 21))

ArithmeticException integer overflow  clojure.lang.Numbers.throwIntOverflow (Numbers.java:1501)

Normally no one would use explicit boxed Long arguments like in the example above, but these can easily occur, unintentionally, if the arguments to the unchecked functions are not explicitly type hinted as primitive long values.



 Comments   
Comment by Alex Miller [ 26/Oct/15 9:03 AM ]

I think this is a reasonable complaint. The trickiness of handling is of course doing it without affecting performance for the non-error case.

Comment by Gary Fredericks [ 26/Oct/15 8:04 PM ]

My first thought was that there shouldn't be any perf concern because it's just a matter of modifying lines such as this one, where the type dispatching has already been done. But maybe you're thinking that that line has to be more complex since the arguments could be of various different numeric types, not just Long and Long?

Comment by Alex Miller [ 27/Oct/15 7:19 AM ]

That was a general comment. I haven't actually looked at the code changes necessary.





[CLJ-1831] Add map-entry? predicate Created: 19/Oct/15  Updated: 27/Oct/15  Resolved: 27/Oct/15

Status: Closed
Project: Clojure
Component/s: None
Affects Version/s: Release 1.8
Fix Version/s: Release 1.8

Type: Enhancement Priority: Major
Reporter: Alex Miller Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File clj-1831.patch    
Patch: Code and Test
Approval: Ok

 Description   

Due to changes in 1.8 with making vector implement IMapEntry for 2-element vectors, checking whether something is a map entry has become a bit trickier. This enhancement proposes a new function `map-entry?` to encapsulate that check and any future updates to it.

The check for map-entry? will return true if the instance implements java.util.Map$Entry and if it is also a vector, if it's size is exactly 2.

Patch: clj-1831.patch

Screened by Joe Smith.



 Comments   
Comment by Nicola Mometto [ 24/Oct/15 3:33 PM ]

Joe R. Smith Only members of the clojure core team are allowed to screen tickets

Comment by Ghadi Shayban [ 24/Oct/15 3:39 PM ]

Nicola, Joe Smith is core team.

Comment by Nicola Mometto [ 24/Oct/15 5:21 PM ]

Sorry about that then, I restored the ticket status

Comment by Nicola Mometto [ 24/Oct/15 5:23 PM ]

http://dev.clojure.org/display/community/Screeners should probably be updated then

Comment by Alex Miller [ 24/Oct/15 9:08 PM ]

Yep, will do.





[CLJ-1830] Test equality ignore decimal Created: 18/Oct/15  Updated: 19/Oct/15  Resolved: 18/Oct/15

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

Type: Enhancement Priority: Minor
Reporter: Nick DeCoursin Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None


 Description   
user> (= {:a 1.00} {:a 1.0})
true
user> (= {:a 1} {:a 1.0})
false

This ^ is expected because (not (= 1 1.0)), so instead == is used to compare number equivalence: (== 1 1.0). But, == fails on collections:

user> (== {:a 1} {:a 1.0})
ClassCastException clojure.lang.PersistentArrayMap cannot be cast to java.lang.Number  clojure.lang.Numbers.equiv (Numbers.java:208)

In summary, there's not way to make this assertion (= {:a 1} {:a 1.0})



 Comments   
Comment by Alex Miller [ 18/Oct/15 4:50 PM ]

This would require a significant number of changes across the collection hierarchy to define a new additional kind of equivalence. I do not expect that we will add this functionality. If we did embark on this, it would be done through a design effort, not through a ticket like this. Thanks for the suggestion.

Comment by Andy Fingerhut [ 18/Oct/15 5:13 PM ]

Nick: Clojure core members make the final calls on things like this, but I am pretty sure that (= 1 1.0) is false by design. The inability to use == to compare anything other than numbers is also by design.

If you want a function, which for sake of example I will call "deep==" here, that uses == on numbers inside of collections, or collections nested inside of other collections, etc., I don't think it would be difficult to write one, as long as the values you wanted to compare using == were the values in the maps only, and not the keys.

If you wanted a function where (deep== {1 :a} {1.0 :a}) returned true, you would have to do something other than the normal key lookup functions built into Clojure, because they use clojure.core/hash to put items into 'hash buckets'. (clojure.core/hash x) and (clojure.core/hash (* 1.0 x)) are in general different from each other, again I believe by design.

Comment by Nick DeCoursin [ 19/Oct/15 1:34 AM ]

Thank you for looking at this, for your input, and the details.

I think a design change might be worth it. This would probably need to happen at 2.0 or something. But this is pretty fundamental that can serious hidden bugs.

Anyways, I don't mean to start a debate, but it's something that I think deserves some consideration. Thank you.





[CLJ-1829] VerifyError on Android Created: 16/Oct/15  Updated: 11/Jan/16  Resolved: 11/Jan/16

Status: Closed
Project: Clojure
Component/s: None
Affects Version/s: Release 1.8
Fix Version/s: Release 1.8

Type: Defect Priority: Major
Reporter: Konstantin Mikheev Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: android, compiler
Environment:

Android API >= 21


Attachments: Text File clj-1829.patch    
Patch: Code
Approval: Ok

 Description   

Android Lollipop (API level 21) introduced an advanced bytecode checker that does not allow Clojure 1.8 to run.

Here is an example repo that reproduces the issue:
https://github.com/konmik/try_clojure_on_android/blob/master/app/src/main/java/com/clojure_on_android/TestInvokeUnit.java#L8

It uses 'org.clojure:clojure:1.8.0-beta1' dependency.

To reproduce the exception you need to install Android Studio
https://developer.android.com/sdk/index.html
and an android emulator https://www.genymotion.com/

Run the emulator, open the project and press "run" in the IDE.

The expected result that I get on Android API < 21 is:
https://github.com/konmik/try_clojure_on_android/blob/master/expected.png

On Android versions >= 21 I get:

10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: java.lang.VerifyError: Rejecting class clojure.core.server$stop_server because it failed compile-time verification (declaration of 'clojure.core.server$stop_server' appears in /data/app/com.clojure_on_android-1/base.apk)
10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: at clojure.core.server__init.load(Unknown Source)
10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: at clojure.core.server__init.<clinit>(Unknown Source)
10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: at java.lang.Class.classForName(Native Method)
10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: at java.lang.Class.forName(Class.java:309)
10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: at clojure.lang.RT.classForName(RT.java:2168)
10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: at clojure.lang.RT.classForName(RT.java:2177)
10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: at clojure.lang.RT.loadClassForName(RT.java:2196)
10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: at clojure.lang.RT.load(RT.java:443)
10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: at clojure.lang.RT.load(RT.java:419)
10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: at clojure.core$load$fn__5669.invoke(core.clj:5885)
10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: at clojure.core$load.invokeStatic(core.clj:5884)
10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: at clojure.core$load_one.invokeStatic(core.clj:5685)
10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: at clojure.core$load_one.invoke(core.clj)
10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: at clojure.core$load_lib$fn__5618.invoke(core.clj:5730)
10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: at clojure.core$load_lib.invokeStatic(core.clj:5729)
10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: at clojure.core$load_lib.doInvoke(core.clj)
10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: at clojure.lang.RestFn.applyTo(RestFn.java:142)
10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: at clojure.core$apply.invokeStatic(core.clj:647)
10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: at clojure.core$load_libs.invokeStatic(core.clj:5767)
10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: at clojure.core$load_libs.doInvoke(core.clj)
10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: at clojure.lang.RestFn.applyTo(RestFn.java:137)
10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: at clojure.core$apply.invokeStatic(core.clj:647)
10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: at clojure.core$require.invokeStatic(core.clj:5789)
10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: at clojure.core$require.doInvoke(core.clj)
10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: at clojure.lang.RestFn.invoke(RestFn.java:408)
10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: at clojure.lang.Var.invoke(Var.java:379)
10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: at clojure.lang.RT.doInit(RT.java:480)
10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: at clojure.lang.RT.<clinit>(RT.java:331)
10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: at clojure.lang.Namespace.<init>(Namespace.java:34)
10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: at clojure.lang.Namespace.findOrCreate(Namespace.java:176)
10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: at clojure.lang.Var.intern(Var.java:146)
10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: at clojure.java.api.Clojure.var(Clojure.java:82)
10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: at clojure.java.api.Clojure.<clinit>(Clojure.java:96)
10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: at com.clojure_on_android.TestInvokeUnit.invokePlus(TestInvokeUnit.java:8)
10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: at com.clojure_on_android.MainActivity.onCreate(MainActivity.java:15)
10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: at android.app.Activity.performCreate(Activity.java:5990)
10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1106)
10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2278)
10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:2387)
10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: at android.app.ActivityThread.access$800(ActivityThread.java:151)
10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1303)
10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: at android.os.Handler.dispatchMessage(Handler.java:102)
10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: at android.os.Looper.loop(Looper.java:135)
10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: at android.app.ActivityThread.main(ActivityThread.java:5254)
10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: at java.lang.reflect.Method.invoke(Native Method)
10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: at java.lang.reflect.Method.invoke(Method.java:372)
10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:903)
10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:698)

Cause: In Clojure 1.8, the socket server code is loaded during startup, which causes classes to be loaded that are compiled with the locking macro. The locking macro generates monitorenter and monitorexit instructions (and exception handlers) that do not conform to the (optional) structured locking section of the JVM spec. While this code is considered valid in OracleJDK, OpenJDK, etc the new Android bytecode checker will fail with it. Other versions of Clojure also have this verification issue, but the use of the locking macro during language boot time changes this potential issue to always being a problem.

Approach: The proposed patch sidesteps this issue by avoiding the locking macro and replaces it with a similar macro that uses ReentrantLock instead. This approach has been verified on the provided test case.

Patch: clj-1829.patch

Alternatives: There is a patch available for the locking macro (CLJ-1472) that replaces the locking macro by a synchronized block instead.



 Comments   
Comment by Alex Miller [ 16/Oct/15 2:32 PM ]

This could be a result of CLJ-1809 (hard to tell). The clojure.core.server/stop-server fn is a new fn with the socket server feature and should be direct-linked, which could implicate 1809.

Comment by Alex Miller [ 16/Oct/15 3:14 PM ]

I tried to reproduce this (using Run -> Run 'build' in Android Studio). The build was successful. I suspect I'm missing one or more steps in how to run correctly. Do I need to add a virtual device in Genymotion and if so, does it matter which one?

Comment by Konstantin Mikheev [ 16/Oct/15 3:17 PM ]

Yes, the build is successful.

The issue appears when you run it on a device.

You need to add a device in the emulator with API level >= 21 and to run it there.

Comment by Konstantin Mikheev [ 16/Oct/15 3:20 PM ]

When you run it, the logcat (Android logging panel) appears that is showing you the system log. The application's crash log can be found there.

Comment by Alex Miller [ 16/Oct/15 4:39 PM ]

Well, I tried pretty hard but I still can't figure out how to make Android Studio run the project in Genymotion. This was helpful https://www.genymotion.com/#!/developers/user-guide#genymotion-plugin-for-android-studio and I installed the Genymotion plugin etc but I never seem to get the opportunity to choose a device when I run the build.

What I would like to try (in case anyone else is able to do this) is to apply the patch from CLJ-1809 to clojure master, do a build, and then see if that fixes the problem in the Android emulator. If so, then this is just a dupe of CLJ-1809. If not, then probably some more digging is needed.

Comment by Konstantin Mikheev [ 16/Oct/15 4:46 PM ]

Oh no, you don't need the geny plugin for android studio.
It is sad you wasn't able to run it.
I'll run any test builds for you, just let me know when the next version become available.

Comment by Konstantin Mikheev [ 28/Oct/15 3:12 PM ]

I've just tried the org.clojure:clojure:1.8.0-beta2 release and the bug is still there.

Comment by Alex Miller [ 28/Oct/15 4:17 PM ]

I believe you, but I will need more explicit instructions on how to reproduce. (Or someone else needs to track this down.)

Comment by Konstantin Mikheev [ 28/Oct/15 4:25 PM ]

OK, I'll make a series of screenshots.

Comment by Konstantin Mikheev [ 15/Nov/15 3:33 PM ]

Sorry for the delay.

1. At first you need to setup Android Studio: http://developer.android.com/sdk/installing/index.html?pkg=studio

2. And setup the Android SDK: http://developer.android.com/sdk/installing/adding-packages.html

you will need to install:
Tools -> Android SDK Tools,
Tools -> Android SDK Platform Tools,
Tools -> Android SDK Build Tools 23.0.1,
Android 6.0 -> SDK Platform
Extras -> Google Repository (not sure if it is needed)

3. Run Android Studio and open the project.

4. Running the project on the genymotion emulator: https://github.com/konmik/try_clojure_on_android/tree/master/run_with_genymotion

Comment by Alex Miller [ 20/Nov/15 10:03 AM ]

Have you tried 1.8.0-RC2 with this problem?

Comment by Alex Miller [ 20/Nov/15 10:22 AM ]

stop-server uses the locking macro which reminds me of CLJ-1472.

Comment by Konstantin Mikheev [ 20/Nov/15 11:57 AM ]

Yes I've tried, it still doesn't work.

There is something with the new Android bytecode validator.
We had similar problems with validation while using retrolambda.

Comment by Konstantin Mikheev [ 16/Dec/15 3:51 PM ]

Does not work with org.clojure:clojure:1.8.0-RC4 still.

Comment by Alex Miller [ 21/Dec/15 9:29 AM ]

I was able to reproduce this and verify the hypothesis I had above - this is a duplicate of CLJ-1472. The clojure.core/locking macro seems to generate bytecode that fails on the latest ART bytecode validator (that ticket has more detail on this and some links to issues filed on Android in this regard).

Clojure 1.8 is not actually new in this regard - any version of Clojure would fail in the same way as the locking macro has not changed. The difference here is that the new Clojure socket server code in 1.8 causes it to be used during runtime startup, so the failure occurs during bootstrapping when it did not previously.





[CLJ-1828] Remove trailing whitespace in clojure.test Created: 13/Oct/15  Updated: 13/Oct/15  Resolved: 13/Oct/15

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

Type: Enhancement Priority: Trivial
Reporter: Daniel Compton Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None

Attachments: Text File clojure-test-whitespace.patch    

 Description   

Removes trailing whitespace from clojure.test.



 Comments   
Comment by Daniel Compton [ 13/Oct/15 9:00 PM ]

Declined by Alex on Slack.





[CLJ-1827] Reflection warning introduced in CLJ-1259 Created: 13/Oct/15  Updated: 27/Oct/15  Resolved: 27/Oct/15

Status: Closed
Project: Clojure
Component/s: None
Affects Version/s: Release 1.8
Fix Version/s: Release 1.8

Type: Defect Priority: Major
Reporter: Alex Miller Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: print, reflection, regression
Environment:

1.8.0-beta1


Attachments: Text File clj-1827-v1.patch    
Patch: Code
Approval: Ok

 Description   

The patch for CLJ-1259 addressed the reflection warnings in pprint. However, a different ticket introduced some new code in between when this patch was written and applied and thus there is a new reflection warning produced in the Clojure build:

[java] Reflection warning, clojure/pprint/pretty_writer.clj:419:12 - call to method write can't be resolved (target class is unknown).

This ticket is to address that one newly introduced case to remove the warning.

Patch: clj-1827-v1.patch
Screened by: Alex Miller



 Comments   
Comment by Andy Fingerhut [ 15/Oct/15 10:23 AM ]

Patch clj-1827-v1.patch dated Oct 15 2015 eliminates the one reflection warning in pretty_writer.clj.





[CLJ-1826] drop-last docstring refers to 'coll' args refer to 's' Created: 13/Oct/15  Updated: 18/Jan/16

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

Type: Enhancement Priority: Trivial
Reporter: Marc O'Morain Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring, ft

Attachments: Text File 0001-CLJ-1826-drop-last-docstring-refers-to-coll-args-ref.patch    
Patch: Code
Approval: Prescreened

 Description   

The doc-string for drop-last refers to coll, but the arguments are named n and s, rather than coll.

user=> (doc drop-last)
-------------------------
clojure.core/drop-last
([s] [n s])
  Return a lazy sequence of all but the last n (default 1) items in coll
nil
user=> (source drop-last)
(defn drop-last
  "Return a lazy sequence of all but the last n (default 1) items in coll"
  {:added "1.0"
   :static true}
  ([s] (drop-last 1 s))
  ([n s] (map (fn [x _] x) s (drop n s))))
nil


 Comments   
Comment by Yen-Chin, Lee [ 20/Oct/15 9:39 AM ]

Patch based on current master. (f76b343d)

Comment by Alex Miller [ 20/Oct/15 9:51 AM ]

Thanks, looks good. This won't make it in 1.8 but we will look at it in the next release.





[CLJ-1825] Compilation errors on anonymous recursive function Created: 12/Oct/15  Updated: 12/Nov/15  Resolved: 12/Nov/15

Status: Closed
Project: Clojure
Component/s: None
Affects Version/s: Release 1.8
Fix Version/s: Release 1.8

Type: Defect Priority: Major
Reporter: Nicolas Modrzyk Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None
Environment:

OS X, Yosemite, jdk 1.8.0_60


Approval: Vetted

 Description   

Seems the below does not compile with 1.8:

(def lazy-fib
  "Lazy sequence of fibonacci numbers"
  ((fn rfib [a b]
     (lazy-seq (cons a (rfib b (+' a b)))))
   0 1))

(defn even-lazy-fib[n]
  (filter even? (take n lazy-fib)))

(even-lazy-fib 10)

Status:

  • 1.7.0 - works
  • 1.8.0-alpha2 - works
  • 1.8.0-alpha3-1.8.0-beta1 - VerifyError, see below
  • 1.8.0-beta2 - NPE, see below
  • 1.8.0-RC1 - ClassCastException, see below
  • 1.8.0 master - NPE, see below

1.8.0-alpha3:

CompilerException java.lang.VerifyError: (class: vecperf/bench$rfib__1233, method: invokeStatic signature: (Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;) Illegal local variable number, compiling:(form-init3780016918836504993.clj:3:3)
	clojure.lang.Compiler$InvokeExpr.eval (Compiler.java:3661)
	clojure.lang.Compiler$DefExpr.eval (Compiler.java:455)
	clojure.lang.Compiler.eval (Compiler.java:6948)
	clojure.lang.Compiler.eval (Compiler.java:6906)
	clojure.core/eval (core.clj:3084)
	clojure.core/eval (core.clj:-1)
	clojure.main/repl/read-eval-print--7081/fn--7084 (main.clj:240)
	clojure.main/repl/read-eval-print--7081 (main.clj:240)
	clojure.main/repl/fn--7090 (main.clj:258)
	clojure.main/repl (main.clj:258)
	clojure.main/repl (main.clj:-1)
	clojure.tools.nrepl.middleware.interruptible-eval/evaluate/fn--630 (interruptible_eval.clj:58)
Caused by:
VerifyError (class: vecperf/bench$rfib__1233, method: invokeStatic signature: (Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;) Illegal local variable number
	java.lang.Class.getDeclaredConstructors0 (Class.java:-2)
	java.lang.Class.privateGetDeclaredConstructors (Class.java:2658)
	java.lang.Class.getConstructor0 (Class.java:2964)
	java.lang.Class.newInstance (Class.java:403)
	clojure.lang.Compiler$ObjExpr.eval (Compiler.java:4943)
	clojure.lang.Compiler$InvokeExpr.eval (Compiler.java:3652)
	clojure.lang.Compiler$DefExpr.eval (Compiler.java:455)

1.8.0-beta2 / 1.8.0 master:

NullPointerException
	clojure.lang.Numbers.ops (Numbers.java:1013)
	clojure.lang.Numbers.addP (Numbers.java:132)
	user/rfib--1250/fn--1251 (form-init4987495233354047014.clj:4)

1.8.0-RC1:

ClassCastException java.lang.Long cannot be cast to clojure.lang.IFn
	user/rfib--1250/fn--1251 (form-init1118919529313120594.clj:4)


 Comments   
Comment by Alex Miller [ 12/Oct/15 10:07 PM ]

Dupe of CLJ-1809

Comment by Nicolas Modrzyk [ 11/Nov/15 8:51 PM ]

With 1.8-RC1, and the code above, I now get the following:

java.lang.ClassCastException: java.lang.Long cannot be cast to clojure.lang.IFn
/Users/niko/projects/maths/src/maths/fastfib.clj:41 maths.fastfib/rfib[fn]
          LazySeq.java:40 clojure.lang.LazySeq.sval
          LazySeq.java:49 clojure.lang.LazySeq.seq
              RT.java:521 clojure.lang.RT.seq
             core.clj:137 clojure.core/seq
            core.clj:2777 clojure.core/take[fn]
          LazySeq.java:40 clojure.lang.LazySeq.sval
          LazySeq.java:49 clojure.lang.LazySeq.seq
              RT.java:521 clojure.lang.RT.seq
             core.clj:137 clojure.core/seq
            core.clj:2702 clojure.core/filter[fn]
          LazySeq.java:40 clojure.lang.LazySeq.sval
          LazySeq.java:49 clojure.lang.LazySeq.seq
Comment by Alex Miller [ 12/Nov/15 10:26 AM ]

The generated bytecode for rfib seems fishy to me. In 1.7 for example, it does aload_0 to load the this reference to self-invoke, but in 1.8 that winds up in an invokestatic where aload_0 is a, not this, so the stack is messed up when invokespecial is invoked.

1.7:

public java.lang.Object invoke(java.lang.Object, java.lang.Object);
    Code:
       0: new           #14                 // class clojure/lang/LazySeq
       3: dup           
       4: new           #16                 // class fib$rfib__25$fn__26
       7: dup           
       8: aload_0       
       9: aload_1       
      10: aconst_null   
      11: astore_1      
      12: aload_2       
      13: aconst_null   
      14: astore_2      
      15: invokespecial #19                 // Method fib$rfib__25$fn__26."<init>":(Ljava/lang/Object;Ljava/lang/Object;Ljava/lang/Object;)V

In 1.8:

public static java.lang.Object invokeStatic(java.lang.Object, java.lang.Object);
    Code:
       0: new           #14                 // class clojure/lang/LazySeq
       3: dup           
       4: new           #16                 // class fib$rfib__25$fn__26
       7: dup           
       8: aload_0
       9: aload_0
      10: aconst_null
      11: astore_0
      12: aload_1
      13: aconst_null
      14: astore_1
      15: invokespecial #19                 // Method fib$rfib__25$fn__26."<init>":(Ljava/lang/Object;Ljava/lang/Object;Ljava/lang/Object;)V
Comment by Alex Miller [ 12/Nov/15 4:36 PM ]

Rich made a commit to fix this in master:

https://github.com/clojure/clojure/commit/7faeb3a5e1fb183539a8638b72d299a3433fe990

Comment by Nicolas Modrzyk [ 12/Nov/15 6:46 PM ]

Confirming, master with commit "7faeb3a5e1fb183539a8638b72d299a3433fe990" fixes it for me too.





[CLJ-1824] Splicing macros Created: 12/Oct/15  Updated: 14/Oct/15  Resolved: 14/Oct/15

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

Type: Enhancement Priority: Minor
Reporter: Mike Anderson Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None


 Description   

In many cases, it would be convenient for a macro to "splice" its return value into the resulting form.

Example use cases:

  • `comment` can return no forms, so that it can be inserted in places where nil would be disruptive
  • a macro can return two forms to create a condition in a `cond` block
  • a macro can create multiple forms to support a variable-arity function (e.g. passing a set of keyword arguments)
  • a macro can create one or more complete `binding`, `let` or `loop` bindings

Proposed syntax and usage:

(defmacro-splicing multi-test [v values exp]
  (mapcat 
    (fn [value] `[(= ~v ~value) ~exp]`
    values))

(let [v (compute-some-result)]
  (cond 
    (multi-test v [1 3 5 7 9] "Odd digit")
    (multi-test v [0 2 4 6 8] "Even digit")
    :else "Not a digit"))


 Comments   
Comment by Ghadi Shayban [ 13/Oct/15 7:08 PM ]

These sorts of things need a design discussion prior to a JIRA ticket.

Comment by Alex Miller [ 14/Oct/15 8:00 AM ]

I'm declining this as a ticket as it does really need design evaluation in some way before it gets to this point (either clojure-dev mailing list or a design wiki page or something).

I'm not passing any judgement on the idea, which is potentially interesting.





[CLJ-1823] Document new :load-ns option to deftype/defrecord introduced by CLJ-1208 Created: 09/Oct/15  Updated: 27/Oct/15  Resolved: 27/Oct/15

Status: Closed
Project: Clojure
Component/s: None
Affects Version/s: Release 1.8
Fix Version/s: Release 1.8

Type: Enhancement Priority: Trivial
Reporter: Nicola Mometto Assignee: Nicola Mometto
Resolution: Completed Votes: 0
Labels: defrecord, deftype, docstring

Attachments: Text File 0001-CLJ-1823-document-load-ns-option-to-deftype-defrecor.patch     Text File clj-1823-2.patch    
Patch: Code
Approval: Ok

 Description   

CLJ-1208 was applied in 1.8 alphas and contains a new :load-ns option for defrecord and deftype but did not document that in the docstring.

This ticket adds documentation for that feature to the docstring.

Additionally, text should be added to http://clojure.org/datatypes.

Patch: clj-1823-2.patch



 Comments   
Comment by Alex Miller [ 09/Oct/15 10:54 AM ]

Pulling into 1.8 as it would be nice to doc this in the release.

Comment by Alex Miller [ 12/Oct/15 10:23 AM ]

Modified docstring format slightly, retained attribution in -2 patch.





[CLJ-1821] Move map-invert from clojure.set to clojure.core Created: 28/Sep/15  Updated: 28/Sep/15

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

Type: Enhancement Priority: Minor
Reporter: Lars Andersen Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

map-invert is hard to find when it lives in the clojure.set namespace, because it acts on maps and not sets. To my eyes set/map-invert also looks strange when reading code, but this is the preferred way to bring in the clojure.set namespace.

This is one of the minor warts I'd like to see fixed in clojure 2.0.






[CLJ-1820] Move rename-keys from clojure.set to clojure.core Created: 28/Sep/15  Updated: 28/Sep/15

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

Type: Enhancement Priority: Trivial
Reporter: Lars Andersen Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

rename-keys is hard to find when it lives in the clojure.set namespace, because it acts on maps and not sets. To my eyes set/rename-keys also looks strange when reading code, but this is the preferred way to bring in the clojure.set namespace.

This is one of the minor warts I'd like to see fixed in clojure 2.0.






[CLJ-1819] Add removev function Created: 28/Sep/15  Updated: 28/Sep/15  Resolved: 28/Sep/15

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

Type: Enhancement Priority: Minor
Reporter: Daniel Compton Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None


 Description   

We already have mapv and filterv from http://dev.clojure.org/jira/browse/CLJ-847. What is Core's opinion on adding removev to complement filterv?



 Comments   
Comment by Alex Miller [ 28/Sep/15 9:55 AM ]

I do not expect that the set of vector fns will expand. Use transducers instead:

(into [] (remove odd?) [1 2 3 4 5])




[CLJ-1818] cl-format does not respect aesthetic ~A with a keyword Created: 26/Sep/15  Updated: 12/Jan/16

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

Type: Defect Priority: Trivial
Reporter: Jong-won Choi Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: print

Approval: Triaged

 Description   

In Common Lisp, (format nil "~a" :A) returns "A". However, in Clojure, it returns with the colon:

(clojure.pprint/cl-format false "~a" :A)
=> ":A"


 Comments   
Comment by Jong-won Choi [ 28/Sep/15 6:26 AM ]

Found another problem of cl-format:

(clojure.pprint/cl-format false "SELECT * from RateSchedules ~@[WHERE ~{~A=?~^ ~}~]" '())
=> "SELECT * from RateSchedules WHERE" ;; instead of "SELECT * from RateSchedules"

I guess the problem is () or [] has to be treated as falsey but not.

Comment by Alex Miller [ 28/Sep/15 9:58 AM ]

:a is a keyword and I would expect it's ascii format to be :a. I'm not sure what case sensitivity has to do with it.

Comment by Andy Fingerhut [ 28/Sep/15 10:08 AM ]

Alex, case is a side issue. Common Lisp's (format nil "~a" :A) returns "A", not ":A". It is the presence of the colon in the output that is the issue, not the case of the string.

Comment by Jong-won Choi [ 28/Sep/15 4:41 PM ]

For a record, what Alex described is for ~S - standard. See http://www.lispworks.com/documentation/lw50/CLHS/Body/22_cd.htm





[CLJ-1817] Allow AssertionError messages for function :pre and :post conditions to be specified. Created: 23/Sep/15  Updated: 23/Sep/15

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

Type: Enhancement Priority: Trivial
Reporter: Tristan Strange Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None
Environment:

All Clojure platforms



 Description   

A failing in a predicate in a list of :pre or :post conditions currently causes messages similar to one below to be displayed:

(defn must-be-a-map [m] {:pre [(map? m)]} m)
(must-be-a-map [])

AssertionError Assert failed: (map? m) user/must-be-a-map (form-init.....clj:1)

These exception messages could be made significantly more descriptive by allowing specific messages strings to be associated with each predicate in :pre and :post conditions.

Predicate functions and there associated messages strings could be specified as a pair of values in a map:

(defn must-be-a-map
[m]
{:pre [{(map? m) "m must be a map due to some domain specific reason."}]}
m)

The following would then produce an error message as follows:
(must-be-a-map 10)
AssertionError Assert failed: m must be a map due to some domain specific reason.
(map? m) user/must-be-a-map (form-init.....clj:1)

This would allow predicates without messages to specified alongside pairs of associated predicate message pairs as follows:
(defn n-and-m [n m] {:pre [(number? n) {(map? m) "You must provide a map!"}]})

This change would not break any existing functionality and still allow for predicates to be predefined elsewhere in code.

As a result pre and post conditions could provide a natural means of further documenting the ins and outs of a function, simplify the process of providing meaningful output when developing libraries and perhaps make the language better suited to teaching environments[1]

[1] http://wiki.science.ru.nl/tfpie/images/2/22/TFPIE2013_Steps_Towards_Teaching_Clojure.pdf






[CLJ-1816] Allow AssertionError messages for function :pre and :post conditions to be specified. Created: 23/Sep/15  Updated: 28/Sep/15  Resolved: 28/Sep/15

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

Type: Enhancement Priority: Trivial
Reporter: Tristan Strange Assignee: Unassigned
Resolution: Duplicate Votes: 0
Labels: None
Environment:

All Clojure platforms



 Description   

A failing in a predicate in a list of :pre or :post conditions currently causes messages similar to one below to be displayed:
{{(defn must-be-a-map [m] {:pre [(map? m)]} m)
(must-be-a-map [])

AssertionError Assert failed: (map? m) user/must-be-a-map (form-init.....clj:1)}}

These exception messages could be made significantly more descriptive by allowing specific messages strings to be associated with each predicate in :pre and :post conditions.

Predicate functions and there associated messages strings could be specified as a pair of values in a map:
{{
(defn must-be-a-map
[m]
{:pre [{(map? m) "m must be a map due to some domain specific reason."}]}
m)}}

The following would then produce an error message as follows:
{{(must-be-a-map 10)
AssertionError Assert failed: m must be a map due to some domain specific reason.
(map? m) user/must-be-a-map (form-init.....clj:1)}}

This would allow predicates without messages to specified alongside pairs of associated predicate message pairs as follows:
{{(defn n-and-m [n m] {:pre [(number? n) {(map? m) "You must provide a map!"}]})}}

This change would not break any existing functionality and still allow for predicates to be predefined elsewhere in code.

As a result pre and post conditions could provide a natural means of further documenting the ins and outs of a function, simplify the process of providing meaningful output when developing libraries and perhaps make the language better suited to teaching environments[1]

[1] http://wiki.science.ru.nl/tfpie/images/2/22/TFPIE2013_Steps_Towards_Teaching_Clojure.pdf



 Comments   
Comment by Tristan Strange [ 23/Sep/15 6:55 PM ]

Many apologies this is a duplicate of 1817! Confusion over create and preview buttons was the problem! Many apologies.

Comment by Alex Miller [ 28/Sep/15 9:53 AM ]

CLJ-1817





[CLJ-1815] select-keys should throw when not called with a map Created: 21/Sep/15  Updated: 21/Sep/15  Resolved: 21/Sep/15

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

Type: Enhancement Priority: Trivial
Reporter: Michiel Borkent Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: maps


 Description   

Currently select-keys accepts associative data structures that are not maps:

(select-keys [:a :b] [:a]) ;;=> {}
(select-keys [:a :b] [0]) ;;=> {0 :a}

I think select-keys should just throw when not called with a map.



 Comments   
Comment by Alex Miller [ 21/Sep/15 9:15 AM ]

vectors are associative and select-keys works with any map type so this is expected behavior.





[CLJ-1814] Make `satisfies?` as fast as a protocol method call Created: 11/Sep/15  Updated: 02/Nov/15

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

Type: Enhancement Priority: Major
Reporter: Nicola Mometto Assignee: Unassigned
Resolution: Unresolved Votes: 5
Labels: performance, protocols

Attachments: Text File 0001-CLJ-1814-cache-protocol-impl-satisfies-as-fast-as-me.patch     Text File 0001-CLJ-1814-cache-protocol-impl-satisfies-as-fast-as-me-v2.patch     Text File 0001-CLJ-1814-cache-protocol-impl-satisfies-as-fast-as-me-v3.patch    
Patch: Code
Approval: Triaged

 Description   

Currently `satisfies?` doesn't use the same impl cache used by protocol methods, making it too slow for real world usage.

With:

user=> (defprotocol p (f [_]))
p
user=> (deftype x [])
user.x
user=> (deftype y [])
user.y
user=> (extend-type x p (f [_]))
nil

Before patch:

user=> (let [x (x.)] (bench (satisfies? p x)))
Evaluation count : 548182380 in 60 samples of 9136373 calls.
             Execution time mean : 108.856460 ns
    Execution time std-deviation : 4.151711 ns
   Execution time lower quantile : 103.306368 ns ( 2.5%)
   Execution time upper quantile : 117.597299 ns (97.5%)
                   Overhead used : 1.681820 ns
nil
user=> (let [y (y.)] (bench (satisfies? p y)))
Evaluation count : 20220420 in 60 samples of 337007 calls.
             Execution time mean : 3.325396 µs
    Execution time std-deviation : 277.917798 ns
   Execution time lower quantile : 3.035664 µs ( 2.5%)
   Execution time upper quantile : 3.915870 µs (97.5%)
                   Overhead used : 1.681820 ns
nil

After patch:

user=> (let [x (x.)] (bench (satisfies? p x)))
Evaluation count : 3091276560 in 60 samples of 51521276 calls.
             Execution time mean : 19.048289 ns
    Execution time std-deviation : 0.724232 ns
   Execution time lower quantile : 17.558597 ns ( 2.5%)
   Execution time upper quantile : 20.067082 ns (97.5%)
                   Overhead used : 1.639685 ns
niluser=> (let [y (y.)] (bench (satisfies? p y)))
Evaluation count : 2699888040 in 60 samples of 44998134 calls.
             Execution time mean : 20.968108 ns
    Execution time std-deviation : 0.658803 ns
   Execution time lower quantile : 20.336564 ns ( 2.5%)
   Execution time upper quantile : 22.508062 ns (97.5%)
                   Overhead used : 1.639685 ns
nil

Patch: 0001-CLJ-1814-cache-protocol-impl-satisfies-as-fast-as-me-v3.patch



 Comments   
Comment by Michael Blume [ 11/Sep/15 4:17 PM ]

Nice. Honeysql used to spend 80-90% of its time in satisfies? calls before we refactored them out.

Comment by Michael Blume [ 24/Sep/15 3:55 PM ]

I realize this is a deeply annoying bug to reproduce, but if I clone core.match, point its Clojure dependency to 1.8.0-master-SNAPSHOT, start a REPL, connect to the REPL from vim, and reload clojure.core.match, I get

|| java.lang.Exception: namespace 'clojure.tools.analyzer.jvm.utils' not found, compiling:(clojure/tools/analyzer/jvm.clj:9:1)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5647| clojure.core$throw_if.invokeStatic
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5733| clojure.core$load_lib.invokeStatic
|| clojure.core$load_lib.doInvoke(core.clj)
|| clojure.lang.RestFn.applyTo(RestFn.java:142)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|647| clojure.core$apply.invokeStatic
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5765| clojure.core$load_libs.invokeStatic
|| clojure.core$load_libs.doInvoke(core.clj)
|| clojure.lang.RestFn.applyTo(RestFn.java:137)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|647| clojure.core$apply.invokeStatic
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5787| clojure.core$require.invokeStatic
|| clojure.core$require.doInvoke(core.clj)
|| clojure.lang.RestFn.invoke(RestFn.java:703)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/tools.analyzer.jvm/0.6.5/tools.analyzer.jvm-0.6.5.jar::clojure/tools/analyzer/jvm.clj|9| clojure.tools.analyzer.jvm$eval4968$loading__5561__auto____4969.invoke
zipfile:/Users/michael.blume/.m2/repository/org/clojure/tools.analyzer.jvm/0.6.5/tools.analyzer.jvm-0.6.5.jar::clojure/tools/analyzer/jvm.clj|9| clojure.tools.analyzer.jvm$eval4968.invokeStatic
|| clojure.tools.analyzer.jvm$eval4968.invoke(jvm.clj)
|| clojure.lang.Compiler.eval(Compiler.java:6934)
|| clojure.lang.Compiler.eval(Compiler.java:6923)
|| clojure.lang.Compiler.load(Compiler.java:7381)
|| clojure.lang.RT.loadResourceScript(RT.java:372)
|| clojure.lang.RT.loadResourceScript(RT.java:363)
|| clojure.lang.RT.load(RT.java:453)
|| clojure.lang.RT.load(RT.java:419)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5883| clojure.core$load$fn__5669.invoke
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5882| clojure.core$load.invokeStatic
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5683| clojure.core$load_one.invokeStatic
|| clojure.core$load_one.invoke(core.clj)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5728| clojure.core$load_lib$fn__5618.invoke
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5727| clojure.core$load_lib.invokeStatic
|| clojure.core$load_lib.doInvoke(core.clj)
|| clojure.lang.RestFn.applyTo(RestFn.java:142)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|647| clojure.core$apply.invokeStatic
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5765| clojure.core$load_libs.invokeStatic
|| clojure.core$load_libs.doInvoke(core.clj)
|| clojure.lang.RestFn.applyTo(RestFn.java:137)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|647| clojure.core$apply.invokeStatic
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5787| clojure.core$require.invokeStatic
|| clojure.core$require.doInvoke(core.clj)
|| clojure.lang.RestFn.invoke(RestFn.java:457)
src/main/clojure/clojure/core/match.clj|1| clojure.core.match$eval4960$loading__5561__auto____4961.invoke
src/main/clojure/clojure/core/match.clj|1| clojure.core.match$eval4960.invokeStatic
|| clojure.core.match$eval4960.invoke(match.clj)
|| clojure.lang.Compiler.eval(Compiler.java:6934)
|| clojure.lang.Compiler.eval(Compiler.java:6923)
|| clojure.lang.Compiler.load(Compiler.java:7381)
|| clojure.lang.RT.loadResourceScript(RT.java:372)
|| clojure.lang.RT.loadResourceScript(RT.java:363)
|| clojure.lang.RT.load(RT.java:453)
|| clojure.lang.RT.load(RT.java:419)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5883| clojure.core$load$fn__5669.invoke
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5882| clojure.core$load.invokeStatic
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5683| clojure.core$load_one.invokeStatic
|| clojure.core$load_one.invoke(core.clj)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5728| clojure.core$load_lib$fn__5618.invoke
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5727| clojure.core$load_lib.invokeStatic
|| clojure.core$load_lib.doInvoke(core.clj)
|| clojure.lang.RestFn.applyTo(RestFn.java:142)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|647| clojure.core$apply.invokeStatic
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5765| clojure.core$load_libs.invokeStatic
|| clojure.core$load_libs.doInvoke(core.clj)
|| clojure.lang.RestFn.applyTo(RestFn.java:137)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|647| clojure.core$apply.invokeStatic
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5787| clojure.core$require.invokeStatic
|| clojure.core$require.doInvoke(core.clj)
|| clojure.lang.RestFn.invoke(RestFn.java:421)
|| clojure.core.match$eval4949.invokeStatic(form-init2494799382238714928.clj:1)
|| clojure.core.match$eval4949.invoke(form-init2494799382238714928.clj)
|| clojure.lang.Compiler.eval(Compiler.java:6934)
|| clojure.lang.Compiler.eval(Compiler.java:6897)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|3096| clojure.core$eval.invokeStatic
|| clojure.core$eval.invoke(core.clj)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/main.clj|240| clojure.main$repl$read_eval_print__7404$fn__7407.invoke
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/main.clj|240| clojure.main$repl$read_eval_print__7404.invoke
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/main.clj|258| clojure.main$repl$fn__7413.invoke
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/main.clj|258| clojure.main$repl.invokeStatic
|| clojure.main$repl.doInvoke(main.clj)
|| clojure.lang.RestFn.invoke(RestFn.java:1523)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/tools.nrepl/0.2.10/tools.nrepl-0.2.10.jar::clojure/tools/nrepl/middleware/interruptible_eval.clj|58| clojure.tools.nrepl.middleware.interruptible_eval$evaluate$fn__637.invoke
|| clojure.lang.AFn.applyToHelper(AFn.java:152)
|| clojure.lang.AFn.applyTo(AFn.java:144)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|645| clojure.core$apply.invokeStatic
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|1874| clojure.core$with_bindings_STAR_.invokeStatic
|| clojure.core$with_bindings_STAR_.doInvoke(core.clj)
|| clojure.lang.RestFn.invoke(RestFn.java:425)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/tools.nrepl/0.2.10/tools.nrepl-0.2.10.jar::clojure/tools/nrepl/middleware/interruptible_eval.clj|56| clojure.tools.nrepl.middleware.interruptible_eval$evaluate.invokeStatic
|| clojure.tools.nrepl.middleware.interruptible_eval$evaluate.invoke(interruptible_eval.clj)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/tools.nrepl/0.2.10/tools.nrepl-0.2.10.jar::clojure/tools/nrepl/middleware/interruptible_eval.clj|191| clojure.tools.nrepl.middleware.interruptible_eval$interruptible_eval$fn__679$fn__682.invoke
zipfile:/Users/michael.blume/.m2/repository/org/clojure/tools.nrepl/0.2.10/tools.nrepl-0.2.10.jar::clojure/tools/nrepl/middleware/interruptible_eval.clj|159| clojure.tools.nrepl.middleware.interruptible_eval$run_next$fn__674.invoke
|| clojure.lang.AFn.run(AFn.java:22)
|| java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
|| java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
|| java.lang.Thread.run(Thread.java:745)

Same thing with reloading a namespace in my own project which depends on clojure.core.match

Comment by Nicola Mometto [ 24/Sep/15 3:59 PM ]

is it possible that AOT is involved?

Comment by Michael Blume [ 24/Sep/15 5:31 PM ]

Narrowed it down a little, if I check out tools.analyzer.jvm, open a REPL, and do (require 'clojure.tools.analyzer.jvm.utils) I get

|| java.lang.ClassCastException: java.lang.Class cannot be cast to clojure.asm.Type, compiling:(utils.clj:260:13)
|| clojure.lang.Compiler$InvokeExpr.eval(Compiler.java:3642)
|| clojure.lang.Compiler$InvokeExpr.eval(Compiler.java:3636)
|| clojure.lang.Compiler$DefExpr.eval(Compiler.java:450)
|| clojure.lang.Compiler.eval(Compiler.java:6939)
|| clojure.lang.Compiler.load(Compiler.java:7381)
|| clojure.lang.RT.loadResourceScript(RT.java:372)
|| clojure.lang.RT.loadResourceScript(RT.java:363)
|| clojure.lang.RT.load(RT.java:453)
|| clojure.lang.RT.load(RT.java:419)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5883| clojure.core$load$fn__5669.invoke
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5882| clojure.core$load.invokeStatic
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5683| clojure.core$load_one.invokeStatic
|| clojure.core$load_one.invoke(core.clj)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5728| clojure.core$load_lib$fn__5618.invoke
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5727| clojure.core$load_lib.invokeStatic
|| clojure.core$load_lib.doInvoke(core.clj)
|| clojure.lang.RestFn.applyTo(RestFn.java:142)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|647| clojure.core$apply.invokeStatic
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5765| clojure.core$load_libs.invokeStatic
|| clojure.core$load_libs.doInvoke(core.clj)
|| clojure.lang.RestFn.applyTo(RestFn.java:137)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|647| clojure.core$apply.invokeStatic
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5787| clojure.core$require.invokeStatic
|| clojure.core$require.doInvoke(core.clj)
|| clojure.lang.RestFn.invoke(RestFn.java:421)
|| clojure.tools.analyzer.jvm.utils$eval4392.invokeStatic(form-init8663423518975891793.clj:1)
|| clojure.tools.analyzer.jvm.utils$eval4392.invoke(form-init8663423518975891793.clj)
|| clojure.lang.Compiler.eval(Compiler.java:6934)
|| clojure.lang.Compiler.eval(Compiler.java:6897)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|3096| clojure.core$eval.invokeStatic
|| clojure.core$eval.invoke(core.clj)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/main.clj|240| clojure.main$repl$read_eval_print__7404$fn__7407.invoke
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/main.clj|240| clojure.main$repl$read_eval_print__7404.invoke
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/main.clj|258| clojure.main$repl$fn__7413.invoke
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/main.clj|258| clojure.main$repl.invokeStatic
|| clojure.main$repl.doInvoke(main.clj)
|| clojure.lang.RestFn.invoke(RestFn.java:1523)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/tools.nrepl/0.2.10/tools.nrepl-0.2.10.jar::clojure/tools/nrepl/middleware/interruptible_eval.clj|58| clojure.tools.nrepl.middleware.interruptible_eval$evaluate$fn__637.invoke
|| clojure.lang.AFn.applyToHelper(AFn.java:152)
|| clojure.lang.AFn.applyTo(AFn.java:144)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|645| clojure.core$apply.invokeStatic
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|1874| clojure.core$with_bindings_STAR_.invokeStatic
|| clojure.core$with_bindings_STAR_.doInvoke(core.clj)
|| clojure.lang.RestFn.invoke(RestFn.java:425)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/tools.nrepl/0.2.10/tools.nrepl-0.2.10.jar::clojure/tools/nrepl/middleware/interruptible_eval.clj|56| clojure.tools.nrepl.middleware.interruptible_eval$evaluate.invokeStatic
|| clojure.tools.nrepl.middleware.interruptible_eval$evaluate.invoke(interruptible_eval.clj)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/tools.nrepl/0.2.10/tools.nrepl-0.2.10.jar::clojure/tools/nrepl/middleware/interruptible_eval.clj|191| clojure.tools.nrepl.middleware.interruptible_eval$interruptible_eval$fn__679$fn__682.invoke
zipfile:/Users/michael.blume/.m2/repository/org/clojure/tools.nrepl/0.2.10/tools.nrepl-0.2.10.jar::clojure/tools/nrepl/middleware/interruptible_eval.clj|159| clojure.tools.nrepl.middleware.interruptible_eval$run_next$fn__674.invoke
|| clojure.lang.AFn.run(AFn.java:22)
|| java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
|| java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
|| java.lang.Thread.run(Thread.java:745)

I don't see where AOT would be involved?

Comment by Nicola Mometto [ 27/Sep/15 2:28 PM ]

Michael Blume The updated patch should fix the issue you reported

Comment by Michael Blume [ 28/Sep/15 12:39 PM ]

Cool, thanks =)

New patch no longer deletes MethodImplCache, which is not used – is that deliberate?

Comment by Alex Miller [ 02/Nov/15 3:08 PM ]

It would be cool if there was a bulleted list of the things changed in the patch in the description. For example: "Renamed MethodImplCache to ImplCache", etc. That helps makes it easier to review.

Comment by Nicola Mometto [ 02/Nov/15 3:35 PM ]

Attached is an updated patch that doesn't replace MethodImplCache with ImplCache but simply reuses MethodImplCache, reducing the impact of this patch and making it easier (and safer) to review.





[CLJ-1813] Improve use-fixtures docstring Created: 10/Sep/15  Updated: 13/Oct/15

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

Type: Enhancement Priority: Minor
Reporter: Daniel Compton Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: clojure.test, docstring

Attachments: Text File fixture-docstring.patch    

 Description   

The docstring for use-fixtures says

Wrap test runs in a fixture function to perform setup and teardown. 
Using a fixture-type of :each wraps every test individually, 
while: once wraps the whole run in a single function

I think it would be helpful to explain what a fixture function is and how it performs setup and teardown. I know because I've looked at examples, but I don't think the docstring explains this at all.

Is this something Core is interested in taking a patch on?



 Comments   
Comment by Alex Miller [ 10/Sep/15 9:08 PM ]

It's explained in the clojure.test ns docstring in more depth:

Fixtures are attached to namespaces in one of two ways.  \"each\"
   fixtures are run repeatedly, once for each test function created
   with \"deftest\" or \"with-test\".  \"each\" fixtures are useful for
   establishing a consistent before/after state for each test, like
   clearing out database tables.

   \"each\" fixtures can be attached to the current namespace like this:
   (use-fixtures :each fixture1 fixture2 ...)
   The fixture1, fixture2 are just functions like the example above.
   They can also be anonymous functions, like this:
   (use-fixtures :each (fn [f] setup... (f) cleanup...))

   The other kind of fixture, a \"once\" fixture, is only run once,
   around ALL the tests in the namespace.  \"once\" fixtures are useful
   for tasks that only need to be performed once, like establishing
   database connections, or for time-consuming tasks.

   Attach \"once\" fixtures to the current namespace like this:
   (use-fixtures :once fixture1 fixture2 ...)

I'm not really answering your question, just wondering if you saw that and whether it changes your question.

Comment by Daniel Compton [ 10/Sep/15 9:15 PM ]

Perhaps just pointing the user towards the ns docstring would be a good alternative? I had forgotten about the docstring on the ns, and I'm not sure whether duplicating the docs about fixtures in use-fixtures is a great idea.

Perhaps something like this?

Wrap test runs in a fixture function to perform setup and teardown. 
Using a fixture-type of :each wraps every test individually, 
while :once wraps the whole run in a single function

See the clojure.test docstring for more details.




[CLJ-1812] Recently-added test fails on Windows Created: 06/Sep/15  Updated: 11/Sep/15  Resolved: 11/Sep/15

Status: Closed
Project: Clojure
Component/s: None
Affects Version/s: Release 1.8
Fix Version/s: Release 1.8

Type: Defect Priority: Minor
Reporter: Andy Fingerhut Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: regression
Environment:

Windows


Attachments: Text File CLJ-1812-fix1.patch    
Patch: Code and Test
Approval: Ok

 Description   

Inside of deftest test-pprint-calendar in file test/clojure/test_clojure/pprint/test_pretty.clj, it compares the output of pprint against a string ending in a newline character. This works fine on Linux and Mac OS X, but fails on Windows systems, as there the pprint'ed string ends with a carriage return followed by newline.

Approach: A straightforward fix is to call clojure.string/split-lines on the string, which has a return value that is independent of OS.

Screened: Alex Miller



 Comments   
Comment by Andy Fingerhut [ 06/Sep/15 4:21 PM ]

Patch CLJ-1812-fix1.patch dated Sep 6 2015 allows the test to pass on Windows as well as Mac OS X.

Comment by Alex Miller [ 08/Sep/15 8:02 AM ]

Since this breaks the build on windows, I added it to 1.8 and screened it.





[CLJ-1811] test line reporting doesn't always report test's file & line number Created: 02/Sep/15  Updated: 03/Dec/15

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

Type: Enhancement Priority: Minor
Reporter: Ryan Fowler Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: test

Attachments: Text File clj-1811.patch     File error_reporting.clj     Text File LINE_REPORING_1.patch     Text File LINE_REPORING_2.patch    
Patch: Code and Test
Approval: Triaged

 Description   

If an Exception is thrown during test execution, the filename and line number are frequently not helpful for finding the problem. For instance, this code:

error_reporting.clj
(require '[clojure.test :refer [deftest test-var]])

(deftest foo
  (meta))

(test-var #'foo)

Will output an error at AFn.java:429.

ERROR in (foo) (AFn.java:429)
Uncaught exception, not in assertion.
expected: nil
  actual: clojure.lang.ArityException: Wrong number of args (0) passed to: core/meta--4144
 at clojure.lang.AFn.throwArity (AFn.java:429)
    clojure.lang.AFn.invoke (AFn.java:28)
    user/fn (error_reporting.clj:4)
    clojure.test$test_var$fn__7670.invoke (test.clj:704)
    clojure.test$test_var.invoke (test.clj:704)
    user$eval6.invoke (error_reporting.clj:6)
    clojure.lang.Compiler.eval (Compiler.java:6782)
    ...etc

Rich's Comment 24016 on CLJ-377 says that he thinks the message should report the test file line rather than where the exception was thrown.

Approach: Filter the stacktrace class prefix clojure.lang.AFn from the top of error stacktraces.

After applying the patch, the above example outputs error_reporting.clj:4:

ERROR in (foo) (error_reporting.clj:4)
Uncaught exception, not in assertion.
expected: nil
  actual: clojure.lang.ArityException: Wrong number of args (0) passed to: core/meta--4141
 at clojure.lang.AFn.throwArity (AFn.java:429)
    clojure.lang.AFn.invoke (AFn.java:28)
    user$fn__3.invokeStatic (error_reporting.clj:4)
    user/fn (error_reporting.clj:3)
    clojure.test$test_var$fn__114.invoke (test.clj:705)
    clojure.test$test_var.invokeStatic (test.clj:705)
    clojure.test$test_var.invoke (test.clj:-1)
    user$eval6.invokeStatic (error_reporting.clj:6)
    user$eval6.invoke (error_reporting.clj:-1)
    clojure.lang.Compiler.eval (Compiler.java:6939)
    ...etc

Patch: clj-1811.patch



 Comments   
Comment by Ryan Fowler [ 02/Sep/15 5:36 PM ]

example file from description

Comment by Alex Miller [ 04/Sep/15 10:04 AM ]

A quick search on Github shows many cases where people call into the (admittedly private) file-and-line function. These users would be broken by the patch. Perhaps it would be better to create a new function or a new arity rather than removing the existing arity.

Just eyeballing it, but I suspect you've introduced reflection in a couple places in the new code, particularly these might need another type hint:

1. (.getName (.getClass (:test (meta test-var))))
2. #(= (.getClassName %) test-var-class-name)

I need to look at more of the code to make a judgement on everything else. Seeing testing-vars in there means that this function is now dependent on external state, so need to think carefully to be sure that every calling context has that global state (or won't fail in bad ways if it doesn't). It would be helpful to see a discussion of your thinking about that in the "approach" section of the ticket description.

Comment by Ryan Fowler [ 30/Sep/15 3:41 PM ]

Second attempt at a patch to resolve this issue. Corrects issues Alex pointed out

Comment by Ryan Fowler [ 30/Sep/15 3:53 PM ]

I've filled in some detail in the approach section.

I also added a new patch LINE_REPORTING_2.patch that addresses reflection warnings, restores the old arity of file-and-line and adds protection from people calling file-and-line from outside a testing context.

Comment by Ryan Fowler [ 30/Sep/15 6:20 PM ]

While discussing an issue with my coworker James, I realized that this fix helps with shared functions calling (is). Notice how the run of this sample code reports line 7 with LINE_REPORTING_2.patch applied. This test line is generally much more useful than the shared function line.

example_2
ryans-mbp:~/oss/clojure% cat -n error_reporting.clj
     1  (require '[clojure.test :refer [deftest test-var is]])
     2
     3  (defn shared-code [arg]
     4    (is arg))
     5
     6  (deftest test-shared-code
     7    (shared-code false))
     8
     9  (test-var #'test-shared-code)
ryans-mbp:~/oss/clojure% java -jar ~/.m2/repository/org/clojure/clojure/1.7.0/clojure-1.7.0.jar ./error_reporting.clj

FAIL in (test-shared-code) (error_reporting.clj:4)
expected: arg
  actual: false
ryans-mbp:~/oss/clojure% java -jar target/clojure-1.8.0-master-SNAPSHOT.jar ./error_reporting.clj

FAIL in (test-shared-code) (error_reporting.clj:7)
expected: arg
  actual: false
Comment by Michael Blume [ 02/Dec/15 5:55 PM ]

Patch doesn't apply anymore, but maybe this has been sorted by CLJ-1856?

Comment by Alex Miller [ 03/Dec/15 9:57 AM ]

This is not fixed by CLJ-1856, but could be if clojure.lang.AFn was filtered out of error stacktraces when finding the location. This is pretty specific - it might make sense to broaden to other calling paths too, not sure.

Attached a new patch that applies this filtering.





[CLJ-1810] ATransientMap not marked public Created: 31/Aug/15  Updated: 12/Oct/15  Resolved: 12/Oct/15

Status: Closed
Project: Clojure
Component/s: None
Affects Version/s: Release 1.7, Release 1.8
Fix Version/s: Release 1.8

Type: Enhancement Priority: Trivial
Reporter: Gregg Reynolds Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: ft, transient
Environment:

all


Attachments: Text File CLJ-1810-v1.patch    
Patch: Code
Approval: Ok

 Description   

All the other abstract classes are explicitly marked "public". Seems like ATransientMap should be marked like the others.



 Comments   
Comment by Michael Blume [ 31/Aug/15 5:04 PM ]

Here is the obvious patch =)





[CLJ-1809] Compiler produces VerifyError when compiling simple let expression inside a finally block Created: 30/Aug/15  Updated: 27/Oct/15  Resolved: 27/Oct/15

Status: Closed
Project: Clojure
Component/s: None
Affects Version/s: Release 1.8
Fix Version/s: Release 1.8

Type: Defect Priority: Critical
Reporter: Nicola Mometto Assignee: Fogus
Resolution: Completed Votes: 0
Labels: compiler, regression

Attachments: Text File 0001-CLJ-1809-fix-off-by-one-error-in-direct-linking.patch     Text File clj-1809-2.patch     Text File clj-1809-3.patch    
Patch: Code and Test
Approval: Ok

 Description   

A variant of this issue showed up as it was preventing compilation in ClojureScript.

This is a simplified case (see original in comments):

(defn x [y]
  (try
    (finally
      (let [z y]))))

produces

VerifyError (class: user$x, method: invokeStatic signature: (Ljava/lang/Object;)Ljava/lang/Object;) Can only throw Throwable objects  java.lang.Class.getDeclaredConstructors0 (Class.java:-2)

See below for comparison bytecode.

Cause: In this code, there are locals with these indexes:

0 - this (if not static call)
1 - y (arg)
2 - retLocal (created by compiler to hold result of try body)
3 - finalLocal (created by compiler to hold result of finally body)
4 - z (let local)

The following block was added to FnExpr.parse() for static methods to adjust local binding stack indexes based on not having a "this":

if(fn.canBeDirect){
    for(FnMethod fm : (Collection<FnMethod>)methods)
    {
      if(fm.locals != null)
      {
        for(LocalBinding lb : (Collection<LocalBinding>)RT.keys(fm.locals))
        {
          lb.idx -= 1;
	}
	fm.maxLocal -= 1;
      }
    }
  }

However, in this example locals 2 and 3 are never registered with fn (registerLocal is not called). This (doesn't) happen in TryExpr.parse() where retLocal and finallyLocal call getAndIncLocalNum() but not via registerLocal(). From a search, there are several other places where this happens as well.

The result in the example above is that we end up with the following indexes:

0 - y (arg)
2 - retLocal (created by compiler to hold result of try body)
3 - finalLocal (created by compiler to hold result of finally body)
3 - z (let local)

The overlap in the last 2 indices leads ultimately to the verifier error.

Approach: Make the lb.index adjustment only happen when lb.isArg - these should always be at the beginning of the locals table and therefore reducing their indexes will not affect any other added locals. Also, do not adjust fm.maxlocal (fyi, maxlocal is never used for anything).

Patch: clj-1809-3.patch

Disabling the verifier, here's a dump of the emitted bytecode for inspection

// 1.8
  public static java.lang.Object invokeStatic(java.lang.Object);
    Code:
       0: aconst_null
       1: astore_1
       2: aload_0
       3: aconst_null
       4: astore_0
       5: astore_2
       6: aconst_null
       7: pop
       8: goto 20
      11: astore_2
      12: aload_0
      13: aconst_null
      14: astore_0
      15: astore_2
      16: aconst_null
      17: pop
      18: aload_2
      19: athrow
      20: aload_1
      21: areturn
    Exception table:
       from to target type
           0 2 11 any

Here's the bytecode emitted by clojure 1.7.0 for comparison

// 1.7
  public java.lang.Object invoke(java.lang.Object);
    Code:
       0: aconst_null
       1: astore_2
       2: aload_1
       3: aconst_null
       4: astore_1
       5: astore_3
       6: aconst_null
       7: pop
       8: goto          22
      11: astore        4
      13: aload_1
      14: aconst_null
      15: astore_1
      16: astore_3
      17: aconst_null
      18: pop
      19: aload         4
      21: athrow
      22: aload_2
      23: areturn
    Exception table:
       from    to  target type
           0     2    11   any


 Comments   
Comment by Nicola Mometto [ 30/Aug/15 11:30 PM ]

The attached patch fixes the issue however I'm unfamiliar with the direct linking support in the compiler so I'm not sure this is the right fix.

Comment by Keith Irwin [ 11/Sep/15 12:25 PM ]

I discovered this issue compiling ClojureScript applications using lein-figwheel, which invokes cljsbuild. Using just lein-cljsbuild produces the same issue.

Stack Trace:

Exception in thread "main" java.lang.VerifyError: (class: cljs/util$last_modified, method: invokeStatic signature: (Ljava/lang/Object;)Ljava/lang/Object;) Can only throw Throwable objects, compiling:(util.cljc:142:1)
	at clojure.lang.Compiler$DefExpr.eval(Compiler.java:463)
	at clojure.lang.Compiler.eval(Compiler.java:6939)
	at clojure.lang.Compiler.load(Compiler.java:7381)
	at clojure.lang.RT.loadResourceScript(RT.java:372)
	at clojure.lang.RT.loadResourceScript(RT.java:363)
	at clojure.lang.RT.load(RT.java:453)
	at clojure.lang.RT.load(RT.java:419)
	at clojure.core$load$fn__5661.invoke(core.clj:5883)
	at clojure.core$load.invokeStatic(core.clj:5882)
	at clojure.core$load_one.invokeStatic(core.clj:5683)
	at clojure.core$load_one.invoke(core.clj)
	at clojure.core$load_lib$fn__5610.invoke(core.clj:5728)
	at clojure.core$load_lib.invokeStatic(core.clj:5727)
	at clojure.core$load_lib.doInvoke(core.clj)
	at clojure.lang.RestFn.applyTo(RestFn.java:142)
	at clojure.core$apply.invokeStatic(core.clj:647)
	at clojure.core$load_libs.invokeStatic(core.clj:5765)
	at clojure.core$load_libs.doInvoke(core.clj)
	at clojure.lang.RestFn.applyTo(RestFn.java:137)
	at clojure.core$apply.invokeStatic(core.clj:647)
	at clojure.core$require.invokeStatic(core.clj:5787)
	at clojure.core$require.doInvoke(core.clj)
	at clojure.lang.RestFn.invoke(RestFn.java:2088)
	at cljs.repl$eval15$loading__5553__auto____16.invoke(repl.cljc:9)
	at cljs.repl$eval15.invokeStatic(repl.cljc:9)
	at cljs.repl$eval15.invoke(repl.cljc)
	at clojure.lang.Compiler.eval(Compiler.java:6934)
	at clojure.lang.Compiler.eval(Compiler.java:6923)
	at clojure.lang.Compiler.load(Compiler.java:7381)
	at clojure.lang.RT.loadResourceScript(RT.java:372)
	at clojure.lang.RT.loadResourceScript(RT.java:363)
	at clojure.lang.RT.load(RT.java:453)
	at clojure.lang.RT.load(RT.java:419)
	at clojure.core$load$fn__5661.invoke(core.clj:5883)
	at clojure.core$load.invokeStatic(core.clj:5882)
	at clojure.core$load_one.invokeStatic(core.clj:5683)
	at clojure.core$load_one.invoke(core.clj)
	at clojure.core$load_lib$fn__5610.invoke(core.clj:5728)
	at clojure.core$load_lib.invokeStatic(core.clj:5727)
	at clojure.core$load_lib.doInvoke(core.clj)
	at clojure.lang.RestFn.applyTo(RestFn.java:142)
	at clojure.core$apply.invokeStatic(core.clj:647)
	at clojure.core$load_libs.invokeStatic(core.clj:5765)
	at clojure.core$load_libs.doInvoke(core.clj)
	at clojure.lang.RestFn.applyTo(RestFn.java:137)
	at clojure.core$apply.invokeStatic(core.clj:647)
	at clojure.core$require.invokeStatic(core.clj:5787)
	at clojure.core$require.doInvoke(core.clj)
	at clojure.lang.RestFn.invoke(RestFn.java:3204)
	at figwheel_sidecar.repl$eval9$loading__5553__auto____10.invoke(repl.clj:1)
	at figwheel_sidecar.repl$eval9.invokeStatic(repl.clj:1)
	at figwheel_sidecar.repl$eval9.invoke(repl.clj)
	at clojure.lang.Compiler.eval(Compiler.java:6934)
	at clojure.lang.Compiler.eval(Compiler.java:6923)
	at clojure.lang.Compiler.load(Compiler.java:7381)
	at clojure.lang.RT.loadResourceScript(RT.java:372)
	at clojure.lang.RT.loadResourceScript(RT.java:363)
	at clojure.lang.RT.load(RT.java:453)
	at clojure.lang.RT.load(RT.java:419)
	at clojure.core$load$fn__5661.invoke(core.clj:5883)
	at clojure.core$load.invokeStatic(core.clj:5882)
	at clojure.core$load_one.invokeStatic(core.clj:5683)
	at clojure.core$load_one.invoke(core.clj)
	at clojure.core$load_lib$fn__5610.invoke(core.clj:5728)
	at clojure.core$load_lib.invokeStatic(core.clj:5727)
	at clojure.core$load_lib.doInvoke(core.clj)
	at clojure.lang.RestFn.applyTo(RestFn.java:142)
	at clojure.core$apply.invokeStatic(core.clj:647)
	at clojure.core$load_libs.invokeStatic(core.clj:5765)
	at clojure.core$load_libs.doInvoke(core.clj)
	at clojure.lang.RestFn.applyTo(RestFn.java:137)
	at clojure.core$apply.invokeStatic(core.clj:647)
	at clojure.core$require.invokeStatic(core.clj:5787)
	at clojure.core$require.doInvoke(core.clj)
	at clojure.lang.RestFn.invoke(RestFn.java:408)
	at user$eval5.invokeStatic(form-init6950918879748180251.clj:1)
	at user$eval5.invoke(form-init6950918879748180251.clj)
	at clojure.lang.Compiler.eval(Compiler.java:6934)
	at clojure.lang.Compiler.eval(Compiler.java:6923)
	at clojure.lang.Compiler.load(Compiler.java:7381)
	at clojure.lang.Compiler.loadFile(Compiler.java:7319)
	at clojure.main$load_script.invokeStatic(main.clj:275)
	at clojure.main$init_opt.invokeStatic(main.clj:277)
	at clojure.main$init_opt.invoke(main.clj)
	at clojure.main$initialize.invokeStatic(main.clj:308)
	at clojure.main$null_opt.invokeStatic(main.clj:342)
	at clojure.main$null_opt.invoke(main.clj)
	at clojure.main$main.invokeStatic(main.clj:421)
	at clojure.main$main.doInvoke(main.clj)
	at clojure.lang.RestFn.invoke(RestFn.java:421)
	at clojure.lang.Var.invoke(Var.java:383)
	at clojure.lang.AFn.applyToHelper(AFn.java:156)
	at clojure.lang.Var.applyTo(Var.java:700)
	at clojure.main.main(main.java:37)
Caused by: java.lang.VerifyError: (class: cljs/util$last_modified, method: invokeStatic signature: (Ljava/lang/Object;)Ljava/lang/Object;) Can only throw Throwable objects
	at java.lang.Class.getDeclaredConstructors0(Native Method)
	at java.lang.Class.privateGetDeclaredConstructors(Class.java:2671)
	at java.lang.Class.getConstructor0(Class.java:3075)
	at java.lang.Class.newInstance(Class.java:412)
	at clojure.lang.Compiler$ObjExpr.eval(Compiler.java:4923)
	at clojure.lang.Compiler$DefExpr.eval(Compiler.java:450)
	... 93 more
Comment by Keith Irwin [ 11/Sep/15 12:39 PM ]

Reproducible using ClojureScript 1.7.122, but not 1.7.48 or 1.7.58.

Comment by Keith Irwin [ 11/Sep/15 12:43 PM ]

Here's where the error is thrown:

https://github.com/clojure/clojurescript/blob/master/src/main/clojure/cljs/util.cljc#L142-L155

(defn last-modified [src]
  (cond
    (file? src) (.lastModified ^File src)
    (url? src)
    (let [conn (.openConnection ^URL src)]
      (try
        (.getLastModified conn)
        (finally
          (let [ins (.getInputStream conn)]
            (when ins
              (.close ins))))))
    :else
    (throw
      (IllegalArgumentException. (str "Cannot get last modified for " src)))))
Comment by Nicola Mometto [ 11/Sep/15 12:50 PM ]

Keith Irwin yeah that's how the bug originally got reported and that's the function I used to find a minimal reproducible example

Comment by Alex Miller [ 18/Sep/15 10:41 AM ]

clj-1809-2.patch is identical to prior patch, just updated to apply to current master.

Comment by Fogus [ 09/Oct/15 12:35 PM ]

With some digging I was able to determine the problem and how the solution works to fix that problem. In the future, whenever reporting bytecode verification errors it might help to show the equivalent Java code pertaining to the problemmatic bytecode.

Comment by Rich Hickey [ 26/Oct/15 3:00 PM ]

Thanks for chasing this down Nicola.





[CLJ-1808] map-invert should use transients and reduce-kv instead of reduce Created: 30/Aug/15  Updated: 18/Jan/16

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

Type: Enhancement Priority: Minor
Reporter: Nikita Prokopov Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: ft

Attachments: Text File clj-1808-map-invert-should-use-reduce-kv-and-transient.patch    
Patch: Code
Approval: Prescreened

 Description   

Two performance enhancements on clojure.set/map-invert:

1) Use reduce-kv to avoid realizing mapentry's from input map
2) Use transients to create the output map

Patch: clj-1808-map-invert-should-use-reduce-kv-and-transient.patch



 Comments   
Comment by Alex Miller [ 04/Sep/15 10:42 AM ]

Would be nice to see a quick perf test that compared before/after times.





[CLJ-1807] Add prefer-proto, like prefer-method but for protocols Created: 30/Aug/15  Updated: 04/Sep/15

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

Type: Enhancement Priority: Major
Reporter: Nicola Mometto Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: protocols

Attachments: Text File 0001-CLJ-1807-add-prefer-proto.patch    
Patch: Code and Test
Approval: Triaged

 Description   

Currently it's possible to extend a protocol to multiple interfaces but there's no mechanism like prefer-method for multimethods to prefer one implementation over another, as a result, if multiple interfaces match, a random one is picked.

One particular example where this is a problem, is trying to handle generically records and maps (this come up in tools.analyzer): when extending a protocol to both IRecord and IPersistentMap there's no way to make the IRecord implementation be chosen over the IPersistentMap one and thus protocols can't be used.

The attached patch adds a prefer-proto function that's like prefer-method but for protocols.

No performance penalty is paid if prefer-proto is never used, if it's used there will be a penalty during the first protocol method dispatch to lookup the perference table but the protocol method cache will remove that penalty for further calls.

Example:

user=> (defprotocol p (f [_]))
p
user=> (extend-protocol p clojure.lang.Counted (f [_] 1) clojure.lang.IObj (f [_] 2))
nil
user=> (f [1])
2
user=> (prefer-proto p clojure.lang.Counted clojure.lang.IObj)
nil
user=> (f [1])
1

Patch: 0001-CLJ-1807-add-prefer-proto.patch






[CLJ-1806] group-by as reducer / reduction fn Created: 29/Aug/15  Updated: 04/Sep/15

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

Type: Enhancement Priority: Minor
Reporter: Karsten Schmidt Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: reducers, transducers

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

 Description   

Whilst working on a query engine heavily based on transducers, I noticed that it'd be great to have group-by able to be used as reduction fn. The attached patch adds a single arity version to group-by which enables this use case and also includes a few tests.



 Comments   
Comment by Karsten Schmidt [ 29/Aug/15 8:08 PM ]

patch w/ described changes





[CLJ-1805] :rettag encourages wrong runtime type hints Created: 28/Aug/15  Updated: 11/Nov/15  Resolved: 09/Nov/15

Status: Closed
Project: Clojure
Component/s: None
Affects Version/s: Release 1.8
Fix Version/s: Release 1.8

Type: Defect Priority: Critical
Reporter: Nicola Mometto Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: typehints

Attachments: Text File clj-1805.patch    
Patch: Code
Approval: Ok

 Description   

Currently :rettag works only for expressions like:

(defn ^long x [..] ..)

or

(defn ^double x [..] ..)

But at runtime those type-hints are resolved to

#<clojure.core$long ..>
and
#<clojure.core$double ..>

which can cause the compiler to fail, see CLJ-1674 for an example

Patch: clj-1805.patch fixes the bad boolean logic mentioned in the comments.



 Comments   
Comment by Nicola Mometto [ 29/Aug/15 1:27 PM ]

It also appears that the current impl is completely useless: see David Miller's comment: https://github.com/clojure/clojure/commit/2344de2b2aadd5b0e47f1594a6f9e4eb2fdbdf5c#commitcomment-12962027

Comment by Andy Fingerhut [ 11/Sep/15 3:58 PM ]

Alex, it appears that the code commented on by David Miller definitely has a bug, with a simple fix (change || to && would be one fix, I believe). Should there be a separate ticket for that fix from this ticket?

Comment by Alex Miller [ 29/Sep/15 11:53 AM ]

It seems like the existing code also works for things like:

(defn ^"long" x [..] ..)

?

Comment by Nicola Mometto [ 29/Sep/15 12:09 PM ]

Yes, which is equally useless:

user=> (set! *warn-on-reflection* true)
true
user=> (defn ^"long" a [] 1)
#'user/a
user=> (loop [x 0] (if (= x 1) x (recur (a))))
NO_SOURCE_FILE:3 recur arg for primitive local: x is not matching primitive, had: Object, needed: long
Auto-boxing loop arg: x
1
Comment by Nicola Mometto [ 28/Oct/15 6:01 AM ]

I'm reopening this issue as the committed patch doesn't deal with the issues I've originally opened this ticket for.
Namely that I think that in its current incarnation there is no value in :rettag as all its possible usages end up in an invalid tag either at runtime or at compile time.
Additionally it breaks the eastwood linter, an issue that I would be willing to fix on the eastwood side if the feature was valuable but I don't think this is the case.

Comment by Alex Miller [ 28/Oct/15 9:03 AM ]

Can you explain how this breaks Eastwood?

Comment by Andy Fingerhut [ 28/Oct/15 9:49 AM ]

I haven't looked into it in enough detail to fix it yet, but I am pretty sure that the reason Eastwood misbehaves with Clojure 1.8.0, at least since :rettag was added, is simply because Eastwood assumes that some ASTs will be the direct children of other ASTs, but when :rettag was added, they now have a new intermediate AST node between them. I haven't modified Eastwood to handle that change yet, but as Nicola mentioned, it should be straightforward to generalize Eastwood's AST handling here.

Comment by Alex Miller [ 28/Oct/15 10:00 AM ]

That sounds different than the problem described in the subject of this ticket.

Comment by Andy Fingerhut [ 28/Oct/15 10:04 AM ]

I'm not sure I'm understanding Nicola here, but wanted to ask a question that may clarify it.

Nicola, are you saying that with :rettag, it doesn't actually make things worse for adding tags in Clojure source code, but it isn't clear to you that it makes anything better, either, so why bother making such a change?

That is, in 1.8.0-beta2, functions like (defn ^long x [...] ...) still have a type tag that is evaluated to the function called 'long', and are thus incorrect type tags as they were in Clojure 1.7.0, so what use is :rettag in improving anything?

Comment by Nicola Mometto [ 28/Oct/15 12:39 PM ]

Right, what I was trying to say is that there is no apparent bug caused by the current implementation of :rettag, however:

  • it doesn't seem to be useful at all given that all the possible usages seem lead to an invalid tag
  • it complicates the compiler for no apparent reason
  • it breaks eastwood and possibly other libraries that rely on the current concrete expansion of defn

I would not bring the last point up if the feature was in any way valuable, but given that it doesn't seem so, I brought it up to make the point about why I'd like this change to be reverted or revisited.

Comment by Andy Fingerhut [ 29/Oct/15 9:32 AM ]

As far as I know (I haven't dug into the :rettag implementation the way Nicola has), (defn ^long x [...] ...) was a useless type tag in Clojure 1.7 and earlier, and it is still a useless type tag in Clojure 1.8-beta2.

(defn {:tag 'long} x [...] ...) is a useful type tag in Clojure 1.7 and earlier (back to some version number I'm not going to check right now), and it is still a useful type tag in Clojure 1.8-beta2.

Is that all correct?

Are there any type tags whose behavior changes from Clojure 1.7 to 1.8-beta2?

Comment by Nicola Mometto [ 29/Oct/15 10:05 AM ]

Correct.
:rettag was never intended (as far as I understan by RIch's commits) to have any behavioural change, rather it should have served as an optimization to avoid checkcasts/boxing w/ direct-linking, but it doesn't work so I see no point in keeping non-working code around that might even cause some tooling libraries to error-out (like eastwood).

Comment by Andy Fingerhut [ 30/Oct/15 1:58 PM ]

Nicola, could it be that Rich wants to add :rettag to make a single common place to go to find that information that is in metadata on the function, as opposed to more deeply hidden inside the compiler?

Comment by Nicola Mometto [ 31/Oct/15 10:00 AM ]

No, :rettag is still just compile-time metadata

Comment by Alex Miller [ 09/Nov/15 3:48 PM ]

I'm re-closing this as the ticket really was intended to be a question about rettag's role and purpose. I have added that to a list of items to discuss with Rich.

Comment by Nicola Mometto [ 11/Nov/15 1:18 PM ]

I'm hoping to have the questions I've raised re: why are we keeping :rettag around if it serves no purpose answered before 1.8 is released





[CLJ-1804] take transducer optimization Created: 25/Aug/15  Updated: 26/Aug/15

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

Type: Enhancement Priority: Minor
Reporter: Karsten Schmidt Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: performance, transducers

Attachments: Text File clj-1804-1.patch     Text File clj-1804-2.patch    
Patch: Code
Approval: Triaged

 Description   

A basic refactoring to remove the let form and only requires a single counter check for each iteration, yields an 25% performance increase. With the patch, 2 checks are only required for the last iteration (in case counter arg was <= 0)...

;; master
(quick-bench (into [] (take 1000) (range 2000)))
WARNING: Final GC required 34.82584189073624 % of runtime
Evaluation count : 13050 in 6 samples of 2175 calls.
             Execution time mean : 46.921254 µs
    Execution time std-deviation : 1.904733 µs
   Execution time lower quantile : 45.124921 µs ( 2.5%)
   Execution time upper quantile : 49.427201 µs (97.5%)
                   Overhead used : 2.367243 ns

;; w/ patch
(quick-bench (into [] (take 1000) (range 2000)))
WARNING: Final GC required 34.74448252054369 % of runtime
Evaluation count : 18102 in 6 samples of 3017 calls.
             Execution time mean : 34.301193 µs
    Execution time std-deviation : 1.714105 µs
   Execution time lower quantile : 32.341349 µs ( 2.5%)
   Execution time upper quantile : 37.046851 µs (97.5%)
                   Overhead used : 2.367243 ns


 Comments   
Comment by Karsten Schmidt [ 25/Aug/15 10:35 AM ]

Proposed patch, passes all existing tests

Comment by Alex Miller [ 25/Aug/15 11:28 AM ]

From a superficial skim, I see checks for pos? and then neg? which both exclude 0 and that gives me doubts about that branch. That may actually be ok but I will have to read it more closely.

Comment by Karsten Schmidt [ 25/Aug/15 11:58 AM ]

Hi Alex, try running the tests... AFAICT it's all still working as advertised: For (take 0) or (take -1) then the pos? check fails, but we must ensure to not call rf for that iteration. For all other (take n) cases only the pos? check is executed apart from the last iteration (which causes a single superfluous neg? call) The current path/impl always does 2 checks for all iterations and hence is (much) slower.

Comment by Alex Miller [ 25/Aug/15 7:22 PM ]

The only time that neg? case will be hit is if take is passed n <= 0, so I think this is correct. However, you might consider some different way to handle that particular case - for example, it could be pulled out of the transducer function entirely and be created as a separate transducer function entirely. I'm not sure that's worth doing, but it's an idea.

Comment by Karsten Schmidt [ 26/Aug/15 6:01 AM ]

Good idea, Alex! This 2nd patch removes the neg? check and adds a quick bail transducer for cases where n <= 0. It also made it slightly faster still:

(quick-bench (into [] (take 1000) (range 2000)))
Evaluation count : 20370 in 6 samples of 3395 calls.
             Execution time mean : 30.302673 µs

(now ~35% faster than original)





[CLJ-1803] Enable destructuring of sequency maps Created: 22/Aug/15  Updated: 26/Aug/15

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

Type: Enhancement Priority: Minor
Reporter: Reid McKenzie Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: destructuring

Attachments: Text File 0001-Enable-destructuring-of-seq-map-types.patch    
Patch: Code
Approval: Triaged

 Description   

At present, Clojure's destructuring implementation will create a hash-map from any encountered value satisfying clojure.core/seq? This has the I argue undesirable side effect of making it impossible to employ destructuring on a custom Associative type which is also a Seq. This came up when trying to destructure instances of a tagged value class which for the purpose of pattern matching behave as [k v] seqs, but since the v is known to be a map, are also associative on the map part so as to avoid the syntactic overhead of updates preserving the tag.

;; A sketch of such a type
(deftype ATaggedVal [t v]
  clojure.lang.Indexed
  (nth [self i]    (nth self i nil))
  (nth [self i o] (case i (0) t (1) v o))

  clojure.lang.Sequential
  clojure.lang.ISeq
  (next  [this]     (seq this))
  (first [this]     t)
  (more  [this]     (.more (seq this)))
  (count [this]     2)
  (equiv [this obj] (= (seq this) obj))
  (seq   [self]     (cons t (cons v nil)))

  clojure.lang.Associative
  (entryAt [self key] (.entryAt v key))
  (assoc [_ sk sv]    (ATaggedVal. t (.assoc v sk sv)))

  clojure.lang.ILookup
  (valAt [self k]   (.valAt v k))
  (valAt [self k o] (.valAt v k o))

  clojure.lang.IPersistentMap
  (assocEx [_ sk sv] (ATaggedVal. t (.assocEx v sk sv)))
  (without [_ sk]    (ATaggedVal. t (.without v sk))))

So using such a thing,

(let [{:keys [x]} (ATaggedVal. :foo {:x 3 :y 4})] x)
;; expect 3
=> nil

Since for any type T such that clojure.core/get will behave, T should satisfy clojure.core/map? it should be correct simply to change the behavior of destructure to only build a hash-map if map? isn't already satisfied.

The attached patch makes this change.



 Comments   
Comment by Alex Miller [ 22/Aug/15 1:21 PM ]

Probably worth watching CLJ-1778 too which might cause this not to apply anymore.

Could you add an example of what doesn't work to the description?

Comment by Ragnar Dahlen [ 24/Aug/15 1:20 AM ]

The current patch for CLJ-1778 does not address this issue.

The idea seems sound to me, if we're map destructuring a value that's
already a map (as determined by map?), we don't need to create a
new one by calling seq and HashMap/create, unless there's a
really good reason it should be exactly that map implementation (I
don't see one).

I don't think the current patch is OK though as it makes an
(unneccessary) breaking change to the behaviour of map destructuring.
Previously, destructuring a non-seqable value returned nil, but with
patch, seq is always called on value and for non-seqble types this
will instead throw an exception. It should be trivial to change the
patch to retain the original behaviour.

1.8.0-master:

user=> (let [{:keys [x]} (java.util.Date.)] x)
nil

with 0001-Enable-destructuring-of-seq-map-types.patch:

user=> (let [{:keys [x]} (java.util.Date.)] x)
IllegalArgumentException Don't know how to create ISeq from: java.util.Date  clojure.lang.RT.seqFrom (RT.java:528)
Comment by Reid McKenzie [ 26/Aug/15 1:15 PM ]

I contend that the behavior broken is, at best, undefined behavior consequent from the implementation and that the failure to cast exception is at least clearer than the silent nil behavior of the original implementation.

I would personally prefer to extend the destructuring checks logically to (cond map? x seq? (hash-map (seq x)) :else (throw "Failed to destructure non-map type") but I think that change is sufficiently large that it would meaningfully decrease the chances of this patch being accepted.





[CLJ-1802] StackTraceElement's FileName null Created: 21/Aug/15  Updated: 21/Aug/15  Resolved: 21/Aug/15

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

Type: Defect Priority: Critical
Reporter: Amir Assignee: Unassigned
Resolution: Not Reproducible Votes: 0
Labels: None
Environment:

linux



 Description   

I tried DBAppender from logback but after logging 16-20 events no event was being logged in Database, when i debuged the app i came to know that FileName from StackTraceElement is null which is raising the following exception

java.sql.SQLIntegrityConstraintViolationException: ORA-01400: cannot insert NULL into ("XYZ"."LOGGING_EVENT"."CALLER_FILENAME")

I made a simple application in java and issue was not present there.

The DBAppender of lobback fills information about log event in method bindCallerDataWithPreparedStatement.

i suspect clojure is not passing the information completely sometimes. in this case when i call the following

(dotimes[x 100]
(logr/infor "abcd" x))

please keep in mind it happens when using a marker

<appender name="REPORTING-DB" class="ch.qos.logback.classic.db.DBAppender">
<filter class="ch.qos.logback.core.filter.EvaluatorFilter">
<evaluator class="ch.qos.logback.classic.boolex.OnMarkerEvaluator">
<marker>DB_REPORT</marker>
</evaluator>
<onMismatch>DENY</onMismatch>
<onMatch>ACCEPT</onMatch>
</filter>

<connectionSource class="ch.qos.logback.core.db.DataSourceConnectionSource">
<dataSource class="com.zaxxer.hikari.HikariDataSource">
<!-- <dataSource class="com.mchange.v2.c3p0.ComboPooledDataSource"> -->
<driverClassName>oracle.jdbc.driver.OracleDriver</driverClassName>
<jdbcUrl>jdbc:oracle:thin:xxxx/xxxx@xdomain:1521:EDB
</jdbcUrl>
<user>xxxx</user>
<password>xxxx</password>
</dataSource>
</connectionSource>
</appender>

<appender name="FILE" class="ch.qos.logback.core.FileAppender">
<file>test.log</file>
<append>true</append>
<encoder>
<pattern>%-4relative [%thread] %-5level %logger{35} - %msg%n
</pattern>
</encoder>
</appender>

<root level="INFO">
<appender-ref ref="REPORTING-DB" />
<appender-ref ref="FILE" />
</root>



 Comments   
Comment by Amir [ 21/Aug/15 5:27 AM ]

(defmacro infor [ & args ]
`(let logger# (impl/get-logger logging/*logger-factory* ~*ns*)
(println (class logger#))
(when (impl/enabled? logger# :info)
(.info logger# DB_REPORT (print-str ~@args)))))

Comment by Alex Miller [ 21/Aug/15 7:15 AM ]

First, there is not enough info here to reproduce the problem apart from the logging framework and oracle db.

Second, StackTraceElement's getFileName() is allowed to return null, so this seems like a faulty assumption in the database.

If there is a specific example (with code to reproduce) where a stack trace is missing a file name, but one could have been provided, then this could be reopened.





[CLJ-1801] Boxed Boolean values break `if` special form Created: 13/Aug/15  Updated: 13/Aug/15  Resolved: 13/Aug/15

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

Type: Defect Priority: Critical
Reporter: Adrian Medina Assignee: Unassigned
Resolution: Declined Votes: 1
Labels: None


 Description   

We came across this in production code where at some point a value in a map (e.g., {:some-key false}) was being used as the test form of a conditional statement which was evaluating counter-intuitively. After much bewilderment, we figured out that it was because somewhere along the line the literal false value was being boxed. You can reproduce this by evaluating the following form in a REPL:

(if (Boolean. false)
true
false)

=> true



 Comments   
Comment by Ghadi Shayban [ 13/Aug/15 3:05 PM ]

Not a bug in Clojure. See [1] Make sure the library you are using does the proper canonicalization of boolean [2].

[1] http://clojure.org/special_forms#toc2

[2] https://github.com/ghadishayban/squee/blob/master/src/squee/impl/resultset.clj#L66-L69

Comment by Alex Miller [ 13/Aug/15 3:08 PM ]

Long ago, it was decided that only the canonical Boolean/FALSE value would be considered false in Clojure and there are no plans to change this.

Comment by Adrian Medina [ 13/Aug/15 3:15 PM ]

This is really not considered a bug? I didn't mean to imply we were using boxed booleans purposefully - in fact we weren't (the map in question get assoc'd a literal false value), and I had no idea the boxing was occurring until I dug deeper into the bug. The printed representation of a boxed boolean false value is false making debugging this issue very difficult. Perhaps the printed representation of a boxed boolean value should be changed?

Comment by Nicola Mometto [ 13/Aug/15 5:42 PM ]

Boolean/FALSE and Boolean/TRUE are already boxed booleans so your code must be constructing a new boxed boolean and using that.





[CLJ-1800] Doc that lazy-seq with-meta forces realization Created: 13/Aug/15  Updated: 19/Aug/15

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

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

Attachments: Text File CLJ-1800-no-realize-v1.patch     Text File CLJ-1800-v2.patch    
Approval: Triaged

 Description   

Applying meta to a lazy-seq causes realization:

(def x (vary-meta (lazy-seq (prn :realized)) assoc :foo :bar))
:realized

This might be surprising, so modify docstring of lazy-seq to mention it.

Patch:



 Comments   
Comment by Alex Miller [ 13/Aug/15 9:02 AM ]

I think it's likely that seq() is called here so that the old LazySeq instance and the new one share the sequence. Otherwise the pre-meta and post-meta versions would be performing the same function calls on the same inputs but would be disconnected, which seems bad.

Comment by Alex Miller [ 13/Aug/15 9:03 AM ]

I'm not really sure where this would be documented. Maybe on the http://clojure.org/metadata page?

Comment by Max Penet [ 13/Aug/15 9:18 AM ]

That would make sense yes and on the docstring of lazy-seq as well.

Comment by Alex Miller [ 13/Aug/15 9:47 AM ]

I added a sentence to the metadata page and updated the description to be more applicable here to a docstring change.

Comment by Michael Blume [ 13/Aug/15 1:29 PM ]

With this patch, with-meta doesn't realize the seq, but realization still only happens once – would this be an acceptable approach?

Comment by Michael Blume [ 19/Aug/15 4:46 PM ]

Added test





[CLJ-1799] Replace refs in pprint Created: 13/Aug/15  Updated: 14/Aug/15

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

Type: Enhancement Priority: Minor
Reporter: Alex Miller Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: performance, print

Attachments: Text File CLJ-1799.patch    

 Description   

I noticed that pprint uses refs and dosync transactions in a number of places, which seems unlikely to be necessary. It seems like these could be replaced by atoms, or even volatiles, given that printing typically happens in a single thread. Presumably this would improve performance of pprint significantly.



 Comments   
Comment by dennis zhuang [ 14/Aug/15 11:28 AM ]

I develop a patch to fix this issue.I run all the tests in clojure and clojure.data.json, and no one fails.

Use criterium to do a simple benchmark as below:

(use 'criterium.core)
(require '[clojure.data.json :as json])
(bench (json/write-str 
  {:a 1 :b 2 :c (range 10) :d "hello world"
   :e (apply hash-set (range 10))}))

before patch:

Evaluation count : 6180060 in 60 samples of 103001 calls.
             Execution time mean : 10.302604 µs
    Execution time std-deviation : 597.958933 ns
   Execution time lower quantile : 9.631444 µs ( 2.5%)
   Execution time upper quantile : 11.618551 µs (97.5%)
                   Overhead used : 1.724553 ns

After patch:

Evaluation count : 6000900 in 60 samples of 100015 calls.
             Execution time mean : 10.212543 µs
    Execution time std-deviation : 564.874941 ns
   Execution time lower quantile : 9.528383 µs ( 2.5%)
   Execution time upper quantile : 11.334033 µs (97.5%)
                   Overhead used : 1.827143 ns




[CLJ-1798] The RetryEx in LockingTransaction should be static Created: 13/Aug/15  Updated: 13/Aug/15

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

Type: Enhancement Priority: Minor
Reporter: dennis zhuang Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: STM, performance
Environment:

java version "1.7.0_17"
Java(TM) SE Runtime Environment (build 1.7.0_17-b02)
Java HotSpot(TM) 64-Bit Server VM (build 23.7-b01, mixed mode)

Mac OSX 10.10.4


Attachments: PNG File profile.png     Text File static_retryex.patch    
Patch: Code
Approval: Triaged

 Description   

We are using clojure.data.json, and we profiled our project with jprofiler, it shows that there is a hotspot in LockingTransaction.Too many RetryEx instances were created during the profiling.
The retryex instance variable should be static as a class variable to avoid creating when a new STM transaction begins.

The attacments are the profile result screen snapshot and the patch to make the retryex to be static.
I don't do any benchmark right now,but maybe a clojure.data.json performance benchmark will approve the patch works better.



 Comments   
Comment by Alex Miller [ 13/Aug/15 8:04 AM ]

I think the suggestion here is sound, but I have a hard time believing it will make much of a difference. My real question is why pprint is using dosync.

Comment by dennis zhuang [ 13/Aug/15 9:03 AM ]

I found it uses many dosync in writer as below:

https://github.com/clojure/clojure/blob/master/src/clj/clojure/pprint/pretty_writer.clj

It seems that using the dosync to change some mutable states.Maybe they can be rewrote into other forms such as atom,java object/lock etc.
But it's another story.





[CLJ-1797] Mention cljc when require fails Created: 10/Aug/15  Updated: 20/Jan/16

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

Type: Enhancement Priority: Minor
Reporter: Mike Fikes Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: errormsgs

Attachments: Text File clj-1797.patch    
Patch: Code
Approval: Prescreened

 Description   

If you attempt to require a namespace for which the code cannot be located, now that cljc files are considered when locating code, it could be mentioned in the list of files considered on the classpath.

$ java -jar clojure-1.7.0.jar 
Clojure 1.7.0
user=> (require 'foo.bar)
FileNotFoundException Could not locate foo/bar__init.class or foo/bar.clj on classpath.  clojure.lang.RT.load (RT.java:449)

Note: FWIW, ClojureScript has similar error messages, and the order listed in the error message is the order tried when loading. If this were followed, I suspect the text of the error message above would end up looking like:

Could not locate foo/bar__init.class, foo/bar.clj, or foo/bar.cljc on classpath.

Patch: clj-1797.patch
Approach: add info about cljcfile not found in exception thrown from load
Screened by: Alex Miller



 Comments   
Comment by Cezary Kosko [ 18/Jan/16 6:21 AM ]

Hey,

I'm a newbie, so I thought I'd do that one (even though it's not vetted yet, but seems it 's bound to be), to get a tad more familiar w/ the sources.

Kind regards,
Cezary

Comment by Alex Miller [ 18/Jan/16 6:53 AM ]

Go for it!





[CLJ-1796] Protocol functions fail to find future extensions when assigned to a local or new var Created: 08/Aug/15  Updated: 10/Aug/15

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

Type: Defect Priority: Minor
Reporter: Nathan Marz Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: protocols

Approval: Triaged

 Description   
(defprotocol TestProtocol
  (tester [o]))

(let [t tester]
  (defn another-tester [o]
  	(t o)))

(def another-tester2 tester)

(extend-protocol TestProtocol
  String
  (tester [o] (println "Strings work!")))

(another-tester "A") ;; Error
(another-tester2 "A") ;; Error
(tester "A") ;; Works fine

(let [t tester]
  (defn another-tester [o]
  	(t o)))

(another-tester "A") ;; Works fine

(def another-tester2 tester)

(another-tester2 "A") ;; Works fine

(extend-protocol TestProtocol
  Long
  (tester [o] (println "Longs work!")))

(another-tester "A") ;; Works fine
(another-tester 3) ;; Error
(another-tester2 3) ;; Error


 Comments   
Comment by Nathan Marz [ 08/Aug/15 12:47 PM ]

This issue appears to be Clojure specific – I did some testing in CLJS and was unable to reproduce the issue.

Comment by Ghadi Shayban [ 09/Aug/15 9:51 AM ]

Nathan,
Not sure if you tried this, but using:

(def another-handle #'the-protocol-function)
rather than dereffing outright.

Comment by Nathan Marz [ 09/Aug/15 6:25 PM ]

That's a good workaround but it does seem that my test case should work. I ran into this because I was passing around functions dynamically and saving them for later execution – and this issue popped up with protocol methods. Having to pass around protocol methods differently than regular functions doesn't seem right.

Comment by Kevin Downey [ 10/Aug/15 11:21 AM ]

this is a result of the protocol implementation in clojure, protocol extension mutates the vars, once you have taken then value of the var (which happens once for top level forms) you will not see further mutations of the var so no more protocol extension





[CLJ-1795] Protocol functions don't work properly when metadata is added to them Created: 08/Aug/15  Updated: 10/Aug/15  Resolved: 10/Aug/15

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

Type: Defect Priority: Major
Reporter: Nathan Marz Assignee: Unassigned
Resolution: Duplicate Votes: 0
Labels: metadata, protocols
Environment:

Clojure 1.7.0



 Description   

When you add metadata to a protocol function, the version with metadata will not work for any extensions added afterwards.

(defprotocol TestProtocol
  (tester [o]))

(def tester-with-meta (with-meta tester {:a 1}))

(extend-protocol TestProtocol
  String
  (tester [o] (println "Strings work!")))

(tester-with-meta "A") ;; Error
(tester "A") ;; Works fine

(def tester-with-meta (with-meta tester {:a 1}))

(extend-protocol TestProtocol
  Long
  (tester [o] (println "Longs work!")))

(tester-with-meta "A") ;; Works fine
(tester-with-meta 3) ;; Error


 Comments   
Comment by Alex Miller [ 08/Aug/15 9:16 AM ]

Can you specify version you're testing with too...

Comment by Nathan Marz [ 08/Aug/15 9:21 AM ]

Clojure 1.7.0

Comment by Nathan Marz [ 08/Aug/15 10:53 AM ]

This is subsumed by http://dev.clojure.org/jira/browse/CLJ-1796 which seems to be closer to the root cause

Comment by Alex Miller [ 10/Aug/15 9:10 AM ]

Subsumed by CLJ-1796





[CLJ-1794] Sorting vector yields non-indexed ArraySeq Created: 05/Aug/15  Updated: 10/Aug/15

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

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

Attachments: Text File 0001-CLJ-1794-Make-ArraySeqs-implement-Indexed.patch    
Approval: Triaged

 Description   

Sorting a vector gives back an ArraySeq with O(n) gets instead of O(log N) gets. This means it can be more efficient to take a vector, sort, then turn it back into a vector.

Cause: sort works by copying the collection to be sorted into an array, calls Arrays/sort to sort it, and then returns a seq on the sorted array. The seq returned is an ArraySeq and doesn't implement Indexed.

Alternatives:

1. Make ArraySeq (and primitive specializations thereof) implement Indexed, providing constant time lookup by index.
2. Specialize sorting for different collection types
3. ???



 Comments   
Comment by Ragnar Dahlen [ 06/Aug/15 2:28 AM ]

Update description, attach patch.

Comment by Ragnar Dahlen [ 06/Aug/15 2:31 AM ]

Added link to current patch.

Comment by Alex Miller [ 06/Aug/15 6:50 AM ]

Another alternative to consider here is to have sort do something smarter.

Comment by Ragnar Dahlen [ 06/Aug/15 7:44 AM ]

Having thought a bit more about the approach and implications of this I'm not sure this patch is a good idea at all. It makes a little bit sense for the particular case of sorting a vector, but on the other hand sort only promises to return a sorted sequence of given coll. Implementing Indexed for a sequence type just because the underlying data structure supports efficient lookup by index feels wrong. Like you suggest, effort is maybe better spent thinking about making sort smarter, which is a different issue, or just using sorted collections instead.

Comment by Kevin Downey [ 06/Aug/15 12:49 PM ]

It seems like the best thing here would be to change sort to return a vector. Usages of sort in the middle of sequence pipelines will continue to work, but a sort followed by conj will break (I cannot recall an instance of this off hand, but I am sure they exist). Sorting seems to imply a fully realized collection, and vectors are the "strongest" realized collections that can be returned here.

Given the conservative nature of core, and the issue with conj ordering above, the next best thing might be to add a sortv similar to the existing mapv.

Another option might be to remove the call to seq, so sort returns the sorted array. This would actually be really useful because you can use Arrays.binarySearch. Calls to conj after a sort would then fail with an exception instead of conj to the "wrong" place.





[CLJ-1793] Clear 'this' before calls in tail position Created: 05/Aug/15  Updated: 24/Aug/15

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

Type: Defect Priority: Critical
Reporter: Alex Miller Assignee: Alex Miller
Resolution: Unresolved Votes: 0
Labels: compiler
Environment:

1.8.0-alpha2 - 1.8.0-alpha4


Attachments: Text File 0001-Clear-this-before-calls-in-tail-position.patch     Text File clj-1793-2.patch    
Patch: Code and Test
Approval: Vetted

 Description   

(This ticket started life as CLJ-1250, was committed in 1.8.0-alpha2, pulled out after alpha4, and this is the new version that fixes the logic about whether in a tail call as well as addresses direct linking added in 1.8.0-alpha3.)

Problem: Original example was with reducers holding onto the head of a lazy seq:

(time (reduce + 0 (map identity (range 1e8))))    ;; works
(time (reduce + 0 (r/map identity (range 1e8))))  ;; oome from holding head of range

Example of regression from CLJ-1250 that now works correctly (doesn't clear this in nested loop) with CLJ-1793:

(let [done (atom false)
        f (future-call
            (fn inner []
              (while (not @done)
                (loop [found []]
                  (println (conj found 1))))))]
    (doseq [elem [:a :b :c :done]]
      (println "queue write " elem))
    (reset! done true)
    @f)

Approach: When invoking a method in a tail call, clear this prior to invoking.

The criteria for when a tail call is a safe point to clear 'this':

1) Must be in return position
2) Not in a try block (might need 'this' during catch/finally)
3) When not direct linked

Return position (#1) isn't simply (context == C.RETURN) because loop bodies are always parsed in C.RETURN context

A new dynvar METHOD_RETURN_CONTEXT tracks whether an InvokeExpr in tail position can directly leave the body of the compiled java method. It is set to RT.T in the outermost parsing of a method body and invalidated (set to null) when a loop body is being parsed where the context for the loop expression is not RETURN parsed. Added clear in StaticInvokeExpr as that is now a thing with direct linking again.

Removes calls to emitClearLocals(), which were a no-op.

Patch: clj-1793-2.patch

Screened by: Alex Miller



 Comments   
Comment by Alex Miller [ 05/Aug/15 12:16 PM ]

The this ref is cleared prior to the println, but the next time through the while loop it needs the this ref to look up the closed over done field (via getfield).

Adding an additional check to the inTailCall() method to not include tail call in a loop addresses this case:

static boolean inTailCall(C context) {
-    return (context == C.RETURN) && (IN_TRY_BLOCK.deref() == null);
+    return (context == C.RETURN) && (IN_TRY_BLOCK.deref() == null) && (LOOP_LOCALS.deref() == null);
}

But want to check some more things before concluding that's all that's needed.

Comment by Alex Miller [ 05/Aug/15 1:36 PM ]

This change undoes the desired behavior in the original CLJ-1250 (new tests don't pass). For now, we are reverting the CLJ-1250 patch in master.

Comment by Ghadi Shayban [ 05/Aug/15 3:12 PM ]

Loop exit edges are erroneously being identified as places to clear 'this'. Only exits in the function itself or the outermost loop are safe places to clear.

Comment by Ghadi Shayban [ 05/Aug/15 8:43 PM ]

Patch addresses this bug and the regression in CLJ-1250.

See the commit message for an extensive-ish comment.

Comment by Alex Miller [ 18/Aug/15 12:33 PM ]

New patch is same as old, just adds jira id to beginning of commit message.

Comment by Rich Hickey [ 24/Aug/15 10:00 AM ]

Not doing this for 1.8, more thought needs to go into whether this is the right solution to the problem. And, what is the problem? This title of this patch is just something to do.

Comment by Alex Miller [ 24/Aug/15 10:21 AM ]

changing to vetted so this is at a valid place in the jira workflow

Comment by Ghadi Shayban [ 24/Aug/15 10:45 AM ]

Rich the original context is in CLJ-1250 which was a defect/problem. It was merged and revert because of a problem in the impl. This ticket is the continuation of the previous one, but unfortunately the title lost the context and became approach-oriented and not problem-oriented. Blame Alex. (I kid, it's an artifact of the mutable approach to issue management.)





[CLJ-1792] array-map and hash-map differ in handling of keys comparing identical but not equal Created: 04/Aug/15  Updated: 04/Aug/15

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-do-pointer-check-in-Util.EquivPred.patch    
Patch: Code

 Description   
user=> (let [a (array-map Double/NaN 1)] (assoc a (key (first a)) "foo"))
{NaN 1, NaN "foo"}
user=> (let [a (hash-map Double/NaN 1)] (assoc a (key (first a)) "foo"))
{NaN "foo"}

Approach: Array-map's comparison skips identity-check and always delegates to .equals calls. The attached patch alignes array-map behaviour with hash-map by doing the appropriate pointer checks before delegating to .equals






[CLJ-1791] Issue defining a defrecord protocol method named "clear" Created: 04/Aug/15  Updated: 08/Aug/15

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

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


 Description   

There seems to be a problem in trying to define a protocol with a method named "clear"

(defprotocol PClear
(clear [o]))
=> PClear

(defrecord Foo []
PClear
(clear [o] o))
=> CompilerException java.lang.ClassFormatError: Duplicate method name&signature in class file xxxx/Foo, compiling:(NO_SOURCE_PATH:1:1)

I assume this is due to a name conflict with the Java method Collection.clear() in the underlying implementation. However the error is very unclear about this, and the potential for conflict appears to be undocumented as far as I can see.

There seem to be two possible approaches to fixing this:
a) Disallow the use of "clear" as a protocol method name (in which case the error should be more informative, and the rule should be documented)
b) Find a way to support this in the class file format (possibly by overloading on JVM return types, since Collection.clear() returns void??)



 Comments   
Comment by Nicola Mometto [ 04/Aug/15 6:58 AM ]

Mike, the jvm doesn't support return type overloading so your second suggestion is not technically possible.

Reading the doc for defrecord " The class will have implementations of several (clojure.lang)
interfaces generated automatically: IObj (metadata support) and
IPersistentMap, and all of their superinterfaces.
"

Perharps java.util.Collection (or even better, java.util.Map) should be mentioned here.

Comment by Alex Miller [ 04/Aug/15 7:46 AM ]

I think this should be a doc enhancement request.

Comment by OHTA Shogo [ 08/Aug/15 3:42 AM ]

It might be out of the scope of this ticket, but protocol method conflicts can cause some other kinds of errors:

user=> (defprotocol P1 (finalize [this]))
P1
user=> (defrecord R1 [] P1 (finalize [this]))

CompilerException java.lang.VerifyError: (class: user/R1, method: finalize signature: ()Ljava/lang/Object;) Unable to pop operand off an empty stack, compiling: ...
user=> (defprotocol P2 (wait [this]))
P2
user=> (defrecord R2 [] P2 (wait [this]))
user.R2
user=> (def r (->R2))
#'user/r
user=> (wait r)
CompilerException java.lang.IllegalArgumentException: No single method: wait of interface: user.P2 found for function: wait of protocol: P2, compiling: ...
user=>

IMHO it would be nicer if defprotocol would warn method conflicts with a more informative message.





[CLJ-1790] Error extending protocols to Java arrays Created: 29/Jul/15  Updated: 26/Jan/16

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

Type: Defect Priority: Major
Reporter: Mike Anderson Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: compiler, protocols

Attachments: Text File 0001-CLJ-1790-emit-a-cast-to-the-interface-during-procol-.patch    
Patch: Code
Approval: Triaged

 Description   

Root cause appears to be related to how protocols are being handled when used with Java arrays:

e.g. for the protocol implementation:

(extend-protocol mp/PImplementation
  (Class/forName "[Ljava.lang.Object;")
    (implementation-key [m] :object-array)
    (meta-info [m]
      {:doc "Clojure.core.matrix implementation for Java Object arrays"})
    (new-vector [m length] (construct-object-vector (long length)))
    (new-matrix [m rows columns]
      (let [columns (long columns)
            m (object-array rows)]
        (dotimes [i rows]
          (aset m i (construct-object-vector columns)))
        m))
    (new-matrix-nd [m shape]
      (construct-nd shape))
    (construct-matrix [m data]
      (construct-object-array data))
    (supports-dimensionality? [m dims]
      (>= dims 1)))

When called as:

(clojure.core.matrix.protocols/construct-matrix (object-array 1) [1])

Gives exception:

VerifyError (class: clojure/core/matrix$eval10586, method: invokeStatic signature: ()Ljava/lang/Object;) Incompatible object argument for function call  java.lang.Class.getDeclaredConstructors0 (:-2)

Also see: CLJ-1381

Patch: 0001-CLJ-1790emit-a-cast-to-the-interface-during-procol.patch



 Comments   
Comment by Nicola Mometto [ 06/Nov/15 3:53 PM ]

Mike Anderson does 1.8.0-beta2 fix this issue?
Alex Miller if core.matrix is still affected this must be fixed before 1.8.0 as it'd mean that direct linking is still broken

Comment by Nicola Mometto [ 06/Nov/15 6:26 PM ]

I could reproduce the bug with 1.8.0-beta2 btw

Comment by Nicola Mometto [ 06/Nov/15 7:27 PM ]

Apparently this is not a 1.8 regression.

At least 1.6 and 1.7 both manifest the same issue:

Clojure 1.6.0
user=> (defprotocol p (f [_]))
p
user=> (fn [] (f (object-array [])))

VerifyError (class: user$eval15920$fn__15921, method: invoke signature: ()Ljava/lang/Object;) Incompatible object argument for function call  user/eval15920 (form-init9183379085801704163.clj:1)

Comment by Michael Blume [ 06/Nov/15 8:24 PM ]

Do we know why core.matrix works with Clojure 1.6/1.7 then?

Comment by Nicola Mometto [ 06/Nov/15 9:09 PM ]

It doesn't.

Clojure 1.7.0
Java HotSpot(TM) 64-Bit Server VM 1.8.0_60-b27

user=> (require 'clojure.core.matrix.protocols)
nil
user=> (clojure.core.matrix.protocols/construct-matrix (object-array 1) [1])

VerifyError (class: user$eval6935, method: invoke signature: ()Ljava/lang/Object;) Incompatible object argument for function call  java.lang.Class.getDeclaredConstructors0 (Class.java:-2)
user=>

I attached a patch that fixes this issue.
It's caused by the jvm verifier understanding that the object on the stack is an array and thus can never be an instance of the protcol interface, but not being able to understant that the code path leading to the direct protocol interface method invocation can never be reached because of a branch guided by an instance check for that interface on the target

Comment by Mike Anderson [ 06/Nov/15 10:10 PM ]

Apologies, it is possible I just hadn't tested this code path thoroughly before.

It only seems to get triggered in certain circumstances, the following behaviour is interesting:

=> (let [o (identity (object-array 1))]
     (clojure.core.matrix.protocols/dimensionality o))
1
=> (let [o (object-array 1)]
     (clojure.core.matrix.protocols/dimensionality o))
VerifyError (class: clojure/core/matrix$eval17775, method: invokeStatic signature: ()Ljava/lang/Object;) Incompatible object argument for function call  java.lang.Class.getDeclaredConstructors0 (:-2)

Perhaps it only happens when the callsite has type information about the protocol parameter?

Comment by Nicola Mometto [ 07/Nov/15 4:53 AM ]

Correct, apparently the jvm verifier doesn't like situations where we have an array on the stack typed as such, and on a later codepath it is used as target for an invokeinterface even if that path is unreachable because of a previous instance check.

here's an explaination of exactly our case in pseudo bytecode:

..
 load obj // Object[]
 dup
 instanceof SomeInterface
 iftruejmp label1
 pop
 jmp end
label1:
 // here is where the verifier chokes.
 // it can figure out that the target is of type Object[] which can never be a SomeInterface
 // but it cannot figure out that this code path can never be reached because of the previous
 // instance check with jump
 // to fix this we need to insert an explicit checkcast to SomeInterface on the target
 invokeinterface SomeInterface/someMethod
end:
 return




[CLJ-1789] Use transients with select-keys if possible Created: 28/Jul/15  Updated: 28/Jul/15

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

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


 Description   

Currently select-keys uses conj to add entries. If the map is editable, conj! could be used instead to improve select-keys performance.

Additionally keyseq is traversed as a seq but could be traversed via reduce instead, which might be faster.






[CLJ-1788] Integrate lein with the clojure distribution. Created: 27/Jul/15  Updated: 27/Jul/15  Resolved: 27/Jul/15

Status: Closed
Project: Clojure
Component/s: None
Affects Version/s: Release 1.8
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Morten Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: enhancement, newbie
Environment:

All



 Description   

As a new clojure user I think clojure would be more approachable by beginners and less confusing to get started with if "lein" could get included in the standard clojure distribtion.

It seems almost all tools, books and clojure users use "lein" anyway so why make it a seperate download? It may seem as a trivial issue for existing power clojure developers but offering a integrated clojure distribution with a official package manager etc. would make it easier for new users to get started. After all clojure is not the simplest thing to get started with and "low hanging fruits" like this could help a lot. For tool vendors there would also be a benefit if "lein" was included.



 Comments   
Comment by Alex Miller [ 27/Jul/15 1:05 PM ]

This is not something we're going to do.

Comment by Morten [ 27/Jul/15 1:23 PM ]

I am new so I don't know how things are decided here but can I politely ask why? As a new user I can firmly say it would have helped me (who did not know about lein when I first got started) and is helping get new users easily started not a worthwhile goal (especially if it is easily done) ?

Comment by Andy Fingerhut [ 27/Jul/15 1:38 PM ]

Morten, I can't comment officially on what changes will or will not be made to Clojure.

I did want to point out that if you get Leiningen, you don't need to do a separate manual download of Clojure in addition to that. Running Leiningen the first time auto-downloads it and a few other things for you.

Leiningen is not the only way to run Clojure, so the Clojure getting started page here: http://clojure.org/getting_started mentions Leiningen as one possible approach, but it does not happen to mention that if you choose that approach, downloading the JAR as described at the top of that page is unnecessary. Might be nice if that page mentioned that fact about Leiningen.

Comment by Morten [ 27/Jul/15 1:55 PM ]

I downloaded clojure first and only when I found out that my IDE insisted on a project file that belongs to "leiningen" did I find and install lein.

As for leining just being one approach I did read on InfoQ that according to a recent survey "Leiningen, at a whopping 98% adoption rate," (among Clojure users I assume).

So the 98% users seems to make lein a defacto standard and the fact that the IDE's I have tried require leining to work with clojure make another compelling argument.

But anyway - it is up to the clojure dev community to decide. I am just a new users and just wondered at the very quick and prompt No without any readon why?

Comment by Alex Miller [ 27/Jul/15 2:03 PM ]

Leiningen is an external tool built with a different contribution model and set of goals than Clojure itself. It's also not the only build tool in use in the Clojure community. As such, there are no plans to bundle Leiningen into Clojure.





[CLJ-1787] Add executables to distribution (suggested changes included) Created: 27/Jul/15  Updated: 27/Jul/15

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

Type: Enhancement Priority: Major
Reporter: Morten Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Linix, OS X and Windows


Patch: Code

 Description   

Being new to clojure I was surprised that there was not a executable included in the download. It makes Clojure appear less proff then it is and is unnecessary complex to remember the exact java cmd line to use so I suggest that shell executable files are added to the distibution for the next version of Clojure. They could look as suggest below for Windows and OS X:

1) Linux and OS X:
#!/bin/bash
java -jar `dirname "$0"`/clojure-1.8.0.jar $@

2) Windows:
java -jar %~dp0\clojure-1.8.0.jar %1 %2 %3 %4 %5 %6 %7 %8






[CLJ-1786] Unused defaults are evaluted in `:or` destructoring Created: 26/Jul/15  Updated: 26/Jul/15  Resolved: 26/Jul/15

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

Type: Defect Priority: Major
Reporter: Leon Grapenthin Assignee: Unassigned
Resolution: Duplicate Votes: 0
Labels: compiler


 Description   
(defn example [{:keys [foo] :or {foo (println "Evaluated!")}}])

(example {:foo 42})
;; prints "Evaluated!"


 Comments   
Comment by Alex Miller [ 26/Jul/15 8:38 AM ]

Dupe of CLJ-1676





[CLJ-1785] Reader conditionals throws when they have nil expressions Created: 21/Jul/15  Updated: 31/Jul/15  Resolved: 31/Jul/15

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

Type: Defect Priority: Critical
Reporter: Jozef Wagner Assignee: Jozef Wagner
Resolution: Completed Votes: 1
Labels: reader, readerconditionals

Attachments: Text File clj-1785.patch     Text File clj-1785-v2.patch    
Patch: Code and Test
Approval: Ok

 Description   

Reader conditional that has nil as an expression fails.

e.g. (read-string {:read-cond :allow} "#?(:default nil)")

The fact that nil values are valid expressions are documented at both official documentation and design page.

Patch: clj-1785-v2.patch

Screened by: Alex Miller



 Comments   
Comment by Jozef Wagner [ 21/Jul/15 3:53 PM ]

Added patch with tests

Comment by Jozef Wagner [ 21/Jul/15 4:06 PM ]

v2 patch that uses static final sentinel value

Comment by Nicola Mometto [ 22/Jul/15 7:23 AM ]

CLJ-1138 reports a similar bug with data readers.





[CLJ-1784] Reflector.getMethods should be cached Created: 21/Jul/15  Updated: 11/Dec/15

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

Type: Enhancement Priority: Major
Reporter: Vladimir Sitnikov Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File clj-1784.patch    

 Description   

Currently Reflector.getMethods performs expensive logic that includes java.lang.reflect.Method copying.
See: https://github.com/clojure/clojure/blob/b8607d5870202034679cda50ec390426827ff692/src/jvm/clojure/lang/Reflector.java#L373

In our application I see the following back-traces:

at Reflector.copyMethods
at Reflector.invokeInstanceMethod
at ...

These kind of backtraces are second top consumers of all the heap allocation.

JDK cannot cache Methods / Fields since they are mutable (e.g. user can call setAccessible here and there).
However, for the purposes of Clojure, I believe it should be fine to cache Methods and Fields.

What do you think?
E.g. WeakHashMap<Class, WeakReference<List<Method>>> or more sophisticated structure to account String name.



 Comments   
Comment by Alex Miller [ 23/Jul/15 8:19 AM ]

If you are seeing Reflector as a hot spot in your application, you should probably turn on warn-on-reflection and use type hints to avoid reflection.

Comment by Vladimir Sitnikov [ 28/Jul/15 6:10 AM ]

Do you mean there is absolutely no reason to use reflection in Clojure ever?
I do understand that if developer gives enough type hints the reflection would go away.

However:
1) I just do not know if it is easily doable (in other words, if it is possible at all, maintainable, etc)
2) I'm not sure if "always use type hints" is considered a best practice. For instance, warn-on-reflection documentation page says nothing like "always use type hints"
3) Caching copyMethods seems to be a low-hanging fruit here, so it would shave cpu cycles for those who omitted type hints

PS. I'm a java performance engineer, not a Clojure engineer (as in "my Clojure knowledge is somewhere near (+ x y)"), so I kindly beg on your forgiveness for me not doing RTFM.

Comment by Alex Miller [ 28/Jul/15 7:44 AM ]

No, I'm saying that if reflection is a hotspot in your application, usually it's worth investing a few minutes to add type hints in those hotspot areas and this is common advice for Clojure apps. Once that minimal work is done, few Clojure apps are bound by reflection.

Caching seems like an easy solution until you consider all of the management aspects. How does the cache get cleaned? Are the instances mutable and able to be reused? Are there cases where class loaders or code reloading create unexpected side effects? What are the concurrency effects of putting a shared resource in the invocation path? What is the memory impact of a cache and is it configurable?

Those are all things that would need to be investigated, meaning that this is not low-hanging fruit.

Comment by Mike Anderson [ 08/Dec/15 8:39 PM ]

Patch for simple caching of Reflector.getMethods calls for small arities

Comment by Mike Anderson [ 08/Dec/15 8:46 PM ]

I created a small patch to add very simple (fixed size, 1 element for each arity) caching for Reflector.getMethods calls. The aim is to keep this super simple to avoid issues like concurrency effects and having a variable-sized cache.

This helps a small amount in my tests (about 15-20%) on reflection calling the same method in a loop, which is probably the common case where people actually care about reflection performance.

Performance could certainly be improved further due to the fact that I think most of the overhead is actually is the `invokeMatchingMethod`, but that is an orthogonal issue. This patch opens the way for further performance optimisation in that area.

;; clojure 1.8.0-RC3
user=> (let [v (identity 1)] (time (dotimes [i 1000] (.doubleValue v))))
"Elapsed time: 1.598779 msecs"

;; with cached arities
user=> (let [v (identity 1)] (time (dotimes [i 1000] (.doubleValue v))))
"Elapsed time: 1.359888 msecs"

Comment by Vladimir Sitnikov [ 09/Dec/15 2:24 AM ]

Mike Anderson, I wonder if your patch results in a performance regression for concurrent workload.

You've created a single point of contention as lots of threads will try to update private static InstanceMethodCache[] instanceMethodCache entries, so it will hit both "true sharing" and "false sharing" problems.

Should instanceMethodCache be final and written in capital letters?

Comment by Alex Miller [ 09/Dec/15 10:33 AM ]

This ticket needs a better problem definition. That is: "I am doing ____" (with an example) that shows Reflector.getMethods() as the bottleneck.

If I guess at what the problem is, I remain unconvinced that this is the best solution.

A ThreadLocal is likely the cache solution with the lowest concurrency impact.

Comment by Mike Anderson [ 09/Dec/15 9:00 PM ]

This shouldn't have any noticeable concurrency impact: no locking is required for this very simple approach. Most of the time it is simply an unlocked read from an array on the heap, the Java memory model is enough to guarantee correct behaviour. That's cheaper than even a threadlocal, e.g. there's some evidence here that this is 10-20x faster: http://stackoverflow.com/questions/609826/performance-of-threadlocal-variable

At the very least, any concurrency impact is so tiny it will be dwarfed by the benefits of often avoiding the getMethods calls, which are expensive. The cost of array access is a few nanoseconds compared to the cost of getMethods which appears from the benchmark above to be a few hundred nanoseconds.

The worst concurrency case I can think of is the case where two different threads are calling getMethods on different methods at a high rate and these calls are perfectly interleaved so that they always invalidate the cache. But even in that case, it's probably not measurably worse than the current code.

@Vladimir yes, insntanceMethodCache could be final. Might help the JVM very marginally, I guess.

@Alex, I proposed this patch because it is an improvement over what is currently there, I certainly don't think it will be the "best possible solution". In the spirit of open source and making incremental progress, I'd like you to consider accepting it, even if this issue stays open for future consideration. This is also linked to clj-1866, I'm trying to make the "fast path" for reflection better in a few different ways. If you'd rather have a single large patch with a whole bunch of improvements I can certainly do that, I has under the impression that smaller, more "obvious" patches would be easier for you to review but happy either way.

Comment by Vladimir Sitnikov [ 10/Dec/15 1:18 AM ]

Mike Anderson, you are missing the point.

Please check here: http://shipilev.net/blog/2014/jmm-pragmatics/#_benchmarks, slide 77/100 "SC-DRF: Writes"

Alexey Shipilev: This reinforces the idea that data sharing is what you should avoid in the first place, not volatiles

Having ThreadLocal cache would eliminate "shared update" problem.

This ticket needs a better problem definition. That is: "I am doing ____" (with an example) that shows Reflector.getMethods() as the bottleneck

That is true.
My particular case was somehow resolved by development team.
I just thought some basic cache would make Clojure do the right thing by default and require less type specialization written manually.

Comment by Alex Miller [ 10/Dec/15 7:56 AM ]

I see a lot of "should" type statements in there. The whole point is that no change like this is going to go in until we know that there aren't impacts. But more importantly I'm not even going to mark it triaged until it's a good ticket that starts with a problem statement.

Comment by Mike Anderson [ 10/Dec/15 7:22 PM ]

Alex, what do you mean by "know there aren't impacts"? That seems an absurd position on the face of it, it is a perfectly acceptable trade-off to allow minor regressions in rare corner cases if you are improving the fast path / common case significantly.

Also, this definitely isn't a standard that is universally applied for changes to Clojure. Plenty of changes go into Clojure which cause performance regressions in other areas, you only need to look at Andy Fingerhut's excellent benchmarking efforts here to see that: https://jafingerhut.github.io/clojure-benchmarks-results/Clojure-expression-benchmark-graphs.html

Problem statement is IMHO obvious for anything performance related: "Performance of X is sub-optimal, which hurts users who are doing X." If you want a new ticket / changed description that says something like that then I'm happy to do it, but that seriously just feels like bureaucratic box ticking. Please consider this as constructive feedback on your contribution process.

What exactly (i.e. which benchmarks) do you need to see as a valid demonstration of improvement in performance-related issues like this?

Comment by Alex Miller [ 11/Dec/15 10:33 AM ]

Similar to my comments in CLJ-1866, the title of this ticket is "Reflector.getMethods should be cached". That is again a solution, not a problem. What I'm looking for is a title like "Repeated reflection in a loop is slow" and a description that starts with some example code demonstrating the problem. Without a good problem statement, I cannot triage this ticket. I may still consider the priority of the problem to be low enough that it's not worth triaging at this time - I'll withhold judgement though until the ticket is improved.

The fact that prior changes have had unexpected performance impacts only lends additional credence to my suggestion that this (performance-oriented) ticket should validate its claims. You have added code, which makes the "miss" path of this code slower than it was before. How much slower? It should make the "hit" path faster - how much faster? In typical code, how often do we encounter hit vs miss paths? My presumption is that the example will demonstrate a case where the hit path is common. These are the kinds of questions I, as a screener, must ask to evaluate any proposed solution.

Additionally, you are introducing concurrency concerns and some additional work is required to verify both correctness (the current patch has visibility issues) and that you have not introduced contention or memory issues. These are typical problems for any caching-related optimization and I could point you to any number of prior tickets that have wrestled with them.

Comment by Mike Anderson [ 11/Dec/15 9:13 PM ]

Thanks Alex for explaining your concerns.

I agree that a problem-oriented approach to patches is better, so I suggest the following:

  • We close both this issue and clj-1866
  • I create a separate problem-focused ticket for reflection performance
  • I'll benchmark the changes as whole for a number of different cases
  • You'll triage the patch on the assumption that we can demonstrate noticeable improvement in the common cases, all tests pass as before, and no major regressions occur in the corner cases (concurrent access, frequent cache misses etc.)

If you want a problem-oriented issue then I don't think it makes much sense to create separate tickets / patches for each "solution" (although some OSS projects choose to do it that way, they usually have have a much more streamlined process for minor changes / optimisations which probably doesn't suit the Clojure dev process)

Agreed?

Comment by Alex Miller [ 11/Dec/15 9:45 PM ]

As I said, we would prefer small focused tickets and patches, rather than one big patch.

I will reiterate that I'm not convinced doing any of this work makes sense if the scenario is one where a type hint would solve the problem.





[CLJ-1783] Remove unnecessary call to seq() in clojure.lang.Cons.next() Created: 21/Jul/15  Updated: 22/Jul/15  Resolved: 21/Jul/15

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

Type: Enhancement Priority: Minor
Reporter: Mike Anderson Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None

Attachments: Text File clj-1783.patch    

 Description   

Currently clojure.lang.Cons has the following implementation for next():

public ISeq next(){
	return more().seq();
}

This appears to be unnecessary and could be replaced with the following, since _more is already an ISeq

public ISeq next(){
	return _more;
}

There are minor performance gains from this, because:
a) It avoids an unnecessary null check
b) In the null case it avoids an unnecessary call to PersistentList.EMPTY.seq() (which is guaranteed to return null, although the JVM probably optimises this away)
c) In the non-null case it avoids an unnecessary interface dispatch call to ISeq.seq()

It is possible that this change affects laziness, since the value contained in _more could theoretically perform some processing opon the invocation of `seq`. However, any such change should be an improvement since it increases rather than reduces laziness.



 Comments   
Comment by Jozef Wagner [ 21/Jul/15 5:46 AM ]

Have all tests passed? IMO your patch would break (assert (nil? (next (cons 1 (lazy-seq nil))))).

next must call seq(), because it promises to return nil for empty Seqs.

Comment by Mike Anderson [ 21/Jul/15 6:00 AM ]

Hmmmm that is an interesting point Jozef. I hadn't realised that there are valid cases where ISeq instances are allowed to be both non-nil and empty.

Will have to give more thought to see if these cases can be handled.

Comment by Mike Anderson [ 21/Jul/15 10:21 PM ]

Closing because I don't think this can be resolved without more invasive changes.

For future reference, it seems a bit unfortunate that non-null ISeq instances are allowed to be empty. Allowing for this seems to create a requirement for a lot of unnecessary seq() calls throughout the Clojure code base.

It seems like it might be better if:
a) ISeq instances were constrained to be non-empty or nil
b) Objects representing potentially empty sequences (LazySeq etc.) are simple Sequable / Sequentional, not ISeq instances

I don't think this is easily fixable at this point however.

Comment by Mike Anderson [ 21/Jul/15 10:22 PM ]

Probably not fixable without invasive changes across the code base

Comment by Kevin Downey [ 22/Jul/15 12:24 PM ]

historical note:

at one point clojure did not have empty seqs, it had seqs with stuff in them and nil. in order to achieve this the laziness was of a slightly different character, seqs effectively called `seq` on themselves when created so either at least the first element was realized, or you got nil. a blog post or two came out discussing the nature of the laziness of seqs, and I think one even turned its nose up at any kind of laziness that wasn't completely lazy, and some time after that Rich changed the laziness of lazy seqs. If you look at #clojure's irc log around this time you'll see lots of discussion about nil-punning being broken, and a brief window when `if` was a macro that expanded in to `if*` and the `if` macro checked for broken nil punning. I don't recall exactly but I think that was sometime in 2009.

this also is where `next` came from. clojure had `first` and `rest`, but now `rest` could return an empty seq, so if you wanted to continue nil punning and not worry about empty seqs, `next` does the same thing as `rest`, but it never returns an empty seq, so its result can be safely used in an `if`





[CLJ-1782] Hide local IDE files in .gitignore Created: 21/Jul/15  Updated: 21/Jul/15

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

Type: Enhancement Priority: Trivial
Reporter: Mike Anderson Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File clj-1782.patch    

 Description   

Several IDEs (e.g. Eclipse/CCW, IntelliJ IDEA/Cursive) create local files in project workspaces which should normally be ignored for the purposes of source control.

This patch proposes to add some common files that should be ignored to the .gitignore






[CLJ-1781] Tuples don't extend IKVReduce Created: 19/Jul/15  Updated: 30/Jul/15  Resolved: 30/Jul/15

Status: Closed
Project: Clojure
Component/s: None
Affects Version/s: Release 1.8
Fix Version/s: Release 1.8

Type: Defect Priority: Major
Reporter: Alex Miller Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: regression
Environment:

1.8.0-alpha1 or 1.8.0-alpha2


Approval: Vetted

 Description   

This is a regression from the tuple stuff (both return nil in 1.7):

(reduce-kv (fn [acc idx in] acc) nil [1 2 3 4 5 6 7]) ; => nil
(reduce-kv (fn [acc idx in] acc) nil [1 2])
;; =>  No implementation of method: :kv-reduce of protocol:
;; #'clojure.core.protocols/IKVReduce found for class: clojure.lang.Tuple$T2


 Comments   
Comment by Michael Blume [ 19/Jul/15 11:47 AM ]

CLJ-1689 would sort this.

Comment by Alex Miller [ 30/Jul/15 1:14 PM ]

Since tuples were pulled out in 1.8.0-alpha3, declining.





[CLJ-1780] Test OOME from locals clearing change Created: 17/Jul/15  Updated: 21/Aug/15  Resolved: 21/Aug/15

Status: Closed
Project: Clojure
Component/s: None
Affects Version/s: Release 1.8
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Alex Miller Assignee: Unassigned
Resolution: Not Reproducible Votes: 0
Labels: regression
Environment:

1.8.0-alpha2


Attachments: Text File strengthen-clearing-test.patch    
Approval: Triaged

 Description   

IBM JDK 1.6 in test matrix is throwing errors from the new test for CLJ-1250 in 1.8.0-alpha2.

[java] ERROR in (test-closed-over-clearing) (Range.java:133)
     [java] expected: (number? (reduce + 0 (r/map identity (range 1.0E8))))
     [java]   actual: java.lang.OutOfMemoryError: null
     [java]  at clojure.lang.Range.forceChunk (Range.java:133)
     [java]     clojure.lang.Range.chunkedFirst (Range.java:150)
     [java]     clojure.core$chunk_first.invoke (core.clj:667)
     [java]     clojure.core.protocols/fn (protocols.clj:136)
     [java]     clojure.core.protocols$fn__6478$G__6473__6487.invoke (protocols.clj:19)
     [java]     clojure.core.protocols$seq_reduce.invoke (protocols.clj:31)
     [java]     clojure.core.protocols/fn (protocols.clj:95)
     [java]     clojure.core.protocols$fn__6452$G__6447__6465.invoke (protocols.clj:13)
     [java]     clojure.core.reducers$folder$reify__21452.coll_reduce (reducers.clj:126)
     [java]     clojure.core$reduce.invoke (core.clj:6519)
     [java]     clojure.test_clojure.reducers/fn (reducers.clj:95)
     [java]     clojure.test$test_var$fn__7669.invoke (test.clj:703)
     [java]     clojure.test$test_var.invoke (test.clj:703)
     [java]     clojure.test$test_vars$fn__7691$fn__7696.invoke (test.clj:721)
     [java]     clojure.test$default_fixture.invoke (test.clj:673)
     [java]     clojure.test$test_vars$fn__7691.invoke (test.clj:721)
     [java]     clojure.test$default_fixture.invoke (test.clj:673)
     [java]     clojure.test$test_vars.invoke (test.clj:717)
     [java]     clojure.test$test_all_vars.invoke (test.clj:727)
     [java]     clojure.test$test_ns.invoke (test.clj:746)
     [java]     clojure.core$map$fn__4553.invoke (core.clj:2624)
     [java]     clojure.lang.LazySeq.sval (LazySeq.java:40)
     [java]     clojure.lang.LazySeq.seq (LazySeq.java:49)
     [java]     clojure.lang.Cons.next (Cons.java:39)
     [java]     clojure.lang.RT.next (RT.java:681)
     [java]     clojure.core/next (core.clj:64)
     [java]     clojure.core$reduce1.invoke (core.clj:909)
     [java]     clojure.core$reduce1.invoke (core.clj:900)
     [java]     clojure.core$merge_with.doInvoke (core.clj:2936)
     [java]     clojure.lang.RestFn.applyTo (RestFn.java:139)
     [java]     clojure.core$apply.invoke (core.clj:632)
     [java]     clojure.test$run_tests.doInvoke (test.clj:761)
     [java]     clojure.lang.RestFn.applyTo (RestFn.java:137)
     [java]     clojure.core$apply.invoke (core.clj:630)
     [java]     user$eval28810.invoke (run_test.clj:8)
     [java]     clojure.lang.Compiler.eval (Compiler.java:6850)


 Comments   
Comment by Nicola Mometto [ 18/Jul/15 12:02 AM ]

I don't understand how forcing a 32 element chunk could consume the memory. If locals clearing worked properly there should be no other part of that sequence in memory but I might be missing some detail.

Might there be something going on with the recent change in impl for Range?

Comment by Ghadi Shayban [ 18/Jul/15 12:19 AM ]

An interesting note is that (reduce + 0 (r/map identity (range 1e8))) is going through the seq path, despite reducers && reducible range. This is because coll-reduce doesn't prefer IReduceInit.

The CLJ-1250 test should be modified to intentionally hold the head of a seq in order to exercise the locals clearing. A good hypothesis from Alex is that GC is a bit slower on the archaic IBM JDK.

Comment by Ghadi Shayban [ 18/Jul/15 1:21 AM ]

I can't reproduce this on Linux x86-64 with IBM JDK and an artificially tiny heap.

[ghadi@amdhex ibm-java-x86_64-60]$ ./bin/java -Xmx128m -cp $HOME/jsr166y-1.7.0.jar:$HOME/clojure-1.8.0-master-SNAPSHOT.jar clojure.main
Clojure 1.8.0-master-SNAPSHOT
user=> (require '[clojure.core.reducers :as r])
nil
user=> (number? (reduce + 0 (r/map identity (range 1e8))))
true

Can you post details on the IBM JDK environment in CI? Need point release, heap size, kernel, distro, and jvm opts.

I've strengthened the CLJ-1250 test case by relying on neither reducer impl nor range impl, and I reverified that the bug is in fact present on <1.7 and gone on -master using Oracle JDK and 128m heap.

Comment by Alex Miller [ 18/Jul/15 8:52 AM ]

OS is CentOS 5.5 afaict

JDK details:

java version "1.6.0"
Java(TM) SE Runtime Environment (build pxa6460sr9fp2-20110625_01(SR9 FP2))
IBM J9 VM (build 2.4, JRE 1.6.0 IBM J9 2.4 Linux amd64-64 jvmxa6460sr9-20110624_85526 (JIT enabled, AOT enabled)
J9VM - 20110624_085526
JIT - r9_20101028_17488ifx17
GC - 20101027_AA)
JCL - 20110530_01

As far as I can tell, nothing is being done to alter the default heap size or other jvm opts during the build.

Comment by Ghadi Shayban [ 18/Jul/15 2:27 PM ]

The IBM JDK6 version I used was:

java version "1.6.0"
Java(TM) SE Runtime Environment (build pxa6460sr16fp7-20150708_01(SR16 FP7))
IBM J9 VM (build 2.4, JRE 1.6.0 IBM J9 2.4 Linux amd64-64 jvmxa6460sr16fp7-20150701_255724 (JIT enabled, AOT enabled)
J9VM - 20150701_255724
JIT - r9_20150630_95420
GC - GA24_Java6_SR16_20150701_1008_B255724)
JCL - 20150628_01

Seems like SR16 FP7 == 6.0.16.7, and the one on the CI build is SR9 FP2 == 6.0.9.2, a four or five year difference between point releases.

Comment by Ghadi Shayban [ 18/Jul/15 2:51 PM ]

OK I found a download for the (old) IBM JDK 6.0.9.2 and installed it on Linux x86-64. I cannot reproduce the bug.

Comment by Alex Miller [ 18/Jul/15 3:53 PM ]

I'd be happy to update the ibm jdk 1.6 version and n the build box too to see what happens.

Comment by Alex Miller [ 21/Aug/15 3:09 AM ]

Since the CLJ-1250 patch has been reverted, this is no longer failing in the build and I'm going to close it for now. Will reopen if necessary when CLJ-1793 (the updated version of CLJ-1250) goes in.





[CLJ-1779] Optimise compiler usage of getMethod calls Created: 17/Jul/15  Updated: 17/Jul/15

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

Type: Enhancement Priority: Major
Reporter: Mike Anderson Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Currently the Clojure compiler makes multiple redundant calls to Method.getMethod(...) while emitting code, e.g.

gen.invokeStatic(Type.getType(Long.class), Method.getMethod("Long valueOf(long)"));

It seems to be the case that that:

a) These getMethod calls are effectively returning equivalent, immutable constant values
b) getMethod is moderately expensive (performs string analysis and quite a few object allocations)
c) These calls are very common during compilation of typical Clojure code

The proposed enhancement is to replace all of these getMethod calls with constant static values. This should improve compilation performance noticeably with no effect on behaviour.






[CLJ-1778] let-bound namespace-qualified bindings should throw (if not map destructuring) Created: 16/Jul/15  Updated: 11/Sep/15  Resolved: 11/Sep/15

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