<< Back to previous view

[CLJ-2100] s/nilable form should include the original spec, not the resolved spec Created: 19/Jan/17  Updated: 19/Jan/17

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

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

Attachments: Text File clj-2100.patch    
Patch: Code
Approval: Vetted

 Description   
=> (clojure.spec/def :test/name string?)
:test/name

=> (clojure.spec/form (clojure.spec/and :test/name))
(clojure.spec/and :test/name)

=> (clojure.spec/form (clojure.spec/nilable :test/name))
(clojure.spec/nilable clojure.core/string?)

In the final line, s/nilable form has the resolved spec rather than the original spec.

Proposed: Instead of getting the internal spec description, resolve the original spec form.

After:

user=> (clojure.spec/form (clojure.spec/nilable :test/name))
(clojure.spec/nilable :test/name)

Patch: CLJ-2100.patch






[CLJ-2099] Keywords with aliased namespaces cannot be read when the namespace is required in a reader conditional Created: 13/Jan/17  Updated: 13/Jan/17  Resolved: 13/Jan/17

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

Type: Defect Priority: Minor
Reporter: Viktor Magyari Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: keywords, reader, readerconditionals


 Description   

The title in itself isn't entirely true, but I couldn't find a way to describe it succintly (feel free to change).

The issue is easier to demonstrate with an example:

(ns foo
  #?(:cljs (:require [clojure.core :as c])))

#?(:cljs ::c/x)

When reading this in a :clj context, the reader cannot read ::c/x ("Invalid token: ::c/x"), despite the code being correct (presumably).
The same thing happens if the reader conditional branches are :clj and the source is read in a :cljs context.



 Comments   
Comment by Alex Miller [ 13/Jan/17 8:07 AM ]

This looks like expected behavior to me. Auto-resolved keywords rely on a resolution context and there isn't one at the point where ::c/x is being read.

Comment by Viktor Magyari [ 13/Jan/17 9:05 AM ]

To me it seems reasonable to expect the resolution context to include the clojure.core alias - more generally, include <platform> specific aliases in the <platform> branches of reader conditionals. Maybe consider this as an enhancement ticket?

Comment by Alex Miller [ 13/Jan/17 9:18 AM ]

To do this would require adding special handling specifically for ns or other things that create aliases, which implies conditional evaluation of some forms at read-time. You would also need some place (other than the current platform's namespace maps) to track other platform's namespace aliases. That's a lot of very special, custom stuff.

We're not going to add this.





[CLJ-2098] autodoc fails to load clojure/spec.clj Created: 12/Jan/17  Updated: 13/Jan/17

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

Type: Defect Priority: Critical
Reporter: Alex Miller Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: spec

Attachments: Text File clj-2098.patch    
Patch: Code
Approval: Vetted

 Description   

Reported by Tom F for the autodoc process. The following (essentially) is what autodoc is doing that currently blows up:

tom@renoir:~/src/clj/autodoc-work-area/clojure/src$ java -cp clojure.jar clojure.main
Clojure 1.9.0-master-SNAPSHOT
user=> (load-file "src/clj/clojure/spec.clj")
CompilerException java.lang.IllegalArgumentException: No implementation of method: :conform* \
of protocol: #'clojure.spec/Spec found for class: clojure.spec$regex_spec_impl$reify__14279, \
compiling:(/home/tom/src/clj/autodoc-work-area/clojure/src/src/clj/clojure/spec.clj:684:1)

Cause: There is code in Compiler.macroexpand() with the intention to suspend spec checking inside clojure.spec. However, it is currently doing an exact path match on SOURCE_PATH. In the load-file call above, this ends up being an absolute path.

Approach: Check for a path suffix rather than an exact match in Compiler.

Patch: clj-2098.patch






[CLJ-2097] Improve generation failure exception Created: 10/Jan/17  Updated: 11/Jan/17

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

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


 Description   

It's pretty easy to write a spec whose generator fails like this:

Couldn't satisfy such-that predicate after 100 tries.

This is of course expected in many ways, but it's a very unhelpful error. Some things that could make this better include:

  • Including the spec that failed in the exception. I only see one invocation of gen/such-that in spec.clj, and it appears to have the spec's form at hand. gen/such-that takes an exception constructor where this could be used.
  • Allow max-tries to be changed from the hardcoded value of 100. When dealing with an intermittent failure, it can be useful to crank down max-tries to a very small number, making the failure easier to reproduce.


 Comments   
Comment by Alex Miller [ 11/Jan/17 8:41 AM ]

These are reasonable suggestions and this area is likely to evolve in tandem with test.check to provide better info.





[CLJ-2096] "Key must be integer" thrown when vector lookup done with short or byte Created: 04/Jan/17  Updated: 06/Jan/17  Resolved: 05/Jan/17

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

Type: Defect Priority: Major
Reporter: Aaron Cummings Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None
Environment:

clojure-1.9.0-alpha14 and clojure-1.8.0 tested.



 Description   

Looking up a value in a vector with a long or integer index works:
([:a :b :c] (long 1)) => :b
([:a :b :c] (int 1)) => :b

However, lookups with short or byte fail with "IllegalArgumentException Key must be integer"
([:a :b :c] (short 1))
([:a :b :c] (byte 1))

Root cause seems to be clojure.lang.Util.isInteger() which returns true only if the argument is an instance of Integer, Long, clojure.lang.BigInt, or BigInteger. I think it would be more correct for clojure.lang.Util.isInteger() to be consistent with clojure.core/integer? which additionally returns true for both Short and Byte.



 Comments   
Comment by Alex Miller [ 05/Jan/17 8:24 AM ]

I don't see any good reason to make changes in this area so declining.

Comment by Aaron Cummings [ 05/Jan/17 8:44 AM ]

Hi Alex - the case where we ran into this exception was in parsing a binary file where the record descriptor is a byte, and we used this byte to do a lookup in a vector (the vector holding keywords which describe the record type). The workaround is pretty simple though; just cast the byte to an int.

Curiously, a map lookup like ({0 :a, 1 :b, 2 :c} (byte 1)) does work.

I wondering though if the non-support of short and byte lookups in vectors is intentional, and if so, the reason for this choice (I don't see any obvious problems, so perhaps Rich knows something I don't here). If instead this is an oversight, and is deemed not broken enough to fix, then I can accept that.

Comment by Alex Miller [ 06/Jan/17 8:26 AM ]

I would say that given that the check and error message exists, it is intentional. Certainly there is a performance impact to checking for a broader set of types.





[CLJ-2095] Doc s/gen overrides do not take effect inside custom generators Created: 03/Jan/17  Updated: 03/Jan/17

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

Type: Enhancement Priority: Minor
Reporter: Russell Mull Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring, generator, spec
Environment:

clojure 1.9.0-alpha14



 Description   

Custom generators may build (via fmap/bind) on spec generators. Generator overrides at the top level will not take effect inside custom generators:

(require '[clojure.spec :as s])
(require '[clojure.test.check.generators :as gen])

;; A map that holds a single integer value
(s/def ::val integer?)
(s/def ::body (s/keys :req [::val]))

;; This spec matches stringified versions of 'body'.
;; (read-string is for demonstration purposes only, of course)
(s/def ::stringy-body
  (s/with-gen
    (s/and string? #(s/valid? ::body (read-string %)))
    #(gen/fmap pr-str (s/gen ::body))))

(s/valid? ::stringy-body "{:user/val 37}") ;; => true

;; This makes various stringified maps, as expected
(take 3 (gen/sample (s/gen ::stringy-body)))
;; => ("#:user{:val -1}" "#:user{:val 0}" "#:user{:val -1}")

;; *** But the overrides don't get passed through ***
(take 3 (gen/sample (s/gen ::stringy-body {::val #(s/gen #{42})})))
;; ("#:user{:val -1}" "#:user{:val 0}" "#:user{:val 0}")

Should consider documenting this in s/gen, s/with-gen, etc.



 Comments   
Comment by Alex Miller [ 03/Jan/17 5:39 PM ]

When you use with-gen, you're basically overriding the built-in gen mechanism (which supports overrides) and providing your own (opaque to spec) generator. You should not expect overrides to take effect inside a custom generator.

Comment by Russell Mull [ 03/Jan/17 5:41 PM ]

That makes sense, but in lieu of that I expected (and went looking for) some way to get at the overrides map from the function passed to s/with-gen, and found none.

Comment by Russell Mull [ 03/Jan/17 5:48 PM ]

... I didn't fully parse your comment the first time around. I can see from the implementation that a custom generator (gfn internally) is never passed any of the contextual information that the builtin specs have at hand. As it sounds like this is intentional, it would be useful to note this limitation in the docstring for s/gen or perhaps s/with-gen.

Comment by Alex Miller [ 03/Jan/17 5:58 PM ]

It's not a crazy idea, but it doesn't seem like there's any way this could be done in the current impl without some pretty significant changes.





[CLJ-2094] clojure.spec: bug with protocols and extend-type Created: 01/Jan/17  Updated: 03/Jan/17  Resolved: 03/Jan/17

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

Type: Defect Priority: Major
Reporter: John Schmidt Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: spec


 Description   

I have the following two clj files (I've tried to come up with a minimal example):

core.clj
--------------
(ns spec-test.core
  (:require [clojure.spec :as s]))

(defprotocol Game
  (move [game]))

(s/def ::game1 #(satisfies? Game %))
(s/def ::game2 (partial satisfies? Game))

foo.clj
--------------
(ns spec-test.foo
  (:require [spec-test.core :refer [Game]]))

(defrecord Foo [])

(extend-type Foo
  Game
  (move [game]))

Here's a REPL session that explains my problem:

user=> (ns spec-test.core)
nil
spec-test.core=> (require 'spec-test.core :reload)
nil
spec-test.core=> (require 'spec-test.foo :reload)
nil
spec-test.core=> (satisfies? Game (spec-test.foo/->Foo))
true
spec-test.core=> ((partial satisfies? Game) (spec-test.foo/->Foo))
true
spec-test.core=> (s/explain ::game1 (spec-test.foo/->Foo))
Success!
nil
spec-test.core=> (s/explain ::game2 (spec-test.foo/->Foo))
val: #spec_test.foo.Foo{} fails spec: :spec-test.core/game2 predicate: (partial satisfies? Game) <---- WAAAAAT
nil

I would expect ::game1 and ::game2 to be equivalent, but somehow they're not.

Also see: https://groups.google.com/forum/#!topic/clojure/igBlMpqGU3A



 Comments   
Comment by Steve Miner [ 02/Jan/17 12:43 PM ]

I gave some incorrect advice on the mailing list so I'll try to correct myself here. Basically, the protocol is stored in a var, Game. If a predicate captures a particular state of the protocol, it won't respond correctly when additions are made to the protocol (with extend-type, etc.) So the problem with the usage of `partial` is that it evaluates the current value of the protocol at that point in time, before it has been extended to cover Foo.

The #(...) function definition would have used the var itself, not its current value. Naturally, the var allows the protocol to be updated such that the function sees the updated value. Basically, this is just normal Clojure evaluation. A `defn` style function would have worked fine as well. It's just the `partial` that evaluated its args that leads to the problem.

The same kind of issue could come up if you passed any var to partial and captured the current value of the var. Later changes to the var would not affect the result of partial.

I'll say the bug is the confusing error message. It seems that s/explain is implying it is using the var Game, but it really captured an "old" value of Game.

Comment by Alex Miller [ 03/Jan/17 2:10 PM ]

I think Steve's assessment is all correct.

I'm not sure that it's possible for spec to know what's happened here though wrt giving a better error message.

I don't think I really see anything that can be "fixed", so I'm going to mark this as declined.





[CLJ-2093] partial and fn behave differently with eval Created: 28/Dec/16  Updated: 31/Dec/16  Resolved: 31/Dec/16

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

Type: Defect Priority: Major
Reporter: N/A Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None


 Description   

(eval (list map (partial + 1) [0]))
CompilerException java.lang.ExceptionInInitializerError
(eval (list map (fn [x] (+ 1 x)) [0]))
=> (1)



 Comments   
Comment by Kevin Downey [ 29/Dec/16 9:22 PM ]

same issue as http://dev.clojure.org/jira/browse/CLJ-1206 and all the issues connected to that

Comment by Alex Miller [ 31/Dec/16 11:17 AM ]

You should not expect either of these to work as the expressions contain function objects (not function forms).

You should be doing something like this:

(eval (list 'map '(partial + 1) [0]))




[CLJ-2092] deftype instances with mutable fields cannot be compiled Created: 24/Dec/16  Updated: 31/Dec/16

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

Type: Defect Priority: Major
Reporter: Herwig Hochleitner Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: compiler, deftype

Approval: Triaged

 Description   

When evaluating or compiling an implementer of clojure.lang.IType, the compiler tries to reflectively access its fields. This fails, when a field is marked mutable (hence private):

Clojure 1.9.0-master-SNAPSHOT
user=> (deftype T [^:unsynchronized-mutable t])
user.T
user=> (T. :t)
#object[user.T 0x2654635 "user.T@2654635"]
user=> (eval (T. :t))
CompilerException java.lang.IllegalArgumentException: No matching field found: t for class user.T
            Reflector.java:  271  clojure.lang.Reflector/getInstanceField
             Compiler.java: 4724  clojure.lang.Compiler$ObjExpr/emitValue
             Compiler.java: 4851  clojure.lang.Compiler$ObjExpr/emitConstants
             Compiler.java: 4529  clojure.lang.Compiler$ObjExpr/compile
             Compiler.java: 4049  clojure.lang.Compiler$FnExpr/parse
             Compiler.java: 6866  clojure.lang.Compiler/analyzeSeq
             Compiler.java: 6669  clojure.lang.Compiler/analyze
             Compiler.java: 6924  clojure.lang.Compiler/eval
             Compiler.java: 6890  clojure.lang.Compiler/eval
                  core.clj: 3105  clojure.core/eval
...

For classes that don't implement IType, no such problem exists.

user> (deftype* user/U user.U
        [^:unsynchronized-mutable u]
        :implements [])
nil
user> (eval (user.U. :u))
#object[user.U 0x34699051 "user.U@34699051"]

This problem commonly occurs, when implementing a tagged literal for a deftype with cached hash.



 Comments   
Comment by Alex Miller [ 31/Dec/16 12:01 PM ]

Yeah, this is interesting. The compiler compiles a deftype into a call to the constructor with the current values of the fields, but mutable fields are not accessible. One alternative would be to provide some standard method to "read" the field set rather than relying on reflection. (Another would be changing the access modifiers for mutable fields but I think that's probably a non-starter.)





[CLJ-2091] clojure.lang.APersistentVector#hashCode is not thread-safe Created: 24/Dec/16  Updated: 10/Jan/17

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

Type: Defect Priority: Major
Reporter: thurston n Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: collections, concurrency

Attachments: File clj-2091-0.diff     File clj-2091-default-initialization.diff    
Patch: Code
Approval: Prescreened

 Description   

Problem: clojure.lang.APersistentVector#hashCode contain a deliberate data race on hash computation. However, the code as written does not follow safe practices for the intended data race. Specifically, the problem arises because the hashCode() (and hasheq()) method make multiple reads of the (unsynchronized) _hash field. The JMM permits these reads to return different values. Specifically, the last read in the return may return the pre-computed value -1, which is not the desired hash value. This problem also applies to APersistentMap, APersistentSet, and PersistentQueue.

See: http://jeremymanson.blogspot.com/2008/12/benign-data-races-in-java.html for a good description of the problem.

Fix: The main fix is to read the cached hash field only once and return the value of the local computation, not the value of the field.

A secondary change that is also beneficial is to use the default initializer value (which has special ordering in the JMM to the beginning of the thread) rather than setting and using -1 as the sentinel value.

In both cases these changes follow the canonical idioms used in java.lang.String for lazy hash computation. The patch covers both.

Patch: clj-2091-default-initialization.diff - note that this patch will indicate whitespace errors when applied due to the wacky line endings in PersistentQueue. The problem here is really the PQ formatting, not the patch.

Prescreened by: Alex Miller

There are some hash-related tests already but I also spot-checked that hash computations are returning the same value with and without the patch for the collections in question.



 Comments   
Comment by thurston n [ 24/Dec/16 4:38 PM ]

I can of course provide a patch, but as this is my first issue and am generally unfamiliar with clojure's practices and because this issue is not restricted to APersistentVector#hashCode, I thought it best to hold off and let the stewards decide how to proceed and how I could best help

Comment by Alex Miller [ 31/Dec/16 12:47 PM ]

Patch welcome (but please sign the contributor's agreement first - http://clojure.org/community/contributing). Also, see the processes for developing patches at http://dev.clojure.org/display/community/Developing+Patches.

AFAICT, the affected classes are APersistentVector, APersistentMap, APersistentSet, and PersistentQueue? Would be ok by me to cover all of them in a single patch.

Comment by thurston n [ 03/Jan/17 4:00 PM ]

AFAICT, the affected classes are APersistentVector, APersistentMap, APersistentSet, and PersistentQueue?

  • Dunno. However my experience tells me that the broken idiom (racy cache/memoization) is likely elsewhere; but I know of no systematic way of finding them. Regardless, I'll just focus on those 4 classes.
  • My plan is to also fix #hasheq(). It's the same problem; if you don't want that then just let me know and I'll refrain.
  • I'm not planning to deal with the initialization of #_hash and #_hasheq (currently inline-initialized to -1); that's a separate (although related) thread-safety problem. Might they be just what we refer to as a "legacy idiosyncrasy"? If so, then they really should be changed to just be default-initialized. I did, as an experiment, change one to default initialization, and the tests passed - that should be enough, but given that the persistent classes are serializable, code-coverage, et al., I can't say for sure. So I'll leave it to others more familiar with the codebase to make that determination. I note that if it is in future determined to change them, then #hashCode() and #hasheq() will need to be modified (trivially) accordingly.

That's the plan. Sound good?

Comment by Alex Miller [ 03/Jan/17 9:49 PM ]

I thought the non-default initialization was part of what you were describing, so now I'm not sure we're on the same page. Maybe you can just patch one so we have something concrete to talk about.

Comment by thurston n [ 03/Jan/17 9:56 PM ]

I'm not sure what you mean by "patch one" - I just submitted a patch, did you look at that?

Comment by Alex Miller [ 03/Jan/17 10:13 PM ]

I meant one class - sorry, I didn't see the patch. I will look at it tomorrow with fresh eyes.

Comment by thurston n [ 03/Jan/17 11:02 PM ]

Sure.

To be clear, as I mentioned in today's earlier comment, I would advise removing the inline-initialization, viz.

int _hash = -1;
int _hasheq = -1;

with

int _hash;
int _hasheq;

As I wrote, the extant tests would pass (of course, changing #hashCode() and #hasheq() appropriately)

But the initialization issue is a different, although certainly not orthogonal, issue than the one my patch addresses.

Currently, (i.e. pre-patch), #hashCode() can return a spurious -1 even if an APersistentVector instance is safely published - my patch fixes that.

However, because of the inline-initialization, an APersistentVector instance that is not safely published could return a spurious 0 from #hashCode(), even with my patch.

Now if the inline-initialization is just a "legacy idiosyncrasy" (and we all do that at one time or another), then it could be safely replaced (along with the appropriate modification to my patch) and all APersistentVector instances (safely published or not), would have #hashCode() implementations that are correct.

Comment by Alex Miller [ 06/Jan/17 3:19 PM ]

Ok, I went over all this again and it makes sense to me. I think you should proceed and also make the initializer change (remove the -1 as sentinel and replace with no initializer and 0 for the comparison checks in the methods).

Comment by thurston n [ 06/Jan/17 11:36 PM ]

Combines the 2 commits into a single commit patch
So incorporates the original patch changes (single read) with default initialization and checks for zero
Don't know what to do with PersistentQueue's mixed line-endings – that I'll leave to you to deal with

Comment by thurston n [ 09/Jan/17 8:16 PM ]

Problem also in core.lang.ASeq#hashCode() and core.lang.ASeq#hasheq() - although thankfully without inline initialization

Surely not the last place either

Comment by Alex Miller [ 10/Jan/17 8:33 AM ]

Feel free to update the patch if you like





[CLJ-2090] Improve clojure.core/distinct perf by using transient set Created: 23/Dec/16  Updated: 04/Jan/17

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

Type: Enhancement Priority: Major
Reporter: Nikita Prokopov Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: performance, transducers, transient

Attachments: Text File clj-2090-use-transient-set-in-distinct-2.patch    
Patch: Code
Approval: Triaged

 Description   

Current implementation of clojure.core/distinct uses persistent set. This patch improves performance of lazy arity by ~25%-30% and transducer by ~40%-50% by using transient set instead.

10 elements
(doall (distinct coll)) 	 5.773439 µs => 4.179092 µs (-27%)
(into [] (distinct) coll) 	 3.238236 µs => 1.943254 µs (-39%)

100 elements
(doall (distinct coll)) 	 67.725764 µs => 42.129993 µs (-37%)
(into [] (distinct) coll) 	 35.702741 µs => 16.495947 µs (-53%)

1000 elements
(doall (distinct coll)) 	 540.652739 µs => 399.053873 µs (-26%)
(into [] (distinct) coll) 	 301.423077 µs => 164.025500 µs (-45%)

10000 elements
(doall (distinct coll)) 	 3.439137 ms => 3.058872 ms (-11%)
(into [] (distinct) coll) 	 1.437390 ms => 848.277178 µs (-40%)

Benchmarking code: https://gist.github.com/tonsky/97dfe1f9c48eccafc983a49c7042fb21



 Comments   
Comment by Alex Miller [ 23/Dec/16 8:52 AM ]

You can't remove the volatile - you still need that for safe publication in multi threaded transducing contexts.

Comment by Nikita Prokopov [ 23/Dec/16 11:50 AM ]

Alex Miller How do you mean?

  • I don’t update seen link because transient set can be mutated in-place
  • Are transducers meant to be used from multiple threads? Because even existing implementation clearly has race condition. I imagine fixing that would be costly (we’ll need a synchronized section), so maybe it should be a specialized transducer that you use only when needed?
Comment by Alex Miller [ 23/Dec/16 12:26 PM ]

Transient sets can NOT be mutated in place - you must use the return value.

Yes, transducers are used from multiple threads in (for example) transducer chans in core.async go blocks.

Comment by Alex Miller [ 23/Dec/16 12:28 PM ]

I should also say transducers are not expected to be used from more than one thread at a time, so there are no race problems. But being used from multiple threads over time requires proper safe publication.

Comment by Nikita Prokopov [ 24/Dec/16 3:07 AM ]

But being used from multiple threads over time requires proper safe publication.

Does that imply that no transients could be used in transducers (because underlying arrays on which transient impl is based are mutated in place, so different threads could potentially see different states of transient object)?

Does that also mean that partition-by and partition-all should be fixed (they use java.util.ArrayList which, being array of references, has no safe publication semantics)?

Transient sets can NOT be mutated in place - you must use the return value.

I was thinking that clojure/core.clj and clojure.lang.ATransientSet.java are both part of Clojure internals, colocated, so can share a little bit of internal knowledge about each other. It seems safe to do that, because that knowledge does not leak outside, and, if at any point impl of ATransientSet would change, core.clj could be updated accordingly in the same release. I wouldn’t do that in any third-party library, of course.

Comment by Alex Miller [ 24/Dec/16 9:13 AM ]

Does that imply that no transients could be used in transducers (because underlying arrays on which transient impl is based are mutated in place, so different threads could potentially see different states of transient object)?

Transients require only that they are asked by no more than a single thread at a time and so are safe to use in a transducer. However, they should guarantee safe publication. core.async channels already do this as an artifact of their implementation, but other transducing contexts may not.

Transients should NEVER be used as "mutate in place", regardless of concurrency. While they will appear to "work" in some circumstances, this is never correct (eventually an update operation will return a new instance and if you are mutating in place, your data will then be missing). This is discussed and correct examples are shown at http://clojure.org/reference/transients.

Does that also mean that partition-by and partition-all should be fixed (they use java.util.ArrayList which, being array of references, has no safe publication semantics)?

That's something Rich and I are discussing but, probably.

Comment by Nikita Prokopov [ 24/Dec/16 12:56 PM ]

Alex Miller Here’s quick test that shows that changes to transient set (which is nothing more but a wrapper around transient map) made in one thread are not always visible from another thread.

https://gist.github.com/tonsky/62a7ec6d539fc013186bee2df0812cf6

That means that if we try to use transients for e.g. distinct it will miss duplicate items

Comment by Nikita Prokopov [ 24/Dec/16 1:02 PM ]

Removed transients from transducer arity of distincts because transducers might be accessed from multiple threads

Comment by Nikita Prokopov [ 24/Dec/16 1:12 PM ]

Maybe that doc http://clojure.org/reference/transients should be updated re: transients are not safe to use from multiple threads because changes made by one thread are not necessarily visible to another. Even if they don’t compete

Comment by Alex Miller [ 31/Dec/16 12:54 PM ]

I would say that test is demonstrating a bug in transient sets/maps and you should file a ticket for that as it's a lot more important than this enhancement.

distinct should be able to use transients in both the transducer and lazy seq impls. The issue with contains? not working on transients is actually a separate ticket - http://dev.clojure.org/jira/browse/CLJ-700 that will likely require some class hierarchy rearrangement. I don't think we would take this change until that is fixed (so that you can avoid relying on the class and Java method variants).

Comment by Nikita Prokopov [ 04/Jan/17 11:47 AM ]

I have to admit my test was demonstrating something else: there were no proper thread isolation. So it was a concurrency issue, not “safe publication” issue. My current understanding is this:

Transients require thread isolation. Use of a particular transient instance should be controlled either by using it in an single-threaded scope, or in a framework that enforces this.

That guarantee implicitly presumes that there’s happens-before relation between transient usage from multiple threads. There’s no other way to define “only one thread is in this section at a time”.

That, in turn, means that all writes that happened in thread 1 are visible in thread 2, regardless to volatility of the variables involved. In fact, we can remove all volatiles from transients implementation and probably make them faster, because, by asking “no more than one thread at a time” we enforce users to establish happens-before between sections, and that would give us all the safe publication guarantees we need.

Is my understanding correct? Am I missing something?

Comment by Nikita Prokopov [ 04/Jan/17 11:55 AM ]

Also, long-living transients (e.g. in a transducers associated with a queue, for example) will hold a reference to a thread that created them. Is that a bad thing? Should we switch to boolean flag instead?





[CLJ-2089] Sorted colls with default comparator don't check that first element is Comparable Created: 19/Dec/16  Updated: 13/Jan/17

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

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

Attachments: Text File clj-2089.patch    
Patch: Code and Test
Approval: Vetted

 Description   

Sorted maps and sets use the default comparator. The default comparator requires elements to be Comparable. PersistentTreeMap will not actually invoke the comparator until the second element is added to the map/set, so it is possible to create an invalid single element sorted map/set that will blow up only on later use.

The following examples create invalid "timebomb" collections that will throw ClassCastException if another element is added or if they're used in other ways (because sets are not comparable):

(sorted-set #{1})
(conj (sorted-set) #{1})
(sorted-map #{} 1)
(assoc (sorted-map) #{} 1)

Example:

(def s (sorted-set #{1}))  ;; this doesn't fail
(conj s #{2})              ;; first conj triggers error
ClassCastException clojure.lang.PersistentHashSet cannot be cast to java.lang.Comparable  clojure.lang.Util.compare (Util.java:153)

Cause: In PersistentTreeMap.add(), in the case where the existing tree is null, the comparator is never invoked and so the default comparator will never throw the ClassCastException seen on later compares. Note that none of this applies for a custom comparator (sorted-set-by and sorted-map-by) - those comparators can do whatever.

Proposed: In add(), if the map is empty AND the comparator is the default comparator, then check whether the added key is Comparable and throw CCE if not. Note that PersistentTreeMap is also the impl used by PersistentTreeSet so this covers both. The error message is customized to give you a better hint about the problem (and written to be applicable for both maps and sets). In the patch some existing (bad) tests had to be adjusted.

user=> (def s (sorted-set #{1}))
ClassCastException Default comparator requires nil, Number, or Comparable: #{1}  clojure.lang.PersistentTreeMap.add (PersistentTreeMap.java:335)

One downside of the current patch is that does not catch the equivalent problem if using a custom comparator and a bad first element. You could maybe do this by calling comp.compare(k,k) (although many comparators, like the default comparator, have an early identity check that would not fail on this). For now, decided that if you supply a custom comparator, it's up to you to not use it with elements that don't work with that comparator.

Patch: clj-2089.patch






[CLJ-2088] 'into' can make maps from collection of lists, but vectors are ok. Created: 19/Dec/16  Updated: 20/Dec/16  Resolved: 19/Dec/16

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

Type: Defect Priority: Major
Reporter: Eugene Aksenov Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: bug
Environment:

OS: Ubuntu 16.04, x86_64, Linux 4.2.0-42-generic



 Description   

'into' can make maps from collection of lists, but vectors are ok.

Good behavior:
(into {} [[:a "a"] [:b "b"]])
;;=> {:a "a", :b "b"}

(into {} '([1 "a"] [2 "b"]))
;;=> {1 "a", 2 "b"}

Bad examples:
(into {} ['(:a "a") '(:b "b")])
;;=> ClassCastException clojure.lang.Keyword cannot be cast to java.util.Map$Entry clojure.lang.ATransientMap.conj (ATransientMap.java:44)

(into {} [(list [:a "a"]) [:b "b"]])
;;=> ClassCastException clojure.lang.PersistentVector cannot be cast to java.util.Map$Entry clojure.lang.ATransientMap.conj (ATransientMap.java:44)

(into {} ['(1 "a") '(2 "b")])
;;=> ClassCastException java.lang.Long cannot be cast to java.util.Map$Entry clojure.lang.ATransientMap.conj (ATransientMap.java:44)

(into {} '('(1 "a") '(2 "b")))
;;=> ClassCastException clojure.lang.Symbol cannot be cast to java.util.Map$Entry clojure.lang.ATransientMap.conj (ATransientMap.java:44)



 Comments   
Comment by Alex Miller [ 19/Dec/16 11:53 AM ]

into is built on conj (takes a sequential collection of elements to invoke with conj). conj for maps takes a map entry - a subclass of java.util.Map$MapEntry or a 2-element vector. Both of these choices can access keys and vals directly without "traversing" the entry (going through the key to get to the value).

We don't want to support that for performance reasons, so lists or seqs or not valid as map entries (and conj on a map does not support them).

into takes a sequential collection of these entries though, so vector, or list, or seq are all fine as the src collection for into.

So, all of this is working as intended and I am declining the ticket.

Comment by Eugene Aksenov [ 20/Dec/16 2:09 AM ]

Ok, live and learn
I've just added this code case and brief explanation to clojuredocs.org. Hope no one will be confused anymore.





[CLJ-2087] map does not work with core.async/<! Created: 17/Dec/16  Updated: 17/Dec/16  Resolved: 17/Dec/16

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

Type: Defect Priority: Major
Reporter: Roy Varghese Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None


 Description   

The construct
(map <! [chan1 chan2 chan3])

does not work for reasons outlined here https://github.com/clojure/core.async/wiki/Go-Block-Best-Practices

This leads to very obscure bugs.

If this cannot be fixed in map, please consider throwing an exception when it cannot handle the passed function.



 Comments   
Comment by Alex Miller [ 17/Dec/16 5:08 PM ]

It's unlikely that we're going to "fix" this as it part of the design of Clojure.

When I try your example I see:

AssertionError Assert failed: <! used not in (go ...) block




[CLJ-2086] A macro in a nested syntax-quote causes an exception Created: 15/Dec/16  Updated: 03/Jan/17  Resolved: 15/Dec/16

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

Type: Defect Priority: Major
Reporter: N/A Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None


 Description   

(defmacro foo
[x y]
``(~~x ~~y))

(foo and true)

CompilerException java.lang.RuntimeException: Can't take value of a macro: #'clojure.core/and



 Comments   
Comment by Alex Miller [ 15/Dec/16 10:15 PM ]
user=> (macroexpand '(foo and true))
(clojure.core/seq (clojure.core/concat (clojure.core/list and) (clojure.core/list true)))

What do you expect this to do?

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

Declining till more info about a) what problem you are trying to solve and b) what the expected behavior is.

Comment by N/A [ 22/Dec/16 8:30 PM ]

a) I'm trying to define a macro that defines a macro.
(defmacro make-defmacro
[macro-name macro]
`(defmacro ~macro-name
x# & more#
(do (~macro x#)
`(~~macro-name ~@more#))))

(make-defmacro bar and)
CompilerException java.lang.RuntimeException: Can't take value of a macro

b) (defmacro foo
[x y]
``(~~x ~~y))

(macroexpand '(foo and true))
=> (and true)

Comment by Viktor Magyari [ 03/Jan/17 10:52 AM ]

Try

(defmacro foo [x y]
  ``(~'~x ~~y))




[CLJ-2085] Add additional info to explain-data to help explain printers Created: 15/Dec/16  Updated: 15/Dec/16

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

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

Attachments: Text File explain-data.patch    
Patch: Code
Approval: Vetted

 Description   

Right now, the explain-data provided to the explain printer function only has the list of problem data. There are many interesting things a printer could do with access to the root spec and value but those are not currently available.

Proposed: Provide these values in the explain-data map (as ::spec and ::value).

Patch: explain-data.patch






[CLJ-2084] Support MapEquivalence for defrecords Created: 13/Dec/16  Updated: 13/Dec/16  Resolved: 13/Dec/16

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

Type: Enhancement Priority: Major
Reporter: Herwig Hochleitner Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: defrecord


 Description   

defrecord equality implies matching types. This has been discussed and decided upon such that people should only bother with defrecords when they want an equality partition. There are, however, use cases for defrecords as small structmaps to save memory and those require map-based equivalence. Clojure has a marker interface for map equivalence: clojure.lang.MapEquivalence, which gets used in = to decide whether something qualifies as a persistent map. This can be reused to change the equiv implementation of a defrecord.

Current Behavior

When MapEquivalence is implemented in a defrecord, = becomes non-reflexive

(= {} (->Rec)), (not= (->Rec) {})

Proposed Behavior

Add clojure.lang.MapEquivalence to implemented interfaces in a defrecord, to treat it as a PersistenMap in equality, disregarding the type tag.
APersistentMap.equiv already does this, defrecord implementation can call APersistentMap/mapEquals



 Comments   
Comment by Alex Miller [ 13/Dec/16 10:27 AM ]

Wouldn't this be a breaking change for anyone relying on records to compare not equals based on type?

Comment by Herwig Hochleitner [ 13/Dec/16 4:35 PM ]

The key phrase is "Add clojure.lang.MapEquivalence to implemented interfaces". Any existing defrecords would not be affected. Au contraire, having a defrecord implement MapEquivalence right now results in a non-reflective-equals clojure-gigo scenario, so it's completely uncharted territory and ripe for definition. Sorry for not being clearer in the description, would you consider reopening?

Comment by Alex Miller [ 13/Dec/16 5:15 PM ]

It's not uncharted territory. Records intentionally compare different for equality based on type.

If you don't want that, don't use records. Or pour your record into a map before comparing via into or select-keys.

Comment by Herwig Hochleitner [ 13/Dec/16 5:51 PM ]

Yes, and I'm not proposing to change a single bit of that, agreed?
I am proposing the possibility to have specific defrecords implement map-like equality by using an option that a) naturally fits with what's already there b) compiles but yields GIGO right now:

(defrecord Rec []
  clojure.lang.MapEquivalence)

How is this already charted? If it is, then this ticket needs to be refiled as a Defect, because then the current behavior makes no sense.

So, if you're not declining this on technical grounds, what is it then? If it is not wanting to add use cases to defrecords for philosophical reasons, you're using the term "breaking change" very loosely here. There can't be any existing expectations on defrecords implementing MapEquivalence right now, because such defrecords are invalid programs right now.
Are you arguing some sort of "general expectation" on defrecords? One that would stand without looking at specific definitions? What use would that be to anybody. Who works with a defrecord without having a general idea of what it definition is?

Comment by Herwig Hochleitner [ 13/Dec/16 6:19 PM ]

Just to be clear, I did my due diligence on this and I'm aware that the current equality situation for records is completely intentional, hence I'm proposing growing it. The argument might still be that we don't want to grow it in that particular direction, but it's definitely not breakage.

Comment by Alex Miller [ 13/Dec/16 6:34 PM ]

I thought you were asking to add MapEquivalence to all records, which would be a breaking change.

The docstring for defrecord says:

{pre}
In addition, defrecord will define type-and-value-based =,
and will defined Java .hashCode and .equals consistent with the
contract for java.util.Map.{pre}

The important thing there is "type-and-value-based", and that's part of the design of records. Asking for MapEquivalence on a map means value-based only. These two things are in conflict and thus I would agree that asking for MapEquivalence on a record makes no sense.

deftype exists so that you can make your own types with whatever equality/hash/behavior you want - if you want something like this, then build it on top of deftype.

Comment by Herwig Hochleitner [ 13/Dec/16 6:41 PM ]

yeah, changing defrecord to implement MapEquivalence by default would make no sense.

However, declaring a (specific) defrecord to be in the MapEquivalence partition makes perfect sense, doesn't it?
That way, we can type our records an structmap them too.

Comment by Herwig Hochleitner [ 13/Dec/16 6:49 PM ]

Anyway, I'll build this on deftype for data.xml in any case, because we want this to work on current and older clojure versions. I just thought it would fit neatly with clojure itself (since there is already a marker interface for that purpose, only its use currently leads to breakage) and wanted to make you aware of the idea. I'll recycle it through the mailing list, as soon as the deftype based thing is in data.xml





[CLJ-2083] spec for printable/readable/edn data Created: 12/Dec/16  Updated: 12/Dec/16

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

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


 Description   

When spec'ing some things, I've used `any?` in a few cases where it is overly permissive. In particular, sometimes I need to specify a value must be printable/readable, such as when a value may wind up in an edn file. Similarly, I've needed to spec something must have non-generative value-identity, ie. ban closures, etc. Printable/readable or simply `edn?` would be a much better approximation than `any?`.



 Comments   
Comment by Brandon Bloom [ 12/Dec/16 1:08 PM ]

I realize that an edn? predicate would have O(N) runtime, vs an edn spec that could take advantage of every/every-kv etc for sampling conformance.

Comment by Herwig Hochleitner [ 12/Dec/16 11:12 PM ]

Related: CLJ-1527





[CLJ-2082] Improve documentation of clojure.walk/walk Created: 11/Dec/16  Updated: 11/Dec/16

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

Type: Enhancement Priority: Minor
Reporter: David Cook Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docs, docstring, documentation


 Description   

The documentation for the clojure.walk module isn't clear on which methods recurse through data structures, and which only operate on the outermost layer. The documentation for clojure.walk/walk and clojure.walk/postwalk both use forms of the word "traverse," and there's nothing calling out that clojure.walk/walk, unlike the rest of the functions in the namespace, doesn't recurse through the provided form.






[CLJ-2081] for macro spec should know :let can't go in the first position Created: 09/Dec/16  Updated: 09/Dec/16

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

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

Approval: Triaged

 Description   

The for macro does not support :let in the first position. This was reported in CLJ-207 and a patch was produced but rejected. Since we have spec for macros now, it might be an opportunity to provide a better error message. (I think first-place let should be fine, but that's neither here nor there.)






[CLJ-2080] clojure.spec/every-kv does not work correctly on vectors Created: 08/Dec/16  Updated: 13/Dec/16

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

Type: Defect Priority: Major
Reporter: Brandon Bloom Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: spec
Environment:

alpha 14


Attachments: Text File clj-2080.patch    
Patch: Code
Approval: Vetted

 Description   

The every-kv doc states "takes separate key and val preds and works on associative collections". Vectors return true for associative? but do not currently work:

user=> (s/conform (s/every-kv any? any?) [])
[]
user=> (s/conform (s/every-kv any? any?) [1 2 3])
:clojure.spec/invalid
user=> (s/conform (s/every-kv integer? string?) [])
[]
user=> (s/conform (s/every-kv integer? string?) ["x"])
:clojure.spec/invalid

Cause: The combination of every-kv and every-impl assume that the collection passed to every-kv can provide a seq of entries (and the checks in every-impl specifically segment to vector?, map?, etc). So the code seems to be pretty far from handling vectors as associative collections for this kind of use.

Proposed: Rather than change the implementation, narrow the documentation for every-kv to only state that it works for maps.

Patch: clj-2080.patch

Alternative: Make significant changes to every-kv and every-impl to vectors being treated as associative collections.



 Comments   
Comment by Alex Miller [ 09/Dec/16 8:35 AM ]

At the moment, I'm inclined to say the doc in every-kv should be tightened to say "map" instead of "associative collection" but will check with Rich.





[CLJ-2079] Generator overrides for spec aliases are not respected Created: 08/Dec/16  Updated: 08/Dec/16

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

Type: Defect Priority: Major
Reporter: Nate Smith Assignee: Unassigned
Resolution: Unresolved Votes: 4
Labels: generator, spec

Approval: Vetted

 Description   

Generator overrides for spec aliases are not respected.

Unable to find source-code formatter for language: clojure. Available languages are: javascript, sql, xhtml, actionscript, none, html, xml, java
(require '[clojure.spec :as s])
(require '[clojure.spec.gen :as gen])
(s/def ::original number?)
(s/def ::alias ::original)

(every? double? (gen/sample (s/gen ::alias {::alias gen/double})))
;; => false

Providing a generator override for the original spec works as expected:

Unable to find source-code formatter for language: clojure. Available languages are: javascript, sql, xhtml, actionscript, none, html, xml, java
(every? double? (gen/sample (s/gen ::alias {::original gen/double})))
;; => true


 Comments   
Comment by Alex Miller [ 08/Dec/16 5:02 PM ]

Probably a missing delay in the alias case - there's another ticket that has the same cause.

Comment by Nate Smith [ 08/Dec/16 6:43 PM ]

Looks like it might be because gensub looks for matching overrides by calling spec-name, which returns the wrong value for spec aliases.

(require '[clojure.spec :as s])
(s/def ::original number?)
(s/def ::alias ::original)
(@#'clojure.spec/spec-name (s/get-spec ::alias))
;; => :user/original




[CLJ-2078] Add a maven profile to connect with an nrepl client Created: 07/Dec/16  Updated: 07/Dec/16  Resolved: 07/Dec/16

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

Type: Enhancement Priority: Trivial
Reporter: Herwig Hochleitner Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: development

Attachments: Text File clj-2078-0.patch    
Patch: Code

 Description   

Opening an nrepl port into a project is a popular development strategy with clojure projects. It should also work with the clojure project.

Maven profiles make it possible to add development tools without affecting the rest of the build.

clojure-maven-plugin has this tool built right in as the clojure:nrepl goal and cider provides middlewares for many advanced ide functionalities.

Attached patch provides and documents a -P cider profile that lets you jack into a clojure source checkout.



 Comments   
Comment by Herwig Hochleitner [ 07/Dec/16 7:38 PM ]

Assuming that non-contrib covered code is OK for optional dev tools.

Comment by Alex Miller [ 07/Dec/16 8:44 PM ]

Thanks, I don't think we're going to do this.





[CLJ-2077] Clojure can't be loaded from the boot classpath under java 9 Created: 06/Dec/16  Updated: 15/Dec/16

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

Type: Defect Priority: Major
Reporter: Toby Crawley Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: java9

Approval: Triaged

 Description   

As part of the changes for the jigsaw module system in Java 9, the
java packages available to the boot classloader are now a subset of
the the full java distribution. This means that classes loaded via the
boot classloader cannot access any classes outside of that subset.

The list of packages not visible to the boot classloader are:

java.activation
java.annotations.common
java.compact1
java.compact2
java.compact3
java.compiler
java.corba
java.scripting
java.se
java.se.ee
java.sql
java.sql.rowset
java.transaction
java.xml.bind
java.xml.ws
jdk.accessibility
jdk.charsets
jdk.crypto.ec
jdk.crypto.pkcs11
jdk.dynalink
jdk.jsobject
jdk.localedata
jdk.naming.dns
jdk.scripting.nashorn
jdk.xml.dom
jdk.zipfs
jdk.attach
jdk.compiler
jdk.hotspot.agent
jdk.internal.le
jdk.internal.opt
jdk.jartool
jdk.javadoc
jdk.jconsole
jdk.jdeps
jdk.jdi
jdk.jlink
jdk.jshell
jdk.jstatd
jdk.jvmstat

Clojure itself only uses one package on that list: java.sql. It is
used in clojure.instant to provide print-method and
print-dup implementations for java.sql.Timestamp, and in
clojure.core/resultset-seq.

This can be seen with (using Clojure 1.4.0 or higher, and a early-access build
of Java 9, most recently tested with 9-ea+147):

java -Xbootclasspath/a:clojure.jar clojure.main -e "(require 'clojure.instant)"

This affects any clojure-based tool that puts itself on the boot
classpath in order to gain a startup time boost (both lein
and boot are affected currently).



 Comments   
Comment by Toby Crawley [ 06/Dec/16 12:34 PM ]

More details on the underlying change that is triggering this are available at http://openjdk.java.net/jeps/261 (search for java.sql to find the relevant section).

Comment by Alex Miller [ 06/Dec/16 8:41 PM ]

Does this need to be a ticket here? Or is this really an issue for build tools?

Comment by Toby Crawley [ 08/Dec/16 4:30 PM ]

That depends on if we want using Clojure from the boot classpath to be an acceptable use case. If not, then I agree, it is just an issue for tooling.

Comment by Toby Crawley [ 09/Dec/16 2:21 PM ]

I realized today that this issue doesn't actually affect boot, since it doesn't use the bootclasspath. So lein is the only tooling I know of that is affected by this.

Comment by Ivan Pierre [ 12/Dec/16 4:59 AM ]

https://docs.oracle.com/javase/8/docs/api/java/sql/Timestamp.html
Would it be possible to use java.util.Date instead. Alas it's not possible to downcast :
Due to the differences between the Timestamp class and the java.util.Date class mentioned above, it is recommended that code not views Timestamp values generically as an instance of java.util.Date. The inheritance relationship between Timestamp and java.util.Date really denotes implementation inheritance, and not type inheritance.

Comment by Ivan Pierre [ 12/Dec/16 8:22 AM ]

The problem with Date is that it doesn't deal with nanoseconds. But Timestamp is created by a long giving the TimeDate value in milliseconds.
The use of setNano and getNano are only to store the SQL TIMESTAMP. Wouldn't it be better to deal with this value in another way?

The other way is to take just what we need from TimeStamp, and it's just a little thing, I'll try to compile with that to see if some other thing comes after...

Test code : https://gist.github.com/ivanpierre/b0ea937dac97d910a7c3c1e5774028e0

Comment by Ivan Pierre [ 12/Dec/16 1:13 PM ]

Ok, I pass to the GNU version of Timestamp. The code is neater. I mixed some of Sun's for more consistency. I dropped the string management of dates as Clojure will do it in clojure.instant.

It still works. I had a doubt...

If I type (clojure.lang.TimeStamp. 3678141) the response will be :
==> #inst "1970-01-01T01:01:18.141000000-00:00" with a nano of
141000000

But is if set nano to 1 : (.setNanos (clojure.lang.TimeStamp. 3678141) 1) the response is : #inst "1970-01-01T01:01:18.000000001-00:00"

This is correct, but it's a little disturbing to see my nice .141 disappear...

I put a fork on my GitHub. Last commit : https://github.com/ivanpierre/clojure/commit/749a0184ee7409290dad9ff353605fcaabd64f69

So, good, now pass to Leinigen...

Comment by Alex Miller [ 13/Dec/16 10:32 AM ]

I think the first question here is: do we expect that Clojure should be loadable from the bootclasspath?

While I realize this is a hack people use, my initial answer would be no (that was never a design constraint afaik). But will need to defer to Rich on that.

Comment by Ivan Pierre [ 13/Dec/16 10:41 AM ]

Well Leiningen do it, even with the lein repl. I test to see if TimeStamp is the only thing.
Alas Leinigen is not 1.9 compatible, so I have to go down to version 1.8 to make the tests. (problem of conflict between clojure.spec and hara library)
A funny thing would be to pass the whole Clojure test in bootstrap so we would know...

Comment by Ivan Pierre [ 13/Dec/16 1:06 PM ]

The java Timestamp could be directly integrated into clojure.instant as it's a new datatype. So no need to worry about copyright stuff, and integer it in a complete manner to accept SQL TIMESTAMP, and some correct protocols.
The worst is that looking across the DBs documentation doesn't help a lot and some are quite contradictory.





[CLJ-2076] s/coll-of and s/map-of do not unform their elements Created: 05/Dec/16  Updated: 09/Dec/16

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

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

Attachments: Text File clj-2076-2.patch     Text File clj-2076.patch    
Patch: Code and Test
Approval: Vetted

 Description   

s/coll-of and s/map-of unform with identity but should unform their elements:

(s/def ::o (s/coll-of (s/or :i integer? :s string?)))
(->> [1 2 "blah"] (s/conform ::o) (s/unform ::o))
=> [[:i 1] [:i 2] [:s "blah"]]

Expected: [1 2 "blah"]

Cause: every-impl unform* just returns x

Approach: Use the init/add/complete fns to generate an unformed value (when needed)

Patch: clj-2076-2.patch



 Comments   
Comment by Alex Miller [ 07/Dec/16 5:50 PM ]

This needs tests and a bunch of verification, but first pass at fixing.

Comment by Alex Miller [ 09/Dec/16 8:03 AM ]

Added tests, ready to screen





[CLJ-2075] Add three-arities to < <= > >= = == not= Created: 03/Dec/16  Updated: 23/Dec/16

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

Type: Enhancement Priority: Major
Reporter: Nikita Prokopov Assignee: Unassigned
Resolution: Unresolved Votes: 5
Labels: None

Attachments: Text File clj-2075-add-three-arities-to-comparisons.patch     Text File clj-2075-over-clj-1912.patch    

 Description   

In my practice, using three-arities of less/greater operations is pretty common for e.g. checking a number is in range:

(< 0 temp 100)

The problem is, it is almost three times as slow compared to (and (< 0 temp) (< temp 100)).

This happens because three-arities are handled by the generic vararg arity branch:

(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)

This patch adds special handling for three-arities to these fns: < <= > >= = == not=

(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 z] (and (. clojure.lang.Numbers (lt x y))
                (. clojure.lang.Numbers (lt y z))))
  ([x y z & more]
   (if (< x y)
     (let [nmore (next more)]
       (if nmore
         (recur y z (first more) nmore)
         (< y z (first more))))
     false)))

The performance gains are quite significant:

(= 5 5 5) 	 24.508635 ns => 4.802783 ns (-80%)
(not= 1 2 3) 	 122.085793 ns => 21.828776 ns (-82%)
(< 1 2 3) 	 30.842993 ns => 6.714757 ns (-78%)
(<= 1 2 2) 	 30.712399 ns => 6.011326 ns (-80%)
(> 3 2 1) 	 22.577751 ns => 6.893885 ns (-69%)
(>= 3 2 2) 	 21.593219 ns => 6.233540 ns (-71%)
(== 5 5 5) 	 19.700540 ns => 6.066265 ns (-69%)

Higher arities also become faster, mainly because there's one less iteration now:

(= 5 5 5 5) 	 50.264580 ns => 31.361655 ns (-37%)
(< 1 2 3 4) 	 68.059758 ns => 43.684409 ns (-35%)
(<= 1 2 2 4) 	 65.653826 ns => 45.194730 ns (-31%)
(> 3 2 1 0) 	 119.239733 ns => 44.305519 ns (-62%)
(>= 3 2 2 0) 	 65.738453 ns => 44.037442 ns (-33%)
(== 5 5 5 5) 	 50.773521 ns => 33.725097 ns (-33%)

This patch also changes vararg artity of not= to use next/recur instead of apply:

(defn not=
  "Same as (not (= obj1 obj2))"
  {:tag Boolean
   :added "1.0"
   :static true}
  ([x] false)
  ([x y] (not (= x y)))
  ([x y z] (not (= x y z)))
  ([x y z & more]
   (if (= x y)
     (let [nmore (next more)]
       (if nmore
         (recur y z (first more) nmore)
         (not= y z (first more))))
     true)))

Results are good:

(not= 1 2 3 4) 	 130.517439 ns => 29.675640 ns (-77%)

I'm also doing what Jozef Wagner did in CLJ-1912 (calculating (next more) just once), although perf gains from that alone are not that big.

My point here is that optimizing three-arities makes sence because they appear in the real code quite often. Higher arities (4 and more) are much less widespread.



 Comments   
Comment by Nikita Prokopov [ 03/Dec/16 2:32 AM ]

Benchmark code here https://gist.github.com/tonsky/442eda3ba6aa4a71fd67883bb3f61d99

Comment by Alex Miller [ 03/Dec/16 8:24 AM ]

It might make more sense to combine this with CLJ-1912, otherwise these patches will fight.

Comment by Nikita Prokopov [ 03/Dec/16 1:02 PM ]

Use this patch if CLJ-1912 would be applied first

Comment by Nikita Prokopov [ 23/Dec/16 7:50 AM ]

I found a problem with previous patches that during defining = (equality), and is not yet defined. Replaced with if





[CLJ-2074] ::keys spec conflicts with destructuring spec Created: 02/Dec/16  Updated: 11/Jan/17

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

Type: Defect Priority: Major
Reporter: Nicola Mometto Assignee: Alex Miller
Resolution: Unresolved Votes: 1
Labels: destructuring, spec

Attachments: File close-destructuring-keys-specs.diff    
Patch: Code

 Description   

As a consequence of the destructuring specs being implemented in terms of `s/keys`, defining a spec for `::keys` or `::strs` is problematic at the moment, because it will conflict with trying to use `::keys` for destructuring:

user=> (require '[clojure.spec :as s])
nil
user=> (s/def ::keys nil?)
:user/keys
user=> (let [{::keys [a]} {::a 1}] a)
ExceptionInfo Call to clojure.core/let did not conform to spec:
In: [0 0] val: #:user{:keys [a]} fails spec: :clojure.core.specs/local-name at: [:args :bindings :binding :sym] predicate: simple-symbol?
In: [0 0 0] val: ([:user/keys [a]]) fails spec: :clojure.core.specs/seq-binding-form at: [:args :bindings :binding :seq] predicate: (cat :elems (* :clojure.core.specs/binding-form) :rest (? (cat :amp #{(quote &)} :form :clojure.core.specs/binding-form)) :as (? (cat :as #{:as} :sym :clojure.core.specs/local-name))),  Extra input
In: [0 0 :user/keys] val: [a] fails spec: :user/keys at: [:args :bindings :binding :map :user/keys] predicate: nil?
:clojure.spec/args  ([#:user{:keys [a]} #:user{:a 1}] a)
  clojure.core/ex-info (core.clj:4725)

This feels like an implementation detail leak.



 Comments   
Comment by Brandon Bloom [ 10/Jan/17 5:36 PM ]

I also just ran in to this problem. Just wanted to say that I'd like to see a fix, but I'm not quite sure about the proposed solution. Or, at least, the name "closed?" seems to imply a non-extensible map, when in reality the flag more or less means "not a map that participates in the global keys system", for which I do not have a better name suggestion.

Comment by Alex Miller [ 11/Jan/17 8:35 AM ]

The proposed patch is a non-starter. I have some ideas on how to address this, but just haven't gotten around to working on it yet.

Comment by Alex Miller [ 11/Jan/17 8:37 AM ]

Removed proposal and patch from the ticket as we will not be going this direction. Captured here for reference:

"The attached patch implements a proposed solution to this issue, by adding a `:closed?` option to `s/keys` and using it for the destructuring spec. If `s/keys` is used with `:closed?` set to true, `conform` will only validate declared specs as opposed to the default behaviour of `s/keys` of validating all namespaced keywords with existing specs.

After this patch, the above example runs fine and usages of `s/keys` without `:closed?` set to true will validate against `::keys` as per current behaviour.

Patch: close-destructuring-keys-specs.diff"





[CLJ-2073] AOT compilation can result in spurious ClassCastException during compile Created: 02/Dec/16  Updated: 02/Dec/16

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

Type: Defect Priority: Major
Reporter: Paul Mooser Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: aot, compiler
Environment:

java version "1.8.0_112"
Java(TM) SE Runtime Environment (build 1.8.0_112-b16)
Java HotSpot(TM) 64-Bit Server VM (build 25.112-b16, mixed mode)


Attachments: File consumer.clj     File implementer.clj     File protocol.clj    

 Description   

If you try to compile the attached files as follows (assuming they are in "src"):

java -Dclojure.compile.path=out -cp "./clojure-1.8.0.jar:out:src" clojure.lang.Compile implementer protocol consumer

an exception will be thrown:

Exception in thread "main" java.lang.ClassCastException: implementer.Obj cannot be cast to protocol.Dependent, compiling:(consumer.clj:5:1)
	at clojure.lang.Compiler$InvokeExpr.eval(Compiler.java:3657)
	at clojure.lang.Compiler.compile1(Compiler.java:7474)
	at clojure.lang.Compiler.compile(Compiler.java:7541)
	at clojure.lang.RT.compile(RT.java:406)
	at clojure.lang.RT.load(RT.java:451)
	at clojure.lang.RT.load(RT.java:419)
	at clojure.core$load$fn__5677.invoke(core.clj:5893)
	at clojure.core$load.invokeStatic(core.clj:5892)
	at clojure.core$load.doInvoke(core.clj:5876)
	at clojure.lang.RestFn.invoke(RestFn.java:408)
	at clojure.core$load_one.invokeStatic(core.clj:5697)
	at clojure.core$compile$fn__5682.invoke(core.clj:5903)
	at clojure.core$compile.invokeStatic(core.clj:5903)
	at clojure.core$compile.invoke(core.clj:5895)
	at clojure.lang.Var.invoke(Var.java:379)
	at clojure.lang.Compile.main(Compile.java:67)
Caused by: java.lang.ClassCastException: implementer.Obj cannot be cast to protocol.Dependent
	at protocol$fn__12$G__8__14.invoke(protocol.clj:3)
	at protocol$fn__12$G__7__17.invoke(protocol.clj:3)
	at protocol$expand_deps.invokeStatic(protocol.clj:8)
	at protocol$expand_deps.invoke(protocol.clj:6)
	at clojure.lang.AFn.applyToHelper(AFn.java:154)
	at clojure.lang.AFn.applyTo(AFn.java:144)
	at clojure.lang.Compiler$InvokeExpr.eval(Compiler.java:3652)
	... 15 more
  • This does not occur with 1.6 or earlier versions
  • This does not occur if you do not try to invoke AOT
  • This may not occur for some orderings of the arguments

This appears to be related to the class being loaded by two different class loaders, and also may result in the namespace being compiled more than once. This issue has popped up for us multiple times in our production build, but it took a while to realize it was a compiler issue and to find a minimal example.






[CLJ-2072] Primitive type aliases do not always work due to meta data evaluation Created: 01/Dec/16  Updated: 01/Dec/16

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

Type: Enhancement Priority: Minor
Reporter: Volkert Oakley Jurgens Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: typehints


 Description   

Primitive alias do not work when the meta data is evaluated, for example in the case of def.

In this example, char is interpreted to be the function char rather than the type alias. This is because clojure.lang.Compiler$DefExpr$Parser.parse evaluates the meta data on the symbol.

(def ^char x \space)
(String/valueOf x)
=> CompilerException java.lang.IllegalArgumentException: Unable to resolve classname: clojure.core$char@7b82f7d1

Instead, this has to be written as

(def ^Character/TYPE x \space)

However, when using primitive type hints in-line, they work fine:

(def x \space)
(String/valueOf ^char x)

Primitive type aliases should be handled consistently.

Googling shows that this seems to be a well-known problem, but I have not found a Jira issue for it.



 Comments   
Comment by Alex Miller [ 01/Dec/16 7:48 PM ]

Var meta is evaluated. Meta on function signatures and other locations is not. Those two things are that way for historical reasons but would at this point be breaking changes if we changed either of them, so they are not going to change.

One thing that could potentially be done is to detect this particular problem when it happens and create a warning or error. In particular, this would present as a var whose meta :tag is a function.

So, if you want to re-write this ticket as a request for error message, that is something worth considering.





[CLJ-2071] Unexpected behavior with clojure.spec/tuple and clojure.spec.test/instrument Created: 01/Dec/16  Updated: 01/Dec/16  Resolved: 01/Dec/16

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

Type: Defect Priority: Major
Reporter: Ben Rady Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None
Environment:

Clojure 1.9, JDK 8



 Description   

It looks like stest/instrument is comparing a sequence of actual args to a vector created by spec/tuple and it doesn't match, however clojure.spec.test/instrument appears to work fine. Reading the clojure.spec guide I would think the two approaches would be equivalent.

(ns sample
  (:require [clojure.spec.test :as stest]
            [clojure.spec :as spec]))

    (defn myinc [i]
      (inc i))

    (spec/fdef myinc
               :args (spec/tuple integer?) ; Fails with the error below
               ; :args (spec/cat :i integer?) ; This works
               :ret integer?)

    (stest/instrument `myinc)
    (myinc 1)
    ; clojure.lang.ExceptionInfo: Call to #'specific.core-spec/myinc did not conform to spec:
    ; val: (1) fails at: [:args] predicate: vector?
    ; :clojure.spec/args  (1)
    ; :clojure.spec/failure  :instrument
    ; :clojure.spec.test/caller  {:file "core_spec.clj", :line 28, :var-scope specific.core-spec/fn--6046}


 Comments   
Comment by Ben Rady [ 01/Dec/16 8:11 AM ]

Note that the namespace in this example was changed. It used to be specific.core-spec, which is shown in the error.

Comment by Alex Miller [ 01/Dec/16 10:22 AM ]

Spec will create a list or a seq of the args for checking the :args spec. Tuples can only be used on vectors (because they match by index). So, this is not currently expected to work. It is recommended that you use a regex spec (s/cat) instead.





[CLJ-2070] Faster clojure.core/delay implementation Created: 29/Nov/16  Updated: 30/Nov/16

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

Type: Enhancement Priority: Major
Reporter: dennis zhuang Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: performance
Environment:

macOS Sierra
intel Core i7 2.5G Hz
Memory 16GB

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


Attachments: Text File fast_delay_with_volatile_fn2.patch    
Patch: Code and Test
Approval: Prescreened

 Description   

clojure.lang.Delay uses a synchronized lock to protect the deref method, because it must make sure the Delay object is realized exactly once.

As we known synchronized lock plays worse performance under heavy concurrency. Instead, using volatile and double-checking lock in this situation improves the performance. The benchmark code is at test-delay.clj. The benchmark-delay function accepts thread count number as an argument, and it will start as many threads as the argument to access delay object concurrently as in (time (benchmark-delay 1)).

threads 1.9.0-alpha14 + patch % better
1 0.570196 ms 0.499905 ms 12
10 19.66194 ms 1.313828 ms 93
20 40.740032 ms 2.149794 ms 95
100 184.041421 ms 8.317295 ms 95

Patch: fast_delay_with_volatile_fn2.patch
Prescreened by: Alex Miller



 Comments   
Comment by Alex Miller [ 29/Nov/16 8:52 AM ]

A faster version of delay would be helpful - these are used extensively inside spec so would actually help the average case spec performance.

These whitespace errors need to be cleaned up...

$ git apply ~/Downloads/fast_delay.patch
/Users/alex/Downloads/fast_delay.patch:67: trailing whitespace.
	                try
/Users/alex/Downloads/fast_delay.patch:105: trailing whitespace.

/Users/alex/Downloads/fast_delay.patch:115: space before tab in indent.
        	    (fn []
/Users/alex/Downloads/fast_delay.patch:116: space before tab in indent.
          		    (.await barrier)
/Users/alex/Downloads/fast_delay.patch:117: space before tab in indent.
          		    (dotimes [_ 10000]
warning: squelched 8 whitespace errors
warning: 13 lines add whitespace errors.

More importantly, the double-check is on fn, so it's critical that fn is marked as volatile. You should re-run the perf test afterwards.

Comment by dennis zhuang [ 29/Nov/16 9:13 AM ]

Sorry, white spaces errors should be fixed before my attached.

But the fn doesn't need to be marked as volatile, because it's protected by synchronized in all blocks. And writing it to be null is fine here.

fn=null;

It's not like double-checking in singleton example, there is no reordering here.

Comment by Alex Miller [ 29/Nov/16 9:25 AM ]

fn is read at the top before the synchronized block - it needs to be volatile or one thread may not see the write inside the synchronized block from another thread.

Comment by dennis zhuang [ 29/Nov/16 9:41 AM ]

Yep ,but it's fine here.
If one thread can't see the writing null for fn at the top, it will enter the locking block.
The double-checking on fn!=null will make sure the fn is called at most once, and if the fn was called by another thread and was set to be null ,then current thread will fail on second checking on fn!=null and exit the locking to go on reading value or exception.

So, in the worst situation, maybe two or more threads would enter the locking block,but they all will fail on second checking on fn!=null except one thread of them success.

I don't want to declare fn to be volatile, because volatile also has a cost on reading. The fn variable may be flushed into main memory too late, but it's acceptable and safe here, and we avoid the cost of volatile reading.

Comment by Alex Miller [ 29/Nov/16 9:45 AM ]

I think you're wrong, and I'm not screening it without it being marked as volatile.

Comment by dennis zhuang [ 29/Nov/16 9:54 AM ]

The patch which mark fn volatile.

Comment by dennis zhuang [ 29/Nov/16 9:54 AM ]

The patch which does't mark fn volatile.

Comment by dennis zhuang [ 29/Nov/16 9:59 AM ]

Hi, alex.

I understand your opinion here. Though i still insist that fn doesn't need to be marked as volatile, but it's not a critical concern here.

I uploaded two patches, one is marked fn volatile, the other is not. All of them fixed the whitespace errors and update the benchmark result in ticket description.

Thanks.

Comment by dennis zhuang [ 29/Nov/16 10:15 AM ]

Rebase master.

Comment by Nicola Mometto [ 30/Nov/16 11:53 AM ]

dennis, here's an article describing why fn needs to be volatile: https://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html

Comment by dennis zhuang [ 30/Nov/16 6:01 PM ]

@Nicola

I knew the double-checking issue in old JVM memory model, but it is different here.
There is no instance constructed, it's only assigning fn to be null, so it doesn't have instruments reordering. And we don't have a partial constructed instance that escapes.

But it's not critical concern here, it seems that volatile doesn't compact performance of this patch.

Thanks.





[CLJ-2069] lazy seq that encounters an exception has differing behavior on repeated use Created: 27/Nov/16  Updated: 29/Nov/16

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

Type: Defect Priority: Major
Reporter: TianJun Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: lazy
Environment:

OS X EI Capitan, Java HotSpot(TM) 64-Bit Server VM 1.8.0_101-b13


Attachments: Text File 0001-CLJ-2069-cache-exceptions-thrown-during-lazy-seq-rea.patch    
Patch: Code
Approval: Prescreened

 Description   

It seems the below does not compile with 1.8.0 and 1.9.0-alpha14, the same errors appear in both versions.

user=> (def fibonacci-1
  ((fn fib  [a b]
    (lazy-seq  (cons a  (fib b  (+ a b)))))
    0 1))

user=> (filter #(< % 100) fibonacci-1)

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

user=> (filter #(< % 100) fibonacci-1)

NullPointerException   clojure.lang.Numbers.ops (Numbers.java:1013)

user=> (def fibonacci-2
         (lazy-cat [0 1] (map + (rest fibonacci-2) fibonacci-2)))

user=> (filter #(< % 100) fibonacci-2)

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

user=> (filter #(< % 100) fibonacci-2)
(0 1 1 2 3 5 8 13 21 34 55 89)

Patch: 0001-CLJ-2069-cache-exceptions-thrown-during-lazy-seq-rea.patch

Proposal: Cache exceptions thrown during lazy-seq realization, to avoid re-running bodyfn which is declared as `^:once`

Prescreened by: Alex Miller



 Comments   
Comment by TianJun [ 27/Nov/16 10:42 AM ]

Maybe I should use take-while instead of filter.

However, can anyone explain why I get ArithmeticException while running

(filter #(< % 100) fibonacci-2)

for the first time and get the right result at the second time?

Comment by Nicola Mometto [ 28/Nov/16 3:27 AM ]

The NPE is caused by the interaction between:

  • lazy-seq throwing an exception while realizing part of the sequence
  • lazy-seq internally using ^:once for locals clearing

lazy-seq expects the bodyfn to be run exactly once and then the result to be cached, but if an exception gets thrown during the execution of bodyfn, the function will get run again when the sequence tries to be realized a second time. However if locals clearing has already happened (even partially) this means some locals in bodyfn will now be nil rather than holding their actual value.

Comment by Nicola Mometto [ 28/Nov/16 3:36 AM ]

Attached patch that fixes this issue





[CLJ-2068] s/explain of evaluated predicate yields :s/unknown Created: 23/Nov/16  Updated: 13/Dec/16

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

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

Attachments: Text File clj-2068-2.patch     Text File clj-2068.patch    
Patch: Code and Test
Approval: Vetted

 Description   

Got:

(s/explain #{1 2 3} 4)
val: 4 fails predicate: :clojure.spec/unknown

(s/explain odd? 10)
val: 10 fails predicate: :clojure.spec/unknown

Expected to receive a description of the failing predicate as in:

(s/def ::s #{1 2 3})
(s/explain ::s 4)
;; val: 4 fails spec: :user/s predicate: #{1 3 2}

(s/def ::o odd?)
(s/explain ::o 10)
val: 10 fails spec: :user/o predicate: odd?

Cause: specize was falling through on these cases to Object and just returning unknown.

Proposed:
Special handling for 2 cases:
1. Sets - explictly catch IPersistentSet and use the set as the form.
2. Functions - demunge the function name and use the qualified function name symbol as the form. Add a special check for anonymous functions and revert to ::unknown for those (not much we can do with an eval'ed anonymous function).

Patch: clj-2068-2.patch



 Comments   
Comment by Alex Miller [ 13/Dec/16 6:52 PM ]

Simplified anon fn check and added a few basic tests.





[CLJ-2067] (s/def ::a ::b) throws unable to resolve error if ::b is not defined Created: 22/Nov/16  Updated: 22/Nov/16

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

Type: Defect Priority: Minor
Reporter: Oliver George Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: spec

Approval: Vetted

 Description   

It should be possible to do `(s/def ::a ::b)` before declaring ::b.

Currently this throws an "Unable to resolve" error.

Alex indicated that everything should have delays but that they are missing in some cases. This seems like one of those cases.

Examples where things work fine are `(s/def ::a (s/and ::b))` and `(s/def ::a (s/keys :req [::b]))`.






[CLJ-2066] Reflection on internal classes fails under Java 9 Created: 22/Nov/16  Updated: 22/Nov/16

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

Type: Defect Priority: Major
Reporter: Toby Crawley Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: interop, reflection

Approval: Triaged

 Description   

With the module system (jigsaw) as it is currently implemented in Java 9 early access builds (9-ea+144), calling a method via reflection is only allowed if the Method was retrieved for a class/interface in a package that is exported by its containing module. Reflector.java currently uses only target.getClass() to locate the Method, so reflective method invocation on a non-exported class will fail even if the method is provided by an exported parent interface or superclass.

The current workaround is to export the package to the unnamed module (where an application that doesn't explicitly use the module system runs) when invoking java/javac:

java --add-exports=java.xml/com.sun.xml.internal.stream=ALL-UNNAMED --add-exports=java.xml/com.sun.xml.internal.stream.writers=ALL-UNNAMED --add-exports=java.xml/com.sun.org.apache.xerces.internal.impl=ALL-UNNAMED ...

It's possible that this will be addressed in jigsaw before the release of Java 9. If not, Reflector.java could be modified to walk the ancestor chain if the initial invocation fails. Note that even with that change, accessing methods that are only defined on the non-exported class (i.e. methods that don't override a method from an exported superclass/interface) will require an --add-exports option.

For more details, see https://groups.google.com/d/msg/clojure-dev/Tp_WuEEcdWI/LMMQVAUYBwAJ



 Comments   
Comment by Toby Crawley [ 22/Nov/16 10:02 AM ]

This is the root cause of http://dev.clojure.org/jira/browse/DXML-32





[CLJ-2065] reduce-kv fails on subvec Created: 20/Nov/16  Updated: 21/Nov/16

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

Type: Defect Priority: Minor
Reporter: Steve Miner Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: collections

Approval: Triaged

 Description   

reduce-kv works as expected on vectors with the element index passed as the "key" argument. However, it fails with a subvec because clojure.lang.APersistentVector$SubVector does not implement IKVReduce.

(reduce-kv + 0 [1 2 3])
9

(reduce-kv + 0 (subvec [1 2 3] 1))
IllegalArgumentException No implementation of method: :kv-reduce of protocol: #'clojure.core.protocols/IKVReduce found for class: clojure.lang.APersistentVector$SubVector clojure.core/-cache-protocol-fn (core_deftype.clj:583)

One work around is to copy the subvec into a vector:

(reduce-kv + 0 (into [] (subvec [1 2 3] 1)))
6

Note however, the `vec` would not work here. Since Clojure 1.7, vec will return a subvec rather than copying.

(reduce-kv + 0 (vec (subvec [1 2 3] 1)))
IllegalArgumentException No implementation of method: :kv-reduce of protocol: #'clojure.core.protocols/IKVReduce found for class: clojure.lang.APersistentVector$SubVector clojure.core/-cache-protocol-fn (core_deftype.clj:583)



 Comments   
Comment by Steve Miner [ 20/Nov/16 12:53 PM ]

Here is my current work-around:

(extend-type clojure.lang.APersistentVector$SubVector
  clojure.core.protocols/IKVReduce
  (kv-reduce [subv f init]
    (transduce (map-indexed vector)
               (fn ([ret] ret) ([ret [k v]] (f ret k v)))
               init
               subv)))

In my tests it was usually faster to copy the subvec into a regular vector but I like the look of the transduce fix. It would probably be faster to add a native Java implementation in APersistentVector.java. I'm willing to do the work if the Clojure/core team wants a patch.





[CLJ-2063] Show longest path explain error first Created: 17/Nov/16  Updated: 17/Nov/16

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

Type: Enhancement Priority: Major
Reporter: Alex Miller Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: error-reporting, spec

Attachments: Text File longest-explain.patch    
Patch: Code
Approval: Vetted

 Description   

It is observed that the explain problem with the longest path is most likely the one that parsed the furthest and is thus the closest to the user's actual intent.

Proposed: Sort the explain problems with longest path first.

Patch: longest-explain.patch






[CLJ-2062] Spec import and refer-clojure macros Created: 17/Nov/16  Updated: 17/Nov/16

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

Type: Enhancement Priority: Major
Reporter: Alex Miller Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: core.specs, spec

Attachments: Text File import-referclj-2.patch    
Patch: Code
Approval: Vetted

 Description   

Add specs for import and refer-clojure.

Patch:

  • Fixes some indentation of previous specs
  • Factors out ::filters spec from ::ns-refer-clojure
  • Factors out ::import-list from ::ns-import
  • Reuses ::filters in ::ns-refer
  • Reuses ::filters in ::use-prefix-list
  • Removes :ret any? in ::ns-use (no need for it)
  • Adds clojure.core/import spec
  • Adds clojure.core/refer-clojure spec

Patch: import-referclj-2.patch






[CLJ-2061] Better error message when exercise-fn called on fn without :args spec Created: 17/Nov/16  Updated: 17/Nov/16

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

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

Attachments: Text File clj-2061.patch    
Patch: Code
Approval: Vetted

 Description   
;; no spec
user=> (s/exercise-fn str)
IllegalArgumentException No implementation of method: :specize* of protocol: #'clojure.spec/Specize found for class: nil  clojure.core/-cache-protocol-fn (core_deftype.clj:583)

;; no :args spec
user=> (s/fdef str :ret string?)
user=> (s/exercise-fn str)
IllegalArgumentException No implementation of method: :specize* of protocol: #'clojure.spec/Specize found for class: nil  clojure.core/-cache-protocol-fn (core_deftype.clj:583)

Proposed: Check for missing :args spec and throw better error

user=> (s/exercise-fn str)
Exception Unable to resolve args spec  clojure.spec/exercise-fn (spec.clj:1811)

user=> (s/fdef str :ret string?)
user=> (s/exercise-fn str)
Exception Unable to resolve args spec  clojure.spec/exercise-fn (spec.clj:1811)

Patch: clj-2061.patch






[CLJ-2060] Add undef to remove a spec Created: 16/Nov/16  Updated: 16/Nov/16

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

Type: Enhancement Priority: Major
Reporter: Alex Miller Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: spec

Attachments: Text File clj-2060.patch    
Patch: Code
Approval: Vetted

 Description   

Currently there is no provided way to remove a spec from the registry. During interactive development, particularly when working on complicated or recursive specs, it would be useful to have this ability.

Proposed: Add s/undef that removes a spec from the registry. In the patch it returns the updated registry, although that may be more harmful than helpful at the repl (where receiving nil would probably be less noise). Another option would be to return true/false indicating whether the key was in the registry. However, as the registry is held in an atom, this has a race and would be more involved to implement (so I didn't).

Patch: clj-2060.patch



 Comments   
Comment by Alex Miller [ 16/Nov/16 11:55 AM ]

Moving to 1.9 so it will get looked at, may not be added.





[CLJ-2059] explain-data problems don't provide resolved symbols under :pred Created: 15/Nov/16  Updated: 16/Nov/16

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

Type: Defect Priority: Major
Reporter: David Nolen Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: error-reporting, spec

Attachments: Text File clj-2059.patch    
Patch: Code and Test
Approval: Vetted

 Description   

Currently, explain-data returns unresolved preds. This is a problem when trying to write a custom explain print function that chooses what to do based on the predicate as it does not have enough information.

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

(defn password-valid-length? [pass]
  (> (count pass) 12))

(s/def ::password (s/and string? password-valid-length?))

(-> (s/explain-data ::password "foobar")
  ::s/problems first :pred) 
;;=> password-valid-length?  
;;expected: user/password-valid-length?

Cause: Currently, explain* returns preds in the abbrev form for all spec impls.

Proposed: Have explain* return resolved preds. In cases where the abbreviated form should be used (anything for human consumption at either the repl or an error message), convert to it. For example, explain-printer should (and already does) do this.

Patch: clj-2059.patch

  • Changes all spec impls to avoid calling abbrev on preds when building explain-data
  • Undoes op-describe change for s/? in CLJ-2042 and fixes this at a higher level by calling res on the incoming pred (this is a better fix)
  • Changes the expected test output for spec tests to expect the resolved pred





[CLJ-2058] s/keys doesn't check non-keyword elements in :req and :req-un vectors Created: 15/Nov/16  Updated: 15/Nov/16  Resolved: 15/Nov/16

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

Type: Defect Priority: Minor
Reporter: Eugene Pakhomov Assignee: Unassigned
Resolution: Not Reproducible Votes: 0
Labels: spec


 Description   

As can be seen here https://github.com/clojure/clojure/blob/master/src/clj/clojure/spec.clj#L430 the s/keys macro filters out any non-keyword element from :req and :req-un before checking it, but later it still uses them in the arguments to map-spec-impl.
Compare the behavior when passing non-keyword elements to :opt and :opt-un.



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

:req and :req-un support `and` and `or` connectives, :opt and :opt-un do not. So this all seems right to me.

I don't see any bug here?

Comment by Eugene Pakhomov [ 15/Nov/16 9:58 AM ]

I see your point. But how are `and` and `or` related to "all keys must be namespace-qualified keywords"?
And why to do the check at all for :opt and :opt-un that are not even used, when the check for :req and :req-un just allows any forms, not just documented `and` and `or`?

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

and and or are connectors, not keys.

I'd be fine if the ticket showed something invalid - no actual bug is being shown here. If you improve the ticket, I will re-open.





[CLJ-2057] Function spec missing :ret can yield wrong answer for valid? Created: 14/Nov/16  Updated: 16/Nov/16

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

Type: Defect Priority: Major
Reporter: James Gatannah Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: spec

Attachments: Text File clj-2057.patch    
Patch: Code
Approval: Vetted

 Description   

Create a spec on a function, leaving off the :ret.

user> (s/fdef ::foo :args (s/cat :n integer?))
=> :user:foo
user> (defn f [n] (* n 2))
=> #'user/f
;; Need org.clojure/test.check on classpath
user> (s/valid? ::foo f)
=> false
user> (s/explain-data ::foo f)
=> nil

Cause: Originally, :ret spec was required. We loosened that requirement, but parts of the implementation still assume that the :ret spec exists (valid-fn, etc). Here, s/valid? is incorrectly returning false because the returned value does not match the non-existent :ret spec, even though f should be fine. explain-data is doing the right thing (it's not failing).

Proposed: Patch in any? as the default :ret spec if it's missing. Another way to go would be to verify that all of the existing fspec conform and explain code worked as intended when :ret spec is missing - it seems like we would effectively be swapping in an any? spec in all of those cases though, so the proposed path seemed easier.

Patch: clj-2057.patch



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

fyi, fdef should take a qualified symbol, not a qualified keyword. To do what you're doing here, I would do:

(s/def ::foo (s/fspec :args (s/cat :n integer?)))
(defn f [n] (* n 2))
(s/valid? ::foo f)
(s/explain-data ::foo f)

Not that you will get a different result, but that's really the intent of the api. You're leaning a bit too much on implementation details that may change (namely that fdef is effectively def of an fspec - this didn't used to be the case and may not be the case in the future).





[CLJ-2056] Efficient shortcut for (first (filter pred coll)) idiom Created: 11/Nov/16  Updated: 14/Nov/16

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

Type: Enhancement Priority: Major
Reporter: Nikita Prokopov Assignee: Unassigned
Resolution: Unresolved Votes: 27
Labels: None

Attachments: Text File clj-2056-clojure-core-seek-2.patch    
Patch: Code and Test
Approval: Prescreened

 Description   

It's a common task to look up for an item in a collection based on predicate. Currently Clojure has no direct support for that in clojure.core. Instead, our options are:

1. (first (filter pred coll)) will create intermediate lazy sequence and might evaluate pred up to 31 extra times in case of chunked sequence

2. (some #(when (pred %) %) coll) will short-circuit on first match, but won't catch false value in something like (some #(when false? %) [true false true])

Additionally, both of these workarounds a) obscure the purpose of the code, and b) do not handle custom not-found values.

Attached is a patch that makes use of efficiency of reduce-able collections, handles edge cases like looking for false? or nil?, and supports optional not-found value.

Examples:

(seek odd? (range)) => 1
(seek pos? [-1 1]) => 1
(seek pos? [-1 -2] ::not-found) => ::not-found
(seek nil? [1 2 nil 3] ::not-found) => nil

Patch: clj-2056-clojure-core-seek-2.patch

Prescreening notes: I think the general approach is good. Is it necessary to support nil? and false? preds? Or would a transduce formulation like the one in comments be sufficient.



 Comments   
Comment by Alex Miller [ 11/Nov/16 8:54 AM ]

Just as an interesting aside, the new halt-when transducer could actually be used to create something like this too (if you set aside the desire to support nil? and false? preds).

(transduce (comp (filter pred) (halt-when any?)) identity nil coll)

Patch has some trailing whitespace in the test code - could you clean that up?

Comment by Nikita Prokopov [ 12/Nov/16 3:46 AM ]

Attaching patch with trailing whitespace cleaned

Comment by Nikita Prokopov [ 12/Nov/16 3:46 AM ]

Thanks Alex! Attached new patch with whitespace cleaned





[CLJ-2055] binding-form spec parses symbol-only maps incorrectly Created: 08/Nov/16  Updated: 09/Dec/16

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

Type: Defect Priority: Major
Reporter: Brandon Bloom Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: spec
Environment:

1.9.0-alpha14


Attachments: Text File CLJ-2055-01.patch    
Patch: Code
Approval: Screened

 Description   

The :clojure.core.specs/binding-form spec incorrectly treats some maps as sequential bindings.

Actual:

user=> (s/conform :clojure.core.specs/binding-form '{x y})
[:seq {:elems [[:seq {:elems [[:sym x] [:sym y]]}]]}]

Expected:

user=> (s/conform :clojure.core.specs/binding-form '{x y})
[:map {x y}]

Cause:

When there is no :keys, :strs, or :syms from :clojure.core.specs/map-special-binding, then :clojure.core.specs/seq-binding-form treats a map as sequential.

Proposed fix:

Include an (s/and vector? ...) check. See patch.

Patch: CLJ-2055-01.patch
Screened by: Alex Miller






[CLJ-2054] generator for `any?` occasionally generates `Double/NaN` for which equality semantics don't apply, and that is a problem for the :ret spec of many functions. Created: 07/Nov/16  Updated: 14/Dec/16

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

Type: Enhancement Priority: Major
Reporter: Dimitrios Piliouras Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: generator, spec
Environment:

Ubuntu 16.10 - Oracle Java 8


Approval: Triaged

 Description   

The generator for `any?` will occasionally give back Double/NaN value(s). Since NaNs & equality (via `=`) don't get along, :ret spec'ing a fn which transforms/processes a collection according to a predicate, becomes rather problematic. That's because the most obvious thing to check under :ret (the case where the predicate didn't return true for any value, and so the output coll should be equal to the input coll because nothing was transformed/processed), cannot be expressed trivially.

The workaround I've come up with in my own specs is to spec the elements of the collection with `(s/and any? (complement double-NaN?))` instead of just `any?`, and it works. However, even though I can live without NaNs in the tests, I must admit it still feels sort of hacky.

Ideas:

1) The generator for `any?` could be hardcoded to never return Double/NaN. Sounds rather invasive.
2) The generator for `any?` could be reworked to somehow be configurable wrt allowing/prohibiting Double/NaNs. Then perhaps a dynamic-var and/or a macro (e.g. `without-NaNs`) could expose this (just brainstorming here).
3) The generator for `any? could stay as is, but a new equality operator could be added (e.g. `clojure.spec/===`), which somehow ignores NaNs (a naive implementation for instance might walk the data-structures and replace all NaNs with keywords, and only then perform a regular comparison).



 Comments   
Comment by Alex Miller [ 08/Nov/16 10:29 AM ]

Should consider whether this change is more appropriate in test.check or in the spec generator for any?.

Comment by Dimitrios Piliouras [ 11/Nov/16 12:31 PM ]

It turns out that my workaround does not fully work. I literally just stumbled in the following case:

{nil {[] {NaN 0}}}

which is a conforming value for:

(s/def ::persistent-map
(s/map-of ::anything-but-NaN ::anything-but-NaN)) ;; (s/and any? (complement double-NaN?))

So basically, the inner collections can still have NaNs. So far I've got 4 specs that I've written and faced this problem on all of them.





[CLJ-2052] .class vs .clj isn't picked correctly when either .class or .clj are not in a jar Created: 03/Nov/16  Updated: 04/Nov/16  Resolved: 04/Nov/16

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

Type: Defect Priority: Major
Reporter: Mike Kaplinskiy Assignee: Unassigned
Resolution: Not Reproducible Votes: 0
Labels: None


 Description   

The code that figures out the last modification date (https://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/RT.java#L389) uses URLConnection.getLastModified. For `file://` URLs this always returns 0.

One of the resulting bugs: when both .class and .clj files are present on a directory-based classpath, .clj files are always preferred, regardless of modification time.



 Comments   
Comment by Alex Miller [ 03/Nov/16 6:15 PM ]

Can you provide OS and JVM info? This might be env-specific.

Comment by Mike Kaplinskiy [ 03/Nov/16 6:20 PM ]

Sure - I'm on macOS Sierra on Oracle Java 8:

$ java -version
java version "1.8.0_74"
Java(TM) SE Runtime Environment (build 1.8.0_74-b02)
Java HotSpot(TM) 64-Bit Server VM (build 25.74-b02, mixed mode)
$ uname -a
Darwin mikekap-mbp.local 16.1.0 Darwin Kernel Version 16.1.0: Thu Oct 13 21:26:57 PDT 2016; root:xnu-3789.21.3~60/RELEASE_X86_64 x86_64 i386 MacBookPro11,5 Darwin
Comment by Alex Miller [ 03/Nov/16 6:28 PM ]

It's prob not too fun but some example code would be awfully handy.

Comment by Mike Kaplinskiy [ 03/Nov/16 7:13 PM ]

Sorry this looks like it was my fault - the path I was looking at when testing didn't exist (seems .getLastModified always returns 0 on those instead of throwing an exception).

Sorry for the noise.

Comment by Alex Miller [ 04/Nov/16 9:26 AM ]

Closed per comments.





[CLJ-2051] Typo in clojure.instant/validated docstring Created: 03/Nov/16  Updated: 03/Nov/16

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

Type: Defect Priority: Trivial
Reporter: Greg Leppert Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring, typo

Attachments: Text File 58f6bca6ba64ca343b43585463eff1be74aeb965.patch    
Patch: Code
Approval: Prescreened

 Description   
  • "Return a function which constructs and instant by calling constructor
    + "Return a function which constructs an instant by calling constructor

Prescreened by: Alex Miller






[CLJ-2050] Remove redundant key comparisons in HashCollisionNode Created: 03/Nov/16  Updated: 03/Nov/16

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

Type: Enhancement Priority: Minor
Reporter: Kwang Yul Seo Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: collections, performance

Attachments: Text File 0001-Remove-redundant-key-comparisons-in-HashCollisionNod.patch    
Patch: Code
Approval: Prescreened

 Description   

Comparing key to array[idx] is redundant as findIndex already performed key comparison to find the index.

Prescreened by: Alex Miller






[CLJ-2049] Improve clojure.zip documentation Created: 25/Oct/16  Updated: 27/Oct/16

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

Type: Enhancement Priority: Minor
Reporter: Brighid M Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring, zip
Environment:

All


Attachments: File improve-zip-docs.diff    
Patch: Code
Approval: Triaged

 Description   

The clojure.zip module has extremely terse docstrings that are helpful as reference material but completely unhelpful to someone approaching the module cold. Expanded docstrings would make this piece of code much more visible to users who might find it helpful.



 Comments   
Comment by Brighid M [ 25/Oct/16 9:03 PM ]

A patch that improves clojure.zip's docstrings.

Comment by Alex Miller [ 27/Oct/16 8:42 AM ]

I would prefer if we could minimize the size of the changes by removing diffs that just add periods, change line breaks, add whitespace, or make other non-essential changes. That will help focus on the things that matter like changes in the ns docstring, zipper, etc. Additionally this ticket talks about doc strings but also makes changes in exception messages - I'd prefer code changes to be in a separate ticket. Also, please don't remove the comment sections in the files.

If you would like to convert the test comment section into actual tests, that would be a great separate ticket (I feel like I might have even written a patch to do that at one point but I don't see a ticket for it!).

Keeping this patch entirely focused on essential docstring changes is the best way to ensure its timely inclusion.

Comment by Brighid M [ 27/Oct/16 4:32 PM ]

Alex — to clarify, I'm hearing "this ticket should include two patches: one with the major prose additions and one with small proofreading-ish changes" and "this ticket's patches should not include the changes to exception messages nor the comment-move"?

Supplemental question: is there a style guide hanging around for the code & documentation style of the Clojure core? I poked around for such a guide on clojure.org and in JIRA: I didn't find one, but maybe I overlooked it. I was looking for a guideline about the comment section. It would be good to have a hint about "please don't touch these," because AFAICT they don't communicate that by themselves.

Comment by Alex Miller [ 27/Oct/16 5:43 PM ]

Actually, I'd prefer to have just one patch with the major prose additions. I don't think the minor changes are worth doing and will be a distraction from the more important prose.

Unfortunately, there is no style guide for Clojure core, and in fact it's been written by many people over many years so there generally isn't a consistent style throughout the code. Generally Rich prefers that debug code or comments that trace to him are left intact (git blame can help there). More generally, the simpler a patch is to review, the easier it is for it to stay good and be easily reviewed.

Thanks!

Comment by Brighid M [ 27/Oct/16 6:34 PM ]

> Actually, I'd prefer to have just one patch with the major prose additions.
Okay, I'll produce that patch and attach it to this ticket.

> I don't think the minor changes are worth doing and will be a distraction from the more important prose. Unfortunately, there is no style guide for Clojure core, and in fact it's been written by many people over many years so there generally isn't a consistent style throughout the code.
I think you just made an argument for why it is worth doing minor changes: consistency generally makes things more accessible. However, now is clearly not the time to litigate that, so I'm gonna come back with just the ns/docstring version of this patch.





[CLJ-2048] java.lang.ClassCastException: [Ljava.lang.Object; cannot be cast to [Ljava.lang.StackTraceElement Created: 21/Oct/16  Updated: 21/Oct/16

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

Type: Defect Priority: Major
Reporter: Gerrit Jansen van Vuuren Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJ-2048-b.patch    
Patch: Code
Approval: Prescreened

 Description   

clojure.core/throw-if creates an array to call Exception.setStracktrace() without specifying the array type. This works fine when passed at least one StackTraceElement, but in the case where passed no stack trace elements (all are filtered or stack traces are being elided by the JVM), this will be an Object[] which results in a ClassCastException:

Exception in thread "main" java.lang.ClassCastException: [Ljava.lang.Object; cannot be cast to [Ljava.lang.StackTraceElement;
        at clojure.core$throw_if.invokeStatic(core.clj:5649)
        at clojure.core$load_one.invokeStatic(core.clj:5698)
        at clojure.core$compile$fn__5682.invoke(core.clj:5903)
        at clojure.core$compile.invokeStatic(core.clj:5903)
        at clojure.core$compile.invoke(core.clj:5895)
        at clojure.lang.Var.invoke(Var.java:379)
        at clojure.lang.Compile.main(Compile.java:67)

This is tricky to reproduce because it involves stack trace filtering so there is no reproducing case here.

Patch: CLJ-2048-b.patch
Prescreened by: Alex Miller



 Comments   
Comment by Gerrit Jansen van Vuuren [ 21/Oct/16 4:18 AM ]

patch calls into-array with StackTraceElement type

Comment by Alex Miller [ 21/Oct/16 8:01 AM ]

How do you cause this to occur?

Comment by Alex Miller [ 21/Oct/16 8:11 AM ]

into-array will create a typed array based on the first element of the seq it is passed, so generally you should see a StackTraceElement[] here. I think the only time this would fail is if it was passed no stack trace elements.

Comment by Alex Miller [ 21/Oct/16 8:19 AM ]

I'd be happy to move this through screening, but the patch needs to be in the proper format (see http://dev.clojure.org/display/community/Developing+Patches).

Comment by Gerrit Jansen van Vuuren [ 21/Oct/16 8:42 AM ]

I'm trying to reproduce this in a way that can be presented here, but I got the compile error just after doing some serious package renaming, and can't reproduce it outside of the project itself.

Comment by Gerrit Jansen van Vuuren [ 21/Oct/16 8:45 AM ]

ok, I'll reformat the patch after reading (http://dev.clojure.org/display/community/Developing+Patches)

Comment by Gerrit Jansen van Vuuren [ 21/Oct/16 9:15 AM ]

I've created a new patch based on the guidelines, attached as file: CLJ-2048-b.patch.

Just to summarise:
The into-array returns the correct type if its provided with a none empty sequence, but if the sequence is empty it cannot know the type and then returns an object array. Because we set the array here to a java method Exception::setStackTrace passing it an object array causes a ClassCastException. One fix is to check for an empty sequence, but a less verbose way is just to pass the type which is known as part of the call to into-array, this is what is done in the patch CLJ-2048-b.patch.





[CLJ-2046] generate random subsets of or'd required keys in map specs Created: 17/Oct/16  Updated: 17/Oct/16

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

Type: Enhancement Priority: Major
Reporter: David Chelimsky Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: generator, spec

Attachments: Text File map-spec-gen-enhancements.patch    
Patch: Code and Test
Approval: Screened

 Description   

(s/keys :req [(or ::x ::y)]) always generates maps with both ::x and ::y but it should also generate maps with either ::x or ::y.

The attached patch supports arbitrarily deeply nested or and and expressions within the values of :req and :req-un in map specs. It also uses the same 'or' mechanism for :opt and :opt-un keys, thereby replacing the use of clojure.core/shuffle with clojure.test.check.generators/shuffle, ensuring repeatability of the generators.

Patch: map-spec-gen-enhancements.patch

Screened by: Alex Miller






[CLJ-2045] bean function not working Created: 17/Oct/16  Updated: 17/Oct/16  Resolved: 17/Oct/16

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

Type: Defect Priority: Major
Reporter: Roy Varghese Assignee: Unassigned
Resolution: Duplicate Votes: 0
Labels: None
Environment:

(defproject hello-jcloud "0.1.0-SNAPSHOT"
:description "FIXME: write description"
:url "http://example.com/FIXME"
:license {:name "Eclipse Public License"
:url "http://www.eclipse.org/legal/epl-v10.html"}
:dependencies [#_[org.clojure/clojure "1.8.0"]
[org.clojure/clojure "1.9.0-alpha13"]
[org.clojure/tools.logging "0.1.0"]
[org.apache.jclouds/jclouds-all "2.0.0-SNAPSHOT"]]
:repositories {"jclouds-snapshots" "https://repository.apache.org/content/repositories/snapshots"})



 Description   

user> (bean (java.util.Date.))
UnsupportedOperationException empty clojure.core.proxy$clojure.lang.APersistentMap$ff19274a.empty (:-1)
user>

It works when clojure/1.8.0 is uncommented.



 Comments   
Comment by Alex Miller [ 17/Oct/16 7:38 AM ]

This is a dupe of http://dev.clojure.org/jira/browse/CLJ-2027 which will probably be in the next alpha.





[CLJ-2044] Four functions in clojure.instant have incomplete documentation Created: 15/Oct/16  Updated: 24/Oct/16

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

Type: Enhancement Priority: Major
Reporter: Bruce Adams Assignee: Bruce Adams
Resolution: Unresolved Votes: 0
Labels: docstring, instant

Attachments: Text File defns-for-instant-def-timestamp.patch     Text File defns-for-instant.patch    
Patch: Code
Approval: Prescreened

 Description   

Of the five public functions defined in clojure.instant, these four:

  • parse-timestamp
  • read-instant-calendar
  • read-instant-date
  • read-instant-timestamp

are each declared as a Var without any arglists metadata. This means documentation does not contain function calling information.

In http://clojure.github.io/clojure/clojure.instant-api.html, each of these functions is described as a var and there is no Usage: ... information given.

The output of doc does not include argument list information. For example:

user=> (doc clojure.instant/read-instant-date)
-------------------------
clojure.instant/read-instant-date
  To read an instant as a java.util.Date, bind *data-readers* to a map with
this var as the value for the 'inst key. The timezone offset will be used
to convert into UTC.

A related problem is that stack traces show anonymous functions instead of the names for any of these functions. For example:

user=> (clojure.instant/read-instant-timestamp "123")
RuntimeException Unrecognized date/time syntax: 123  clojure.instant/fn--6879/fn--6880 (instant.clj:107)

Proposed: Refactor the code into defn functions which makes the code clearer and addresses the documentation issue. An alternate approach would be to apply :arglists metadata to the vars.

Patch: defns-for-instant-def-timestamp.patch
Prescreened: Alex Miller



 Comments   
Comment by Bruce Adams [ 15/Oct/16 12:40 PM ]

Proposed solution: refactor the definitions of the four problematic functions to be defined using defn.

Comment by Bruce Adams [ 16/Oct/16 5:24 PM ]

Some of my thinking leading to the solution I propose.

Initially, when I realized that I didn't know what arguments parse-timestamp required, I assumed the appropriate fix was to enhance the docstring. Then I noticed that the on-line documentation for functions was formatted quite differently from the text output by doc. Any decent fix was going to have to feed function information into different variants of documentation formatting code.

I can guess, from other examples such as first, that :arglists metadata is what indicates that a var is a function. One possible solution would be to add :arglists to each of the four functions. It felt cleaner to refactor the code into simple defn functions. Refactoring code just for the side effect of documentation seems a bit odd, but the code itself strikes me as more legible after my refactoring.

Comment by Alex Miller [ 17/Oct/16 9:53 AM ]

This seems reasonable to me. I would move the timestamp regex into a separate (private) var instead of creating it in parse-timestamp.

It's possible the way these functions were defined was designed to minimize startup time or something like that, but I don't have any background on that.

Comment by Bruce Adams [ 23/Oct/16 4:06 PM ]

Updated patch based on Alex's great suggestion. This adds a separate, private, def for the timestamp regex.





[CLJ-2043] s/form of conformer is broken Created: 14/Oct/16  Updated: 14/Oct/16

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

Type: Defect Priority: Major
Reporter: Alex Miller Assignee: Unassigned
Resolution: Unresolved Votes: 3
Labels: spec

Attachments: Text File clj-2043.patch    
Patch: Code
Approval: Screened

 Description   

s/form of s/conformer is wrong:

(s/form (s/conformer str))
=> str

Proposed: Fix the form for conformer to match the conformer call.

Patch: clj-2043.patch






[CLJ-2042] s/form of s/? does not resolve pred Created: 14/Oct/16  Updated: 28/Oct/16  Resolved: 28/Oct/16

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

Type: Defect Priority: Major
Reporter: Alex Miller Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: spec

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

 Description   
user=> (require '[clojure.spec :as s])
nil
user=> (s/form (s/? int?))
(clojure.spec/? int?)

Patch: clj-2042.patch






[CLJ-2041] clojure.spec/keys requires input collections conform to clojure.core/map? Created: 11/Oct/16  Updated: 14/Nov/16

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

Type: Defect Priority: Minor
Reporter: Timothy Baldridge Assignee: Unassigned
Resolution: Unresolved Votes: 5
Labels: spec

Approval: Vetted

 Description   

I would like to use specs to validate Datomic entities. However, `s/keys` is too restrictive in that it requires input collections to conform to `clojure.core/map?` instead of some more primitive interface (for example clojure.lang.ILookup or clojure.lang.Associative).



 Comments   
Comment by Alex Miller [ 04/Nov/16 8:21 AM ]

s/keys uses IPersistentMap's Iterable support for iterating through all entries for validation. ILookup and Associative do not support iteration. So, that's why it is the way it is. But, understand the desire.

Comment by Alex Miller [ 04/Nov/16 8:34 AM ]

Datomic entities are seqable so maybe that's a potential path (would be slower for actual PHMs though).

Comment by Odin Standal [ 04/Nov/16 8:35 AM ]

Thanks for following up. So any ideas or guidance on how to use clojure.spec with Datomic entities?

Comment by Alex Miller [ 04/Nov/16 8:37 AM ]

For now, you could use into to pour an entity into a PHM before validating. I hesitate to suggest it, but that could even be in the spec with a leading conformer.

Comment by Alex Miller [ 14/Nov/16 12:16 PM ]

Moving this into 1.9 for the moment just so we don't lose it. Not sure whether we can or will actually do anything with this though.





[CLJ-2040] Allow runtime modification of REPL exception handling Created: 11/Oct/16  Updated: 11/Oct/16

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

Type: Enhancement Priority: Major
Reporter: Luke VanderHart Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: repl

Attachments: Text File CLJ-2040-dynamic-repl-exceptions.patch    
Patch: Code
Approval: Triaged

 Description   

Problem Statement

Clojure's REPL is capable of paramterizing almost every aspect of its functionality, including how uncaught exceptions are printed. In the current implementation, these customization hooks are passed in as arguments and closed over, meaning that they cannot be changed once the REPL is started.

Many development tools want to override how the REPL handles uncaught errors. Examples of useful customizations include (but are not limited to):

  • Formatted exception messages (including whitespace and ANSI coloring)
  • Alternative representations for certain types of exceptions (e.g, Spec errors)
  • Dropping into a graphical interaction mode to better inspect ex-data.

Currently, this type of customization must be applied before a REPL is started, meaning that changing how a REPL displays errors requires support from (or plugins to) a third-party tool such as Boot or Leiningen.

Alternatives


1. Take no action.

Third-party tool support is required to create customized exception handling in the REPL. Tools have different techniques for doing this:

  • nREPL can intercept the exception on the wire and passes it through middleware
  • Leiningen plugins alter the root binding of clojure.main/repl-caught.
  • Boot allows users to build a task to invoke clojure.main/repl with the desired arguments.

Users will continue to select one of these according to their tooling preferences.

Benefits:
1. No effort or changes to the existing code.

Tradeoffs:
1. Tools will continue to implement their own diverse, sometimes hacky techniques for printing custom exceptions.
2. Any library intended to provide alternative exception handling will be tied to a specific launcher tool.

2. Make the REPL exception handler dynamically rebindable

If the REPL exception handler were a dynamic, thread-local var, users and libraries could change the behavior of the currently running REPL.

Benefits:
1. Users and libraries can freely override how exceptions are printed, regardless of how Clojure was launched.
2. Fully backwards compatible with existing tools.

Tradeoffs:
1. It will be possible for library authors to provide "bad" or poorly reasoned error printers. This is still possible with launch tools, but the barrier of entry is even lower with libraries.

The attached patch implements this option.

3. Encourage users to start new REPLs instead

In many Clojure environments, it's possible to explicitly launch a REPL from within another REPL. This sub-REPL could have the desired :caught hook.

Benefits:
1. No effort or changes to the existing code.
2. "Functionally pure", and in alignment with the evident design of the current REPL.

Tradeoffs:
1. There is a non-trivial subset of Clojure developers who do not know exactly how REPLs work. They are likely to be confused or subject to increased cognitive load. Insofar as this set of beginner/intermediate developers are precisely who enhanced error messages are meant to help in the first place, this solution is counterproductive.
2. For better or for worse, many existing and widely used tools do not support this. This does not work at all in nREPL, for example. However, even the simplest command-line REPLs behavior would change for the worse; sending a EOF (accidentally or otherwise) would always kill the sub-REPL with no feedback as to what just happened.






[CLJ-2039] typo in deftype doc string Created: 09/Oct/16  Updated: 11/Oct/16

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

Type: Enhancement Priority: Trivial
Reporter: Steve Miner Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring, typo

Attachments: Text File CLJ-2039-deftype-typo.patch    
Patch: Code
Approval: Prescreened

 Description   

The doc string for deftype refers to "record class" where it should say "type".






[CLJ-2038] Clojure.spec/exercise-fn should accept custom generator map Created: 08/Oct/16  Updated: 07/Nov/16

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

Type: Enhancement Priority: Minor
Reporter: Andrea Richiardi Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: spec

Attachments: File CLJ-2038-exercise-fn-should-accept-custom-generator.diff    
Approval: Triaged

 Description   

I tried to generate some data with exercise-fn but could not do it because I need to carry my own custom generators.
At the moment though, there is no way to add them, like an optional parameter.



 Comments   
Comment by Alex Miller [ 11/Oct/16 12:09 PM ]

Patch would be welcome for this!

Comment by Laszlo Török [ 07/Nov/16 9:23 AM ]

First attempt to form a patch.

I am unsure whether it is ok to use the overloaded 3rd argument approach or take a keyword arg approach for the optional arguments.

i.e. ([sym-or-fn n & {:keys [fspec gen]}])

On the other hand, it only make sense to pass one or the other and given sym-or-fn, fspec-or-gen seems appropriate.

Currently the test suite for c.spec is lacking, I'm happy to add a few example-based tests for exercise-fn, once the approach is approved.
Also





[CLJ-2037] specs in registry lack :file metadata despite having :line, :column Created: 08/Oct/16  Updated: 01/Dec/16

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

Type: Enhancement Priority: Major
Reporter: Felix Andrews Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: spec

Approval: Triaged

 Description   

As of 1.9.0-alpha13, specs in the registry lack :file metadata despite having :line, :column

user=> (require '[clojure.spec :as s])
user=> (-> (s/registry) (get :clojure.core.specs/arg-list) (meta))
{:line 1118, :column 5, :clojure.spec/name :clojure.core.specs/arg-list}
user=> (-> (s/registry) (get 'clojure.core/let) (meta))
{:line 1675, :column 5, :clojure.spec/name clojure.core/let}

This would be useful because:

  • we could list all the specs defined in a project, by filtering the registry.
  • we could read the source of a spec, like clojure.repl/source, for pretty formatting.

(specifically, for use in Codox https://github.com/weavejester/codox/pull/134 )

I had a quick look but couldn't see where the metadata is set.
Cheers



 Comments   
Comment by Alex Miller [ 08/Oct/16 11:12 AM ]

You can use s/describe or s/form to grab the source of a spec now, btw.

Comment by Felix Andrews [ 12/Oct/16 11:29 PM ]

The following works in my tests. (For testing I used in-ns, @#'registry-ref, #'ns-qualify)).

The approach is to set the registry item metadata after a def. It is not enough to set metadata on the def'd value because it is subsequently altered inside def.

(ns clojure.spec)
(alias 'c 'clojure.core)

(defmacro def
  [k spec-form]
  (let [k (if (symbol? k) (ns-qualify k) k)
        m (assoc (meta &form) :file *file*)]
    `(do
       (def-impl '~k '~(res spec-form) ~spec-form)
       (swap! registry-ref update '~k vary-meta c/merge ~m)
       '~k)))

(defmacro fdef
  [fn-sym & specs]
  (let [k (ns-qualify fn-sym)
        m (assoc (meta &form) :file *file*)]
    `(do
       (clojure.spec/def ~fn-sym (clojure.spec/fspec ~@specs))
       (swap! registry-ref update '~k vary-meta c/merge ~m)
       '~k)))

You can use s/describe or s/form to grab the source of a spec now, btw.

Yes, that's nice except for longer specs when line wrapping and indentation would help.

Comment by Jozef Wagner [ 01/Dec/16 12:31 PM ]

Note that current :line and :column meta are not pointing to the place where the spec was defined but to the clojure/spec.clj file, e.g. second example (c.c/let) points to fspec-impl





[CLJ-2036] Generators for some? and any? only return collections or nil Created: 07/Oct/16  Updated: 11/Oct/16

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

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

Approval: Vetted

 Description   

Both of these should generate a better variety of values (strings, keywords, symbols, numbers) in addition to collections and nil.



 Comments   
Comment by Sean Corfield [ 07/Oct/16 11:55 AM ]

See also http://dev.clojure.org/jira/browse/TCHECK-111 which may solve this directly?

Comment by Gary Fredericks [ 09/Oct/16 2:08 PM ]

This is probably http://dev.clojure.org/jira/browse/TCHECK-83, which is fixed on test.check master.

Comment by Alex Miller [ 11/Oct/16 12:14 PM ]

I'm going to leave this open pending a new release and dependency update to test.check, which I presume will address it.





[CLJ-2035] Bad s/form for collection specs Created: 07/Oct/16  Updated: 14/Oct/16

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

Type: Defect Priority: Major
Reporter: Johan Gall Assignee: Alex Miller
Resolution: Unresolved Votes: 5
Labels: spec

Attachments: Text File clj-2035-2.patch     Text File clj-2035.patch    
Patch: Code and Test
Approval: Screened

 Description   

There are several problems with s/form for collection specs (coll-of,map-of,every,every-kv):

1. coll spec forms expose implementation details of building on every:

(s/form (s/coll-of int?))
=> (clojure.spec/every int? :clojure.spec/cpred #object[user$eval16$fn__18 0xd506900 "user$eval16$fn__18@d506900"] :clojure.spec/kind-form nil :clojure.spec/conform-all true)

2. form does not resolve nested spec preds:

(s/def ::a (s/every (s/tuple ::b)))

(s/form ::a)
=> (clojure.spec/every (*s/tuple* :user/b) [ ... ])

(which impacts map-of and coll-of).

3. :kind fn is not resolved

(s/form (s/coll-of int? :kind vector?))
=> (clojure.spec/every int? :clojure.spec/cpred #object[user$eval4$fn__6 0x8fc095 "user$eval4$fn__6@8fc095"] :clojure.spec/kind-form clojure.core/vector? :kind #object[clojure.core$vector_QMARK___6428 0x6596f6ef "clojure.core$vector_QMARK___6428@6596f6ef"] :clojure.spec/conform-all true)

Ignoring the rest of the problems from #1, the :kind should be here but should be the resolved form (clojure.core/vector?).

Patch: clj-2035-2.patch






[CLJ-2034] comment macro doesn't ignore namespace keyword Created: 04/Oct/16  Updated: 04/Oct/16  Resolved: 04/Oct/16

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

Type: Defect Priority: Minor
Reporter: Nuttanart Pornprasitsakul Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None


 Description   

In fresh started REPL:

user=> (comment (x/y))
nil
user=> (comment ::x/y)
RuntimeException Invalid token: ::x/y  clojure.lang.Util.runtimeException (Util.java:221)
RuntimeException Unmatched delimiter: )  clojure.lang.Util.runtimeException (Util.java:221)


 Comments   
Comment by Alex Miller [ 04/Oct/16 11:37 PM ]

This code is not valid because x is an invalid alias (not bound to anything).

This works for example:

user=> (alias 'x 'clojure.string)
nil
user=> (comment ::x/y)
nil

comment still reads the code, so code that is unable to be read is still invalid.

Comment by Alex Miller [ 04/Oct/16 11:38 PM ]

Note that CLJ-2030 would make this code work (by auto-creating x as a new namespace).





[CLJ-2033] s/conformer conversion loss in (very common) s/merge edge case Created: 04/Oct/16  Updated: 04/Oct/16  Resolved: 04/Oct/16

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

Type: Defect Priority: Major
Reporter: Alexander Kahl Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: spec
Environment:

1.9.0-alpha14



 Description   

When using s/conform on a spec generated by s/merge that uses non-namespaced keys from s/keys, keys that use a custom s/conformer lose their conversion.

Steps to reproduce:

Unable to find source-code formatter for language: clojure. Available languages are: javascript, sql, xhtml, actionscript, none, html, xml, java
(require '[clojure.spec :as s])
(defn convert [n] (if (double? n) n (double n)))
(s/def ::value (s/conformer convert))
(s/conform (s/keys :req [::value]) {::value 5}) => #:user{:value 5.0} ; correct
(s/conform (s/keys :req-un [::value]) {:value 5}) => {:value 5.0} ; correct
(s/conform (s/merge (s/keys :req [::value]) (s/keys)) {::value 5}) => #:user{:value 5.0} ; correct
(s/conform (s/merge (s/keys :req-un [::value]) (s/keys)) {:value 5}) => {:value 5} ; WRONG

While this seems like an edge case, it is very likely to occur since using s/merge with unqualified keys is a typical use case for configuration files. I first spotted this behavior in alpha13 and it still occurs in alpha14.



 Comments   
Comment by Alex Miller [ 04/Oct/16 8:36 AM ]

alpha14 isn't out yet, so thanks for traveling back in time to report this!

I think this is not a bug, just a subtlety in how merge conform works. Specifically, merge does not flow conformed results, you only get the conformed result of the last spec in the merge.

In this case:

(s/conform (s/merge (s/keys :req [::value]) (s/keys)) {::value 5})

the spec determining the conform value is (s/keys), which will conform all namespaced keys, including ::value.

In this case:

(s/conform (s/merge (s/keys :req-un [::value]) (s/keys)) {:value 5})

the spec determining the conform value is also (s/keys), but s/keys only conforms namespaced keys and there aren't any here, so you get the original map.

If you want the conformed value, you can swap the order in the merge because the first spec is conforming the unqualified key:

(s/conform (s/merge (s/keys) (s/keys :req-un [::value])) {:value 5})
;;=> {:value 5.0}
Comment by Alexander Kahl [ 04/Oct/16 8:50 AM ]

Oh geez, I meant 13 (and first observed in 12), wish I could actually travel back in time!

If what you're saying is right, why doesn't this work, as both (s/keys) use only unqualified keys?

(s/def ::other string?)
(s/conform (s/merge (s/keys :req-un [::value]) (s/keys :req-un [::other])) {:value 5 :other "5"}) => {:value 5, :other "5"}
Comment by Alexander Kahl [ 04/Oct/16 9:08 AM ]

I just read again your comment Alex Miller and finally started to understand how (s/keys) conforms all namespaced keys, so please disregard my other comment.
I still wish there was a solution that worked for multiple maps of unqualified keys. Otherwise, I'd have to expect my users to use qualified keys throughout configuration files which looks a lot like redundancy, unless I convert all the keys first.





[CLJ-2032] Confusing error conforming fspec with missing arg spec Created: 03/Oct/16  Updated: 28/Oct/16  Resolved: 28/Oct/16

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

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

1.9.0-alpha13


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

 Description   
(require '[clojure.spec :as s])
(def my-spec (s/fspec :ret string?))
(s/conform my-spec (fn [j] (str j)))
IllegalArgumentException No implementation of method: :specize* of protocol: #'clojure.spec/Specize found for class: nil  clojure.core/-cache-protocol-fn (core_deftype.clj:568)
	clojure.core/-cache-protocol-fn (core_deftype.clj:583)
	clojure.spec/fn--13560/G--13555--13569 (spec.clj:121)
	clojure.spec/specize (spec.clj:138)
	clojure.spec/gensub (spec.clj:262)
	clojure.spec/gen (spec.clj:275)
	clojure.spec/gen (spec.clj:275)
	clojure.spec/validate-fn (spec.clj:1664)
	clojure.spec/fspec-impl/reify--14270 (spec.clj:1686)
	clojure.spec/conform (spec.clj:150)
	clojure.spec/conform (spec.clj:146)

Proposed: When conforming, throw if no args spec specified for the fspec:

Can't conform fspec without args spec: (fspec :args nil :ret string? :fn nil)

Alternatives

  • absence of args spec always conforms ::invalid
  • absence of args spec always conforms any args
  • disallow fspecs without args (still potentially useful for other uses like documentation, so not sure we want to do that)

Patch: clj-2032.patch






[CLJ-2031] clojure.walk/postwalk does not preserve MapEntry type objects Created: 01/Oct/16  Updated: 27/Oct/16

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

Type: Defect Priority: Major
Reporter: Sean Corfield Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: walk

Attachments: File clj-2031-w-test.diff     File clj-2031-w-test-v2.diff    
Patch: Code and Test
Approval: Prescreened

 Description   

This came up on Slack. A naïve implementation of "lispify" to turn vectors into lists used this code:

(defn lispify [s]
  (w/postwalk (fn [e] (if (vector? e) (apply list e) e)) s))

But when called like this:

(lispify [:html {:a "b"} ""])

It produces this error: java.lang.ClassCastException: clojure.lang.Keyword cannot be cast to java.util.Map$Entry

My initial reaction was to change the condition to (and (vector? e) (not (map-entry? e))) but that still failed, because while walking the hash map, the MapEntry [:a "b"] was turned into a PersistentVector.

At this point, we can switch to using prewalk and it works as expected:

(defn lispify [s]
  (w/prewalk (fn [e] (if (and (vector? e) (not (map-entry? e))) (apply list e) e)) s))

Now we get the expected result:

boot.user> (lispify [:html {:a "b"} ""])
(:html {:a "b"} "")

This seems unintuitive at best and feels like a bug: postwalk should preserve the MapEntry type rather than converting it to a PersistentVector.

The problem seems to be this line https://github.com/clojure/clojure/blob/master/src/clj/clojure/walk.clj#L45:

(instance? clojure.lang.IMapEntry form) (outer (vec (map inner form)))

Would it be reasonable for this to become:

(instance? clojure.lang.IMapEntry form) (outer (clojure.lang.MapEntry/create (inner (first form)) (inner (second form)))))

This would preserve the type of the subelement.

Patch: clj-2031-w-test-v2.diff
Prescreened by: Alex Miller



 Comments   
Comment by Alex Miller [ 11/Oct/16 12:19 PM ]

seems reasonable

Comment by Jozef Wagner [ 27/Oct/16 8:19 AM ]

Added patch with test

Comment by Alex Miller [ 27/Oct/16 8:34 AM ]

Instead of the calls to .key and .val you should just call key and val.

Comment by Jozef Wagner [ 27/Oct/16 8:42 AM ]

Good catch, thanks! Added patch clj-2031-w-test-v2.diff that uses key and val instead.





[CLJ-2030] Auto-create alias namespaces Created: 28/Sep/16  Updated: 06/Dec/16

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

Type: Enhancement Priority: Major
Reporter: Herwig Hochleitner Assignee: Unassigned
Resolution: Unresolved Votes: 6
Labels: keywords, namespace, portability, spec

Attachments: Text File clj-2030-2.patch     Text File clj-2030-3-1.patch     Text File clj-2030-3-2.patch     Text File clj-2030.patch    
Patch: Code and Test
Approval: Screened

 Description   

It is useful to name keywords in namespaces, without creating or requiring those namespaces. When wanting to do that via an ::alias/keyword, the aliased namespace has to actually exist, in order to be aliased.
Currently, in Clojure, this can be achieved dynamically, through a combination of create-ns and alias, Clojurescript requires a dummy file and a :require :as.

Proposals:

1. Extend clojure.core/alias to auto-create missing namespaces
2. Extend clojure.core/alias to accept varargs & {:as kvs}
3. Extend ns to accept (:alias ...) clauses

Patch:

  • clj-2030.patch does 1 (but not 2 or 3), and was screened by SDH
  • clj-2030-2.patch does 1+2 (but not 3)
  • clj-2030-3-1.patch does 1+3 (but not 2)
  • clj-2030-3-2.patch does 1+2+3

Before:

user=> (alias 'parts 'company.domain.parts)
java.lang.Exception: No namespace: company.domain.parts found

After:

user=> (alias 'parts 'company.domain.parts)
nil
user=> ::parts/widget
:company.domain.parts/widget


 Comments   
Comment by Alex Miller [ 28/Sep/16 10:04 PM ]

From original description:

My use case is a simplification of data.xml, which would benefit greatly from a uniform way to alias + auto-create namespaces within the ns clause.

I would like to support the syntax:

(ns foo.bar
  (:alias xh  #xml/ns "http://..<xhtml>.."
          svg #xml/ns "http://..<svg>.."))

{:tag ::xh/div
 :content [{:tag ::svg/g}]}

see https://github.com/bendlas/data.xml/commit/22cbe21181175d302c884b4ec9162bd5ebf336d7

Comment by Alex Miller [ 28/Sep/16 10:08 PM ]

Thanks for filing this, it is something we've looked at a bit already. I simplified the description a bit and moved the use case and syntax to comments. I don't really understand the ns :alias example given in your syntax proposal but I think it's very unlikely we would go that far.

Comment by Herwig Hochleitner [ 29/Sep/16 3:55 AM ]

My syntax example could already be implemented, if alias had the proposed behavior and was available in an ns clause. In the linked commit, I implemented a scheme to encode xml namespaces in clojure namespaces, by using percent-encoding. I could easily provide that reader tag, if clojure and clojurescript provided the proposed extensions to alias and ns.

Comment by Alex Miller [ 29/Sep/16 8:53 AM ]

Yeah, I get it now (sleep!). I think the particular example is distracting to understand the enhancement request though.

Comment by Alex Miller [ 18/Oct/16 9:13 AM ]

moving this to vetted just so we don't lose track of it, but Rich has not actually ok'ed this for 1.9 yet

Comment by Herwig Hochleitner [ 06/Dec/16 10:50 PM ]

added patches for 3





[CLJ-2029] (nth nil <anything>) does not throw an exception Created: 28/Sep/16  Updated: 28/Sep/16  Resolved: 28/Sep/16

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

Type: Defect Priority: Minor
Reporter: Hans Hübner Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None


 Description   

If `nth` is being passed `nil` as `coll` argument, no exception is thrown for arbitrary indices. This does not match the expected behavior (all indices should throw an "Index out of bounds" exception) and also not documented.



 Comments   
Comment by Alex Miller [ 28/Sep/16 9:21 AM ]

It is by design that nth works on nil to return nil for any index and changing this would likely break many existing programs. An enhancement for the docstring would be considered.





[CLJ-2028] Docstring error in clojure.core/filter, remove, and take-while Created: 26/Sep/16  Updated: 27/Oct/16

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

Type: Enhancement Priority: Minor
Reporter: Alan Thompson Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: docstring
Environment:

All


Attachments: File clj-2028.diff    
Patch: Code
Approval: Prescreened

 Description   

The docstring for filter could be clearer about responding to logical true values:

​​Returns a lazy sequence of the items in coll for which
(pred item) returns true. pred must be free of side-effects.
Returns a transducer when no collection is provided.

should be corrected to read:

​Returns a lazy sequence of the items in coll for which
(pred item)​ ​​​returns logical true​. pred must be free of side-effects.​
Returns a transducer when  o collection is provided.

Similar changes could be applied to remove and take-while.

Patch: clj-2028.diff
Prescreened by: Alex Miller



 Comments   
Comment by Alex Miller [ 26/Sep/16 12:49 PM ]

"logical true" is the phrase used for this in other docstrings.

Comment by Jozef Wagner [ 27/Oct/16 7:13 AM ]

Added patch that updates docstrings for filter, filterv, remove and take-while





[CLJ-2027] bean printing regression from namespace map printing Created: 24/Sep/16  Updated: 28/Oct/16  Resolved: 28/Oct/16

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

Type: Defect Priority: Major
Reporter: Trey Sullivan Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: print, regression
Environment:

Clojure 1.9.0-alpha12


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

 Description   

The new namespace map printing is causing a failure in printing bean maps (which are proxies that don't support empty):

user=> (bean (java.util.Date.))
UnsupportedOperationException empty  clojure.core.proxy$clojure.lang.APersistentMap$ff19274a.empty (:-1)

user=> (pst *e)
UnsupportedOperationException empty
	clojure.core.proxy$clojure.lang.APersistentMap$ff19274a.empty (:-1)
	clojure.core/empty (core.clj:5151)
	clojure.core/lift-ns (core_print.clj:237)

Cause: The internal lift-ns function calls empty on the map too early (here it doesn't need to call it at all).

Proposed: Defer calling (empty m) until we know map has namespace keywords and namespace maps will be used for printing.

Patch: clj-2027.patch (note that into is not used in the change b/c it has not yet been defined at this point)






[CLJ-2026] Transient exceptions thrown in clojure.spec.test/check Created: 21/Sep/16  Updated: 14/Oct/16

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

Type: Defect Priority: Major
Reporter: Coda Hale Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: spec
Environment:

Java Virtual Machine 1.8
Clojure 1.9.0-alpha12
test.check 0.9.0


Attachments: Text File clj-2026.patch    
Patch: Code
Approval: Vetted

 Description   

So far I've seen two transient exceptions from running stest/check against some very simple functions:

First, while checking this spec:

(s/fdef str->long
        :args (s/cat :s (s/or :s string? :nil nil?))
        :ret (s/or :int int? :nil nil?)))

This exception was raised:

#error {
 :cause "Assert failed: Arg to one-of must be a collection of generators\n(every? generator? generators)"
 :via
 [{:type java.lang.AssertionError
   :message "Assert failed: Arg to one-of must be a collection of generators\n(every? generator? generators)"
   :at [clojure.test.check.generators$one_of invokeStatic "generators.cljc" 275]}]
 :trace
 [[clojure.test.check.generators$one_of invokeStatic "generators.cljc" 275]
  [clojure.test.check.generators$one_of invoke "generators.cljc" 264]
  [clojure.lang.AFn applyToHelper "AFn.java" 154]
  [clojure.lang.AFn applyTo "AFn.java" 144]
  [clojure.core$apply invokeStatic "core.clj" 657]
  [clojure.spec.gen$fn__13064$one_of__13067 doInvoke "gen.clj" 92]
  [clojure.lang.RestFn invoke "RestFn.java" 408]
  [clojure.spec$or_spec_impl$reify__13853 gen_STAR_ "spec.clj" 1060]
  [clojure.spec$gensub invokeStatic "spec.clj" 269]
  [clojure.spec$re_gen invokeStatic "spec.clj" 1565]
  [clojure.spec$re_gen$ggens__14185$gen__14186 invoke "spec.clj" 1554]
  [clojure.core$map$fn__6863 invoke "core.clj" 2739]
  [clojure.lang.LazySeq sval "LazySeq.java" 40]
  [clojure.lang.LazySeq seq "LazySeq.java" 49]
  [clojure.lang.RT seq "RT.java" 525]
  [clojure.core$seq__6397 invokeStatic "core.clj" 137]
  [clojure.core$every_QMARK_ invokeStatic "core.clj" 2652]
  [clojure.spec$re_gen invokeStatic "spec.clj" 1573]
  [clojure.spec$regex_spec_impl$reify__14229 gen_STAR_ "spec.clj" 1643]
  [clojure.spec$gensub invokeStatic "spec.clj" 269]
  [clojure.spec$gen invokeStatic "spec.clj" 275]
  [clojure.spec.test$quick_check$fn__13374 invoke "test.clj" 305]
  [clojure.spec.test$quick_check invokeStatic "test.clj" 305]
  [clojure.spec.test$check_1 invokeStatic "test.clj" 333]
  [clojure.spec.test$check$fn__13395 invoke "test.clj" 409]
  [clojure.core$pmap$fn__9360$fn__9361 invoke "core.clj" 6895]
  [clojure.core$binding_conveyor_fn$fn__6747 invoke "core.clj" 2020]
  [clojure.lang.AFn call "AFn.java" 18]
  [java.util.concurrent.FutureTask run "FutureTask.java" 266]
  [java.util.concurrent.ThreadPoolExecutor runWorker "ThreadPoolExecutor.java" 1142]
  [java.util.concurrent.ThreadPoolExecutor$Worker run "ThreadPoolExecutor.java" 617]
  [java.lang.Thread run "Thread.java" 745]]}

Second, while checking this spec:

(s/fdef percentage
        :args (s/cat :dividend nat-int? :divisor (s/and nat-int? pos?))
        :ret nat-int?)

This exception was thrown:

#error {
 :cause "Can't take value of a macro: #'clojure.test.check.random/bxoubsr"
 :via
 [{:type clojure.lang.Compiler$CompilerException
   :message "java.lang.RuntimeException: Can't take value of a macro: #'clojure.test.check.random/bxoubsr, compiling:(clojure/test/check/random.clj:135:25)"
   :at [clojure.lang.Compiler analyze "Compiler.java" 6719]}
  {:type java.lang.RuntimeException
   :message "Can't take value of a macro: #'clojure.test.check.random/bxoubsr"
   :at [clojure.lang.Util runtimeException "Util.java" 221]}]
 :trace
 [[clojure.lang.Util runtimeException "Util.java" 221]
  [clojure.lang.Compiler analyzeSymbol "Compiler.java" 7124]
  [clojure.lang.Compiler analyze "Compiler.java" 6679]
  [clojure.lang.Compiler analyze "Compiler.java" 6656]
  [clojure.lang.Compiler$InvokeExpr parse "Compiler.java" 3766]
  [clojure.lang.Compiler analyzeSeq "Compiler.java" 6920]
  [clojure.lang.Compiler analyze "Compiler.java" 6700]
  [clojure.lang.Compiler analyzeSeq "Compiler.java" 6906]
  [clojure.lang.Compiler analyze "Compiler.java" 6700]
  [clojure.lang.Compiler analyzeSeq "Compiler.java" 6906]
  [clojure.lang.Compiler analyze "Compiler.java" 6700]
  [clojure.lang.Compiler analyzeSeq "Compiler.java" 6906]
  [clojure.lang.Compiler analyze "Compiler.java" 6700]
  [clojure.lang.Compiler analyze "Compiler.java" 6656]
  [clojure.lang.Compiler$BodyExpr$Parser parse "Compiler.java" 6029]
  [clojure.lang.Compiler$NewInstanceMethod parse "Compiler.java" 8345]
  [clojure.lang.Compiler$NewInstanceExpr build "Compiler.java" 7852]
  [clojure.lang.Compiler$NewInstanceExpr$DeftypeParser parse "Compiler.java" 7728]
  [clojure.lang.Compiler analyzeSeq "Compiler.java" 6918]
  [clojure.lang.Compiler analyze "Compiler.java" 6700]
  [clojure.lang.Compiler analyze "Compiler.java" 6656]
  [clojure.lang.Compiler$BodyExpr$Parser parse "Compiler.java" 6029]
  [clojure.lang.Compiler$LetExpr$Parser parse "Compiler.java" 6347]
  [clojure.lang.Compiler analyzeSeq "Compiler.java" 6918]
  [clojure.lang.Compiler analyze "Compiler.java" 6700]
  [clojure.lang.Compiler analyze "Compiler.java" 6656]
  [clojure.lang.Compiler$BodyExpr$Parser parse "Compiler.java" 6029]
  [clojure.lang.Compiler$FnMethod parse "Compiler.java" 5406]
  [clojure.lang.Compiler$FnExpr parse "Compiler.java" 3972]
  [clojure.lang.Compiler analyzeSeq "Compiler.java" 6916]
  [clojure.lang.Compiler analyze "Compiler.java" 6700]
  [clojure.lang.Compiler eval "Compiler.java" 6974]
  [clojure.lang.Compiler load "Compiler.java" 7429]
  [clojure.lang.RT loadResourceScript "RT.java" 374]
  [clojure.lang.RT loadResourceScript "RT.java" 365]
  [clojure.lang.RT load "RT.java" 455]
  [clojure.lang.RT load "RT.java" 421]
  [clojure.core$load$fn__7821 invoke "core.clj" 6008]
  [clojure.core$load invokeStatic "core.clj" 6007]
  [clojure.core$load doInvoke "core.clj" 5991]
  [clojure.lang.RestFn invoke "RestFn.java" 408]
  [clojure.core$load_one invokeStatic "core.clj" 5812]
  [clojure.core$load_one invoke "core.clj" 5807]
  [clojure.core$load_lib$fn__7766 invoke "core.clj" 5852]
  [clojure.core$load_lib invokeStatic "core.clj" 5851]
  [clojure.core$load_lib doInvoke "core.clj" 5832]
  [clojure.lang.RestFn applyTo "RestFn.java" 142]
  [clojure.core$apply invokeStatic "core.clj" 659]
  [clojure.core$load_libs invokeStatic "core.clj" 5889]
  [clojure.core$load_libs doInvoke "core.clj" 5873]
  [clojure.lang.RestFn applyTo "RestFn.java" 137]
  [clojure.core$apply invokeStatic "core.clj" 659]
  [clojure.core$require invokeStatic "core.clj" 5911]
  [clojure.core$require doInvoke "core.clj" 5911]
  [clojure.lang.RestFn invoke "RestFn.java" 436]
  [clojure.test.check.generators$eval40270$loading__7707__auto____40271 invoke "generators.cljc" 10]
  [clojure.test.check.generators$eval40270 invokeStatic "generators.cljc" 10]
  [clojure.test.check.generators$eval40270 invoke "generators.cljc" 10]
  [clojure.lang.Compiler eval "Compiler.java" 6977]
  [clojure.lang.Compiler eval "Compiler.java" 6966]
  [clojure.lang.Compiler load "Compiler.java" 7429]
  [clojure.lang.RT loadResourceScript "RT.java" 374]
  [clojure.lang.RT loadResourceScript "RT.java" 365]
  [clojure.lang.RT load "RT.java" 455]
  [clojure.lang.RT load "RT.java" 421]
  [clojure.core$load$fn__7821 invoke "core.clj" 6008]
  [clojure.core$load invokeStatic "core.clj" 6007]
  [clojure.core$load doInvoke "core.clj" 5991]
  [clojure.lang.RestFn invoke "RestFn.java" 408]
  [clojure.core$load_one invokeStatic "core.clj" 5812]
  [clojure.core$load_one invoke "core.clj" 5807]
  [clojure.core$load_lib$fn__7766 invoke "core.clj" 5852]
  [clojure.core$load_lib invokeStatic "core.clj" 5851]
  [clojure.core$load_lib doInvoke "core.clj" 5832]
  [clojure.lang.RestFn applyTo "RestFn.java" 142]
  [clojure.core$apply invokeStatic "core.clj" 659]
  [clojure.core$load_libs invokeStatic "core.clj" 5889]
  [clojure.core$load_libs doInvoke "core.clj" 5873]
  [clojure.lang.RestFn applyTo "RestFn.java" 137]
  [clojure.core$apply invokeStatic "core.clj" 659]
  [clojure.core$require invokeStatic "core.clj" 5911]
  [clojure.spec.gen$dynaload invokeStatic "gen.clj" 18]
  [clojure.spec.gen$fn__13223$fn__13224 invoke "gen.clj" 115]
  [clojure.lang.Delay deref "Delay.java" 37]
  [clojure.core$deref invokeStatic "core.clj" 2310]
  [clojure.spec.gen$fn__13223$simple_type_printable__13226 doInvoke "gen.clj" 115]
  [clojure.lang.RestFn invoke "RestFn.java" 397]
  [clojure.spec.gen$fn__13280 invokeStatic "gen.clj" 131]
  [clojure.spec.gen$fn__13280 invoke "gen.clj" 130]
  [clojure.lang.Delay deref "Delay.java" 37]
  [clojure.core$deref invokeStatic "core.clj" 2310]
  [clojure.spec.gen$gen_for_pred invokeStatic "gen.clj" 191]
  [clojure.spec$spec_impl$reify__13794 gen_STAR_ "spec.clj" 877]
  [clojure.spec$gensub invokeStatic "spec.clj" 269]
  [clojure.spec$re_gen invokeStatic "spec.clj" 1565]
  [clojure.spec$re_gen$ggens__14185$gen__14186 invoke "spec.clj" 1554]
  [clojure.core$map$fn__6863 invoke "core.clj" 2739]
  [clojure.lang.LazySeq sval "LazySeq.java" 40]
  [clojure.lang.LazySeq seq "LazySeq.java" 49]
  [clojure.lang.RT seq "RT.java" 525]
  [clojure.core$seq__6397 invokeStatic "core.clj" 137]
  [clojure.core$every_QMARK_ invokeStatic "core.clj" 2652]
  [clojure.spec$re_gen invokeStatic "spec.clj" 1573]
  [clojure.spec$regex_spec_impl$reify__14229 gen_STAR_ "spec.clj" 1643]
  [clojure.spec$gensub invokeStatic "spec.clj" 269]
  [clojure.spec$gen invokeStatic "spec.clj" 275]
  [clojure.spec.test$quick_check$fn__13374 invoke "test.clj" 305]
  [clojure.spec.test$quick_check invokeStatic "test.clj" 305]
  [clojure.spec.test$check_1 invokeStatic "test.clj" 333]
  [clojure.spec.test$check$fn__13395 invoke "test.clj" 409]
  [clojure.core$pmap$fn__9360$fn__9361 invoke "core.clj" 6895]
  [clojure.core$binding_conveyor_fn$fn__6747 invoke "core.clj" 2020]
  [clojure.lang.AFn call "AFn.java" 18]
  [java.util.concurrent.FutureTask run "FutureTask.java" 266]
  [java.util.concurrent.ThreadPoolExecutor runWorker "ThreadPoolExecutor.java" 1142]
  [java.util.concurrent.ThreadPoolExecutor$Worker run "ThreadPoolExecutor.java" 617]
  [java.lang.Thread run "Thread.java" 745]]}

I was unable to reproduce either exception during consequent runs.

Cause: See further investigation in the comments - this appears to be caused by the pmap in check triggering concurrent requires of the test.check.generators namespace.

Approach: Add locking to prevent concurrent loads in dynaload.

Patch: clj-2026.patch



 Comments   
Comment by Alex Miller [ 21/Sep/16 1:27 PM ]

On the first one, you should use this instead:

(s/fdef str->long
        :args (s/cat :s (s/nilable string?))
        :ret (s/nilable int?))

s/nilable is performance optimized, works better as a generator, and is shorter.

I'll look into the failures though.

Comment by Gary Fredericks [ 21/Sep/16 9:18 PM ]

The second error seemed crazy spooky, and the only thing I could imagine was that it was a problem that would manifest itself any time compiling clojure.test.check.random, but only occasionally.

So I decided to just continually compile that namespace in a loop and see what would happen. After ~30 minutes I got this, which is not obviously related as far as I can tell:

Exception in thread "main" java.lang.ExceptionInInitializerError, compiling:(clojure/test/check/random.clj:16:1)
        at clojure.lang.Compiler.load(Compiler.java:7441)
        at clojure.lang.RT.loadResourceScript(RT.java:374)
        at clojure.lang.RT.loadResourceScript(RT.java:365)
        at clojure.lang.RT.load(RT.java:455)
        at clojure.lang.RT.load(RT.java:421)
        at clojure.core$load$fn__7821.invoke(core.clj:6008)
        at clojure.core$load.invokeStatic(core.clj:6007)
        at clojure.core$load.doInvoke(core.clj:5991)
        at clojure.lang.RestFn.invoke(RestFn.java:408)
        at user$eval5.invokeStatic(NO_SOURCE_FILE:1)
        at user$eval5.invoke(NO_SOURCE_FILE:1)
        at clojure.lang.Compiler.eval(Compiler.java:6977)
        at clojure.lang.Compiler.eval(Compiler.java:6940)
        at clojure.core$eval.invokeStatic(core.clj:3187)
        at clojure.main$eval_opt.invokeStatic(main.clj:290)
        at clojure.main$eval_opt.invoke(main.clj:284)
        at clojure.main$initialize.invokeStatic(main.clj:310)
        at clojure.main$null_opt.invokeStatic(main.clj:344)
        at clojure.main$null_opt.invoke(main.clj:341)
        at clojure.main$main.invokeStatic(main.clj:423)
        at clojure.main$main.doInvoke(main.clj:386)
        at clojure.lang.RestFn.applyTo(RestFn.java:137)
        at clojure.lang.Var.applyTo(Var.java:700)
        at clojure.main.main(main.java:37)
Caused by: java.lang.ExceptionInInitializerError
        at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
        at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:57)
        at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
        at java.lang.reflect.Constructor.newInstance(Constructor.java:526)
        at java.lang.Class.newInstance(Class.java:383)
        at clojure.lang.Compiler$ObjExpr.eval(Compiler.java:4939)
        at clojure.lang.Compiler.eval(Compiler.java:6976)
        at clojure.lang.Compiler.eval(Compiler.java:6966)
        at clojure.lang.Compiler.load(Compiler.java:7429)
        ... 23 more
Caused by: java.lang.ClassNotFoundException: clojure.test.check.random.IRandom
        at java.net.URLClassLoader$1.run(URLClassLoader.java:366)
        at java.net.URLClassLoader$1.run(URLClassLoader.java:355)
        at java.security.AccessController.doPrivileged(Native Method)
        at java.net.URLClassLoader.findClass(URLClassLoader.java:354)
        at clojure.lang.DynamicClassLoader.findClass(DynamicClassLoader.java:69)
        at java.lang.ClassLoader.loadClass(ClassLoader.java:425)
        at clojure.lang.DynamicClassLoader.loadClass(DynamicClassLoader.java:77)
        at java.lang.ClassLoader.loadClass(ClassLoader.java:358)
        at java.lang.Class.forName0(Native Method)
        at java.lang.Class.forName(Class.java:278)
        at clojure.lang.RT.classForName(RT.java:2183)
        at clojure.lang.RT.classForName(RT.java:2192)
        at clojure.test.check.random$eval1059936.<clinit>(random.clj:16)
        ... 32 more
Comment by Kevin Downey [ 22/Sep/16 12:49 AM ]

The Random thing seems like it might be the issue that was fixed here (https://github.com/clojure/clojure/commit/2ac93197e356af3c826ca895b5a538ad08c5715) for other constructs, creating a class and having it get gc'ed before it can be used.

Comment by Colin Jones [ 22/Sep/16 7:56 AM ]

Here's a fairly small repro case that I got to throw the same error as that second one (once), with some comments in the test file noting various ways in which the failures seem to go away: https://github.com/trptcolin/spec-race-repro

I've seen all of the following errors on a `lein test` of the linked project:

  • Wrong number of args (0) passed to: dynaloadable/asdf
  • Var spec-race.dynaloadable/asdf-consumer is not on the classpath
  • Can't take value of a macro: #'spec-race.dynaloadable/asdf-consumer

This last one was by far the rarest - only seen once, over many minutes of running. But both the first and last are errors related to confusing whether `asdf` is a function or a macro.

I'm reasonably confident it comes down to dynaload / require'ing the same file concurrently. Locking the dynaload require, eager loading all to-be-dynaloaded nses before adding concurrency, and just avoiding concurrency all appear work without issues. In the interest of keeping things flexible & letting consumers do what they want, I'd personally lean towards the locking approach (maybe striping per-file), but hopefully this repro case at least helps us study the issue more.

Comment by Alex Miller [ 22/Sep/16 8:39 AM ]

Just a note of thanks for those that have looked at this so far - thanks! Certainly concurrent requires during dynaload sounds like a reasonable candidate. The only source of concurrency that I'm aware of is the pmap inside `check` (presuming there is not something concurrent in the original testing environment).

Comment by Stuart Halloway [ 14/Oct/16 12:41 PM ]

If we are going to add locking, why not add it to ns and require in core? Wouldn't all users of require want these better semantics, or is that too expensive for general use?

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

Comment by Stuart Halloway [ 14/Oct/16 12:41 PM ]

If we are going to add locking, why not add it to ns and require in core? Wouldn't all users of require want these better semantics, or is that too expensive for general use?

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





[CLJ-2025] When a generator fails to gen, state which spec/pred failed Created: 21/Sep/16  Updated: 22/Sep/16

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

Type: Enhancement Priority: Minor
Reporter: David Collie Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: spec

Approval: Triaged

 Description   

Given a generator using such-that that fails to find a value, the error does not give enough information to determine which spec or predicate was at fault:

(require '[clojure.spec :as s])
(s/exercise (s/and string? #{"hi"}))
ExceptionInfo Couldn't satisfy such-that predicate after 100 tries.  clojure.core/ex-info (core.clj:4725)

Another special case of this is when providing a custom generator that produces a valid that doesn't satisfy the spec (Clojure adds this filter internally):

(require '[clojure.spec :as s])
(s/exercise (s/with-gen int? #(s/gen #{:a})))

Proposal: Indicate in the error which spec failed to generate and possibly the path in the overall spec if feasible.

(Note: original description moved to comment)



 Comments   
Comment by Alex Miller [ 22/Sep/16 3:54 PM ]

[Original description from ticket:]

I created a generator that did not conform to the spec (doh!). The generator contained the such-that predicate. When I tried creating a sample from the generator I got this error:

ExceptionInfo Couldn't satisfy such-that predicate after 100 tries.  clojure.core/ex-info (core.clj:4725)

I assumed that it referred to my custom generator but that was a red herring because in fact spec must be using such-that to ensure that the generated value conforms to the spec, and it was this such-that that generated the failure, not the one in my custom generator.

Code (with the problem corrected but showing the such-that in my generator:

(defn mod11-checkdigit
  "Calculate the checkdigit see http://freagra.com/imthealth/mitNNC.html"
  [n]
  (let [x (->> (map #(Integer/parseInt (str %)) (take 9 n))
               (map * (range 10 1 -1))
               (reduce +))
        y (mod x 11)
        c (- 11 y)]
    (cond (== 10 c) nil
          (== 11 c) 0
          :else c)))

(def nhs-number-gen
  "Generates a valid NHS number"
  (gen/fmap #(str (+ (* 10 %) (mod11-checkdigit (str %))))
            (gen/such-that #(mod11-checkdigit (str %))
                           (gen/choose 100000000 999999999))))

(defn nhs-number?
  "Returns true if passed a valid nhs number else returns false"
  [n]
  (and (string? n) (= 10 (count n)) (= (str (mod11-checkdigit n)) (str (last n)))))

(s/def ::nhs-number (s/with-gen nhs-number?
                                (fn [] nhs-number-gen)))

It would be nicer if the error thrown due to the generated value being non-conformant with the spec stated this.

Comment by Alex Miller [ 22/Sep/16 4:07 PM ]

I'm not sure that this is possible right now based on what we give to and get back from test.check.





[CLJ-2024] Check should specize function specs before checking Created: 19/Sep/16  Updated: 28/Oct/16  Resolved: 28/Oct/16

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

Type: Defect Priority: Major
Reporter: James Reeves Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: spec

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

 Description   

This code works fine in 1.9.0-alpha12:

(defn f [x] (+ x 1))
(s/def f (s/fspec :args (s/cat :x number?) :ret number?))
(stest/check `f)

But if we factor the fspec out into its own keyword:

(defn f [x] (+ x 1))
(s/def ::inc (s/fspec :args (s/cat :x number?) :ret number?))
(s/def f ::inc)
(stest/check `f)

The check fails with the exception:

({:failure #error {
 :cause "No :args spec"
 :data #:clojure.spec{:failure :no-args-spec}
 :via
 [{:type clojure.lang.ExceptionInfo
   :message "No :args spec"
   :data #:clojure.spec{:failure :no-args-spec}
   :at [...]}]
 :trace
 [...]}, :sym user/f, :spec :user/inc})

The check function doesn't seem to be resolving ::inc, when presumably it should.

Patch: clj-2024-2.patch



 Comments   
Comment by Rich Hickey [ 28/Oct/16 7:44 AM ]

this should be fixed in fspec, not its use by test

Comment by Alex Miller [ 28/Oct/16 8:23 AM ]

fspec is not the problem as far as I can tell - it is already making specs of its args.

The problem is that f is registered as an alias of ::inc. I don't think you want to resolve that at registration time (as ::inc might not exist yet).

The problem as far as I understand it is that at the time of use (by check), f is not resolved to it's final spec and that's what the patch does.

Comment by Alex Miller [ 28/Oct/16 8:43 AM ]

Added new patch that uses `spec` instead of private `specize` function.





[CLJ-2023] clojure.spec edge case failure Created: 14/Sep/16  Updated: 14/Sep/16  Resolved: 14/Sep/16

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

Type: Defect Priority: Major
Reporter: Brian Noguchi Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None
Environment:

Using clojure 1.9.0-alpha12



 Description   

I stumbled onto an odd edge case. The following is a minimal example you can run in the REPL:

```
(require '[clojure.spec :as s])
(s/def :tv/star (s/or :starring/ernie #{:char/ernie}
:starring/big-bird #{:char/big-bird}))

; The following behaves as expected.
(s/explain
(s/and (s/keys :req [:tv/star]
:opt [:tv/co-star])
(s/or
:ernie-and-bert (s/and #(do (println "!!!!!") (println %) (= (:tv/star %) :char/ernie))
#(= (:tv/co-star %) :char/bert))
:just-the-bird #(not (contains? % :tv/co-star))))
{:tv/star :char/big-bird})
; This outputs the following:
; !!!!!
; #:tv{:star [:starring/big-bird :char/big-bird]}
; Success!
; => nil

; The following is unexpected
(s/explain
(s/and (s/keys)
(s/or
:ernie-and-bert (s/and #(do (println "!!!!!") (println %) (= (:tv/star %) :char/ernie))
#(= (:tv/co-star %) :char/bert))
:just-the-bird #(not (contains? % :tv/co-star))))
{:tv/star :char/ernie :tv/co-star :char/bert})
; This outputs the following:
; !!!!!
; #:tv{:star [:starring/ernie :char/ernie], :co-star :char/bert}
; !!!!!
; #:tv{:star [:starring/ernie :char/ernie], :co-star :char/bert}
; !!!!!
; #:tv{:star [:starring/ernie :char/ernie], :co-star :char/bert}
; !!!!!
; #:tv{:star [:starring/ernie :char/ernie], :co-star :char/bert}
; val: #:tv{:star [:starring/ernie :char/ernie], :co-star :char/bert} fails at: [:ernie-and-bert] predicate: (do (println "!!!!!") (println %) (= (:tv/star %) :char/ernie))
; val: #:tv{:star [:starring/ernie :char/ernie], :co-star :char/bert} fails at: [:just-the-bird] predicate: (not (contains? % :tv/co-star))
; => nil
```

I would have expected the second `(s/explain ...)` to succeed, given my understanding of `clojure.spec` semantics. However, it seems as though the argument inside `#(do (println "!!!!!") (println %) (= (:tv/star %) :char/ernie))` does not resolve `%` to the original map `{:tv/star :char/ernie :tv/co-star :char/bert}` but rather the transformed map `#:tv{:star [:starring/ernie :char/ernie], :co-star :char/bert}` that seems to mix in the output of applying conform against the spec definition of `:tv/star`.

Miscellaneous edge case observations:

  • If I replace the sibling spec `(s/keys)` with a simple predicate like `some?`, then it succeeds.

```
(s/explain
(s/and some?
(s/or
:ernie-and-bert (s/and #(do (println "!!!!!") (println %) (= (:tv/star %) :char/ernie))
#(= (:tv/co-star %) :char/bert))
:just-the-bird #(not (contains? % :tv/co-star))))
{:tv/star :char/ernie :tv/co-star :char/bert})
```

  • If I elide the first, solitary registered clojure.spec definition `(s/def :tv/star ...)`, then it succeeds.

I haven't investigated a solution yet.



 Comments   
Comment by Brian Noguchi [ 14/Sep/16 4:20 AM ]

It looks like the observed behavior is the expected correct behavior. I just noticed that successive conformed values are supposed to propagate through the rest of the predicates.

https://github.com/clojure/clojure/blob/d920ada9fab7e9b8342d28d8295a600a814c1d8a/src/clj/clojure/spec.clj#L439-L440

Comment by Brian Noguchi [ 14/Sep/16 4:21 AM ]

This issue can be ignored and closed. Sorry!





[CLJ-2022] Add fixed arities for mapcat Created: 13/Sep/16  Updated: 13/Sep/16

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

Type: Enhancement Priority: Minor
Reporter: Robert C Faber Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: performance

Attachments: Text File CLJ-2022.patch    
Patch: Code

 Description   

Add fixed arities for mapcat, in the pattern of map.

Same change as CLJS-1776.



 Comments   
Comment by Alex Miller [ 13/Sep/16 10:29 AM ]

Presumably this is to improve performance. Please include a benchmark showing the difference.

Comment by Ghadi Shayban [ 13/Sep/16 12:47 PM ]

Please consider interactions with apply and laziness CLJ-1218





[CLJ-2021] case where spec/conform -> spec/unform -> spec/conform gives invalid result Created: 12/Sep/16  Updated: 12/Sep/16

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

Type: Defect Priority: Major
Reporter: Jeroen van Dijk Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: spec
Environment:

clojure 1.9, mac osx, java 1.8



 Description   

The example belows shows a case where a conform-ed form, does not conform any after an unform. It would be my expectation that you can repeat conform -> unform -> conform endlessly and get the same result.

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

(s/def ::defn-macro (s/cat :type #

Unknown macro: {'defn}
:definition :clojure.core.specs/defn-args))

(let [form '(defn foo "bar" ([a & b] a a c) ([a b] a))]

(-> form
(->> (s/conform ::defn-macro))) ;;=> {:type defn, :definition {:name foo, :docstring "bar", :bs [:arity-n {:bodies [{:args {:args [[:sym a]], :varargs {:amp &, :form [:sym b]}}, :body [a a c]} {:args {:args [[:sym a] [:sym b]]}, :body [a]}]}]}}

;; Unforming returns the function definition, but with the args in a list instead of a vector:
(->> form
(s/conform ::defn-macro)
(s/unform ::defn-macro)) ;;=> (defn foo "bar" ((a (& b)) a a c) ((a b) a)))

;; Conforming after unforming doesn't work anymore
(->> form
(s/conform ::defn-macro)
(s/unform ::defn-macro)
(s/conform ::defn-macro)) ;;=> :clojure.spec/invalid

)



 Comments   
Comment by Jeroen van Dijk [ 12/Sep/16 8:22 AM ]

This gist shows the above code with better formatting https://gist.github.com/jeroenvandijk/28c6cdd867dbc9889565dca92673a531





[CLJ-2020] New prohibited field names (__hash __hasheq) break existing software Created: 07/Sep/16  Updated: 08/Sep/16  Resolved: 08/Sep/16

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

Type: Defect Priority: Major
Reporter: lvh Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: defrecord


 Description   

The most recent Clojure alpha (1.9.0-alpha12) contained a patch closing CLJ-1224. This had the unfortunate side effect of breaking some existing software, notably datascript. I've filed an issue upstream as well: https://github.com/tonsky/datascript/issues/176

It's not clear to me what the best resolution is; IIUC the behavior datascript was trying to accomplish is what records now do automagically, although I might have misunderstood. Ideally, datascript wouldn't have the serious performance regression on >=1.8.0, but it definitely should compile on 1.9.0, regardless of how that's resolved.



 Comments   
Comment by Alex Miller [ 07/Sep/16 9:02 PM ]

My initial reaction is that datascript should not be using fields starting with __ as that is at least implied to be "internal stuff" in the defrecord docstring. But, I reserve the right to think about it more.

Comment by Alex Miller [ 08/Sep/16 9:51 AM ]

After thinking about it more, I will double down to say that extending a defrecord to IHashEq is also bad form. Records are intended to have the hash semantics of maps and to be controlled by the language, not by a record extender. I did quite a bit of searching and have found no other project that does this.

Implementing IHashEq for deftypes is a normal and perfectly acceptable thing to do as deftypes are considered to be opaque from Clojure's point of view - you give it the semantics.

Comment by Alex Miller [ 08/Sep/16 9:58 AM ]

Rich concurs so I am declining. The lib code should be changed.

Comment by lvh [ 08/Sep/16 9:59 AM ]

IIUC the options for datascript are either moving to deftype, or removing the IHashEq impl (and living with the perf penalty). Is there an option that lets them keep the perf, conditionally implementing only on 1.9.0? (That may not be desirable at all.)

Comment by Alex Miller [ 08/Sep/16 10:41 AM ]

It's conceivable to conditionally load different versions of a namespace that defines the records based on Clojure version. I'm not sure whether that's worth doing or not (and how it plays with AOT). I don't know whether it even affects perf or by how much - seems like that's the first question to answer. If it doesn't matter, then just remove it.





[CLJ-2019] Loosen constraint between key name and spec name in clojure.spec/keys Created: 04/Sep/16  Updated: 19/Sep/16

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

Type: Enhancement Priority: Major
Reporter: Peter Schuck Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: spec


 Description   

According to the Clojure Spec Rationale

Most systems for specifying structures conflate the specification of the key set (e.g. of keys in a map, fields in an object) with the specification of the values designated by those keys. I.e. in such approaches the schema for a map might say :a-key's type is x-type and :b-key's type is y-type. This is a major source of rigidity and redundancy.

Currently clojure.spec/keys does exactly the complecting the rationale says is a major source of rigidity and redundancy. clojure.spec/keys requires that any keys in it's key set have the name the have in the spec registry. For example:

(ns my.ns
  (:require
    [clojure.spec :as spec]))

(spec/def ::x-type integer?)
(spec/def ::y-type bool?)

;;The only map that can be checked for is {::x-type <x-type> ::y-type <y-type>}
(spec/def ::my-map (spec/keys :req [::x-type ::y-type]))

;;To validate a map like {::a-key <x-type> ::b-key <y-type>} You need to do this
(spec/def ::a-key ::x-type)
(spec/def ::b-key ::y-type)
(spec/def ::my-map (spec/keys :req [::a-key ::b-key]))

What clojure.spec/keys should allow you to do is this

(ns my.ns
  (:require
    [clojure.spec :as spec]))

(spec/def ::x-type integer?)
(spec/def ::y-type bool?)

;;The key set is now independent of the spec's names. You can validate
;;a map like {::a-key <x-type> ::b-key <y-type>}
(spec/def ::my-map (spec/keys :req {::a-key ::x-type ::b-key ::y-type}))

The exact implementation may vary from what I show here but the end result should be allowing users to check for x-type under ::a-key with out having to do the redundant step of (clojure.spec/def ::a-key (clojure.spec/spec <x-type>).



 Comments   
Comment by Laszlo Török [ 19/Sep/16 4:34 PM ]

Spec advocates to use namespaced keys to convey contextual semantics of the value.

Relevant quote from the Spec guide:

"These maps represent various sets, subsets, intersections and unions of the same keys, and in general ought to have the same semantic for the same key wherever it is used."

There may be different pieces of information that end up having the same representation (e.g. both are of type integer).

The ::x-type vs ::a-key nomenclature above is misleading. One should rather look a keyword-value pair in a map as an attribute-value pair, where you can specify the valid values of that attribute using a spec.





[CLJ-2018] clojure.spec.gen lazy-loaded fns should contain wrapped thing's docstring Created: 03/Sep/16  Updated: 06/Sep/16  Resolved: 06/Sep/16

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

Type: Enhancement Priority: Major
Reporter: lvh Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: docstring, spec


 Description   

Right now, using clojure.spec.gen's lazily loaded fns is harder than using the test.check versions because the former doesn't have useful docstrings and the latter does.

clojure.spec.gen lazy-loaded fns should contain docstring of the thing they refer to, or at least what it was at compile time, or at least a universally-understood reference to that thing that IDEs can follow. I don't think the latter exists, so perhaps the docstring at compile time should be embedded into the lazy-loaded fn if possible.

(As a general note, and I don't know if my experience generalizes, but I find myself grabbing for stuff in e.g. test.chuck so often that the "no runtime dependency" thing doesn't really work out anyway.)



 Comments   
Comment by Alex Miller [ 03/Sep/16 6:44 PM ]

I don't think they can contain those docstrings without actually loading the remote var to get its meta, which defies the whole point of lazy loading. So while I sympathize with the request, I'm not sure that I understand how it is possible to achieve?

Comment by lvh [ 04/Sep/16 10:28 AM ]

I was hoping a macro would be able to load the var, but only at compile time (e.g. when building the jar), not affecting a runtime consumer of the same ns; the same way that some of my ClojureScript code is built with macros that read resource files at compile time. (Perhaps that is not technologically possible right now in Clojure.)

Comment by Alex Miller [ 06/Sep/16 11:20 AM ]

I'm going to decline this as I'm not sure how it would be done while still satisfying the other goals of this code. But if someone finds a reasonable way to do it, would reconsider.





[CLJ-2017] with-gen should specify if the generator should return conformed or unformed data Created: 03/Sep/16  Updated: 04/Sep/16

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

Type: Enhancement Priority: Major
Reporter: lvh Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring, spec


 Description   

I think the answer is "unformed", but this isn't very clear from the docstring.



 Comments   
Comment by Alex Miller [ 03/Sep/16 6:46 PM ]

The answer is definitely unconformed. Conforming only happens when you call conform. This doesn't seem confusing to me, but maybe it should be clearer. I suspect it would be better to clarify this in a reference documentation page though.

Comment by lvh [ 04/Sep/16 10:29 AM ]

I agree that a reference documentation change would be most helpful.

I rely heavily on my environment showing me docstrings, so a small point (maybe just unconformed/unformed/whatever in parens) in the docstring would still be helpful.





[CLJ-2016] function contains? return value error Created: 30/Aug/16  Updated: 30/Aug/16  Resolved: 30/Aug/16

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

Type: Defect Priority: Major
Reporter: wenjixiao Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None
Environment:

ubuntu 14.04



 Description   

hello.core=> (contains? [1 2 3] 1)
true
hello.core=> (contains? [1 2 3] 2)
true
hello.core=> (contains? [1 2 3] 3)
false

i am not sure, it's a bug or not, because it's so simple.
But, [1 2 3] should contains 3 right?!



 Comments   
Comment by Alex Miller [ 30/Aug/16 9:18 AM ]

This is a common question and this is the expected behavior.

contains? checks for containment of a key in an indexed collection. In a map, contains? works on keys. In a set, it works on the (hashed) elements. In a vector, it vector on the vector indexes (not the values).

So asking (contains? [1 2 3] 1) is asking whether there is an element at index 1 in [1 2 3], which there is (here it's 2). When you ask (contains? [1 2 3] 3) you are asking if [1 2 3] has index 3 (0-based), which it does not.

Hope that helps.

Comment by Alex Miller [ 30/Aug/16 9:27 AM ]

Also, more info here http://insideclojure.org/2015/01/27/clojure-tips-contains/





[CLJ-2015] with-instrument Created: 29/Aug/16  Updated: 30/Aug/16

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

Type: Enhancement Priority: Major
Reporter: lvh Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: spec

Approval: Triaged

 Description   

Right now, instrument and unstrument are great for unconditional instrumentation for tests and for development. I also want to run instrument for just a particular piece of code. For example, I want a test with some stubs or some overrides. Right now I need to instrument and unstrument; I'd prefer to have a with-instrument macro that does the obvious try/finally block for me.



 Comments   
Comment by Alex Miller [ 30/Aug/16 2:30 PM ]

So (like most things), obvious things aren't.

There are several ways to call instrument:

  • (instrument)
  • (instrument sym)
  • (instrument [syms])
  • (instrument sym opts)
  • (instrument [syms] opts)

The number there is variable. Similarly, a "body" is typically also variadic in other with-style macros. Parsing those two variadic things is ambiguous.

You mentioned the opts map, so I'm assuming you'd want that as an option. So you could narrow the args to: [sym-or-syms opts & body]. Not sure whether you've then introduced things you don't need in common cases and ruined the usefulness of the macro.

(with-instrument `my-fun {my-opts ...} (test-something))

would expand to

(do
  (instrument user/my-fun)
  (try
    (test-something)
    (finally
      (unstrument user/my-fun))))

There are maybe interesting things to think about with how much you take into account what's already instrumented. Do you unstrument what you instrument, or do you try to return the instrumentation to what it was before (where some stuff may already have been instrumented)?

Comment by Daniel Solano Gómez [ 30/Aug/16 3:24 PM ]

So, here's the implementation I have been using, which isn't necessarily the one to use, but I think it helps with some of the ambiguity with respect to arguments:

(defmacro with-instrumentation
  [args & body]
  `(let [[arg1# arg2#] ~args
         [sym-or-syms# opts#] (cond
                                (nil? arg1#) [(stest/instrumentable-syms) arg2#]
                                (map? arg1#) [(stest/instrumentable-syms) arg1#]
                                :default     [arg1# arg2#])]
     (try
       (stest/instrument sym-or-syms# opts#)
       ~@body
       (finally
         (stest/unstrument sym-or-syms#)))))

It's not perfect, but it has served me well enough.

The question of what happens at the end is a very good one. Ideally, with-instrumentation would have stack-like semantics where instrumentation would return to its previous state. Is that something that can be done with spec?





[CLJ-2014] (keyword "@type") can be printed, but not read Created: 26/Aug/16  Updated: 26/Aug/16  Resolved: 26/Aug/16

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

Type: Defect Priority: Minor
Reporter: David Smith Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None

Approval: Incomplete
Waiting On: Rich Hickey

 Description   

user=> (keyword "")
:
user=> (prn-str *1)
":\n"
user=> (read-string *1)
java.lang.RuntimeException: java.lang.Exception: Invalid token: : (NO_SOURCE_FILE:0)

This obviously isn't a huge defect, but I'd argue that anything that can be printed should be readable.



 Comments   
Comment by David Smith [ 26/Aug/16 5:02 AM ]

This is a clone of http://dev.clojure.org/jira/browse/CLJ-732 which it appears was closed with no explanation. I have recently come up against this when deserializing json. IMO it doesn't make sense for the keyword function to be able to produce non-valid keywords. What is the reason for rejecting this?

Comment by Alex Miller [ 26/Aug/16 7:47 AM ]

This is a feature used by a lot of Clojure programs. See:

http://clojure.org/guides/faq#unreadable_keywords

Comment by David Smith [ 26/Aug/16 7:49 AM ]

Thank you for the explanation, this can therefor be closed.





[CLJ-2013] spec doesn't explain failing path of a s/cat with purely optional branches Created: 24/Aug/16  Updated: 12/Oct/16

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

Type: Enhancement Priority: Major
Reporter: Leon Grapenthin Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: errormsgs, spec
Environment:

alpha11


Approval: Triaged

 Description   

In an s/cat with two optional regex branches, e. g. via s/? or s/*, spec doesn't explain their individual problems, but the whole spec as failed.

(s/explain (s/cat :begin (s/? (s/cat :num number?))
                  :end (s/* #{:foo}))
           [:bar])

In: [0] val: (:bar) fails predicate: (cat :begin (? (cat :num number?)) :end (* #{:foo})),  Extra input

Spec does not explain the full optional paths that failed, but instead explains that the s/cat spec failed as a whole.

If one forces spec down into one branch, it explains the error at the deepest possible path and explains the failing predicate

(s/explain (s/cat :begin (s/? (s/cat :num number?))
               ;; :end (s/* #{:foo})
                  )
           [:bar])
In: [0] val: :bar fails at: [:begin :num] predicate: number?

An interesting case is if one makes the second branch non-optional

(s/explain (s/cat :begin (s/? (s/cat :num number?))
                  :end #{:foo})
           [:bar])
In: [0] val: :bar fails at: [:end] predicate: #{:foo}

It does not explain why the first branch has failed as a potential option, but only the second. This makes sense from the perspective that it successfully parses the :begin branch as legally non-existent and then explains a detailed failure on the second one. However it omits valuable information in real world use cases as shown in https://groups.google.com/d/msg/clojure/mIlKaOiujlo/tF71zZ2BCwAJ .

Desired behavior would be at least that if all branches are optional in a cat and all fail they are all reported.

At most that if a cat fails but an optional branch was parsed as non-existent it is retried without being allowed to be parsed as non-existent.



 Comments   
Comment by Nuttanart Pornprasitsakul [ 12/Oct/16 8:43 AM ]

I'll put a somewhat different issue here because I think it has the same root cause.

It should specifically say :ret of fspec is missing but it says failing at :args.

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

(defn x [f] (f 1))

(s/fdef x
  :args (s/cat :f (s/fspec :args (s/cat :i int?))))

(st/instrument `x)

(x (fn [a] a))
Exception in thread "main" clojure.lang.ExceptionInfo: Call to #'user/x did not conform to spec:
In: [0] val: (#object[user$eval20$fn__21 0x3e521715 "user$eval20$fn__21@3e521715"]) fails at: [:args] predicate: (cat :f (fspec :args (cat :i int?))),  Extra input
:clojure.spec/args  (#object[user$eval20$fn__21 0x3e521715 "user$eval20$fn__21@3e521715"])
:clojure.spec/failure  :instrument
:clojure.spec.test/caller  {:file "debug.clj", :line 16, :var-scope user/eval20}
 {:clojure.spec/problems [{:path [:args], :reason "Extra input", :pred (cat :f (fspec :args (cat :i int?))), :val (#object[user$eval20$fn__21 0x3e521715 "user$eval20$fn__21@3e521715"]), :via [], :in [0]}], :clojure.spec/args (#object[user$eval20$fn__21 0x3e521715 "user$eval20$fn__21@3e521715"]), :clojure.spec/failure :instrument,
...




[CLJ-2012] ns spec breaks gen-class forms that use strings as class names Created: 24/Aug/16  Updated: 07/Sep/16  Resolved: 07/Sep/16

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

Type: Defect Priority: Major
Reporter: Nicola Mometto Assignee: Alex Miller
Resolution: Completed Votes: 0
Labels: regression, spec

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

 Description   

The following valid `ns` gen-class form is reported as invalid by spec:

(ns foo
  (:gen-class
     :name ^{org.apache.logging.log4j.core.config.plugins.Plugin
             {:name "LogstashConverter" :category "Converter"}
             org.apache.logging.log4j.core.pattern.ConverterKeys ["logstashJson"]} social.common.logging.LogstashPatternConverter
     :constructors {["[Ljava.lang.String;"] [String String]}
     :factory newInstance
     :init init
     :state state
     :extends org.apache.logging.log4j.core.pattern.LogEventPatternConverter))

This is because the ns spec assumes that all class names can be represented by simple-symbols, while in reality some can only be represented by strings.

Approach: Pull out spec for class identifiers which can be either a simple-symbol (class name) or a string and use that in signature (which is used for both gen-class constructors and methods.

Patch: clj-2012-2.patch



 Comments   
Comment by Stuart Halloway [ 26/Aug/16 7:23 AM ]

Nicola: Good catch, thanks for the report!

Alex: argtype is a pretty general name to grab at the top level spec ns. How about something like class-ident?





[CLJ-2011] clojure.walk.macroexpand-all will not properly expand macros that depend on &env Created: 23/Aug/16  Updated: 23/Aug/16

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

Type: Defect Priority: Major
Reporter: Collin Bell Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: macro, walk
Environment:

MacOSX, Clojure 1.9.0-alpha10, Java 1.8.0_45, CIDER 0.13.0snapshot (package: 20160602.809), nREPL 0.2.12



 Description   

(clojure.walk/macroexpand-all '(defn foo [a] (go [] a)))

Unhandled clojure.lang.ExceptionInfo
Could not resolve var: a
{:var a}

This is because go depends on &env and macroexpand-all does not handle &env.

The reason this issue is important is because it breaks the cider debugger for async.






[CLJ-2010] clojure.spec/fdef does not add specs to doc Created: 22/Aug/16  Updated: 22/Aug/16  Resolved: 22/Aug/16

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

Type: Defect Priority: Major
Reporter: lvh Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: spec


 Description   

fdef docstring claims:

Once registered, function specs are included in doc, checked by
instrument, tested by the runner clojure.spec.test/run-tests, and (if
a macro) used to explain errors during macroexpansion.

When specifying an fdef for a fn, (:doc (meta my-fn) does not include any information about the spec, which is what I was expecting.



 Comments   
Comment by Alex Miller [ 22/Aug/16 10:52 PM ]

You are misinterpreting the intent there. What we mean there is that the clojure.repl/doc function will emit the spec as part of its output once you have declared a spec.

There is no intention to update the meta for a var when you declare an fdef spec.





[CLJ-2009] 'symbol and 'keyword turn "" into unreadable symbol and keyword respectively Created: 22/Aug/16  Updated: 23/Aug/16  Resolved: 22/Aug/16

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

Type: Defect Priority: Major
Reporter: Aaron Brooks Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None


 Description   

In playing with clojure.spec for our upcoming Clojure meetup exercise and I found that (symbol "") returns an empty symbol which is unprintable and unreadable but can still be converted back to an empty string (name (symbol "")) => "". Similarly, (keyword "") returns ":" (which is invalid Clojure and cannot be read) but does round-trip as an object (name (keyword "")) => ""

I'm happy to provide a patch if we can determine the correct behavior. I'll start by making the assertion that the current behavior seems tricksy and prone to cause Great Mystery followed by Great Sadness.



 Comments   
Comment by Alex Miller [ 22/Aug/16 9:32 PM ]

Hey Aaron, symbols and keyword are programmatically constructable (and usable) without being constrained to what the reader can read and the printer can print, and that's often a useful feature (and something we don't consider to be a bug). It is possible that we might in the future have some kind of delimited keyword or symbol (| has been reserved for this) that would allow the reader to read these. I've done some research on this in the past and there were more complexities to it than initially thought so we ended up shelving it, but it's not dead, just in the deep freeze.

I've answered this question so many times that it's silly I haven't put it in the http://clojure.org/guides/faq, so I will do so for the next time.

I think when this comes up, what people most commonly want is some way to avoid making a non-readable keyword or symbol. So some kind of readable-keyword and readable-symbol functions that did validation and either threw an exception or created the keyword or symbol would maybe be a good enhancement idea.

Comment by Aaron Brooks [ 22/Aug/16 10:50 PM ]

Ships passing... I just saw the FAQ update and rechecked here.

The FAQ update is helpful. It might be worth noting in the docstrings of both 'symbol and 'keyword — that they are able to produce keywords and symbols which are unprintable/readable. I do also like the idea of readable-symbol and readable-keyword. Having a defined grammar for those would be great too.

Thanks for taking the time with this.

I am satisfied with my care. ;-D

Comment by Alex Miller [ 22/Aug/16 10:55 PM ]

If you want to file an enhancement for the readable-symbol/keyword, go for it. There is a grammar (kind of) on the http://clojure.org/reference/reader page, but it does not exactly line up with what's actually accepted in the regex, and there are several tickets already out there about the details of that misalignment. I think due to the intent that readable-symbol/keyword could take a conservative approach though and this would be pretty handy.

Comment by Aaron Brooks [ 22/Aug/16 11:03 PM ]

I meant grammar ala [E]BNF or similar. Something you can data rather than human. Also, as you note, the Java regex is pretty loose. I ran into this pretty directly when creating https://github.com/abrooks/clj-chopshop.

I've given some thought about proposing a grammar that would be used by the compiler (or other tools). Would patches for that be generally welcome or are we really attached to the regexes as they are? I understand that there are natural performance and compatibility concerns.

Comment by Alex Miller [ 23/Aug/16 8:23 AM ]

I don't think we're looking to replace the general reader strategy currently in use unless you could demonstrate significantly better performance.





[CLJ-2008] clojure.spec.test/check without args tests fns from clojure.core, erroring out with #:clojure.spec{:failure :no-fn} Created: 21/Aug/16  Updated: 07/Sep/16  Resolved: 07/Sep/16

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

Type: Defect Priority: Major
Reporter: lvh Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: spec
Environment:

N/A


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

 Description   

When running (clojure.spec.test/check (in CIDER, but I doubt that's relevant other than it changes the formatting of the following snippet):

0. { :failure clojure.lang.ExceptionInfo: No fn to spec #:clojure.spec{:failure :no-fn}, :sym clojure.core/when-let, :spec clojure.spec$fspec_impl$reify__13891@6ee9efaf }
  1. { :failure clojure.lang.ExceptionInfo: No fn to spec #:clojure.spec{:failure :no-fn}, :sym clojure.core/let, :spec clojure.spec$fspec_impl$reify__13891@4ff7da85 }
  2. { :failure clojure.lang.ExceptionInfo: No fn to spec #:clojure.spec{:failure :no-fn}, :sym clojure.core/if-let, :spec clojure.spec$fspec_impl$reify__13891@35daec9c }
  3. { :failure clojure.lang.ExceptionInfo: No fn to spec #:clojure.spec{:failure :no-fn}, :sym clojure.core/fn, :spec clojure.spec$fspec_impl$reify__13891@98fcf1f }
... (some elided)
  8. { :failure clojure.lang.ExceptionInfo: No fn to spec #:clojure.spec{:failure :no-fn}, :sym clojure.core/ns, :spec clojure.spec$fspec_impl$reify__13891@283add16 }
  9. { :failure clojure.lang.ExceptionInfo: No fn to spec #:clojure.spec{:failure :no-fn}, :sym clojure.core/defn, :spec clojure.spec$fspec_impl$reify__13891@59681966 }
... (some elided)
  12. { :failure clojure.lang.ExceptionInfo: No fn to spec #:clojure.spec{:failure :no-fn}, :sym clojure.core/defn-, :spec clojure.spec$fspec_impl$reify__13891@3b34b0db }

It doesn't seem appropriate to tell me that these macros have no fn to spec. Perhaps it should know about macros.

Problem: checkable-syms does not omit spec'ed macros so when they flow down to check-1, they throw errors. Since these can't be checked, they should be omitted from checkable-syms. I put the change in fn-spec-name?, which is also used (and has the same problem in) instrumentable-syms.

Approach: Skip spec'ed macros.

Patch: clj-2008.patch



 Comments   
Comment by Alex Miller [ 22/Aug/16 10:10 AM ]

That's interesting as there were changes in alpha11 designed to catch and omit macros from check. So, something still amiss there.

Comment by Stuart Halloway [ 26/Aug/16 7:31 AM ]

lvh: thanks for the report!

Not sure that we want to give up on generatively testing macros, but it certainly doesn't work atm, so this seems good until we take it on.





[CLJ-2007] if-let* & when-let* with multiple bindings implementation Created: 21/Aug/16  Updated: 21/Aug/16

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

Type: Enhancement Priority: Major
Reporter: Ertuğrul Çetin Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Patch: Code

 Description   

I think Clojure programs will be more elegant if we use let versions of if & when with multiple bindings.

;; if-let* imp.

(defmacro if-let*
  "Multiple binding version of if-let"
  ([bindings then]
   `(if-let* ~bindings ~then nil))
  ([bindings then else]
   (when (seq bindings)
     (assert-args
       (vector? bindings) "a vector for its binding"
       (even? (count bindings)) "exactly even forms in binding vector"))
   (if (seq bindings)
     `(if-let [~(first bindings) ~(second bindings)]
        (if-let* ~(vec (drop 2 bindings)) ~then ~else)
        ~(if-not (second bindings) else))
     then)))

;;Example if-let*

(if-let* [a 1
          b (+ a 1) ]
          b)

;;=> 2

(if-let* [a 1
          b (+ a 1)
          c false] ;;false or nil - does not matter
          b
          a)

;;=> 1

;; when-let* imp.

(defmacro when-let*
  "Multiple binding version of when-let"
  [bindings & body]
  (when (seq bindings)
    (assert-args
      (vector? bindings) "a vector for its binding"
      (even? (count bindings)) "exactly even forms in binding vector"))
  (if (seq bindings)
    `(when-let [~(first bindings) ~(second bindings)]
       (when-let* ~(vec (drop 2 bindings)) ~@body))
    `(do ~@body)))

;;Example when-let*

  (when-let* [a 1 
             b 2 
             c (+ a b)]
             (println "yeah!")
             c)
  ;;=>yeah!
  ;;=>3

  (when-let* [a 1 
             b nil 
             c 3]
             (println "damn! b is nil")
             a)
  ;;=>nil





[CLJ-2006] clojure.spec/fdef mentions nonexistent clojure.spec.test/run-tests Created: 21/Aug/16  Updated: 07/Sep/16  Resolved: 07/Sep/16

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

Type: Defect Priority: Minor
Reporter: lvh Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: docstring, spec
Environment:

N/A


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

 Description   

Once registered, function specs are included in doc, checked by
instrument, tested by the runner clojure.spec.test/run-tests, and (if
a macro) used to explain errors during macroexpansion.

Should be: clojure.spec.test/check



 Comments   
Comment by lvh [ 21/Aug/16 6:55 AM ]

Crud, I messed up the title of this issue copy-pasting from the docstring, and I appear to lack the permissions to resolve it.

Comment by Alex Miller [ 21/Aug/16 3:34 PM ]

lvh I've added you to the groups with edit permission.

Comment by Stuart Halloway [ 26/Aug/16 7:34 AM ]

Thanks again lvh!





[CLJ-2005] Type hint fails with direct linking disabled Created: 19/Aug/16  Updated: 20/Aug/16

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

Type: Defect Priority: Minor
Reporter: Viktor Magyari Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: compiler, directlinking, typehints

Attachments: Text File 0001-CLJ-2005-assoc-arglist-ret-tag-as-tag-in-constructed.patch    
Patch: Code
Approval: Triaged

 Description   

Minimal example, using 1.9.0-alpha11:

user=> (set! *warn-on-reflection* true)
true
user=> (defn foo ^String [^long x] "")
#'user/foo
user=> (.length (foo 10))
Reflection warning, (...) - reference to field length on java.lang.Object can't be resolved.
0

The warning is present only if direct linking is disabled.

Explanation:
this is another manifestation of CLJ-1533 – because of the lexical transformation the compiler is doing when routing the invoke through invokePrim, the arglists type hints are lost. This doesn't happen when DL is on because invokeStatic isn't compiled via a lexical transformation but through StaticInvokeExpr which properly tracks the original var's type hints

Patch: 0001-CLJ-2005-assoc-arglist-ret-tag-as-tag-in-constructed.patch



 Comments   
Comment by Nicola Mometto [ 19/Aug/16 5:24 PM ]

With DL on:

public static java.lang.Object invokeStatic();
    descriptor: ()Ljava/lang/Object;
    flags: ACC_PUBLIC, ACC_STATIC
    Code:
      stack=2, locals=0, args_size=0
         0: ldc2_w        #12                 // long 10l
         3: invokestatic  #18                 // Method test$foo.invokeStatic:(J)Ljava/lang/Object;
         6: checkcast     #20                 // class java/lang/String
         9: invokevirtual #24                 // Method java/lang/String.length:()I
        12: invokestatic  #30                 // Method java/lang/Integer.valueOf:(I)Ljava/lang/Integer;
        15: areturn
      LineNumberTable:
        line 5: 0
        line 5: 9

with DL off:

public static java.lang.Object invokeStatic();
    descriptor: ()Ljava/lang/Object;
    flags: ACC_PUBLIC, ACC_STATIC
    Code:
      stack=3, locals=0, args_size=0
         0: getstatic     #15                 // Field const__0:Lclojure/lang/Var;
         3: invokevirtual #20                 // Method clojure/lang/Var.getRawRoot:()Ljava/lang/Object;
         6: checkcast     #22                 // class clojure/lang/IFn$LO
         9: ldc2_w        #23                 // long 10l
        12: invokeinterface #28,  3           // InterfaceMethod clojure/lang/IFn$LO.invokePrim:(J)Ljava/lang/Object;
        17: ldc           #30                 // String length
        19: iconst_0
        20: invokestatic  #36                 // Method clojure/lang/Reflector.invokeNoArgInstanceMember:(Ljava/lang/Object;Ljava/lang/String;Z)Ljava/lang/Object;
        23: areturn
      LineNumberTable:
        line 5: 0
        line 5: 12
        line 5: 17
Comment by Nicola Mometto [ 19/Aug/16 5:43 PM ]

bytecode with DL off and current patch:

public static java.lang.Object invokeStatic();
    descriptor: ()Ljava/lang/Object;
    flags: ACC_PUBLIC, ACC_STATIC
    Code:
      stack=3, locals=0, args_size=0
         0: getstatic     #15                 // Field const__0:Lclojure/lang/Var;
         3: invokevirtual #20                 // Method clojure/lang/Var.getRawRoot:()Ljava/lang/Object;
         6: checkcast     #22                 // class clojure/lang/IFn$LO
         9: ldc2_w        #23                 // long 10l
        12: invokeinterface #28,  3           // InterfaceMethod clojure/lang/IFn$LO.invokePrim:(J)Ljava/lang/Object;
        17: checkcast     #30                 // class java/lang/String
        20: invokevirtual #34                 // Method java/lang/String.length:()I
        23: invokestatic  #40                 // Method java/lang/Integer.valueOf:(I)Ljava/lang/Integer;
        26: areturn
      LineNumberTable:
        line 5: 0
        line 5: 12
        line 5: 20




[CLJ-2004] s/multi-spec doesn't include :retag in `s/form` Created: 18/Aug/16  Updated: 07/Sep/16  Resolved: 07/Sep/16

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

Type: Defect Priority: Major
Reporter: Allen Rohner Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: spec

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

 Description   

`s/form` on a multispec doesn't include the :retag, so it's impossible to recover that information from an existing spec. This is important for tooling that interacts with specs.

(require '[clojure.spec :as s])
(defmulti mm :mm/type)
(s/def ::foo (s/multi-spec mm :mm/type))
(s/form ::foo)
;; (clojure.spec/multi-spec user/mm) ;; :mm/type is missing

Approach: Include retag in form

Patch: clj-2004.patch



 Comments   
Comment by Alex Miller [ 18/Aug/16 6:18 PM ]

Yeah, this is one of many known form issues.





[CLJ-2003] Nesting cat inside ? causes unform to return nested result Created: 11/Aug/16  Updated: 08/Nov/16

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

Type: Defect Priority: Major
Reporter: Sam Estep Assignee: Unassigned
Resolution: Unresolved Votes: 3
Labels: spec

Approval: Triaged

 Description   

Calling conform and then unform with a spec that consists of some cat nested inside of some ? creates an extra level of nesting in the result:

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

(let [spec (s/? (s/cat :foo #{:foo}))
      initial [:foo]
      conformed (s/conform spec initial)
      unformed (s/unform spec conformed)]
  [initial conformed unformed])
;;=> [[:foo] {:foo :foo} [(:foo)]]

This behavior does not occur with just ? or cat alone:

(let [spec (s/? #{:foo})]
  (s/unform spec (s/conform spec [:foo])))
;;=> [:foo]

(let [spec (s/cat :foo #{:foo})]
  (s/unform spec (s/conform spec [:foo])))
;;=> (:foo)


 Comments   
Comment by Phil Brown [ 14/Aug/16 9:55 PM ]

I came across another case of extra nesting, when repeating one or more sequences with an optional element at the beginning or end, where that element's predicate also matches the element at the other end:

user=> (s/conform (s/+ (s/cat :k any? :v (s/? any?))) [:a 1 :b 2])
[{:k :a, :v 1} [{:k :b, :v 2}]]

where I expected

[{:k :a, :v 1} {:k :b, :v 2}]

The following give expected results:

user=> (s/conform (s/+ (s/cat :k any? :v (s/? any?))) [:a 1 :b])
[{:k :a, :v 1} {:k :b}]
user=> (s/conform (s/+ (s/cat :k keyword? :v (s/? int?))) [:a 1 :b 2])
[{:k :a, :v 1} {:k :b, :v 2}]
user=> (s/conform (s/* (s/cat :k any? :v (s/? any?))) [:a 1 :b 2])
[{:k :a, :v 1} {:k :b, :v 2}]
Comment by Alex Miller [ 01/Sep/16 11:06 AM ]

Phil, I think your example is a different issue and you should file a new jira for that.

Comment by Alex Miller [ 01/Sep/16 3:05 PM ]

Well, maybe I take that back, they may be related.

Comment by Brandon Bloom [ 08/Nov/16 6:10 PM ]

I just ran in to this trying to make sense of some defn forms. Here's an example:

user=> (s/unform :clojure.core.specs/defn-args (s/conform :clojure.core.specs/defn-args '(f [& xs])))
(f ((& xs)))





[CLJ-2002] StackOverflowError in clojure.spec Created: 11/Aug/16  Updated: 26/Aug/16

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

Type: Defect Priority: Major
Reporter: Michiel Borkent Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: spec

Approval: Triaged

 Description   

In this example a non-conforming value is passed to conform, which should return ::s/invalid but instead throws StackOverflow.

(s/conform (s/* (s/alt :n (s/* number?) :s (s/* string?))) [[1 2 3]])

CompilerException java.lang.StackOverflowError, compiling:(/Users/alex/code/clojure.spec/src/spec/examples/tree.clj:44:1)
	clojure.lang.Compiler.load (Compiler.java:7415)
	user/eval2674 (form-init3668332544888233146.clj:1)
	user/eval2674 (form-init3668332544888233146.clj:1)
	clojure.lang.Compiler.eval (Compiler.java:6951)
	clojure.lang.Compiler.eval (Compiler.java:6914)
	clojure.core/eval (core.clj:3187)
	clojure.core/eval (core.clj:3183)
	clojure.main/repl/read-eval-print--9692/fn--9695 (main.clj:241)
	clojure.main/repl/read-eval-print--9692 (main.clj:241)
	clojure.main/repl/fn--9701 (main.clj:259)
	clojure.main/repl (main.clj:259)
	clojure.tools.nrepl.middleware.interruptible-eval/evaluate/fn--675 (interruptible_eval.clj:69)
Caused by:
StackOverflowError 
	clojure.spec/deriv (spec.clj:1296)
	clojure.spec/deriv (spec.clj:1311)
	clojure.spec/deriv/fn--13794 (spec.clj:1312)
	clojure.core/map/fn--6680 (core.clj:2728)
	clojure.lang.LazySeq.sval (LazySeq.java:40)
	clojure.lang.LazySeq.seq (LazySeq.java:49)
	clojure.lang.RT.seq (RT.java:525)
	clojure.core/seq--6221 (core.clj:137)
	clojure.core/map/fn--6687 (core.clj:2736)
	clojure.lang.LazySeq.sval (LazySeq.java:40)
	clojure.lang.LazySeq.seq (LazySeq.java:49)
	clojure.lang.RT.seq (RT.java:525)


 Comments   
Comment by Phil Brown [ 14/Aug/16 9:50 PM ]

While the following isn't super useful, it causes one too:

user=> (s/conform (s/+ (s/? any?)) [:a])

StackOverflowError   clojure.lang.RT.first (RT.java:683)




[CLJ-2001] Invalid conversion from BigDecimal to long using clojure.core/long Created: 09/Aug/16  Updated: 09/Aug/16

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

Type: Defect Priority: Major
Reporter: Eugene Aksenov Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: math
Environment:

Ubuntu Linux 15


Attachments: Text File clj-2001.patch    
Patch: Code and Test
Approval: Triaged

 Description   

Trying to convert from BigDecimal to long

(long 201608081812113241M)
=> 201608081812113248                  ;; not really our number

let's just use BigDecimal.longValue()

(.longValue 201608081812113241M)
=> 201608081812113241                  ;; ok, correct value

looking into clojure.lang.RT and suspecting incorrect conversion chain

(.longValue (.doubleValue 201608081812113241M))
=> 201608081812113248                  ;; yep, incorrect

Cause: long cast from BigDecimal will use Number.longValue(), which in this case produces an incorrect value even though the conversion is possible. The javadoc indicates that this call is equivalent to a double to long conversion and is potentially lossy in several ways.

Approach: add explicit case in long cast to handle BigDecimal and instead call longValueExact(). Patch adds additional cast tests for some BigInteger and BigDecimal values. The unchecked-long cast does not seem to be affected (returned the proper value with no changes).

Questions: while it may be confusing, the incorrect result may actually be the one that is consistent with Java. unchecked-long would give the expected result and may be the better choice for the example here. So it's possible that we should NOT apply this patch and instead do nothing. If we do move forward with the patch, we may want to also apply an equivalent change to call byteValueExact(), shortValueExact(), intValueExact(), and toBigIntegerExact() in the appropriate places as well.

Patch: clj-2001.patch



 Comments   
Comment by Alex Miller [ 09/Aug/16 8:14 AM ]

Yeah, RT.longCast() doesn't seem to explicitly handle BigDecimal.

Comment by Ghadi Shayban [ 09/Aug/16 10:07 AM ]

Patch seems like it may negatively affect inlining

Comment by Alex Miller [ 09/Aug/16 7:36 PM ]

Indeed that's a possibility, although I think it's probably rare in this case.





[CLJ-2000] (transduce) does an unexpected extra step Created: 06/Aug/16  Updated: 07/Aug/16  Resolved: 07/Aug/16

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

Type: Defect Priority: Minor
Reporter: Vadim Liventsev Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None


 Description   

https://stackoverflow.com/questions/38809642/applying-a-transducer-directly-and-with-transduce-yield-different-results/38809928#38809928

(reduce) calls (reducer aggregate element) for every element in the collection. A total of n calls for a collection of n elements.

(transduce) calls (reducer aggregate element) for every element and then for some reason calls (reducer aggregate) again, making n+1 calls. As a result, (transduce) doesn't work as expected with .

Is it a bug?



 Comments   
Comment by Kevin Downey [ 06/Aug/16 10:21 PM ]

this is a feature of transduce. the docstring for transduce says "f should be a reducing step function that accepts both 1 and 2 arguments, if it accepts only 2 you can add the arity-1 with 'completing'." which, if you know about the completing arity of reducing functions clues you in, but if you don' the docstring is not very helpful about it.

Comment by Alex Miller [ 07/Aug/16 11:42 AM ]

As Kevin said in the comments, the question assumes things that are not accurate. Everything here is working as expected/designed.





[CLJ-1999] Infinity and -Infinity do not equal literal versions if within a data structure Created: 03/Aug/16  Updated: 03/Aug/16  Resolved: 03/Aug/16

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

Type: Defect Priority: Minor
Reporter: Darrick Wiebe Assignee: Unassigned
Resolution: Duplicate Votes: 0
Labels: None


 Description   

The first statement below is correctly true, but the second one is false.

(= [0 Double/POSITIVE_INFINITY] [0 Double/POSITIVE_INFINITY])

(= [0 Double/POSITIVE_INFINITY] (clojure.edn/read-string "[0 Infinity]"))



 Comments   
Comment by Alex Miller [ 03/Aug/16 1:37 PM ]

The problem here is that Infinity is read as a symbol, not as Double/POSITIVE_INFINITY.

I think these issues are captured better in http://dev.clojure.org/jira/browse/CLJ-1074 so I'm going to mark this as a dupe of that one.





[CLJ-1998] clj.spec: improve boolean kw option naming Created: 03/Aug/16  Updated: 09/Dec/16  Resolved: 09/Dec/16

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

Type: Enhancement Priority: Trivial
Reporter: Max Penet Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: spec


 Description   

We have a mix of boolean keyword options with and without trailing "?" at the moment. It would be good to settle to 1 style, hopefully the one with the trailing "?".

Ex: in map-of we have :conform-keys, in double-in: NaN? and :infinite? and possibly others.



 Comments   
Comment by Alex Miller [ 09/Dec/16 4:21 PM ]

I don't think we're going to make any changes for this, thanks





[CLJ-1997] Macros cannot reliably detect usage of locals Created: 02/Aug/16  Updated: 02/Aug/16

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

Type: Enhancement Priority: Major
Reporter: Gary Fredericks Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: macro


 Description   

Problem

The motivating problem is the implementation of gen/let in test.check (see also TCHECK-98).

A common usage of gen/let might look something like this:

(gen/let [a gen-a
          b gen-b]
  (f a b))

The crucial characteristic of this code is that the generator for b does not depend on the value a (though in general it could). Because of this independence, the ideal expansion is:

(gen/fmap 
  (fn [[a b]] (f a b)) 
  (gen/tuple gen-a gen-b))

However, because gen/let cannot, in general, tell whether or not the expression for the generator for b depends on a, it needs to fallback to a more general expansion:

(gen/fmap
  (fn [[a b]] (f a b))
  (gen/bind 
    gen-a
    (fn [a]
      (gen/tuple (gen/return a) gen-b))))

Using gen/bind greatly reduces shrinking power, and so it's best to avoid it when possible.

A knowledgeable user could get around this by using gen/tuple explicitly, e.g.:

(gen/let [[a b] (gen/tuple gen-a gen-b)]
  (f a b))

But I think most users would prefer not to have to think about these things.

Possible Solutions

tools.analyzer

tools.analyzer is probably adequate, but is a large dependency for a library.

a subset of tools.analyzer

Nicola has mentioned the idea of carving out some subset of the analyzer that would be sufficient for this case, and that might be the best option.

a mechanism for macroexpanding a macro body

I believe if there were a robust mechanism for a macro to fully macroexpand an expression that this problem would be easier (clojure.core/macroexpand and friends have a few known incorrectnesses) – a simple tree-seq over the expanded expression could prove that a local is not used (though a naive approach might falsely conclude that a local *is* used, which might be an acceptable compromise for the test.check case, and otherwise a robust code walker should not be difficult to implement on expanded code).

I believe zach's riddley library does something like this, and depending on riddley would probably be the best option for a non-contrib library, but is not an acceptable dependency for a contrib library.






[CLJ-1996] clojure.spec stubs don't cooperate with clojure.spec.test/check Created: 31/Jul/16  Updated: 31/Jul/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: None


 Description   

This is just like CLJ-1949, but for stubs instead of higher-order-function arguments.

The solution is more difficult, though, since cst/check and cst/instrument can be called/used seperately.

My only idea is to have a dynamic var where the two can coordinate. Stubs would use gen/generate when not called during testing, but in the context of a call to cst/check the dynamic var would contain an alternate implementation that works similarly to the patch in CLJ-1949.

I'd be happy to prepare a patch with that implementation (or any other) if desired.






[CLJ-1995] Improved docstring for explain-data Created: 30/Jul/16  Updated: 30/Jul/16

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

Type: Enhancement Priority: Minor
Reporter: Marshall Abrams Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring, spec

Approval: Triaged

 Description   

In 1.9.0-alpha10, the docstring for explain-data doesn't mention or describe the meaning of some standard keys/values of its return value, and the use of "path" in the docstring could be clarified to avoid conflation with file paths or namespace paths. Here is the current docstring:

Given a spec and a value x which ought to conform, returns nil if x conforms, else a map with at least the key ::problems whose value is a collection of problem-maps, where problem-map has at least :path :pred and :val keys describing the predicate and the value that failed at that path.

Here is a possible replacement:

Given a spec and a value x which ought to conform, returns nil if x conforms, else a map with at least the key ::problems whose value is a collection of problem-maps, where problem-map has at least :path :pred and :val keys describing the predicate and the value that failed at that path (through possibly embedded specs). The map may also contain a :via key for specs that failed, an :in key for data key(s) of the value that failed, and a :reason key for a string describing the reason for failure.

This differs from the existing docstring in two ways:

1. It inserts "(through possibly embedded specs)" at the end of the existing dostring to clarify and disambiguate the meaning of "path" here.

2. It adds an additional sentence describing the :via, :in, and :reason keys.






[CLJ-1994] the-ns / clojure.lang.Namespace/find should handle nil values Created: 29/Jul/16  Updated: 31/Jul/16  Resolved: 31/Jul/16

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

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


 Description   

After some refactoring, I had the (broken) code:

(test-all-vars (find-ns 'missing.ns))

when I ran my tests, I got this error:

...
Caused by: java.lang.NullPointerException
	at java.util.concurrent.ConcurrentHashMap.get(ConcurrentHashMap.java:936)
	at clojure.lang.Namespace.find(Namespace.java:188)
	at clojure.core$find_ns.invokeStatic(core.clj:4000)
	at clojure.core$the_ns.invokeStatic(core.clj:4030)
	at clojure.core$ns_interns.invokeStatic(core.clj:4077)
	at clojure.test$test_all_vars.invokeStatic(test.clj:736)
	at clojure.test$test_all_vars.invoke(test.clj:736)
...

I expected there would be an exception thrown that the namespace could not be found.

Looking into it further, it seems like the issue is that find-ns was called twice in this code, once at the top level on the non-existent namespace which returned nil, and then a second time in the-ns with nil as the value. The second nil blows up inside clojure.lang.Namespace/find. It seems like there should be a nil check somewhere along the way, probably either in the-ns, or clojure.lang.Namespace/find, which would then throw a descriptive error message.



 Comments   
Comment by Alex Miller [ 29/Jul/16 7:58 AM ]

find-ns says "Returns the namespace named by the symbol or nil if it doesn't exist." and you're getting nil in this case.

So can't this just be simplified to (test-all-vars nil)? Why should we expect that to work?

Comment by Daniel Compton [ 31/Jul/16 5:15 AM ]

You're right, I'm not expecting it to work, I'm suggesting that we throw a more descriptive error. Thinking about this over the last few days, there are a few options:

Add a nil check in the-ns so that an exception is also thrown on nil passed as a namespace:

;; Something with this intent:
(if (instance? clojure.lang.Namespace x)
    x
    (or (and x (find-ns x)) (throw (Exception. (str "No namespace: " x " found")))))

Add a nil check in find-ns:

(defn find-ns
  "Returns the namespace named by the symbol or nil if it doesn't exist."
  {:added  "1.0"
   :static true}
  [sym] (when (some? sym) (clojure.lang.Namespace/find sym)))

Add a nil check in clojure.lang.Namespace/find:

public static Namespace find(Symbol name) {
        // If nil throw more descriptive error than NPE
        return (Namespace)namespaces.get(name);
    }

Thinking some more about it, all of these changes could be breaking to people who relied on the old behaviour. It's not clear to me whether this is worth continuing. Thoughts?

Comment by Alex Miller [ 31/Jul/16 8:16 AM ]

I think find-ns, Namespace/find, and the-ns are behaving as documented and intended. If anything, having a spec for test-all-vars would catch this more explicitly.

(s/fdef clojure.test/test-all-vars :args #(instance? clojure.lang.Namespace %))
(st/instrument 'clojure.test/test-all-vars)

;; results in:
user=> (clojure.test/test-all-vars (find-ns 'foo))
ExceptionInfo Call to #'clojure.test/test-all-vars did not conform to spec:
val: (nil) fails at: [:args] predicate: (instance? clojure.lang.Namespace %)
:clojure.spec/args  (nil)
:clojure.spec/failure  :instrument
:clojure.spec.test/caller  {:file "NO_SOURCE_FILE", :line 11, :var-scope user/eval23}
  clojure.core/ex-info (core.clj:4724)

which would have pointed you pretty precisely to the problem. But I don't think it's worth keeping this ticket open re the spec as we are working on specs via other means.





[CLJ-1993] Print flag to suppress namespace map syntax Created: 28/Jul/16  Updated: 19/Aug/16  Resolved: 16/Aug/16

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

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

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

 Description   

Add new print flag *print-namespace-maps* to optionally suppress namespace map syntax. Default flag to false.

Set flag to true as part of the standard REPL bindings. This allows repl users to (set! *print-namespace-maps* false) if desired.

Patch: clj-1993-2.patch



 Comments   
Comment by Rich Hickey [ 16/Aug/16 7:33 AM ]

the plan/approach should be somewhere in the description and not just the code please. Also, I wonder if the default should be false other than at the REPL?

Comment by Alex Miller [ 16/Aug/16 6:53 PM ]

Committed for next alpha





[CLJ-1992] Add explanation for clojure.test-clojure.metadata/public-vars-with-docstrings-have-added failure Created: 26/Jul/16  Updated: 26/Jul/16  Resolved: 26/Jul/16

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

Type: Enhancement Priority: Trivial
Reporter: Daniel Compton Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: docstring

Patch: Code and Test

 Description   

While I was adding the async macro patch, I ran the tests and clojure.test-clojure.metadata/public-vars-with-docstrings-have-added failed. As someone unfamiliar to the codebase, it wasn't very obvious why it had failed, or what I had to do to fix it. I traced it back to understand I needed to add {:added "1.9"}. This is obvious in hindsight, but wasn't apparent from the test failure message. The attached patch adds an explanation of why the test failed.

Compare the test output from before and after:

[java] FAIL in (public-vars-with-docstrings-have-added) (metadata.clj:46)
     [java] expected: (= [] (remove (comp :added meta) public-vars-with-docstrings-not-generated))
     [java]   actual: (not (= [] (#'clojure.test/async)))
[java] FAIL in (public-vars-with-docstrings-have-added) (metadata.clj:46)
     [java] All public vars with docstrings must have :added metadata, e.g. {:added "1.0"}
     [java] expected: (= [] (remove (comp :added meta) public-vars-with-docstrings-not-generated))
     [java]   actual: (not (= [] (#'clojure.test/async)))


 Comments   
Comment by Alex Miller [ 26/Jul/16 5:33 PM ]

"public vars with docstrings have added" is the documentation, not going to bother with this

Comment by Daniel Compton [ 26/Jul/16 5:38 PM ]

It's not the documentation, it's the name of the test, and it wasn't very clear. Changing the test name to public-vars-with-docstrings-have-added-metadata would improve things. This is a small thing, but it tripped me up, and it seems like it could make contributing to Clojure a tiny bit nicer and easier.





[CLJ-1991] ClassCastException in Compiler.java Created: 26/Jul/16  Updated: 27/Jul/16  Resolved: 26/Jul/16

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

Type: Defect Priority: Major
Reporter: Jeremy Betts Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None
Environment:

Linux Zulu JDK



 Description   

on line 272 of Compiler.java there is a class cast exception when the value from the system property is an Integer. We cannot upgrade from 1.6 until this is fixed

it is probably best to do a toString() rather than a hard cast.

for (Map.Entry e : System.getProperties().entrySet())
{
String name = (String) e.getKey();
String v = (String) e.getValue();



 Comments   
Comment by Alex Miller [ 26/Jul/16 12:53 PM ]

Hi Jeremy,

This has been filed and declined in the past (see CLJ-1717) on the grounds that System property values are defined to be Strings and to do otherwise is an error in whomever set that property, not on Clojure's assumption.

Alex

Comment by Jeremy Betts [ 26/Jul/16 1:33 PM ]

You realize that you are basically locking my company at version 1.6 right?

This should be fixed from a code quality perspective regardless.

A) why do an unchecked casts to string rather than using toString() and
B) its a one line change!

This type of thinking is only going to stop the adoption of Clojure from being used as anything more than a toy language.

In the 'real world' other code has bugs and it cannot always be fixed. You should be writing robust code that doesn't 'assume' that the world it is in is a pristine environment.

Comment by Sean Corfield [ 26/Jul/16 10:39 PM ]

Jeremy, Java itself defines System property values as Strings: https://docs.oracle.com/javase/7/docs/api/java/util/Properties.html

Can you show how you are "locked" at Clojure version 1.6 because of this?

It's not like you can store non-String values in there:

user=> (System/setProperty "foo" 123)

ClassCastException java.lang.Long cannot be cast to java.lang.String  user/eval1227 (form-init3357403087479620173.clj:1)

That's on Clojure 1.6.0.

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

Sean, some (bad) Java libs do this - Properties extends Hashtable and thus it's possible to use the super-class put method to put a non-String value into the System properties map after startup - Java calls these "compromised" properties. Creating properties like this breaks many API assumptions in the Properties class and is a known Bad thing to do.

Jeremy, I'm sorry that this is an issue for you, but the problem here is really with the code that is setting the System property, not Clojure.

Comment by Jeremy Betts [ 27/Jul/16 9:00 AM ]

I realize that Clojure has a garbage in garbage out approach - but since this code is in a static {} block, where exactly should I put 'safety code' to ensure that no non String values exist in the system properties? The JVM controls when static blocks get run!

Obviously non stings can get in there since this issue has been reported before - Alex you even new the previous issue number off the top of your head! Any third party code could be doing it.

It would be wise not go blundering into the well known areas of bad design in Java.

The solution is to call toString() instead of doing a unchecked hard cast to (String). This is simply a good coding standard anyway.

can you give any real reason not to make this simple change? You can't even claim performance since this is a static block which gets run exactly once!

We all know that java isn't the best language, that is why we seek other languages like Clojure. But if those other languages force the worst features of Java to the forefront to cause pain why bother?

I would need a reliable work around to this issue that ensures it can never happen. But since the issue can reoccur at the fancy of the JVM executing the static code elsewhere, this is a total show stopper.

Comment by Jeremy Betts [ 27/Jul/16 9:23 AM ]

Also, attempting to remove values from system properties to mitigate this issue has that high risk of throwing a concurrent modification exception.

Comment by Jeremy Betts [ 27/Jul/16 12:20 PM ]

here is a simple fix that represents better java programming practice and makes the code faster and safer.

for (Map.Entry e : System.getProperties().entrySet())
{
String name = (String) e.getKey();
if (name.startsWith("clojure.compiler."))

{ String v = (String) e.getValue(); compilerOptions = RT.assoc(compilerOptions, RT.keyword(null, name.substring(1 + name.lastIndexOf('.'))), RT.readString(v)); }

}

Comment by Alex Miller [ 27/Jul/16 1:25 PM ]

What is putting bad properties in your system? Shouldn't you be spending your effort on the root cause?

Comment by Jeremy Betts [ 27/Jul/16 1:37 PM ]

an ancient mail library. I'm pushing on both bugs.

Why don't you want to fix yours? I'm sure you've spent more time fighting with JIRA issues than what it would take to fix it.

Comment by Alex Miller [ 27/Jul/16 2:34 PM ]

Because it's not our bug.

Comment by Jeremy Betts [ 27/Jul/16 2:41 PM ]

you assign values you never use. You obtain these values from a global static hashmap that anyone can add any value to, even though they "should" only add strings. You do an unchecked assignment. You have a bug.





[CLJ-1990] Add an async macro that behaves the same as ClojureScript's Created: 24/Jul/16  Updated: 26/Jul/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: 2
Labels: clojure.test, portability

Attachments: Text File clj-1990v1.patch    
Patch: Code and Test
Approval: Triaged

 Description   

We want to run the same tests between Clojure and ClojureScript. However some of our CLJS tests are asynchronous and require the use of the async macro. Clojure doesn't have the async macro, which makes test portability difficult. If we were to add the async macro, there are at least two possible routes to go down: imitating the async behaviour of cljs.test, or copying the implementation. If we were to imitate the behaviour, we could use the following async macro. It creates a promise, runs the body, blocks on the promise, and done delivers the promise, allowing the test to continue.

(defmacro async
  [done & body]
  `(let [p# (promise)
         ~done #(deliver p# nil)]
     ~@body
     (deref p#)))

This has the advantage that it integrates with the current way tests are run in Clojure, and doesn't require any of the surrounding tooling to be aware of the async testing.

Imitating the implementation would be much more invasive. It would probably mean changing the behaviour of run-tests to return nil like ClojureScript does, and a host of other changes. This means it is likely a non-starter.

I lean heavily towards the first option.

To answer the question "Why do we need this at all when it is a no-op?", this is useful so we can write the same tests in both Clojure and ClojureScript. It would be much more tricky to write without it.



 Comments   
Comment by Daniel Compton [ 26/Jul/16 12:12 AM ]

v1, patch and tests





[CLJ-1989] `let` ported from `test.check/let` to `clojure.spec.gen` Created: 24/Jul/16  Updated: 23/Aug/16

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

Type: Enhancement Priority: Minor
Reporter: Matthew Wampler-Doty Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: spec

Attachments: Text File gen-let.patch    
Patch: Code and Test

 Description   

When using `clojure.spec` for elaborate specifications and `clojure.spec.gen` for generative testing, developers often find themselves writing code which heavily relies on `clojure.spec.gen/fmap`. This is sometimes unnatural and difficult to read.

To make writing custom generators easier, this patch ports `test.check/let` to `clojure.spec.gen`. Now developers can write generators more simply.



 Comments   
Comment by Matthew Wampler-Doty [ 24/Jul/16 5:55 PM ]

For example, if a user wanted to make a generator of vectors with length between 5 and 11 or 20 to 40 elements, consisting of keywords which were either `:a` or `:b`, they would have to write something like:

(gen/fmap (fn [[n gens]] (take n gens)) 
          (gen/tuple (spec/gen (spec/or :short (int-in 5 11) 
                                        :long (int-in 20 40)))
                     (gen/vector (gen/elements #{:a :b}) 40)))

With this patch they could write this as:

(gen/let [length (spec/or :short (int-in 5 11)
                          :long  (int-in 20 40))]
  (repeat length #{:a :b}))




[CLJ-1988] collection specs conform to reversed list when used on a sequence Created: 24/Jul/16  Updated: 07/Sep/16  Resolved: 07/Sep/16

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

Type: Defect Priority: Major
Reporter: Greg Chapman Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: spec

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

 Description   

Example

Clojure 1.9.0-alpha10
user=> (require '[clojure.spec :as s])
nil
user=> (s/conform (s/coll-of int?) (range 5))
(4 3 2 1 0)

Problem: Current code handles vectors, maps, and lists but falls through on sequences to the last case and adds to an empty sequence (at the head).

Approach: Add seqs to the list case.

Patch: clj-1988.patch



 Comments   
Comment by Alex Miller [ 23/Aug/16 2:01 PM ]

In case it helps, a workaround might be: (s/conform (s/coll-of int? :into ()) (range 5))





[CLJ-1987] Update clojure.java.javadoc to open JDK8 docs by default Created: 24/Jul/16  Updated: 24/Jul/16  Resolved: 24/Jul/16

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

Type: Enhancement Priority: Trivial
Reporter: Richard Hull Assignee: Unassigned
Resolution: Duplicate Votes: 0
Labels: documentation

Attachments: Text File jdk8_javadoc.patch    

 Description   

The clojure.java.javadoc function opens up JavaSE 7 docs in a browser - looking at the source (https://github.com/clojure/clojure/blob/master/src/clj/clojure/java/javadoc.clj#L21-L24), this hasn't been updated for a few years.

The attached trivial patch to set the default to use the Java 8 docs



 Comments   
Comment by Alex Miller [ 24/Jul/16 6:32 AM ]

There is already a ticket/patch for this at http://dev.clojure.org/jira/browse/CLJ-1398 - any comments should go there.

Comment by Richard Hull [ 24/Jul/16 10:12 AM ]

Ah ok, I did a search in JIRA and didn't spot CLJ-1398 in the results + couldn't find anything committed in git, so assumed that this was something that was worth submitting. Anyway, thanks for the quick response,
regards
Richard





[CLJ-1986] Suppress printing namespace map literal syntax when only one namespaced key Created: 21/Jul/16  Updated: 21/Jul/16

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

Type: Enhancement Priority: Major
Reporter: Alex Miller Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: maps, print

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

 Description   

Really an aesthetic choice, but right now maps with only a single namespaced key are printed in namespace map literal syntax:

user=> {:my.ns/b 1}
#:my.ns{:b 1}

And that seems unnecessarily complicated (and longer).

Proposal: Only print namespace map literal syntax when >1 key is using the same namespace.

Patch: clj-1986.patch






[CLJ-1985] with-gen of conformer loses unform fn Created: 21/Jul/16  Updated: 16/Aug/16  Resolved: 16/Aug/16

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

Type: Defect Priority: Major
Reporter: Alex Miller Assignee: Unassigned
Resolution: Completed Votes: 1
Labels: spec

Attachments: Text File conformer-with-gen.patch    
Patch: Code and Test
Approval: Ok

 Description   
(def ex (s/conformer inc dec))
(s/conform ex 1) ;; 2
(s/unform ex 2)  ;; 1
(def exc
  (s/with-gen
    (s/conformer inc dec)
    (fn [] (s/gen int?))))
(s/conform exc 1) ;; 2
(s/unform exc 2) ;; fails, no unformer

Cause: with-gen doesn't re-apply the unform fn to the new spec

Patch: conformer-with-gen.patch



 Comments   
Comment by Alex Miller [ 16/Aug/16 8:47 AM ]

Committed for next alpha.





[CLJ-1984] clojure.spec/double-in should allow strict greater-than, less-than tests Created: 20/Jul/16  Updated: 21/Jul/16

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

Type: Enhancement Priority: Minor
Reporter: Marshall Abrams Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: spec


 Description   

clojure.spec/double-in defines a spec that tests whether a double is greater than or equal to a minimum value and less than or equal to a maximum value. This seems like an arbitrary choice from the point of view of mathematics and practical concerns. Sometimes you need to test whether a double is greater than a minimum or less than a maximum. Example: The application will divide by the tested double later.

Of course we can add tests to double-in, e.g. like

(s/and (s/double-in :min 0.0 :max 1.0) #(not= 0.0 %))}}

but

#(and (> % 0.0) (<= % 1))

might be clearer if double-in's NaN and Infinity tests aren't needed.

Why not have a common interface to all four interval tests? Rather than four different spec functions, which is one option, I suppose, I suggest adding two keywords to double-in. When true, these would change the >= or <= tests to > or < tests:

:min-greater

(or? :min+, :min-greater-than, :greater-than-min, :strict-min, :min-open, or possibly :infinmum, :inf, but that could be misleading)

:max-less

(or :max- :max-less-than, :less-than-max, :strict-max, :max-open, or possibly :supremum, :sup etc.)

For example,

(s/valid? (s/double-in :min 0.0 :max 0.1 :min-greater true) 0.0)

would return false, but

(s/valid? (s/double-in :min 0.0 :max 0.1 :min-greater false) 0.0)

would return true.

Default values for these keywords should probably be false, for compatibility with the current definition of double-in.






[CLJ-1983] clojure.data/diff throws an exception when comparing map keys of different types when used on sorted maps Created: 19/Jul/16  Updated: 19/Jul/16  Resolved: 19/Jul/16

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

Type: Defect Priority: Major
Reporter: Thomas Scheiblauer Assignee: Unassigned
Resolution: Duplicate Votes: 0
Labels: None


 Description   

e.g.

(clojure.data/diff (sorted-map :foo 42) (sorted-map 1 42))
(clojure.data/diff (sorted-map :foo 42) (sorted-map "x" 42))
(clojure.data/diff (hash-map :foo 42) (sorted-map 1 42))
(clojure.data/diff (hash-map :foo 42) (sorted-map "x" 42))

will throw
java.lang.ClassCastException: java.lang.Long cannot be cast to clojure.lang.Keyword
while e.g.

(clojure.data/diff (hash-map :foo 42) (hash-map 1 42))
(clojure.data/diff (hash-map :foo 42) (hash-map "x" 2))
(clojure.data/diff (sorted-map :foo 42) (sorted-map :bar 42))

will not.

The same applies to ClojureScript with a different exception (e.g. "Error: Cannot compare :foo to 1")



 Comments   
Comment by Alex Miller [ 19/Jul/16 8:23 AM ]

This is the same root problem as CLJ-1242, so duping to that one.

Comment by Thomas Scheiblauer [ 19/Jul/16 10:30 AM ]

It's not exactly a duplicate since diff should work in any case regardless of (compare x y) not working in this situation (possibly by design?).
(= (sorted-map :foo 42) (sorted-map 1 42)) works by the way.
(compare (sorted-map :foo 42) (sorted-map 1 42)) throws the exception.
In my opinion this could (and maybe should) be fixed in diff.

Comment by Alex Miller [ 19/Jul/16 12:41 PM ]

The stack traces for the two tickets are identical. diff is not using compare, it's using =. (= (sorted-map :foo 42) (sorted-map 1 42)) throws.

user=> (clojure.data/diff (hash-map :foo 42) (sorted-map "x" 42))
ClassCastException java.lang.String cannot be cast to clojure.lang.Keyword  clojure.lang.Keyword.compareTo (Keyword.java:114)
user=> (pst *e)
ClassCastException java.lang.String cannot be cast to clojure.lang.Keyword
	clojure.lang.Keyword.compareTo (Keyword.java:114)
	clojure.lang.Util.compare (Util.java:153)
	clojure.lang.RT$DefaultComparator.compare (RT.java:280)
	clojure.lang.PersistentTreeMap.doCompare (PersistentTreeMap.java:311)
	clojure.lang.PersistentTreeMap.entryAt (PersistentTreeMap.java:298)
	clojure.lang.PersistentTreeMap.containsKey (PersistentTreeMap.java:94)
	clojure.lang.APersistentMap.equiv (APersistentMap.java:87)
	clojure.lang.Util.pcequiv (Util.java:124)
	clojure.lang.Util.equiv (Util.java:32)
	clojure.data/diff (data.clj:134)
	clojure.data/diff (data.clj:120)
	user/eval20 (NO_SOURCE_FILE:11)
Comment by Thomas Scheiblauer [ 19/Jul/16 1:28 PM ]

You are of course right as I can see clearly now.
I did overlook the asymmetrical behavior of '=' in context of a sorted map.
Please excuse my ignorance.





[CLJ-1982] Better explain reporting on a failed zero or one match with an embedded spec. Created: 18/Jul/16  Updated: 26/Aug/16

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

Type: Enhancement Priority: Minor
Reporter: Nick Jones Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: spec
Environment:

OSX, Java 8, Clojure 1.9.0-alpha10


Approval: Triaged

 Description   

Problem:

When attempting to validate a vector containing an optional map, the spec will validate correctly if the vector contains a valid map. If however the optional map does not satisfy the spec misleading error messages are produced. It would be nice if on a partial match of an optional map that some indication of this would be given to the user.

Example REPL session to illustrate problem:

The optional nested map (:optional-nested-map) below fails validation because :nested-element-b is a string instead of an int however the explain report says the spec fails at the parent predicate: :user/vector-schema at: [:element-value] predicate: string?.

It would be more helpful for the user in this case if spec reported that the optional nested map at :optional-nested-map had failed due to ::nested-element-b failing the int? predicate.

user=> (require '[clojure.spec :as s])
nil
user=> (s/def ::nested-element-a string?)
:user/nested-element-a
user=> (s/def ::nested-element-b int?)
:user/nested-element-b
user=> (s/def ::nested-element-schema
          (s/keys :opt-un [::nested-element-a ::nested-element-b]))
:user/nested-element-schema
user=> (s/def ::vector-schema
         (s/cat :tag-kw               #{:tag}
                :optional-nested-map  (s/? (s/spec ::nested-element-schema))
                :element-value        string?))
:user/vector-schema
user=> (s/valid? ::vector-schema [:tag {:nested-element-a "bla" :nested-element-b 10} "Element"])
true
user=> (s/valid? ::vector-schema [:tag {:nested-element-a "bla" :nested-element-b ""} "Element"])
false
user=> (s/explain ::vector-schema [:tag {:nested-element-a "bla" :nested-element-b ""} "Element"])
In: [1] val: {:nested-element-a "bla", :nested-element-b ""} fails spec: :user/vector-schema at: [:element-value] predicate: string?
nil
user=>


 Comments   
Comment by Alex Miller [ 18/Jul/16 7:43 AM ]

Can you update this description with a self-contained example that demonstrates the problem? It's too hard to repro and understand this larger example.

Comment by Nick Jones [ 19/Jul/16 3:30 AM ]

Hi,

Sorry I don't seem to have access to edit the description of the ticket after creation. Here is a simplified sample that I hope will help illustrate the case better.

When the optional nested map below fails validation because :nested-element-b is a string instead of an int the explain report says the spec fails at the parent predicate: :user/vector-schema at: [:element-value] predicate: string?.

As it is an optional map I could see how this would be the case. When no match is found it moves onto the next predicate in the parent.

That said I think it could be helpful (especially in a large optional nested data structure) that if a partial match is achieved that that could be reported to the user as a potential spot for the spec failing.

user=> (require '[clojure.spec :as s])
nil
user=> (s/def ::nested-element-a string?)
:user/nested-element-a
user=> (s/def ::nested-element-b int?)
:user/nested-element-b
user=> (s/def ::nested-element-schema
          (s/keys :opt-un [::nested-element-a ::nested-element-b]))
:user/nested-element-schema
user=> (s/def ::vector-schema
         (s/cat :tag-kw               #{:tag}
                :optional-nested-map  (s/? (s/spec ::nested-element-schema))
                :element-value        string?))
:user/vector-schema
user=> (s/valid? ::vector-schema [:tag {:nested-element-a "bla" :nested-element-b 10} "Element"])
true
user=> (s/valid? ::vector-schema [:tag {:nested-element-a "bla" :nested-element-b ""} "Element"])
false
user=> (s/explain ::vector-schema [:tag {:nested-element-a "bla" :nested-element-b ""} "Element"])
In: [1] val: {:nested-element-a "bla", :nested-element-b ""} fails spec: :user/vector-schema at: [:element-value] predicate: string?
nil
user=>
Comment by Nick Jones [ 19/Jul/16 3:45 AM ]

Added simplified version of project archive matching comment at 2016-07-19.

Comment by Alex Miller [ 19/Jul/16 8:27 AM ]

Nick, I've given you edit rights here. Generally, we don't like to have external projects for repro - if you can boil it down to a few line example in the description, that would be ideal.

Comment by Nick Jones [ 19/Jul/16 8:15 PM ]

Thanks Alex. I've updated the description and removed the project attachments. I've also added a REPL session to the description to reproduce the problem in a standalone Clojure 1.9.0-alpha10 REPL.





[CLJ-1981] `spec/merge` does not flow conformed values across preds per docstring Created: 13/Jul/16  Updated: 18/Jul/16  Resolved: 18/Jul/16

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

Type: Defect Priority: Major
Reporter: Moritz Ulrich Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: spec
Environment:

[org.clojure/clojure "1.9.0-alpha10"]


Approval: Vetted

 Description   

The order of specs passed to spec/merge affect the spec/conform behavior of the keys specified. This seem to happen only with non-prefixed keys via (spec/keys :req-un [..])

The following code snippet shows the broken/non-intuitive behavior:

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

(s/def ::id (s/conformer str))
(s/def ::m (s/keys :req-un [::id]))

;; Correct behavior on ::id
(s/conform ::id 42)
;;=> "42"

;; Fine if unmerged
(s/conform ::m {:id 42})
;;=> {:id "42"}

;; Fine if merged with ::m in the *last* position
(s/conform (s/merge map? ::m) {:id 42})
;;=> {:id "42"}

;; Broken because `map?` is the last arg
(s/conform (s/merge ::m map?) {:id 42})
;;=> {:id 42}

;; Broken because another `s/keys` is used as the last argument
(s/conform (s/merge ::m (s/keys :req-un [::foo]))
           {:id 42 :foo 23})
;;=> {:id 42, :foo 23}


 Comments   
Comment by Alex Miller [ 13/Jul/16 8:36 AM ]

Perhaps a simpler pair of examples - the first should return the result of the second if conformed values are flowing through the predicates.

(s/conform
  (s/merge (s/map-of keyword? (s/or :s string? :n number?))
           map?)
  {:x "s"})
=> {:x "s"}
(s/conform
  (s/merge map?
           (s/map-of keyword? (s/or :s string? :n number?)))
  {:x "s"})
=> {:x [:s "s"]}
Comment by Alex Miller [ 18/Jul/16 9:04 AM ]

This is working as designed. s/merge should not flow conformed values. The docstring has been corrected in https://github.com/clojure/clojure/commit/d920ada9fab7e9b8342d28d8295a600a814c1d8a





[CLJ-1980] Unable to construct gen in indirectly recursive specs with s/every and derivations Created: 12/Jul/16  Updated: 26/Aug/16

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

Type: Defect Priority: Major
Reporter: Leon Grapenthin Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: spec
Environment:

alpha-10


Approval: Triaged

 Description   

Problem statement: Some spec implementations return no generator but nil, in their gen* implementation when their recursion-limit has been reached (e. g. s/or). Specs that implement composition of other specs sometimes respect getting no generator from other specs gen* and adjust behavior of their own gen* accordingly, sometimes to the extent of returning nothing themselves (e. g. s/or's gen* returns nil if of all of its branches specs also don't have a gen and otherwise uses only those gens it got). However, there are various specs that don't respect getting no generator from gen* (like s/every, s/map-of) and they are essential building blocks in many real world recursive specifications. They then end up throwing an exception "Unable to construct gen ...".

Here is a minimal example (not real world usecase illustration) of the problem with actual specs:

;; A ::B is an s/or with branches going through ::B recursively
(s/def ::B (s/or :A ::A))

;; An ::A is a map of keywords to ::Bs (or it is empty as recursive termination)

(s/def ::A (s/map-of keyword? ::B
                     :gen-max 3))

(gen/sample (s/gen ::A))

ExceptionInfo Unable to construct gen at: [1 :A 1 :A 1 :A 1 :A 1] for: :spec.examples.tree/B  clojure.core/ex-info (core.clj:4725)

Valid values for the spec above (I can mail you a real usecase that enforces above pattern in which we parse an internal query DSL) are: {}, {:a {}}, {:foo {:bar {}}} etc.

The problem why the current implementation of spec fails to generate values for above spec is that ::A's map-of doesn't generate an empty map when ::B's gen* returns nil, but instead throws an exception. s/every and all derived specs are affected by this and there might be others.

Proposed fix: A spec's gen* impl must always respect other spec's gen* returning nil not by throwing but by either adjusting the returned gen or by returning nil itself so that the not-returning-gen behavior propagates back to the caller where an exception should be thrown instead.






[CLJ-1978] recursion-limit not respected Created: 08/Jul/16  Updated: 19/Oct/16

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

Type: Defect Priority: Major
Reporter: Maarten Truyens Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: generator, spec
Environment:

1.9.0-alpha11


Approval: Triaged

 Description   

(Also see closed http://dev.clojure.org/jira/browse/CLJ-1964)

(require '[clojure.spec :as s])
(s/def ::map-tree (s/map-of keyword? (s/or :tree ::map-tree :leaf nil?)))
(s/exercise ::map-tree)

hangs on my machine.

Another example from https://groups.google.com/forum/#!topic/clojure/IvKJc8dEhts, which immediately results in a StackOverflowError on my machine:

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

(defrecord Tree [name children])
(defrecord Leaf [name])

(s/def ::name string?)
(s/def ::children (s/coll-of (s/or :tree ::Tree, :leaf ::Leaf)))

(s/def ::Leaf (s/with-gen
                (s/keys :req-un [::name])
                #(gen/fmap (fn [name] (->Leaf name)) (s/gen ::name))))

(s/def ::Tree (s/with-gen
                (s/keys :req-un [::name ::children])
                #(gen/fmap
                   (fn [[name children]] (->Tree name children))
                   (s/gen (s/tuple ::name ::children)))))

;; occasionally generates but usually StackOverflow
(binding [s/*recursion-limit* 1]
    (gen/generate (s/gen ::Tree)))

StackOverflowError 
	clojure.lang.RT.seqFrom (RT.java:533)
	clojure.lang.RT.seq (RT.java:527)
	clojure.core/seq--6221 (core.clj:137)
	clojure.core/map/fn--6687 (core.clj:2736)
	clojure.lang.LazySeq.sval (LazySeq.java:40)
	clojure.lang.LazySeq.seq (LazySeq.java:49)
	clojure.lang.RT.seq (RT.java:525)
	clojure.core/seq--6221 (core.clj:137)
	clojure.core/every? (core.clj:2652)
	clojure.spec/tuple-impl/reify--13509 (spec.clj:905)
	clojure.spec/gensub (spec.clj:228)
	clojure.spec/gen (spec.clj:234)


 Comments   
Comment by Leon Grapenthin [ 12/Jul/16 1:03 PM ]

As the author of CLJ-1964 I can't confirm this.

(binding [s/*recursion-limit* 1]
  (s/exercise ::map-tree))

... immediately generates.

Using the new :gen-max argument spec can also generate with a higher recursion limit in reasonable time

(s/def ::map-tree (s/map-of keyword? (s/or :tree ::map-tree :leaf nil?)
                            :gen-max 3))
(time (s/exercise ::map-tree))
"Elapsed time: 0.135683 msecs"

Note that :gen-max defaults to 20, so with 4 recursion steps this quickly ends up generating 20^5 3.2 million values

Comment by Alex Miller [ 26/Aug/16 11:31 AM ]

I tried this again today and the first example still works just fine for me. I'm using Java 1.8 with default settings in a basic Clojure repl (not lein).

Comment by Maarten Truyens [ 19/Oct/16 4:32 AM ]

With the :gen-max option, everything works now. Thanks for the suggestion!





[CLJ-1977] Printing a Throwable throws if Throwable has no cause / stacktrace Created: 06/Jul/16  Updated: 08/Jul/16  Resolved: 08/Jul/16

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

Type: Defect Priority: Major
Reporter: Leon Grapenthin Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: regression
Environment:

alpha9


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

 Description   

Throwable->map in core_print.clj doesn't handle Throwable.getCause returning null in L463. This results in a NPE in StrackTraceElement->vec in the same file in some cases, so printing a stacktrace results in a new exception being thrown which is a bit confusing.

Repro:

(def t (Throwable.))
(.setStackTrace t (into-array StackTraceElement []))
(Throwable->map t) ;; throws npe during conversion
(pr t) ;; throws during printing

Approach: Check that at least one StackTraceElement exists before using the top frame. Make printing tolerant of a missing :at value. Add test for this omitted stack trace case.

Patch: clj-1977.patch






[CLJ-1976] using spec/fspec seems to require clojure.test.check Created: 05/Jul/16  Updated: 05/Jul/16  Resolved: 05/Jul/16

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

Type: Defect Priority: Major
Reporter: Nikita Prokopov Assignee: Unassigned
Resolution: Duplicate Votes: 0
Labels: spec
Environment:

clojure 1.9.0-alpha8



 Description   
(spec/def ::translate
  (spec/fspec
    :args (spec/cat :locale keyword?
                    :key keyword?
                    :args (spec/* ::spec/any))
    :ret  string?))

(defn tr [l k & args] ...)

(spec/conform ::translate tr)

Uncaught exception, not in assertion.
expected: nil
  actual: java.io.FileNotFoundException: Could not locate clojure/test/check/generators__init.class or clojure/test/check/generators.clj on classpath.


 Comments   
Comment by Alex Miller [ 05/Jul/16 10:52 AM ]

Dupe of CLJ-1936 I believe.





[CLJ-1975] clojure.spec attempts to make `empty` records Created: 05/Jul/16  Updated: 23/Sep/16

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

Type: Defect Priority: Major
Reporter: Chas Emerick Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: spec
Environment:

1.9.0-alpha11


Approval: Triaged

 Description   
user> (require '[clojure.spec :as s])
nil
user> (defrecord Box [a])
user.Box
user> 
user> (s/conform
        (s/cat :boxes (s/* #(instance? Box %))
               :name (s/coll-of integer?))
        [(Box. 0) [5]])
UnsupportedOperationException Can't create empty: user.Box  user.Box (form-init8049111656025227309.clj:1)
user> (clojure.repl/pst *e)
UnsupportedOperationException Can't create empty: user.Box
       	user.Box (NO_SOURCE_FILE:2)
	clojure.core/empty (core.clj:5151)
	clojure.spec/every-impl/cfns--14008/fn--14014 (spec.clj:1215)
	clojure.spec/every-impl/reify--14027 (spec.clj:1229)
	clojure.spec/conform (spec.clj:150)
	clojure.spec/dt (spec.clj:731)
	clojure.spec/dt (spec.clj:727)
	clojure.spec/deriv (spec.clj:1456)
	clojure.spec/deriv (spec.clj:1463)
	clojure.spec/deriv (spec.clj:1467)
	clojure.spec/re-conform (spec.clj:1589)
	clojure.spec/regex-spec-impl/reify--14267 (spec.clj:1633)

This is a regression from -alpha7; the same sort of spec (modulo the default-value arg to `coll-of`) works as expected there.






[CLJ-1974] Clojure.org URLs in docstrings are broken Created: 03/Jul/16  Updated: 04/Jul/16  Resolved: 04/Jul/16

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

Type: Defect Priority: Major
Reporter: Brandon Bloom Assignee: Unassigned
Resolution: Completed Votes: