<< Back to previous view

[CLJ-1971] Update docstring of empty? to suggest not-empty instead of seq Created: 27/Jun/16  Updated: 27/Jun/16

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

Type: Enhancement Priority: Trivial
Reporter: Daniel Compton Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

The docstring for empty? says

clojure.core/empty? [coll]
Returns true if coll has no items - same as (not (seq coll)). Please use the idiom (seq x) rather than (not (empty? x))

Would it make more sense to suggest using not-empty, instead of seq here?






[CLJ-1970] instrumented macros never conform valid forms Created: 25/Jun/16  Updated: 25/Jun/16

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

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


 Description   

Although macros can be speced without &form and &env, once they are instrumented they will try to conform the args including &form/&env and fail:

user=> (require '[clojure.spec :as s])
nil
user=> (defmacro m [x] x)
#'user/m
user=> (s/fdef m :args (s/cat :arg integer?) :ret integer?)
user/m
user=> (m 1)
1
user=> (m a)
CompilerException java.lang.IllegalArgumentException: Call to user/m did not conform to spec:
In: [0] val: a fails at: [:args :arg] predicate: integer?
:clojure.spec/args  (a)
, compiling:(NO_SOURCE_PATH:5:1) 
user=> (s/instrument)
[user/m]
user=> (m 1)
ExceptionInfo Call to #'user/m did not conform to spec:
In: [0] val: (m 1) fails at: [:args :arg] predicate: integer?
:clojure.spec/args  ((m 1) nil 1)
  clojure.core/ex-info (core.clj:4718)
user=>

To resolve the situation, I think instrument/instrument-all should avoid speced macros.



 Comments   
Comment by Alex Miller [ 25/Jun/16 9:42 AM ]

This is an issue we're discussing - for the moment, you should not instrument macros. There is no point in instrumenting them as they are automatically checked during macroexpansion.

Comment by OHTA Shogo [ 25/Jun/16 10:00 AM ]

Yes, I know the compiler checks macro specs automatically, but just thought it would be nice if explicit calls to instrument (with no args) and instrument-all would check whether or not each speced var is a macro and filter it out if so.

Comment by Alex Miller [ 25/Jun/16 2:30 PM ]

Totally agreed.





[CLJ-1966] :clojure.spec/invalid is not a valid :clojure.spec/any value Created: 21/Jun/16  Updated: 24/Jun/16

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

Type: Defect Priority: Major
Reporter: Alexander Kiel Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

(clojure.spec/valid? :clojure.spec/any :clojure.spec/invalid) returns false

This issue gets serious, if one likes to write specs for core functions like = which are used by spec itself. I observed this bug as I wrote a spec for assoc.

A possible solution could be to use an (Object.) sentinel internally and :clojure.spec/invalid only at the API boundary. But I have not thought deeply about this.



 Comments   
Comment by Alexander Kiel [ 24/Jun/16 9:48 AM ]

I have another example were the described issue arises. It's not possible to test the return value of a predicate suitable for conformer, because it should return :clojure.spec/invalid itself.

(ns coerce
  (:require [clojure.spec :as s]))

(s/fdef parse-long
  :args (s/cat :s (s/nilable string?))
  :ret (s/or :val int? :err #{::s/invalid}))

(defn parse-long [s]
  (try
    (Long/parseLong s)
    (catch Exception _
      ::s/invalid)))




[CLJ-1965] clojure.spec/def should support an optional doc-string Created: 19/Jun/16  Updated: 25/Jun/16

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

Type: Enhancement Priority: Minor
Reporter: Alexander Kiel Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: spec


 Description   

Like clojure.core/def clojure.spec/def should support an optional doc string because one usually likes to describe specs in more detail as one could through keyword naming.






[CLJ-1963] clojure.spec/map-of has confusing error message when input is not a map Created: 15/Jun/16  Updated: 15/Jun/16

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

Type: Defect Priority: Major
Reporter: Russell Mull Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: errormsgs, spec


 Description   

When using map-of specs, the error message given when checking a non-map value is less than enlightening:

user> (require '[clojure.spec :as s])
nil
user> (s/def ::int-map (s/map-of integer? integer?))
:user/int-map
user> (s/explain ::int-map :not-a-map)
val: :not-a-map fails spec: :user/int-map predicate: (coll-checker (tuple integer? integer?))
nil

This can be worked around to some degree by requiring that the value be a map explicitly:

user> (s/def ::fancy-int-map (s/and map? ::int-map))
:user/fancy-int-map
user> (s/explain ::fancy-int-map :not-a-map)
val: :not-a-map fails spec: :user/fancy-int-map predicate: map?
nil

The definition

map-of
looks like it's trying to do just this, but the
map?
predicate comes second for some reason.






[CLJ-1954] clojure.set/intersection mishandles vectors Created: 09/Jun/16  Updated: 09/Jun/16

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

Type: Defect Priority: Major
Reporter: Ashton Kemerling Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: set


 Description   

clojure.set/intersection appears to use the indexes of vectors as values. This results in very strange behavior if you accidentally end up passing a vector in as one of the arguments.

ti.repl-init=> (clojure.set/intersection #{0 1} [2 2 2 2 2])
#{0 1}
ti.repl-init=> (clojure.set/intersection [2 2 2 2] #{0 1})
#{0 1}
ti.repl-init=> (clojure.set/intersection [0 1] [2 2 2 2])
[0 1]
ti.repl-init=> (clojure.set/intersection [2 2 2 2] [2 2 2 2])
[2 2 2 2]
ti.repl-init=> (clojure.set/intersection [3 3 3 ] [2 2 2 2])
[3 3 3]
ti.repl-init=> (clojure.set/intersection [55] [2 2 2 2])

ClassCastException clojure.lang.PersistentVector cannot be cast to clojure.lang.IPersistentSet  clojure.core/disj (core.clj:1476)

If any of the arguments are lists, you get a ClassCastException which is maybe a bit less clear than one would hope.

ti.repl-init=> (clojure.set/intersection #{0 1} (list 2 2 2 2))

IllegalArgumentException contains? not supported on type: clojure.lang.PersistentList  clojure.lang.RT.contains (RT.java:814)

The same also happens if all arguments are lists:



 Comments   
Comment by Ashton Kemerling [ 09/Jun/16 9:44 AM ]

More odd side effects.

ti.repl-init=> (clojure.set/intersection #{:foo} {:foo 1})
#{:foo}
ti.repl-init=> (clojure.set/intersection #{:foo} {})
{}
ti.repl-init=> (clojure.set/intersection #{:foo} [:foo])
#{}
ti.repl-init=> (clojure.set/intersection [:foo] [:foo])

ClassCastException clojure.lang.PersistentVector cannot be cast to clojure.lang.IPersistentSet  clojure.core/disj (core.clj:1476)
ti.repl-init=> (clojure.set/intersection [0] [:foo])
[0]
Comment by Alex Miller [ 09/Jun/16 9:54 AM ]

See comments on CLJ-1953





[CLJ-1953] clojure.set should check or throw on non-set inputs Created: 09/Jun/16  Updated: 09/Jun/16

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

Type: Defect Priority: Major
Reporter: Ashton Kemerling Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: set
Environment:

Not Relevant



 Description   

clojure.set/union is very sensitive to the types of its inputs. It does not attempt to check or fix the input types, raise an error, or even document this behavior.

If all inputs are sets, it works.

ti.repl-init=> (clojure.set/union #{1 2 3} #{1 2 3 4})
#{1 4 3 2}

If the arguments are both vectors or sequences, it returns the same type with duplicates.

ti.repl-init=> (clojure.set/union [1 2 3] [1 2 3])
[1 2 3 1 2 3]
ti.repl-init=> (clojure.set/union (list 1 2 3) (list 1 2 3))
(3 2 1 1 2 3)

If the arguments are mixed, the correct result is returned only if the longest input argument is a set.

ti.repl-init=> (clojure.set/union #{1 2 3} [2 3])
#{1 3 2}
ti.repl-init=> (clojure.set/union [1 2 3] #{2 3})
[1 2 3 3 2]
ti.repl-init=> (clojure.set/union [2 3] #{1 2 3})
#{1 3 2}
ti.repl-init=> (clojure.set/union #{2 3} [1 2 3])
[1 2 3 3 2]


 Comments   
Comment by Alex Miller [ 09/Jun/16 9:40 AM ]

This has been raised a number of times. See CLJ-1682, CLJ-810.

Comment by Ashton Kemerling [ 09/Jun/16 9:52 AM ]

I do not see set/union being covered in the tickets you mentioned.

Furthermore, this issue differs from the intersection bugs in a few ways important ways:

  1. It silently returns data that is the wrong type, and which contains the wrong values.
  2. It never raises an exception.

But it does share the following bugs with the intersection problem:

  1. This behavior is not only type dependent, but data dependent. It will happen to work depending on the lengths of the given sets.
  2. It isn't even documented that this function expects sets.
  3. It runs directly contrary to the definition of the mathematical function it purports to represent.

I only caught this bug in my own code because I hand inspected the result. I had just assumed that set/union would do the right thing, and was deeply surprised when against both definition and documentation it did not.

Comment by Andy Fingerhut [ 09/Jun/16 11:07 AM ]

I am sympathetic to your desires, Ashton, but have no new arguments that might convince those who decide what changes are made to Clojure that it would be a good enough idea to do so.

I would point out an answer to one of your comments: "It isn't even documented that this function expects sets." It seems to me from past comments that the point of view of the Clojure core team is that this is documented, e.g. "Return a set that is the union of the input sets" tells you what clojure.set/union does when you give it sets as arguments. It specifies nothing about what it does when you give it non-set arguments, so it is free to do anything at all in those cases, including what it currently does.





[CLJ-1951] bigint? predicate and generator Created: 08/Jun/16  Updated: 08/Jun/16

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

Type: Enhancement Priority: Minor
Reporter: Alex Miller Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: generator


 Description   

Add bigint? and spec.gen support.

This part is easy:

(defn bigint?
  "Returns true if n is a BigInt"
  {:added "1.9"}
  [n] (instance? clojure.lang.BigInt n))

The generator is the tricky bit. test.check doesn't have a generator for bigints, just large-integer for things in long range. I think we'd want numbers beyond long range in a bigint generator (as that's a likely place where bugs might lie). Making a really high-quality bigint generator (with good growth and shrinking characteristics) is something that needs more thought.

http://clojure.github.io/test.check/clojure.test.check.generators.html#var-large-integer






[CLJ-1949] Generator for fspec is not deterministic & ignores sizing Created: 05/Jun/16  Updated: 13/Jun/16

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

Type: Defect Priority: Major
Reporter: Gary Fredericks Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: generator, spec

Attachments: Text File CLJ-1949-impure.patch     Text File CLJ-1949-pure.patch    
Patch: Code

 Description   

Problem

One of the goals of test.check is for users to be able to write arbitrarily rich generators while maintaining determinism, which has obvious benefits for reproducing failures.

Currently the fspec generator generates a function which itself generates random return values by calling clojure.test.check.generators/generate, which is a function intended only for development use as it circumvents test.check's controlled source of psuedorandomness. It also circumvents test.check's sizing mechanism, since the generate function always uses a size of 30.

Possible Solutions

I see two reasonable solutions to this, depending on whether the generated function ought to be a pure function (which it currently isn't, since it ignores its arguments and randomly generates a return value).

Pure Function

We can generate a non-empty vector of possible return values and use that to create a function that selects one of the possible return values using the hash of the arguments.

Impure Function

We can generate a non-empty collection of possible return values and use that to create a function with internal state that cycles through the possible return values.



 Comments   
Comment by Gary Fredericks [ 05/Jun/16 5:44 PM ]

Added a patch for each of the approaches listed. Would be happy to add tests too if feedback is given about either approach being preferred.





[CLJ-1947] Add vec-of spec Created: 05/Jun/16  Updated: 09/Jun/16

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

Type: Enhancement Priority: Minor
Reporter: Maarten Truyens Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: spec


 Description   

It would be great to add a "vec-of" (and perhaps also a "set-of") Spec, similar to the already existing map-of. I find myself writing (coll-of ::foo []) writing too often.






[CLJ-1946] improve error messages for `map-of` spec Created: 04/Jun/16  Updated: 13/Jun/16

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

Type: Enhancement Priority: Major
Reporter: Chris Price Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: errormsgs, spec


 Description   

When using a map-of spec where the value predicate refers to another spec, the error message if a value does not conform does not seem like it is as helpful as it could be:

(spec/def ::myint integer?)
(spec/explain
 (spec/map-of string? ::myint)
 {"x" 1 "y" "not an int!"})
=> :user.swagger-ui-service/myint
val: {"x" 1, "y" "not an int!"} fails predicate: (coll-checker (tuple string? :user.swagger-ui-service/myint))

The explain result doesn't indicate which key/value pair in the map failed to conform, and it also doesn't make it clear that the integer? predicate is ultimately the one that caused the failure.

From reading through the introduction and docs, it seemed like error messaging was a primary motivation for spec, so any improvements that might be possible to this error message would be extremely valuable.



 Comments   
Comment by Chris Price [ 04/Jun/16 11:22 AM ]

This becomes particularly important with deeply-nested specs and/or large maps.

Comment by Sean Corfield [ 04/Jun/16 11:53 PM ]

In my opinion, the failure should use the spec name, not the underlying predicate function, in the message – isn't that the whole point of using named specs in this?

(as for explaining the value that failed, I agree on the surface but haven't looked at the implementation in detail to see if there might be a good reason)

Comment by Chris Price [ 06/Jun/16 12:23 PM ]

If nothing else, it's inconsistent with what gets logged for other types of specs:

(spec/explain
 (spec/cat :myint ::myint)
 ["hi"])
 
In: [0] val: "hi" fails spec: :puppetlabs.trapperkeeper.main/myint at: [:myint] predicate: integer?
=> nil
(spec/explain
 (spec/tuple ::myint)
 ["hi"])
 
In: [0] val: "hi" fails spec: :puppetlabs.trapperkeeper.main/myint at: [0] predicate: integer?
=> nil

Having spent a good chunk of a day hacking on a project that involved some very deeply-nested specs, I can definitively say that the logging from cat and tuple was much easier to debug. With those, I could keep poking at my "production" code via the REPL, tweaking things until the specs were correct. With map-of, the only way I was able to debug was to copy/paste the entire failed val into the REPL and cobble together a one-off spec/explain form to repro, and then delete things from it until I got past the map-of so that the error was less opaque. Then once I fixed the actual issue I'd need to copy/paste the REPL stuff back into my "production" code. It wasn't impossible, but it was a much more tedious workflow than when dealing with errors from cat or tuple.

Comment by Alex Miller [ 13/Jun/16 3:50 PM ]

The issues here is due to the way map-of and coll-of sample their contents rather than fully conforming all of them. This is done for performance but is not what most people expect. It's likely there will be some more additions in this area.





[CLJ-1943] clojure.spec should implicitly convert classes to specs Created: 03/Jun/16  Updated: 09/Jun/16

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

Type: Enhancement Priority: Minor
Reporter: Kevin Corcoran Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: spec


 Description   

It would be nice if clojure.spec implicitly converted Java classes to specs, as it does for predicates. As a comparison, plumatic/schema allows classes to be used as schemas directly, and I take advantage of this regularly, as I currently use both schema and interop quite heavily.

For example, the spec guide contains the following:

(import java.util.Date)
(s/valid? #(instance? Date %) (Date.))  ;; true

... and then, later, defines:

(s/def ::date #(instance? Date %))

If classes were implicitly converted to specs, ::date would be unnecessary, and the first example could be simplified to:

(import java.util.Date)
(s/valid? Date (Date.))  ;; true

This would make clojure.spec a lot easier to use and adopt on my projects.



 Comments   
Comment by Alex Miller [ 04/Jun/16 9:07 PM ]

This was proposed and we decided not to include it in the initial release of spec. I do not know that we will in the future though, so leaving this open for now.

Comment by Sean Corfield [ 04/Jun/16 11:37 PM ]

At World Singles we use Expectations and it also automatically treats Java classes as type-based predicates. That said, I don't think a core library should do this. It's convenient "magic" but it doesn't actually feel very Clojure-y. I think I would vote against this being added to clojure.spec.

Comment by Alex Miller [ 09/Jun/16 9:03 AM ]

Note that for this particular example, inst? is now available in core.





[CLJ-1941] Instrumentation of fns with primitive type hints fails Created: 01/Jun/16  Updated: 14/Jun/16

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

Type: Defect Priority: Major
Reporter: Kenny Williams Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: spec
Environment:

Ubuntu 15.10
Using boot 2.6.0 on openjdk version "1.8.0_91"



 Description   
(require '[clojure.spec :as s])
(defn foo [^double val] val)
(s/fdef foo :args (s/cat :val double?))
(s/instrument #'foo)
(foo 5.2)

ClassCastException clojure.spec$spec_checking_fn$fn__12671 cannot be cast to clojure.lang.IFn$DO  user/eval9 (NO_SOURCE_FILE:6)
	user/eval9 (NO_SOURCE_FILE:6)
	user/eval9 (NO_SOURCE_FILE:6)
	clojure.lang.Compiler.eval (Compiler.java:6942)
	clojure.lang.Compiler.eval (Compiler.java:6905)
	clojure.core/eval (core.clj:3177)
	clojure.core/eval (core.clj:3173)
	clojure.main/repl/read-eval-print--9414/fn--9417 (main.clj:240)
	clojure.main/repl/read-eval-print--9414 (main.clj:240)
	clojure.main/repl/fn--9423 (main.clj:258)
	clojure.main/repl (main.clj:258)
	clojure.main/repl-opt (main.clj:322)
	clojure.main/main (main.clj:421)

Cause: spec replaces var values with instrumented functions that will not work with primitive function interfaces

Approach: Take primitive interfaces into account and make them work, or document/fail that instrumentation will not work with these.



 Comments   
Comment by Kevin Downey [ 02/Jun/16 1:41 AM ]

spec replaces var values with instrumented functions, which works for the default linking case, var deref cast to ifn, invoke, but in the other cases (primitive functions, direct linking, others?) this won't work

Comment by Kenny Williams [ 02/Jun/16 3:39 PM ]

Hmm. Well this should be at least be documented. So, spec cannot be used on functions with a type hinted arg?

Comment by Sean Corfield [ 02/Jun/16 4:16 PM ]

Spec cannot be used on functions with primitive typed hinted arguments or returns – non-primitive type hints seem to be fine.

But documentation isn't enough here: instrumenting a namespace and then discovering it broke a function (that happened to have a primitive type hint) isn't acceptable. If the instrumentation isn't going to work, the function should be skipped (and a warning produced, hopefully).

Comment by Kevin Downey [ 02/Jun/16 8:10 PM ]

yeah, I was giving the root cause of the issue, not excusing the issue.

Understanding the root cause predicts other places where there will be issues: where ever some non-default function linking strategy is used.

One such place is direct linked functions, but I suspect for direct linked functions, Clojure/Core will just say you should only instrument code for testing, and you should only turn on direct liking for production.

Another case, which I am sort of surprised we haven't heard more about yet is protocol functions.

Comment by Sean Corfield [ 02/Jun/16 8:35 PM ]

Your comment about direct linking made me wonder about the validity of spec'ing and instrumenting clojure.core functions. The examples show clojure.core/symbol, but Clojure's core library is shipped as direct linked, as of 1.8.0 isn't it?

Comment by Kevin Downey [ 03/Jun/16 3:14 AM ]

what alters the calling convention isn't the function being compiled with direct linking on, but a caller of that function being compiled with direct linking on.

This code will throw a non-conforming error for the bogus symbol spec with direct linking off, and return the symbol foo with direct linking on

(require '[clojure.spec :as s])

(s/fdef symbol
  :args string?
  :ret symbol?)

(defn foo
  []
  (symbol 'foo))

(s/instrument-all)

(foo)
Comment by Kevin Downey [ 03/Jun/16 3:26 AM ]

This code returns true because m is a protocol function, if you replace it with a regular function it throws a non-conforming error

(require '[clojure.spec :as s])

(defprotocol P
  (m [_]))

(deftype T []
  P
  (m [_] true))

(s/fdef m
  :args (s/cat :p (constantly false))
  :ret string?)

(defn foo
  []
  (m (T.)))

(s/instrument-all)

(foo)
Comment by Alex Miller [ 13/Jun/16 3:53 PM ]

@Sean instrumenting core functions will work for calls from your code into core (which are presumably not direct-linked), but will not affect calls from one core function to another as they are direct-linked and do not go through the var. One thing we've considered for a long while is building a dev version of core that would not be direct-linked and could potentially turn on instrumentation or other helpful dev-time tooling.

Comment by Sean Corfield [ 14/Jun/16 5:48 PM ]

Thanks for that answer @alexmiller – We have dev set to non-direct-linking but QA / production set to direct linking, so I'm only concerned about possible issues in dev with (s/instrumental-all) and wanting to be sure "code won't break". If instrumentation won't affect existing (direct-linked) calls within core, that's good enough for me. I am concerned about primitive hinting and protocols (and whatever crawls out of the woodwork here) since you don't want to be forced to read the source of every library you include, just to see whether (s/instrument-all) is safe or whether it will bite you in some weird way while you're developing.





[CLJ-1940] spec has no way to specify a non-fn var should always conform Created: 30/May/16  Updated: 09/Jun/16

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

Type: Enhancement Priority: Minor
Reporter: Allen Rohner Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: spec


 Description   

It appears there's no way to specify that a non-function var should always conform, after e.g. alter-var-root or binding.



 Comments   
Comment by Alex Miller [ 05/Jun/16 3:20 PM ]

I'm not sure it makes sense to do this at all in the case of a def. If you really want to check it on definition you could do so by explicitly calling valid?.

If you want to check changes via alter-var-root, you can do so by setting a var validator using http://clojure.github.io/clojure/clojure.core-api.html#clojure.core/set-validator!

I again don't think it makes a lot of sense to do anything automatic in binding either. You can always validate it explicitly if you want to.

Basically, I think this is outside the use case spec is trying to cover but I'll check with Rich before declining.





[CLJ-1936] spec generative testing should be optional Created: 28/May/16  Updated: 05/Jun/16

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

Type: Defect Priority: Major
Reporter: Allen Rohner Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: spec, test


 Description   

When using spec on side-effecting functions, I'd like to configure instrumentation (i.e. checking args and return value conform) separately from generative testing.

Proof that generative testing happens without calling clojure.test:

(defn foo [fnn] (fnn 42))
(s/fdef foo :args (s/cat :f (s/fspec :args (s/cat :i integer?)
                                     :ret integer?)))

(foo #(do (println %) (when (even? %) 42)))

=> (foo #(do (println %) (when (even? %) 42)))
42

(s/instrument 'foo)

=> (foo #(do (println %) (when (even? %) 42)))
0
-1
0
0
0
-1
0
0
-1
0
-1

ExceptionInfo Call to #'repl/foo did not conform to spec:
In: [0] val: nil fails at: [:args :f :ret] predicate: integer?
:clojure.spec/args  (#object[repl$eval67350$fn__67351 0x7bc3811c "repl$eval67350$fn__67351@7bc3811c"])
  clojure.core/ex-info (core.clj:4617)


 Comments   
Comment by Zach Oakes [ 28/May/16 9:01 PM ]

I think it would make sense to add something like core.typed's ^:no-check for this. For example:

(s/fdef ^:no-check foo :args (s/cat :f (s/fspec :args (s/cat :i integer?) :ret integer?)))

As a stopgap measure, I made a boot task that has a copy of clojure.spec.test/run-all-tests and modifies it to ignore vars with that metadata. That means I have to add it to the metadata in defn rather than fdef but it still seems to work.

Comment by Sean Corfield [ 01/Jun/16 9:32 PM ]

Yes, there are definitely situations where I would want argument / return spec checking on calls during dev/test but absolutely need the function excluded from generative testing.

Comment by Sean Corfield [ 01/Jun/16 9:38 PM ]

If you don't have test.check on your classpath, the call to s/instrument succeeds but then attempting to call foo fails with:

boot.user=> (defn foo [fnn] (fnn 42))
#'boot.user/foo
boot.user=> (s/fdef foo :args (s/cat :f (s/fspec :args (s/cat :i integer?)
       #_=>                                      :ret integer?)))
boot.user/foo
boot.user=> 

boot.user=> (foo #(do (println %) (when (even? %) 42)))
42
42
boot.user=> (s/instrument 'foo)
#'boot.user/foo
boot.user=> (foo #(do (println %) (when (even? %) 42)))

java.io.FileNotFoundException: Could not locate clojure/test/check/generators__init.class or clojure/test/check/generators.clj on classpath.

That is certainly unexpected and not very friendly!





[CLJ-1935] clojure.spec/multi-spec ignores the mutlimethod hierarchy Created: 28/May/16  Updated: 06/Jun/16

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

Type: Defect Priority: Major
Reporter: Magyari Viktor Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: multimethods, spec


 Description   

Minimal example:

(require '[clojure.spec :as s])

(def h (derive (make-hierarchy) :a :b))

(defmulti spec-type identity :hierarchy #'h)

(defmethod spec-type :b [_]
  (s/spec (constantly true)))

(s/def ::multi (s/multi-spec spec-type identity))

(s/explain ::multi :b)
;; => Success!
;; as expected

(s/explain ::multi :a)
;; => val: :a fails at: [:a] predicate: spec-error/spec-type,  no method
;; fails even though :a derives from :b

Also fails with the default hierarchy. Worked fine in alpha1, broken in alpha2 and alpha3.






[CLJ-1928] Provide meaning error message when eval of function fails. Created: 15/May/16  Updated: 15/May/16

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

Type: Defect Priority: Minor
Reporter: Richard Davies Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: errormsgs
Environment:

All



 Description   

When attempting to eval a function, in some cases this fails with "No matching ctor found". This error does not clearly indicate the root cause. Suggest something like "cannot eval function" or something similar. See http://dev.clojure.org/jira/browse/CLJ-1206 for history relating to this ticket.






[CLJ-1925] Add uuid and random-uuid functions Created: 10/May/16  Updated: 11/May/16

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

Type: Enhancement Priority: Minor
Reporter: Daniel Compton Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: function


 Description   

ClojureScript has uuid and random-uuid functions. These are handy to have in ClojureScript, and I think would be useful also in Clojure to improve code portability. Is there interest in a patch for this?



 Comments   
Comment by Alex Miller [ 10/May/16 8:00 AM ]

I think the main reason to do this would be portability. It would make most sense to generate java.util.UUIDs - is that harmonious with what is being done in ClojureScript? That is, could the same code for creating and using uuids work on both platforms? If not, then there might not be a good reason to do so.

Comment by Daniel Compton [ 10/May/16 3:45 PM ]

> It would make most sense to generate java.util.UUIDs - is that harmonious with what is being done in ClojureScript?

ClojureScript defines it's own UUID type, as one doesn't exist in JavaScript. https://github.com/clojure/clojurescript/blob/dd589037f242b4eaace113ffa28ab7b3791caf47/src/main/cljs/cljs/core.cljs#L10088-L10128. I'm not quite sure what you mean by harmonious.

> That is, could the same code for creating and using uuids work on both platforms?

The CLJS UUID doesn't support all of the methods of the Java UUID, but the important things are there (equivalence, constructing from a string, printing to a string) and they would be enough to significantly improve portability when working with UUID's.

Comment by Nicola Mometto [ 10/May/16 4:27 PM ]

both clojure and clojurescript have uuid tagged literals, that should be good enough for interop

Comment by Alex Miller [ 11/May/16 2:48 PM ]

I'm aware of that, just wondering if there are any functions you might invoke on a uuid that would need some portable equivalent, like the stuff in http://docs.oracle.com/javase/8/docs/api/java/util/UUID.html.

Comment by Daniel Compton [ 11/May/16 3:27 PM ]

Most of the extra methods here are useful for distinguishing between multiple types of UUID's, or getting information out of time based UUIDs.

clockSequence() - time based
compareTo(UUID val) - not sure if equivalent required?
boolean	equals(Object obj) - no action required
static UUID	fromString(String name) - constructor
long	getLeastSignificantBits() - not sure how important these two are
long	getMostSignificantBits()
int	hashCode() - no action required
static UUID	nameUUIDFromBytes(byte[] name) - is this useful/important?
long	node() - only useful for time UUID
static UUID	randomUUID() - would implement this
long	timestamp() - time based UUID
String	toString() - no action required
int	variant() - for distinguishing between different types of UUID's
int	version() - for distinguishing between different versions of UUID's

I could potentially see an argument for time based UUID's being included in a patch here too, but I'm not sure if they are used enough to be worth it, and they'd need to go into CLJS, e.t.c.

There is use of some of these methods in Clojure code:
https://github.com/search?l=clojure&q=.getLeastSignificantBits&type=Code&utf8=
https://github.com/search?utf8=✓&q=%22.nameUUIDFromBytes%22+language%3Aclojure&type=Code&ref=searchresults

But less than the literal constructor by a factor of ~100:
https://github.com/search?utf8=✓&q=java+util+UUID+language%3Aclojure&type=Code&ref=searchresults (this is a flawed search query, but the best I could do).

Comment by Alex Miller [ 11/May/16 3:56 PM ]

I guess my greater point is: rather than consider just the functions uuid/random-uuid, let's consider the problem to be: how can we add portable uuid support in Clojure/ClojureScript? That's a lot more work, but a lot more valuable in my opinion.

So would also want to consider (some of these exist already, but may not have been tested for portability):

  • construction
  • printing - print, pr, pretty print
  • reading
  • hash code
  • conversion to/from bits
  • conversion to/from string
  • extraction of components

And then I think it's worth considering how much of this should be in core vs in a data.uuid or something.

I think it's probably better to work it off a design page than here (this ticket is but one unit of the greater problem). Perhaps http://dev.clojure.org/pages/viewpage.action?pageId=950382 could suggest some pointers.





[CLJ-1921] Wrong numeric result from Math/abs on Java 8 Created: 09/May/16  Updated: 10/May/16

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

Type: Defect Priority: Major
Reporter: Alex Miller Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: math, reflection
Environment:

does not seem specific to Clojure version
occurs only in Java 1.8



 Description   

This is with Java 1.8 (Oracle or Open JDK):

weird-abs.core=> (Math/abs -10000000000000)
10000000000000
weird-abs.core=> (def a    -10000000000000)
#'weird-abs.core/a
weird-abs.core=> (Math/abs a)
1316134912

In Java 1.7, the expected results are returned instead (10000000000000).

Cause: It appears that Math.abs(int) is being invoked. Both the int and long versions are considered by the reflector but Java 1.7 and 1.8 return these signatures in different orders and the first one found is picked.

Workaround: Use hint or cast to inform the reflector which one to pick.



 Comments   
Comment by Alex Miller [ 09/May/16 9:03 AM ]

In the first case, -10000000000000 is a long and the compiler unambiguously finds Math.abs(long).

In the second case, a is an Object and all abs signatures are considered (this is in Reflector.invokeMatchingMethod). In both Java 1.7 and 1.8, the long and int signatures are found "congruent".

In Java 1.7, the long version is found first and treated as a match, then int is checked and Compiler.subsumes([int], [long]) returns false, leading to the long method being kept as the match.

In Java 1.8, the int version is found first and treated as a match, then long is checked and Compiler.subsumes([long], [int]) returns false, leading to the int method being kept as the match.

Both of these return false on both JDKs:

(Compiler/subsumes (into-array [Long/TYPE]) (into-array [Integer/TYPE]))
(Compiler/subsumes (into-array [Integer/TYPE]) (into-array [Long/TYPE]))

so the real difference is just the ordering that is considered, which is JDK-specific.

The considered signatures could be sorted in some canonical way making this behavior consistent, or could maybe express a preference between the two signatures somehow.

In any case, getting rid of the reflection here by hinting or casting a resolves the problem - it should be considered only luck not intention that the correct results comes out with Java 7 in this case, I think.

Comment by Nicola Mometto [ 10/May/16 7:58 AM ]

It seems to me that the non-deterministic behaviour of clojure's reflector of randomly picking one overload when more than one is available is both highly counterintuitive and undesirable.

IMHO the only sane approach would be to:

  • pick the most specific type if possible (e.g. if what's available is [Object, CharSequence, String] and the reflected type is a StringBuffer, we use CharSequence rather than Object.
  • pick the widest primitive type if possible (e.g. in this case we'd use long rather than int)
  • Fail with a `More than one matching method found` exception if conflicts can't be resolved (this already happens in some cases)

(I'm still scarred from previous experiences of reading/patching the complex beast that is Reflector.java and the reflective bits of Compiler.java, so I propose this with no intention of ever working on this myself )

Comment by Alex Miller [ 10/May/16 8:03 AM ]

I think the subsumes check is effectively trying to do your option #1 already - this is a case where the types of the arguments in the two cases have no hierarchical relationship. Probably #2 would make more sense - expressing a preference, although there are certainly cases where "widest" has no meaning, so not sure what the general form of this is.

Comment by Nicola Mometto [ 10/May/16 8:05 AM ]

To clarify, that wasn't a list of different options, it was a list of steps to take.
i.e. if it's possible to pick the most specific type from a hierarchy, do that, ELSE if the types are primitive, pick the widest ELSE fail





[CLJ-1920] Create an easy way to gracefully shutdown agents Created: 03/May/16  Updated: 03/May/16

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

Type: Enhancement Priority: Major
Reporter: Ruslan Al-Fakikh Assignee: Ruslan Al-Fakikh
Resolution: Unresolved Votes: 1
Labels: agents


 Description   

Currently there is no easy way to shutdown agents while making sure all the submitted actions are completed and no new actions are sent.

Here is the naive approach:

(shutdown-agents)

There are two problems with that:
1) It will discard all the actions that have already been submitted, but haven't been started.
2) It won't prohibit from sending further actions to agents (no explicit error will be thrown, just silent ignoring).

Here is the proof:

(def my-agent (agent 1))

(defn sleep-and-inc [number]
  (Thread/sleep 3000)
  (println "action number" number "complete")
  (inc number))

(println "sending off 2 times")
(send-off my-agent sleep-and-inc)
(send-off my-agent sleep-and-inc)
(println "sending off complete")

(shutdown-agents)
(println "shutdown requested")

(println "sending off a 3rd time")
(send-off my-agent sleep-and-inc)
(println "sending off complete")

Here is the output:

sending off 2 times
sending off complete
shutdown requested
sending off a 3rd time
sending off complete
action number 1 complete

As you can see - the 2nd action got discarded, the 3rd action got ignored.

And btw, the shutdown-agents' docstring is misleading (not clear):
"...Running actions will complete, but no new actions will be accepted"
1) It doesn't say anything about already submitted actions
2) "no new actions will be accepted" sounds like there should be an error, but it's silently ignored.
So, the better docstring should be "...Running actions will complete, waiting actions will be discarded and new actions will be ignored"

A similar naive approach works perfectly well in Java:

ExecutorService executor = Executors.newSingleThreadExecutor();

        executor.submit(new Runnable() {
            @Override
            public void run() {
                try {
                    Thread.sleep(3000);
                } catch (InterruptedException e) {
                    throw new RuntimeException(e);
                }
                System.out.println("Action 1 complete");
            }
        });
        executor.submit(new Runnable() {
            @Override
            public void run() {
                try {
                    Thread.sleep(3000);
                } catch (InterruptedException e) {
                    throw new RuntimeException(e);
                }
                System.out.println("Action 2 complete");
            }
        });

        executor.shutdown();
        System.out.println("Shutdown requested");

//        //will throw RejectedExecutionException
//        executor.submit(new Runnable() {
//            @Override
//            public void run() {
//                try {
//                    Thread.sleep(3000);
//                } catch (InterruptedException e) {
//                    throw new RuntimeException(e);
//                }
//                System.out.println("Action 3 complete");
//            }
//        });

Output:

Shutdown requested
Action 1 complete
Action 2 complete

By "perfectly well" I mean:
1) It will complete all the waiting tasks (not just running)
2) It will throw an error on a new task after "shutdown" was called.

So, back to Clojure - currently we are only left with this idiom (not trivial!):

(await my-agent)
(shutdown-agents)

It is not a trivial and straightforward idiom, because:
1) You need to keep track of all the agents in the system. Becomes close to impossible if you are dealing with third-party code that uses agents.
2) Still doesn't even throw an exception if you happen to send another action while waiting or shutting down.

Proposal
(inspired by Java):
1) Create a new function called "shutdown-agents-gracefully" which will do 2 additional things:
1.1) Put the agents system to "shutting down" state
1.2) Completes the running actions as well as the waiting actions
2) Modify "send" and "send-off" so that they throw an error in case the agent system is in "shutting down" state.
3) Fix the docstring of "shutdown-agents" (see above)

I'll start developing a patch when this jira ticket is validated.






[CLJ-1916] AOT compilation sometimes results in extra classes for already compiled namespaces Created: 19/Apr/16  Updated: 19/Apr/16

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

Type: Defect Priority: Minor
Reporter: Mike Kaplinskiy Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: aot


 Description   

Case-in-point: clojure/tools.logging.

Repro:

  • AOT compile all the namespaces in clojure/tools.logging (clojure.tools.logging & clojure.tools.logging.impl)
  • With the result on the classpath, AOT compile clojure/java.data (clojure.java.data)
  • Observe `clojure/tools/logging$eval32$fn__33.class` in the output of the second compile (make sure to have different output directories for the two compiles).

This is normally harmless, but becomes an issue if you try to cache AOT compilation output. When you try to cache previous AOT runs this way, you sometimes end up with two otherwise unrelated namespaces generating the same filename. If these had the same contents that would be fine, but there's no guarantee that they have the same contents (since 32 & 33 there are just (gensym)s). Depending on which one "wins" in a classpath this could end badly.

I'm not an expert here, but it would be nice if these "extras" were either generated as part of tools.logging or were somehow aliased into the namespace they were compiled from (e.g. clojure/java/data/$clojure$tools$logging$eval32$fn__33.class or clojure/tools/logging/$clojure$java$data$eval32$fn_33.class).



 Comments   
Comment by Kevin Downey [ 19/Apr/16 6:42 PM ]

tools.logging uses eval to generate some code only when certain classes are present on the classpath, eval generates class files, when you have aot compiling turned on, those class files will be output to the filesystem.

the reason the name is the way it is, is because the eval happens when the tools.logging namespace is loading, so the value of the ns var is the tools.logging namespace, which is what the compiler is generating the name from.





[CLJ-1913] core.reducers wrong documentation Created: 14/Apr/16  Updated: 14/Apr/16

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

Type: Defect Priority: Trivial
Reporter: Camilo Roca Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring, reducers


 Description   

Two issues regarding the documentation of core.reducers

  • There is a contradiction between the documentation mentioned in http://clojure.github.io/clojure/clojure.core-api.html#clojure.core.reducers/fold, with respect to the one mentioned here http://clojure.org/reference/reducers. Specifically on the line that states "(with a seed value obtained by calling (combinef) with no arguments)" on the former and "The reducef function will be called with no arguments to produce an identity value in each partition." on the later. Those two documentation references are contradictory. Either combinef is called with no arguments or reducef is called with no arguments.
  • The second doc issue is regarding the arities of most functions in core.reducers. With the introduction of transducers in Clojure 1.7. The single arity in functions like r/map or r/filter gives the impression that they return a transducer, whereas they just return a curried version of them. Nothing in the docstrings or the reference page mentions what is the return value of those functions with a single argument.





[CLJ-1912] Optimized version of the '<' and '>' functions for arties larger than 2 Created: 08/Apr/16  Updated: 08/Apr/16

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

Type: Enhancement Priority: Minor
Reporter: Anton Harald Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: performance


 Description   

When looking at the code of the build-in functions '<' and '>', I was wondering, why (next more) is invoked twice in each comparison of two neighboring arguments.

Here is the original code of e.g. <

(defn <
  "Returns non-nil if nums are in monotonically increasing order,
  otherwise false."
  {:inline (fn [x y] `(. clojure.lang.Numbers (lt ~x ~y)))
   :inline-arities #{2}
   :added "1.0"}
  ([x] true)
  ([x y] (. clojure.lang.Numbers (lt x y)))
  ([x y & more]
   (if (< x y)
     (if (next more)
       (recur y (first more) (next more))
       (< y (first more)))
     false)))

Here is a possible replacement for the n-arity part of the function:

([x y & more]
   (if (< x y)
     (if-let [n (next more)]
       (recur y (first more) n)
       (< y (first more)))
     false))

Now, (next more) would be computed only once per 'iteration'. On my machine, the modified version had 7% better performance. Of course, this only shows up when invoked with more than 2 arguments. e.g.: (apply < (range 100000...))

I'd be curious to hear, if there was a particular reason for taking this decision in the built-in function.



 Comments   
Comment by Alex Miller [ 08/Apr/16 4:23 PM ]

I don't think there is a particular reason, feel free to make a patch.





[CLJ-1906] Clojure should make representing iterated api calls easier Created: 30/Mar/16  Updated: 06/Jun/16

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

Type: Enhancement Priority: Minor
Reporter: Kevin Downey Assignee: Unassigned
Resolution: Unresolved Votes: 3
Labels: None

Attachments: Text File 0001-CLJ-1906-add-ingeminate-function.patch     Text File 0001-CLJ-1906-add-unfold-function.patch     Text File 0001-CLJ-1906-transducer-enabled-iterate.patch     File CLJ-1906-seqable-reducible.diff     Text File CLJ-1906-successions.patch    

 Description   

Many apis (elasticsearch, github, s3, etc) have parts of the api
which, in usage, end up being used in an interative way. You make an
api call, and you use the result to make another api call, and so
on. This most often shows up in apis have some concept of pages of
results that you page through, and is very prevalent in http apis.

This appears to be such a common pattern that it would be great if
Clojure had in built support for it.

You may think Clojure already does have support for it, after all,
Clojure has `iterate`. In fact the docstring for `iterate`
specifically says the function you give it must be free of side
effects.

I propose adding a function `unfold` to clojure.core to support this
use case. `unfold` would return an implementation of ReduceInit. The
name `unfold` matches what would be a similar Haskell function
(https://hackage.haskell.org/package/base-4.8.2.0/docs/Data-List.html#v:unfoldr)
and also matches the name for a similar function used in some existing
Clojure libraries
(https://github.com/amalloy/useful/blob/develop/src/flatland/useful/seq.clj#L128-L147).

`unfold` in some ways looks like a combination of `take-while` and
`iterate`, except for the fact that `iterate` requires a pure
function. Another possible solution would be a version of `iterate`
that doesn't require a pure function.

It seems like given the use case I envision for `unfold`, a
non-caching reducible would be perfect. But that would leave those
that prefer seqs high and dry, so maybe at least some consideration
should be given to seqs.

Mailing list discussion is here
(https://groups.google.com/forum/#!topic/clojure-dev/89RNvkLdYc4)

A sort of dummy api you might want to interact with would look something like

(import '(java.util UUID))

(def uuids (repeatedly 1000 #(UUID/randomUUID)))

(def uuid-index
  (loop [uuids uuids
         index  {}]
    (if (seq uuids)
      (recur (rest uuids) (assoc index (first uuids) (rest uuids)))
      index)))

(defn api
  "pages through uuids, 10 at a time. a list-from of :start starts the listing"
  [list-from]
  (let [page (take 10 (if (= :start list-from)
                        uuids
                        (get uuid-index list-from)))]
    {:page page
     :next (last page)}))

given the above api, if you had an implementation of `unfold` that took a predicate that decided when to continue unfolding, a producer which given a value in a sequence produced the next value, and an initial value, you could do something like this:

(= uuids (into [] (mapcat :page) (unfold :next (comp api :next) (api :start))))

and the result would be true.

The equivilant take-while + iterate would be something like:

;; the halting condition is not strictly the same
(= uuids (into [] (mapcat :page) (take-while (comp seq :page) (iterate (comp api :next) (api :start)))))


 Comments   
Comment by Kevin Downey [ 31/Mar/16 4:21 PM ]

I made two patches, one adds unfold as discussed above, one adds ingeminate which is like iterate but without the function purity restrictions, and doesn't return a seq.

Comment by Ghadi Shayban [ 11/Apr/16 10:46 AM ]

Though syntax is less important than the semantics, may I propose the name `progression` for this? Clojure's fold is called reduce, so unfold is too much like Haskell. Other names I was considering include evolve & derivations.

Comment by Alex Miller [ 11/Apr/16 11:23 AM ]

Another option would be `productions` (reminiscent of `reductions`).

Comment by Kevin Downey [ 11/Apr/16 9:32 PM ]

productions has a nice ring to it. emanate could work too, would sort near eduction

Comment by Ghadi Shayban [ 12/Apr/16 10:08 PM ]

Adding a patch with a generator impl that is clojure.lang.{Seqable,IReduceInit}.

Generative tests assert that the seq and reduce halves are equivalent.

Tests assert basic functionality, obeying reduced, and maximal laziness of the seq impl.

Docstring has been wordsmithed and the function named `productions`.

Comment by Kevin Downey [ 18/Apr/16 3:21 PM ]

apparently unfold is part of SRFI 1: List Library in scheme land http://srfi.schemers.org/srfi-1/srfi-1.html#FoldUnfoldMap

it looks like their unfold is take-while + iterate + map

Comment by Ghadi Shayban [ 23/Apr/16 11:06 PM ]

Main differences between Scheme's impl and this proposed one:
Predicate reversed (stop? vs continue?)
Scheme has a "mapping function" to produce a different value from the current seed, Clojure doesn't (but has transducers)
Scheme has an extra optional arg to build the tail of the list

Now I'm partial to the name successions.

Comment by Michał Marczyk [ 10/May/16 11:07 AM ]

I can confirm that I found unfold quite useful in my Scheme days.

In Clojure, this general pattern can be expressed using transducers at a modest cost in keystrokes:

(def numbers (doall (range 1000)))

(defn api [list-from]
  (if list-from
    (let [page (vec
                 (take 10 (if (= :start list-from)
                            numbers
                            (drop list-from numbers))))]
      {:page page
       :next (some-> (last page) inc)})))

(= numbers
   (sequence (comp (take-while some?)
                   (mapcat :page))
             (iterate (comp api :next)
                      (api :start))))
;= true

Maybe this could be simplified with an xform-enabled version of iterate?

(defn iterate*
  ([f seed]
   (iterate f seed))
  ([xform f seed]
   (sequence xform (iterate f seed))))

(= numbers
   (iterate*
     (comp (take-while some?) (mapcat :page))
     (comp api :next)
     (api :start)))
;= true

Admittedly this takes more characters, but is quite generic and a transducer-enabled overload in iterate feels pretty natural to me. Attaching a simple patch implementing this in clojure.core/iterate – I'll look at clojure.lang.Iterate to see if it's worth implementing direct support later, unless of course nobody wants this.

Comment by Michał Marczyk [ 10/May/16 11:08 AM ]

0001-CLJ-1906-transducer-enabled-iterate.patch adds a ternary overload to iterate that delegates to the binary overload and sequence.

Comment by Ghadi Shayban [ 10/May/16 12:56 PM ]

A few unsatisfactory things about overloading {iterate}
1) iterate's docstring says {f must be free of side-effects}
2) There is boilerplate and subtlety around the terminating item. In this case the final API call is made unconditionally, leading to an extra empty/marker item that is filtered by take-while. With the current proposal, the predicate controls iteration from the inside out

Comment by Ghadi Shayban [ 06/Jun/16 8:40 AM ]

updated patch to apply cleanly to core





[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-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: 1
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-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-1876] calling require from java is not thread safe Created: 07/Jan/16  Updated: 17/Feb/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.



 Comments   
Comment by Kevin Downey [ 17/Feb/16 10:19 AM ]

calling require from clojure isn't thread safe either, no different from this





[CLJ-1874] Var redefinition breaks in AOT-compiled code Created: 05/Jan/16  Updated: 18/Feb/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: 1
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.

Comment by Kevin Downey [ 17/Feb/16 10:21 AM ]

was this fixed by clj-1604?

Comment by Nicola Mometto [ 18/Feb/16 8:38 AM ]

No, CLJ-1604 only deals with clojure.core Vars, the patch attached to this ticket is an extension on top of the patch committed for CLJ-1604 that deals with other namespaces





[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-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-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: 25/Feb/16

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: 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-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: 09/Feb/16

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

Attachments: File CLJ_1857_split_docs_update.diff    

 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.

Comment by Stephen Hopper [ 09/Feb/16 10:00 PM ]

The docs are a bit ambiguous as "splits" is a verb in the first sentence, but a noun in the other two occurrences. I believe the ClojureDocs example being referred to is likely this one (mostly because of the comment in it):

; Note that the 'limit' arg is the maximum number of strings to
; return (not the number of splits)
user=> (str/split "q1w2e3r4t5y6u7i8o9p0" #"\d+" 5)
["q" "w" "e" "r" "t5y6u7i8o9p0"]

Because split is the name of the function and the action being performed, I think it makes sense to leave it as the verb in the first sentence and replace the other two occurrences with "parts". Does that sound reasonable?





[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-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-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-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-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-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-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: 01/Apr/16

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.

Comment by Mike Anderson [ 31/Mar/16 7:53 PM ]

@Nicola Mometto : I believe the JVM does in fact support return type overloading:

"Note that there may be more than one matching method in a class because while the Java language forbids a class to declare multiple methods with the same signature but different return types, the Java virtual machine does not. This increased flexibility in the virtual machine can be used to implement various language features. For example, covariant returns can be implemented with bridge methods; the bridge method and the method being overridden would have the same signature but different return types."

See : http://docs.oracle.com/javase/8/docs/api/java/lang/Class.html#getMethod-java.lang.String-java.lang.Class...-

Comment by Nicola Mometto [ 01/Apr/16 3:55 AM ]

Ah, yes of course, thanks.





[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-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-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-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: 1
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-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-1777] Add function version of vswap! Created: 13/Jul/15  Updated: 19/Mar/16

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

Type: Enhancement Priority: Major
Reporter: Nicola Mometto Assignee: Unassigned
Resolution: Unresolved Votes: 3
Labels: None

Attachments: Text File 0001-CLJ-1777-Change-vswap-from-a-macro-to-an-inlineable-.patch     Text File 0001-CLJ-1777-Change-vswap-from-a-macro-to-an-inlineable-v2.patch    
Patch: Code

 Description   

Volatiles and vswap! were added in 1.7 as performant mechanisms to achieve uncoordinated mutation to the language.
given the fact that their addition was a performance-centric one, vswap! was implemented as a macro rather than a normal function to avoid runtime dereference of the function var and the optional apply overhead in case of multiple args.

However this:
-is not necessary
-breaks the api parallelism between volatile/atom swap!/vswap! reset!/vreset!
-makes impossible certain use cases (vswaps! in update-in forms)
-is potentially confusing given that swap! is a function

Infact the macro can be replaced with a function with :inline metadata.
This is a strictly additive change that will make so that for all the current valid usages of vswap! nothing will change, it will still be macroexpanded by the inliner and additionally since it is now a function it can be used in HOF contexts where it's not unusual to see swap! used.



 Comments   
Comment by Alex Miller [ 13/Jul/15 8:07 PM ]

Nicola, please don't set the fix version on tickets.

Comment by Nicola Mometto [ 13/Jul/15 8:10 PM ]

Sorry, I mixed the fix version with the affected field

Comment by Greg Chapman [ 09/Mar/16 11:41 AM ]

An additional drawback to the current macro is that it will result in double evaluation of the vol expression:

user=> (macroexpand '(vswap! (return-a-vol) inc))
(. (return-a-vol) reset (inc (.deref (return-a-vol))))
Comment by Alex Miller [ 10/Mar/16 9:22 AM ]

I don't think Rich is interested in expanding the use of inline.

Re Greg's comment, that would be considered if you want to make a second ticket.

Comment by Nicola Mometto [ 10/Mar/16 12:31 PM ]

Not really sure I understand the adversion to using inline – it works perfectly fine and this is the exact use-case for it: a function that we want to inline for performance reason.

I really fail to see the reasoning behind making what should be a function, a macro, just for the sake of not using inline.

Comment by Phill Wolf [ 19/Mar/16 8:59 AM ]

Adding :inline to existing functions may be deplored as an unprincpled, sloppy, desperate, tragic compromise, and a slippery slope. But vswap! is the opposite case: The macro is what already exists, somewhat idiosyncratically. Pairing it with a function would make it more wholesome, not less.





[CLJ-1776] Test that collections are valid statements Created: 08/Jul/15  Updated: 08/Jul/15

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

Type: Enhancement Priority: Minor
Reporter: Michael Blume Assignee: Michael Blume
Resolution: Unresolved Votes: 0
Labels: compiler, test

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

 Description   

It's possible to break the compiler such that vectors are emitted incorrectly when they're in statement position (I accidentally did this). This doesn't break any part of the Clojure test suite, but does break valid Clojure code (for me it hit taoensso's encore). Add tests to the test suite so defects of this kind are caught.






[CLJ-1775] Add any? Created: 07/Jul/15  Updated: 09/Jul/15

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

Type: Enhancement Priority: Minor
Reporter: Joshua Griffith Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Given the presence of `every?` and `not-every?`, it seems that `any?` should also exist given `not-any?`. While similar to `some`, indeed `(def any? (comp boolean some))`, it would make the interface consistent. Also, having `any?` would be nice when writing typed Clojure, where one often wants a boolean.



 Comments   
Comment by Colin Taylor [ 09/Jul/15 5:34 PM ]

Previous discussion - https://groups.google.com/d/msg/clojure/02msnrXJsSg/BdgfoeyVX7sJ.
My bad example notwithstanding, I still think this is symmetric and useful for interop. The typed Clojure point is valid too..





[CLJ-1774] Field access on typed record does not preserve type Created: 02/Jul/15  Updated: 03/Jul/15

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

Type: Enhancement Priority: Major
Reporter: Michael Blume Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: defrecord, reflection, typehints


 Description   
(ns field-test.core
  (:import [java.util UUID]))

(defrecord UUIDWrapper [^UUID uuid])

(defn unwrap [^UUIDWrapper w]
  (.-uuid w)) ; <- No reflection

(defn get-lower-bits [^UUIDWrapper w]
  (-> w .-uuid .getLeastSignificantBits)) ; <- Reflection :(

The compiler seems to have all the information it needs, but lein check prints

Reflection warning, field_test/core.clj:10:3 - reference to field getLeastSignificantBits on java.lang.Object can't be resolved.

(test case also at https://github.com/MichaelBlume/field-test)



 Comments   
Comment by Alex Miller [ 02/Jul/15 4:31 PM ]

afaik, that ^UUID type hint on the record field doesn't do anything. The record field will be of type Object (only ^double and ^long affect the actual field type).

Perhaps more importantly, it is bad form to use Java interop to retrieve the field values of a record. Keyword access for that is preferred.

Comment by Nicola Mometto [ 03/Jul/15 4:48 AM ]

The same issue applies for deftypes where keyword access is not an option

Comment by Alex Miller [ 03/Jul/15 12:17 PM ]

Per http://clojure.org/datatypes: "You should always program to protocols or interfaces -
datatypes cannot expose methods not in their protocols or interfaces"

Along those lines, usually for deftypes, I gen an interface with the proper types if necessary, then have the deftype implement the interface to expose the field.

Also per http://clojure.org/datatypes:

"note that currently a type hint of a non-primitive type will not be used to constrain the field type nor the constructor arg, but will be used to optimize its use in the class methods" - that is, inside a method implemented on the record/type, referring to the field should have the right hint. So in the example above, if unwrap was an interface or protocol implementation method on the record, and you referred to the field, you should expect the hint to be utilized in that scenario.

So, my contention would be that all of the behavior described in this ticket should be expected based on the design, which is why I've reclassified this as an enhancement, not a defect.





[CLJ-1767] Documentation Issues with sort Created: 22/Jun/15  Updated: 22/Jun/15

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

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


 Description   

The documentation for sort seems to be incomplete:

  • sort can return nil in some situations. There is a discussion in CTYP-228 with more backstory. There is a repro case here: http://sprunge.us/VIFc?clj supplied by Nicola Mometto. The doc string states that sort "Returns a sorted sequence of the items in coll.", which does not indicate that sort can return nil.
  • It is stated that the "comparator must implement java.util.Comparator.", but this is not true - the comparator can be any IFn that can accept 2 arguments and return either a Boolean or a Number.

For the first issue (nil return), changing the implementation to never return nil might be the neatest fix. For the second issue, the docstring could reference a description of how what functions are valid comparison functions, which can be referenced by other functions that also accept comparators (e.g., sort-by, sorted-set-by, sorted-map-by).



 Comments   
Comment by Alex Miller [ 22/Jun/15 7:51 AM ]

The first issue is being covered by CLJ-1763, so I would remove it from the ticket.

The second is technically true - Arrays.sort() will invoked and it takes a Comparator. The tricky bit is that AFn base class implements Comparator so all function implementations that extend from it that support a 2-arg function satisfy this constraint. But it might be helpful to call that out better.





[CLJ-1758] xf overload for vec and set Created: 17/Jun/15  Updated: 17/Jun/15

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

Type: Enhancement Priority: Major
Reporter: Leon Grapenthin Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Having (vec xf coll) and (set xf coll) overloads seem useful as opposed to writing (into [] ...).

One might also consider these as variadic overloads, like the sequence function has. I am unsure about that since into doesn't have one and I know too little about multiple input transducers.






[CLJ-1754] Destructuring with :merge Created: 16/Jun/15  Updated: 11/Jan/16

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

Type: Enhancement Priority: Major
Reporter: Henrik Heine Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: destructuring


 Description   

Destructuring with :or {...} does not add those defaults to :as binding. Destructuring with :merge adds this.
Usefull when you're wrapping calls to functions and want to add defaults that your callers need not pass in.

(defn foo [& {:merge {:c "C" :d "D"}
:as opt-args}]
opt-args)

should behave like:

(defn foo [& {:keys [c d]
:or {c "C" d "D"}
:as opt-args}]
(let [opt-args (merge {:c c :d d} opt-args)]
opt-args))

Options:
(a) the bindings for c and d in the example may be usefull or not. For the :merge example above they are not needed.
(b) :merge could use keywords or symbols. keywords make it look like the (merge) and symbols make it look like :keys/:or.

Suggestion: using symbols will build a binding for those names and using keywords will not. So users can get the bindings if they need them.

see also https://groups.google.com/forum/#!folder/Clojure$20Stuff/clojure/gG6Tzssn9Nw



 Comments   
Comment by Alex Miller [ 16/Jun/15 7:14 AM ]

Destructuring is about extracting parts of a composite input value. This seems to go a step beyond that into transformation of the input value. Can't say I am a fan of that but I will leave it open.





[CLJ-1748] Change clojure.core/reverse to return rseq for args that are Reversible Created: 07/Jun/15  Updated: 07/Jun/15

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

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


 Description   

There may be issues with this suggestion about concrete types of return values, or doc strings that promise things that you want to preserve that cannot be preserved with this suggested change.

However, if those issues are not show stoppers, changing clojure.core/reverse to check if its arg is Reversible, and if so, return rseq, else do as reverse does today, could be faster in many situations.






[CLJ-1747] Eduction's print-method expects its collection to be Iterable/Sequential Created: 07/Jun/15  Updated: 07/Jun/15

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

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

Attachments: Text File CLJ-1747-eduction-print.patch    
Patch: Code

 Description   

eduction expects its source collection to be Iterable [1], and its print-method goes through print-sequential [2]. This implies a promise that may restrict the use-case of an eduction over a virtual collection, e.g. an IReduceInit source that may be backed by I/O or some other resource. I have found it useful to construct these I/O reducibles and wrap them with an eduction. If you are careful not to call seq on it, this works fine. However, printing it at the REPL will call seq. Does the print-method impl for eduction promise too much? This is a only minor annoyance more than anything else, obviously I could create my own flavor of eduction.

Totally hypothetical example:

(defn database-index
  [name]
  (reify clojure.lang.IReduceInit
    (reduce [_ f init]
      (with-open [rdr (fressian/create-reader (io/input-stream name))]
       (loop [] ...reduce impl...)))))

(eduction (filter (as-of #inst "2012-01-01")) (database-index "eavt.fress"))
;; ^ throws when printed by repl

[1] https://github.com/clojure/clojure/blob/master/src/clj/clojure/core.clj#L7336-L7338
[2] https://github.com/clojure/clojure/blob/master/src/clj/clojure/core.clj#L7360

Proposed Approach:
Let eduction print like an opaque #object






[CLJ-1746] new keyword for `require` that both refers other namespace's symbol and exclude the same in clojure.core Created: 06/Jun/15  Updated: 09/Jun/15

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

Type: Enhancement Priority: Minor
Reporter: Hoang Minh Thang Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: enhancement


 Description   

I find myself repeat code like this

(ns foo.bar
(:refer-clojure :exclude [doseq])
(:require [clojure.core.typed :refer [doseq]]))

and just think why not something like:

(ns foo.bar
(:require [clojure.core.typed :override [doseq]]))



 Comments   
Comment by Mike Anderson [ 09/Jun/15 10:40 AM ]

I agree this is very annoying.

I think it is a duplicate of my issue though: The patch for CLJ-1257 would make this unnecessary (it would allow the user to override any vars, without getting an exception).





[CLJ-1734] Display more descriptive error message when trying to use reader conditionals in a non-cljc file Created: 19/May/15  Updated: 20/May/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: 1
Labels: errormsgs, reader


 Description   

I spent a few puzzled minutes trying to understand the following message from the Clojure compiler:

CompilerException java.lang.RuntimeException: Conditional read not allowed, compiling: <filename>

Eventually I realised it was because I was trying to use reader conditionals in a .clj file that I hadn't renamed to cljc. I think it would be really helpful for people working in mixed clj and cljc codebases to have this error message extended to something like:

"Conditional read not allowed because file does not have extension .cljc"



 Comments   
Comment by Alex Miller [ 19/May/15 11:45 PM ]

The reader doesn't know this - it can be called in multiple ways (from repl, via clojure.core/read, via clojure.core/read-string, load/compile .cljc, load/compile .clj) so that description would actually be wrong in some of those. It seems like you're getting a pretty good error message already - it told you the problem and gave you the file name.

The message could be tweaked to something like "Reader conditionals not allowed in this context" which might give you a better clue.

Comment by Daniel Compton [ 20/May/15 3:12 PM ]

Perhaps I'm not understanding how the reader determines whether reader conditionals are allowed or not, but those would all seem to have different reasons for not being allowed and would be caught by different checks. Each of these checks could give a more specific warning explaining why the read wasn't allowed?

Counteracting my point, it looks like there is only one place where this exception is thrown - https://github.com/clojure/clojure/blob/7b9c61d83304ff9d5f9feddecf23e620c0b33c6e/src/jvm/clojure/lang/LispReader.java#L1406. I'm not sure if this could be extended to give more details in different error cases or if that information isn't available at that point?

Comment by Alex Miller [ 20/May/15 3:36 PM ]

The reader is invoked with an options map which will (or will not) have {:read-cond :allow} or {:read-cond :preserve}. That's the only info the reader has - if either of those is set and a reader conditional is encountered, it throws.

The compiler decides how to initialize these options when it's calling the reader. Users of read and read-string similarly decide which options are allowed when they call it. It would be possible to pass more info into the reader or to catch and rethrow in the compiler where more context is known, but both of those complicate the code for what is already a decent error imho.

Comment by Daniel Compton [ 20/May/15 4:03 PM ]

I agree it is a reasonable error message, I guess we can wait and see how other people find it once 1.7 is released. If it turns out to be an issue for lots of other people then we could revisit this then?





[CLJ-1729] Make Counted and count() return long instead of integer Created: 12/May/15  Updated: 08/Jul/15

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

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


 Description   

Currently count() returns an int - should bump that up to a long.

On long overflow, count() should throw ArithmeticException. Also see CLJ-1229.



 Comments   
Comment by Erik Assum [ 07/Jul/15 9:24 AM ]

Looking at this, there are some problems like in
clojure.lang.RT#toArray line 1658
where you create a new Object array based on the count of a collection.
It seems as if new Object[] takes an int as a param, so one would have to downcast the long to an int for this to work.

Comment by Alex Miller [ 07/Jul/15 9:39 AM ]

If you're creating an Object[] greater than 2147483647, you may have other problems.

But yes, this ticket definitely needs a more thorough analysis as to what is affected. In this case, I think if the count is <= Integer/MAX_VALUE, then it should proceed and otherwise should throw an exception.

Comment by Erik Assum [ 08/Jul/15 8:19 AM ]

hmmm, this also causes problems wrt java.util.Collection size:
clojure.lang.APersistentSet#size line 164
Where size is specified by

http://docs.oracle.com/javase/7/docs/api/java/util/Collection.html#size()





[CLJ-1721] Enable test case for char? Created: 03/May/15  Updated: 03/May/15

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

Type: Enhancement Priority: Major
Reporter: Brandon Bloom Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: patch

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

 Description   

clojure.core/char? already exists, but there was no test for it (despite a comment suggesting one).






[CLJ-1720] Add clojure.core/pattern? predicate Created: 03/May/15  Updated: 03/May/15

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

Type: Enhancement Priority: Major
Reporter: Brandon Bloom Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: patch

Attachments: Text File CLJ-1720_v01.patch     Text File CLJ-1720_v02.patch    
Patch: Code and Test

 Description   

Just like http://dev.clojure.org/jira/browse/CLJ-1719 , this helps with clj/cljs compatibility.



 Comments   
Comment by Brandon Bloom [ 03/May/15 2:37 PM ]

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

Comment by Brandon Bloom [ 03/May/15 2:48 PM ]

Whoops, uploaded wrong patch. Tests actually pass in this v02 patch.





[CLJ-1715] Use AFn.applyToHelper rather than IFn.applyTo in InvokeExpr.eval Created: 23/Apr/15  Updated: 23/Apr/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: 0
Labels: None

Attachments: Text File 0001-CLJ-1715-Use-AFn.applyToHelper-rather-than-IFn.apply.patch    
Patch: Code

 Description   

Because of implementation details of how def forms are compiled, invokations in its args are evaluated using applyTo rather than directly using the invoke method. This forces IFn implementors to implement applyTo even when not necessary.

The proposed patch changes InvokeExpr.eval to use AFn.applyToHelper, so that invoke rather than applyTo is used when possible.

Example of currently failing code that will work with patch:

user=> (deftype x [] clojure.lang.IFn (invoke [_] 1))
user.x
user=> (def a ((x.)))
AbstractMethodError   clojure.lang.Compiler$InvokeExpr.eval (Compiler.java:3553)





[CLJ-1707] conditional form is not consumed when :read-allow is falsey Created: 15/Apr/15  Updated: 15/Apr/15

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

Type: Defect Priority: Major
Reporter: Nicola Mometto Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: reader


 Description   
user=> (def a (java.io.PushbackReader. (java.io.StringReader. "#?(:clj [1 2])")))
#'user/a
user=> (read a)
RuntimeException Conditional read not allowed  clojure.lang.Util.runtimeException (Util.java:221)
user=> (read a)
(:clj [1 2])

the expected result would be an EOF exception on the second read.






[CLJ-1704] Clarify cond documentation to explain about using :else Created: 14/Apr/15  Updated: 14/Apr/15

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

Type: Enhancement Priority: Minor
Reporter: Daniel Compton Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring


 Description   

The documentation for cond doesn't explicitly mention that you can use :else (or any other keyword) to catch any values that don't match the previous conditions. While it is true that the documentation does say that a test will evaluate and return the value of logical true, it could be more helpful by pointing out that a keyword like :else will always be logical true.

I'm not 100% sure about whether this is necessary, but wanted to see what others thought and whether it would be helpful or not.






[CLJ-1702] Silent fail on unspecified map destructuring Created: 13/Apr/15  Updated: 16/Apr/15

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

Type: Enhancement Priority: Trivial
Reporter: Linus Ericsson Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: destructuring


 Description   

When accidentally switching keyword and (previously undefined) symbol in map destructuring, an error is correctly thrown:

(let [{:b b} {:b 1}] b)

=> CompilerException java.lang.RuntimeException: Unable to resolve symbol: b in this context, compiling: (/tmp/form-init7939480206147277345.clj:1:1)

When the symbol ("a" used below) is defined, however, there is a more subtle error:

(def a 0)
(let [{:a a} {:a 1}] a)
=> nil

Expected: Destructuring should only accept the defined keywords :or, :keys, :as, :strs and :syms as keys in a destructuring map.



 Comments   
Comment by Michael Blume [ 14/Apr/15 1:35 PM ]

This may be a duplicate of CLJ-1613

Comment by Linus Ericsson [ 16/Apr/15 9:13 AM ]

Michael, I do think this is a somewhat different problem.





[CLJ-1701] Serialization of protocol methods broken: java.io.NotSerializableException: clojure.lang.MethodImplCache Created: 13/Apr/15  Updated: 20/Apr/15

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: Dr. Christian Betz Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

With this test, you can see that we cannot serialize methods from protocols (i.e. time-from-tweet), as this results in a java.io.NotSerializableException: clojure.lang.MethodImplCache
at java.io.ObjectOutputStream.writeObject0 (ObjectOutputStream.java:1183)
java.io.ObjectOutputStream.defaultWriteFields (ObjectOutputStream.java:1547)
java.io.ObjectOutputStream.writeSerialData (ObjectOutputStream.java:1508)
java.io.ObjectOutputStream.writeOrdinaryObject (ObjectOutputStream.java:1431)
java.io.ObjectOutputStream.writeObject0 (ObjectOutputStream.java:1177)
java.io.ObjectOutputStream.writeObject (ObjectOutputStream.java:347)
sparkling.protocol_test$serialize.invoke (protocol_test.clj:11)

This is the actual test:

(ns sparkling.protocol-test
(:require [clojure.test :refer :all])
(:import [java.io ObjectInputStream ByteArrayInputStream ObjectOutputStream ByteArrayOutputStream]))

(defn- serialize
"Serializes a single object, returning a byte array."
[v]
(with-open [bout (ByteArrayOutputStream.)
oos (ObjectOutputStream. bout)]
(.writeObject oos v)
(.flush oos)
(.toByteArray bout)))

(defn- deserialize
"Deserializes and returns a single object from the given byte array."
[bytes]
(with-open [ois (-> bytes ByteArrayInputStream. ObjectInputStream.)]
(.readObject ois)))

(defprotocol timestamped
(time-from-tweet [item]))

(defrecord tweet [username tweet timestamp]
timestamped
(time-from-tweet [_]
timestamp
))

(deftest sequable-serialization
(testing "Serialization of function"
(let [item identity]
(is item (-> item serialize deserialize))))

(testing "Serialization of protocol method"
(let [item time-from-tweet]
(is item (-> item serialize deserialize)))))



 Comments   
Comment by Dr. Christian Betz [ 13/Apr/15 6:15 AM ]

BTW: Same is true for multimethods, here the exception is java.io.NotSerializableException: clojure.lang.MultiFn

Comment by Alex Miller [ 13/Apr/15 9:35 AM ]

I don't think we expect functions to be serializable in this way. Both protocols and multimethods effectively have runtime state based on what implementations have extended them. What would it mean to serialize these functions? Would you serialize them with whatever implementations have been loaded at that point? Or with none? Both seem problematic to me. Regular functions are closures and can capture the state of their environment. I think better answers are either AOT or for regular functions, something like the serializable-fn library.

Comment by Dr. Christian Betz [ 13/Apr/15 1:13 PM ]

Hi,

thanks for the comments. First, something to the background: I'm developing Sparkling, a Clojure API to Apache Spark. For distributing code in the cluster it depends on AOT compiled functions, so yes, you cannot simply serialize any function around, it needs to be AOT'd. Serializiation provides us with support for the current bindings etc, and everything works as expected. So, AFunction is serializable for a reason and so are other implementations of AFn/IFn, everything works well.

Regarding the state of protocols and multimethods - I think it's conceptually the same as the state of functions (which function definition, the var might be bound multiple times, etc.), and the closures given in bindings. There's no reason for me as the user of a protocol to believe that the method from the protocol differs from a function. In fact (ifn? protocol-method) also returns true.

serializable-fn, not being intended for over-the-wire serialization in the first place, has problems with collections of functions in bindings of the serializble function, together with an issue with PermGen pollution by creating classes for the same function over and over again in the context of Spark.

I think I'm fine for the moment, as I can wrap the protocol method in a function, but I still believe, that this is a bug.

Regards

Chris

Comment by Dr. Christian Betz [ 20/Apr/15 3:10 AM ]

actually, this is the code snippet from clojure.lang.AFunction causing the pain:

clojure.lang.AFunction.java
public abstract class AFunction extends AFn implements IObj, Comparator, Fn, Serializable {

public volatile MethodImplCache __methodImplCache;

AFunction is serializable, but MethodImplCache is not. I'm not sure if it's enough to mark it as transient, because I did not check where initialization happens.

Comment by Dr. Christian Betz [ 20/Apr/15 3:29 AM ]

My comment per mail got lost in SMTP-nirvana: There's an easy workaround. Wrap the protocol method in a function, that will do the trick at the cost of uglifying your code





[CLJ-1690] Make Range, Repeat and Cycle implement Indexed Created: 31/Mar/15  Updated: 01/Apr/15

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

Type: Enhancement Priority: Major
Reporter: Mike Anderson Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File clj-1690.patch    

 Description   

Currently, Cycle, Range and Repeat do not implement Indexed, which means that "nth" is O( n ) on average.

The proposed change is to implement Indexed for these classes, so that "nth" becomes an O( 1 ) operation.



 Comments   
Comment by Alex Miller [ 31/Mar/15 9:37 PM ]

This is an expansion of capabilities and commitments beyond what these functions have done in the past. We've already committed to more than we really wanted to with them, so I'm not sure we want to add yet more commitments. In any case, we won't do it for 1.7.

FYI, that Range you're patching is not currently used for anything - the current impl uses a chunked seq definition in core.clj. CLJ-1515 will (likely) replace the Range class with an all new impl. In any case, patching Range here probably isn't useful until CLJ-1515 is resolved.

Comment by Mike Anderson [ 31/Mar/15 9:50 PM ]

Understood re 1.7. Though I personally think this is small enough that you could squeeze it in. Your call.

However I still think this approach is useful though: nth is a very common operation and I note you aren't benchmarking it yet in CLJ-1515. Whatever new Range implementation is used will benefit from implementing Indexed.

Comment by Alex Miller [ 31/Mar/15 11:22 PM ]

None of these functions currently promises to return something Indexed. If we add that and people come to rely on it, we can never change the implementation in a way that removes it. So I'm not sure that's a promise we want to make.

Comment by Mike Anderson [ 01/Apr/15 1:39 AM ]

I am not proposing that we make any "promise" of an indexed return value, simply that such classes implement the interface as an implementation detail (where it makes sense).

This then causes the fast path in functions like RT.nth to be taken, so we get O(1) instead of O( n) for the most common indexed lookup cases.

TBH, my assumption was that this was the main purpose of the "Indexed" interface, i.e. to allow concrete collection types to participate in Clojure's indexed access functions with an efficient implementation.

Comment by Ghadi Shayban [ 01/Apr/15 10:25 AM ]

People regularly rely on implementation details, promise or no promise. If clojure were to add Indexed then remove it, people's code would either break or get slower.

Implementation behavior (whether overt or implicit) is necessarily treated as future constraints (shackles), so it is considered carefully.





[CLJ-1688] Object instance members should resolve to Object Created: 30/Mar/15  Updated: 30/Mar/15

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

Type: Enhancement Priority: Minor
Reporter: Michael Blume Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: reflection, typehints


 Description   
(defn unparse-pattern ^String [pattern] (.toString pattern))
Reflection warning, ring/swagger/coerce.clj:22:41 - reference to field toString can't be resolved.

Reflection isn't really necessary here, we could just special-case the methods on Object.






[CLJ-1687] Clojure doesn't resolve static calls even when it has all information needed to do so Created: 30/Mar/15  Updated: 30/Mar/15

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

Type: Enhancement Priority: Minor
Reporter: Michael Blume Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: reflection, typehints


 Description   

If I create a class with two methods, one of which takes (String, String), and the other taking (String, Number), and then write a function

(defn foo
  [x ^String y]
  (Thing/hello x y))

it seems obvious that I'm trying to call the first method and not the second. But on lein check, clojure prints

Reflection warning, resolve_fail/core.clj:6:3 - call to static method hello on resolve_fail.Thing can't be resolved (argument types: unknown, java.lang.String).

unless I also type-hint x.



 Comments   
Comment by Nicola Mometto [ 30/Mar/15 2:32 PM ]

I have looked at this countless times while working on tools.analyzer and hacking the reflector and found out that there doesn't seem to be a way to make things like this "work" without breaking other cases





[CLJ-1679] Add fast path in seq comparison for structurally sharing seqs Created: 21/Mar/15  Updated: 23/Mar/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: 0
Labels: collections, seq

Attachments: Text File 0001-CLJ-1679-do-pointer-checks-in-seq-equality.patch     Text File CLJ-1679-v2.patch     Text File CLJ-1679-v3.patch    
Patch: Code and Test

 Description   

Currently comparing two non identical seqs requires iterating through both seqs comparing value by value, ignoring the possibility of seq `a` and `b` having the same (pointer-equal) rest.

The proposed patch adds a pointer equality check on the seq tails that can make the equality short-circuit if the test returns true, which is helpful when comparing large (or possibly infinite) seqs that share a common subseq.

After this patch, comparisons like

(let [x (range)] (= x (cons 0 (rest x))))
which currently don't terminate, return true in constant time.

Patch: CLJ-1679-v3.patch



 Comments   
Comment by Michael Blume [ 23/Mar/15 12:52 PM ]

When this test fails (it fails on my master, but I've got a bunch of other development patches, I'm still figuring out where the conflict is), it fails by hanging forever. Maybe it'd be better to check equality in a future and time out the future?

Comment by Michael Blume [ 23/Mar/15 1:01 PM ]

like so =)

Comment by Nicola Mometto [ 23/Mar/15 1:11 PM ]

Makes sense, thanks for the updated patch

Comment by Michael Blume [ 23/Mar/15 1:24 PM ]

Hm, previous patch had a problem where the reporting logic still tries to force the sequence and OOMs, this patch prevents that.

Comment by Michael Blume [ 23/Mar/15 1:41 PM ]

ok, looks like CLJ-1515, CLJ-1603, and this patch, all combine to fail together, though any two of them work fine.

Comment by Michael Blume [ 23/Mar/15 1:43 PM ]

(And really there's nothing wrong with the source of this patch, it still works nicely to short-circuit = where there's structural sharing, it's just that the other two patches break structural sharing for ranges, so the test fails)

Comment by Nicola Mometto [ 23/Mar/15 2:02 PM ]

I see, I guess we'll have to change the test if the patches for those tickets get applied.





[CLJ-1675] IOFactory protocol extension on String does not call its Coercions Created: 12/Mar/15  Updated: 12/Mar/15

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

Type: Defect Priority: Trivial
Reporter: Andrea Richiardi Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: io
Environment:

-


Attachments: File fix-string-protocol.diff    
Patch: Code

 Description   

The IOFactory protocol extension on String doesn't call the respective as-file and as-url implemented in Coercions.

I found it odd and fixed it. If it had been done on purpose, I apologize.






[CLJ-1674] Boolean return type-hint confusing the compiler Created: 12/Mar/15  Updated: 12/Mar/15

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

Type: Defect Priority: Major
Reporter: Mikkel Kamstrup Erlandsen Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: typehints
Environment:

OSX, Clojure 1.6.0



 Description   

Saving the below snippet and running it like

java -jar clojure-1.6.0.jar snippet.clj

Produces

$ java -jar clojure-1.6.0.jar snippet.clj 
Exception in thread "main" java.lang.IllegalArgumentException: Unable to resolve classname: clojure.core$boolean@1356d4d4, compiling:(/Users/kamstrup/tmp/snippet.clj:15:1)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6651)
	at clojure.lang.Compiler.analyze(Compiler.java:6445)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6632)
	at clojure.lang.Compiler.analyze(Compiler.java:6445)
	at clojure.lang.Compiler.access$100(Compiler.java:38)
	at clojure.lang.Compiler$DefExpr$Parser.parse(Compiler.java:538)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6644)
	at clojure.lang.Compiler.analyze(Compiler.java:6445)
	at clojure.lang.Compiler.analyze(Compiler.java:6406)
	at clojure.lang.Compiler.eval(Compiler.java:6707)
	at clojure.lang.Compiler.load(Compiler.java:7130)
	at clojure.lang.Compiler.loadFile(Compiler.java:7086)
	at clojure.main$load_script.invoke(main.clj:274)
	at clojure.main$script_opt.invoke(main.clj:336)
	at clojure.main$main.doInvoke(main.clj:420)
	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)
Caused by: java.lang.IllegalArgumentException: Unable to resolve classname: clojure.core$boolean@1356d4d4
	at clojure.lang.Compiler$HostExpr.tagToClass(Compiler.java:1069)
	at clojure.lang.Compiler$InvokeExpr.getJavaClass(Compiler.java:3659)
	at clojure.lang.Compiler$LocalBinding.hasJavaClass(Compiler.java:5657)
	at clojure.lang.Compiler$LocalBindingExpr.hasJavaClass(Compiler.java:5751)
	at clojure.lang.Compiler.maybePrimitiveType(Compiler.java:1283)
	at clojure.lang.Compiler$IfExpr.doEmit(Compiler.java:2631)
	at clojure.lang.Compiler$IfExpr.emit(Compiler.java:2613)
	at clojure.lang.Compiler$BodyExpr.emit(Compiler.java:5826)
	at clojure.lang.Compiler$LetExpr.doEmit(Compiler.java:6180)
	at clojure.lang.Compiler$LetExpr.emit(Compiler.java:6133)
	at clojure.lang.Compiler$BodyExpr.emit(Compiler.java:5826)
	at clojure.lang.Compiler$FnMethod.doEmit(Compiler.java:5374)
	at clojure.lang.Compiler$FnMethod.emit(Compiler.java:5232)
	at clojure.lang.Compiler$FnExpr.emitMethods(Compiler.java:3771)
	at clojure.lang.Compiler$ObjExpr.compile(Compiler.java:4410)
	at clojure.lang.Compiler$FnExpr.parse(Compiler.java:3904)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6642)
	... 19 more

The snippet:

snippet.clj
;; Bug in the Clojure compiler (1.6.0): If we annotate the return here with ^boolean we get:
;; 'IllegalArgumentException: Unable to resolve classname: clojure.core$boolean' from the compiler.
;; Removing it, everything is as expected
(defn ^boolean foo-bar?
  [node]
  (= node "foo-bar"))

;; Check it out, we can have ^boolean here, but not on foo-bar? !! :-)
(defn ^boolean bar-foo?
  [node]
  (= node "bar-foo"))

;; Instead of removing the ^boolean return on foo-bar? we can also remove this function
;; to have all work as expected
(defn ^boolean interesting?
  [node]
  (or (foo-bar? node) (bar-foo? node)))

(println "Foo-Bar?" (foo-bar? "baz"))


 Comments   
Comment by Mikkel Kamstrup Erlandsen [ 12/Mar/15 5:25 AM ]

Typo in comment 2 in the snippet: s/xtc-scenario?/foo-bar?/

Comment by Nicola Mometto [ 12/Mar/15 6:01 AM ]

Metadata on def's symbol is evaluated as per the doc (http://clojure.org/special_forms), evaluating `boolean` results in the clojure.core/boolean function which is not a valid type hint.

As a rule of thumb, attach the return tag in the argvec rather than on the def symbol, in this case you should write

(defn foo-bar?
   ^boolean [node]
  (= node "foo-bar"))

I understand why the fact that

(defn ^boolean foo [] true)

and

(defn foo ^boolean [] true)

behave differently and the fact that the compiler will throw iff the type hint is used rather than throwing at the function definition time is confusing (and I've complained about this and the lack of documentation/specification regarding type hints for a while) but this is not a bug

Comment by Mikkel Kamstrup Erlandsen [ 12/Mar/15 6:36 AM ]

Thanks for clarifying Nicola, you are indeed correct.

Putting return type annotations before the method name seems to be common practice in a lot of Clojure code I've read online. Perpetuated by some online tutorials, and the clojure.org docs them selves (fx.

(defn ^:private ^String my-fn ...)
is found in http://clojure.org/cheatsheet)

Comment by Andy Fingerhut [ 12/Mar/15 8:36 AM ]

Mikkel: If the type tags are Java classes, not primitives, then ^Classname is a correct type tag. If you use Eastwood, it can warn about these incorrect type tags, and has some documentation on what works and what does not here: https://github.com/jonase/eastwood#wrong-tag

Also here: https://github.com/jonase/eastwood#unused-meta-on-macro





[CLJ-1672] Better error message when passing a list to update-in Created: 11/Mar/15  Updated: 11/Mar/15

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

Type: Enhancement Priority: Minor
Reporter: John Gabriele Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: checkargs, errormsgs
Environment:

OpenJDK 1.7 on GNU/Linux



 Description   

This one confused me when I'd accidentally passed a list (returned by a function) in to `update-in` instead of a vector.

Example:

some-app.core=> (update-in [:a :b :c] [1] name)
[:a "b" :c]
some-app.core=> (update-in '(:a :b :c) [1] name)

NullPointerException   clojure.core/name (core.clj:1518)

Similar result if passing in another function; for example:

some-app.core=> (update-in ["a" "b" "c"] [1] str/capitalize)
["a" "B" "c"]
some-app.core=> (update-in '("a" "b" "c") [1] str/capitalize)

NullPointerException   clojure.string/capitalize (string.clj:199)


 Comments   
Comment by Alex Miller [ 11/Mar/15 9:26 AM ]

I think this is effectively a dupe of CLJ-1107 re throwing on get with a non-Associative collection?





[CLJ-1664] Inconsistency in overflow-handling between type-hinted and reflective calls Created: 19/Feb/15  Updated: 19/Feb/15

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

Type: Defect Priority: Minor
Reporter: Michael Blume Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: numerics, reflection


 Description   
(import 'java.io.DataOutputStream)
(import 'java.io.ByteArrayOutputStream)

(defn- ->bytes
  "Convert a Java primitive to its byte representation."
  [write v]
  (let [output-stream (ByteArrayOutputStream.)
        data-output (DataOutputStream. output-stream)]
    (write data-output v)
    (seq (.toByteArray output-stream))))

(defn int->bytes [n]
  (->bytes 
    #(.writeInt ^DataOutputStream %1 %2)
    n))

(defn int->bytes-ref [n]
  (->bytes 
    #(.writeInt %1 %2)
    n))

user=> (int->bytes 5)
(0 0 0 5)
user=> (int->bytes-ref 5)
(0 0 0 5)
user=> (int->bytes (inc Integer/MAX_VALUE))

IllegalArgumentException Value out of range for int: 2147483648  clojure.lang.RT.intCast (RT.java:1115)
user=> (int->bytes-ref (inc Integer/MAX_VALUE))
(-128 0 0 0)

So it looks like type-hinting the DataOutputStream results in bytecode calling RT.intCast, which throws because the value is too large. In the reflective case, we locate the method writeInt at runtime, and then do not call RT.intCast, but instead allow the long to be downcast without bounds checking.

It seems like we should be calling RT.intCast in both cases?






[CLJ-1662] folding over hash-map nested hash-map throws exception Created: 17/Feb/15  Updated: 17/Feb/15

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: Francis Avila Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: reducers
Environment:

JVM 1.7.0_76



 Description   

I got a baffling exception in a recursive function that folds. REPL transcript below:

nREPL server started on port 57818 on host 127.0.0.1 - nrepl://127.0.0.1:57818
REPL-y 0.3.5, nREPL 0.2.6
Clojure 1.7.0-alpha5
Java HotSpot(TM) 64-Bit Server VM 1.7.0_76-b13
    Docs: (doc function-name-here)
          (find-doc "part-of-name-here")
  Source: (source function-name-here)
 Javadoc: (javadoc java-object-or-class-here)
    Exit: Control+D or (exit) or (quit)
 Results: Stored in vars *1, *2, *3, an exception in *e

user=> (use 'foldtest.core)
nil
user=> (source leafs)
(defn leafs [xs]
  (->> (r/mapcat (fn [k v]
                   (if (map? v)
                     (leafs v)
                     [[k v]])) xs)
       (r/foldcat)))
nil
user=> (leafs (hash-map :a (hash-map :b 1 :c 2)))

ClassCastException clojure.lang.PersistentHashMap$1 cannot be cast to clojure.lang.IFn  clojure.core.reducers/fjinvoke (reducers.clj:48)
user=> (pst)
ClassCastException clojure.lang.PersistentHashMap$1 cannot be cast to clojure.lang.IFn
	clojure.core.reducers/fjinvoke (reducers.clj:48)
	clojure.lang.PersistentHashMap.fold (PersistentHashMap.java:207)
	clojure.core.reducers/eval1347/fn--1348 (reducers.clj:367)
	clojure.core.reducers/eval1220/fn--1221/G--1211--1232 (reducers.clj:81)
	clojure.core.reducers/folder/reify--1247 (reducers.clj:130)
	clojure.core.reducers/fold (reducers.clj:98)
	clojure.core.reducers/fold (reducers.clj:96)
	clojure.core.reducers/foldcat (reducers.clj:318)
	foldtest.core/leafs (core.clj:5)
	foldtest.core/leafs/fn--1367 (core.clj:7)
	clojure.core.reducers/mapcat/fn--1277/fn--1280 (reducers.clj:185)
	clojure.lang.PersistentHashMap$NodeSeq.kvreduce (PersistentHashMap.java:1127)
nil
user=>

Note that it must be a hash-map nested in a hash-map. Other combinations of array and hash maps seem fine:

user=> (leafs (array-map :a (hash-map :b 1 :c 2)))
[[:c 2] [:b 1]]
user=> (leafs (hash-map :a (array-map :b 1 :c 2)))
[[:b 1] [:c 2]]
user=> (leafs (hash-map :a (hash-map :b 1 :c 2)))

ClassCastException clojure.lang.PersistentHashMap$1 cannot be cast to clojure.lang.IFn  clojure.core.reducers/fjinvoke (reducers.clj:48)
user=> (leafs (array-map :a (array-map :b 1 :c 2)))
[[:b 1] [:c 2]]
user=>

Possibly related: CLJCLR-63

It took me a while to discover this because of this inconsistency (which I am not sure is a bug):

user=> (def a {:a 1})
#'user/a
user=> (type a)
clojure.lang.PersistentHashMap
user=> (let [a {:a 1}] (type a))
clojure.lang.PersistentArrayMap
user=> (type {:a 1})
clojure.lang.PersistentArrayMap
user=>

(I had put test input in a def, but using the defed var always failed but literals always worked!)






[CLJ-1655] Dorun's behavior when called with two argument's is both unintuitive and undocumented. Created: 04/Feb/15  Updated: 11/Jun/16

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

Type: Enhancement Priority: Minor
Reporter: Michael Blume Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Dorun can be called as (dorun n coll). When called this way, dorun will force n+1 elements from coll, which seems unintuitive. I can't necessarily call this a defect, though. It doesn't deviate from the documented behavior because there is no documented behavior – the two-argument arity is not mentioned in the docstring.

user=> (defn printing-range [n] (lazy-seq (println n) (cons n (printing-range (inc n)))))
#'user/printing-range
user=> (dorun 0 (printing-range 1))
1
nil
user=> (dorun 3 (printing-range 1))
1
2
3
4
nil


 Comments   
Comment by Stuart Halloway [ 11/Jun/16 5:36 PM ]

I do not think this is a bug, as it is caused by the pervasive semantics of seq, not just of dorun. Consider

(def x (seq (printing-range 0)))
0

i.e. just calling seq consumes the first item. I am leaving open as a feature request for improved docstring.





[CLJ-1651] Erroneous error message when using into to create a map. Created: 29/Jan/15  Updated: 29/Jan/15

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

Type: Defect Priority: Minor
Reporter: Justin Glenn Smith Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: error-reporting


 Description   

If you provide a sequence instead of a vector type for the entries provided to into for creating a hash-map, the error message is misleading.

org.noisesmith.orsos=> (into {} '((:a 0) (:b 1)))

ClassCastException clojure.lang.Keyword cannot be cast to java.util.Map$Entry clojure.lang.ATransientMap.conj (ATransientMap.java:44)

As we see, it reports the type of the first item in the entry, rather than the actual error, the type of the entry itself, which can be particularly confusing if the key in the entry is actually a valid type to be an entry:

=> (into {} '((["a" 1] ["b" 2]) (["c" 3] ["d" 4])))

ClassCastException clojure.lang.PersistentVector cannot be cast to java.util.Map$Entry clojure.lang.ATransientMap.conj (ATransientMap.java:44)






[CLJ-1628] Accept list as lib specification in clojure.core/require and clojure.core/use Created: 28/Dec/14  Updated: 30/Dec/14

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

Type: Enhancement Priority: Minor
Reporter: Petr Gladkikh Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File NS-macro-accept-lists-as-libspecs.patch    
Patch: Code

 Description   

Currently function clojure.core/load-libs treats '(a.namespace.name) as prefix list so this construct has no effect at all (as if it was prefix list without suffixes). At the same time '[a.namespace.name] causes require call (require 'a.namespace.name). In other cases functions require or use are ambivalent about differences between [] and (). In this particular case there is difference between no-op and lib loading. E.g. Clojure validation tool Eastwood includes rule for this case since the behavior of (require '(a.namespace.name)) is not obvious.

The suggested change lets to avoid this special case in require or use calls (including ones that stem from ns macro expansion). Accepting both list and vector as library specification makes behavior uniform and similar to the way suffix items are handled in prefix lists.

The patch is minimal in order to avoid reordering sequential? functions in clojure/core.clj
Should I include tests for these cases also?



 Comments   
Comment by Petr Gladkikh [ 30/Dec/14 2:39 AM ]

If on the other hand representing prefix lists as Clojure lists is intentional and list-for-prefix, vector-for-libspec-or-suffix should be distinguishing feature then we should issue error when

  • prefix list is enclosed in Clojure vector
  • libspec or suffix is in Clojure list

If backwards compatibility is important then one may at least write a warning in ':verbose' mode.

Also there should be error or warning if prefix list is empty.





[CLJ-1626] ns macro: compare ns name during macroexpansion. Created: 23/Dec/14  Updated: 02/Jul/15

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

Type: Enhancement Priority: Trivial
Reporter: Petr Gladkikh Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None

Attachments: File compare-ns-name-at-macroexpansion.diff    

 Description   

Macroexpansion of 'ns' produces 'if' form that is executed at runtime. However comparison can be done during macroexpansion phase producing clearer resulting form in most cases.

Patch for suggested change is in attachment.



 Comments   
Comment by Andy Fingerhut [ 02/Jul/15 3:53 PM ]

Petr, I do not know whether this change is of interest to the Clojure core team or not. I do know that it is not in the expected format for a patch. See this link for instructions on creating a patch in the expected format: http://dev.clojure.org/display/community/Developing+Patches

Same comment applies to your patch for CLJ-1628





[CLJ-1625] Cannot implement protocol methods of the same name inline Created: 23/Dec/14  Updated: 23/Dec/14

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: Tassilo Horn Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: protocols


 Description   

One major benefit of protocols (IMHO) is that the protocol methods are properly namespace qualified. Thus I can have multiple protocols in different namespaces that define a foo method and extend them all (or a subset of them) upon existing types. However, that's not true with extending protocols inline with defrecord and deftype, or with extending protocols on the Java side by implementing their interfaces.

Example:

;; file: protocoltest/foo.clj
(ns prototest.foo)
(defprotocol Foo
  (mymethod [this]))

;; file: protocoltest/bar.clj
(ns prototest.bar)
(defprotocol Bar
  (mymethod [this]))

;; file: protocoltest/core.clj
(ns prototest.core
  (:require [prototest.foo :as foo]
            [prototest.bar :as bar]))

;; inline extension of both mymethod methods doesn't work
(defrecord MyRec [x]
  foo/Foo
  (mymethod [this] :foo)
  bar/Bar
  (mymethod [this] :bar))
;;=> java.lang.ClassFormatError
;;   Duplicate method name&signature in class file prototest/core/MyRec

;; I have to resort to either half-inline-half-dynamic...
(defrecord MyRec [x]
  foo/Foo
  (mymethod [this] :foo))
(extend-type MyRec
  bar/Bar
  (mymethod [this] :bar))

;; ... or fully dynamic extension.
(defrecord MyRec [x])
(extend-type MyRec
  foo/Foo
  (mymethod [this] :foo)
  bar/Bar
  (mymethod [this] :bar))

;; Then things work just fine.
(foo/mymethod (->MyRec 1))
;;=> :foo
(bar/mymethod (->MyRec 1))
;;=> :bar

I know that I get the error because both the Foo and the Bar interfaces backing the protocols have a mymethod method and thus they cannot be implemented both at once (at least not with different behavior).

But why throw away the namespacing advantages we have with protocols? E.g., why is the protocoltest.foo.Foo method not named protocoltest$foo$mymethod (or some other munged name) in the corresponding interface? That way, both methods can be implemented inline where you gain the speed advantage, and you can do the same even from the Java side. (Currently, invoking clojure.core.extend from the Java side using clojure.java.api is no fun because you have to construct maps, intern keywords, define functions, etc.)

Of course, the ship of changing the default method naming scheme has sailed long ago, but maybe a :ns-qualified-method-names option could be added to defprotocol.






[CLJ-1615] transient set "keys" and "values" wind up with different metadata Created: 12/Dec/14  Updated: 13/Dec/14

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

Type: Defect Priority: Minor
Reporter: Michael Blume Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: collections, meta, transient

Attachments: Text File 0001-CLJ-1615-ensure-transient-set-keys-and-values-have-c.patch     Text File 0001-demonstrate-CLJ-1615.patch     Text File CLJ-1615-entryAt.patch    
Patch: Code and Test

 Description   
(let [s (-> #{} 
          transient 
          (conj! (clojure.core/with-meta [-7] {:mynum 0}))
          (conj! (clojure.core/with-meta [-7] {:mynum -1})) 
          persistent!)]
  [(meta (s [-7])) (meta (first s))])
=> [{:mynum -1} {:mynum 0}]

basically it looks like the "key" (the value we get by seqing on the set) retains the metadata from the first conj! but the "value" (what we get by calling invoke with the "key") carries the metadata from the second conj!. This does not match the behavior if we don't use transients:

(let [s (-> #{} 
          (conj (clojure.core/with-meta [-7] {:mynum 0}))
          (conj (clojure.core/with-meta [-7] {:mynum -1})))]
  [(meta (s [-7])) (meta (first s))])
=> [{:mynum 0} {:mynum 0}]

(found playing with zach tellman's collection-check)



 Comments   
Comment by Michael Blume [ 12/Dec/14 5:07 PM ]

Attached patch demonstrating problem (not a fix)

Comment by Michael Blume [ 12/Dec/14 5:40 PM ]

More investigation:

The difference between "keys" and "vals" arises from the fact that clojure sets use maps under the covers.

The difference between persistent and transient seems to be because PersistentHashSet.cons short-circuits on contains (https://github.com/clojure/clojure/blob/clojure-1.6.0/src/jvm/clojure/lang/PersistentHashSet.java#L97) and ATransientSet.conj does not (https://github.com/clojure/clojure/blob/clojure-1.6.0/src/jvm/clojure/lang/ATransientSet.java#L27)

Adding a contains check to ATransientSet.conj makes the behavior consistent and passes the attached test, but I imagine this could cause a performance hit. Thoughts?

Comment by Michael Blume [ 12/Dec/14 5:43 PM ]

Attached proposed fix – note that this may cause a performance hit for transient sets.

Comment by Michael Blume [ 13/Dec/14 2:40 PM ]

Attaching an alternative fix – instead of doing a contains check on every transient conj, back set.get with entryAt. More invasive but possibly faster.





[CLJ-1612] clojure.core.reducers/mapcat can call f1 with undefined arity of 0 arguments? Created: 10/Dec/14  Updated: 10/Dec/14

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


 Description   

I have not run across this with running code, so perhaps it is impossible for reasons I have not understood. Also not sure whether fixing issues with reducers is of any importance, given transducers. This was found while testing the Eastwood lint tool on some Clojure namespaces, including clojure.core.reducers.

(defcurried mapcat
  "Applies f to every value in the reduction of coll, concatenating the result
  colls of (f val). Foldable."
  {:added "1.5"}
  [f coll]
  (folder coll
   (fn [f1]
     (let [f1 (fn
                ([ret v]
                  (let [x (f1 ret v)] (if (reduced? x) (reduced x) x)))
                ([ret k v]
                  (let [x (f1 ret k v)] (if (reduced? x) (reduced x) x))))]
       (rfn [f1 k]
            ([ret k v]
               (reduce f1 ret (f k v))))))))

The definition of macro rfn expands to a (fn ...) that can call f1 with no arguments, which is not a defined arity for f1.






[CLJ-1610] Unrolled small maps Created: 08/Dec/14  Updated: 22/Jan/16

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

Type: Enhancement Priority: Major
Reporter: Alex Miller Assignee: Zach Tellman
Resolution: Unresolved Votes: 9
Labels: collections


 Description   

Placeholder for unrolled small maps enhancement (companion for vectors at CLJ-1517).



 Comments   
Comment by Andy Fingerhut [ 09/Jul/15 10:59 PM ]

Is there an expectation that these would perform better that PersistentArrayMap?

Comment by Zach Tellman [ 09/Jul/15 11:53 PM ]

Yes, in some cases significantly so, for three reasons (in rough order of importance):

  • positional constructors, without any need for array instantiation/population
  • short-circuiting equality checks using hash comparisons
  • no iteration on any operation

There are a series of benchmarks at https://github.com/ztellman/cambrian-collections/blob/master/test/cambrian_collections/map_test.clj#L64-L148, which compare operations against maps with both keywords (which don't benefit from the hash comparisons) and symbols (which do). The 7-entry map cases cause the unrolled maps to overflow, so they only exist to test the overflow mechanism.

I've run the benchmark suite on my laptop, and the results are at https://gist.github.com/ztellman/961001e1a77e4f76ee1d. Some notable results:

The rest of the benchmarks are marginally faster due to unrolling, but most of the performance benefits are from the above behaviors. In a less synthetic benchmark, I found that Cheshire JSON decoding (which is 33% JSON lexing and 66% data structure building) was sped up roughly 30-40%.





[CLJ-1595] Nested doseqs leak with sequence of huge lazy-seqs Created: 20/Nov/14  Updated: 25/May/15

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

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

Attachments: File doseq_leaks.diff    
Patch: Code

 Description   

Hello!

This little snippet demonstrates the problem:

(doseq [outer-seq (list (range)) inner-seq outer-seq])

That's it. It is not just eats my processor, but also eats all available memory. Practically it can affect (and it is) at consuming of complex lazy structures like huge XML-documents.

I think this is at least non trivial behaviour.

It can be fixed by this small patch. We can get next element before current iteration, not after, so outer loop will not hold reference to the head of inner-seq.

This patch doesn't solve all problems, for example this code:

(doseq [outer-seq [(range)] inner-seq outer-seq])

leaks. Because chunked-seqs (vector in this case) holds current element by design.



 Comments   
Comment by Andy Fingerhut [ 25/May/15 3:15 PM ]

Andrew, sorry but I do not know whether this ticket is of interest to the Clojure core team.

I do know that patches are only considered for inclusion in Clojure if the submitter has signed the contributor agreement (CA). If you were interested in doing that, you can do it fairly quickly on-line here: http://clojure.org/contributing





[CLJ-1592] Ability to suppress warnings on name conflict with clojure.core Created: 14/Nov/14  Updated: 14/Nov/14

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

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


 Description   

In numerical code, it is often useful and idiomatic to replace clojure.core functions with augmented versions (e.g. clojure.core.matrix.operators defines + in a way that works with whole arrays, not just scalar numbers)

Currently there seems to be no way to avoid a warning in client code when a library does this, e.g.:

;; library namespace
(ns foo
  (:refer-clojure :exclude [+]))
(def + clojure.core/+)

;; later on, in some other namespace
(require '[foo :refer :all])
=> WARNING: + already refers to: #'clojure.core/+ in namespace: bar, being replaced by: #'foo/+

A workaround exists by using (:refer-clojure :exclude ...) in the user namespace, however this creates unnecessary work for the user and requires maintenance of boilerplate code.

Proposed solution is to allow vars to be annotated with additional metadata (e.g. ^:replace-var ) that when added to library functions will suppress this warning. This will allow library authors to specify that a function should work as a drop-in replacement for clojure.core (or some other namespace), and that a warning is therefore not required.



 Comments   
Comment by Andy Fingerhut [ 14/Nov/14 9:46 PM ]

Duplicate with CLJ-1257 ?

Comment by Mike Anderson [ 14/Nov/14 9:53 PM ]

Hi Andy, it refers to the same warning - but the scope of the solution is different:

  • CLJ-1257 is more like a global way to turn off this warning
  • CLJ-1592 is for suppressing this warning on specific vars

If CLJ-1257 is implemented and the warning is off be default, CLJ-1592 becomes mostly unnecessary. Without CLJ-1257 or if the warning defaults to on, CLJ-1592 is needed.





[CLJ-1591] Symbol not being bound in namespace when name clashes with clojure.core Created: 14/Nov/14  Updated: 03/Jun/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: Mike Anderson Assignee: Unassigned
Resolution: Unresolved Votes: 3
Labels: None


 Description   

The following code fails (both in 1.6 and latest 1.7-alpha4):

user=> (ns foo)
nil
foo=>  (def inc inc)
WARNING: inc already refers to: #'clojure.core/inc in namespace: foo, being replaced by: #'foo/inc
#'foo/inc

;; Note inc is unbound at this point, which causes the exception below
foo=> inc
#<Unbound Unbound: #'foo/inc>
foo=> (ns bar)
nil
bar=> (require ['foo :refer ['inc]])
WARNING: inc already refers to: #'clojure.core/inc in namespace: bar, being replaced by: #'foo/inc
nil
bar=> (inc 8)

IllegalStateException Attempting to call unbound fn: #'foo/inc  clojure.lang.Var$Unbound.throwArity (Var.java:43)

Further investigation shows that foo/inc is unbound:

foo/inc
=> #<Unbound Unbound: #'foo/inc>

Further investigation also shows that replacing the (def inc inc) with almost anything else, e.g. (def inc dec), (def inc clojure.core/inc), or (def inc (fn [n] (+ n 1))), causes no exception (but the warnings remain).

I would expect:
a) foo/inc should be bound and have the same value as clojure.core/inc
b) No error when requiring foo/inc
c) bar/inc should be bound to foo/inc



 Comments   
Comment by Nicola Mometto [ 14/Nov/14 10:04 PM ]

The second error should be expected, the right syntax should be (require ['foo :refer ['inc]]) (note the leading quote before inc)

Comment by Mike Anderson [ 14/Nov/14 10:20 PM ]

Thanks for the catch Nicola - I've edited the description. Still get the same error however (just with a slightly different message)

Comment by Alex Miller [ 14/Nov/14 10:22 PM ]

See comment...

Comment by Mike Anderson [ 14/Nov/14 10:24 PM ]

@Alex what comment? Note that the error still occurs even with the right syntax....

Comment by Mike Anderson [ 14/Nov/14 10:26 PM ]

Appears to have been closed prematurely

Comment by Alex Miller [ 14/Nov/14 10:39 PM ]

I can't reproduce with the correct syntax:

Clojure 1.7.0-master-SNAPSHOT
user=> (ns foo)
nil
foo=> (def inc inc)
WARNING: inc already refers to: #'clojure.core/inc in namespace: foo, being replaced by: #'foo/inc
#'foo/inc
foo=> (ns bar)
nil
bar=> (require ['foo :refer ['inc]])
WARNING: inc already refers to: #'clojure.core/inc in namespace: bar, being replaced by: #'foo/inc
nil
Comment by Mike Anderson [ 14/Nov/14 10:55 PM ]

The problem is that the var is still unbound and causes e.g. the following error:

=> (foo/inc 8)
IllegalStateException Attempting to call unbound fn: #'foo/inc clojure.lang.Var$Unbound.throwArity (Var.java:43)

I don't think that should be expected - or am I missing something?

Comment by Alex Miller [ 14/Nov/14 10:57 PM ]

Ah, will take a look. But not right now.

Comment by Andy Fingerhut [ 15/Nov/14 1:09 PM ]

Updated the description with a few more details. The exception goes away if you do (def inc (fn [n] (+ n 1))) instead of (def inc inc), for example. The warnings remain.

Comment by Tom Crayford [ 20/Nov/14 11:07 AM ]

Unsure if this is the same issue (I think it might be?), but I reproduced the exact same error message with AOT compilation involved:

reproduced in this git repository: https://github.com/yeller/compiler_update_not_referenced_bug

clone it, run `lein do clean, uberjar, test`, and that error message will show up every time for me

Comment by Andy Fingerhut [ 20/Nov/14 5:43 PM ]

Mike, I think replacing (def inc inc) in your example with (def inc clojure.core/inc) should be considered as a reasonable workaround for this issue, unless you have some use case where you need to def inc to something that is not in clojure.core (and if so, why?)

The reason (def inc inc) behaves this way is, if not absolutely necessary, at least commonly used in Clojure programs to define recursive functions, e.g. (defn fib [n] (if (<= n 1) 1 (+ (fib (dec n)) (fib (- n 2))))), so that the occurrences of fib in the body are resolved to the fib being defined.

Comment by Alex Miller [ 22/Nov/14 9:05 AM ]

Moving to 1.7 until I can look at this more deeply.

Comment by Mike Anderson [ 23/Nov/14 6:08 PM ]

Andy - yes the workaround is fine for me right now.

I don't think this is an urgent issue but it may be exposing a subtle complexity regarding assumptions about the state of the namespace at different times. Perhaps the semantics should be something like:

  • The def statement itself should be run before the var is interned. e.g. (def inc (inc 5)) should result in (def inc 6)
  • Anything complied / deferred to run after completion of the def statement should use the new var (i.e. the new var should be referenced by fns, lazy sequences etc.)
Comment by Andy Fingerhut [ 23/Nov/14 6:36 PM ]

I'm not sure what your proposal means in a case like this:

(def inc (fn [x] (inc x)))

Is the second inc to be interpreted/resolved before or after the new inc is created? Because it is (fn ...) it should be the after-behavior? What else besides fn should cause the after-behavior, rather than the before-behavior?

Even more fun (not saying that people often write code like this, but the compiler can handle it today):

(def inc (if (> (inc y) 5)
           (fn [x] (inc x))
           (fn [x] (dec x))))

I think the current compiler behavior of 'in the body of a def, the def'd symbol always refers to the new var, not any earlier def'd vars' is fairly straightforward to explain.

Comment by Tom Crayford [ 23/Nov/14 9:15 PM ]

Should I file the AOT issue reproduced in that thing as a new issue?

Comment by Andy Fingerhut [ 24/Nov/14 5:16 PM ]

Tom: Alex Miller or another screener would be best to say whether the AOT issue should be a separate ticket, but my best guess would be "go for it". I tried to look at the link you gave but it seems not to point to anything. Could you double-check that link?

Comment by Tom Crayford [ 24/Nov/14 6:48 PM ]

Andy,

Great. I'll write one up tomorrow sometime. I accidentally left that repo as private, should be visible now.

Comment by Andy Fingerhut [ 24/Nov/14 8:11 PM ]

This comment is really most relevant for ticket CLJ-1604, where it has been copied:

Tom, looked at your project. Thanks for that. It appears not to have anything like (def inc inc) in it. It throws exception during test step of 'lein do clean, uberjar, test' consistently for me, too, but compiles with only warnings and passes tests with 'lein do clean, test'. I have more test results showing in which Clojure versions these results change. To summarize, the changes to Clojure that appear to make the biggest difference in the results are below (these should be added to the new ticket you create – you are welcome to do so):

Clojure 1.6.0, 1.7.0-alpha1, and later changes up through the commit with description "CLJ-1378: Allows FnExpr to override its reported class with a type hint": No errors or warnings for either lein command above.

Next commit with description "Add clojure.core/update, like update-in but takes a single key" that adds clojure.core/update: 'lein do clean, test' is fine, but 'lein do clean, uberjar' throws exception during compilation, probably due to CLJ-1241.

Next commit with description "fix CLJ-1241": 'lein do clean, test' and 'lein do clean, uberjar' give warnings about clojure.core/update, but no errors or exceptions. 'lein do clean, uberjar, test' throws exception during test step that is same as the one I see with Clojure 1.7.0-alpha4. Debug prints of values of clojure.core/update and int-map/update (in data.int-map and in Tom's namespace compiler-update-not-referenced-bug.core) show things look fine when printed inside data.int-map, and in Tom's namespace when not doing the uberjar, but when doing the uberjar, test, int-map/update is unbound in Tom's namespace.

In case it makes a difference, my testing was done with Mac OS X 10.9.5, Leiningen 2.5.0 on Java 1.7.0_45 Java HotSpot(TM) 64-Bit Server VM

Comment by Nicola Mometto [ 25/Nov/14 3:44 PM ]

Tom, I've opened a ticket with a patch fixing the AOT issue: http://dev.clojure.org/jira/browse/CLJ-1604

Comment by Kevin Downey [ 03/Jun/16 5:09 AM ]

the consequences of this bug can be very hard to track back to this bug. it would be really nice to get it fixed in someway.

(defmulti update identity)
... pages of other code ...
(defmethod update :foo [_])

will throw a compiler error on the defmethod saying update is unbound





[CLJ-1585] Report boxed math warning on function that boxes primitive return value Created: 11/Nov/14  Updated: 14/Dec/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: 1
Labels: errormsgs, math

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

 Description   

With the new :warn-on-boxed (CLJ-1325), these examples do not report a boxed math warning although they each do boxing:

user=> (defn f1 [^long x] (inc x))
f1
user=> (defn f2 [x] (aget (long-array [1 2]) 0))
f2
user=> (defn f3 [x] (aget (int-array [1 2]) 0))
f3
user=> (defn f4 [^String s] (.indexOf s "a"))

Cause: emitBoxReturn has a hard-coded call to box a prim return value.

Solution: If *unchecked-math* is set to :warn-on-boxed, emit a warning on boxing of primitive numeric return types.

Patch:



 Comments   
Comment by Alex Miller [ 12/Nov/14 12:39 AM ]

Attached patch does the job, but from trying it out on some real code, it finds both problematic cases and lots of cases that could safely be ignored and/or where there is no obvious way to fix the warning. I think it may need some more tuning to reduce the rate of unfixable things a bit.





[CLJ-1583] Apply forces the evaluation of one element more than necessary Created: 07/Nov/14  Updated: 09/Nov/14

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

Attachments: Text File 0001-make-RT.boundedLength-lazier.patch    
Patch: Code

 Description   

Given a function with one fixed argument and a vararg, it should be sufficient to force evaluation of 2 elements for apply to know which arity it should select, however it currently forces 3:

user=> (defn x ([a & b]))
#'user/x
user=> (apply x (map println (iterate inc 0)))
0
1
2
nil

This makes lazy functions that use apply (for example mapcat) less lazy than they could be.
The proposed patch makes RT.boundedLength short-circuit immediately after the seq count is greater than the max fixed arity:

user=> (defn x ([a & b]))
#'user/x
user=> (apply x (map println (iterate inc 0)))
0
1
nil


 Comments   
Comment by Nicola Mometto [ 09/Nov/14 3:37 PM ]

The patch in this ticket slightly improves the issue reported at http://dev.clojure.org/jira/browse/CLJ-1218





[CLJ-1582] Overriding in-ns and ns is problematic Created: 07/Nov/14  Updated: 07/Nov/14

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

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

Attachments: Text File 0001-Allow-overriding-of-clojure.core-in-ns-and-clojure.c.patch    
Patch: Code

 Description   

Currently it is possible to override clojure.core/in-ns and clojure.core/ns, but it is not possible to refer to the namespace-specific vars without fully qualifying them:

user=> (ns foo (:refer-clojure :exclude [in-ns]))
nil
foo=> (def in-ns 1)
#'foo/in-ns
foo=> in-ns
#<clojure.lang.RT$1@76b5e4c5>

After this patch, overriding in-ns and ns works like for every other clojure.core var:

user=> (ns foo (:refer-clojure :exclude [in-ns]))
nil
foo=> (def in-ns 1)
#'foo/in-ns
foo=> in-ns
1


 Comments   
Comment by Reid McKenzie [ 07/Nov/14 11:46 AM ]

This is motivated by https://github.com/jonase/eastwood/issues/100





[CLJ-1577] Some hints accept both symbols and class objects, others only symbols Created: 30/Oct/14  Updated: 30/Oct/14

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

Type: Defect Priority: Minor
Reporter: Brandon Bloom Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: typehints


 Description   

In order to hint primitives, such as longs, you can hint with the symbol 'long. In some places, you can also use the class object java.lang.Long/TYPE. However, in some places, that doesn't work. This is particularly problematic when working with hints in macros, where subtle changes to when metadata is evaluated can lead to changes in whether or not hints are respected.

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

user=> (defmacro mac []
         (let [field (with-meta 'x {:tag 'long})]
           (-> field meta :tag class prn)
           `(deftype Foo# [~field]
              clojure.lang.IDeref
              (deref [this#]
                (inc ~(with-meta field nil))))))
#'user/mac

user=> (mac)
clojure.lang.Symbol
#<java.lang.Class@1c76c583 class user.Foo__13651__auto__>

user=> (defmacro mac []
         (let [field (with-meta 'x {:tag java.lang.Long/TYPE})]
           (-> field meta :tag class prn)
           `(deftype Foo# [~field]
              clojure.lang.IDeref
              (deref [this#]
                (inc ~(with-meta field nil))))))
#'user/mac

user=> (mac)
java.lang.Class
Boxed math warning, /private/var/folders/43/mnwlkd2s7r1gbjwq6t__mgt40000gn/T/form-init5463347341158437534.clj:1:1 - call: public static java.lang.Number clojure.lang.Numbers.unchecked_inc(java.lang.Object).
#<java.lang.Class@74626b21 class user.Foo__13663__auto__>





[CLJ-1575] Using a (def ^:const instance) of a deftype that implements IPersistentCollection, triggers compiler errors Created: 29/Oct/14  Updated: 30/Oct/14

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

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

fresh repl


Attachments: Text File 0001-Test-for-analyzer-bug-CLJ-1575.patch    

 Description   

The compiler has a lot of assumptions about the possible types of IPersistentCollection literals and rightfully so. The strange thing with this case is, that taking the (constant) value works as soon as count is defined, but using it as an argument hits a closed dispatch for emitting the empty variants of the various literals.

> (deftype T [] clojure.lang.IPersistentCollection (count [_] 0)
> (def ^:const t (T.))
> (meta t)
java.lang.UnsupportedOperationException: Unknown Collection type
Compiler.java:2860 clojure.lang.Compiler$EmptyExpr.emit
Compiler.java:3632 clojure.lang.Compiler$InvokeExpr.emitArgsAndCall
...

EDIT updated the ticket after some investigation
NOTE attached test patch doesn't even implement (count []) for the deftype, which just triggers a rightful AbstractMethodError



 Comments   
Comment by Herwig Hochleitner [ 29/Oct/14 10:00 PM ]

The test had a typo, sorry

Comment by Alex Miller [ 30/Oct/14 7:14 AM ]

Looks like a variant of CLJ-1093.





[CLJ-1573] Support (Java) transient fields in deftype, e.g. for hashcodes Created: 26/Oct/14  Updated: 29/Dec/14

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

Type: Enhancement Priority: Minor
Reporter: Ghadi Shayban Assignee: Unassigned
Resolution: Unresolved Votes: 4
Labels: compiler, deftype

Attachments: Text File 0001-transient-field-deftype.patch    
Patch: Code and Test

 Description   

Enhance deftypes to allow fields to be marked ACC_TRANSIENT.

strawman syntax:
(deftype AType [^:transient hash])

Came across this need while experimenting with a reified range written in a deftype, not in Java.

Patch doesn't include docstring change, but has a test.



 Comments   
Comment by Adrian Medina [ 29/Dec/14 11:54 AM ]

Perhaps ^:transient-mutable would be a more appropriate modifier name to be consistent with the ^:unsynchronized-mutable and ^:volatile-mutable field modifiers. In any event, this feature would eliminate the need to drop down to Java for types that require transient fields.

Comment by Andy Fingerhut [ 29/Dec/14 12:07 PM ]

Roberto, there is a "Vote" word you can click on to actually vote for tickets, and ticket wranglers actually look at those votes at times to examine popular ones sooner. +1 comments don't do that.





[CLJ-1570] Core clojure code mixes tabs with spaces Created: 20/Oct/14  Updated: 19/Jul/15

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

Type: Enhancement Priority: Trivial
Reporter: Michael Blume Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

A handful of functions in clojure.core, clojure.core-proxy, clojure.inspector, clojure.xml, clojure.pprint, clojure.stacktrace, clojure.set, and clojure.test switch partway through from indenting with spaces to indenting with tabs. This may cause them to display incorrectly depending on how the developer's editor is configured.

(not sure if this should be marked defect or task)



 Comments   
Comment by Andy Fingerhut [ 20/Oct/14 1:41 PM ]

Some similarities to CLJ-1026, although this problem does not cause the same issues with warnings on git patches as CLJ-1026 does, as far as I know.

One similarity is that if it is of interest (I don't know if it is), Alex or other Clojure screeners may want a procedure to clean them all up, and perhaps repeat that process periodically, e.g. before each major release.





[CLJ-1566] Documentation for clojure.core/require does not document :rename Created: 16/Oct/14  Updated: 02/Jul/15

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

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

Attachments: Text File refer.patch    
Patch: Code

 Description   

By contrast, clojure.core/use does mention :rename.

I attach a patch



 Comments   
Comment by Andy Fingerhut [ 16/Oct/14 1:33 PM ]

James, your patch removes any mention of the :all keyword, and that keyword is not mentioned in the doc string for clojure.core/refer.

I haven't checked whether refer can take :all as an argument, but clojure.core/require definitely can.

Comment by James Laver [ 16/Oct/14 1:39 PM ]

Ah, you're quite right. Fixed now. See updated patch in a sec.

Comment by Andy Fingerhut [ 16/Oct/14 8:16 PM ]

For sake of reduced confusion, it would be better if you could either name your patches differently, or delete obsolete ones with identical names as later ones. JIRA allows multiple patches to have the same names, without replacing the earlier ones.

Comment by James Laver [ 17/Oct/14 12:44 AM ]

Okay, that's done. The JIRA interface is a bit tedious in places.

Comment by Bozhidar Batsov [ 19/Oct/14 1:34 AM ]

Seems to me the sentence should end with a dot.

Comment by James Laver [ 19/Oct/14 4:36 AM ]

Added a dot.

Comment by Andy Fingerhut [ 02/Jul/15 3:56 PM ]

James, I do not know whether this change is of interest to the Clojure core team, but see this link for instructions on creating patches in the expected format (yours is not in that format): http://dev.clojure.org/display/community/Developing+Patches





[CLJ-1563] How About Default Implementations on Protocols Created: 11/Oct/14  Updated: 12/Oct/14

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

Type: Enhancement Priority: Minor
Reporter: David Williams Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Consider this example

user=> (defprotocol Foo (foo [x] x))
Foo
user=> (defrecord Bar [gaz waka] Foo)
user.Bar
user=> (def bar (Bar. 1 2))
#'user/bar
user=> (.foo bar)

AbstractMethodError user.Bar.foo()Ljava/lang/Object;  sun.reflect.NativeMethodAccessorImpl.invoke0 (NativeMethodAccessorImpl.java:-2)
user=>

What about the default implementation.



 Comments   
Comment by David Williams [ 11/Oct/14 8:48 PM ]

As it stands you have to workaround with this

http://stackoverflow.com/questions/15039431/clojure-mix-protocol-default-implementation-with-custom-implementation

Comment by Jozef Wagner [ 12/Oct/14 1:01 AM ]

I don't think we need it. What's the rationale behind extending some protocol, not implementing its methods, and then calling those methods, expecting them not to throw. Be explicit about what yout type should do, whether it is a default or custom behavior. You basically have three options

(defn default-foo 
  [this] 
  :foo)

(defprotocol P
  (-foo [this]))

(deftype T
  P
  (-foo [this] (default-foo))

(defn foo 
  [x]
  (-foo x))

or

(defprotocol P
  (-foo [this]))

(deftype T)

(defn foo 
  [x]
  (if (satisfies? P x)
    (-foo x)
    :foo))

or

(defprotocol P
  (-foo [this]))

(extend-protocol P
  java.lang.Object
  (-foo [this] :foo))

(deftype T)

(defn foo 
  [x]
  (-foo x))

I think however that my first approach is unidiomatic and you should prefer the latter ones.

Comment by David Williams [ 12/Oct/14 12:36 PM ]

I agree, this is a low priority enhancement. I think it could make the Protocol experience more DWIMy, and Java 8 has default implementations on interfaces for the same kind of convenience.





[CLJ-1560] Forbid closing over mutable fields completely Created: 10/Oct/14  Updated: 19/Jul/15

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

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


 Description   

Closing over mutable fields should be forbidden completely (and generate compiler exception), not just when trying to set! them. As the change of the mutable field does not propagate into closed over ones, this leads to surprising bugs:

(defprotocol P 
  (-set [this]) 
  (-get [this]) 
  (-get-fn [this]))

(deftype T [^:unsynchronized-mutable val] 
  P 
  (-set [this] (set! val :bar)) 
  (-get [this] val) 
  (-get-fn [this] (fn [] val)))

(def x (->T :foo))

(def xf (-get-fn x))

user> (-set x)
:bar
user> (-get x)
:bar
user> (xf)
:foo ;; should be :bar !!!


 Comments   
Comment by Jozef Wagner [ 10/Oct/14 1:42 PM ]

related issue CLJ-274

Comment by Ghadi Shayban [ 28/Dec/14 10:42 PM ]

In the given example, the close-over happens before the set!, so the closure gets the value, not an assignable container. This is consistent with the rest of the language (pass-by-value not by mutable container)

Comment by Jozef Wagner [ 29/Dec/14 2:21 AM ]

Thanks for explanation. The ticket is about a proposal that closing over a mutable field should result in error being thrown, an not in a value. If value is desired, an explicit let binding will have to be used. So far, I haven't found a valid use case where closing over mutable field and getting the value closed over is the intended and wanted behavior.





[CLJ-1556] Add instance check functions to defrecord/deftype Created: 09/Oct/14  Updated: 09/Oct/14

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

Type: Enhancement Priority: Minor
Reporter: Nicola Mometto Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: defrecord, deftype

Attachments: Text File 0001-CLJ-1556-Generate-type-functions-with-instance-check.patch    
Patch: Code

 Description   

It is often necessarty to test for instance? on deftypes/defrecords, this patch makes the two macros automatically generate a type? function implemented as (fn [x] (instance? type x)), to complement ->type and map->type
Example:

user=>(deftype x [])
user.x
user=>(x? (x.))
true


 Comments   
Comment by Jozef Wagner [ 09/Oct/14 9:11 AM ]

What about camel cased types? predicate SomeType? does not look like an idiomatic type predicate. I suggest to have this type predicate function and its name optional, through e.g. :predicate metadata on a type name. Moreover, it is far more useful to have such predicate on protocols, rather than types.

Comment by Nicola Mometto [ 09/Oct/14 9:17 AM ]

I don't think camel cased types should pose any issue. we use ->SomeType just as fine, I don't see why SomeType? should be problematic.

I disagree that it's more useful to have a predicate for protocols since protocols are already regular Vars and it's just a matter of (satisfies? theprotocol x), the value of the predicate on types/record is to minimize the necessity of having to import the actual class





[CLJ-1553] Parallel transduce Created: 07/Oct/14  Updated: 22/Jan/16

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

Type: Enhancement Priority: Major
Reporter: Alex Miller Assignee: Unassigned
Resolution: Unresolved Votes: 8
Labels: transducers


 Description   

Consider how to create a parallel path for transducers, similar to reducers fold.






[CLJ-1552] Consider kv support for transducers (similar to reducers fold) Created: 07/Oct/14  Updated: 22/Jan/16

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

Type: Enhancement Priority: Major
Reporter: Alex Miller Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: transducers


 Description   

In reducers, fold over a map has special support for kv. Consider whether/how to add this for transducers.



 Comments   
Comment by Marshall T. Vandegrift [ 16/Dec/14 11:13 AM ]

We don't have a JIRA "unvote" feature, but I'd like to register my vote against this proposed enhancement. As a heavy user of clojure.core.reducers, I consider the switch to k-v semantics when reducing a map to be a significant mis-feature. As only an initial transformation function applied directly to a map is able to receive the k-v semantics (a limitation I can’t see how would not carry over to transducers), this behavior crops up most frequently when re-ordering operations and discovering that an intermediate map has now caused an airity error somewhere in the middle of a chain of threaded transformations. I’ve never found cause to invoke it intentionally.

Comment by Ghadi Shayban [ 21/Jan/16 9:17 PM ]

Marshall, there really isn't a proposed enhancement, yet. So there's nothing to be against! Your input is valuable. (Regarding c.c.reducers, that is a separate problem – yes that behavior is surprising)

Considering kv-support for transducers:
Is it useful to have some functions that transform reduce-kv style reducing functions (fn [result k v])?

Ignore naming:
map-key
map-val
map-keyval
filter-*

These could be mechanically generated. You wouldn't have to have a kv-version for every transducer currently in core. Some like map or filter could specifically apply to the key and ignore the val, or v.v.

Some things like map's transducer would be arity-incompatible (map's transducer has a varargs arity).





[CLJ-1550] Classes generated by deftype and defrecord don't play nice with .getPackage Created: 07/Oct/14  Updated: 22/Jul/15

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

Type: Enhancement Priority: Minor
Reporter: Bozhidar Batsov Assignee: Unassigned
Resolution: Unresolved Votes: 15
Labels: classloader, deftype

Attachments: Text File 0001-CLJ-1550-define-package-for-class-in-DynamicClassLoa.patch     Text File CLJ-1550-v2.patch    
Patch: Code and Test

 Description   

Classes generated loaded by DynamicClassLoader return nil for .getPackage

(.getPackage String)
;; => #<Package package java.lang, Java Platform API Specification, version 1.7>
(deftype T [])
(.getPackage T)
;; => nil

This seems like a bug to me as it's not obvious why the class generated by deftype should exhibit different behaviour.

Patch: CLJ-1550-v2.patch



 Comments   
Comment by Alex Miller [ 07/Oct/14 8:54 AM ]

According to http://docs.oracle.com/javase/7/docs/api/java/lang/Class.html#getPackage() this method returns the package information found by the class loader or null if there is none. Its not clear to me that the current behavior is wrong per the spec. I would need to experiment more to see if this is unusual or not.

Comment by Bozhidar Batsov [ 07/Oct/14 9:05 AM ]

A bit of background for the issue. I'm no expert on the topic, but being able to procure all the class information except its package definitely looks strange to me.

Comment by Kevin Downey [ 07/Oct/14 11:46 AM ]

if you AOT compile(generate a class file on disk for a deftype), getPackage works fine, which suggests to me it is a jvm issue

Comment by Kevin Downey [ 07/Oct/14 11:49 AM ]

actually, it must just be that dynamicclassloader doesn't define a package for classes it loads

Comment by Alex Miller [ 07/Oct/14 12:13 PM ]

Yep, I believe that's correct.

Comment by Stuart Halloway [ 21/Jul/15 8:01 AM ]

There is no problem statement here. What is package information needed for?

Comment by Bozhidar Batsov [ 21/Jul/15 8:05 AM ]

I've linked the problem above. Basically tools like CIDER and vim-fireplace are relying on this information to implement things like completion hints.
This might not be problem when running your apps, but it's definitely a problem when inspecting their state...

Comment by Michael Blume [ 22/Jul/15 12:32 PM ]

s/Packate/Package





[CLJ-1548] primitive type hints on protocol methods break call sites Created: 04/Oct/14  Updated: 04/Oct/14

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

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


 Description   
user=> (defprotocol P (f [this ^long x]))
P
user=> (deftype T [] P (f [_ x] x))
#<java.lang.Class class user.T>
user=> (f (T.) 5)

ClassCastException user$eval7289$fn__7290$G__7280__7297 cannot be cast to clojure.lang.IFn$OLO  user/eval7313 (NO_SOURCE_FILE:1)





[CLJ-1543] Type tags on argument vector appear to help avoid reflection when used with defn, but not with def foo (fn ...) Created: 30/Sep/14  Updated: 02/Oct/14

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

Type: Defect Priority: Minor
Reporter: Andy Fingerhut Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: interop, typehints


 Description   

I would have expected that both of the Java interop calls below would avoid reflection, but only the first involving f1 does.

Clojure 1.6.0
user=> (set! *warn-on-reflection* true)
true
user=> (defn f1 ^java.util.LinkedList [coll] (java.util.LinkedList. coll))
#'user/f1
user=> (def f2 (fn ^java.util.LinkedList [coll] (java.util.LinkedList. coll)))
#'user/f2
user=> (.size (f1 [2 3 4]))
3
user=> (.size (f2 [2 3 4]))
Reflection warning, NO_SOURCE_PATH:5:1 - reference to field size can't be resolved.
3

Not sure if this has anything to do with CLJ-1232, but was discovered when testing variants of that issue.



 Comments   
Comment by Andy Fingerhut [ 30/Sep/14 9:08 PM ]

What a nice number for a ticket, 1543. The year Copernicus's most celebrated book was published: http://en.wikipedia.org/wiki/Nicolaus_Copernicus

Comment by Jozef Wagner [ 01/Oct/14 4:05 AM ]

Isn't type hinting of arg vector meant only for primitive type hints? AFAIK non-primitive type hints should be on a function name, everything else is non idiomatic.

Comment by Nicola Mometto [ 01/Oct/14 7:05 AM ]

This isn't an issue of arg vector hinting vs function name hinting.
The issue here is that return type hinting cannot be put on anonymous functions but only on defns as the :arglists will be added by defn on the Var's metadata.

This is one of the reasons why I'd like to have that information as a field on the fn rather than as metadata on the Var

Comment by Andy Fingerhut [ 01/Oct/14 10:55 AM ]

Jozef, you may be correct that non-primitive type hints on the argument vector are non idiomatic. Do you have any source for that I could read?

Comment by Tassilo Horn [ 02/Oct/14 12:19 AM ]

Only the version with hints on the argument vectors is documented at http://clojure.org/java_interop#Java Interop-Type Hints. However, in the case you have just one arity (or all arities return a value of the same type) the hint on the var name also works. But the two versions seem to have different semantics. Have a look at CLJ-1232.

Comment by Jozef Wagner [ 02/Oct/14 5:48 AM ]

Type hinting is a very intricate part of Clojure but you can almost always apply a 'place hint on a symbol' idiom. Type hinting on an arg vector must be done only in two cases:

  • primitive hints
  • different return classes for different arities

In the first case, compiler needs type hints when compiling fn* (see [1]), not later, thus you must specify them on arg vector.

Second case, which is the issue discussed here, must be used only when defining with defn. Compiler first looks for the tag in the metadata of a var, and if it does not find one, it has a special case in which it looks for a return class inside :arglist metadata. This is clearly a very special case [2] to handle situations where you have different return classes for different arities. Obviously, using def instead of defn won't create an :arglist metadata for you thus you see a reflection warning. Example:

user=> (def f2 (fn ^java.util.LinkedList [coll] (java.util.LinkedList. coll)))
#'user/f2
user=> (.size (f2 [2 3 4]))
Reflection warning, /tmp/form-init.clj:1:1 - reference to field size can't be resolved.
3
user=> (alter-meta! #'f2 assoc :arglists '(^java.util.LinkedList [coll]))
{:ns #<Namespace user>, :name f2, :file "/tmp/form-init.clj", :column 1, :line 1, :arglists ([coll])}
user=> (.size (f2 [2 3 4]))
3

BTW CLJ-1491 has a discussion slightly relevant to this topic.

[1] https://github.com/clojure/clojure/blob/03cd9d159a2c49a21d464102bb6d6061488b4ea2/src/jvm/clojure/lang/Compiler.java#L5134
[2] https://github.com/clojure/clojure/blob/03cd9d159a2c49a21d464102bb6d6061488b4ea2/src/jvm/clojure/lang/Compiler.java#L3572

Comment by Jozef Wagner [ 02/Oct/14 7:15 AM ]

Andy, I've found sources that speak against my recommendations See CLJ-811 and [1].

[1] https://groups.google.com/d/msg/clojure/b005zQCPxOQ/6G0AlWKKKa0J





[CLJ-1538] Set literal duplicate check occurs too early. Created: 27/Sep/14  Updated: 09/Oct/14

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

Type: Defect Priority: Minor
Reporter: Chhi'mèd Künzang Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: reader


 Description   

I cannot use literal syntax to create a set/map with unique members/keys if the elements are generated with an identical form. Examples of such legal forms: (rand), (read), (clojure.core.async/<!!), etc. I will use (rand) in these examples.

user=> #{(rand) (rand)}
IllegalArgumentException Duplicate key: (rand)  clojure.lang.PersistentHashSet.createWithCheck (PersistentHashSet.java:68)

user=> {(rand) 1, (rand) 2}

IllegalArgumentException Duplicate key: (rand)  clojure.lang.PersistentArrayMap.createWithCheck (PersistentArrayMap.java:70)

It appears that the input is being checked for duplicates before the arguments to the collection constructors are evaluated. However, this doesn't prevent the need to run the check again later.

Note that duplicates are still (correctly) detected, after evaluation, even if duplicates do not appear as literals in the source:

user=> #{(+ 1 1) 2}

IllegalArgumentException Duplicate key: 2  clojure.lang.PersistentHashSet.createWithCheck (PersistentHashSet.java:56)
user=> {(+ 1 1) :a, 2 :b}

IllegalArgumentException Duplicate key: 2  clojure.lang.PersistentArrayMap.createWithCheck (PersistentArrayMap.java:70)

The first duplicate check therefore seems to be both redundant and incorrect.

Note that this eager duplicate-checking seems to have higher precedence even than the syntax-quote reader macro.

user=> `#{~(rand) ~(rand)}

IllegalArgumentException Duplicate key: (clojure.core/unquote (rand))  clojure.lang.PersistentHashSet.createWithCheck (PersistentHashSet.java:68)

user=> `{~(rand) 1, ~(rand) 2}

IllegalArgumentException Duplicate key: (clojure.core/unquote (rand))  clojure.lang.PersistentArrayMap.createWithCheck (PersistentArrayMap.java:70)

This is odd – since syntax-quote should not realize a collection at all at read time:

For Lists/Vectors/Sets/Maps, syntax-quote establishes a template of the corresponding data structure. Within the template, unqualified forms behave as if recursively syntax-quoted, but forms can be exempted from such recursive quoting by qualifying them with unquote or unquote-splicing, in which case they will be treated as expressions and be replaced in the template by their value, or sequence of values, respectively. (http://clojure.org/reader)

Definitions aside, based on the apparent expansion of syntax-quote, I would expect the previous to have worked correctly.

If I fake the expected macroexpansion by manually substituting the desired inputs, I get the expected results:

user=> '`#{~:a ~:b}
(clojure.core/apply clojure.core/hash-set (clojure.core/seq (clojure.core/concat (clojure.core/list :b) (clojure.core/list :a))))
user=> (clojure.core/apply clojure.core/hash-set (clojure.core/seq (clojure.core/concat (clojure.core/list (rand)) (clojure.core/list (rand)))))
#{0.27341896385866227 0.3051522362827035}
user=> '`{~:a 1, ~:b 2}
(clojure.core/apply clojure.core/hash-map (clojure.core/seq (clojure.core/concat (clojure.core/list :a) (clojure.core/list 1) (clojure.core/list :b) (clojure.core/list 2))))
user=> (clojure.core/apply clojure.core/hash-map (clojure.core/seq (clojure.core/concat (clojure.core/list (rand)) (clojure.core/list 1) (clojure.core/list (rand)) (clojure.core/list 2))))
{0.12476921225204185 2, 0.5807961046096718 1}

It seems to me that there is a superfluous duplicate check being run before the set/map reader macros evaluate their arguments. This check should seemingly be removed. Even if the check did not catch some false-positive duplicates (as it does), it would be unnecessary since the apparent second post-evaluation check would catch all true duplicates.

All that said, it's unclear that this check should happen at all. If I try to create sets/map with duplicate members/keys, I don't get an error. The duplicates are silently removed or superseded.

user=> (set (list 1 1))
#{1}
user=> (hash-map 1 2 1 3)
{1 3}

It seems it would be most consistent for literals constructed by the reader syntax to do the same.

I can see the argument that a literal representation is not a 'request to construct' but rather an attempt to simulate the printed representation of a literal data object. From that perspective, disallowing 'illegal' printed representations seems reasonable. Unfortunately, the possibility of evaluated forms inside literal vectors, sets, and maps (since lists are evaluated at read time) already breaks this theory. That is, the printed representation of such collections is not an accurately readable form, so read-time duplicate checking still cannot prevent seeming inconsistencies in print/read representations:

user=> '#{(+ 1 1) 2}
#{(+ 1 1) 2}
user=> #{(+ 1 1) 2}

IllegalArgumentException Duplicate key: 2  clojure.lang.PersistentHashSet.createWithCheck (PersistentHashSet.java:56)

Given that the problem cannot be completely avoided at all, it seems simplest and most consistent to treat reader literal constructors like their run-time counterparts, as syntax quote would in the absence of the spurious duplicate check.



 Comments   
Comment by Alex Miller [ 09/Oct/14 8:04 AM ]

Also see CLJ-1555

Comment by Nicola Mometto [ 09/Oct/14 8:09 AM ]

Potentially related: http://dev.clojure.org/jira/browse/CLJ-1425





[CLJ-1536] Remove usage of sun.misc.Signal (which may not be available in Java 9) Created: 26/Sep/14  Updated: 26/Sep/14

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

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


 Description   

It looks like Java 9 will not continue to provide access to "internal" classes like sun.misc.Signal. Clojure currently uses this in the REPL to trap ctrl-c (SIGINT) and cancel current evaluation instead of process shutdown.

There is a page of alternatives here:
https://wiki.openjdk.java.net/display/JDK8/Java+Dependency+Analysis+Tool

But there is no suggested alternative for sun.misc.Signal and I'm not aware of a portable solution to it.






[CLJ-1534] Adding condp-> and condp->> macros to core library Created: 24/Sep/14  Updated: 28/Jan/15

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

Type: Enhancement Priority: Major
Reporter: Kuldeep Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: enhancement, macro

Attachments: File clj_1534.diff     File condp-threading-macros-25sept2014.diff    
Patch: Code

 Description   

After introduction of cond-> and cond->> macros in 1.5. It makes sense to have condp-> and condp->> macros in the core library.

(condp-> {}
(complement :a) (assoc :a 1)
:a (assoc :b 2)) ;=> {:b 2, :a 1}

In the above example the result of each expr which was evaluated is being passed to the next predicate.



 Comments   
Comment by Andy Fingerhut [ 01/Oct/14 6:37 PM ]

Kuldeep, I cannot comment on whether this change is of interest to the Clojure developers, because I do not know.

I can say that the patch you have attached is not in the expected format. See the page below for instructions on creating a patch in the expected format:

http://dev.clojure.org/display/community/Developing+Patches

Comment by Kuldeep [ 28/Jan/15 11:31 AM ]

Rebased against master and generated patch as described in wiki.





[CLJ-1532] pr-str captures stdout from printing side-effects of lazily evaluated expressions. Created: 23/Sep/14  Updated: 19/Jul/15

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

Type: Defect Priority: Minor
Reporter: Silas Davis Assignee: Unassigned
Resolution: Unresolved Votes: 7
Labels: print
Environment:

Linux



 Description   

Because clojure.core/pr-str uses with-out-str to capture the output of pr (and pr cannot be parsed a writable thing - just uses out).

If you pr-str the result of something lazy you can get side-effects written to stdout with println interspersed with the output. For example in my case I was extracting benchmarks from the library criterium and trying to print the data structure to the file. The solution would be to provide an overload of pr/pr-str that takes a writer. I note that pr-on provides some of the functionality but it is private.

This is an ugly bug when you're trying to persist program output in EDN, because the randomly interspersed stdout messages make it invalid for read-string. We shouldn't need our functions to be pure for pr-str to work as expected.

I've omitted a patch because although I think a fix is straight-forward I'm not sure quite where it should go (e.g. make pr-on public, change pr, change pr-str)



 Comments   
Comment by Stuart Halloway [ 19/Jul/15 7:48 AM ]

as a workound for this, use print-dup or print-method





[CLJ-1526] clojure.core/> inconsistent behavior wrt to documentation. Created: 17/Sep/14  Updated: 22/Sep/14

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

Type: Enhancement Priority: Minor
Reporter: Phillip Lord Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: math


 Description   

The > function is inconsistent wrt to their behaviour for 0 arity.

user> (doc >)
-------------------------
clojure.core/>
([x] [x y] [x y & more])
  Returns non-nil if nums are in monotonically decreasing order,
  otherwise false.
nil
user> (> 3 2)
true
user> (> 3)
true
user> (>)
ArityException Wrong number of args (0) passed to: core/>  clojure.lang.AFn.throwArity (AFn.java:429)

This is mostly likely to become problematic when using > via apply where

(or (= 0 (count l))
    (apply > l))

It seems that the documentation should be updated, 0-arg case should return true, or the 1-arg case should also throw an exception.

This affects the other comparators also.



 Comments   
Comment by Robert Tweed [ 17/Sep/14 9:48 AM ]

As per my original post on this (here: https://groups.google.com/d/msg/clojure/8zkpO9FBN64/u2LAQsR93IgJ), while the question of whether an empty set has monotonic order perhaps has more than one answer in theory, from a purely pragmatic engineering perspective, it makes the most sense to evaluate to true here.

This /should/ not be a breaking change. Therefore it is fairly safe to introduce into a minor revision. It's a also a trivial fix. But it is possible (though highly unlikely) that someone could have code that depends on the exception being raised at runtime (as it does now) to handle empty lists in some special way. Such code is horrible and ought to be rewritten, so should not be seen as justification for retaining the current behaviour, which limits the general usefulness of these functions and may be responsible for subtle bugs in existing production code.

However such a change should probably not be backported to existing 1.6.x branches, just to be 100% safe, since it is not a security issue. My suggestion therefore would be to add a note to the docs in existing maintenance branches (any future 1.6.x) and evaluate to true in future versions (1.7+).





[CLJ-1521] A little improvement for parsing let expr Created: 07/Sep/14  Updated: 08/Sep/14

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

Type: Enhancement Priority: Trivial
Reporter: dennis zhuang Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: let, parser
Environment:

Mac OSX 10.9.4
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)


Attachments: File improve_parse_let_expr.diff    
Patch: Code

 Description   

The recurMismatches vector in LetExpr parser as see in

https://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/Compiler.java#L6062-6065

There is not necessary to add initialize value 'false' into it when it is not a loop expression.

We can rewrite it into:

if(isLoop)
			    {
				for (int i = 0; i < bindings.count()/2; i++)
				    {
				    recurMismatches = recurMismatches.cons(RT.F);
				    }				
			    }

It's a little improvement for parsing let expression.



 Comments   
Comment by Andy Fingerhut [ 07/Sep/14 11:16 AM ]

Dennis, you might want to clarify the description a little bit, if I understand this ticket correctly. The proposed change would be no change to the behavior of the compiler, except a small speed improvement during compilation?

Comment by dennis zhuang [ 08/Sep/14 2:36 AM ]

Yep,the patch doesn't change the behavior of the compiler.All test is fine.

The recurMismatches vector in LetExpr parser as see in

https://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/Compiler.java#L6062-6065

is only used when detecting type mismatch for loop special form,it's not necessary to be initialized for let special form.So i just added a if(isLoop) clause before initializing it.





[CLJ-1520] assoc-in with empty key path assoc-es to nil Created: 05/Sep/14  Updated: 05/Sep/14

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

Type: Defect Priority: Major
Reporter: Francis Avila Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   
(assoc-in {} [] 1) ;=> {nil 1}

This should probably throw an exception.

CLJ-373 has a patch (CLJ-373-nested-ops.patch) which fixes this (by throwing an exception on empty key paths), the related broken behavior of update-in, and documents empty key path behavior in get-in et al. I can pull just the assoc-in stuff out of that into a separate patch, but I am really hoping that all the issues in the patch addresses are resolved at once, I.e.:

(get-in {} [] :notfound) ;=> {} ; ok
(get-in {nil 1} [] :notfound) ;=> {nil 1} ; ok
(assoc-in {} [] 1) ;=> {nil 1} ; wat?
(assoc-in {nil 0} [] 1) ;=> {nil 1} ; wat?
(update-in {} [] identity) ;=> {nil nil} ; wat?
(update-in {nil 0} [] inc) ;=> {nil 1} ; wat?





[CLJ-1519] Added extra arity to clojure.core/ns-* fns Created: 04/Sep/14  Updated: 10/Sep/14

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

Type: Enhancement Priority: Major
Reporter: Alex Baranosky Assignee: Unassigned
Resolution: Unresolved Votes: 4
Labels: enhancement, patch

Attachments: Text File new-ns-arity.patch    
Patch: Code and Test

 Description   

Hello,

Adds another arity where the "ns" parameter is set to a default value of *ns* in these fns:

ns-unmap, ns-resolve, ns-name, ns-map, ns-publics, ns-imports, ns-interns, ns-refers, ns-aliases, ns-unalias

I find I very often use ns-unalias and ns-unmap from the repl, and passing the *ns* arg gets a little tedious.






[CLJ-1513] Enhancing reader Created: 25/Aug/14  Updated: 25/Aug/14

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

Type: Enhancement Priority: Trivial
Reporter: Anton Rambold Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: edn, reader


 Description   

Attach "character start" and "character end" to the meta information of read forms produced by clojure.lang.EdnReader and clojure.lang.LispReader.
This will allows for better code inspection by linters for example. Currently only line number and column are attached to the meta information.



 Comments   
Comment by Andy Fingerhut [ 25/Aug/14 4:59 PM ]

I am not certain, but perhaps the EDN and regular reader in the tools.reader contrib library already do what you want here? That is, besides :line and :column metadata, they also have :end-line and :end-column metadata for the end of the expression.





[CLJ-1509] Some clojure namespaces not AOT-compiled and included in the clojure jar Created: 20/Aug/14  Updated: 12/Jan/16

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

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

Patch: Code

 Description   

There is a list of namespaces to AOT in build.xml and several namespaces are missing from that list, thus no .class files for those namespaces are created or included in the standard clojure jar file as part of the build.

Missing namespaces include:

  • clojure.core.reducers
  • clojure.instant
  • clojure.parallel
  • clojure.uuid

Proposal: Attached patch sorts the ns list alphabetically (for easier maintenance) and adds clojure.instant and clojure.uuid to the compiled namespaces. clojure.parallel is deprecated and requires the JSR-166 jar so was not included (perhaps it's a separate ticket to remove this). clojure.core.reducers uses a compile-time check to choose the fork/join packages to use so cannot be compiled early.

Patch: clj-1509.diff

Screened by:



 Comments   
Comment by Alex Miller [ 20/Aug/14 1:06 PM ]

Looking at this a bit further, clojure.core.reducers uses the compile-if macro to determine what version of fork/join is available so AOT-compiling this namespace would fix that decision at build time rather than runtime, so it cannot be included.





[CLJ-1508] Supplied-p parameter in clojure Created: 18/Aug/14  Updated: 18/Aug/14

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: destructuring
Environment:

Mac OSX 10.9.4

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)


Attachments: File supplied_p.diff    
Patch: Code and Test

 Description   

As see in https://groups.google.com/forum/?hl=en#!topic/clojure/jWc51JOkvsA

I think we can add a ? option for destructure ,then we can write a test like :

(deftest supplied-p-in-destructuring
  (let [{:keys [a b c d] :p? {a a-p? b b-p? c c-p? d d-p?} :or {a 1}} {:b 2 :c 3 }]
    (is (= a 1))
    (is (false? a-p?))
    (is (= 2 b))
    (is (true? b-p?))
    (is (= 3 c))
    (is (true? c-p?))
    (is (nil? d))
    (is (false? d-p?))))

Even if the a var has a default value 1 by :or option,but the a-p? is still false.
Just like the supplied-p-parameter in Commons LISP.

The patch is attached with code and test.



 Comments   
Comment by Steve Miner [ 18/Aug/14 8:24 AM ]

As mentioned on the mailing list, you could use {:as arg} destructuring to get same information. Here's a slightly modified example that works in the current Clojure:

(deftest supplied-p-in-destructuring
  ;; (let [{:keys [a b c d] :p? {a a-p? b b-p? c c-p? d d-p?} :or {a 1}} {:b 2 :c 3 }]
  (let [{:keys [a b c d] :or {a 1} :as argmap} {:b 2 :c 3 }
        supplied? (partial contains? argmap)
        a-p? (supplied? :a)
        b-p? (supplied? :b)
        c-p? (supplied? :c)
        d-p? (supplied? :d)]
    (is (= a 1))
    (is (false? a-p?))
    (is (= 2 b))
    (is (true? b-p?))
    (is (= 3 c))
    (is (true? c-p?))
    (is (nil? d))
    (is (false? d-p?))))




[CLJ-1507] Throw NPE in eval reader Created: 16/Aug/14  Updated: 18/Jan/16

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: eval-reader
Environment:

Mac OSX 10.9.4
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)


Attachments: File fix_npe_eval_reader.diff    
Patch: Code

 Description   
Clojure 1.7.0-master-SNAPSHOT
user=> #=(var a)
NullPointerException   clojure.lang.Symbol.hashCode (Symbol.java:84)
user=> (.printStackTrace *e)
clojure.lang.LispReader$ReaderException: clojure.lang.LispReader$ReaderException: java.lang.NullPointerException
	at clojure.lang.LispReader.read(LispReader.java:218)
	at clojure.core$read.invoke(core.clj:3580)
	at clojure.core$read.invoke(core.clj:3578)
	at clojure.core$read.invoke(core.clj:3576)
	at clojure.core$read.invoke(core.clj:3574)
	at clojure.main$repl_read.invoke(main.clj:139)
	at clojure.main$repl$read_eval_print__6807$fn__6808.invoke(main.clj:237)
	at clojure.main$repl$read_eval_print__6807.invoke(main.clj:237)
	at clojure.main$repl$fn__6816.invoke(main.clj:257)
	at clojure.main$repl.doInvoke(main.clj:257)
	at clojure.lang.RestFn.invoke(RestFn.java:421)
	at clojure.main$repl_opt.invoke(main.clj:323)
	at clojure.main$main.doInvoke(main.clj:421)
	at clojure.lang.RestFn.invoke(RestFn.java:397)
	at clojure.lang.Var.invoke(Var.java:375)
	at clojure.lang.AFn.applyToHelper(AFn.java:152)
	at clojure.lang.Var.applyTo(Var.java:700)
	at clojure.main.main(main.java:37)
Caused by: clojure.lang.LispReader$ReaderException: java.lang.NullPointerException
	at clojure.lang.LispReader.read(LispReader.java:218)
	at clojure.lang.LispReader$CtorReader.invoke(LispReader.java:1164)
	at clojure.lang.LispReader$DispatchReader.invoke(LispReader.java:609)
	at clojure.lang.LispReader.read(LispReader.java:183)
	... 17 more
Caused by: java.lang.NullPointerException
	at clojure.lang.Symbol.hashCode(Symbol.java:84)
	at java.util.concurrent.ConcurrentHashMap.hash(ConcurrentHashMap.java:332)
	at java.util.concurrent.ConcurrentHashMap.get(ConcurrentHashMap.java:987)
	at clojure.lang.Namespace.findOrCreate(Namespace.java:173)
	at clojure.lang.RT.var(RT.java:341)
	at clojure.lang.LispReader$EvalReader.invoke(LispReader.java:1042)
	at clojure.lang.LispReader$DispatchReader.invoke(LispReader.java:616)
	at clojure.lang.LispReader.read(LispReader.java:183)
	... 20 more

If the var symbol doesn't contains namespace ,it will throw the NPE exception in above code.Instead,i think it should use Compiler.currentNS() when doesn't find the var's namespace.

The patch is attached, after patched:

Clojure 1.7.0-master-SNAPSHOT
user=> #=(var a)
#'user/a





[CLJ-1506] A little improvement when reading syntax quote form Created: 16/Aug/14  Updated: 30/Aug/14

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: syntax-quote
Environment:

Mac OSX 10.9.4
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)


Attachments: File fast_syntax_quote_reader.diff    
Patch: Code

 Description   

When reading syntax quote on keyword,string or number etc,it returns the form as result directly. Read it in:
https://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/LispReader.java#L844-847

else if(form instanceof Keyword
       || form instanceof Number
       || form instanceof Character
       || form instanceof String)
   ret = form;

But missing check if it is a nil,regular pattern or boolean constants.
After patched:

else if(form == null
       || form instanceof Keyword
       || form instanceof Number
       || form instanceof Character
       || form instanceof Pattern
       || form instanceof Boolean
       || form instanceof String)
    ret = form;

It's a little patch, i am not sure if it is worth a try.






[CLJ-1504] Add :inline to most core predicates Created: 15/Aug/14  Updated: 15/Aug/14

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

Attachments: Text File 0001-add-inline-to-some-core-predicates.patch    
Patch: Code

 Description   

This will allow instance? predicates calls to be emitted using the instanceof JVM bytecode and will also allow tools like core.typed or tools.analyzer.jvm to infer the type of a var/local on a per branch basis without having to special-case all the core predicates.



 Comments   
Comment by Jozef Wagner [ 15/Aug/14 1:32 PM ]

Related ticket CLJ-1227 and related quote from Alex:

definline is considered to be an experimental feature and Rich would like to discourage its use as the hope is to remove it in the future. The desired replacement is something like common lisp compiler macros that could allow the compiler to detect special situations and optimize the result but leave behind a function invocation for the case where no special behavior is available.

Comment by Nicola Mometto [ 15/Aug/14 1:42 PM ]

This patch uses "manual" :inline metadata on functions, it's used by many other core functions (like +,- et), not definline so Rich's comment doesn't apply.





[CLJ-1502] Clojure Inspector navigation error Created: 12/Aug/14  Updated: 15/Aug/14

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

Type: Defect Priority: Minor
Reporter: Dan Campbell Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: bug, inspector, navigation
Environment:

Windows 7 and 8, Java 7, Clojure repl


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

 Description   

With Clojure 1.6.0 on some platforms (details below), if you create an object such as

(def nst (vec '((3 7 22) 99 (123 18 225 437))))

and then you inspect the tree representing the object

(inspect-tree nst)

Most of the navigation with the keyboard proceeds fine. However, when you point to an individual value - e.g. the 99 or the 437 - and press the right arrow key, there is an error

Exception in thread "AWT-EventQueue-0" java.lang.UnsupportedOperationException: count not supported on this type: Long
	at clojure.lang.RT.countFrom(RT.java:556)
	at clojure.lang.RT.count(RT.java:530)
	at clojure.inspector$fn__6907.invoke(inspector.clj:40)
	at clojure.lang.MultiFn.invoke(MultiFn.java:227)
	at clojure.inspector$tree_model$fn__6929.invoke(inspector.clj:63)
	at clojure.inspector.proxy$java.lang.Object$TreeModel$775afa87.getChildCount(Unknown Source)
	at javax.swing.plaf.basic.BasicTreeUI$Actions.traverse(BasicTreeUI.java:4395)
	at javax.swing.plaf.basic.BasicTreeUI$Actions.actionPerformed(BasicTreeUI.java:4052)
	at javax.swing.SwingUtilities.notifyAction(SwingUtilities.java:1662)
	at javax.swing.JComponent.processKeyBinding(JComponent.java:2878)
	at javax.swing.JComponent.processKeyBindings(JComponent.java:2925)
	at javax.swing.JComponent.processKeyEvent(JComponent.java:2841)
	at java.awt.Component.processEvent(Component.java:6282)
	at java.awt.Container.processEvent(Container.java:2229)
	at java.awt.Component.dispatchEventImpl(Component.java:4861)
	at java.awt.Container.dispatchEventImpl(Container.java:2287)
	at java.awt.Component.dispatchEvent(Component.java:4687)
	at java.awt.KeyboardFocusManager.redispatchEvent(KeyboardFocusManager.java:1895)
	at java.awt.DefaultKeyboardFocusManager.dispatchKeyEvent(DefaultKeyboardFocusManager.java:762)
	at java.awt.DefaultKeyboardFocusManager.preDispatchKeyEvent(DefaultKeyboardFocusManager.java:1027)
	at java.awt.DefaultKeyboardFocusManager.typeAheadAssertions(DefaultKeyboardFocusManager.java:899)
	at java.awt.DefaultKeyboardFocusManager.dispatchEvent(DefaultKeyboardFocusManager.java:727)
	at java.awt.Component.dispatchEventImpl(Component.java:4731)
	at java.awt.Container.dispatchEventImpl(Container.java:2287)
	at java.awt.Window.dispatchEventImpl(Window.java:2719)
	at java.awt.Component.dispatchEvent(Component.java:4687)
	at java.awt.EventQueue.dispatchEventImpl(EventQueue.java:735)
	at java.awt.EventQueue.access$200(EventQueue.java:103)
	at java.awt.EventQueue$3.run(EventQueue.java:694)
	at java.awt.EventQueue$3.run(EventQueue.java:692)
	at java.security.AccessController.doPrivileged(Native Method)
	at java.security.ProtectionDomain$1.doIntersectionPrivilege(ProtectionDomain.java:76)
	at java.security.ProtectionDomain$1.doIntersectionPrivilege(ProtectionDomain.java:87)
	at java.awt.EventQueue$4.run(EventQueue.java:708)
	at java.awt.EventQueue$4.run(EventQueue.java:706)
	at java.security.AccessController.doPrivileged(Native Method)
	at java.security.ProtectionDomain$1.doIntersectionPrivilege(ProtectionDomain.java:76)
	at java.awt.EventQueue.dispatchEvent(EventQueue.java:705)
	at java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:242)
	at java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:161)
	at java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:150)
	at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:146)
	at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:138)
	at java.awt.EventDispatchThread.run(EventDispatchThread.java:91)

Environments where this has been reproduced:
+ Windows 7 Enterprise, SP1, Oracle JDK 1.7.0_51, Clojure 1.6.0
+ Ubuntu Linux 14.04.1, Oracle JDK 1.7.0_65, Clojure 1.6.0

Environments where the same sequence of events does not cause an exception:
+ Mac OS X 10.8.5, Oracle JDK 1.7.0_51, Clojure 1.6.0



 Comments   
Comment by Andy Fingerhut [ 13/Aug/14 6:08 PM ]

Patch clj-1502-v1.patch avoids the exception in the situation reported. Tested manually on OS X, Linux, and Windows 7 versions mentioned in the patch comment. I suspect it is not worth the effort to write an automated test for this.

Comment by Dan Campbell [ 15/Aug/14 6:40 PM ]

Thanks, Andy

  • DC




[CLJ-1496] Added a new arity to 'ex-info' that only accepts a message. Created: 08/Aug/14  Updated: 11/Aug/14

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: ex-info, exceptions
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.9.4


Attachments: File ex_info_arity.diff    
Patch: Code

 Description   

We often use 'ex-info' to throw a custom exception.But ex-info at least accepts two arguments: a string message and a data map.
In most cases,but we don't need to throw a exception that taken a data map.
So i think we can add a new arity to ex-info:

(ex-info "the exception message")

That created a ExceptionInfo instance carries empty data.

I am not sure it's useful for other people,but it's really useful for our developers.

The patch is attached.






[CLJ-1489] Implement var-symbol Created: 02/Aug/14  Updated: 06/Aug/14

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

Type: Enhancement Priority: Trivial
Reporter: Reid McKenzie Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File 0001-Implement-var-symbol.patch    

 Description   

var-symbol provides the obvious complement operation to resolve. Where resolve maps from a symbol to a var by resolving it in the environment, var-symbol allows a user to recover the root binding symbol from a var if the var is named. If the var is not named, var-symbol returns nil.

This is related to CLJ-1488 in that it handles the common case of symbolically manipulating Vars in terms of the Symbols they bind without requiring that users manually reconstruct the bound symbol. Futhermore this patch nicely handles the non-obvious implementation consequent case of an unnamed var.

Depends on CLJ-1488



 Comments   
Comment by Andy Fingerhut [ 06/Aug/14 2:30 PM ]

Patch 0001-Implement-var-symbol.patch dated Aug 2 2014 does not apply cleanly. I haven't checked whether it used to apply cleanly before some commits made to Clojure master earlier today, but if it did, then those commits have made this patch become 'stale'.

See the section "Updating stale patches" at http://dev.clojure.org/display/community/Developing+Patches for suggestions on how to update patches.





[CLJ-1486] Make fnil var-arg Created: 31/Jul/14  Updated: 18/Sep/14

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

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

Attachments: Text File 0001-make-fnil-vararg.patch    
Patch: Code and Test

 Description   

Currently fnil is defined only for 1 to 3 args, this patch makes it var-arg






[CLJ-1482] Replace a couple of (filter (complement ...) ...) usages with (remove ...) Created: 27/Jul/14  Updated: 27/Jul/14

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

Type: Enhancement Priority: Trivial
Reporter: Bozhidar Batsov Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: enhancement

Attachments: Text File 0001-Replace-a-couple-of-filter-complement-usages-with-re.patch    
Patch: Code

 Description   

The title basically says it all - remove exists so we can express our intentions more clearly.






[CLJ-1471] Option to print type info Created: 21/Jul/14  Updated: 21/Jul/14

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

Type: Enhancement Priority: Minor
Reporter: Pascal Germroth Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: pprint


 Description   

I've had an issue with defrecord-types being converted into ordinary maps somewhere, which was relatively hard to track down inside a deep structure since they are pprinted as the same thing by default.
The following code patches into the pprint dispatch and prints the type around values; it turned out to be quite useful, but feels hackish.
Maybe something like that would be useful to integrate into clojure.pprint directly (there are a number of cosmetic options already), i.e. into clojure.pprint/write-out.

Only printing (type) may not be enough in some cases; so an option to print all metadata would be nice.
Maybe something like :metadata nil as default, :metadata :type to print types (but also for non-IMetas, using (type) and :metadata true to print metadata for IMetas using (meta).

(defn pprint-with-type
  ([object] (pprint object *out*))
  ([object writer]
   ; keep original dispatch.
   ; calling it directly will print only that object,
   ; but return to our dispatch for subobjects.
   (let [dispatch clojure.pprint/*print-pprint-dispatch*]
     (binding [clojure.pprint/*print-pprint-dispatch*
               (fn [obj]
                 (if (instance? clojure.lang.IMeta obj)
                   (do (print "^{:type ")
                       (dispatch (type obj))
                       (print "} ")
                       (clojure.pprint/pprint-newline :fill)
                       (dispatch obj))
                   (do (print "(^:type ")
                       (dispatch (type obj))
                       (print " ")
                       (clojure.pprint/pprint-newline :fill)
                       (dispatch obj)
                       (print ")"))))]
       (clojure.pprint/pprint object writer)))))





[CLJ-1470] Make Atom and ARef easy to subclass Created: 20/Jul/14  Updated: 23/Jul/14

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

Type: Enhancement Priority: Minor
Reporter: Aaron Craelius Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJ-1470-v1.patch    
Patch: Code

 Description   

Atom is currently defined as final and ARef.validate() is package-private. This makes it impossible to define a subclass of an Atom and difficult subclass ARef (if validate() needs to be called).

I propose removing the final modifier from Atom, making ARef.validate() protected and also making Atom.state protected (it is currently package-private).

I'm not sure if there is a specific reason why Atom is final - if this is for performance reasons or to prevent someone from doing strange things with Atom's, but I can see a use case for sub-classing it.

One use-case is to create reactive Atom that allows derefs to be tracked (as in reagent). I have some Clojure (not Clojurescript) code where I'm trying to play with this idea and I've had to copy the entire Atom class (because it's sealed) and place it in the clojure.lang package (because ARef.validate() is package-private): https://github.com/aaronc/freactive/blob/master/src/java/clojure/lang/ReactiveAtom.java. In addition, I need to copy the defns for swap! and reset! into my own namespace. This seems a bit inconvenient.



 Comments   
Comment by Kevin Downey [ 23/Jul/14 12:55 AM ]

related to http://dev.clojure.org/jira/browse/CLJ-803





[CLJ-1469] Emit KeywordInvoke callsites only when keyword is not namespaced Created: 18/Jul/14  Updated: 22/Oct/14

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

Type: Enhancement Priority: Trivial
Reporter: Ghadi Shayban Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None

Attachments: Text File kwinvoke.patch    
Patch: Code

 Description   

Summary: Don't emit KeywordLookup thunks and machinery for namespaced keyword access

Description: When the compiler sees a keyword at the beginning of a sexpr, (:foo x), it emits some machinery that takes into account that 'x' could be a defrecord with a defined 'foo' field. This exists to fast-path it into a field lookup. Here is the supporting code from the target defrecord: https://github.com/clojure/clojure/blob/master/src/clj/clojure/core_deftype.clj#L185-L198
The compiler currently emits the same machinery for (:foo/bar x), a namespaced keyword access, but defrecords don't have any fast path field access for that. This trivial patch turns that scenario into a normal invocation.

Here is the disassembly for (fn [x] (:foo/bar x))
https://gist.github.com/anonymous/d94fc56fba4a1665f73f

There are two static fields on the IFn also for every kw access.

With the trivial patch, it turns into a normal invoke. (emit the fn aka the namespaced keyword, then the args Aka the target, and call IFn invoke: kw.invoke(target))






[CLJ-1467] Implement Comparable in PersistentList Created: 17/Jul/14  Updated: 15/Jun/16

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

Type: Enhancement Priority: Major
Reporter: Pascal Germroth Assignee: Unassigned
Resolution: Unresolved Votes: 8
Labels: collections

Attachments: Text File 0001-first-try-for-adding-compare.patch    
Patch: Code and Test

 Description   

PersistentVector implements Comparable already.



 Comments   
Comment by Bart Kastermans [ 13/Nov/14 11:17 AM ]

Patch for this issue; done with Jeroen van Dijk and Razvan Petruescu at a clojure meetup. Any feedback welcome; the learning for me here is not the fix, but learning how to deal with ant and jira etc.

Comment by Andy Fingerhut [ 13/Nov/14 12:31 PM ]

Looks like you have navigated the steps for creating a patch in the desired format, and attaching it to a JIRA ticket, just fine. I see your name on the list of contributors, which is a precondition before a patch can be committed to Clojure or a contrib library.

You've gotten past what are actually the easier parts. There is still the issue of whether this ticket is even considered by the Clojure core team to be an enhancement worth making a change to Clojure. Take a look at the JIRA workflow here if you haven't seen it already and are curious: http://dev.clojure.org/display/community/JIRA+workflow

If you like Pascal think that this is a change you really want to see in Clojure, you may vote on this or any other JIRA ticket (except ones you create yourself – the creator is effectively the 0th voter for a ticket). Log in and click on the Vote link near the top right, and/or Watch to get email updates of changes.

Comment by Bart Kastermans [ 14/Nov/14 3:12 AM ]

Andy, thanks for the info. I was not aware of the JIRA workflow.

Comment by Nicola Mometto [ 15/Jun/16 10:49 AM ]

It seems like other sequences besides PersistentLists should be comparable, one obvious example is `clojure.lang.APersistentVector$RSeq`. I stumbled upon it not being Comparable when trying to rewrite `(sort-by (juxt val key) m)` as `(sort-by rseq m)`





[CLJ-1463] Providing own ClassLoader for eval is broken Created: 10/Jul/14  Updated: 13/Jun/16

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

Type: Enhancement Priority: Minor
Reporter: Volkert Oakley Jurgens Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: compiler
Environment:

Clojure 1.6.0



 Description   

clojure.lang.Compiler has a method with the signature

public static Object eval(Object form, boolean freshLoader)

but the freshLoader argument is ignored since https://github.com/clojure/clojure/commit/2c2ed386ed0f6f875342721bdaace908e298c7f3

Is there a good reason this still needs to be "hotfixed" like this?

We would like to provide our own ClassLoader for eval to manage the lifecycle of the generated classes.



 Comments   
Comment by Stuart Halloway [ 21/Jul/15 8:04 AM ]

This is not part of the public API of Clojure. We would need to understand more about the use case.

Comment by Stephen Nelson [ 12/Jun/16 9:33 PM ]

Sorry Stuart, we only just noticed your response, thanks to the publicity around http://ashtonkemerling.com/blog/2016/06/11/my-increasing-frustration-with-clojure/

I'm going to try and explain our use-case a bit for context, but please understand that this issue was simply a question about what appears to be inconsistent behaviour from a function that looks and smells a lot like a public API function (who would have thought 'eval' would be private

Our company uses Clojure to build a cloud platform that does computation in response to user requests using modules loaded from a database. The modules are trusted code, but they are independent from our main platform so there might be multiple versions of the same module running at the same time (we want to avoid namespace collisions). We've looked at lots of approaches to keep modules from interfering with each other including containers and micro services running separate JVMs, but in order to have acceptable response times for simple queries like "is this input valid?" we want to run simple queries in the same JVM as the web server.

The general approach we use to answer a query from a user is to build a namespace for our computation (which might require loading other namespaces from the computation module), then eval the expression in the context we've built. We have a LRU cache for module namespaces but we still end up with a lot of metaspace churn for the eval, which we mitigate by using a clojure interpreter to handle simple queries (eval is too slow).

When we implemented our LRU modules namespace cache we wanted to experiment with loading module namespaces into their own class loader to help track class lifecycle and GC, and hopefully to allow multiple namespace versions to coexist. We've since concluded that this is impractical because clojure has so many global lookups related to namespaces, so now we preprocess module namespaces and perform name mangling on load, and explicitly unregister loaded namespaces when the cache expires so that their classes can be collected. We avoid using language features like multimethods and protocols that use globals in modules.

Once again, we're not looking for the Clojure team to implement containers for us (though that would certainly be a nice feature to have!), this was simply an inconsistency we noticed between API and implementation. What is the expected entry point for Java-interop eval?

Comment by Alex Miller [ 13/Jun/16 4:57 PM ]

You can use the Clojure Java API as documented at http://clojure.github.io/clojure/javadoc/clojure/java/api/Clojure.html.

A basic example that read and eval'ed code from a Java string would look like:

import clojure.java.api.Clojure;
import clojure.lang.IFn;

// ...

IFn read = Clojure.var("clojure.core", "read-string");
IFn eval = Clojure.var("clojure.core", "eval");

Object code = read.invoke("(+ 1 1)");
Object result = eval.invoke(code);
System.out.println("read: " + code + ", eval: " + result);

You could do more complicated things though like generate a string for a namespace and call load-string via the same mechanism as above.





[CLJ-1462] cl-format throws ClassCastException: Writer cannot be cast to Future/IDeref Created: 07/Jul/14  Updated: 09/Jul/14

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

Type: Defect Priority: Minor
Reporter: Pascal Germroth Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: print


 Description   

Using ~I and ~_ etc fails in many situations, the most trivial one being:

Clojure 1.6.0 and 1.5.1:

user=> (clojure.pprint/cl-format true "~I")
ClassCastException java.io.PrintWriter cannot be cast to java.util.concurrent.Future  clojure.core/deref-future (core.clj:2180)
user=> (clojure.pprint/cl-format nil "~I")
ClassCastException java.io.StringWriter cannot be cast to java.util.concurrent.Future  clojure.core/deref-future (core.clj:2180)
user=> (clojure.pprint/cl-format nil "~_")
ClassCastException java.io.StringWriter cannot be cast to java.util.concurrent.Future  clojure.core/deref-future (core.clj:2180)

Clojure 1.4.0

user=> (clojure.pprint/cl-format true "~I")
ClassCastException java.io.OutputStreamWriter cannot be cast to clojure.lang.IDeref  clojure.core/deref (core.clj:2080)
user=> (clojure.pprint/cl-format nil "~I")
ClassCastException java.io.StringWriter cannot be cast to clojure.lang.IDeref  clojure.core/deref (core.clj:2080)
user=> (clojure.pprint/cl-format nil "~_")
ClassCastException java.io.StringWriter cannot be cast to clojure.lang.IDeref  clojure.core/deref (core.clj:2080)

These work in other implementations, i.e. clisp, creating empty output in these trivial cases:

> (format t "~I")
NIL
> (format nil "~I")
""
> (format nil "~_")
""


 Comments   
Comment by Andy Fingerhut [ 07/Jul/14 11:01 AM ]

The tilde-underscore sequence is for "conditional newline", according to the CLHS here: http://www.lispworks.com/documentation/lw51/CLHS/Body/22_cea.htm

Tilde-capital-letter-I is for indent: http://www.lispworks.com/documentation/lw51/CLHS/Body/22_cec.htm

Comment by Pascal Germroth [ 07/Jul/14 12:09 PM ]

Ah, didn't think to try that. It fails without cl-format as well:

user=> (clojure.pprint/pprint-newline :linear)
ClassCastException java.io.PrintWriter cannot be cast to java.util.concurrent.Future  clojure.core/deref-future (core.clj:2180)
user=> (clojure.pprint/pprint-indent :block 0)
ClassCastException java.io.PrintWriter cannot be cast to java.util.concurrent.Future  clojure.core/deref-future (core.clj:2180)

Manually creating a pretty writer does work though:

user=> (binding [*out* (clojure.pprint/get-pretty-writer *out*)] (clojure.pprint/pprint-newline :linear))
nil

In the get-pretty-writer doc it says:

Generally, it is unnecessary to call this function, since pprint,
write, and cl-format all call it if they need to.

Which appears to not be true for cl-format, and it would be nice if it would be applied automatically for all functions that need a pretty writer.

Comment by Pascal Germroth [ 09/Jul/14 6:37 PM ]

More bad news!
Manually creating a pretty-writer doesn't do the trick either, because it is not being properly flushed:

user=> (binding [*out* (get-pretty-writer *out*)] (cl-format true "hello ~_world~%"))
hello world
nil
user=> (binding [*out* (get-pretty-writer *out*)] (cl-format true "hello ~_world"))
hellonil
user=> (binding [*out* (get-pretty-writer *out*)] (cl-format true "hello ~_world") (.ppflush *out*))
hello worldnil

The ~% inserts an unconditional newline like \n, which also works as expected.

Insert ~_ before and it only prints up to that one. But I've also managed to get it to abort at other ~_ s, maybe because other commands flushed it.

Manually flushing it, like the inexplicably private with-pretty-writer macro does works though.
I don't understand why get-pretty-writer is exposed but not the macro that is needed to use it properly. Also all functions using pretty-writer facilities should use with-pretty-writer, that's what it appears to be specifically designed for. Then there's no need to expose it (or get-pretty-writer).





[CLJ-1459] records should support transient Created: 05/Jul/14  Updated: 06/Jul/14

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

Type: Enhancement Priority: Major
Reporter: Yongqian Li Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: defrecord


 Description   

user=> (defrecord R [a])
user.R
user=> (transient (->R nil))
ClassCastException user.R cannot be cast to clojure.lang.IEditableCollection clojure.core/transient (core.clj:3060)






[CLJ-1455] Postcondition in defrecord: Compiler unable to resolve symbol % Created: 28/Jun/14  Updated: 29/Jun/14

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

Type: Enhancement Priority: Minor
Reporter: Phill Wolf Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: defrecord


 Description   

Clojure's postconditions[1] are a splendiferous, notationally
idiot-proof way to scrutinize a function's return value without
inadvertently causing it to return something else.

Functions (implementing protocols) for a record type may be defined in
its defrecord or with extend-type. In functions defined in
extend-type, postconditions work as expected. Therefore, it is a
surprise that functions defined in defrecord cannot use
postconditions.

Actually it appears defrecord sees a pre/postcondition map as ordinary
code, so the postcondition runs at the beginning of the function (not
the end) and the symbol % (for return value) is not bound.

The code below shows a protocol and two record types that implement
it. Type "One" has an in-the-defrecord function definition where the
postcondition does not compile. Type "Two" uses extend-type and the
postcondition works as expected.

Unable to find source-code formatter for language: clojure. Available languages are: javascript, sql, xhtml, actionscript, none, html, xml, java
(defprotocol ITimesThree
  (x3 [a]))

;; defrecord with functions inside cannot use postconditions.
(defrecord One
    []
  ITimesThree
  (x3 [a]
    {:pre [(do (println "One x3 pre") 1)] ;; (works fine)
     :post [(do (println "One x3 post, %=" %) 1)]
     ;; Unable to resolve symbol: % in this context.
     ;; With % removed, it compiles but runs at start, not end.
     }
    (* 1 3)))

;; extend-type can add functions with postconditions to a record.
(defrecord Two
    [])
(extend-type Two
  ITimesThree
  (x3 [a]
    {:pre [(do (println "Two x3 pre") 1)] ;; (works fine)
     :post [(do (println "Two x3 post, %=" %) 1)] ;; (works fine)
     }
    (* 2 3)))

(defn -main
  "Main"
  []
  (println (x3 (->One)))
  (println (x3 (->Two))))

[1] http://clojure.org/special_forms, in the fn section.






[CLJ-1447] Make proxy work with protocols directly (like reify does) Created: 18/Jun/14  Updated: 18/Jun/14

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

Type: