<< Back to previous view

[CLJ-2315] inconsistent use of number as namespace in namespaced maps Created: 22/Jan/18  Updated: 22/Jan/18

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

Type: Defect Priority: Major
Reporter: Chenyu Li Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

;; use number as namespace of a map's keyword is fine and outputs namespaced map
user=> {:3/a 1}
#:3{:a 1}

;; but the same namespaced map reader macro will not be accepted
user=> #:3{:a 1}
RuntimeException Namespaced map must specify a valid namespace: 3 clojure.lang.Util.runtimeException (Util.java:221)



 Comments   
Comment by Andy Fingerhut [ 22/Jan/18 6:47 PM ]

I don't believe Clojure ever documented that it would support keywords like :3/a

I believe the reason that it does not cause an error is an explicit effort to avoid breaking some Clojure programs that came to rely upon this undocumented / unpromised behavior. See comments on ticket CLJ-1252. For officially supported characters in symbols and keywords, see the Symbols and Literals sections of this page: https://clojure.org/reference/reader

Disclaimer: I am not a decision maker in these matters. I am only commenting to provide some background.

I would be somewhat surprised if a decision was made to support "#:3{:a 1}" in the Clojure reader.





[CLJ-2314] String array hinting in :gen-class fails spec - No other method available Created: 22/Jan/18  Updated: 22/Jan/18

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

Type: Defect Priority: Major
Reporter: Mark Bastian Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: core.specs

Approval: Vetted

 Description   

When defining methods via :gen-class in the ns declaration, array return types do not appear to have a method of specification as they used to. Formerly, a String like "[Ljava.lang.Object;" could be used to define a typed array return.

A complete example can be made by creating the following ns then adding `:aot [sandbox.ltoa]` to the project.clj file:

(ns sandbox.ltoa
  (:gen-class
    :name sandbox.LTOA
    :methods [^:static [longToArrayOfLongs [Long] "[Ljava.lang.Long;"]]))

(defn -longToArrayOfLongs [id]
  (into-array [id]))

For reference, examples that formerly worked can be found at:

Per @alexmiller, the problem can be found at https://github.com/clojure/core.specs.alpha/blob/master/src/main/clojure/clojure/core/specs/alpha.clj#L184.






[CLJ-2312] Avoid using keywords as sentinel values in transducers Created: 12/Jan/18  Updated: 12/Jan/18

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

Type: Defect Priority: Minor
Reporter: Rick Moynihan Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: security, transducers
Environment:

All environments



 Description   

The use of keywords as sentinels in transducers could in rare circumstances expose some applications to bugs and potential security risks.

(sequence (partition-by keyword) ["1" "none" "2" "clojure.core/none" "3" "4"])
;(["1"] ["none"] ["2"] ["clojure.core/none" "3"] ["4"])

Ideally a private or local value that cannot be injected into the functions domain should be used instead, e.g. (Object.).



 Comments   
Comment by Rick Moynihan [ 12/Jan/18 6:24 AM ]

Apologies for messing up the formatting on the snippet, I also didn't mean to assign this as major. Though I can't seem to edit it, perhaps someone else can.

Comment by Rick Moynihan [ 12/Jan/18 6:39 AM ]

Also thanks to @reborg and @bronsa on #clojure-uk for sharing the above snippet.

Comment by Alex Miller [ 12/Jan/18 7:53 AM ]

Rick - I added you to the necessary groups for edit privileges.

How is this a security risk?

Comment by Rick Moynihan [ 12/Jan/18 7:49 PM ]

Thanks for upping my privileges Alex.

I didn't mean to over egg the pudding, but perhaps I ended up doing it all the same!

My reasoning was that if you're using a transducer to map/reduce over values taken from user input, then an attacker could cause a loop to abort earlier than expected; which might then have other consequences. I did't have a specific attack vector in mind, but I'd seen also the sentinel ::halt and had imagined that a user could inject "clojure.core/halt" into a HTTP header. If ring has a middleware to keywordize keys then you have the sentinel value for a transducer injected by a user, and that sentinel could then potentially be used to shortcut the validation/encoding/escaping etc of other fields.

However I've looked a bit more at the implementation of halt-when and other transducers and it seems the key isn't ever mixed with user data; and that ::none is used to indicate the first pass over by some transducer functions. So it looks like I was mistaken. I doubt now that this is worth resolving.





[CLJ-2311] Spec generator override won't work on multi-spec dispatch key Created: 12/Jan/18  Updated: 12/Jan/18

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

Type: Defect Priority: Minor
Reporter: Andreas Liljeqvist Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: generator, spec
Environment:

[org.clojure/clojure "1.9.0"]
[org.clojure/spec.alpha "0.1.143"]


Attachments: File multi_spec_bug.clj    
Approval: Triaged

 Description   

Overriding generator is not used in the final result for multi-spec dispatch key.

Complete example is attached.
Of particular interest is that the an invalid custom generator can be shown to introduce a failure in validation.
The default generator is seen in the exercised data when using a valid generator.






[CLJ-2309] Readable keyword validation Created: 05/Jan/18  Updated: 08/Jan/18

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

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

Approval: Triaged

 Description   

A common question is why it is possible to programatically create keyword instances that cannot be read (https://clojure.org/guides/faq#unreadable_keywords).

a) There are many places where programmatic keywords are perfectly valid
b) It is undesirable to validate all keywords on construction due to the perf hit

However, there are use cases for creating keywords that you expect to be safe for pr/read roundtripping and in those cases it would be useful to have either a variant of `keyword` that was more restrictive and would throw on invalid keywords OR a predicate that could tell you whether a string would be a readable keyword.



 Comments   
Comment by Phill Wolf [ 08/Jan/18 7:56 PM ]

1. Keywords are too often prepared far away from the eventual pr.

2. It is impressive how seldom deviant keywords are a problem this way. The fix should be proportionate to the problem.

3. Irreadable keywords are explicitly not the problem. The problem belongs to pr and read. The immediate problem is that pr silently fails to print readably while print-readably.

3b. This problem with pr/read might be temporary.

4. Adding a special predicate for readable keywords presupposes that you can predict whether someone will call later pr. I think such a prediction would usually be baseless without mingling concerns.

5. pr could increment irreadable-forms-printed when emitting a non-readable keyword while print-readably. I/O is expensive anyway.

6. If pr/read someday gained the ability to round-trip all keywords, pr would still have to test a keyword to choose an output format for it. Thus a check installed today could be a first step, not a temporary measure.

7. By comparison, a program littered with "readable-keyword?" predicates would stay littered forever – and the predicate check might not even be necessary anymore.

8. Would "readable-keyword?" match the reader's behavior, the reader's documentation, EDN, or some conservative common subset? related: CLJ-1527 "Clarify and align valid symbol and keyword rules for Clojure (and edn)", CLJ-1530 "Make foo/bar/baz unreadable".

9. Would every spec writer agonize about whether to spec a keyword as "keyword?" or "readable-keyword?" – and then always choose "readable-keyword?" just in case someone might ever someday eventually decide to serialize data?.. thereby cluttering the program and obscuring concerns.

10. Who would test irreadable-forms-printed? An "apologetic", non-solution resolution could get quite complicated.

11. Perhaps the cure would be worse than the disease. Why not improve pr/read instead: eliminate the problem instead of adding features to work around it?

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

The problem here is specifically about keywords created from either user input or other data outside your control (arbitrary keys from json input for example). When you need to verify this property you are accepting inputs, converting them to keywords, and then later expect to print that data out. I have talked through this with many many people and when people ask about it, they know they are in this situation. Having printing fail or report way down the line is not helpful. The problem is avoiding the creation of the non-roundtrippable data in the first place (and either not accepting it or escaping it at that point).

That said, another possible solution is to add an escaping mechanisms for literal symbols and keywords. We've done some design work on this in the past and ended up shelving it at the time but that's still another possible option.

This is not a high priority issue right now, but I felt it was useful to leave this ticket here to capture the idea.





[CLJ-2305] float casting not found in map Created: 03/Jan/18  Updated: 03/Jan/18

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

Type: Defect Priority: Major
Reporter: Asher Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: casting, float


 Description   

When getting the value of a floated number in a map, the value isn't found if the map has more than 8 values.

For example:

(get {1.0 "a" 2.0 "b" 3.0 "c" 4.0 "d" 5.0 "e" 6.0 "f" 7.0 "g" 8.0 "h" 1 "i"} (float 1)) => nil
(get {1.0 "a" 2.0 "b" 3.0 "c" 4.0 "d" 5.0 "e" 6.0 "f" 7.0 "g" 8.0 "h"} (float 1)) => "a"



 Comments   
Comment by Asher [ 03/Jan/18 3:13 AM ]

The problem isn't only with maps.

Here is an example with a Set:
(contains? #{1.0 1.1} (float 1)) => true
(contains? #{1.0 1.1 1.2} (float 1)) => false

Comment by Andy Fingerhut [ 03/Jan/18 4:03 AM ]

I believe that the fact that get lookup of a float in a map with at most 8 keys is an accident of the implementation.

The reason why it fails with larger maps and hash sets is that the hash value is different for (double 1.0) and (float 1.0).

user=> (hash (float 1.0))
1065353216
user=> (hash 1.0)
1072693248

All of the values like 1.0 and others in your example default to type double. Note that if you force the keys of a map to be float, then lookup succeeds, even if the map has more than 8 keys:

user=> (get {(float 1.0) "a" 2.0 "b" 3.0 "c" 4.0 "d" 5.0 "e" 6.0 "f" 7.0 "g" 8.0 "h" 1 "i"} (float 1)) 
"a"

My guess is that except for the accident that get with a small map happens to return a successful lookup, everything here is working as designed.

I believe the reason that get with a small map does find a match for looking up (float 1) is probably because the implementation of small maps is as an ArrayMap, where lookup keys are sequentially compared against all keys in an array using clojure.core/= (or its Java equivalent), and:

user=> (= (float 1) (double 1))
true
Comment by Andy Fingerhut [ 03/Jan/18 4:10 AM ]

Additional note: CLJ-1649 was created by someone hoping that floats and doubles should have hash and = consistent with each other. You can read more about it on that ticket's description. No judgment has been made whether such a change will ever be made in Clojure, but if one of Rich's comments on ticket CLJ-1036 is any indication, it seems unlikely to change.

General suggestion: Don't mix floats and doubles in Clojure code if you can possibly avoid it.

Comment by Asher [ 03/Jan/18 4:20 AM ]

Thank you @Andy!





[CLJ-2304] CLONE - spec passing nil as second argument to `ExceptionInfo` constructor... Created: 02/Jan/18  Updated: 02/Jan/18

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

Type: Defect Priority: Major
Reporter: Jonathan Leonard Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: bug, spec
Environment:

Mac OS X 10.13.2

jonathan$ java -version
java version "1.8.0_131"
Java(TM) SE Runtime Environment (build 1.8.0_131-b11)
Java HotSpot(TM) 64-Bit Server VM (build 25.131-b11, mixed mode)


Attachments: Text File better-messaging.patch    

 Description   

Exception thrown from this line:
https://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/ExceptionInfo.java#L31

Due to this call:
https://github.com/clojure/spec.alpha/blob/master/src/main/clojure/clojure/spec/test/alpha.clj#L279

if `when-not` returns nil, `ExceptionInfo` throws exception on line 31.

A naive fix may be:
(apply ex-info (remove nil? ["Specification-based check failed" (when-not ...)]))

although that is really just masking over the issue and provides no actionable info to the sufferer of the failure.

Unfortunately I have no minimal test case as this is proprietary (and complicated code).

Perhaps the author from reading the code will have more insight.

Here's the full stack trace:

[[clojure.lang.ExceptionInfo <init> "ExceptionInfo.java" 31]
[clojure.lang.ExceptionInfo <init> "ExceptionInfo.java" 22]
[clojure.core$ex_info invokeStatic "core.clj" 4739]
[clojure.core$ex_info invoke "core.clj" 4739]
[clojure.spec.test.alpha$explain_check invokeStatic "alpha.clj" 277]
[clojure.spec.test.alpha$explain_check invoke "alpha.clj" 275]
[clojure.spec.test.alpha$check_call invokeStatic "alpha.clj" 295]
[clojure.spec.test.alpha$check_call invoke "alpha.clj" 285]
[clojure.spec.test.alpha$quick_check$fn__2986 invoke "alpha.clj" 308]
[clojure.lang.AFn applyToHelper "AFn.java" 154]
[clojure.lang.AFn applyTo "AFn.java" 144]
[clojure.core$apply invokeStatic "core.clj" 657]
[clojure.core$apply invoke "core.clj" 652]
[clojure.test.check.properties$apply_gen$fn__16139$fn__16140 invoke "properties.cljc" 30]
[clojure.test.check.properties$apply_gen$fn__16139 invoke "properties.cljc" 29]
[clojure.test.check.rose_tree$fmap invokeStatic "rose_tree.cljc" 77]
[clojure.test.check.rose_tree$fmap invoke "rose_tree.cljc" 73]
[clojure.test.check.generators$fmap$fn__9199 invoke "generators.cljc" 101]
[clojure.test.check.generators$gen_fmap$fn__9173 invoke "generators.cljc" 57]
[clojure.test.check.generators$call_gen invokeStatic "generators.cljc" 41]
[clojure.test.check.generators$call_gen invoke "generators.cljc" 37]
[clojure.test.check$quick_check invokeStatic "check.cljc" 94]
[clojure.test.check$quick_check doInvoke "check.cljc" 37]
[clojure.lang.RestFn invoke "RestFn.java" 425]
[clojure.lang.AFn applyToHelper "AFn.java" 156]
[clojure.lang.RestFn applyTo "RestFn.java" 132]
[clojure.core$apply invokeStatic "core.clj" 657]
[clojure.core$apply invoke "core.clj" 652]
[clojure.spec.gen.alpha$quick_check invokeStatic "alpha.clj" 29]
[clojure.spec.gen.alpha$quick_check doInvoke "alpha.clj" 27]
[clojure.lang.RestFn applyTo "RestFn.java" 137]
[clojure.core$apply invokeStatic "core.clj" 661]
[clojure.core$apply invoke "core.clj" 652]
[clojure.spec.test.alpha$quick_check invokeStatic "alpha.clj" 309]
[clojure.spec.test.alpha$quick_check invoke "alpha.clj" 302]
[clojure.spec.test.alpha$check_1 invokeStatic "alpha.clj" 335]
[clojure.spec.test.alpha$check_1 invoke "alpha.clj" 323]
[clojure.spec.test.alpha$check$fn__3005 invoke "alpha.clj" 411]
[clojure.core$pmap$fn__8105$fn__8106 invoke "core.clj" 6942]
[clojure.core$binding_conveyor_fn$fn__5476 invoke "core.clj" 2022]
[clojure.lang.AFn call "AFn.java" 18]
[java.util.concurrent.FutureTask run "FutureTask.java" 266]
[java.util.concurrent.ThreadPoolExecutor runWorker "ThreadPoolExecutor.java" 1149]
[java.util.concurrent.ThreadPoolExecutor$Worker run "ThreadPoolExecutor.java" 624]
[java.lang.Thread run "Thread.java" 748]]



 Comments   
Comment by Jonathan Leonard [ 02/Jan/18 12:47 PM ]

So, I was able to finally track this down to an actual problem where my function value returned from the function under test would throw and exception on a particular input.

This patch helps point people in that direction (although an even better one would be to surface/explain the underlying failure rather than just allude to it).





[CLJ-2303] Reified object doesn't satisfy protocol at compile time Created: 29/Dec/17  Updated: 02/Jan/18

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

Type: Defect Priority: Major
Reporter: Michael Blume Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

https://github.com/MichaelBlume/reify-fail

(defprotocol Foo
  (hello [this] "Says hello"))

(def bar
  (reify Foo
    (hello [this] "hello, world")))

(when-not (satisfies? Foo bar)
  (throw (Exception. "bar doesn't satisfy Foo")))

Notes:

Project is AOT
Another namespace requires this namespace. That namespace is compiled first. Without that namespace the problem goes away.



 Comments   
Comment by Kevin Downey [ 01/Jan/18 5:15 PM ]

what steps do you take after checking out the repo to reproduce this issue?

I cloned the repo and ran `lein compile` and didn't get any errors.

Comment by Michael Blume [ 01/Jan/18 10:56 PM ]

I was reproducing the issue with `lein check`, but `lein compile` also fails for me, so I'm confused.

Comment by Michael Blume [ 01/Jan/18 10:56 PM ]

I think it's important that the near-empty namespace compiles first, maybe there are platform differences in compilation order?

Comment by Michael Blume [ 01/Jan/18 10:57 PM ]

test repo fails for me under Mac OS and Ubuntu, both with Java 8

Comment by Michael Blume [ 01/Jan/18 10:58 PM ]

Under Ubuntu I have

$ lein -v
Leiningen 2.7.1 on Java 1.8.0_151 OpenJDK 64-Bit Server VM

Comment by Andy Fingerhut [ 02/Jan/18 12:19 AM ]

On both of these OS’s: Ubuntu 16.04.3 Linux, macOS 10.12.6

With both of these versions of Leiningen: 2.7.1, 2.8.1

I believe all of my experiments were using a recent version of JDK 1.8

With any of these Clojure versions: 1.7.0 1.8.0 1.9.0

I get an error with both the commands 'lein check' and 'lein compile' with the project https://github.com/MichaelBlume/reify-fail, but only if I do '/bin/rm -fr target' first (or run it before the target directory has ever been created, e.g. immediately after running the ‘git clone’ command for the repository).

If I run the command twice, the first command creates many .class files in the target directory, and the second such command gives no error.

Comment by Kevin Downey [ 02/Jan/18 12:36 AM ]

I am able to reproduce there error if I replace `:aot :all` with `:aot [com.lendup.citadel.pipeline com.lendup.citadel.providers.mocky]`. I suspect lein could fix this by doing their stale ns check between compiling namespaces to avoid repeated compilation of transitive namespaces.

Comment by Kevin Downey [ 02/Jan/18 12:41 AM ]

could be related to https://github.com/technomancy/leiningen/issues/2316





[CLJ-2302] s/merge fails with plain predicate Created: 29/Dec/17  Updated: 29/Dec/17

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

Type: Defect Priority: Major
Reporter: Tommi Reiman Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

[org.clojure/spec.alpha "0.1.143"]



 Description   

I would expect a plain predicate work with s/merge:

(s/explain any? {:x 1, :y 2})
; Success!

(s/explain (s/merge any?) {:x 1, :y 2})
; val: {:x 1, :y 2} fails predicate: any?

Still, wrapping the predicate into specs seems to work:

(s/explain (s/merge (s/spec any?)) {:x 1, :y 2})
; Success!





[CLJ-2301] 'ensure' can cause livelock Created: 26/Dec/17  Updated: 26/Dec/17

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

Type: Defect Priority: Minor
Reporter: David Bürgin Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

The following program demonstrates livelock when using ensure.

(let [cats (ref 1)
      dogs (ref 1)
      events (atom [])
      john (future
             (dosync
               (swap! events conj :j1)
               (when (< (+ @cats (ensure dogs)) 3)
                 (swap! events conj :j2)
                 (ref-set cats 2))
               (swap! events conj :j3)))
      mary (future
             (dosync
               (swap! events conj :m1)
               (when (< (+ (ensure cats) @dogs) 3)
                 (swap! events conj :m2)
                 (ref-set dogs 2))
               (swap! events conj :m3)))]
  @john @mary @events)
; => [:m1 :j1 :m2 :j2 :j1 :m1 :j2 :m2 :j1 :m1 :j2 :m2 :m1 :j1 :j2 :m2 :m1 :j1 :j2 :m2 :m1 :j1 :m2 :j2 :m1 :j1 :j2 :m2 :j1 :m1 :j2 :m2 :m1 :j1 :m2 :j2 :j1 :m1 :m2 :j2 :m1 :j1 :j2 :m2 :m1 :j1 :m2 :j2 :m1 :j1 :j2 :m2 :m1 :j1 :j2 :m2 :m1 :j1 :j2 :m2 :j1 :m1 :m2 :j2 :m1 :j1 :j2 :m2 :m1 :j1 :m2 :j2 :m1 :j1 :m2 :j2 :j1 :m1 :j2 :m2 :m1 :j1 :m2 :j2 :j1 :m1 :m2 :j2 :m1 :j1 :j2 :m2 :m1 :j1 :m2 :j2 :m1 :m2 :j1 :m3 :j1 :j3]

This program sometimes terminates very quickly, but sometimes there is a delay of several seconds as the transactions are retried dozens or hundreds of times.

(ensure ref) can exhibit livelock in a way that the equivalent (ref-set ref @ref) cannot, because only the latter supports barging. With ref-set older transactions will ultimately win thanks to barging. With ensure there is no barging as both transactions try to get the write lock (ref-set) of the ref whose read lock is held by the other transaction (ensure). Both transactions sit idle for the full 100ms timeout and will then simultaneously retry.

This could be just a doc enhancement. The sentence 'Allows for more concurrency than (ref-set ref @ref)' may not be true generally (and is anyway too vague to be much help). (ref-set ref @ref) is safer than (ensure ref).



 Comments   
Comment by David Bürgin [ 26/Dec/17 4:43 AM ]

From an old mailing list discussion here and here.





[CLJ-2298] Calling static interface methods in Java 9 causes error Created: 21/Dec/17  Updated: 21/Dec/17

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

Type: Defect Priority: Major
Reporter: Piotr Bzdyl Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: compiler, java9
Environment:

Java 9.0.1+11
Clojure 1.9.0



 Description   

The byte code generated for calls to a static interface method when running with Java 9 fails with:

{{java.lang.IncompatibleClassChangeError: Method [.....]()L[......]; must be InterfaceMethodref constant}}

It can be recreated with:

public interface Foo {
  public static String bar() {
    return "test";
  }
}

javac Foo.java

And then calling the method from Clojure namespace results in:

java -cp clojure-1.9.0.jar:spec.alpha-0.1.143.jar:spec.alpha-0.1.143.jar:. clojure.main -e (Foo/bar)

Exception in thread "main" java.lang.IncompatibleClassChangeError: Method Foo.bar()Ljava/lang/String; must be InterfaceMethodref constant
	at user$eval11.invokeStatic(NO_SOURCE_FILE:1)
	at user$eval11.invoke(NO_SOURCE_FILE:1)
	at clojure.lang.Compiler.eval(Compiler.java:7062)
	at clojure.lang.Compiler.eval(Compiler.java:7025)
	at clojure.core$eval.invokeStatic(core.clj:3206)
	at clojure.main$eval_opt.invokeStatic(main.clj:291)
	at clojure.main$eval_opt.invoke(main.clj:285)
	at clojure.main$initialize.invokeStatic(main.clj:311)
	at clojure.main$null_opt.invokeStatic(main.clj:345)
	at clojure.main$null_opt.invoke(main.clj:342)
	at clojure.main$main.invokeStatic(main.clj:424)
	at clojure.main$main.doInvoke(main.clj:387)
	at clojure.lang.RestFn.applyTo(RestFn.java:137)
	at clojure.lang.Var.applyTo(Var.java:702)
	at clojure.main.main(main.java:37)

I think it might be related to an issue in asm that has been already fixed but I didn't have enough time to verify this.



 Comments   
Comment by David Bürgin [ 21/Dec/17 12:53 PM ]

Duplicate of CLJ-2284





[CLJ-2297] PersistentHashMap leaks memory when keys are removed with `without` Created: 20/Dec/17  Updated: 20/Dec/17

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

Type: Defect Priority: Critical
Reporter: Ben Bader Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: collections

Attachments: File demo-phm-leak.clj     Text File fix-bitmapnode-without.patch    
Approval: Triaged

 Description   

The problem is in `PersistentHashMap.BitmapNode#without(Object)`; when the last value is removed, an empty BitmapNode is returned instead of null. This has knock-on effects in large maps that have interior ArrayNodes, which themselves are not preserved.

Attached is a test script that demonstrates the issue, by measuring heap usage, adding a large number of entries to a map, removing those keys one-by-one, then comparing heap usage afterwards. The output, shows the average amount of heap allocated for an empty map that has had 100,000 entries added then removed:

❯ java -jar clojure.jar demo-phm-leak.clj
Avg. heap:8656352/5 bytes
Avg. heap:8656632/5 bytes
Avg. heap:8654664/5 bytes
Avg. heap:8656884/5 bytes
Avg. heap:1731156 bytes

This was from my a Macbook Pro, running a self-made Clojure built from master, using java 1.8.0_65. The variable sizes are due to the fact that the shape of the PHM tree depends on the insertion order of keys, which is randomized in this script.



 Comments   
Comment by Ben Bader [ 20/Dec/17 1:33 PM ]

This patch fixes the leak by changing `BitmapNode#without(Object)` such that, when there are no other values in the node, the method returns `null` instead of an empty BitmapNode.

Output from the demo script, from a Clojure build incorporating the patch:

~/Development/clojure collapse-empty-mapnodes* 16s
❯ java -jar clojure.jar demo-phm-leak.clj
Avg. heap:64 bytes
Avg. heap:64 bytes
Avg. heap:64 bytes
Avg. heap:64 bytes
Avg. heap:64 bytes

Comment by Ben Bader [ 20/Dec/17 1:54 PM ]

Note that this patch doesn't have a similar change for the transient overload of `without`; because it relies on a helper method `editAndRemovePair` that correctly handles the empty case, that method doesn't have this bug.

Comment by Alex Miller [ 20/Dec/17 2:17 PM ]

Thanks for the report!

Comment by Alex Miller [ 20/Dec/17 2:23 PM ]

I assume this came out of some actual usage with surprising behavior?

Comment by Ben Bader [ 20/Dec/17 4:44 PM ]

It came up when I was profiling a service that uses maps in agents to cache things; overall memory usage seemed high, and I got curious. Hard to say whether this would ever noticeably impact a production system, given that this behavior has been in place for nine years!

Comment by Andy Fingerhut [ 20/Dec/17 7:34 PM ]

Cool find. It looks like an average of 1.7 bytes of unreclaimed space per 'without' operation in your test case, so nice to improve upon, but perhaps not something people would notice due to it causing problems very often.





[CLJ-2295] clojure.repl/doc repeats doc string in output for special forms in Clojure 1.9.0 Created: 15/Dec/17  Updated: 15/Dec/17

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

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

Attachments: Text File CLJ-2295-v1.patch    

 Description   

Here is output from Clojure 1.8.0:

user=> (doc if)
-------------------------
if
  (if test then else?)
Special Form
  Evaluates test. If not the singular values nil or false,
  evaluates and yields then, otherwise, evaluates and yields else. If
  else is not supplied it defaults to nil.

  Please see http://clojure.org/special_forms#if
nil

Here is the corresponding output from Clojure 1.9.0:

user=> (doc if)
-------------------------
if
  (if test then else?)
Special Form
  Evaluates test. If not the singular values nil or false,
  evaluates and yields then, otherwise, evaluates and yields else. If
  else is not supplied it defaults to nil.

  Please see http://clojure.org/special_forms#if
  Evaluates test. If not the singular values nil or false,
  evaluates and yields then, otherwise, evaluates and yields else. If
  else is not supplied it defaults to nil.
nil

This repetition only occurs when calling clojure.repl/doc or clojure.repl/print-doc for special form symbols, not for other symbols like macros and functions. It was introduced when modifying clojure.repl/print-doc when adding the ability to print specs in its output, and the fix is straightforward.



 Comments   
Comment by Andy Fingerhut [ 15/Dec/17 6:20 PM ]

Patch CLJ-2295-v1.patch dated 2017-Dec-15 is one possible way to fix this issue. Verified that output for other cases, e.g. macros, is unchanged from Clojure 1.8.0 with these changes, except for the new Spec output, which should be kept.





[CLJ-2294] Allow user to get forms of multi-spec implementations Created: 14/Dec/17  Updated: 14/Dec/17

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

Type: Enhancement Priority: Major
Reporter: Ben Brinckerhoff Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: spec
Environment:

org.clojure/spec.alpha "0.1.143"



 Description   

As noted in this thread (https://groups.google.com/forum/#!topic/clojure/i8Rz-AnCoa8),

s/keys
does not validate that the specs mentioned within it are valid specs.

A suggested workaround is to separately write code that could inspect the registry and warn about specs that are not declared. A sketch of this code was provided at https://gist.github.com/stuarthalloway/f4c4297d344651c99827769e1c3d34e9

While the general approach is a good one, I believe it won't work for multi-specs that contain "keys" specs.

s/form
on a multi-spec will only return that the spec is a multi-spec (which makes sense), but there is no way for a client to get access to code that picks the approach spec given data.

A further workaround is provided in the comments to that gist, but since it relies on

resolve
, it will not work on Clojurescript.

I think

s/form
is behaving correctly in this case, but it would be useful to have an additional function that, given a multi-spec, could return a function or data that could be used to explore the current implementations.






[CLJ-2291] <! and >! break clojure.test/is Created: 13/Dec/17  Updated: 13/Dec/17

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

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


 Description   

`(is (not (<! ...)))` works, but `(is (<! ...))` doesn't.

Under the hood, `is` rewrites the code in such a way that it calls `(apply <!
...)`, because `is` assumes that <! is a plain function (see: `assert-expr
:default`[1], `assert-predicate`[2] in clojure.test). It triggers the ">! used
not in (go ...) block" assertion.

[1]https://github.com/clojure/clojure/blob/2e0c0a9a89ede8221504edeb90e8c4ee6cce7e16/src/clj/clojure/test.clj#L486
[2]https://github.com/clojure/clojure/blob/2e0c0a9a89ede8221504edeb90e8c4ee6cce7e16/src/clj/clojure/test.clj#L435

Example code: https://gist.github.com/augustl/4a679dc95847db4434d0e7348651224f#file-test-cljs-L34
Macroexpansion: https://gist.github.com/amalloy/26ec8b8910c7c00bd7feaeef2307bc92#file-gistfile1-txt-L48

Workaround: make `assert-expr` aware of special core.async functions, and use
`assert-any` for those. It's reasonable since core.async is a widely-used core
library. It'd fix the particular problem described in this issue.

The correct solution would be to make <! and >! macros instead of functions.






[CLJ-2290] into docstring doesn't mention 0- and 1-arity versions Created: 13/Dec/17  Updated: 13/Dec/17

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

Type: Defect Priority: Trivial
Reporter: jcr Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docs, docstring, documentation

Attachments: Text File CLJ-2290.patch    

 Description   

See: https://github.com/clojure/clojure/blob/clojure-1.9.0-alpha14/src/clj/clojure/core.clj#L6763

Docstring should mention that "(into coll) returns coll" and "(into) returns an empty vector" and the fact that "hence it can be used as a reducing function", clarifying the reason to use [] as the default (the same as with `conj`).






[CLJ-2289] conj docstring and arglists don't mention 0- and 1-arity versions Created: 13/Dec/17  Updated: 13/Dec/17

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

Type: Defect Priority: Trivial
Reporter: jcr Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docs, docstring, documentation

Attachments: Text File CLJ-2298.patch    

 Description   

See: https://github.com/clojure/clojure/blob/clojure-1.9.0-alpha14/src/clj/clojure/core.clj#L76

Arglists should be '([] [coll] [coll x] [coll x & xs])

Docstring should mention "(conj coll) returns coll" and "(conj) returns an empty vector", and also the fact that "hence it can be used as a reducing function", clarifying the reason to use [] as the default.






[CLJ-2287] Add clojure.set/union, intersection, difference to core.spec.alpha Created: 12/Dec/17  Updated: 30/Dec/17

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

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

Attachments: Text File CLJ-2287-add-clojure-set-specs-v1.patch    

 Description   

Several tickets have been created suggesting that several functions in the clojure.set namespace might throw an exception if given non-sets as arguments, because in some cases they return unexpected values. This list is a sample, not intended to be complete: CLJ-810, CLJ-1682, CLJ-1953, CLJ-1954

Now that clojure.spec exists, documenting the expected argument types of these functions can be done more precisely. The specs could be used to dynamically detect incorrect argument types passed to these functions during testing, and via any other ways built on top of clojure.spec in the future.

Alex Miller suggested in a Slack discussion that perhaps it would be helpful if contributors would help implement such specs for functions within Clojure.

The approach taken with the proposed patch is simply to spec that the :args of clojure.set/subset?, superset?, union, intersection, and difference must all be sets, and spec the :ret types of the first two as boolean, and the last three as set. This seems to match all known comments given when declining previous tickets proposing changes to the behavior of these functions.

Patch: *CLJ-2287-add-clojure-set-specs-v1.patch*

I (Andy Fingerhut) looked through all of Clojure and contrib projects for occurrences of clojure.set/union, difference, intersection, superset?, and subset?, and there is only one such call that I can quickly determine does not pass sets to them. It is the one in clojure.data/diff for which there is a patch that can correct this (assuming it is considered a bug) on this ticket: CLJ-1087

Many such calls I could quickly determine always passed sets (e.g. because the arguments were wrapped in (set ...) calls). On a small fraction of them, I gave up trying to figure it out after a minute, since determining the answer fully from code inspection would require going back up the call tree a ways.

Please suggest any other ways that these changes could be evaluated in order to increase your confidence that they are correct.



 Comments   
Comment by Alex Miller [ 12/Dec/17 1:37 PM ]

For the moment I’ll say sure. I think this particular example is a good one.

I’m not sure that all the specs here are as obvious as you expect, or at least that’s been my experience elsewhere.

I’d expect these specs to be in clojure.set.specs namespace btw.

Comment by Andy Fingerhut [ 13/Dec/17 4:33 PM ]

Attached CLJ-2287-add-clojure-set-specs-v1.patch dated 2017-Dec-13. These are my first Clojure specs ever, so feel free to go crazy on suggesting improvements.

I used the namespace clojure.set.specs.alpha rather than clojure.set.specs as you suggested, but I can easily change that if you really did want it without the .alpha

Comment by Andy Fingerhut [ 13/Dec/17 4:34 PM ]

If there is some way I can exercise the :args specs with a large suite of tests, let me know how and I can try it.

And clearly I am putting in at least for :args and :ret pretty obvious choices of specs that I believe are correct, given issues raised against those functions in the past. I don't see how they could be any more subtle than that for these functions, but let me know if I'm missing anything, e.g. the Clojure core team has decided to support non-set arguments for these functions.

I agree that some of the other functions in the clojure.set namespace might be a bit trickier to spec the args of.

Comment by Andy Fingerhut [ 14/Dec/17 8:33 PM ]

I looked through all of Clojure and contrib projects for occurrences of clojure.set/union, difference, intersection, superset?, and subset?, and there is only one such call that I can quickly determine does not pass sets to them. It is the one in clojure.data/diff for which there is a patch that can 'correct' this (assuming it is considered a bug) on this ticket: https://dev.clojure.org/jira/browse/CLJ-1087

Many such calls I could quickly determine always passed sets (e.g. because the arguments were wrapped in (set ...) calls). On a small fraction of them, I gave up trying to figure it out after a minute, since determining the answer fully from code inspection would require going back up the call tree a ways.





[CLJ-2284] Incorrect bytecode generated for static methods on interfaces Created: 09/Dec/17  Updated: 24/Dec/17

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

Type: Defect Priority: Major
Reporter: Zach Tellman Assignee: Unassigned
Resolution: Unresolved Votes: 3
Labels: asm, compiler, java19
Environment:

JDK 9 or higher


Attachments: Text File 0001-unbundle-ASM.patch     Text File 0002-update-classfile-version-to-9.patch     Text File 0003-CLJ-2284.patch    
Approval: Triaged

 Description   

A reproduction of the failure can be found here: https://github.com/ztellman/java9-failure.



 Comments   
Comment by Ghadi Shayban [ 09/Dec/17 11:08 PM ]

Static or default interface method invocations are not supported when the bytecode emitted is < 1.8 bytecode (Clojure currently emits 1.6 bytecode). I'm mildly surprised this compiled.

Comment by David Bürgin [ 21/Dec/17 12:54 PM ]

Not sure Ghadi’s assertion is correct? No new bytecodes were introduced for static and default interface methods. Or at least I have been using JDK 8 utilities like CharSequence.codePoints() and Comparator.reverseOrder() in Clojure for a long time. (And if this usage were not supported today that would seem like a critical issue.)

Comment by Zach Tellman [ 22/Dec/17 9:08 PM ]

Yeah, I'd expect default interface methods to just be filled in on classes which don't provide their own implementation, and empirically they don't seem to cause any issues, but I'm not an authority on JVM bytecode.

Comment by Nicola Mometto [ 23/Dec/17 2:10 PM ]

while it is true that no bytecodes were introduced, the JVM spec mandates that invocations to static interface methods refer to an InterfaceMethodref entry in the constant pool rather than simply to a Methodref – as to why this worked in jvm 1.8 and fails with a verify error in jvm 9 , I can only assume the bytecode verifier became stricter since version 9

See https://docs.oracle.com/javase/specs/jvms/se9/html/jvms-5.html#jvms-5.4.3.3, describing the resolution rules for Methodrefs:

When resolving a method reference:

    If C is an interface, method resolution throws an IncompatibleClassChangeError.
Comment by Nicola Mometto [ 23/Dec/17 2:21 PM ]

Indeed here's the openjdk ticket tightening bytecode verification to adhere to the JVM spec: https://bugs.openjdk.java.net/browse/JDK-8145148
According to the last comment on that ticket, upgrading the ASM version provided with clojure to 5.1 should fix this

Comment by Nicola Mometto [ 24/Dec/17 12:31 PM ]

The following 3 patches work to fix this problem, I've kept them split for now so that we can be evaluated separately, but in order to fix this all three are required.

1st patch simply updates the version of ASM that clojure uses to 6.0, removes the bundled ASM and adds ASM as a separate dependency. If this is not desirable we could just update the bundled version of ASM but it seems like now that clojure requires external deps, this is up for debate.

2nd patch updates the classfile version that clojure emits from 1_5 to 9 so that ASM can emit interface method refs, because of changes in bytecode verification since v1.5, several additional changes were needed:
1- it's not possible anymore to assign final fields from methods other than <clinit>, so it was necessary to mark the const__ fields as non static as they are assigned by init__n methods
2- stack map frames are not optional anymore so we need to tell ASM to compute them for us
3- a custom ClassWriter overriding getCommonSuperClass has been implemented, using DCL to load classes and short circuiting on classes that are being defined (as per that method's javadoc)

finally the third patch addresses this issue by emitting `invokeStatic` through the new `visitmethodInsn` arity that supports interface methods

Comment by Nicola Mometto [ 24/Dec/17 12:41 PM ]

Note that I'm not proposing the 3 patches as they are as a fix for this, given that bumping classfile version means discontinuing support for running clojure on java versions earlier than what's specified, which is undesiderable (I've bumped it to V9 but what's needed here should be just V1_8, but still).

That said, if we want to fix this, we definitely need the fixes I've put in those patches and we likely also want to conditionally emit different classfile versions (e.g. defaulting to 1_5 but bumping it to a higher version for specific classfiles when needed)





[CLJ-2283] doseq should return nil with no collections Created: 09/Dec/17  Updated: 15/Dec/17

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

Type: Defect Priority: Major
Reporter: Ambrose Bonnaire-Sergeant Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: None

Attachments: Text File CLJ-2283.patch     Text File CLJ-2283-test.patch     Text File CLJ-2283-test-revised.patch    
Approval: Triaged

 Description   

According to the docstring for doseq, the following should return `nil`.

user> (doseq [] "not nil")
"not nil"


 Comments   
Comment by Michael Zavarella [ 13/Dec/17 3:47 PM ]

I think this fixes this issue.

Theres a `(do ...)` that runs if `(seq exprs)` is nil but the return of that `do` isn't necessarily nil if your body is something that doesn't return nil.

Comment by Mike Fikes [ 14/Dec/17 2:12 PM ]

It is odd that doseq even has code for this case given that the docstring indicates the bindings and filterings are as provided by "for." Since for requires one or more binding forms, it raises the question of whether (doseq [] "not nil") is a valid program.

Comment by Alex Miller [ 14/Dec/17 2:58 PM ]

Test?

Comment by Michael Zavarella [ 15/Dec/17 5:34 PM ]

Here's a bit of tests! I think these are suitable for the use case of `doseq`. I just went with similar tests to `dotimes` since they're so similar.

Comment by Michael Zavarella [ 15/Dec/17 5:36 PM ]

I removed the `println` from the original tests in favor of `identity`. It seemed more appropriate so you don't have a random 'foo' in the test print-out.





[CLJ-2282] Avoid boxing in set! primitive expressions Created: 08/Dec/17  Updated: 08/Dec/17

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

Type: Enhancement Priority: Major
Reporter: Nicola Mometto Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: compiler

Attachments: Text File 0001-CLJ-2282-don-t-unnecessarily-box-primitive-set-val.patch    

 Description   

Currently the clojure compiler always emits values to `set!` expression boxed, and unboxes them if necessary. It is possible to enhance AssignExpr to be a MaybePrimitiveExpr and avoid unnecessary box/unboxing.

Assuming this code:

public class Test {
  public int x;
}
(defn foo []
  (let [a (int 1)]
    (set! (.-x (Test.)) a)
    "asd"))

Before:

public static java.lang.Object invokeStatic();
    descriptor: ()Ljava/lang/Object;
    flags: ACC_PUBLIC, ACC_STATIC
    Code:
      stack=3, locals=1, args_size=0
         0: lconst_1
         1: invokestatic  #17                 // Method clojure/lang/RT.intCast:(J)I
         4: istore_0
         5: new           #19                 // class Test
         8: dup
         9: invokespecial #20                 // Method Test."<init>":()V
        12: checkcast     #19                 // class Test
        15: iload_0
        16: invokestatic  #26                 // Method java/lang/Integer.valueOf:(I)Ljava/lang/Integer;
        19: dup_x1
        20: checkcast     #28                 // class java/lang/Number
        23: invokestatic  #31                 // Method clojure/lang/RT.intCast:(Ljava/lang/Object;)I
        26: putfield      #35                 // Field Test.x:I
        29: pop
        30: ldc           #37                 // String asd
        32: areturn
      LocalVariableTable:
        Start  Length  Slot  Name   Signature
            5      27     0     a   I
      LineNumberTable:
        line 3: 0
        line 4: 1
        line 5: 19

After:

public static java.lang.Object invokeStatic();
    descriptor: ()Ljava/lang/Object;
    flags: ACC_PUBLIC, ACC_STATIC
    Code:
      stack=3, locals=1, args_size=0
         0: lconst_1
         1: invokestatic  #17                 // Method clojure/lang/RT.intCast:(J)I
         4: istore_0
         5: new           #19                 // class Test
         8: dup
         9: invokespecial #20                 // Method Test."<init>":()V
        12: checkcast     #19                 // class Test
        15: iload_0
        16: dup_x1
        17: putfield      #24                 // Field Test.x:I
        20: pop
        21: ldc           #26                 // String asd
        23: areturn
      LocalVariableTable:
        Start  Length  Slot  Name   Signature
            5      18     0     a   I
      LineNumberTable:
        line 3: 0
        line 4: 1
        line 5: 16

Patch: 0001-CLJ-2282-don-t-unnecessarily-box-primitive-set-val.patch






[CLJ-2281] reduce on primitive arrays doesn't use ArraySeq_TYPE.reduce Created: 03/Dec/17  Updated: 03/Dec/17

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

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


 Description   

Currently a reduce on (e.g.) long-array never ends up in the fast ArraySeq_Long.reduce function. Only if the user calls seq on it beforehand.

So:

;; Fast:
(reduce (fn [_ _]) 0 (seq (long-array [1])))
;; >2x slower:
(reduce (fn [_ _]) 0 (long-array [1]))





[CLJ-2280] Speedup mapv for multiple collections using iterators Created: 03/Dec/17  Updated: 04/Dec/17

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

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


 Description   

mapv for multiple collections currently does:

(into [] (map f c1 c2 c3))

I propose to change these to:

(let [it1 (clojure.lang.RT/iter c1)
      it2 (clojure.lang.RT/iter c2)]
     (loop [out (transient [])]
       (if (and (.hasNext it1) (.hasNext it2))
         (recur (conj! out (f (.next it1) (.next it2))))
         (persistent! out))))

This is 5x faster.

For variadic arity we can check if colls is counted?:

  • If yes: Use MultiIterator
  • If no: Forward to map like the current implementation does.

Any thoughts on this or if this could brake something?



 Comments   
Comment by Alex Miller [ 03/Dec/17 10:12 AM ]

Iterators should be treated as dangerous and unsafe by default - they are mutable, stateful, (potentially) non-thread-safe objects. Thus, their use should be carefully scrutinized and in particular you don't want them to a) leak out of a localized context (so they should be eagerly walked and released) or b) be used in a way that triggers the execution of impure functions such that they would be reexecuted later. Sequences cache their realization (and are thread-safe) which makes them slower, but much safer in avoiding this problem.

In this case, you're eagerly producing a concrete collection (not a lazy seq) so the first concern is addressed. However, the second is a little harder to reason through. I think it might be ok, but would have to think about that more.

Note that into uses transduce which uses CollReduce, which will do an iterReduce on Iterable colls, so you could probably get the same effect by simply using the transducer form:

(into [] (map f) c1)

Except that this form only takes a single coll. Multi coll support for transducers exists (see TransformerIterator/createMulti) but is not well-surfaced in the current apis. Seems like leveraging it directly would be vastly preferable to the suggestion above though:

(defn mapv2 [f & cs]
  (let [iters (map #(.iterator %) cs)
        t (clojure.lang.TransformerIterator/createMulti (map f) iters)]
    (loop [out (transient [])]
      (if (.hasNext t)
        (recur (conj! out (.next t)))
        (persistent! out)))))

Note that this assumes all cs are Iterable, which is not a valid assumption. For example: (mapv identity "abc"). So this would still need some work on fallbacks and perf testing.

Comment by A. R [ 04/Dec/17 1:08 AM ]

Alex: I actually did use exactly this very implementation but found it to be quite a bit slower than manually keeping track of the iterators. My guess is it's because of xf.applyTo inside the step function of TransformIterator. We already know source.size() when creating it in createMulti. So this implementation could probably be made smarter by directly invoking the right arity.
Though, I haven't benchmarked anything. So just a guess for now.

Comment by Alex Miller [ 04/Dec/17 8:16 AM ]

Seems like a reasonable guess and something that could be optimized in TransformIterator.

Comment by A. R [ 04/Dec/17 8:34 AM ]

I actually thought about this a bit more. I agree that the proper step here would be to properly leverage transducers and provide better support for multiple collections with transducers.

So I'd suggest changing the game plan to:

1. Make into and transduce work with multiple collections
2. Throw out MultiIterator. We can get much better performance doing this inline in TransformIterator, then we avoid the unnecessary step of "build up sequence in MultiIterator" followed by "spread out this just created sequence in apply".

Though, this would make the TransformIterator much more involved.

After this, we could give many transducers the option of working over multiple sequences (keep, filter, map-indexed, take, drop etc etc) and efficiently using them with into and transduce.

If you agree, I can change the title to reflect the new scope.

Comment by Alex Miller [ 04/Dec/17 9:05 AM ]

You are moving from "enhancing performance of an existing function" into doing api design which has a whole host of larger considerations. I think it's unlikely that we will expand either into or transduce into multi-coll territory and I don't think there are very many transducers that make sense as natively multi-coll. So, I do not think you should expand the scope of this ticket, as I would then decline it.





[CLJ-2277] Add fully-qualified specs to explain-data when map fails to contain required keys Created: 28/Nov/17  Updated: 28/Nov/17

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

Type: Enhancement Priority: Major
Reporter: Ben Brinckerhoff Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: spec
Environment:

org.clojure/spec.alpha "0.1.143"



 Description   

Consider the following spec

(s/def :example/name string?)
(s/def :example/age pos-int?)
(s/def :wine/age (s/and pos-int?
                        #(< 100)))
(s/def :example/employee (s/keys
                          :req [:example/string]
                          :req-un [:example/age]))

(s/explain-data :example/employee {})
;; #:clojure.spec.alpha{:problems ({:path [], :pred (clojure.core/fn [%] (clojure.core/contains? % :example/string)), :val {}, :via [:example/employee], :in []} {:path [], :pred (clojure.core/fn [%] (clojure.core/contains? % :age)), :val {}, :via [:example/employee], :in []}), :spec :example/employee, :value {}}

The explain-data in this case does give the precise problem: the map is required keys. But the user requires at least two actions to resolve the problem with ":age": either add some fake value for the required keys and look at the new explain-data OR guess which :age spec was intended and look up the spec.

The default "explain" message is fine in this case, but other spec printers may want to give users hints on how they can resolve this problem in one step. This is easy in the case of ":req" because the spec is fully qualified, but not possible in general for ":req-un".

A solution is to include the fully-qualified spec along with the failing predicate in the "problem" data above.






[CLJ-2274] Line numbers in stack trace are wrong when type hints satisfaction fails Created: 26/Nov/17  Updated: 27/Nov/17

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

Type: Defect Priority: Minor
Reporter: Stijn Seghers Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: errormsgs
Environment:

I only tested this on macOS 10.13.1 with both Clojure 1.8.0 and 1.9.0-RC1.


Approval: Triaged

 Description   

When I run the following file

example.clj
(defn f [^double x] x)

(defn g []
  (println)
  (f nil))

(g)

I get the following stack trace

Exception in thread "main" java.lang.NullPointerException, compiling:(/.../example.clj:7:1)
	at clojure.lang.Compiler.load(Compiler.java:7526)
	at clojure.lang.Compiler.loadFile(Compiler.java:7452)
	at clojure.main$load_script.invokeStatic(main.clj:278)
	at clojure.main$script_opt.invokeStatic(main.clj:338)
	at clojure.main$script_opt.invoke(main.clj:333)
	at clojure.main$main.invokeStatic(main.clj:424)
	at clojure.main$main.doInvoke(main.clj:387)
	at clojure.lang.RestFn.applyTo(RestFn.java:137)
	at clojure.lang.Var.applyTo(Var.java:702)
	at clojure.main.main(main.java:37)
Caused by: java.lang.NullPointerException
	at clojure.lang.RT.doubleCast(RT.java:1348)
	at user$g.invokeStatic(example.clj:4)
	at user$g.invoke(example.clj:3)
	at user$eval147.invokeStatic(example.clj:7)
	at user$eval147.invoke(example.clj:7)
	at clojure.lang.Compiler.eval(Compiler.java:7062)
	at clojure.lang.Compiler.load(Compiler.java:7514)
	... 9 more

The line at user$g.invokeStatic(example.clj:4) here is quite misleading, because the error happens at line 5.






[CLJ-2273] Add original 'assert' form to explain-data for s/assert Created: 21/Nov/17  Updated: 27/Nov/17

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

Type: Enhancement Priority: Major
Reporter: Ben Brinckerhoff Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: spec
Environment:

org.clojure/spec.alpha "0.1.143"


Approval: Triaged

 Description   

Using s/assert instead of assert has advantages: namely a specific reason why the data failed to meet the spec. But it also has disadvantages: the error message does not contain the assertion code the user wrote, so it's harder to see which assertion failed.

I believe we could have the best of both world if s/assert contained the original "assert" code - that way the error message could (optionally) print out this information.

(require '[clojure.spec.alpha :as s])
  (s/check-asserts true)

  (let [x 1]
    (s/assert string? x))

  ;; Spec assertion failed val: 1 fails predicate:
  ;; :clojure.spec.alpha/unknown

  (let [x 1]
    (assert (string? x)))

  ;; Assert failed: (string? x)





[CLJ-2272] Suppress repl printing of ex-data for macro spec errors Created: 20/Nov/17  Updated: 27/Nov/17

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

Type: Enhancement Priority: Major
Reporter: Greg Chapman Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

When a macro fails a spec check at the repl, the message printed includes the (often large) ex-data map generated by spec explain:

Clojure 1.9.0-RC1
user=> (let [x] 5)
CompilerException clojure.lang.ExceptionInfo: Call to clojure.core/let did not conform to spec:
In: [0] val: () fails spec: :clojure.core.specs.alpha/bindings at: [:args :bindings :init-expr] 
predicate: any?,  Insufficient input
 #:clojure.spec.alpha{:problems [{:path [:args :bindings :init-expr], :reason "Insufficient input", 
:pred clojure.core/any?, :val (), :via [:clojure.core.specs.alpha/bindings 
:clojure.core.specs.alpha/bindings], :in [0]}], :spec 
#object[clojure.spec.alpha$regex_spec_impl$reify__1188 0x3f9270ed 
"clojure.spec.alpha$regex_spec_impl$reify__1188@3f9270ed"], :value ([x] 5), :args ([x] 5)}, 
compiling:(NO_SOURCE_PATH:18:1)

As I understand it, checkSpecs in Compiler.java calls macroexpand-check from spec. For failures, macroexpand-check throws an ExceptionInfo with the explain data in the data map. checkSpecs catches this and throws a new CompilerException, passing the ExceptionInfo as the cause. The CompilerException constructor calls toString on the cause, which for ExceptionInfo generates a string with both the message and the data map.

Possibly, you could add a new overload of the CompilerException constructor, which specifies both an explicit message and a cause (using the message in a call to errorMsg). Then checkSpecs could use this overload, passing ExceptionInfo.getMessage and avoiding printing of the data map.

I've marked this as major, because I think the data map dumps have the potential to be intimidating to newcomers.



 Comments   
Comment by Alex Miller [ 27/Nov/17 8:30 AM ]

There are multiple contributors here to the messiness of the resulting message and I don't agree with the suggested solution. CompilerExceptions are designed to be wrappers (with location info) that can be unrolled to find the cause exception, so to some degree the repl catch handler bears some of the blame here - it should be doing more to unwrap and report. And it could even be doing more for specifically for spec errors if we had a marker that identified them. Also, I think the macro check should not be including the explain data string.





[CLJ-2271] "caller" information missing in explain-data during macro instrumentation failure Created: 19/Nov/17  Updated: 27/Nov/17

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

Type: Defect Priority: Major
Reporter: Ben Brinckerhoff Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: error-reporting, spec
Environment:

org.clojure/spec.alpha "0.1.143"


Approval: Triaged

 Description   

When there is a instrumentation failure for a function, the explain-data includes "caller" information. However, this information is missing if the instrumentation failure is for a macro.

This comment has led me to believe that the intended behavior is for explain-data to contain this info, so third-party error printers can display it.

In the repro below, I'm setting up a custom printer just to capture the raw explain-data (it's not a useful printer, just a means to show what is happenening)

Repro:

(require '[clojure.spec.alpha :as s])
  (require '[clojure.spec.test.alpha :as st])
  (require '[clojure.specs.alpha :as s])


  (s/fdef my-fn
          :args (s/cat :x int?))
  (defn my-fn [x]
    x)

  (s/fdef my-macro
          :args (s/cat :x int?))
  (defmacro my-macro [x]
    x)

  (st/instrument)
  (def !ed (atom nil))
  (set! s/*explain-out* (fn [ed]
                          (reset! !ed ed)))
  (my-fn "")
  @!ed
  ;; {:clojure.spec.alpha/problems [{:path [:args :x], :pred clojure.core/int?, :val "", :via [], :in [0]}], :clojure.spec.alpha/spec #object[clojure.spec.alpha$regex_spec_impl$reify__2436 0x72029b0e "clojure.spec.alpha$regex_spec_impl$reify__2436@72029b0e"], :clojure.spec.alpha/value (""), :clojure.spec.alpha/args (""), :clojure.spec.alpha/failure :instrument, :clojure.spec.test.alpha/caller {:file "form-init8333540581183382896.clj", :line 548, :var-scope expound.alpha/eval27394}}

  ;; ^--- Note there is an entry for :clojure.spec.test.alpha/caller

  (my-macro "")
  @!ed

  ;; #:clojure.spec.alpha{:problems [{:path [:args :x], :pred clojure.core/int?, :val "", :via [], :in [0]}], :spec #object[clojure.spec.alpha$regex_spec_impl$reify__2436 0x479a6a73 "clojure.spec.alpha$regex_spec_impl$reify__2436@479a6a73"], :value (""), :args ("")}

  ;; ^--- No caller information


 Comments   
Comment by Alex Miller [ 27/Nov/17 8:39 AM ]

You can't instrument a macro, so that part of the ticket doesn't make sense as written. But I expect you mean the spec check during macro expansion.

In the macro check case, the caller info is known by the compiler and included in the wrapper CompilerException. I suppose that info could be passed into s/macroexpand-check from the Compiler and possibly produce similar results as with instrumented function calls.





[CLJ-2269] definterface seems not to resolve imported classes in type hints Created: 16/Nov/17  Updated: 27/Nov/17

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

Type: Defect Priority: Minor
Reporter: Greg Chapman Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: interop, typehints
Environment:

Java 1.8.0_152
Windows 10


Approval: Triaged

 Description   

For example:

user=> (import java.util.Map)
java.util.Map
user=> (definterface Foo (^void foo [^Map map]))
user.Foo
user=> (deftype Bar [] Foo (foo [this m]))
CompilerException java.lang.NoClassDefFoundError: java/lang/Map, compiling:(NO_SOURCE_PATH:3:1)
user=> (definterface Foo2 (^void foo2 [^java.util.Map map]))
user.Foo2
user=> (deftype Bar2 [] Foo2 (foo2 [this m]))
user.Bar2

The attempt to type-hint with just Map fails; you have to use java.util.Map to get a usable interface definition.






[CLJ-2268] Spec asserts set : field incorrectly in explain-data Created: 15/Nov/17  Updated: 16/Nov/17

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

Type: Defect Priority: Major
Reporter: Ben Brinckerhoff Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: spec
Environment:

[org.clojure/spec.alpha "0.1.143"] [org.clojure/clojure "1.9.0-beta3"]


Approval: Triaged

 Description   

I would expect that "explain-data" should contain the same "via" entry, regardless of whether it is returned from a call to s/explain-data or if it passed to the printer during an assertion failure.

Repro:

(s/check-asserts true)
  
  (s/def :example/name string?)
  (def !ed (atom nil))


  (s/explain-data :example/name 1)
  ;; #:clojure.spec.alpha{:problems [{:path [], :pred clojure.core/string?, :val 1, :via [:example/name], :in []}], :spec :example/name, :value 1}


  (try
    (binding [s/*explain-out* (fn [ed]
                                (reset! !ed ed)
                                "captured")]
      (s/assert :example/name 1))
    (catch Exception e))

  @!ed
  ;; #:clojure.spec.alpha{:problems [{:path [], :pred clojure.core/string?, :val 1, :via [], :in []}], :spec :example/name, :value 1, :failure :assertion-failed}

Expected: The ":via" entries in both cases should the same
Actual: The ":via" entry is empty in the explain-data that is passed to the printer during the assertion failure.






[CLJ-2264] fspec conform function validation can't access outer gen overrides Created: 08/Nov/17  Updated: 08/Nov/17

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

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

Approval: Triaged

 Description   

Given a situation like this, exercising an fspec will fail:

(def uuid-regex #"^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$")

(s/def ::name string?)
(s/def ::age pos-int?)
(s/def ::id (s/and string? #(re-matches uuid-regex %)))
(s/def ::person (s/keys :req-un [::name ::age ::id]))

;; works - ::id gen override produces valid ids
(s/exercise ::person 1 {::id (fn [] (gen/fmap str (gen/uuid)))})

;; fails - ::id gen override isn't being used
(s/exercise 
  (s/fspec :args (s/cat :p ::person) :ret int?) 
  10 
  {::id #(gen/fmap str (gen/uuid))})
Couldn't satisfy such-that predicate after 100 tries.

Problem: exercise will generate a function that validates the args and returns a gen int. When exercise then conforms that generated fspec function, it calls conform on the fspec-impl. fspec-impl conform* calls validate-function. validate-function runs quick-check on the generated function but conform* and validate-function do not have access to the generator overrides specified in exercise. This same problem also happens if you run stest/check on anything using the fspec (like an fdef with an fspec arg).






[CLJ-2263] When calling a multi method with the wrong number of arguments, the error message could be better. Created: 07/Nov/17  Updated: 09/Nov/17

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

Type: Enhancement Priority: Major
Reporter: Erik Assum Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: errormsgs

Approval: Triaged

 Description   
09:43 $ clj
Clojure 1.8.0
(defmulti foo :bar)
(defmethod foo :qix [quux znoot] (println 'hi))
#'user/foo
user=> #object[clojure.lang.MultiFn 0x205d38da "clojure.lang.MultiFn@205d38da"]
user=> (foo {:bar :qix})
ArityException Wrong number of args (1) passed to: user/eval5/fn--6  clojure.lang.AFn.throwArity (AFn.java:429)
user=>

It is an implementation detail that multi methods are implemented via anonymous functions. I would expect the error message to at least contain the name of the function that failed, in this case it should have been something like

ArityException Wrong number of args (1) passed to: user/eval5/foo--6  clojure.lang.AFn.throwArity (AFn.java:429)

Several approaches can be taken here:

The first (and simplest) is to change the definition of defmethod so that the anonymous function gets a name.
This leads to an error message like:

user=> (defmulti foo :bar)
(defmethod foo :qix [quux znoot] (println 'hi))
(foo {:bar :qix})#'user/foo
user=> #object[clojure.lang.MultiFn 0x4e928fbf "clojure.lang.MultiFn@4e928fbf"]
user=>
ArityException Wrong number of args (1) passed to: user/eval5/foo--6  clojure.lang.AFn.throwArity (AFn.java:429)
user=>

In addition to this, one could modify Compiler.java to look for the calls to "addMethod"

String prefix = "eval";
if (RT.count(form) > 2) {
   Object third = RT.nth(form, 2);
   if (third != null &&
       "clojure.core/addMethod".equals(third.toString()))
      prefix = "multi_fn";
}
ObjExpr fexpr = (ObjExpr) analyze(C.EXPRESSION, RT.list(FN, PersistentVector.EMPTY, form), prefix + RT.nextID());

which would give us error messages like

ArityException Wrong number of args (1) passed to: user/multi-fn5/foo--6  clojure.lang.AFn.throwArity (AFn.java:441)


 Comments   
Comment by Alex Miller [ 09/Nov/17 9:19 AM ]

Interestingly, it is actually possible to give defmethods names that get included in their class names. The arglist for defmethod is: ([multifn dispatch-val & fn-tail]) where fn-tail is the tail arguments as passed to fn, which includes an optional name.

So you can do this:

(defmulti foo class)
;; create defmethod with a name HI:
(defmethod foo String HI [x] (throw (Exception. (str "hi " x))))

;; Invoke:
(foo "bleh")
Exception hi bleh  user/eval1248/HI--1249 (form-init1348837216879020682.clj:1)

Note the HI in the class name. This doesn't address everything above but it is a helpful debugging trick.





[CLJ-2261] dot form silently drops additional (invalid) args Created: 05/Nov/17  Updated: 06/Nov/17

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

Type: Defect Priority: Trivial
Reporter: Thomas Heller Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: compiler, interop

Approval: Triaged

 Description   

If the second argument to . is a seq? every additional arg is silently dropped. In CLJS this throws a compile error.

(. "xyz" (substring 1) (throw :bug?))
=> "yz"

Might be neat to at least get a warning since it's not immediately obvious when accidentally using . instead of ...






[CLJ-2258] When fspec fails, meaning of ":val" is different than in normal spec failure Created: 29/Oct/17  Updated: 30/Oct/17

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

Type: Defect Priority: Major
Reporter: Ben Brinckerhoff Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: spec
Environment:

org.clojure/spec.alpha "0.1.134"


Approval: Triaged

 Description   

Context:

When data fails to conform to a spec, the ":val" of a problem points to the non-conforming data. This has the nice property that the ":val" (for each problem) will exist somewhere within the ":value" (for the entire "explain-data" structure). For example:

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

(s/explain-data
 (s/coll-of int?)
 [1 2 :a])
  
;; #:clojure.spec.alpha{:problems ({:path [], :pred int?, :val :a, :via [], :in [2]}), :spec #object[clojure.spec.alpha$every_impl$reify__934 0x1b235c3e "clojure.spec.alpha$every_impl$reify__934@1b235c3e"], :value [1 2 :a]}

This interpretation of ":val" doesn't seem to apply when a function fails to conform to an fspec spec.

Repro:

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

(s/explain-data
 (s/coll-of (s/fspec
             :args (s/cat :x int?)))
 [(fn [%] (/ 1 %))])

;; #:clojure.spec.alpha{:problems ({:path [], :pred (apply fn), :val (0), :reason "Divide by zero", :via [], :in [0]}), :spec #object[clojure.spec.alpha$every_impl$reify__934 0x420c3355 "clojure.spec.alpha$every_impl$reify__934@420c3355"], :value [#function[expound.alpha/eval28876/fn--28880]]}

Expected: In order to be consistent with the way "val" normally works, "val" should be the anonymous function. While the function arguments are very useful, perhaps they could be associated with a different key?
Actual: "val" contains the arguments to the function, so it's no longer true that the "val" exists within the "value".



 Comments   
Comment by Ben Brinckerhoff [ 29/Oct/17 3:04 PM ]

This also makes the error messages misleading:

(s/explain
 (s/coll-of (s/fspec
             :args (s/cat :x int?)))
 [(fn [%] (/ 1 %))])

;; In: [0] val: (0) fails predicate: (apply fn),  Divide by zero

because it seems to indicate that the args are failing to conform, whereas the issue is that the function itself is not conforming.

Comment by Alex Miller [ 29/Oct/17 7:25 PM ]

Please don't set the fix version - core team will do this when it's targeted to a release. This won't be addressed for 1.9.

Comment by Ben Brinckerhoff [ 30/Oct/17 9:06 AM ]

Sorry about that - I'll be sure to avoid setting that field in the future.

A little more context: there are cases where the "in" for a problem is tricky to interpret (https://dev.clojure.org/jira/browse/CLJ-2192). In Expound, I have use a heuristic to figure out how the "in" path should work, but that heuristic depends on the "val" existing within the "value". Due to this issue, the heuristic doesn't work, which means I can't reliably report the spec error. See https://github.com/bhb/expound/issues/41





[CLJ-2256] When fspec fails due to return value, `:pred` is not a valid spec in problem Created: 25/Oct/17  Updated: 25/Oct/17

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

Type: Defect Priority: Major
Reporter: Ben Brinckerhoff Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: spec
Environment:

org.clojure/spec.alpha "0.1.134"



 Description   

Repro:

(s/def :fspec-test/plus (s/fspec
                         :args (s/cat :x int? :y pos-int?)
                         :ret pos-int?))

(defn my-plus [x y]
  (+ x y))

(s/explain-data :fspec-test/plus my-plus)

;; #:clojure.spec.alpha{:problems [{:path [:ret], :pred clojure.core/pos-int?, :val 0, :via [:fspec-test/plus], :in []}], :spec :fspec-test/plus, :value #function[expound.alpha-test/my-plus]}

Expected: There should be some way to get the spec (i.e. the function itself, not a symbol) that fails for the return value. That way, we can recursively describe why the return value fails the spec.

Note that for a non-fspec failure, the function itself (not the symbol) is included in the explain-data:

(s/explain-data pos-int? 0)

;; #:clojure.spec.alpha{:problems [{:path [], :pred :clojure.spec.alpha/unknown, :val 0, :via [], :in []}], :spec #function[clojure.core/pos-int?], :value 0}





[CLJ-2255] When fspec spec fails due to return value, explain-data should contain args Created: 24/Oct/17  Updated: 25/Oct/17

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

Type: Defect Priority: Major
Reporter: Ben Brinckerhoff Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: spec
Environment:

org.clojure/spec.alpha "0.1.134"


Approval: Triaged

 Description   

Repro:

(require '[clojure.spec.alpha :as s])
(s/def :fspec-test/plus (s/fspec
                         :args (s/cat :x int? :y pos-int?)
                         :ret pos-int?))

(defn my-plus [x y]
  (+ x y))

(s/explain-data :fspec-test/plus my-plus)

Actual:

;; #:clojure.spec.alpha{:problems [{:path [:ret], :pred clojure.core/pos-int?, :val 0, :via [:fspec-test/plus], :in []}], :spec :fspec-test/plus, :value #function[expound.alpha-test/my-plus]}

Expected: I would expect the explain-data to contain the args that produced the invalid return value, so I can reproduce. Note that if my function throws an exception, the explain data will contain the args e.g.

(s/def :fspec-test/plus (s/fspec
                         :args (s/cat :x int? :y pos-int?)))
(defn my-plus [x y]
  (assert (pos? (+ x y))))

(s/explain-data :fspec-test/plus my-plus)
;; #:clojure.spec.alpha{:problems [{:path [], :pred (apply fn), :val (-1 1), :reason "Assert failed: (pos? (+ x y))", :via [:fspec-test/plus], :in []}], :spec :fspec-test/plus, :value #function[expound.alpha-test/my-plus]}





[CLJ-2251] Support Coercion for clojure.spec Created: 11/Oct/17  Updated: 11/Oct/17

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

Type: Enhancement Priority: Major
Reporter: Tommi Reiman Assignee: Unassigned
Resolution: Unresolved Votes: 3
Labels: None
Environment:

[org.clojure/spec.alpha "0.1.134"]



 Description   

Problem

To do runtime coercion, specs need to be walked twice to strip away the branching information: s/conform + s/unform. This introduced extra latency (see the sample below).

Proposal

New versatile s/walk* to support generic spec walking.

Current status

Still, when running s/conform + s/unform, we walk the specs twice - which is performance-wise suboptimal. Below is a sample, with Late 2013 MacBook Pro with 2,5 GHz i7, with JVM running as -server.

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

(s/def ::id int?)
(s/def ::name string?)
(s/def ::languages (s/coll-of #{:clj :cljs} :into #{}))
(s/def ::street string?)
(s/def ::zip string?)
(s/def ::number int?)

(s/def ::address (s/keys
                   :req-un [::street ::zip ::number]))

(s/def ::user (s/keys
                :req [::id]
                :req-un [::name ::address]
                :opt-un [::languages]))

(def value {::id 1
            :name "Liisa"
            :languages #{:clj :cljs}
            :address {:street "Hämeenkatu"
                      :number 24
                      :zip "33200"}})

; 2.0 µs
(cc/quick-bench
  (s/conform ::user value))

; 6.2 µs
(cc/quick-bench
  (s/unform ::user (s/conform ::user value)))

Despite s/conform is relatively fast, we triple the latency in the sample when running also s/unform. As we know already that we are not interested in the branching info, we could just not emit those.

Suggestion

s/walk* to replace both s/confrom* and s/unform*, maybe even s/explain*. It would take extra mode argument, which would be a Keyword of one of the following:

  • :validate - return false on first failing spec
  • :conform - like the current s/conform*, maybe also return s/explain results?
  • :unform - like the current s/unform*
  • :coerce - s/conform* + s/unform*, could be optimized (e.g. if no branching info, just return the value)

The public apis could be remain the same (+ optional extra argument with CLJ-2116), and a new s/coerce to call the s/walk* with :coerce.

Results

Single sweep validation & coercion. Happy runtime.






[CLJ-2250] Avoid initializing Class when using Class as a value Created: 10/Oct/17  Updated: 10/Oct/17

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

Type: Defect Priority: Major
Reporter: Ragnar Dahlen Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJ-2250-avoid-initializing-class-when-used-as-value.patch    

 Description   

Problem:
When an imported class is used as a value, the emitted bytecode uses RT.classForName to obtain the Class object, causing the class to be loaded and static initializers to be executed. This is different from when Java calls static initializers and makes it more difficult to use clojure with code that depend on the Java semantics.

Motivation
Some code has static initializers that can only execute in a certain environment. A prime example is JavaFX, where many JavaFX classes require the JavaFX platform to be started before any static initializers can run.

Consider this code:

example.clj
(import 'javafx.scene.control.Cell)

(defn f [] Cell)

It currently can't be compiled and executed (for example with "clj example.clj" using clojure-1.9.0beta2) failing with a CompilerException java.lang.ExceptionInInitializerError, with the root cause "Toolkit not initialized". This use of Cell as the return value of f causes the class to be loaded and initialised.

Approach
Modify ObjExpr.emitValue to emit a call to RT.classForNameNonLoading instead of RT.classForName when the value being emitted is a Class.

Patch
https://dev.clojure.org/jira/secure/attachment/17426/CLJ-2250-avoid-initializing-class-when-used-as-value.patch

Prior art
The import form previously was changed to similarly not load the class (CLJ-1315) and CLJ-1743 attempts to address similar issues where Clojure differs from the Java semantics.



 Comments   
Comment by Ragnar Dahlen [ 10/Oct/17 5:05 PM ]

Added patch.





[CLJ-2246] spec.test/check returns the wrong value of :failure for failing tests Created: 02/Oct/17  Updated: 18/Jan/18

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

Type: Defect Priority: Major
Reporter: Khalid Jebbari Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: spec
Environment:

[org.clojure/clojure "1.9.0-beta1"]
[org.clojure/spec.alpha "0.1.123"]
[org.clojure/test.check "0.10.0-alpha2" :scope "test"]


Attachments: Text File 0001-Use-result-type-to-test-for-failure-in-abbrev-result.patch    
Approval: Triaged

 Description   

When trying to manually wire clojure.test and spec for fdef'ed functions, I realized that spec.test/check returns a map with [:failure false] for failing tests. I'm (almost) sure it should return true, because spec.test/abbrev-result receives as argument the return of spec.test/check and tests the value of :failure. I couldn't produce a case where spec.test/check returned [:failure true] for failing tests. For information it returns a map without the key :failure for passing test.

Here's a simple reproduction case in the REPL :

(defn foo-fn [x] "bar")

(s/fdef foo-fn
:args (s/cat :x any?)
:ret string?)

(stest/check `foo-fn) ;; => no error, normal small map printed, no :failure entry

(defn foo-fn [x] 1)

(stest/check `foo-fn) ;; => full error map, with :failure equals false. :failure shown below

(-> (stest/check `foo-fn) first :failure) ;; => false



 Comments   
Comment by Gary Fredericks [ 02/Oct/17 7:34 PM ]

This is not a bug, though it is confusing.

stest/check is passing through (at least part of) the return value from clojure.test.check/quickcheck, which returns the value returned by the property, which is evaluated for thruthiness to determine pass vs fail. Since a non-truthy value can only be false or nil, you don't learn very much by looking at it (though it's possible that the :failure key could also have an exception under it, which would be more informative).

The next release of test.check should have a more useful and less confusing way of extracting information from a test, but I don't know for sure how that would look when using stest/check.

Comment by Khalid Jebbari [ 03/Oct/17 3:43 AM ]

I still don't understand why you don't consider it a bug. The docstring of `stest/check` says ":failure optional test failure". I expect to have a `:failure` key only when the tests fail (hence the wording optional), with some valuable info.

I'm using the `stest/abbrev-result` to display the output of `stest/check`, which expects `:failure` to be truthy to display all the valuable failure information. If `stest/check` returns `false`, there's a mismatch I consider a bug. The docstring of `stest/abbrev-result` explicitly says it receives as argument the result of check, so I'm not forcing a square peg into a round hole.

Thank you for answering me so fast and for your time.

Comment by Michael Glaesemann [ 15/Jan/18 6:23 PM ]

The false value for :failure is definitely confusing. The stest/abbrev-result is very confounding in the {:failure false} as it doesn't provide additional information that failures usual do,as Khalid Jebbari points out.

(defn abbrev-result
  "Given a check result, returns an abbreviated version
suitable for summary use."
  [x]
  (if (:failure x)
    (-> (dissoc x ::stc/ret)
        (update :spec s/describe)
        (update :failure unwrap-failure))
    (dissoc x :spec ::stc/ret)))

Here's an example showing how misleading it can be:

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

(alias 'stc 'clojure.spec.test.check)

(defn adder [a b]
  (+ a b))

(s/fdef adder
        :args (s/cat :a int? :b int?)
        :ret string?)

(-> (stest/check `adder) first stest/abbrev-result)
;; => {:sym ex.check-test/adder, :failure false}

;; Writing an alternative version of `abbrev-result` that
;; checks for `true`

(defn- failure-type [x] (::s/failure (ex-data x)))
(defn- unwrap-failure [x] (if (failure-type x) (ex-data x) x))

(defn- abbrev-result [x]
  (let [failure (:failure x)]
    (if-not (or (true? failure)
                (nil? failure))
      (-> (dissoc x ::stc/ret)
          (update :spec s/describe)
          (update :failure unwrap-failure))
      (dissoc x :spec ::stc/ret))))

(-> (stest/check `adder) first abbrev-result)
;; => {:spec (fspec :args (cat :a int? :b int?) :ret string? :fn nil),
;;     :sym ex.check-test/adder,
;;     :failure false}

Again, note that any truthy :failure value is going to provide the additional details, while falsey :failure values will not.

I understand the motivation for not changing the value of the :failure key. If that value is going to be maintained, I think stest/abbrev-result should be updated to likewise test explicitly for nil and true, rather than truthy for consistency with the result of stest/check.

Comment by Khalid Jebbari [ 16/Jan/18 8:13 AM ]

Thanks a lot Michael. In the end I went with a small variation, where I don't `(dissoc x ::stc/ret)` but keep the whole check result because it includes the stack trace, the spec/explain-data map, the shrunk failing case etc.

Comment by Michael Glaesemann [ 17/Jan/18 9:19 PM ]

Looks like abbrev-result should use result-type to test whether the check passed. Attaching a patch which does this. If you'd like tests to accompany it, I'm happy to resubmit.

Edit: Nope: that was too hasty. Will investigate and resubmit. Sorry for the noise.

Edit 2: Second guessing myself incorrectly. I'm pretty sure I was correct the first time. I think the patch is good.

Comment by Gary Fredericks [ 18/Jan/18 6:23 AM ]

note that there are unreleased changes on the master branch of test.check that may influence the best thing to do here. this stuff in particular.

Comment by Michael Glaesemann [ 18/Jan/18 3:40 PM ]

Thanks, Gary Fredericks. The Result protocol is something to keep in mind. I think using result-type is the right abstraction at the level of abbrev-result. I would think Result/passing? would be used when decorating the quick-check results in make-check-result in lieu of the (true? result) call. Does that make sense to you?





[CLJ-2244] double division by zero inconsistency Created: 27/Sep/17  Updated: 04/Oct/17

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

Type: Defect Priority: Minor
Reporter: Greg Chapman Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: math

Approval: Triaged

 Description   

Division by zero with doubles will throw an exception if both arguments are boxed, but not otherwise:

Clojure 1.9.0-beta1
user=> (/ 5.0 0.0)
##Inf
user=> (/ (identity 5.0) 0.0)
##Inf
user=> (/ 5.0 (identity 0.0))
##Inf
user=> (/ (identity 5.0) (identity 0.0))
ArithmeticException Divide by zero  clojure.lang.Numbers.divide (Numbers.java:163)


 Comments   
Comment by Erik Assum [ 28/Sep/17 1:10 PM ]

So, I guess some division by zero is somewhat undefined.
If one removes

if(yops.isZero((Number)y))
	    throw new ArithmeticException("Divide by zero");

then

(/ (identity 5.0) (identity 0.0))

returns infinity as the unboxed versions.

Although doing so, also causes a change to

(/ 5 0)

which then becomes `1/0`, whereas it previously threw Divide by zero.

From Numbers.java we see that unboxed math does not protect against division by zero:

static public double divide(double x, double y){
	return x / y;
}

whereas boxed versions do:

static public Number divide(Object x, Object y){
	if (isNaN(x)){
		return (Number)x;
	} else if(isNaN(y)){
		return (Number)y;
	}
	Ops yops = ops(y);
	if(yops.isZero((Number)y))
		throw new ArithmeticException("Divide by zero");
	return ops(x).combine(yops).divide((Number)x, (Number)y);
}

This inconsistency is also reproducible with clojure 1.8:

Clojure 1.8.0
Java HotSpot(TM) 64-Bit Server VM 1.8.0_144-b01
    Docs: (doc function-name-here)
          (find-doc "part-of-name-here")
  Source: (source function-name-here)
 Javadoc: (javadoc java-object-or-class-here)
    Exit: Control+D or (exit) or (quit)
 Results: Stored in vars *1, *2, *3, an exception in *e

user=> (/ 5.0 0.0)
Infinity
user=> (/ (identity 5.0) (identity 0.0))

ArithmeticException Divide by zero  clojure.lang.Numbers.divide (Numbers.java:158)
user=>
Comment by Erik Assum [ 04/Oct/17 12:43 PM ]

Any thoughts on an approach?
Both throwing on division by zero for natives and allowing it for boxed seems to be a serious change, and the tests expects an exception for boxed division by zero.





[CLJ-2243] clojure.lang.RT should provide a loadObject static method Created: 26/Sep/17  Updated: 27/Sep/17

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

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


 Description   

Similar to CLJ-843, one cannot invoke (System/load "/tmp/a.so") since it successfully loads the so file to the wrong classloader (thanks SUN!)

While trying to make a simple way to load shared-objects embedded in uberjars (using https://github.com/adamheinrich/native-utils/blob/master/src/main/java/cz/adamh/utils/NativeUtils.java), I found that it is not possible to do due to the classloader issue.

Any thoughts?



 Comments   
Comment by Alex Miller [ 27/Sep/17 7:05 AM ]

That linked jira had a patch and was committed, so it seems this already exists as (clojure.lang.RT/loadLibrary "/tmp/a.so") ?

Comment by Shlomi [ 27/Sep/17 11:36 AM ]

Alex,
Sorry for not being clear enough:

(clojure.lang.RT/loadLibrary "/tmp/a.so") - does not work, loadLibrary is expecting properly named library files existing within a path pointed to by either LD_LIBRARY_PATH or java.library.path. For example, if you have the file liba.so existing somewhere on LD_LIBRARY_PATH than (clojure.lang.RT/loadLibrary "a") would work.

For use cases where you dont want to rely on environment settings or preexistence of the .so files, you could use System/load instead of System/loadLibrary. For instance if you want to download the proper .so files for some platform from within the running clojure program, or have the .so files exists inside your jar. In these cases you would want to download or extract them from the jar respectively, place them in some temp folder (which is not in LD_LIBRARY_PATH) and load them using their full path, to be used in the running program (i.e. no special scripts or preparation tasks).

If I was to write a patch to src/jvm/clojure/lang/RT.java, it would be:

// Load a library in the System ClassLoader instead of Clojure's own, from anywhere
public static void loadObject(String libFullPath){ System.load(libFullPath); }





[CLJ-2242] clojure.test: `use-fixtures` should (or should be able to) run for for a `testing` context Created: 25/Sep/17  Updated: 25/Sep/17

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

Type: Enhancement Priority: Minor
Reporter: Víctor M. Valenzuela Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: clojure.test


 Description   

As implemented currently, the clojure.test/use-fixtures hooks will run on a per-deftest basis, but not on a per-testing (clojure.test/testing) basis.

So, for a given deftest with multiple testing clauses, use-fixtures will be only run once.

Coming from an RSpec (Ruby) background this is surprising - the default behavior is the opposite, and in my opinion more intuitive.

Personally, this resulted in me writing tests that gave false positives - i.e. it rendered a portion of my test suite useless until I noticed this (by mere luck).

Anecdotically, I commented the issue with another Clojure developer and he also suffered the issue once. I mean, I'm not alone in this one, and surely other developers have stumbled with this despite the lack of JIRA report until now.

In any case, the current working alternative (one deftest per test which depends on use-fixtures) can be overly verbose (in comparison with my original attempt), and also the need for doing that in the first place can be easy to forget.

I'd ask to:

  • Implement an option for use-fixtures called e.g. :run-nested?
  • Make this option mandatory to specify, i.e. programmers pass either :run-nested? true or :run-nested? false, unless a global default is set beforehand. People should be aware of this nuance and be forced to think about it at least once (else they risk false positives/negatives)

I hope this sounds like a reasonable, needed, non-breaking proposal.

Cheers - Victor



 Comments   
Comment by Alex Miller [ 25/Sep/17 6:56 PM ]

Hi Victor, it would be helpful if you provided a full code example in the description.

Fixtures to me seem pretty well defined in the docs (https://github.com/clojure/clojure/blob/master/src/clj/clojure/test.clj#L158-L193) and are strongly tied to the invocation of tests as functions in vars. Invoking them on a per-`testing` basis seems like significantly different to me.

Another option would be to create a new kind of fixture with a different scope that would get reinvoked in `testing` blocks in particular like a `:group` assertion. I guess that would also apply to nested `testing` blocks?

I don't actually have a good sense of what kind of test would need this, hence the request for an example.





[CLJ-2229] explain-data intermittently produces incorrect report when specs rely on dynamic vars Created: 30/Aug/17  Updated: 30/Aug/17

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

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


 Description   

We've got some specs whose behavior is controlled by the value of a dynamic var. When we run

(binding [*behave-differently* true] (s/explain-data ::spec-with-conditional-behavior nested-structure-including-maps))

the result shows that some of the nested specs are not operating as expect based on the value of *behave-differently*. I believe that this is due to some implementations of explain* producing lazy seqs that aren't realized until after the binding form has closed. Anecdotally, forcing realization within the binding form results in the expected behavior.



 Comments   
Comment by David Chelimsky [ 30/Aug/17 7:55 AM ]

FYI - the description didn't format as I expected and I don't have edit permissions.

Comment by Alex Miller [ 30/Aug/17 8:09 AM ]

I gave you edit groups David.

Comment by David Chelimsky [ 30/Aug/17 8:24 AM ]

Thanks Alex. I updated the Description.

Comment by Alex Miller [ 30/Aug/17 8:52 AM ]

A working example would help.





[CLJ-2224] Support printing and reading of Java 8 java.time.Instant Created: 19/Aug/17  Updated: 20/Aug/17

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

Type: Enhancement Priority: Minor
Reporter: David Bürgin Assignee: Unassigned
Resolution: Unresolved Votes: 4
Labels: instant
Environment:

Java 8


Approval: Triaged

 Description   

In Clojure 1.9 alpha, limited support for Java 8 java.time.Instant was added, namely by (conditionally) extending the Inst protocol to that type.

It would be useful to enhance support for java.time.Instant further by

  • installing a print-method and print-dup for java.time.Instant
  • providing a read-instant function for reading java.time.Instant

This functionality is already provided in Clojure 1.8 today for the types java.util.Date, java.util.Calendar, and java.sql.Timestamp; extending it to java.time.Instant would be very helpful in environments using Java 8.






[CLJ-2217] Disable fspec validation during instrumentation Created: 05/Aug/17  Updated: 08/Aug/17

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: generative-test, spec
Environment:

JVM



 Description   

Problem statement: Enable instrumentation, and invoke a speced function with a lambda. To validate the lambda, spec tests it with generative testing. This results in the lambda being invoked multiple times. If the lambda launches a missile, many missiles are now launched by spec. There are many scenarios in which this is not acceptable because it can for example crash the environment.

Current solutions:

  • Don't spec the lambda. Disadvantage: Spec can't generate it in contexts where its spec is referred.
  • Set fspec-iterations to 0. Disadvantage: Disables all validation of all lambdas.
  • ???

Ideas:

  • An fspec flag to disable the generative testing of its validation.


 Comments   
Comment by Alex Miller [ 05/Aug/17 2:00 PM ]

Another option that has been proposed for this is to make the instrumented function also wrap the function arg in instrumentation according to its spec.

Comment by Leon Grapenthin [ 05/Aug/17 4:49 PM ]

@Alex Miller: Yes, I thought about this as well and believe it would be more consistent with how instrumentation works in regard to functions, i. e. they are checked at invocation time.

However I have not proposed it, because I don't see how we should do it. Spec would have to be able to generically replace all functions that are passed in any arg anywhere with instrumented ones, but also have to know which specs to use. How?

Comment by Leon Grapenthin [ 05/Aug/17 5:24 PM ]

One possible approach would be to implement "descriptive walking" in spec as internal or even public enhancement.
A spec-walk feature would work like prewalk/postwalk, but it takes a spec and a value and invokes the user provided function with both a (sub)value and its corresponding (sub)spec. Instrument wrapper could then replace values that are fspeced with instrumented fns generically. Every spec that composes other specs would have to implement walking over its children and their specs as a new interface method.

Comment by Alex Miller [ 06/Aug/17 11:03 PM ]

Yes, would need something like this (see CLJ-2208 for ticket re spec walking).

Comment by Leon Grapenthin [ 08/Aug/17 6:37 AM ]

@Alex Miller: CLJ-2208 won't do alone. We need to be able to generically walk/replace any given data structure using a spec describing its shape. Should I create a separate ticket and outline a few approaches to get things going? Or should that go here for now?





[CLJ-2215] Extend-protocol for array of Object does not work on array of subtypes of Object. Created: 31/Jul/17  Updated: 31/Jul/17

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

Type: Defect Priority: Minor
Reporter: Didier A. Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: protocols

Approval: Triaged

 Description   

It appears that when you extend a protocol for "[Ljava.lang.Object;", the dispatch can not resolve for child types of "[Ljava.lang.Object;".

(defprotocol Table
  (t [this]))

(extend-protocol Table
  (Class/forName "[Ljava.lang.Object;")
  (t [this] this))

(t (make-array java.lang.String 0))
=> IllegalArgumentException No implementation of method: :t of protocol: #'test-t/Table found for class: [Ljava.lang.String;  clojure.core/-cache-protocol-fn (core_deftype.clj:568)

(t (make-array java.lang.Object 0))
=> ["[Ljava.lang.Object;" 1512480936 "[Ljava.lang.Object;@5a26a0a8"]

Yet Java is covariant on Object[]:

(instance? (Class/forName "[Ljava.lang.Object;") (make-array java.lang.String 0))
=> true
$ cat > Foo.java
public class Foo {
  public Object[] fooey;
  public Foo() {
    fooey = new String[10];
  }
}
$ javac Foo.java
$


 Comments   
Comment by Alex Miller [ 31/Jul/17 2:18 PM ]

Related: CLJ-1381





[CLJ-2214] Add binding conveyance to reducers. Created: 30/Jul/17  Updated: 30/Jul/17

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

Type: Enhancement Priority: Minor
Reporter: Didier A. Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: reducers

Approval: Triaged

 Description   

Clojure reducers is the only parallel construct that does not support binding conveyance.

(def ^:dynamic bar "In main thread.")

(binding [bar "In reducers Thread."]
         (r/fold (fn [& args]
                   bar) (vec (range 100000))))
=> "In main thread."

You have to manually use bound-fn:

(binding [bar "In reducers Thread."]
         (r/fold (bound-fn [& args]
                           bar) (vec (range 100000))))
=> "In reducers Thread."

I propose it is enhanced to behave the same as pmap, future, agents and core.async.






[CLJ-2211] docstring of defn is not precise about the `attr-map?` and `body` arguments Created: 28/Jul/17  Updated: 20/Aug/17

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

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

Attachments: Text File clj-2211.patch    
Approval: Triaged

 Description   

The docstring for defn is:

Same as (def name (fn [params* ] exprs*)) or (def name (fn ([params* ] exprs*)+)) with any doc-string or attrs added to the var metadata. prepost-map defines a map with optional keys :pre and :post that contain collections of pre or post conditions.

The arglist is:

(defn name doc-string? attr-map? [params*] prepost-map? body)

There are two issues that made this docstring hard to understand for me:

1. The docstring does not mention attr-map? - it took me a a bit of jumping around the docs to make the leap from attr-map? to "with any [...] attrs added to the var metadata".
2. The docstring makes reference to exprs*, but the arglist refers to body.



 Comments   
Comment by Alex Miller [ 28/Jul/17 8:24 AM ]

Patch welcome, would appreciate smallest change possible.

Comment by Marc O'Morain [ 19/Aug/17 3:24 PM ]

I've attached a patch to address two issues:

  • change 'exprs*' to 'body' to match the arglist.
  • change 'body+)' to 'body)+' when referring to the multi-artiy form.

I didn't address "any attrs" referring to attr-map, I wasn't sure the most "Clojury" way to phrase it.

Comment by Alex Miller [ 20/Aug/17 4:59 PM ]

Seems like the body change is missing a trailing right paren?

You have: (def name (fn ([params* ] body)+)
Should be: (def name (fn ([params* ] body)+))





[CLJ-2208] Provide a means to ask a spec for its "child" specs Created: 15/Jul/17  Updated: 15/Jul/17

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

Type: Enhancement Priority: Major
Reporter: Eric Normand Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: spec

Approval: Triaged

 Description   

Some kinds of operations on specs are currently hard to implement as there is no uniform way to find what "child" specs are being composed by a spec. Examples:

  • Dependency analysis
  • Deep describe (show all specs used by a top-level spec)
  • Detection of missing or invalid spec names

For example, given:

(s/def ::user-id int?)
(s/def ::user (s/keys :req [::userid])) ;; note misspelling
(s/valid? ::user {::userid "Jim"}) ;; => true but expect false

And the means to determine the "child" specs of ::user, a linter could check whether all of the keys in s/keys are specs that have been defined.

Workarounds:

1. form can be used to get the original spec form, but that must then be further interpreted (and is missing the original lexical environment in which it was created). Example attempt: https://gist.github.com/ericnormand/6cfe6809beeeea3246679e904372cca0
2. Spec form specs (CLJ-2112) are not available yet, but could be used to get a parsed representation of specs, which would still require some processing but would at least have known forms.

Proposed:

Add a mechanism to get the "child" specs a spec is composed of. Each spec implementation could then choose how to implement this in the appropriate way.



 Comments   
Comment by Eric Normand [ 15/Jul/17 8:53 AM ]

I forgot to add this proposal:

Proposal

I propose that we add a children* method to the Spec protocol. It should return a collection of specs directly referred to. The specs in the collection should be a keyword (if it is referred to by name), an instance of Spec (for nested specs), or some other value valid as a spec (such as a fn).

Comment by Alex Miller [ 15/Jul/17 8:59 AM ]

Rewrote title and some of the description to be less dependent on implementation details (which may change) and more about the problem at hand.





[CLJ-2201] proxy-super is not threadsafe, it should be made safe or documented to be unsafe Created: 05/Jul/17  Updated: 05/Jul/17

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

Type: Enhancement Priority: Major
Reporter: Kevin Downey Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Coming from java you might expect proxy-super to be pretty innocuous, but proxy-super operates by mutating the proxy object then restoring it after the call to proxy-super is invoked. This can lead to very weird behavior. If you have a proxy with method M, which invokes proxy-super, then while that proxy-super is running all calls to M on that proxy object will immediately invoke the super M not the proxied M.

Actually making proxy-super safe (not just threadsafe, but also safe when invoked later on in the same callstack) seems like it might be really hard, but it would be nice. Alternatively some blinking hazard lights in the docstring might be a good idea.






[CLJ-2199] Error attempting to unform unconformed keys (no :conform-keys opt) Created: 05/Jul/17  Updated: 10/Nov/17

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

Type: Defect Priority: Minor
Reporter: Daniel Stockton Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: spec

Attachments: Text File clj-2199.patch     Text File clj-2199.patch    
Approval: Vetted

 Description   

Minimal failing case:

(s/def ::key-spec (s/or :kw keyword? :str string?))
(s/def ::map-spec (s/map-of ::key-spec identity))
(println (s/unform ::map-spec (s/conform ::map-spec {:a :b})))
java.lang.UnsupportedOperationException: nth not supported on this type: Keyword

If keys are not conformed, we should also not attempt to unform them.



 Comments   
Comment by Daniel Stockton [ 08/Aug/17 8:29 AM ]

Add test and fix behaviour

Comment by Daniel Stockton [ 08/Aug/17 8:39 AM ]

Actually, although this passes all tests, it's not alright because it bypasses validation.

Comment by Daniel Stockton [ 12/Aug/17 4:23 AM ]

I think it passes for correctness this time but open to advice if this is not a good approach.





[CLJ-2197] instrument :stub doesn't use :gen override Created: 30/Jun/17  Updated: 30/Jun/17

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

Type: Defect Priority: Major
Reporter: Michael Glaesemann Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: spec


 Description   

`instrument` doesn't respect `:gen` override for `:stub`.

(require '[clojure.spec.alpha :as s])
(require '[clojure.spec.gen.alpha :as gen])
(require '[clojure.spec.test.alpha :as stest])

;; [org.clojure/spec.alpha "0.1.123"]

;; The goal is to stub functions which require some kind external
;; dependency, such as a service or other I/O.

(defprotocol Y
  (-do-y [r]))

(def y? (partial satisfies? Y))
(s/def ::y y?)

;; Protocol methods can't be spec'd, so wrap them in a function.

(defn do-y [r]
  (-do-y r))

(s/fdef do-y :args (s/cat :y-er ::y))

;; Example of the protocol implementation that we're going to stub.

(defrecord BadYer []
  Y
  (-do-y [_] (throw (Exception. "can't make me!"))))

;; Confirm BadYer instances are valid with respect to the protol spec.

(s/valid? ::y (->BadYer))
;; => true

;; And confirm BadYer instances will throw when called.

(try
  (do-y (->BadYer))
  (catch Exception e
    (.getMessage e)))
;; => "can't make me!"


(def y-gen (gen/return (->BadYer)))

;; Confirm generator works as expected:

(gen/sample y-gen 1)
;; => (#spec_ex.core.BadYer{})

;; We want to stub `do-y`, providing y-gen as a generator for `::y`

(try
  (stest/instrument `do-y {:stub #{`do-y}
                           :gen {::y (fn [] y-gen)}})
  (catch Exception e
    (ex-data e)))
;; => #:clojure.spec.alpha{:path [:y-er], :form :spec-ex.core/y, :failure :no-gen}

;; However, we *can* stub `do-y` if we replace its spec.

(stest/instrument `do-y
                  {:stub #{`do-y}
                   :spec {`do-y (s/fspec
                                  :args (s/cat :y-er (s/with-gen ::y
                                                       (fn [] y-gen))))}})
;; => [spec-ex.core/do-y]





[CLJ-2196] Allow string keys for `s/key` specs Created: 30/Jun/17  Updated: 30/Jun/17

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

Type: Enhancement Priority: Major
Reporter: Ben Brinckerhoff Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: spec

Approval: Triaged

 Description   

JSON is a common data format, especially when interfacing with non-Clojure systems. All keys in JSON objects are strings (not keywords, as is common in Clojure).

It is desirable to be able to validate incoming JSON data and provide helpful error messages when data is poorly formed. Spec is an excellent tool for both, but `s/keys` only works with keyword keys.

It would be useful to be able to specify string keys, for instance, given some JSON data like

{"city" "Denver" "state" "CO"}

I would like to write a spec like:

(s/def :location/city string?)
(s/def :location/state string?)
(s/keys :req-str [:location/city :location/state])

where `:req-str` is like `:req` and `:req-un-str` would be like `:req-un`. The specs would still be fully-qualified keywords.

The current workaround:

1. Convert string keys to keyword keys using `clojure.walk/keywordize-keys`
2. Validate with spec
3. If there are problems, map over the problems and use `stringify-keys` on each val
4. Format the problems appropriately (basically, reproduce the formatting of `explain`).

This workaround is not particularly difficult, but since I suspect working with JSON is a common case, it may be useful to support this use case more directly.






[CLJ-2194] Spec metadata support Created: 30/Jun/17  Updated: 07/Jul/17

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

Type: Enhancement Priority: Major
Reporter: Max Penet Assignee: Unassigned
Resolution: Unresolved Votes: 11
Labels: spec


 Description   
  1. Spec metadata support

Problem: Currently there is no way to attach metadata to a spec

It would be nice to be able to add a docstring (the primary use case),
or potentially useful information about usage of that spec in different
contexts (static code analyser, custom conversion/coercion, how it
relates to a particular db schema, human readable error message
template, domain specific concerns or even clj.spec itself later, etc...).

In theory one can today create his own meta-data registry and handle
this at the library level, and that's what a few spec related project
already do, but it would be nicer to have a unified/standard way to do
this. By default it would make sense to add support docstrings for a
start. It could take the form of an extra argument of:

;; at least the following two
(s/def ::foo "Something that's a foo" any?)
(s/def ::foo string? {:doc "Something that's a foo" :cassandra-type :varchar})

;; potentially these depending on the implementation
(s/spec #() :gen ... :meta ...)
(with-meta (s/spec ...))

There are a few ways to implement this, with various pros/cons:

  • Implement the IMeta protocol:
    This seems like the clean approach, meta data would/could be supported
    at any Spec level (ex a non registered spec predicate, a Set spec and
    so on). The implementation would require a fair amount changes to the
    current code tho. Mostly adding a meta parameter to the various
    *-spec-impl macros and sugar at `s/def` and derivatives' level.
    A tricky part with that approach is that registered specs that reference
    another spec are just a "link" (a keyword), so we have nowhere to add
    metadata right now.
    They could be reified, return a "pointer" to their original spec and hold
    metadata at their own level.
  • a simple registry (similar to the spec registry, or shared in the main spec registry):
    Basically a map of spec-kw -> metadata if in a separate registry, or integrated into the
    main registry somehow.
    That's the easy approach, only registered spec would be supported, metadata is separated
    from the rest, would keep the Spec instances a bit lighter. Spec referencing other specs
    could have their own metadata.
    As mentioned this could be done in a separate registry or added to a spec value in the main spec
    registry.

It seems to be the IMeta is probably the better solution, we'd leverage the existing "meta" api.



 Comments   
Comment by Nicola Mometto [ 30/Jun/17 4:20 AM ]

(s/def ^{:doc "Something that's a foo" :cassandra-type :varchar} ::foo string?)
is not valid clojure, you can't add metadata to a keyword

Comment by Max Penet [ 30/Jun/17 4:26 AM ]

Changed example as per Nicolas' comment.

Comment by Andy Fingerhut [ 04/Jul/17 10:44 AM ]

Related to (if not a dup of) CLJ-1965

Comment by Alex Miller [ 04/Jul/17 11:40 AM ]

This is related to but not the same as CLJ-1965 - the scope here is larger to potentially support any meta.





[CLJ-2193] Override function spec within check Created: 28/Jun/17  Updated: 29/Jun/17

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

Type: Enhancement Priority: Major
Reporter: Michael Glaesemann Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: spec

Approval: Triaged

 Description   

It's desirable to be able to override a function spec within the scope of a check.

For example:

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

(defn a [x])

(s/fdef a
        :args (s/cat :x int?)
        :fn (fn [_] true))

(s/fdef b
        :args (s/cat :x int?)
        :fn (fn [_] false))

;; should pass
(stest/check `a)

(stest/instrument `a {:spec {`a `b}})
;; should fail
(stest/check `a)

;; Similar cases which should fail:

(stest/instrument `a {:spec {`a (s/fspec :args (s/cat :x int?) :fn (fn [_] false))}})
(stest/check `a)

(stest/instrument `a {:spec {`a (s/get-spec `b)}})
(stest/check `a)





[CLJ-2192] When data fails to conform to `map-of` spec, `:in` path does not point to the invalid (inner) value Created: 27/Jun/17  Updated: 21/Sep/17

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

Type: Defect Priority: Major
Reporter: Ben Brinckerhoff Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: spec
Environment:

org.clojure/clojure "1.9.0-alpha17", org.clojure/clojurescript "1.9.542", org.clojure/core.specs.alpha "0.1.10"



 Description   

Repro:

(require '[clojure.spec.alpha :as s]
(s/def :foo/user-map (s/map-of string? int?))
(s/explain-data :foo/user-map {"hi" "foo"})
;; Actual value:
;; #:cljs.spec.alpha{:problems
;;                 ({:path [1],
;;                   :pred int?,
;;                   :val "foo", 
;;                   :via [:foo/user-map], 
;;                   :in ["hi" 1]})}
;; Expected: the `:in` value to be ["hi"] ? 
(s/explain-data :foo/user-map {:hi 2})
;; Actual value: 
;; #:cljs.spec.alpha{:problems
;;                 ({:path [0],
;;                   :pred string?,
;;                   :val :hi, 
;;                   :via [:foo/user-map], 
;;                   :in [:hi 0]})}
;; Expected: I'm not sure, since a path can't "point to" a key

Motivation: given some top-level data (in this case, `{"hi" "foo"}`) and an `:in` path, I would like to be able to find the problematic data (in this case, `"foo"`).

In the case where the value of a map does not conform, the `:in` path is not compatible with functions like `get-in`, but it could be.

In the case where the key of a map does not conform, there is no way to "point to" a key using `get-in`, so I'm not sure what the right fix is.

I don't know that compatibility with `get-in` is a requirement: if spec provided a function that accomplished the same thing with "spec" paths (i.e. ones that could point to keys), that would be fine.



 Comments   
Comment by juan pedro monetta sanchez [ 12/Jul/17 9:58 AM ]

I think that more important than get-in compatible is a way of matching :clojure.spec.alpha/problems inside :clojure.spec.alpha/value independent of the spec that lead to the problem.
For this I think it's important to know if the problem is with the key or the value.

Currently s/map-of reports a path taking into account the map-entry vec, so 0 will be the key and 1 the value.

The problem with what I'm trying to implement is the :in is s/keys which only reports the key.

So when you see a problem in [::k 1] you don't know if it's a problem in the map value or the value is a seq and the problem is in the value at pos 1 in that seq.

Comment by Ben Brinckerhoff [ 21/Sep/17 8:56 AM ]

I agree with what is said above, and I'd like to expand on it after having more experience using the in path.

AIUI, clojure.spec is not intended to provide easy-to-read error messages. Rather, it is intended to provide a solid foundation so libraries can present errors in a variety of ways.

In my experience in trying to present errors in a different way (Expound), I can report that one of the biggest challenges in trying to interpret the in path.

There are two cases in particular that are challenging: integer indexes that indicate "key" or "value" failures in a map-of spec, and integer indexes that indicate positions in "key/value" vectors in coll-of specs.

I may be just failing to understand the way the 'in' path works, but anecdotally, this is a source of confusion for other library authors. https://groups.google.com/forum/#!topic/clojure/ppnWBJhz-R4 . Additionally, since this path is shown to users when using explain, a less ambiguous path would help all spec users better understand their errors.

Would it be possible to create unambiguous paths that do not require the reader to know anything about how the path is used? As noted above, the path [:key 1] can only be disambiguated in by knowing something about the failing data structure. This would likely require replacing the integer indexes with custom records. That would reduce conciseness and readability, but this might be able to be alleviated with new reader macros?





[CLJ-2185] Standardize the running context of all Java to Clojure entry points. Created: 22/Jun/17  Updated: 22/Jun/17

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

Type: Enhancement Priority: Minor
Reporter: Didier A. Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: bootstrap, interop

Approval: Triaged

 Description   

Clojure always must be handed the program counter from Java, the JVM can't load right into Clojure. So there's a few ways you can do it:

1) With a gen-class
2) From an already bootstrapped REPL
3) With clojure.main
4) With the Clojure runtime (RT.java)
5) With the Clojure Java API (https://clojure.github.io/clojure/javadoc/clojure/java/api/Clojure.html)
6) With popular tools such as Leiningen and Boot

Any time you want to run some Clojure code from Java, you need to do it with one of those mechanisms, and you cannot run Clojure code without going to Java first.

The issue is that, not all mechanisms hand over execution with an equal context.

#1, #4 and #5 all hand over execution from the context that the Clojure runtime executes in. That means that you get a minimal context running inside the "clojure.core" namespace. No binding is setup, and ns points to "clojure.core".

#3 and #6 seem all to hand over execution with a similar context, but one that's different from #1, #4 and #5. That is, they set common bindings such as ns, and set the current namespace to a freshly created namespace generally called user which has clojure.core referred in it. Meaning that code executed from #3 and #6 will be able to set common bindings and won't interfere with the clojure.core namespace.

#2's context will depend on how the REPL itself was bootstrapped from Java. In most cases, clojure.main, lein, boot, it will be similar to #3 and #6.

I believe that it would be healthy for Clojure to standardize the execution context when bootstrapping Clojure from Java, so that all above mechanism behave the same. This will make it so that all code will run equally from all bootstrapping mechanism. This is not currently the case, since missing bindings and the clojure.core current namespace can cause certain rare scenarios to fail under #1, #4 and #5, but work under #3 and #6.

To not break backwards compatibility, it would be better to enhance #1, #4 and #5 so that they also set common bindings and change the current namespace to a fresh user namespace with clojure.core referred in it.

Initial discussion on the mailing list: https://groups.google.com/forum/#!topic/clojure/6CXUNuPIUyQ






[CLJ-2181] try accepts multiple catch blocks for the same class Created: 07/Jun/17  Updated: 07/Jun/17

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

Type: Defect Priority: Minor
Reporter: Justin Glenn Smith Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: compiler
Environment:

any


Approval: Triaged

 Description   

try silently accepts multiple catch blocks for the same class, but only the first one gets called

user=> (try (/ 1 0) (catch Exception _ (println "a")) (catch Exception _ (println "b")))
a
nil





[CLJ-2180] Enhancing :path info for s/merge & s/and & s/& to indicate which subspec raised spec error Created: 07/Jun/17  Updated: 07/Jun/17

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

Type: Enhancement Priority: Major
Reporter: Shogo Ohta Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: spec

Approval: Triaged

 Description   

Description

Suppose we want to traverse a spec that caused some spec error, from the root spec embedded in the explain-data to a leaf of the pred that was the actual cause of the error.

We can usually use :path info in the explain-data for such a purpose:

user=> (s/explain-data (s/tuple integer? string?) [1 :a])
#:clojure.spec.alpha{:problems
                     ({:path [1], ;; <- indicates the 1st subspec, ie. integer?, was the cause of the error
                       :pred clojure.core/string?,
                       :val :a,
                       :via [],
                       :in [1]}),
                     :spec ...,
                     :value [1 :a]}
user=>

If we traverse the spec tree along the :path, we can eventually reach the leaf pred that raised the spec error.

In some cases, however, it doesn't hold since some specs such as s/merge, s/and and s/& don't put any clue into :path that tells which subspec actually raised the error:

user=> (s/explain-data (s/merge (s/map-of integer? string?)
                                (s/coll-of (fn [[k v]] (= (str k) v))))
                       {1 "2"})
#:clojure.spec.alpha{:problems
                     ({:path [], ;; <- doesn't tell us anything at all
                       :pred (fn [[k v]] (= (str k) v)), ;; <- we don't know which subspec this pred occurs in
                       :val [1 "2"],
                       :via [],
                       :in [0]}),
                     :spec ...,
                     :value {1 "2"}}
user=>

To achieve our purpose even in those cases, we have to make a nondeterministic choice: that is, choose a subspec arbitrarily and try traversing it down, and if something is wrong along the way, then backtrack to another subspec and so on.

From my experience that I implemented that backtracking algorithm in a library I'm working on (repo), I think it's much harder to implement correctly than necessary. In fact, my implementation is probably broken in some corner cases, and I don't even know if it's possible in theory to implement it completely correctly.

Proposal

To make it easier to implement the spec traversal, this ticket proposes adding the index into :path that indicates which subspec raised the spec error for s/merge, s/and and s/&, as follows:

user=> (s/explain-data (s/merge (s/map-of integer? string?)
                                (s/coll-of (fn [[k v]] (= (str k) v))))
                       {1 "2"})
#:clojure.spec.alpha{:problems
                     ({:path [1], ;; <- indicates the 1st subspec, ie. (s/coll-of (fn [[k v]] (= (str k) v))) has the actual cause of the error in it
                       :pred (fn [[k v]] (= (str k) v)),
                       :val [1 "2"],
                       :via [],
                       :in [0]}),
                     :spec ...,
                     :value {1 "2"}}
user=>

The enhancement, though it is indeed a breaking change, should reduce radically the effort needed to write the code traversing specs along the :path.






[CLJ-2174] Spec generated exceptions/error messages are a regression in terms of the out-of-the-box experience with Clojure. Created: 01/Jun/17  Updated: 10/Nov/17

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

Type: Defect Priority: Major
Reporter: Russ Olsen Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: errormsgs, spec
Environment:

All


Approval: Triaged

 Description   

While it is clear that spec has a lot of advantages in terms of a uniform way to specify the shape of behavior, using spec to catch programming and API errors within Clojure itself has led to error messages that more verbose and less clear than what was there in previous versions. For example if I supply a bad name to defn, in version an earlier version of Clojure I got a clear, relatively English language messages back:

Clojure 1.7.0

user=> (defn 44 [x] x)
IllegalArgumentException First argument to defn must be a symbol clojure.core/defn--4156 (core.clj:281)

In the current 1.9 release, the same mistake generates the following:

Clojure 1.9.0-master-SNAPSHOT

user=> (defn 44 [x] x)
CompilerException clojure.lang.ExceptionInfo: Call to clojure.core/defn did not conform to spec:
In: [0] val: 44 fails spec: :clojure.core.specs.alpha/defn-args at: [:args :name] predicate: simple-symbol?
:clojure.spec.alpha/spec #object[clojure.spec.alpha$regex_spec_impl$reify__1200 0x3c0f93f1 "clojure.spec.alpha$regex_spec_impl$reify__1200@3c0f93f1"]
:clojure.spec.alpha/value (44 [x] x)
:clojure.spec.alpha/args (44 [x] x)
#:clojure.spec.alpha{:problems [{:path [:args :name], :pred clojure.core/simple-symbol?, :val 44, :via [:clojure.core.specs.alpha/defn-args :clojure.core.specs.alpha/defn-
args], :in [0]}], :spec #object[clojure.spec.alpha$regex_spec_impl$reify__1200 0x3c0f93f1 "clojure.spec.alpha$regex_spec_impl$reify__1200@3c0f93f1"], :value (44 [x] x), :args
(44 [x] x)}, compiling:(NO_SOURCE_PATH:1:1)

There is a similar situation with let. Here is the behavior in earlier versions:

user-> (let (a 1) (println a))
IllegalArgumentException let requires a vector for its binding in user:1 clojure.core/let (core.clj:4309)

And in 1.9:

user=> (let (a 1) (println a))

CompilerException clojure.lang.ExceptionInfo: Call to clojure.core/let did not conform to spec:
In: [0] val: (a 1) fails spec: :clojure.core.specs.alpha/bindings at: [:args :bindings] predicate: vector?
:clojure.spec.alpha/spec #object[clojure.spec.alpha$regex_spec_impl$reify__1200 0x31dc339b "clojure.spec.alpha$regex_spec_impl$reify__1200@31dc339b"]
:clojure.spec.alpha/value ((a 1) (println a))
:clojure.spec.alpha/args ((a 1) (println a))
#:clojure.spec.alpha{:problems [{:path [:args :bindings], :pred clojure.core/vector?, :val (a 1), :via [:clojure.core.specs.alpha/bindings
:clojure.core.specs.alpha/bindings], :in [0]}], :spec #object[clojure.spec.alpha$regex_spec_impl$reify__1200 0x31dc339b
"clojure.spec.alpha$regex_spec_impl$reify__1200@31dc339b"], :value ((a 1) (println a)), :args ((a 1) (println a))}, compiling:(NO_SOURCE_PATH:1:1)

Yes all of the information – and more – is there in the 1.9 version.
But the spec error messages are likely to be incomprehensible to anyone relatively new to Clojure and adds to the cognitive load of even experienced Clojure programmers.



 Comments   
Comment by Russ Olsen [ 01/Jun/17 9:15 AM ]

Typos in that first sentence, should have read 'shape of DATA' not behavior, and 'error messages that ARE more verbose'.

Comment by Alex Miller [ 01/Jun/17 2:19 PM ]

Hey Russ, part of this is actually a bug that crept into alpha17 that is tracked with a patch here: https://dev.clojure.org/jira/browse/CLJ-2171

But also, I would like to overhaul the instrument exception reporting as I think it could be a lot clearer about the signature being invoked and how it is wrong.





[CLJ-2173] LispReader.java and EdnReader.java exception messages could be much more informative. Created: 31/May/17  Updated: 31/May/17

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

Type: Enhancement Priority: Major
Reporter: Russ Olsen Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: errormsgs, reader
Environment:

Any


Approval: Triaged

 Description   

The messages in the exceptions thrown by the readers would be much more informative if they included readily available information. There are many instances of this, but to name a few specific instances (all from LispReader.java, though there in most cases there are corresponding problems in EdnReader.java):

  • If the RegexReader class hits an unexpected EOF, it reports "EOF while reading regex". It would be helpful if the message included the first few characters of the regex it was trying to read – available in sb – as a guide to the person trying to locate the problem.
  • The same logic applies to StringReader.
  • In NamespaceMapReader, the error thrown if the namespaced map is not in fact a map could include the namespace symbol.
  • Whenever an odd number of elements in a map is detected, the exception could at least report the number of elements that the bad map did include, something like: "Map literal cannot contain 7 forms. Map literals must contain an even number of forms." Even better would be the first few forms.
  • The "Metadata can only be applied to IMetas" exception in MetaReader is not nearly as helpful as it could be. At the very least it should report the class of the thing that is not an IMeta.
  • With an additional argument, readDelimitedList could report the kind of thing that it was reading in the event that it hit the EOF. Without the additional argument it still report that it hit an EOF while trying to read the first or 4th or 29 element of a collection.





[CLJ-2172] Error message for non integer index into vector could be improved. Created: 31/May/17  Updated: 31/May/17

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

Type: Enhancement Priority: Minor
Reporter: Russ Olsen Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: errormsgs

Approval: Triaged

 Description   

The exception generated in APersistentVector/invoke and also in APersistentVector/assoc when the index is not an integer would be better if it included the class of the object actually passed in and would be better still if it included the actual value.



 Comments   
Comment by Alex Miller [ 31/May/17 7:25 PM ]

Needs assessment of current paths through core that reach this (and whether the change in question would be better or worse when that happens).





[CLJ-2158] multi-spec retag generator in conflict with user tag spec/gen Created: 22/Apr/17  Updated: 25/Apr/17

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

Type: Defect Priority: Major
Reporter: Leon Grapenthin Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: generator, spec
Environment:

JVM


Approval: Triaged

 Description   

Problem: multi-spec does generate retag values on its own from values for which methods have been implemented. However a user can have purposefully speced the retag key differently and multi-spec will generate incompatible values (resulting in a such-that failure). An example is hierarchy dispatch where methods dispatch values aren't necessarily the valid tag values.

Proposed solution: When generating the retag value, multi-spec should first try the existing spec for that key and generate "such-that" it is a possible dispatch value for the multimethod, only generate direclty from the multimethod based mechanism iff there is no spec for the tag key.



 Comments   
Comment by Leon Grapenthin [ 25/Apr/17 3:00 AM ]

Improved proposed solution to cover both "user spec is a subset of dispatch values" and vice versa.





[CLJ-2157] multi-spec doesn't generate possible tags from hierarchy Created: 22/Apr/17  Updated: 22/Apr/17

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: 0
Labels: generator, spec


 Description   

Problem: Even though multi-spec supports hierarchy dispatch of multi-methods, its generator only generates tags that have direct method implementations.

Proposed solution: It should also generate from hierarchy.






[CLJ-2155] clojure.string/index-of has some ^long type hints on let bindings that don't actually do anything Created: 19/Apr/17  Updated: 19/Apr/17

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

Type: Defect Priority: Trivial
Reporter: Kevin Downey Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: None

Approval: Triaged

 Description   

^long type hints on let binding values don't do anything:

user=> (def x 1)
#'user/x
user=> (set! *warn-on-reflection* true)
true
user=> (let [w ^long x] (Long/valueOf w))
Reflection warning, NO_SOURCE_PATH:13:18 - call to static method valueOf on java.lang.Long can't be resolved (argument types: unknown).
1
user=> (let [w (long x)] (Long/valueOf w))
1
user=>

but clojure.string/index-of has at least two cases of them, and even if they did do something, there is no reflective code that would take advantage of those type hints.






[CLJ-2154] Spec macros keys and keys* silently ignores non-keywords given in the vectors for named arguments :req and :req-un Created: 17/Apr/17  Updated: 17/Apr/17

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

Type: Defect Priority: Minor
Reporter: Rovanion Luckey Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: spec
Environment:

Ubuntu 16.04 32-bit, Clojure 1.9.0-alpha15


Approval: Triaged

 Description   

If we try to pass a non-keyword to `clojure.spec/keys` or `clojure.spec/keys*` on the named argument `:opt` or `:opt-un` we get an assertion error:

(spec/valid? (spec/keys :opt ['a 5]) {})
;1. Caused by java.lang.AssertionError
;   Assert failed: all keys must be namespace-qualified keywords
;   (every? (fn* [p1__13667#] (c/and (keyword? p1__13667#) (namespace
;   p1__13667#))) (concat req-keys req-un-specs opt opt-un))

(spec/valid? (spec/keys* :opt-un ['a 5]) {})
;1. Caused by java.lang.AssertionError
;   Assert failed: all keys must be namespace-qualified keywords                    
;   (every? (fn* [p1__13667#] (c/and (keyword? p1__13667#) (namespace               
;   p1__13667#))) (concat req-keys req-un-specs opt opt-un))

But if we do the same for the named arguments `:req` and `:req-un` the arguments are silently ignored and the call to `keys` returns a spec matching any map without any requirements:

(spec/valid? (spec/keys :req ['a 5]) {})
=> true
(spec/valid? (spec/keys :req-un ['a 5]) {})
=> true
(spec/valid? (spec/keys* :req ['a 5]) {})
=> true
(spec/valid? (spec/keys* :req-un ['a 5]) {})
=> true

An assertion should probably be thrown for the `:req(-un)?` args too.






[CLJ-2152] clojure.spec: s/& has a broken form Created: 12/Apr/17  Updated: 10/Nov/17

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

Type: Defect Priority: Major
Reporter: Tommi Reiman Assignee: Alex Miller
Resolution: Unresolved Votes: 4
Labels: spec
Environment:

1.9-alpha15


Approval: Vetted

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

(s/form (s/& integer?))
; (clojure.spec/& #object[clojure.core$integer_QMARK_ 0x5536db54 "clojure.core$integer_QMARK_@5536db54"])





[CLJ-2146] partition-by and partition-all transducers should ensure visibility of state changes Created: 09/Apr/17  Updated: 17/Apr/17

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

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

Approval: Vetted

 Description   

The partition-by and partition-all transducers use state stored in an ArrayList. This state should be protected (for example, by volatile) to ensure visibility if used in a transducing process that moves computations across threads.



 Comments   
Comment by Léo NOEL [ 13/Apr/17 1:00 PM ]

Discussion here : https://groups.google.com/forum/m/#!topic/clojure/VQj0E9TJWYY

Comment by Léo NOEL [ 16/Apr/17 9:47 AM ]

Note that following this logic, transients are as much broken, as they make use of plain arrays.
This paragraph https://clojure.org/reference/transients#_concurrent_use makes me believe this very problem has already been tackled before. Is the discussion available somewhere ?
In my opinion, documentation should be more precise about what is meant by thread isolation, and explain why it is OK to use unsynchronized mutable objects when they're owned by something that enforces sequential processing (agents, go blocks, channels, single-threading, etc).

Comment by Alex Miller [ 17/Apr/17 9:24 AM ]

Transients originally enforced thread isolation by recording and validating the originating thread. This was weakened to allow for transients passed around go blocks in Clojure 1.7 and has been through some rounds of fixes (like CLJ-1580). If there is an issue with them now, please file a separate ticket.





[CLJ-2143] The result of s/form for s/keys* is different from the original form Created: 05/Apr/17  Updated: 10/Nov/17

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

Type: Defect Priority: Major
Reporter: Shogo Ohta Assignee: Alex Miller
Resolution: Unresolved Votes: 1
Labels: spec

Approval: Vetted

 Description   

If s/form is applied to s/keys*, it returns a value completely different from the original form:

user=> (s/form (s/keys* :req-un [::x ::y]))
(clojure.spec/& (clojure.spec/* (clojure.spec/cat :clojure.spec/k clojure.core/keyword? :clojure.spec/v clojure.core/any?)) :clojure.spec/kvs->map mspec__14270__auto__)
user=>


 Comments   
Comment by Alex Miller [ 05/Apr/17 8:57 AM ]

Thanks for logging - I've been working on an approach for this one but never got around to actually logging it.





[CLJ-2134] Update the docstring of `with-redefs` to reflect how little the macro should be used Created: 23/Mar/17  Updated: 23/Mar/17

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

Type: Enhancement Priority: Minor
Reporter: Timothy Baldridge Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring

Approval: Triaged

 Description   

Currently the docstring for `with-redefs` recommends itself for use in testing. However there are a number of reasons why using this macro for testing is suboptimal:

  • `with-redefs` "bindings" are not transferred to new threads since it's a global mutation
  • users can get runtime errors if they redef a primitive type-hinted function to a function taking only objects
  • If parts of the body of `with-redefs` is delayed (via a delay, go block, etc.) that code may not see the new root
  • The mutation is global so it "leaks" outside the current scope into other code that may currently be running in another thread
  • Clojure tends to shun global mutation, and yet this macro isn't marked with a `!` nor properly warns users about the dangers mentioned here

Due to these reasons I often encounter new users using `with-redefs` without understanding the ramifications of doing so. All this behavior makes sense if a user understands how Vars work, but that's a lot of knowledge to take on for a new user.

Suggestion:
Remove the suggestion that `with-redefs` be used in testing
Add a few notes of warning about global mutation, and concurrency issues with the macro.






[CLJ-2133] Clarify documentation for the satisfies? function. Created: 23/Mar/17  Updated: 23/Mar/17

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: David Chelimsky Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: docstring
Environment:

N/A



 Description   

The docs for satisfies? says "Returns true if x satisfies the protocol", but does not define the meaning of "satisfies". The function returns true when type and protocol are referenced in the same call to either extend-type or extend-protocol even when none of the protocol functions are implemented. I think the doc should be specific about this to avoid confusion.






[CLJ-2109] Protocol methods not instrumented Created: 13/Feb/17  Updated: 13/Feb/17

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

Type: Defect Priority: Minor
Reporter: Stuart Sierra Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: spec
Environment:

Clojure 1.9.0-alpha14


Approval: Triaged

 Description   

Spec instrument does not work on protocol methods. Invalid arguments will be accepted silently with no error. Protocol vars are included in the return value of (instrument).

Steps to reproduce

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

(defprotocol P
  (method [this arg]))

(defrecord R []
  P
  (method [this arg]
    (str "R.method called with " (pr-str arg))))

(s/fdef method
  :args (s/cat :this any?
               :arg number?))

(defn wrapped [this arg]
  (method this arg))

(s/fdef wrapped
  :args (s/cat :this any?
               :arg number?))

(test/instrument)

(println (method (->R) "not a number"))

(println (wrapped (->R) "not a number"))

This code produces the output:

R.method called with "not a number"
clojure.lang.ExceptionInfo: Call to #'user/wrapped did not conform to spec:
In: [1] val: "not a number" fails at: [:args :arg] predicate: number?
...

Possible resolutions

1. Add support to instrument for protocol methods
2. Document that instrument does not work on protocol methods, do not return protocol method Vars from (instrument), throw exception if protocol method Vars are included in the symbols passed to (instrument syms)

See also

CLJ-1941 describes a different case where instrument does not work. This issue was identified in a comment.

Workarounds

This issue can be avoided by wrapping protocol methods in normal functions and spec'ing the functions. This is already common practice.






[CLJ-2105] incorrect spec conform when an optional (?) is inside of a one or more (+) Created: 03/Feb/17  Updated: 03/Feb/17

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

Type: Defect Priority: Major
Reporter: Anthony D'Ambrosio Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: spec
Environment:

1.9.0-alpha14


Approval: Vetted

 Description   
(s/def ::thing (s/cat :a (s/? string?) :b (s/+ number?))) 
(s/def ::seq-of (s/+ ::thing))

(s/conform ::seq-of (list "foo" 1 "bar" 2 3 "qux" 4)) 
;=> [{:a "foo", :b [1]} [{:a "bar", :b [2 3]} {:a "qux", :b [4]}]]

;EXPECTED 
;=> [{:a "foo", :b [1]} {:a "bar", :b [2 3]} {:a "qux", :b [4]}]

;; ONLY 2nd thing matters? 
(s/conform ::seq-of (list "foo" 1 2 "bar" 3)) 
;=> [{:a "foo", :b [1 2]} {:a "bar", :b [2]}]

;; NO OPTIONAL 
(s/def ::thing (s/cat :a string? :b (s/+ number?))) 
(s/def ::seq-of (s/+ ::thing))
(s/conform ::seq-of (list "foo" 1 "bar" 2 3 "qux" 4)) 
;=> [{:a "foo", :b [1]} {:a "bar", :b [2 3]} {:a "qux", :b [4]}]

This also only shows up if there are 2+ numbers in the 2nd or later ::thing
AND the problem goes away if I make :a not optional...

Could be related to http://dev.clojure.org/jira/browse/CLJ-2003 ?






[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: 1
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-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-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-2082] Improve documentation of clojure.walk/walk Created: 11/Dec/16  Updated: 24/May/17

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.



 Comments   
Comment by Martin Clausen [ 24/May/17 1:33 AM ]

The potential recursive behaviour of clojure.walk/walk depends on the inner function passed to it and is not inherent to clojure.walk/walk itself. If you look at the source code for clojure.walk/prewalk and clojure.walk/postwalk, they are both implemented using walk, but passed a recursive inner function.





[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-2079] Generator overrides for spec aliases are not respected Created: 08/Dec/16  Updated: 17/Nov/17

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

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
Comment by Charles Despointes [ 20/Jun/17 1:19 PM ]

I've a somewhat similar issue. I think it is related.
I'm trying to do something like :

(require '[clojure.spec.alpha :as s])
(require '[clojure.spec.gen.alpha :as gen])
(s/def ::bar any?)
(s/def ::foo (s/with-gen any? (fn [] (s/gen ::bar))))
(gen/generate (s/gen ::foo {::bar (fn [] (s/gen int?))}))

I'm somewhat expecting it generates me an integer like it would have with a direct aliasing to ::bar in ::foo definition. But it doesn't and keep the with-gen binded generator.
Is that the same issue or is that an expected behaviour or should i fill a new issue ?





[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-2067] (s/def ::a ::b) throws unable to resolve error if ::b is not defined Created: 22/Nov/16  Updated: 10/Nov/17

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

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-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: 2
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-2041] clojure.spec/keys requires input collections conform to clojure.core/map? Created: 11/Oct/16  Updated: 10/Nov/17

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

Type: Defect Priority: Minor
Reporter: Timothy Baldridge Assignee: Unassigned
Resolution: Unresolved Votes: 9
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.

Comment by Alex Miller [ 08/Mar/17 10:00 AM ]

This is related to CLJ-2080 as it's the same basic issue of what map-of and every-kv actually expect as valid inputs, which is: something that seqs to map entries. We can't really write a predicate for it without an interface "ISeqsToMapEntries" (intentionally bad name) to indicate this. java.util.Map, IPersistentMap, etc imply this, but an object can seq to map entries without meeting all the constraints of those much broader interfaces. ILookup does not and should not imply this.





[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: 3
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: 10/Nov/17

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

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-2025] When a generator fails to gen, state which spec/pred failed Created: 21/Sep/16  Updated: 29/Jan/17

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

Attachments: Text File better-such-that-info.patch    
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.

Comment by Griffin Smith [ 29/Jan/17 6:37 PM ]

It looks like test.check is being updated to support error customization: https://github.com/clojure/test.check/commit/5aea0e275257680b672309b1e940be6dae92c17d . I've got a patch which updates clojure.spec to use it, though obviously it doesn't work since the linked commit hasn't made it into a released version of test.check yet.

Comment by Griffin Smith [ 29/Jan/17 6:40 PM ]

See also http://dev.clojure.org/jira/browse/TCHECK-107 I suppose

Comment by Griffin Smith [ 29/Jan/17 6:51 PM ]

Attached the patch for future reference (`better-such-that-info.patch`).





[CLJ-2021] case where spec/conform -> spec/unform -> spec/conform gives invalid result Created: 12/Sep/16  Updated: 28/Jan/17

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

Comment by Leon Grapenthin [ 28/Jan/17 4:49 PM ]

This can quickly be traced down to :clojure.core.specs/arg-list which is speced as a (s/and <regex> vector?). When unforming, it doesn't create a vector.

Thinking about it, a vcat would be nice for this and similar cases.





[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-2013] Alternative s/cat options not error-reported Created: 24/Aug/16  Updated: 04/Oct/17

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:

alpha14


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

 Description   

This problem was detected in context of this discussion https://groups.google.com/d/msg/clojure/mIlKaOiujlo/tF71zZ2BCwAJ

A minimal version of how specs error reporting failed the users intuition there looks like this:

He used an invalid ns form

(ns foo (require [clojure.spec :as s])) ; should be :require

The error reported by spec:

In: [1] val: ((require [clojure.spec :as s])) fails at: [:args] predicate: (cat :docstring (? string?) :attr-map (? map?) :clauses :clojure.core.specs/ns-clauses),  Extra input
:clojure.spec/args  (foo (require [clojure.spec :as s]))
  clojure.core/ex-info (core.clj:4725)

While the error is technically true, it doesn't show the user /how/ each of the alternative options of the reported s/cat failed.

To get a better understanding why the users data is not correct, he should know precisely what spec tried and how it failed.

A good example of how this works is s/alt, where all failing alternatives are always reported to the user.

The problem has been investigated, first experimentally, then in specs code. Finally, a patch that brings error reporting like s/alts comes attached.

It has been observed that specs error reporting behavior for cat with optional branches is the following:

1. If the cat failed after one or many optional branches, the entire cat is reported as failing
2. If the cat failed after one or many optional branches /and/ a subsequent required branch, only the subsequent required branch is reported with no remarks to the alternative optional branches.

Rule 1 explains the ns example.
Rule 2 can fail the users intuition significantly worse:

(s/explain (s/cat :maybe-num (s/? number?)
                  :keyword keyword?)
           ["3"])

gives

In: [0] val: "3" fails at: [:keyword] predicate: keyword?

The report clearly doesn't address the users intent of putting in a number. Instead he is made to believe that he should have entered a keyword.

Solution:

A simple patch has been programmed that changes op-explain to have the following behaviour:

  • All alternatives that have been tried in a s/cat are reported individually.

It improves the reported errors significantly because it makes clearly transparent how the users data failed the validation.

(ns foo (require [clojure.spec :as s])) ; should be :require

now gives

ExceptionInfo Call to clojure.core/ns did not conform to spec:
In: [1] val: (require [clojure.spec :as s]) fails at: [:args :docstring] predicate: string?
In: [1] val: (require [clojure.spec :as s]) fails at: [:args :attr-map] predicate: map?
In: [1 0] val: require fails spec: :clojure.core.specs/ns-refer-clojure at: [:args :clauses :refer-clojure :clause] predicate: #{:refer-clojure}
In: [1 0] val: require fails spec: :clojure.core.specs/ns-require at: [:args :clauses :require :clause] predicate: #{:require}
In: [1 0] val: require fails spec: :clojure.core.specs/ns-import at: [:args :clauses :import :clause] predicate: #{:import}
In: [1 0] val: require fails spec: :clojure.core.specs/ns-use at: [:args :clauses :use :clause] predicate: #{:use}
In: [1 0] val: require fails spec: :clojure.core.specs/ns-refer at: [:args :clauses :refer :clause] predicate: #{:refer}
In: [1 0] val: require fails spec: :clojure.core.specs/ns-load at: [:args :clauses :load :clause] predicate: #{:load}
In: [1 0] val: require fails spec: :clojure.core.specs/ns-gen-class at: [:args :clauses :gen-class :clause] predicate: #{:gen-class}
:clojure.spec/args  (foo (require [clojure.spec :as s]))
  clojure.core/ex-info (core.clj:4725)

It would be even better if explain-data sorted ::s/problems by length of their :path which would push the first two unintended options to the end.

(s/explain (s/cat :maybe-num (s/? number?)
                  :keyword keyword?)
           ["3"])

now gives

In: [0] val: "3" fails at: [:maybe-num] predicate: number?
In: [0] val: "3" fails at: [:keyword] predicate: keyword?

While examples can be made up where this reporting produces more noise (like defn) I believe its the right tradeoff for aforementioned reasons and:

  • We programmers always ask our users for the most specific information when something went wrong - It is correct to apply the same to specs error reporting
  • Custom error reporters (s/explain-out) get more data to generate narrow reports matching the users intent even better


 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,
...
Comment by Leon Grapenthin [ 29/Sep/17 9:07 AM ]

@alexmiller: How about deciding this before releasing 1.9. since it improves error messages in a major way. Since after the original discussion on the groups there have been more reports of unintuitive error messages that this would fix. Related to this is sorting explanations by length of path, as illustrated above (for which there is no ticket yet).
Please get some input from Rich.

Comment by Ghadi Shayban [ 29/Sep/17 9:15 AM ]

Work is ongoing with spec, and is happening in a separate git repo unimpacted by the Clojure 1.9 release. We'll be able to update the spec dependency independent of Clojure and receive an eventual fix.

Comment by Leon Grapenthin [ 29/Sep/17 10:18 AM ]

This affects compile time error messages of Clojure, not just the opt-in part of spec. This part of compiler error reporting is broken and shouldn't be shipped with a major release. In that regard it really doesn't matter whether spec is a separate dep or not.

Also, users who wait for a stable version of spec won't necessarily update their spec dependency.

Comment by Leon Grapenthin [ 30/Sep/17 11:36 AM ]

Also AFAIK better error reporting is the sole reason that Spec is a dependency of 1.9. - please reconsider.

Comment by Alex Miller [ 04/Oct/17 9:01 AM ]

I don't know whether this will be considered before or after 1.9. Seems reasonable based on what I read.

The sorted by path length does have a ticket at CLJ-2063, which was applied in May and released in spec.alpha 0.1.109.





[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-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-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-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-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-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: 3
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-1975] clojure.spec attempts to make `empty` records Created: 05/Jul/16  Updated: 02/Nov/17

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



 Comments   
Comment by Alex Miller [ 02/Nov/17 3:13 PM ]

In this case, it's considering the s/* to be a non-match and then matching the (Box. 0) against (s/coll-of integer?). This matches the initial predicate (coll?) but falls through to the :else case which presumes it can call `empty`.

You can work around it by adding a :kind predicate to the coll-of:

(s/conform
        (s/cat :boxes (s/* #(instance? Box %))
               :name (s/coll-of integer? :kind #(not (record? %))))
        [(Box. 0) [5]])
;;=> {:boxes [#user.Box{:a 0}], :name [5]}




[CLJ-1968] clojure.test/report :error does not flush *out* when the test fails with an exception Created: 23/Jun/16  Updated: 23/Jun/16

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

Type: Defect Priority: Major
Reporter: Sam Roberton Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: clojure.test

Approval: Triaged

 Description   

Minimal reproduction:

(require 'clojure.test)

(clojure.test/deftest foo-test
  (throw (ex-info "I fail" {})))

(clojure.test/deftest bar-test
  (.println System/out "bar"))

(clojure.test/test-vars [#'foo-test #'bar-test])

Result:

ERROR in (foo-test) (core.clj:4617)
Uncaught exception, not in assertion.
expected: nil
bar
  actual: clojure.lang.ExceptionInfo: I fail
 at clojure.core$ex_info.invokeStatic (core.clj:4617)
...

Note "bar" appearing in the output in the middle of the error report for foo-test.

Analysis:

(clojure.test/report {:type :error, :actual some-exception}) calls stack/print-cause-trace. Unlike other clojure.test/report callpaths, this does not flush on newline. Thus, when tests fail with exceptions and there is anything writing directly to Java's System.out, there can be a large gap between the first part of the error report and the exception trace.

(To explain why this is annoying: we're running Selenium tests via clj-webdriver, and our system under test is logging with log4j via clojure.tools.logging. We invariably see dozens or even hundreds of lines between "expected: ..." and the subsequent "actual: ..." exception trace. This makes it very easy to come to completely the wrong conclusion about when failures occurred with respect to the other events that appear interleaved in the log.)

It would be preferable (in my opinion) if clojure.test/report always constructed the output from each individual invocation into a single string which got written to *out* all at once – that way there could be no way for output to be interleaved from other threads. Absent that, it would at least help a lot if the :error implementation called (flush).






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

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

Type: Defect Priority: Major
Reporter: Alexander Kiel Assignee: Unassigned
Resolution: Unresolved Votes: 3
Labels: spec


 Description   

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

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

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



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

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

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

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

(defn parse-long [s]
  (try
    (Long/parseLong s)
    (catch Exception _
      ::s/invalid)))
Comment by Alexander Kiel [ 12/Jul/16 10:01 AM ]

No change in alpha 10 with the removal of :clojure.spec/any and introduction of any?.

Comment by Sean Corfield [ 12/Sep/16 4:06 PM ]

Another example from Slack, related to this:

(if-let [a 1]
  ::s/invalid)

Fails compilation (macroexpansion) because ::s/invalid causes the spec for if-let to think the then form is non-conforming.

Workaround:

(if-let [a 1]
  '::s/invalid)
Comment by Ambrose Bonnaire-Sergeant [ 05/Sep/17 3:41 PM ]

Another example from the wild: https://github.com/pjstadig/humane-test-output/pull/23

A macro rewriting

(is (= ::s/invalid ..))

to

(let [a ::s/invalid] ...)

resulted in some very strange errors.

Comment by Alexander Kiel [ 13/Sep/17 6:34 AM ]

The macro issues can be solved by just not using ::s/invalid in code directly. I think in general, it better to use the predicate s/invalid?.

Instead of writing:

(= ::s/invalid ...)

one should use

(s/invalid? ...)

But I have no idea to solve the issue where you have ::s/invalid in data which is validated. The function spec for identical? is a good example.

(s/fdef clojure.core/identical?
  :args (s/cat :x any? :y any?)
  :ret boolean?)

world not work.





[CLJ-1960] Bug in clojure.core/mod with large Double argument Created: 14/Jun/16  Updated: 15/Jun/16

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

Type: Defect Priority: Minor
Reporter: William Tozier Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: math, numerics
Environment:

Java 8 update 91 on Mac OS X 10.11.5


Approval: Triaged

 Description   

The `clojure.core/mod` function works just as expected for small positive floating-point dividend and small positive integer divisor. But today I was working on some edge case tests and came across the following inexplicable behavior:

REPL_session
user=> (def big  Double/MAX_VALUE)
#'user/big
user=> (mod big 10)
0.0
user=> (mod big 100)
0.0
user=> (mod big 1000)
1.9958403095347198E292
user=> (mod big 999)
-Infinity
user=> (mod big 998)
0.0
user=> (mod big 997)
1.9958403095347198E292
user=> (mod big 996)
0.0
user=> (mod big 995)
0.0
user=> (mod big 994)
0.0
user=> (mod big 1001)
1.9958403095347198E292
user=> (mod big 1002)
0.0
user=> (mod big 1003)
0.0
user=> (mod big 1004)
-Infinity
user=> (mod big 1005)
0.0

No idea whether this is inherited from a Java bug. I can see nothing special about the values chosen, and I suspect if one scanned it'd be easy to find other glitches.



 Comments   
Comment by Alex Miller [ 14/Jun/16 7:12 PM ]

mod is based on rem - from a glance, mod does not seem to account properly for any case of overflow, and I suspect that's at the root of a lot of these problems.

Comment by Gary Fredericks [ 14/Jun/16 7:15 PM ]

Test.check suggests (mod 6.7772677936779424E16 23) => -8.0 is somewhat close to minimal.

Comment by William Tozier [ 15/Jun/16 12:40 PM ]

Actually, just checked, and rem gives the same results. Thus (rem Double/MAX_VALUE 1001) is 1.9958403095347198E292, and (rem 6.7772677936779424E16 23) => -8.0.





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

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

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


 Description   

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

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

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

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

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

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

The same also happens if all arguments are lists:



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

More odd side effects.

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

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

See comments on CLJ-1953





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

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

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

Not Relevant



 Description   

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

If all inputs are sets, it works.

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

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

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

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

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


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

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

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

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

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

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

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

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

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

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

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

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





[CLJ-1950] cl-format is too slow for production use Created: 05/Jun/16  Updated: 05/Jun/16

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

Type: Enhancement Priority: Minor
Reporter: Alain Picard Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: performance, print
Environment:

Mac OS X - 3GHz i7 16Gb ram


Approval: Triaged

 Description   

Run this example code:

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
(in-ns 'clojure.pprint)

(println "Basic output using raw str.")
(time
(doseq [i (range 1000)]
(apply str (interpose "," [1 2 3]))))

(println "Test 1 - raw cl-format")
(time
(doseq [i (range 1000)]
(clojure.pprint/cl-format nil "~{D^,~}" [1 2 3])))
;; ==> "Elapsed time: 231.345 msecs"

(println "Test 2 - call on the compiled format")
(def myx
(compile-format "~{D^,~}"))

(time
(doseq [i (range 1000)]
(clojure.pprint/cl-format nil myx [1 2 3])))

(println "Test 3 - using a formatter")
(def myy
(formatter "~{D^,~}"))

(time
(doseq [i (range 1000)]
(myy nil myx [1 2 3])))

(time
(dotimes (i 100000)
(format nil "~{D^,~}" '(1 2 3))))

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

It will print something like this:

Basic output using raw str.
"Elapsed time: 2.402 msecs"
Test 1 - raw cl-format
"Elapsed time: 194.405 msecs"
Test 2 - call on the compiled format
"Elapsed time: 87.271 msecs"
Test 3 - using a formatter
"Elapsed time: 199.318 msecs"

So raw `str' is ~ 100X faster.

For reference, on the same hardware, using
SBCL Common Lisp, this test runs in < 1 ms.

There are (at least) 2 problems here:

1. cl-format function begins with a line like:

let [compiled-format (if (string? format-in) (compile-format format-in) format-in)

But there is no api to pass in a compiled-format into it; (as compile-format
is a private function, so can't be used at large) so this is kind of useless.

2. Even using a precompiled formatter is way too slow.

Suggested fix: none, except perhaps warning unwary users that this
function is simply not suitable for tight loops, and should only be
used to pretty print user input strings, etc.

Thank you






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

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

Type: Defect Priority: Minor
Reporter: Kenny Williams Assignee: Unassigned
Resolution: Unresolved Votes: 7
Labels: spec
Environment:

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


Approval: Vetted

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

user=> (foo 5.2)
ClassCastException clojure.spec.test$spec_checking_fn$fn__13069 cannot be cast to clojure.lang.IFn$DO
       	user/eval6 (NO_SOURCE_FILE:5)
       	user/eval6 (NO_SOURCE_FILE:5)
       	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--9704/fn--9707 (main.clj:241)
       	clojure.main/repl/read-eval-print--9704 (main.clj:241)
       	clojure.main/repl/fn--9713 (main.clj:259)
       	clojure.main/repl (main.clj:259)
       	clojure.main/repl-opt (main.clj:323)
       	clojure.main/main (main.clj:422)

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

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



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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

(defn foo
  []
  (symbol 'foo))

(s/instrument-all)

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

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

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

(defprotocol P
  (m [_]))

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

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

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

(s/instrument-all)

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

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

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

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





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

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

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

All



 Description   

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






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

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

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

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



 Description   

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

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

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

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

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



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

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

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

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

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

Both of these return false on both JDKs:

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

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

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

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

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

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

IMHO the only sane approach would be to:

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

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

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

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

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

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





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

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

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


 Description   

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

Repro:

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

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

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



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

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

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





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

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

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


 Description   

Two issues regarding the documentation of core.reducers

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





[CLJ-1907] Document non-caching behaviour of `iterate` when used as generator Created: 31/Mar/16  Updated: 31/Mar/16

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

Type: Enhancement Priority: Minor
Reporter: Nicola Mometto Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring

Approval: Triaged

 Description   

The non-caching behaviour of `iterate` when used as a generator is not documented and counter-intuitive. It should be documented, just like it's documented for e.g. `eduction`.

Even though the docstring for `iterate` requires `f` to be side-effect free, `f` might take a long time to compute, in which case users should be wary that the computation might happen more than once.






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

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

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


 Description   

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

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

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

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

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

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

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

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


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

run.sh in the linked github repo throws:

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

and fails to compile the required java source

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

Comment by Arnout Roemers [ 14/Jul/17 6:47 AM ]

I'm not sure the issue is (only) due to having an AOT compiled library. I started rewriting parts of the slf4j-timbre library, in order to remove the need for AOT [1]. But I noticed the problem in that version remains when the TimbreLoggerAdapter loads the slf4j-timbre.adapter namespace, instead of a Clojure namespace, during compilation. So as soon as a SLF4J log statement is performed during compilation, the `_thunk_` issue still ensued when running the result.

A workaround however is to require the slf4j-timbre.adapter namespace as one of the first in the AOT compiled namespaces. This also works with the current AOT compiled version of slf4j-timbre library. So maybe solving this issue has more to do with transitive loading of namespaces through Java classes during AOT compilation, instead of with AOT compiled libraries per se?

[1] https://github.com/hellodata-org/slf4j-timbre/blob/f9a2f4469fd92063c261276479593de39fffbee3/src-java/com/github/fzakaria/slf4j/TimbreLoggerAdapter.java





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

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

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

all


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

 Description   

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

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

This is inconsistent with doc and normal behavior :

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

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



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

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

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

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

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

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

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

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

There is a test case that should already fail :

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

I get

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

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

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

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

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

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

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

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

More strict tests checking for a returned vector.





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

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

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


 Description   

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






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

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

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

Approval: Triaged

 Description   

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

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

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

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






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

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

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

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

 Description   

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



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

simple implementation attached





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

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

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

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

 Description   

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

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

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



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

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





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

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

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

Crappy Linux VM running RHEL6

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



 Description   

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

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

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

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

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

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

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

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



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

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





[CLJ-1872] empty? is broken for transient collections Created: 26/Dec/15  Updated: 10/Nov/17

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

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

Approval: Vetted

 Description   

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

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

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

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

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

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



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

Probably similar to CLJ-700.





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

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

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

Approval: Triaged

 Description   

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

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

Demonstration of the effect:

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

;; Everything above is as expected

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


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

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





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

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

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

Attachments: Text File clj-1866.patch    

 Description   

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

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



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

Example code where this is an issue?

Benchmark before/after ?

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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





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

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

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


 Description   

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

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

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

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

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

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

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

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

Redefining the same function twice makes it work.

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

(defn a [x]
  (a x))

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

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

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

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

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


 Comments   
Comment by Nicola Mometto [ 06/Jan/17 10:12 AM ]

I just took a quick look at this, not an easy one to fix as handling recursive calls would likely require 2 passes over the whole defn AST, one to determine whether the defn is direct-linkable and the other one to build the AST using StaticInvokeExpr rather than InvokeExpr (using a stub Method using the analysis info rather than reflecting)

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

Yeah, Rich is aware of this and it's not done yet due to the issues you mentioned. It's hard!





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

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

Type: Defect Priority: Major
Reporter: Timothy Baldridge Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: compiler, errormsgs, typehints

Approval: Triaged

 Description   

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

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



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

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

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

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

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

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

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

Comment by Alex Miller [ 10/Jul/17 2:09 PM ]

Description could use some examples





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

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

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

Attachments: File CLJ_1857_split_docs_update.diff    

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

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

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


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

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

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

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

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

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

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





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

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: Martin Raison Assignee: Unassigned
Resolution: Unresolved Votes: 10
Labels: compiler
Environment:

tested on CentOS 6


Approval: Triaged

 Description   

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

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

(def myf (nestfn 100 "body"))

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



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

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

Comment by Philipp Neumann [ 16/Oct/17 10:53 AM ]

I tried clojure.core.match with 13 patterns and the compiliation failed under Windows. I assume this problem is the root cause of it.





[CLJ-1821] Move map-invert from clojure.set to clojure.core Created: 28/Sep/15  Updated: 28/Sep/15

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

Type: Enhancement Priority: Minor
Reporter: Lars Andersen Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

map-invert is hard to find when it lives in the clojure.set namespace, because it acts on maps and not sets. To my eyes set/map-invert also looks strange when reading code, but this is the preferred way to bring in the clojure.set namespace.

This is one of the minor warts I'd like to see fixed in clojure 2.0.






[CLJ-1820] Move rename-keys from clojure.set to clojure.core Created: 28/Sep/15  Updated: 12/Jul/17

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

Type: Enhancement Priority: Trivial
Reporter: Lars Andersen Assignee: Unassigned
Resolution: Unresolved Votes: 5
Labels: None


 Description   

rename-keys is hard to find when it lives in the clojure.set namespace, because it acts on maps and not sets. To my eyes set/rename-keys also looks strange when reading code, but this is the preferred way to bring in the clojure.set namespace.

This is one of the minor warts I'd like to see fixed in clojure 2.0.



 Comments   
Comment by Gordon Syme [ 04/Jul/17 4:56 AM ]

It's not just rename-keys, there are a few fns in clojure.set that don't make sense being there, at first glance anyway.

It certainly harms discoverability of these fns.

I've come across at least one re-implementation of rename-keys and map-invert during my day job because the author didn't know these fns exist.

I'd argue for breaking the relational and map fns out of clojure.set into their own namespaces and deffing some vars in clojure.set for backwards compatibility.
Those compatibility vars could be deleted in 1.10.

I'm happy to do this (or another approach) but would like some buy in on the approach from the core team first.

Comment by Nicola Mometto [ 04/Jul/17 6:25 AM ]

I think this is very, very unlikely to ever happen

Comment by Alex Miller [ 04/Jul/17 11:38 AM ]

We will not delete/move existing vars as this would break existing programs.

Comment by Marc O'Morain [ 12/Jul/17 4:49 AM ]

> We will not delete/move existing vars as this would break existing programs.

This issue can be addressed without deletion as per Gordon's suggestion:

A new var could be added to clojure.core named clojure.core/rename-keys. Then the rename-keys var in clojure.set can be defined as:

(def rename-keys clojure.core/rename-keys)
Comment by Alex Miller [ 12/Jul/17 6:08 AM ]

I am aware of that, which is why I did not close the issue. I was just stating one possible resolution that is off the table.





[CLJ-1818] cl-format does not respect aesthetic ~A with a keyword Created: 26/Sep/15  Updated: 12/Jan/16

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

Type: Defect Priority: Trivial
Reporter: Jong-won Choi Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: print

Approval: Triaged

 Description   

In Common Lisp, (format nil "~a" :A) returns "A". However, in Clojure, it returns with the colon:

(clojure.pprint/cl-format false "~a" :A)
=> ":A"


 Comments   
Comment by Jong-won Choi [ 28/Sep/15 6:26 AM ]

Found another problem of cl-format:

(clojure.pprint/cl-format false "SELECT * from RateSchedules ~@[WHERE ~{~A=?~^ ~}~]" '())
=> "SELECT * from RateSchedules WHERE" ;; instead of "SELECT * from RateSchedules"

I guess the problem is () or [] has to be treated as falsey but not.

Comment by Alex Miller [ 28/Sep/15 9:58 AM ]