<< Back to previous view

[CLJ-2313] string capture mode ignores readLine Created: 19/Jan/18  Updated: 19/Jan/18  Resolved: 19/Jan/18

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

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

Attachments: Text File 0001-CLJ-2313-fix-string-capture-in-readLine.patch    
Patch: Code
Approval: Ok

 Description   

Before patch:

user=> (.captureString *in*)
nil
user=> 1 ;2 3 4
1
user=> 5 6 7
5
6
7
user=> (.getString *in*)
"1 ;25 6 7\n(.getString *in*)\n"

(Note how everything after the first character after a comment gets ignored, until a newline)

After patch:

user=> (.captureString *in*)
nil
user=> 1 ;2 3 4
1
user=> 5 6 7
5
6
7
user=> (.getString *in*)
"1 ;2 3 4\n5 6 7\n(.getString *in*)\n"

Patch: 0001-CLJ-2313-fix-string-capture-in-readLine.patch



 Comments   
Comment by Nicola Mometto [ 19/Jan/18 9:17 AM ]

Committed by Rich: https://github.com/clojure/clojure/commit/76cb5cfdd446db69700c7c619c738a4ef2026947

Comment by Alex Miller [ 19/Jan/18 9:21 AM ]

Set fix version





[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-2310] Note that file-seq doesn't guarantee file order Created: 07/Jan/18  Updated: 07/Jan/18

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

Type: Enhancement Priority: Minor
Reporter: Yegor Timoshenko Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring

Attachments: Text File 0001-Note-that-file-seq-doesn-t-guarantee-file-order.patch    
Patch: Code

 Description   

file-seq uses java.io.File.listFiles that has a note about order not being portable across filesystems: https://docs.oracle.com/javase/9/docs/api/java/io/File.html#listFiles--



 Comments   
Comment by Alex Miller [ 07/Jan/18 7:51 AM ]

Given that the docstring does not explicitly or implicitly imply an order, and there is no such natural ordering, this seems obvious to me without stating it?

Comment by Yegor Timoshenko [ 07/Jan/18 8:06 AM ]

I had a bug in a program where I presumed that file-seq would always return a sorted sequence, and I've catched it only after HFS+ -> APFS switch (file-seq always returns alphabetically sorted sequence in HFS+). I thought JVM would abstract away differences in file systems, and I was wrong. (I understand they don't do that for performance).

Given that, unlike sets or maps, sequences are ordered, I think it would be valuable to have this quality explicitly noted in the docstring, because it might not be evident just from the output.





[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-2308] Reduce based some Created: 04/Jan/18  Updated: 20/Jan/18

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

Type: Enhancement Priority: Major
Reporter: Petter Eriksson Assignee: Petter Eriksson
Resolution: Unresolved Votes: 0
Labels: performance

Attachments: Text File CLJ-2308.patch    
Patch: Code

 Description   

clojure.core/some is implemented using first and recur next. It could be implemented using reduce and reduced to gain a significant speedup on large collections.
I benchmarked range, iterate, vector, sorted-set and a lazy-seq using criterium and got the following results:

  clojure.core/some reduce some :speed-up
:iterate 14.502145 ms 3.994055 ms 3.63x
:range 16.949429 ms 14.903065 ms 1.14x <-- This result is for clojure.lang.Range
:vector 23.706839 ms 5.765865 ms 4.11x
:lazy-seq 28.723150 ms 5.616475 ms 5.11x
:set 53.063608 ms 17.419191 ms 3.05x

Each collection was of size 1e7, and some was called with (some #(< 1e6 %) coll), taking up 1m elements from the collection.

I ran these test on an uberjar with the following java opts:
java -Xmx3g "-Dclojure.compiler.direct-linking=true" -server

The reduce version of some I used and the benchmarking code can be found in this gist:
https://gist.github.com/petterik/63fad1126842ba4550dcb338328e2975



 Comments   
Comment by Petter Eriksson [ 04/Jan/18 5:35 PM ]

I messed up the formatting in the description and I can't find how to edit it (possibly because I don't have permission?). Removing the dotted lines should be somewhat good enough.

Comment by Andy Fingerhut [ 04/Jan/18 5:46 PM ]

I have bumped up the permissions on your JIRA account, since you are on the list of people who have signed a Clojure Contributor Agreement. You should be able to edit the ticket now.

If you have a proposed change to make to Clojure, I'd recommend creating a patch using the instructions found at the "Developing Patches" link on this page: https://dev.clojure.org/display/community/Contributing

Comment by Petter Eriksson [ 04/Jan/18 5:58 PM ]

Sure, Andy. I'm initiating a move tomorrow, so I'll get to it next week.

Comment by Petter Eriksson [ 20/Jan/18 5:51 PM ]

Attached CLJ-2308.patch - 20/Jan/18 5:51 PM:

Implemented clojure.core/some and clojure.core/every? with reduce and early termination with reduced.

To get this to work I had to:

  • Use reduce1 instead of reduce
  • Move deref and reduced above reduce1
  • Take care of reduced values in reduce1

I've benchmarked reduced based clojure.core/every? with collection sizes of 1e4 and pred #(< % 1e3):

  clojure.core/every? reduce every? :speed-up
:iterate 16.897504 µs 8.273847 µs 2.04x
:long-range 15.197158 µs 8.658177 µs 1.76x
:vector 27.534283 µs 2.519784 µs 10.93x
:lazy-seq 25.457922 µs 6.393590 µs 3.98x
:set 56.421657 µs 17.143458 µs 3.29x

Since the results were good and some is so similar to every?, I went ahead and did the change for every?.





[CLJ-2307] #{:} or [:] work fine in state atom but after spit choke on slurp Created: 04/Jan/18  Updated: 05/Jan/18  Resolved: 05/Jan/18

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

Type: Defect Priority: Minor
Reporter: Markus Graf Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None


 Description   

Hi,

I have a small luminus-ring-app that keeps its state in an atom containing a 'map of maps|vectors|sets' on the server.
When something changes I spit this to a 'file.edn'.
When the service on the server restarts it reads the data from 'file.edn' back into the atom.
I have users in the 'map of maps|vectors|sets' that carry a set of rights for access control.
I love that the Keyword can then be given the set-of-rights to have a simple accesscontrol.

(if (:admin set-of-rights) showadminpage)

A web form allows giving a user the right to see some resource.
The web form returns a string, which I turn into a keyword and add it to the set of access rights.

(conj set-of-rights (keyword new-access-string))

When I forgot to write code to check for the empty string the following happened when new-access-string was "":

The app worked fine, until the service restarted. The test build worked fine but the production system would not restart.

This, of course, hit me when I updated the app to a new version. Since it worked before the update I thought the update caused the error.
So I did a roll back. The app still would not start. I found out that slurp 'file.edn' gagged ...

The core issue is clj-732 and clj-2014 and I accept, that it will not be fixed there (http://clojure.org/guides/faq#unreadable_keywords).

But it would be nice, if a state atom can be spit to a file and slurped back in.
Would it, perhaps, be feasible to check the set or vector containing the bad keyword while writing/spitting or reading/slurping?

After dabbling in Basic, Pascal and C64 and Amiga I did not go into computer science.
Not quite 2 years ago a friend showed me clojure.
I love it! I decided to learn serious programming after all and it is my first programming language now. If it weren't for clojure I would not have come back into coding. It is a magical tool!
Thank you!!!!!!!!



 Comments   
Comment by Andy Fingerhut [ 04/Jan/18 5:01 PM ]

Have you seen this FAQ, linked from CLJ-2014? https://clojure.org/guides/faq#unreadable_keywords

I have no decision-making power in this matter – just passing on possibly-relevant info. A state atom can be spit out and slurped back in, as long as you only include contents that can be read back in, i.e. no unreadable keywords, no function objects, no Java objects (or perhaps only a carefully selected subset of them), etc.

Have you considered not using keywords, but strings, as keys in the map / elements of the set? Strings can contain anything a user could enter in a web form.

Comment by Markus Graf [ 05/Jan/18 5:28 AM ]

Hello Andy,

thank your for taking the time.

Yes I have seen the FAQ, I actually linked to it in the text

I will probably take your advice and use strings for things like that
in the future. I probably liked the concept of keywords to much,
especially with the possibility to write (if (:keyword set) do else).
That should work with the set in function position as well, thank you.

Why I filed this issue:

I did not mean to file this under bug, implying that someone did
something wrong, apart from me, of course when not checking for the
empty string. I would have filed this under quirks if jira would have
offered that category.

The fact that there is a faq on the matter shows that more people than
me hit there toes on this one. The state of things seems to be, that
some newbies stumbles over this. It is a rite of passage so to speak.

If there is a pump in the road we can:
1. get rid of the bump.
2. put a big warning sign before the bump.
3. put a repair shop after the bump.

An FAQ is what people need after something broke. And in this case
likely an edge case, likely in production, likely when you are new to
clojure, likely on your first in house test to evaluate the language.

I am not saying clojure does something wrong here, I am just saying
people seem to get bitten when reading keywords from collections
generated by (keyword).

Possible ways of putting up a warning sign could be to:

  • Put a warning in the doc string of keyword.
  • Get exercises into 4clojure and clojure koans and manuals so people
    get sensitized.
  • Put a warning in the cheatsheet

Getting rid of the bump altogether would be my first choice, maybe in 'spit'.

I expect there to be many more newbies to clojure

Or maybe this is an important lesson to learn by hitting your toes.
That's the way I learned it and it is a memorable lesson.
Maybe I wouldn't have learned as deeply with a warning

Greetings and have a formidable 2018!

Markus

Comment by Alex Miller [ 05/Jan/18 10:10 AM ]

As Andy mentioned, things are working as intended here so I'm going to decline the ticket. However, I have also filed enhancement CLJ-2309 to track the idea of either creating a validating version of `keyword` or a readable keyword predicate.





[CLJ-2306] Remove unused Var.rev Created: 03/Jan/18  Updated: 03/Jan/18

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

Type: Enhancement Priority: Minor
Reporter: Nathan Fisher Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: var
Environment:

all


Attachments: Text File remove-rev.patch    
Patch: Code
Approval: Prescreened

 Description   

Var.rev is unused. The usage that does exist is not synchronised correctly.

Alex suggested it be removed in this discussion;

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

The attached patch removes rev from Var.

Prescreened: Alex Miller






[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-2300] `explain-data` returns `nil` reason for IFn spec failure (which seemingly shouldn't be a failure to begin with) Created: 21/Dec/17  Updated: 21/Dec/17  Resolved: 21/Dec/17

Status: Closed
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: Declined 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)



 Description   
user> (require '[clojure.spec.alpha :as s])
nil
user> (defn printable-fn [f to-string]
  (reify clojure.lang.IFn
    (toString [this] (to-string))
    (invoke [this a] (f a))))
#'user/printable-fn
user> (defn printable-const-fn [constant]
  (printable-fn
   (fn [_] constant)
   #(format "(fn [_] %s)" (if (string? constant) (format "\"%s\"" constant) constant))))
#'user/printable-const-fn
user> (s/explain-data (s/fspec :args (s/cat :a any?) :ret boolean?) (printable-const-fn true))
#:clojure.spec.alpha{:problems [{:path [], :pred (apply fn), :val (0), :reason nil, :via [], :in []}], :spec #object[clojure.spec.alpha$fspec_impl$reify__2451 0x1479eb26 "clojure.spec.alpha$fspec_impl$reify__2451@1479eb26"], :value #object[user$printable_fn$reify__35128 0x74e99361 "(fn [_] true)"]}
user> (clojure.pprint/pprint *1)
#:clojure.spec.alpha{:problems
                     [{:path [],
                       :pred (apply fn),
                       :val (0),
                       :reason nil,
                       :via [],
                       :in []}],
                     :spec
                     #object[clojure.spec.alpha$fspec_impl$reify__2451 0x1479eb26 "clojure.spec.alpha$fspec_impl$reify__2451@1479eb26"],
                     :value
                     #object[user$printable_fn$reify__35128 0x74e99361 "(fn [_] true)"]}
nil
user>


 Comments   
Comment by Alex Miller [ 21/Dec/17 2:29 PM ]

IFn also has `applyTo` and spec will try to invoke the function using `apply`.

Use this:

(defn printable-fn [f to-string]
  (reify clojure.lang.IFn
    (toString [this] (to-string))
    (invoke [this a] (f a))
    (applyTo [this x] (apply f x))))  ;; added
Comment by Alex Miller [ 21/Dec/17 2:29 PM ]

Invalid function implementation

Comment by Jonathan Leonard [ 21/Dec/17 2:35 PM ]

Ah, thank you!

FWIW, I grabbed that code from:
https://stackoverflow.com/questions/5306015/equivilent-of-javas-tostring-for-clojure-functions/5306202#5306202

[left a comment there requesting the update]

Comment by Jonathan Leonard [ 21/Dec/17 2:38 PM ]

Is the fact that this reports a `nil` reason though not something that may want to be improved? [I likely would have figured it out on my own if it weren't for that fact. :) ]

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

:reason is an optional key and isn't always set.





[CLJ-2299] spec passing nil as second argument to `ExceptionInfo` constructor... Created: 21/Dec/17  Updated: 21/Dec/17  Resolved: 21/Dec/17

Status: Closed
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: Not Reproducible 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)



 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 Alex Miller [ 21/Dec/17 2:38 PM ]

The assumption made in the code here is that this function is only called when either valid? or conform has found that the value does not conform to the spec. In all cases, the same inputs should return an explain-data when requested. So, those assumptions are correct and this code is also correct.

However, clearly you have encountered a scenario where the spec conform is not holding to that and that is a bug. Unfortunately, I have no way to reproduce it, so I have to close this ticket as not reproducible. If you are able to reproduce this with an example spec+value, I'd be happy to re-open.

Comment by Jonathan Leonard [ 21/Dec/17 2:44 PM ]

Fair enough. I will work on getting a reproduction (which is what the `printable-fn` wrapper from the other ticket is intended to help with).





[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-2296] Refactoring vec function Created: 19/Dec/17  Updated: 20/Dec/17  Resolved: 20/Dec/17

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

Type: Enhancement Priority: Minor
Reporter: Ertuğrul Çetin Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None


 Description   

vec function could be re-written like this:

(defn vec
"Creates a new vector containing the contents of coll. Java arrays
will be aliased and should not be modified."
{:added "1.0"
:static true}
([coll]
(if (and (vector? coll) (instance? clojure.lang.IObj coll))
(with-meta coll nil)
(clojure.lang.LazilyPersistentVector/create coll))))



 Comments   
Comment by Nicola Mometto [ 19/Dec/17 12:44 PM ]

`and` is defined after `vec` in clojure.core, so this refactoring is not as trivial as you're suggesting

Also this would have virtually no impact on correctness or performance so it seems unlikely that this is something the core team will be interested in

Comment by Alex Miller [ 20/Dec/17 7:48 AM ]

Hi Ertuğrul, thanks for the idea. This was considered the last time `vec` and `set` were overhauled (in 1.7).

As Nicola mentions, the bootstrap ordering is a difficult factor. There are also costs to doing this check for non-vector cases and it requires considering the frequency of different types on calls to `vec`. In summary, the current state is the result of a lot of perf testing and considerations and we don't plan to revisit it for the time being.

Comment by Ertuğrul Çetin [ 20/Dec/17 7:53 AM ]

Hi Alex and Nicola,

Fair enough. Thanks for the feedback!





[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-2293] Compiler allows duplicated function parameter names Created: 14/Dec/17  Updated: 17/Dec/17  Resolved: 15/Dec/17

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

Type: Enhancement Priority: Minor
Reporter: Matthew Phillips Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: compiler
Environment:

MacOS, Java 8



 Description   

The Clojure compiler allows a function parameter name to appear more than once, silently selecting the last one. This makes sense for "_", but not much for anything else (and just cost me some serious head scratching!).

The compiler considers other duplicated things (such as map keys) as an error: I'd expect this to be one, too.

user> (defn t [a a a] a)
#'user/t
user> (t 1 2 3)
3



 Comments   
Comment by Andy Fingerhut [ 14/Dec/17 6:26 PM ]

Surprisingly, the Clojure lint tool Eastwood does not warn about this. Seems like a straightforward thing to add to it, but no promises when/if it will be.

Comment by Nicola Mometto [ 15/Dec/17 9:57 AM ]

this is intentional, it works just like normal let shadowing

think about `(fn [a _ _] .. )`

Comment by Alex Miller [ 15/Dec/17 1:19 PM ]

This is as designed and would likely break existing code to change.

Comment by Matthew Phillips [ 15/Dec/17 11:08 PM ]

Fair enough, I kind of thought that might be the case at this point in Clojure's lifetime.

@Nicola: I don't see this as the same as let shadowing: parameter symbols are all defined in parallel, but those in a 'let' can, and so it makes sense to allow shadowing for redefinition. '' seems like a special case: '' seems to mean 'don't care' (but each '_' is still a different anonymous thing, same as assigning to a logical gensym which we don't then refer to).

@Andy: Yes, would be nice if Eastwood could catch this at least. I can't imagine an example where human-written code would want to shadow like this.

Thanks!

Matt.

Comment by Matthew Phillips [ 15/Dec/17 11:25 PM ]

Oops, my reply to Nicola got mangled by some ill-advised last minute edits: I meant to say "I don't see this as the same as let shadowing: parameter symbols are all defined in parallel and can't refer to each other, but those in a 'let' can, and so it makes sense to allow shadowing for redefinition. '' seems like a special case: '' seems to mean 'don't care' (but each '_' is still a different anonymous thing, same as assigning to a logical gensym which we don't then refer to)."

Comment by Nicola Mometto [ 16/Dec/17 11:40 AM ]

`_` is no different to any other symbol at all, it's just used by convention to mean "ignored local"

Comment by Matthew Phillips [ 17/Dec/17 1:01 AM ]

Thanks Nicola, no I do get that. I just meant that, while '' may not be special to the compiler, it is to people. And thus surprising if anything but '' is allowed to be duplicated without error. Generated code should use a gensym (or equivalent), so duplicated non '_' parameters are almost certainly a bug.

Anyway, moving on. Thanks for the feedback.





[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-2288] Add :into parameter to s/keys for spec Created: 12/Dec/17  Updated: 13/Dec/17  Resolved: 13/Dec/17

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

Type: Enhancement Priority: Major
Reporter: Leon Mergen Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: spec


 Description   

As with other spec functions that operate on similar data structures (e.g. map-of), in some situations it can be desirable to control the exact collection type generated by s/gen (for example, when you require a sorted-map).

While you can work around it with writing a custom :gen generator, a simpler solution for the end-user might be to add a :into parameter for s/keys.

If a patch for this would be accepted, I have no problem contributing it.



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

Not interested, thanks.





[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-2286] Update `clj` REPL with hint: (use Crtl-D to exit) Created: 11/Dec/17  Updated: 11/Dec/17

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

Type: Enhancement Priority: Minor
Reporter: Alan Thompson Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: repl
Environment:

All


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

 Description   

Update clojure.main REPL for the `clj` tool with a hint for terminating the session via Ctrl-D, like so (changes in bold):

~ > clj
Clojure 1.9.0 (use Ctrl-D to exit)
user=>
~ >

The lein repl accepts `exit`, `quit`, and Crtl-D to exit; the `clj` repl accepts only Crtl-D. A small hint upon startup on the proper way to terminate the repl session will smooth the experience for users expecting for their old habits to work.

Patch: clj-2286.patch






[CLJ-2285] locking macro is 2x slower than synchronized block of Java Created: 11/Dec/17  Updated: 11/Dec/17

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

Type: Enhancement Priority: Major
Reporter: Tsutomu Yano Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: performance
Environment:

Mac OS X 10.3.1, Java 1.8.0_144, 3.1 GHz Intel Core i5, Clojure 1.9.0


Attachments: Text File CLJ-2285.patch    
Patch: Code

 Description   

Abstract

`locking` macro is 2x slower than `synchronized` block of Java, although what the `locking` macro do is simply generating `monitorenter` and `monitorexit` opcodes.

This problem is able to be solved by using `synchronized` block of Java instead of generating `monitorenter` and `monitorexit` directly.

Thread in dev mailing-list is: https://groups.google.com/forum/#!topic/clojure-dev/dJZtRsfikXU

Related to

CLJ-1472 have relationship to this ticket. The purpose of CLJ-1472 is different than this ticket, but changes needed for solving each problem are same. So the contents of patches of both tickets almost are same.

BENCHMARKS

I made 2 sample programs for verifying this problem, on which I make many threads and updating a Map from the threads for making highly race-conditional situation occurred artifically.

In Java sample, I use simply Thread and synchronized block on a HashMap.

In Clojure sample, I made 2 samples.
In 1st sample, I used atom for holding an associative (Map) and used `swap!` and `assoc` for updating the associative.
In 2nd sample, I used volatile! for holding an java.util.HashMap and used `locking` on the volatile reference before updating the HashMap.

Java Sample:
https://github.com/tyano/MultithreadPerformance

Clojure Sample:
https://github.com/tyano/clj-multithread-performance

The way to run programs is described on the pages above.

Results on my machine (macos 10.13.1, Java 1.8.0_144, 3.1 GHz Intel Core i5, Clojure 1.9.0):

A. Java sample: 6,006ms
B. Clojure - atom with associative: 18,984ms
C. Clojure - locking on a HashMap: 15,883ms

B (Atom and swap! of Clojure) is slower than the java one, but I can understand why it is. Updating an associative creates a new object. Of cause it uses PersistentMap, so it should have better performance than creating a new copy of full Map instance, but it will be slower than updating a simple java.util.Map instance directly (like Java sample do). And swap! might retry the updating action repeatedly. So this result is understandable for me.

But I think the result of C (locking on a HashMap) (15,883ms) is TOO SLOW.

`locking` macro is just generating `monitorenter` and `monitorexit` directly, it nearly is same with what `synchronized` block do, so the result must be near from the result of Java sample (6,006ms).

INVESTIGATION

I suspect that the generated bytecodes of `locking` will not be same with what `synchronized` generates.
Ticket CLJ-1472 also leads my suspicion. This ticket indicates that the bytecodes `locking` generates is not same with what JDK generates.

My suspicion: `locking` will generate different bytecodes and Java Runtime can not optimize the generated bytecodes well.
Instead of generating opcodes directly, if the `locking` macro wraps a macro-body into a Fn and just calls a java method which invoke the supplied Fn in a synchronized block, Runtime might optimize the code well ?

I made a such patch on `locking` macro (attached on this ticket) and tried a sample C again with the patched clojure:

THE RESULT WAS: 6,988ms !

SUMMARY

the current implimentation of `locking` macro have a performance problem. the bytecode `locking` macro generates will not be optimized well on Java Runtime. so the performance is 2x slower than `synchronized` block of Java.

A patch attached on this ticket can solve this problem.






[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-2279] strange behavior with spec valid? Created: 30/Nov/17  Updated: 30/Nov/17  Resolved: 30/Nov/17

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

Type: Defect Priority: Major
Reporter: Hadi Elmougy Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: bug
Environment:

clojure spec



 Description   

(s/def ::delay (s/and (s/or :val pos? :val zero?) int?))
(s/valid? ::delay 0) => false

(s/def ::delay (s/and int? (s/or :val pos? :val zero?)))
(s/valid? ::delay 0) => true



 Comments   
Comment by Hadi Elmougy [ 30/Nov/17 5:24 AM ]

I got the reason why I got this. I need to close this

Comment by Alex Miller [ 30/Nov/17 7:17 AM ]

Closed per request





[CLJ-2278] clojure.core/bigdec? missing in 1.9.0-RC2 Created: 29/Nov/17  Updated: 29/Nov/17  Resolved: 29/Nov/17

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

Type: Defect Priority: Minor
Reporter: Fenton Travers Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None


 Description   

checked the source at hash: 0592567e, 1.9.0-RC2, seems clojure.core/bigdec? is missing.



 Comments   
Comment by Alex Miller [ 29/Nov/17 8:37 PM ]

bigdec? was added in 1.9, accidentally a dupe of the existing decimal?. It was removed in 1.9.0-beta4. See CLJ-2259.





[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-2276] Add build for standalone jar Created: 27/Nov/17  Updated: 27/Nov/17  Resolved: 27/Nov/17

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

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

Attachments: Text File standalone-3.patch    
Patch: Code
Approval: Ok

 Description   

Until Clojure 1.9, the local ant build produced a standalone executable jar. This is no longer the case due to the external libs (spec.alpha and core.specs.alpha).

The attached patch adds a new Maven profile and ant target for a "local" build that is self-contained and includes the spec jars.

# Maven:
mvn -Plocal -Dmaven.test.skip=true package

# Ant:
./antsetup.sh
ant local

# Run:
java -jar clojure.jar





[CLJ-2275] case fails for vectors with negative numbers Created: 27/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: Major
Reporter: Max Lorenz Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: case
Environment:

OSX


Attachments: Text File 0001-switch-to-hasheq-for-case.patch    
Patch: Code and Test
Approval: Triaged

 Description   
user=> (case -1 -1 true false)
true
user=> (case [-1] [-1] true false)
true
user=> (case (int -1) -1 true false)
true
user=> (case [(int 1)] [1] true false)
true
user=> (case [(int -1)] [-1] true false)
false

The last case should return true like the other,

Real life example that triggered this:

(case [">" (compare 2 3)]
  [">" -1] true
  false) ;; false?

Explaination: This is caused by `case` using `hashCode` instead of hashEq for hash comparisons (when not wrapped in a vector, the comparison is direct) and the fact that negative integers and negative longs hash differently while positive ones hash identical.
Porposal: Make `case` use hasheq instead of hashCode

Patch: 0001-switch-to-hasheq-for-case.patch



 Comments   
Comment by Nicola Mometto [ 27/Nov/17 10:38 AM ]

Patch is a straightforward search and replace, apart from line 30 of the diff which is quite tricky: I believe using `h` there was a bug in the previous impl that never survaced because hashCode on positive integers is idempotent. But since hashEq isn't double hashing would cause collision nodes never to match.





[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-2270] Docstring for `doto` return value is ambiguous Created: 17/Nov/17  Updated: 18/Nov/17  Resolved: 18/Nov/17

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

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


 Description   

The docstring for doto is as follows:

Evaluates x then calls all of the methods and functions with the value of x supplied at the front of the given arguments. The forms are evaluated in order. Returns x.

The phrase "Returns x" is not correct here - x is a form, not a value, so the macro returns the "result of evaluating x", I guess.



 Comments   
Comment by Alex Miller [ 18/Nov/17 11:54 AM ]

This kind of level switch is common for all macros and generally the docstrings are written to convey the intent of the effect, not the mechanics of the macro implementation. I think it's better as currently written, so declining.





[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-2267] Unchecked casts from char throw exceptions for all but int casts Created: 15/Nov/17  Updated: 16/Nov/17  Resolved: 16/Nov/17

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

Type: Defect Priority: Minor
Reporter: Nate Austin Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None
Environment:

All



 Description   

With *unchecked-math* enabled, doing a cast from char to any numeric type (except int) throws an exception.

user=> (byte \space)
32
user=> (set! \*unchecked-math\* true)
true
user=> (byte \space)

ClassCastException java.lang.Character cannot be cast to java.lang.Number  clojure.lang.RT.uncheckedByteCast (RT.java:1326)
user=> (int \space)
32

The other unchecked*Cast(Object) functions in RT should be changed to be like int (since Character is not a Number):

static public int uncheckedIntCast(Object x){
    if(x instanceof Number)
	return ((Number)x).intValue();
    return ((Character) x).charValue();
}


 Comments   
Comment by Nate Austin [ 15/Nov/17 1:41 PM ]

Workaround:

user=> (byte (int \space))
32
Comment by Alex Miller [ 16/Nov/17 9:43 AM ]

The whole idea of the unchecked operations is that they are faster because they omit safety checks. It is safe to cast from char into int or long and those work fine. It is not safe to cast a char to byte as the type is too small. For any char > 127 you will wrap into negative byte range.

user=> (byte (int \£))
-93
Comment by Nate Austin [ 16/Nov/17 9:52 AM ]

You trade off code unsafety by instead making code break? -93 is the "correct" byte representation there. The whole point of an unsafe cast is that it's unsafe.

Comment by Nate Austin [ 16/Nov/17 10:41 AM ]

Also, it only works for a long because it forces it down to a checked cast. There is no uncheckedLongCast(char) and the (Object) version would fail. Aside from that, a downcast always has risks. The point of unchecked is that you're willing to trade off safety and implies that you Know What You're Doing.

Here is no.disassemble output from (long \space):

3  invokestatic clojure.lang.RT.longCast(java.lang.Object) : long [42]
     6  invokestatic clojure.lang.Numbers.num(long) : java.lang.Number [48]

and (byte \space):

3  invokestatic clojure.lang.RT.uncheckedByteCast(java.lang.Object) : byte [42]
     6  invokestatic java.lang.Byte.valueOf(byte) : java.lang.Byte [47]

It doesn't seem reasonable that the widening one would fall back to a slower, but functioning path and the other would take the fast, exception throwing path. If anything, I would expect the opposite.

Comment by Nate Austin [ 16/Nov/17 5:11 PM ]

Related to the previous comment, the uncheckedLongCast actually does fail if you get there more directly:

user=> (unchecked-long \space)

ClassCastException java.lang.Character cannot be cast to java.lang.Number  clojure.lang.RT.uncheckedLongCast (RT.java:1450)




[CLJ-2266] instrument conforms args spec but could just check valid? Created: 14/Nov/17  Updated: 27/Nov/17  Resolved: 27/Nov/17

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

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

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

 Description   

Currently, the spec-checking-fn used by instrument conforms the args spec, however it really only needs to check whether it's `valid?`, which may be faster. The reason for this is historical as instrument originally checked the ret and fn specs and needed the conform value to feed the fn spec.

Patch: clj-2266.patch



 Comments   
Comment by Leon Grapenthin [ 16/Nov/17 12:10 PM ]

Doesn't valid? call conform?

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

fair enough





[CLJ-2265] deftype generates new types when evaluation is expected to be suppressed Created: 13/Nov/17  Updated: 14/Nov/17  Resolved: 13/Nov/17

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

Type: Defect Priority: Minor
Reporter: Ramsey Nasser Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: bug, deftype
Environment:

macOs 10.12.6



 Description   

There does not seem to be a consistent way to prevent deftype from emitting a new type. This was discovered in ClojureCLR when trying to write a defonce-style macro wrapper for deftype, but it affects ClojureJVM as well. It seems to emit types on analysis, which defies Clojure's evaluation semantics.

The following REPL session highlights expected and unexpected behavior, namely that when false is unable to prevent deftype from emitting a new type into memory. comment seems to work, however.

nREPL server started on port 52921 on host 127.0.0.1 - nrepl://127.0.0.1:52921
REPL-y 0.3.7, nREPL 0.2.12
Clojure 1.8.0
Java HotSpot(TM) 64-Bit Server VM 1.8.0_60-b27
    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=> (deftype TypeTest [a b])
user.TypeTest
user=> (->> user.TypeTest .getDeclaredFields (map #(.getName %)))
("a" "b")
user=> (deftype TypeTest [a b c])
user.TypeTest
user=> (->> user.TypeTest .getDeclaredFields (map #(.getName %)))
("a" "b" "c")  ;; <= expected redefinition 
user=> (when false (deftype TypeTest [a]))
nil
user=> (->> user.TypeTest .getDeclaredFields (map #(.getName %)))
("a") ;; <= when false fails to prevent redefinition
user=> (comment (deftype TypeTest [a b c]))
nil
user=> (->> user.TypeTest .getDeclaredFields (map #(.getName %)))
("a")  ;; <= comment manages to prevent redefinition
user=>


 Comments   
Comment by Kevin Downey [ 13/Nov/17 1:26 PM ]

not on analysis, on compilation.

deftype has a compilation time effect. similarly def has a compilation time effect.

so

(when false (def a 1))

will result in the var for a being interned, but it won't have a root value.

the reason wrapping in (comment ...) stops whatever from happening is compilation happens after macroexpansions, and the comment macro replaces whatever form with nil.

I am not sure this would be considered a bug. The only way to avoid this would be to have deftype generate types at runtime instead of at compile time, but I am pretty sure the types are generated at compile time to 1. avoid generating a new type every time the compiled expression is run 2. to make class generation fairly predictable so clojure can be used in environments with unusual classloader restrictions.

Comment by Kevin Downey [ 13/Nov/17 1:29 PM ]

maybe take a look at compile-if https://github.com/clojure/clojure/blob/master/src/clj/clojure/core/reducers.clj#L24-L35

Comment by Alex Miller [ 13/Nov/17 5:23 PM ]

As Kevin commented, this is the intended behavior.

Comment by Ramsey Nasser [ 14/Nov/17 10:41 AM ]

Noted. compile-if is what we need. Apologies for the noise.





[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-2260] Regex spec with 0/1 first arg followed by coll-of anything throws error when first arg is a defrecord Created: 02/Nov/17  Updated: 02/Nov/17  Resolved: 02/Nov/17

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

Type: Defect Priority: Major
Reporter: David Chelimsky Assignee: Unassigned
Resolution: Duplicate Votes: 0
Labels: None
Environment:

1.9.0-beta3



 Description   
user=> (require '[clojure.spec.alpha :as s])
nil
user=> (defrecord Foo [])
user.Foo
user=> (s/def ::optional-foo (s/cat :foo (s/? any?) :bar int?))
:user/optional-foo
user=> (s/valid? ::optional-foo [(->Foo) 1])
true
user=> (s/def ::optional-foo-then-coll (s/cat :foo (s/? any?) :bar (s/coll-of int?)))
:user/optional-foo-then-coll
user=> (s/valid? ::optional-foo-then-coll [(->Foo) [1]])
UnsupportedOperationException Can't create empty: user.Foo  user.Foo (NO_SOURCE_FILE:14)
user=> *e
#error {
 :cause "Can't create empty: user.Foo"
 :via
 [{:type java.lang.UnsupportedOperationException
   :message "Can't create empty: user.Foo"
   :at [user.Foo empty "NO_SOURCE_FILE" 14]}]
 :trace
 [[user.Foo empty "NO_SOURCE_FILE" 14]
  [clojure.core$empty invokeStatic "core.clj" 5192]
  [clojure.core$empty invoke "core.clj" 5186]
  [clojure.spec.alpha$every_impl$cfns__915$fn__921 invoke "alpha.clj" 1229]
  [clojure.spec.alpha$every_impl$reify__934 conform_STAR_ "alpha.clj" 1243]
  [clojure.spec.alpha$conform invokeStatic "alpha.clj" 150]
  [clojure.spec.alpha$conform invoke "alpha.clj" 146]
  [clojure.spec.alpha$dt invokeStatic "alpha.clj" 743]
  [clojure.spec.alpha$dt invoke "alpha.clj" 738]
  [clojure.spec.alpha$dt invokeStatic "alpha.clj" 739]
  [clojure.spec.alpha$dt invoke "alpha.clj" 738]
  [clojure.spec.alpha$deriv invokeStatic "alpha.clj" 1480]
  [clojure.spec.alpha$deriv invoke "alpha.clj" 1474]
  [clojure.spec.alpha$deriv invokeStatic "alpha.clj" 1488]
  [clojure.spec.alpha$deriv invoke "alpha.clj" 1474]
  [clojure.spec.alpha$deriv invokeStatic "alpha.clj" 1489]
  [clojure.spec.alpha$deriv invoke "alpha.clj" 1474]
  [clojure.spec.alpha$re_conform invokeStatic "alpha.clj" 1615]
  [clojure.spec.alpha$re_conform invoke "alpha.clj" 1606]
  [clojure.spec.alpha$regex_spec_impl$reify__1188 conform_STAR_ "alpha.clj" 1656]
  [clojure.spec.alpha$valid_QMARK_ invokeStatic "alpha.clj" 755]
  [clojure.spec.alpha$valid_QMARK_ invoke "alpha.clj" 751]
  [user$eval208 invokeStatic "NO_SOURCE_FILE" 18]
  [user$eval208 invoke "NO_SOURCE_FILE" 18]
  [clojure.lang.Compiler eval "Compiler.java" 7062]
  [clojure.lang.Compiler eval "Compiler.java" 7025]
  [clojure.core$eval invokeStatic "core.clj" 3211]
  [clojure.core$eval invoke "core.clj" 3207]
  [clojure.main$repl$read_eval_print__8574$fn__8577 invoke "main.clj" 243]
  [clojure.main$repl$read_eval_print__8574 invoke "main.clj" 243]
  [clojure.main$repl$fn__8583 invoke "main.clj" 261]
  [clojure.main$repl invokeStatic "main.clj" 261]
  [clojure.main$repl_opt invokeStatic "main.clj" 325]
  [clojure.main$main invokeStatic "main.clj" 424]
  [clojure.main$main doInvoke "main.clj" 387]
  [clojure.lang.RestFn invoke "RestFn.java" 397]
  [clojure.lang.AFn applyToHelper "AFn.java" 152]
  [clojure.lang.RestFn applyTo "RestFn.java" 132]
  [clojure.lang.Var applyTo "Var.java" 702]
  [clojure.main main "main.java" 37]]}


 Comments   
Comment by David Chelimsky [ 02/Nov/17 2:03 PM ]

This is actually a dup of https://dev.clojure.org/jira/browse/CLJ-1975. Closing.

Comment by David Chelimsky [ 02/Nov/17 2:03 PM ]

Dup of https://dev.clojure.org/jira/browse/CLJ-1975





[CLJ-2259] Remove unnecessary bigdec? Created: 30/Oct/17  Updated: 31/Oct/17  Resolved: 31/Oct/17

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

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

Attachments: Text File clj-2259-2.patch     Text File clj-bigdec.patch     Text File spec-bigdec.patch    
Patch: Code
Approval: Ok

 Description   

In 1.9, we added bigdec? predicate, but the existing decimal? predicate is the same (this was unintentional).

Patches:

  • spec-bigdec.patch - spec.alpha patch to modify spec generator mapping
  • clj-2259-2.patch - Clojure patch to remove the new bigdec? predicate and update to latest spec (with updated mapping)





[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-2257] Documentation typo Created: 28/Oct/17  Updated: 29/Oct/17

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

Type: Defect Priority: Trivial
Reporter: Seb Martel Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring
Environment:

all


Attachments: Text File 0001-Documentation-typo.patch    
Patch: Code
Approval: Prescreened

 Description   

'methd' to 'method' in proxy documentation.

Prescreened: Alex Miller






[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-2254] Add system property to disable spec macro instrumentation Created: 24/Oct/17  Updated: 25/Oct/17  Resolved: 25/Oct/17

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

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

Attachments: Text File skip-macros-2.patch    
Patch: Code
Approval: Ok

 Description   

Macro specs are checked at macro expansion time and it is not currently possible to disable this. One place where this is an issue is while compiling the spec.alpha library itself. There may be other cases where it is useful to disable these macro spec checks as well for the purposes of debugging something.

The attached patch adds a system property "clojure.spec.skip-macros" which defaults to false but can be set to true to skip the macro checks.

Patch: skip-macros-2.patch

Screened by Stu, who resisted the temptation to bikeshed on what the property should be named. Verified that spec.alpha builds correctly when the property is set.






[CLJ-2253] slurp doesn't properly handle URLs that contain authentication information Created: 19/Oct/17  Updated: 23/Nov/17

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

Type: Enhancement Priority: Minor
Reporter: Peter Monks Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: io
Environment:

n/a


Attachments: Text File 0001-CLJ-2253-Add-support-for-slurping-urls-with-authenti.patch     Text File 0002-CLJ-2253-Add-support-for-slurping-urls-with-authenti.patch    
Patch: Code
Approval: Triaged

 Description   

Because of limitations in the underlying Java classes [1], clojure.core/slurp doesn't support URLs that contain authentication information i.e. https://user:password@hostname.domain/path/to/resource. It would be great if slurp did support this (e.g. by using Java's UrlConnection class, as suggested in [1]), so that application code doesn't have to include special cases for different types of input string that will be sent to slurp.

[1] https://stackoverflow.com/questions/496651/connecting-to-remote-url-which-requires-authentication-using-java

Approach: Check to see if url contains userInfo, if so, open a connection, set authorisation on it and return its input stream. Otherwise call `openStream` on the url as before
Limitations: This approach uses java.xml.bind.DatatypeConverter, which is known to not work with java9



 Comments   
Comment by Alex Miller [ 19/Oct/17 2:38 PM ]

Re the note, this seems like a non-starter then?

Comment by Peter Monks [ 19/Oct/17 2:55 PM ]

What's the minimum version of the JVM that Clojure supports? Java 1.8 added java.util.Base64, and that may be a better choice for the encoding step (assuming the minimum supported JVM version is 1.8).

Alternatively, it seems like some kind of version-dependent "conditional compilation" would meet all cases - use java.xml.bind.DatatypeConverter for JVM <= 1.7, and use java.util.Base64 for JVM >= 1.8. Apologies for my unfamiliarity with the core Clojure codebase, but is there JVM-version-specific logic elsewhere that I could look at to see how this might be done?

Comment by Erik Assum [ 19/Oct/17 3:26 PM ]

I'd really appreciate some pointers to how to solve this, since Base64 encoding is needed. Could an option be to copy
https://github.com/clojure/data.codec/blob/master/src/main/clojure/clojure/data/codec/base64.clj
to a ns in clojure.core?

There is also http://www.docjar.com/html/api/java/util/prefs/Base64.java.html, which of course is package scoped.
I guess we could use reflection to get to this, but feels hacky.

Or should we just wait until java 8 is the lowest supported java platform?

Comment by Peter Monks [ 19/Oct/17 7:48 PM ]

Posting this may be a career limiting move, but this seems to work (tested on JDK 9 as-is, and on JDK 1.8 as-is as well as with the first clause in the or commented out to ensure both versions of the method got tested):

(let [jvm-version (System/getProperty "java.version")]
  (if (or (clojure.string/starts-with? jvm-version "1.8")
          (clojure.string/starts-with? jvm-version "9"))
    (eval
      (read-string
      "(defn- base64-encode
         [s]
         (.encodeToString (java.util.Base64/getEncoder) (.getBytes s)))"))
    (eval
      (read-string
      "(defn- base64-encode
         [s]
         (javax.xml.bind.DatatypeConverter/printBase64Binary (.getBytes s)))"))))

You'd then unconditionally call base64-encode from the existing patch, Erik.

I'm simultaneously horrified and rather chuffed at this ghastly hack...

Comment by Alex Miller [ 19/Oct/17 10:44 PM ]

Doing nothing is always an option.

Comment by Peter Monks [ 19/Oct/17 10:54 PM ]

Where's the fun in that?

Comment by Alex Miller [ 20/Oct/17 8:55 AM ]

As the one maintaining it down the line, the fun is in avoiding future pain.

Comment by Peter Monks [ 20/Oct/17 10:15 AM ]

How about this approach (also tested as before):

(defmacro base64-encode
  [^String s]
  (let [jvm-version (System/getProperty "java.version")]
    (if (or (clojure.string/starts-with? jvm-version "1.6")
            (clojure.string/starts-with? jvm-version "1.7"))
      `(javax.xml.bind.DatatypeConverter/printBase64Binary (.getBytes ~s))
      `(.encodeToString (java.util.Base64/getEncoder) (.getBytes ~s)))))

?

I also inverted the condition, to make the code more future proof (assuming the JVM retains java.util.Base64 going forward, of course).

Note: there's a separate (unanswered) question regarding whether this macro should be ^:private or not.

Comment by Phill Wolf [ 21/Oct/17 9:02 PM ]

What matters is the Java version that runs the program. The macro is pleasantly legible, but it tests java.version at compile-time.

Comment by Peter Monks [ 22/Oct/17 12:04 AM ]

Is this ns AOT compiled? If not, am I misunderstanding when Clojure compilation occurs?

Comment by Alex Miller [ 22/Oct/17 8:24 AM ]

This ns is AOT compiled, but this form will test version at compile-time of the user's program (not compile-time of Clojure). Usually for stuff like this we prefer to check for the presence/absence of a Class as that is less version-sensitive. Here, that might mean checking for presence of DatatypeConverter before invoking it (that also ties the decision to what you're actually doing more closely).

However, I am increasingly skeptical that any of this is worth doing.

Comment by Peter Monks [ 22/Oct/17 10:42 AM ]

Absolutely happy to have modified / alternative approaches proposed / implemented.

But going back to the requirement, I might just point out that slurp (or rather the relevant clojure.java.io fns) don’t fully support the the URI standard. And while this is clearly a deficiency of Java, given the ease of working around it in Clojure it seems to me to be worthwhile addressing (and I’m not just saying that because I have this exact use case).

Comment by Erik Assum [ 22/Oct/17 2:41 PM ]

Uploaded a new patch which unconditionally uses java.util.Base64, but inside a try/catch
This means that if the code is run on a jdk less than 8, the behaviour will be as today, whereas if run on 8 or 9,
authentication will work.

The downside to this approach is that sometime in the future when the lowest supported jdk version is 8, the try/catch code will be
redundant, and should be removed.

Comment by Peter Monks [ 25/Oct/17 12:10 PM ]

FWIW here's a workaround that appears to work, and should be a drop-in to most (all?) Clojure projects.

Thanks to Erik for showing how to "patch" IOFactory from "outside" core Clojure.

Comment by Erik Assum [ 23/Nov/17 2:37 PM ]

FWIW2 The new macro (which unfortunately is marked private), when-class could be a clean way to implement this.
https://github.com/clojure/clojure/commit/ecd15907082d31511f1ed0a249bc48fa532311f4





[CLJ-2252] Add javadoc URL for java 9 to clojure.java.javadoc Created: 17/Oct/17  Updated: 19/Oct/17

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

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

Attachments: Text File clj-2252-2.patch     Text File javadoc-java9.patch    
Patch: Code
Approval: Prescreened

 Description   

Javadoc url for java 9 is missing in clojure.java.javadoc.
As a result, javadoc function is not able to find documentation for java 9 classes:

(javadoc java.lang.StackWalker)
;;=> opens web browser pointing to nonexistent URL: http://docs.oracle.com/javase/8/docs/api/java/lang/StackWalker.html

Patch: clj-2252-2.patch
Prescreened by: Alex Miller



 Comments   
Comment by Alex Miller [ 18/Oct/17 3:04 PM ]

Should probably change the default url too if jdk is unknown. Generally we use the latest stable version of Java for that.

Comment by Juraj Martinka [ 19/Oct/17 2:42 AM ]

Makes sense. I uploaded a new patch.

Comment by Alex Miller [ 19/Oct/17 8:53 AM ]

I added a modified patch that just changes the commit message to include the ticket (attribution retained). Marked prescreened. I don't think we'll get this in for Clojure 1.9, but should be no problem next time.





[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-2249] Document behavior of clojure.core/get on strings and Java arrays Created: 05/Oct/17  Updated: 09/Oct/17

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

Type: Enhancement Priority: Minor
Reporter: Arne Brasseur Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring

Attachments: Text File 0001-Expand-docstring-of-clojure.core-get.patch     Text File CLJ-2249-patch-2.patch    
Patch: Code
Approval: Prescreened

 Description   

get's implementation checks in order for

  • ILookup
  • nil
  • Map
  • IPersistentSet
  • String or Java array

The docstring for get currently reads

"Returns the value mapped to key, not-found or nil if key not present."

That this works on maps and associative data can reasonably be inferred if one knows Clojure's data model. That it works on sets, Strings, and arrays is less obvious, and would be helpful to mention.

Patch: CLJ-2249-patch-2.patch
Prescreened by: Alex Miller



 Comments   
Comment by Arne Brasseur [ 05/Oct/17 12:49 PM ]

I've tried to stick to the same terse style, happy to take suggestions for phrasing. I also changed the first sentence slightly, because I found it difficult to parse, but that is an unrelated change that I can undo if that's preferred.

Comment by Alex Miller [ 05/Oct/17 1:14 PM ]

I'd leave the first sentence as is.

All of these collection ops are kind of tricky in being able to succinctly state the intent, while also covering special cases (which are often related to Java types). Here I think the intent is to cover lookup in "associative data structures" which covers Clojure maps, records, vectors, Java maps, and other less obvious things like weird ILookup impls.

The non-obvious inclusions for me are: Clojure sets (I haven't reviewed but undoubtedly this is implicitly used in a bunch of special cases), and the Java special cases which are Strings and arrays. For an example of wording, I would point to `count` and `nth`, which are similarly weird.

So maybe a sentence like: "get also works on sets to return contained values, and on strings and arrays for value by index." ?

We'll need to answer these same questions in the spec too btw. I expect the act of spec'ing core fns to drive more of these tricky questions.

Comment by David Liepmann [ 05/Oct/17 2:18 PM ]

>I'd leave the first sentence as is.

Would a rephrase be welcome in its own issue? Right now the referent of "the value mapped to" is ambiguous. I agree it's difficult to parse.

Comment by Alex Miller [ 05/Oct/17 2:50 PM ]

David Liepmann no thanks

Comment by Arne Brasseur [ 09/Oct/17 3:27 AM ]

Appended new patch.





[CLJ-2248] Collections' toString implementations are affected by *print-readably* Created: 04/Oct/17  Updated: 05/Oct/17

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

Type: Defect Priority: Major
Reporter: Michał Marczyk Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None

Attachments: Text File 0001-CLJ-2248-binding-proof-toString-in-collection-classe.patch    
Patch: Code and Test

 Description   

Collection classes implement toString by delegating to RT.printString, which in turn is affected by the value of *print-readably*:

Clojure 1.9.0-beta1
user=> (binding [*print-readably* false] (str ["foo"]))
"[foo]"
user=> (binding [*print-readably* true] (str ["foo"]))
"[\"foo\"]"

The attached patch fixes this by replacing the calls to RT.printString in collection toString implementations with calls to a new RT.prString method that explicitly binds *print-readably*.

See https://groups.google.com/d/msg/clojure/S13swxLy1ng/FKLYdY9HAgAJ for the original report of interactions between lazy-seq, print and str that are ultimately caused by the above issue.



 Comments   
Comment by Alex Miller [ 04/Oct/17 9:58 PM ]

It's not clear to me why or whether the current behavior is wrong?

Comment by Michał Marczyk [ 05/Oct/17 2:38 AM ]

I would expect str / toString to be stable when applied to persistent collections of immutable items. In other words, when applied to immutable inputs, I would expect it to be a pure function of the arguments explicitly passed in.

There is certainly no way for users to expect any dependency on any dynamic Vars here, or indeed anything other than the argument – which is a pure value.

The original example in the Google group thread is of a lazy seq that is ostensibly a value – (lazy-seq [(str ["a string"])]) – which returns a different pr-str (and print-method) representation depending on whether or not one passes it to print in between creating it and calling pr-str on it:

Clojure 1.9.0-beta1
user=> (let [x (lazy-seq [(str ["a string"])])] (print x) (pr-str x))
([a string])"(\"[a string]\")"
user=> (let [x (lazy-seq [(str ["a string"])])] (pr-str x))
"(\"[\\\"a string\\\"]\")"

Here the user might expect the pr-str call to be independent of the print call, since the former only takes place once the latter returns, and yet there is a spooky interaction.

The patch fixes this:

Clojure 1.9.0-master-SNAPSHOT
user=> (let [x (lazy-seq [(str ["a string"])])] (print x) (pr-str x))
(["a string"])"(\"[\\\"a string\\\"]\")"
user=> (let [x (lazy-seq [(str ["a string"])])] (pr-str x))
"(\"[\\\"a string\\\"]\")"
Comment by Alex Miller [ 05/Oct/17 9:04 AM ]

This is perhaps a philosophical argument and I don't really have a definitive answer, but happy to kick it back and forth.

Agreed that the persistent collection of immutable values is a value. However, there are many ways to build a string representing a view of that immutable data - we have several built into the Clojure print system (pprint, pr, print) + a variety of knobs like collection size limits, etc. I don't see any principle that leads me to believe that the toString has to be independent from the knobs choosing that view.

In other words, I don't necessarily see this as a problem to be solved.

Comment by Didier A. [ 05/Oct/17 10:57 AM ]

I think it's true this is a little philosophical. However, to add some weight toward the side of the patch, I think toString and thus str is generally expected by programmers to be stable. So I would say the current behaviour breaks the principle of least surprise. I'd vote for making str/toString stable, that said, there is a very small possibility this chamge would be a breaking change to someone.

Comment by Alex Miller [ 05/Oct/17 11:49 AM ]

As a grizzled Java veteran, I have 0 expectations about toStrings. Usually in Java they are built on mutable fields and are wildly *un*stable, so I certainly don't share your expectation.

str inherently involves "printing" (creating a string view of a value). I think str is "stable", just not solely a function of its explicit inputs (boo hidden state). To make an analogy, there are many ways to create a string from a date object and toString of a java.util.Date will format the string using your timezone, which is external hidden state.

I'm still not necessarily opposed to the changes here. I just don't find it to be obviously the right thing to do.

From a general perspective, I think the "principle of least surprise" implies much greater commonality in what people find surprising than actually exists, so I put little stock in that. I put a lot more weight in an argument that follows from some stated principles. I think this area is underdocumented/underspecified though.

Comment by Phill Wolf [ 05/Oct/17 5:24 PM ]

The original issue starts with toString. A more vivid problem is that the issue demonstrates that pr is undependable to emit EDN with.

Absence or presence of the intervening print-str (which the caller of pr might have nothing to do with) makes a material difference:

 
user> (clojure.edn/read-string
        (first
          (clojure.edn/read-string
            (let [mk-str (fn [] (lazy-seq [(str ["ZiZi"])]))
                  a (mk-str)]
              ;(print-str a)
              (pr-str a)))))
["ZiZi"]

user> (clojure.edn/read-string
        (first
          (clojure.edn/read-string
            (let [mk-str (fn [] (lazy-seq [(str ["ZiZi"])]))
                  a (mk-str)]
              (print-str a)
              (pr-str a)))))
[ZiZi]




[CLJ-2247] {min,max}-key return first match in >= 1.9.0-alpha20, used to return the last one Created: 04/Oct/17  Updated: 06/Oct/17  Resolved: 06/Oct/17

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

Type: Defect Priority: Major
Reporter: Michał Marczyk Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: regression

Attachments: Text File 0001-CLJ-2247-restore-last-match-semantics-of-min-max-key.patch     Text File 0002-CLJ-2247-document-last-match-semantics-of-min-max-ke.patch    
Patch: Code
Approval: Ok

 Description   

The CLJ-99 fix, first included in 1.9.0-alpha20, changes behaviour of min-key and max-key to return the first argument among those with the minimum/maximum key, whereas in previously they both returned the last matching argument:

Clojure 1.9.0-alpha19
user=> (apply min-key :x [{:x 1000} {:x 1001} {:x 1002} {:x 1000 :second true}])
{:x 1000, :second true}
Clojure 1.9.0-beta1
user=> (apply min-key :x [{:x 1000} {:x 1001} {:x 1002} {:x 1000 :second true}])
{:x 1000}

Arguably the new behaviour is more natural, however this is a regression from prior behavior.

Proposed: Restore the original behavior and update docstring.
Patch: 0002-CLJ-2247-document-last-match-semantics-of-min-max-ke.patch
Screened by: Alex Miller



 Comments   
Comment by Michał Marczyk [ 04/Oct/17 3:20 PM ]

Restoring the original behaviour (option 2) seems like a reasonable default position, particularly so close to a major release, so here's a patch that does that.

Comment by Michał Marczyk [ 04/Oct/17 3:38 PM ]

As a side note, the original behaviour arose from the use of reduce in the implementation of both functions:

Clojure 1.9.0-beta1
user=> (apply min-key :x [{:x 1000} {:x 1001} {:x 1002} {:x 1000 :second true}])
{:x 1000}
user=> (reduce #(min-key :x %1 %2) [{:x 1000} {:x 1001} {:x 1002} {:x 1000 :second true}])
{:x 1000, :second true}
Comment by Alex Miller [ 04/Oct/17 3:54 PM ]

Agreed on restoring original behavior.

Comment by Bozhidar Batsov [ 05/Oct/17 1:38 AM ]

I also think that the expected behaviour should be mentioned in the documentation, as I'd certainly expect this to behave like it does after the patch. Perhaps down the road we can make this configurable with an extra parameter?

Comment by Michał Marczyk [ 05/Oct/17 3:13 AM ]

That's fair enough re: documenting the behaviour. Here's a patch that applies on top of the 0001 patch and only tweaks the docstrings by adding "If there are multiple such xs, the last one is returned."

Driving the behaviour using extra arguments is not really an option in this case, as min-key and max-key take varargs.

An extra pair of functions along the lines of min-key-first / max-key-first could be contemplated (as could min-key-last / max-key-last to provide a migration path for existing users if the new behaviour were kept). Whether it should is a separate question which it may be ok to address after the release.





[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-2245] API changes report for Clojure Created: 02/Oct/17  Updated: 02/Oct/17  Resolved: 02/Oct/17

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

Type: Enhancement Priority: Major
Reporter: Andrey Ponomarenko Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None

Attachments: PNG File clojure-report-1.png     PNG File clojure-report-2.png    

 Description   

The review of API changes for the Clojure library since 1.0.0 version: https://abi-laboratory.pro/java/tracker/timeline/clojure/

Hope it will be helpful for users and maintainers of the library. The report is generated by the https://github.com/lvc/japi-compliance-checker and https://github.com/lvc/japi-tracker tools for jars found at http://central.maven.org/maven2/org/clojure/clojure/.

Thank you.




 Comments   
Comment by Alex Miller [ 02/Oct/17 5:36 PM ]

Not an issue





[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-2241] Specify different domains in get-spec docstring Created: 24/Sep/17  Updated: 24/Sep/17

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

Type: Enhancement Priority: Trivial
Reporter: Andrea Richiardi Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJ-1962.patch    
Patch: Code

 Description   

I was initially confused by the fact that could not get (data) specs with symbols. Then Alex kindly explained me that there are two domains in the registry basically.
I think this should be described somewhere and the docstring for get-spec looks like a good place.






[CLJ-2240] Load user.cljc in classpath root. Created: 22/Sep/17  Updated: 22/Sep/17

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

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

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

 Description   

Right now Clojure only loads user.clj but it might be a good idea to load user.cljc if it was in classpath as well.






[CLJ-2239] Link to Google Guava API broken in clojure.java.javadoc/*remote-javadocs* Created: 19/Sep/17  Updated: 08/Oct/17  Resolved: 06/Oct/17

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

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

Attachments: Text File javadoc-update-guava-url.patch    
Patch: Code
Approval: Ok

 Description   

googlecode.com is no longer in service. Thus using javadoc on Google Guava classes always results in a 404 error page.

The change proposed here updates the URL to point to the current release of Guava, release 23.0.

Unfortunately, the new URL contains the release version, so will be out-of-date soon as Guava has a higher release cadence (and there can be breakage). The alternative is to simply drop support for Guava in javadoc.



 Comments   
Comment by David Bürgin [ 19/Sep/17 12:01 PM ]

Created by recently merged CLJ-1398.

Comment by Alex Miller [ 19/Sep/17 12:04 PM ]

Ha, I think that happened since the original patch was written for 1.9. Thanks...

Comment by Alex Miller [ 19/Sep/17 12:05 PM ]

Fix url updated in 1.9-alpha20.

Comment by Juraj Martinka [ 08/Oct/17 6:21 AM ]

While looking at this I realized that links to Java 9 docs are missing from `core-java-api` definition





[CLJ-2238] NPE when print-duping Void/TYPE Created: 18/Sep/17  Updated: 18/Sep/17

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

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

Attachments: Text File 0001-CLJ-2238-handle-Void-TYPE-in-print-dup.patch    
Patch: Code
Approval: Triaged

 Description   
user=> (binding [*print-dup* true] (print Void/TYPE))

NullPointerException   java.io.PrintWriter.write (PrintWriter.java:473)
#=(identity user=>

Patch: 0001-CLJ-2238-handle-Void-TYPE-in-print-dup.patch






[CLJ-2237] Provide a type predicate for ex-info? Created: 18/Sep/17  Updated: 19/Sep/17

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

Type: Enhancement Priority: Minor
Reporter: Christian Romney Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: None

Attachments: Text File Add-ex-info-type-predicate.patch    
Patch: Code and Test

 Description   

A type predicate for ExceptionInfo would be as useful when writing specs as the other nice additions landing in Clojure 1.9 (e.g. nat-int?, pos-int?, etc).



 Comments   
Comment by Alex Miller [ 18/Sep/17 9:39 PM ]

It would be helpful to list an example of some code where this would come in handy (I think examples do exist in catch handling) and to give some indication of how common it is.

Comment by Christian Romney [ 19/Sep/17 7:44 AM ]

One place where it can be useful is specing data that's pulled from a channel. http://swannodette.github.io/2013/08/31/asynchronous-error-handling. David's example immediately throws the exception via the <? macro, but the fact that exception info might be put on the channel at all means it can be seen by functions we might have in a transducer. These functions can leverage spec, where this predicate would come in very handy. I'd paste code, but the example immediately at hand is proprietary. Is this a sufficiently good description, or does the case need more support?

Also, while David's example is CLJS I've used this pattern in Clojure core.async code (e.g. asynchronous Pedestal interceptors).

Comment by Christian Romney [ 19/Sep/17 12:17 PM ]

Also, from an aesthetic[1] point of view adding a type predicate (ex-info?) for a pretty important core Clojure abstraction that already has a constructor (ex-info) and an accessor (ex-data) just seems to round out / complete the API. Not sure how that compelling you or Rich might find that argument, but I figured it couldn't hurt the case.

[1] My sense of aesthetic here is colored by some of the ideas in Mitch Wand & Dan Friedman's Essentials of Programming Languages.





[CLJ-2236] Add regexp? Predicate like in ClojureScript Created: 14/Sep/17  Updated: 14/Sep/17  Resolved: 14/Sep/17

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

Type: Enhancement Priority: Minor
Reporter: Alexander Kiel Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None


 Description   

ClojureScript has a regexp? predicate. Clojure should have the same for java.util.regex.Pattern.



 Comments   
Comment by Alex Miller [ 14/Sep/17 1:40 PM ]

This has been considered and rejected multiple times in the past. Could you give me an example of code that would use such a thing in the ClojureScript world?

I think there is an argument to be made for portability that is stronger now than when previously requested, but I can't say I see this (yet) as a need common enough to warrant a core predicate.

Comment by Alex Miller [ 14/Sep/17 1:51 PM ]

cross-clj reports only 2 usages: https://crossclj.info/fun/cljs.core.cljs/regexp%3F.html

  • one in cljs itself and one in another location. I looked through 35 pages of github search results and only saw maybe one place where it might have been used.

Based on that, I'd say the bulk of users don't need a predicate for regexp? and it's not worth adding to core over just #(instance? java.util.regex.Pattern %) in the rare cases where you need it.

Going to decline this but happy to reconsider later if there is a compelling reason.





[CLJ-2235] Add named loop + recur-to facility for nested loops Created: 10/Sep/17  Updated: 17/Sep/17

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

Type: Feature Priority: Major
Reporter: Michał Marczyk Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: compiler

Attachments: Text File 0001-CLJ-2235-implement-named-loop-recur-to.patch     Text File 0001-CLJ-2235-implement-named-loop-recur-to-separate-special-form.patch     Text File 0001-CLJ-2235-recur-to-keyword-loop-names.patch     Text File 0002-CLJ-2235-implement-recur-recur-to-as-separate-specia.patch     Text File 0002-CLJ-2235-use-keywords-as-loop-names.patch    
Approval: Triaged

 Description   

Copied from the proposal email to the Clojure/dev Google group:

https://groups.google.com/d/msg/clojure-dev/zlMGmv60MVA/beyIRTrhAgAJ

Hi,

Currently loop/recur is limited to "single-layered" loops: loop forms can occur inside other loop forms, but there is no facility for "recuring to an outer loop".

A few years ago I posted a proposal to add support for nested loops to Clojure with a proof-of-concept patch to ClojureScript with syntax and semantics that I think suffice to make nested loops feel natural while remaining a natural extension of the core loop/recur model, with the same explicit tail recursion benefits:

(loop foo [...]    ; named loop
  ...
  (loop [...]      ; intervening loop
    ...
    (loop [...]    ; innermost loop
      ...
      (recur-to foo ...)))) ; recur to the named loop;
                            ; NB. this must be in tail position
                            ; w.r.t. *all* three loops

https://groups.google.com/d/msg/clojure-dev/imtbY1uqpIc/8DWLw8Ymf4IJ
https://dev.clojure.org/display/design/Named+loops+with+recur-to
https://github.com/michalmarczyk/clojurescript/tree/recur-to
https://github.com/michalmarczyk/clojurescript/commit/feba0a078138da08d584a67e671415fc403fa093

I have now implemented a complete patch enabling the proposed feature in Clojure (the first link is to a branch based on current master, that is, the "prepare for next development iteration" commit after 1.9.0-alpha17; the second is to the current tip of this branch for future reference):

https://github.com/michalmarczyk/clojure/tree/recur-to

https://github.com/michalmarczyk/clojure/commit/212ea06d21d3b336ac35480c99170e81defaf956

I also opened a ticket in JIRA so as to have a place to post the above in the form of a patch file:

https://dev.clojure.org/jira/browse/CLJ-2235

The remainder of this email sets out the proposal in more detail, states its key properties in a somewhat rigorous form, briefly summarizes the implementation approach and discusses certain design choices made in the patch.

Overview
========

The idea is that one could write e.g.

(let [m (two-dimensional-matrix)]
  (loop iloop [i 0]           ; named loop
    (if (< i i-lim)
      (loop [j 0]             ; nested anonymous loop
        (if (< j j-lim)
          (do
            (work)
            (recur (inc j)))  ; recur to the innermost loop
          (recur-to iloop (inc i)))) ; recur to iloop
      (done))))

and, provided that each recur-to form is in tail position with respect to all its enclosing loop forms up to and including its target and the number of arguments passed to each recur-to form matches the number of loop locals of the target loop (plus one for the leading loop name argument), this should compile and behave much like nested loops in Java.

The proposed syntax is modelled on Scheme's named lets, although semantically
those are quite different - this proposal is strictly limited to expanding the loop/recur model to nested loops in a natural way. Of course named fn forms ought also to be valid recur-to targets.

Key properties of named loops and recur-to
==========================================

With the above-linked patch in place, the following rules are enforced at compilation time:

1. Each recur-to form must be in tail position with respect to all its enclosing loop forms, whether named or not, up to and including its target (which may be a named loop or fn form).

2. It is an error to specify a recur-to target which does not occur among the names of the recur-to form's enclosing loop or fn forms with respect to which it is in tail position.

3. It is not possible to recur-to across try.

4. The number of arguments passed to recur-to beyond the initial target/label argument must match the number of formal parameters of the target loop or fn form.

5. Shadowing loop names is permissible; recur-to can only target the innermost loop of the given name among those with respect to which it is in tail position. Loop locals introduced by a shadowed named loop remain visible within the shadowing loop (unless they are themselves shadowed by identically named locals).

NB. loop names are not locals. In particular, they neither shadow nor are shadowed by locals of the same name. This point merits a longer discussion; see the section on design choices at the end of this email.

The innermost loop or fn form can always be targeted using plain recur, whether it is named or not. Additionally (recur-to nil ...) is equivalent to (recur ...) (even when the innermost loop or fn form is actually named), and (loop nil [...] ...) is equivalent to (loop [...]).

Summary of the implementation approach
======================================

The patch modifies the handling of loop labels in the compiler and implements the few necessary tweaks to the loop macro.

It also introduces an optional name argument to the loop* special form. (It is optional primarily so as to avoid breaking any non-core macros that emit loop* directly.)

Finally, it renames the recur special form to recur*; recur and recur-to become macros defined in clojure.core. See the section on design choices below for alternative approaches.

Design choices
==============

1. During development, purely as a matter of convenience at that stage, I had a separate loop-as macro that accepted a name argument. I thought it reasonable to add the naming feature directly to loop, particularly since fn already takes an optional name. Still, loop-as is a valid alternative design.

2. Should it be desirable to avoid renaming the existing recur special form to recur* and reimplementing recur as a macro, a new recur-to special form could be added. (Alternatively, one could keep recur as it is while adding recur-to as a macro delegating to a new recur* special form.)

3. Should it be desirable to preserve the option of treating loop names as locals in the future, it would probably be preferable to make them shadow and be shadowed by locals now, as otherwise elevating them to the status of locals at a later point would be a breaking change. To give an example of why such a future change might be useful, if tail call elimination support arrives in a future JDK spec, one might consider whether it'd be useful to adopt a Scheme-like approach with the loop name treated as a local bound to a function with a single arglist corresponding to the loop locals of the named loop; the closure allocations this would entail could perhaps be optimized away if the local is never referenced.

It bears pointing out that if TCE support does materialize, it will enable a range of alternative designs. For example, Scheme-style named lets could then be introduced as a feature of the let macro. Thus it seems to me that it is reasonable to restrict loop/recur/recur-to to label+goto-style looping only, even in a hypothetical future with VM TCE support, and that there is no reason to afford local-like treatment to loop names; hence the current no-shadowing-interactions approach of the patch.

Cheers,
Michał



 Comments   
Comment by Alex Miller [ 11/Sep/17 1:38 PM ]

Thanks Michał, it looks like you've done a lot of good work here. I think you've just missed the window for looking at new feature stuff for 1.9 but would like to circle back in next release on this.

It seems undesirable for recur to change from special form to macro, so would probably be better to either extend the existing form or add a recur-to special form.

Did you consider other options for specifying a name, like a keyword? Keywords don't carry the expectation of lexical shadowing you have with locals so could side-step those issues maybe? Maybe they are undesirable for other reasons.

Comment by Michał Marczyk [ 17/Sep/17 8:01 PM ]

Cheers, that's good to know.

recur-to as a separate special form

That's fair enough re: changing recur. Here's a version of the patch that makes recur-to a new, separate special form. Note that it still shares the RecurExpr class in the compiler the way loop and let share LetExpr. This patch is meant to be applied on top of the previous one to make it clear what the delta is:

0002-CLJ-2235-implement-recur-recur-to-as-separate-specia.patch

https://github.com/michalmarczyk/clojure/tree/recur-to

I've also prepared a squashed version that takes the current master directly to the same state:

0001-CLJ-2235-implement-named-loop-recur-to-separate-special-form.patch

https://github.com/michalmarczyk/clojure/tree/recur-to-separate-special-form

Incidentally, in implementing this patch I had to revert a change that the original patch makes to clojure.pprint that I guess demonstrates why it's a better idea to go with a new, separate recur-to – I overlooked it when preparing the original posting, otherwise it probably would have tipped the scales for me. (The change is marked by a #_ comment that I forgot to remove in the original patch. The new squashed patch cleans it up automatically by not touching that file.)

Symbols vs keywords as loop names

I used symbols partly because fn expects the optional name argument to be a symbol if present, and so recur-to had to support symbolic names anyway (assuming here that we want to recur-to to use the same form of the recur target name that was used to introduce it, which seems reasonable); and partly just "by default", because static symbolic references generally use symbols. (clojure.core/extend uses keywords, but those aren't really static references.) Not sure if the ability to attach metadata to loop names is likely to be useful, but there's that as well.

The second part about existing usage may or may not be a major consideration, particularly since loop names are somewhat unique in that they can only be referred to by a single special form – recur-to – and are otherwise invisible in the source. This also distinguishes them from fn-introduced named recur targets which of course do double as locals.

So I suppose we could use keywords for loop names and symbols for fn names if we're happy to have metadata-less loop names. We could even allow fn forms to use keyword names if the intention is to establish a named recur target without providing a local reference to the fn instance. (That'd basically be sugar over fn + loop.)

I'll have to think some more about which approach I prefer. I still like the consistency of using symbols for recur targets everywhere (fn / loop / recur-to), but having the local/not-a-local difference be accompanied by a syntactic distinction is tempting.

In the way of some brainstorming-in-the-open, I find it interesting that by using keyword loop names now we'd keep the possibility open of adding support for symbols in the future – perhaps for those VM-TCE-based Scheme-like loop names that would provide local references to closures. Or we could make "symbol labels" a generic feature of "let-like" forms (the ones backed by LetExpr, i.e. let & loop) once TCE lands. Then we'd have consider whether it should be possible for recur-to to target such named lets… And what about plain let? Might be simpler to stick to label+goto looping in loop/recur/recur-to and Scheme-like lets supported through a separate facility (perhaps simply let), with fn something of a point of intersection of the two models (which it already is, since it does introduce recur targets even when unnamed).

As a final note, the one instance where I think the possibility of using keywords for something comparable was brought up was shorthand field access syntax – IIRC (. x :foo) / (.:foo x) was brought up as a possible syntax for what has ultimately become (. x -foo) / (.-foo x). Despite being a road not taken, I think it illustrates how one could plausibly get used to keywords/symbols in effect accessing two namespaces. (Well, in the longhand version; .:foo is technically a symbol. Using keywords for loop names would not involve affording special treatment to any class of "keyword-derived" symbols.)

In any case, I'll see about preparing a version of the patch using keywords for loop names.

Comment by Michał Marczyk [ 17/Sep/17 10:02 PM ]

Here's a first cut at a "keyword loop names" patch:

0002-CLJ-2235-use-keywords-as-loop-names.patch

This is to be applied on top of the squashed "separate special forms" patch:

0001-CLJ-2235-implement-named-loop-recur-to-separate-special-form.patch

Also attaching squashed patch for convenience:

0001-CLJ-2235-recur-to-keyword-loop-names.patch

Here's an example of the new scheme:

(let [m [[[1 2 3] [4 5 6] [7 8 9]]
                 [[10 11 12] [13 14 15] [16 17 18]]
                 [[19 20 21] [22 23 24] [25 26 27]]]]
          ((fn iloop [i ires]
             (if (== i (count m))
               ires
               (loop :jloop [j 0 jres 0]
                 (if (== j (count (get m i)))
                   (recur-to iloop (inc i) (+ ires jres))
                   (loop [k 0 kres 0]
                     (if (== k (count (get-in m [i j])))
                       (recur-to :jloop (inc j) (+ jres kres))
                       (recur (inc k) (+ kres (get-in m [i j k])))))))))
            0
            0))

Note that recur-to still uses symbols where the target is an fn form.

Also note that the approach taken in this patch has the side effect that a loop named :foo won't shadow an fn-introduced recur target named foo. If we wanted eventually to introduce non-fn-introduced recur target using symbols as names (recur-to-targetable Scheme-style let/loop forms as discussed in the previous comment), that would definitely be the way to go. If not, perhaps it's still less arbitrary than declaring that :foo shadows foo in this context?





[CLJ-2234] MultiFn.prefers() ignores the multimethod's internal hierarchy Created: 09/Sep/17  Updated: 12/Sep/17

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: John Alan McDonald Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: multimethods

Attachments: Text File prefersHierarchy.patch    
Patch: Code and Test
Approval: Triaged

 Description   

See https://groups.google.com/forum/#!topic/clojure/3nMn6TiBGOg, which hasn't had any response.

prefers(x,y) visits ancestors by calling parents.invoke() recursively.
This visits the parents in the global hierarchy, not the multimethod's hierarchy.
Is this the intended behavior? Why would that be?

On the assumption that prefer-method should behave the same for a local vs the global-hierarchy, below are 2 unit tests.
MultiFn-prefers-with-local-hierarchy fails with a "Multiple methods" IllegalArgumentException.
MultiFn-prefers-with-global-hierarchy succeeds.

(test/deftest MultiFn-prefers-with-local-hierarchy
  (def local-hierarchy 
    (let [h (make-hierarchy)
          h (derive h ::c0 ::b0)
          h (derive h ::d0 ::c0)
          h (derive h ::d0 ::a0)]
      h))
  (defmulti local identity :hierarchy #'local-hierarchy)
  (defmethod local ::a0 [x] [::a0 x]) 
  (defmethod local ::c0 [x] [::c0 x]) 
  (prefer-method local ::b0 ::a0)
  (test/is (= [::c0 ::d0] (local ::d0)))))

(test/deftest MultiFn-prefers-with-global-hierarchy
  (derive ::c1 ::b1)
  (derive ::d1 ::c1)
  (derive ::d1 ::a1)
  (defmulti global identity)
  (defmethod global ::a1 [x] [::a1 x]) 
  (defmethod global ::c1 [x] [::c1 x]) 
  (prefer-method global ::b1 ::a1)
  (test/is (= [::c1 ::d1] (global ::d1))))

If this is in fact wrong, the fix is pretty easy. I'll submit a patch once it's confirmed this is a real problem.



 Comments   
Comment by Alex Miller [ 11/Sep/17 1:41 PM ]

Patch welcome

Comment by John Alan McDonald [ 12/Sep/17 6:12 PM ]

I'm not sure about the change to preferMethod.
I've moved resetCache to the beginning so that
perfers() can be called with the current state of the hierarchy. Since we are in a write lock, I don't think it's necessary to call it again.





[CLJ-2233] spec calls like def and keys don't act as semantically expected Created: 02/Sep/17  Updated: 04/Sep/17  Resolved: 04/Sep/17

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

Type: Defect Priority: Major
Reporter: Douglas Fields Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: spec
Environment:

MacOS 10.12.6, current spec as of Sept 2 2017



 Description   

spec/def and spec/keys are macros which prevent their use except at the top level with fixed arguments. It seems impossible to send programmatic arguments to these, making it difficult to make specs using normal Clojure code.

In the below example, I want to define specs that are all identical (positive integers) for numerous keys of a map, and then a spec for a gender (which is either :male or :female), and then combine that into a spec for ::avatar, which is a map containing all those keys.

The two problems I encountered is that I cannot call spec/def from within a doseq and I cannot pass in a vector of key names to spec/keys :key-un. Both are macros which treat their arguments extraordinarily literally. So, despite the fact that what I am doing below seems to be semantically correct, it's clearly not working as one would expect. In essence, these two macros (and perhaps others) are very sensitive to their exact source code text and don't allow for metaprogramming them.

Here's the example which should be runnable at the repl:

(require '[clojure.spec.alpha :as spec])
;; Avatar definition
;; We use a new namespace for these keys so that similarly named ones can
;; also be defined
(spec/def :avatar/gender #{:male :female})
(def avatar-layer?
  "Returns true of the value is a positive integer
   used to define a layer of the avatar."
  (spec/and integer? pos?))
(def avatar-spec-parts
  "Parts of the avatar spec that are all the same"
  (vec (map #(keyword "avatar" %)
           ["background" "body" "hair" "mouth" "nose"
            "skin-color" "hair-color" "eyes" "extras"])))
;; Major hack to define all the parts of the avatar map and
;; work around a bug with spec/def
(doseq [ap avatar-spec-parts]
  #_(spec/def ap avatar-layer?) ; This does not work, treats "ap" as a literal ap and namespace qualifies it without even being a keyword
  (spec/def-impl ap (#'clojure.spec.alpha/res avatar-layer?) avatar-layer?))
(def avatar-spec-all
  "All parts of the avatar spec"
  (conj avatar-spec-parts :avatar/gender))

;; This doesn't work, treats avatar-spec-all as a literal
#_(spec/def ::avatar (spec/keys :req-un avatar-spec-all))

As you can see, I had to unroll the spec/def macro to get the doseq to work. That's extraordinarily fragile.

At the end, you can see I cannot even call spec/keys as it expect a literally typed into the source code vector of spec keywords.



 Comments   
Comment by Douglas Fields [ 02/Sep/17 6:07 PM ]

Here's a hack to make what I'm trying to do work, by using eval. It's really ugly though. There's got to be a better way. Thoughts?

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

(defmacro functionize [macro]
  `(fn [& args#] (eval (cons '~macro args#))))

;; Avatar definition
;; We use a new namespace for these keys so that similarly named ones can
;; also be defined
(spec/def :avatar/gender #{:male :female})
(def avatar-layer?
  "Returns true of the value is a positive integer
   used to define a layer of the avatar."
  (spec/and integer? pos?))
(def avatar-spec-parts
  "Parts of the avatar spec that are all the same"
  (vec (map #(keyword "avatar" %)
           ["background" "body" "hair" "mouth" "nose"
            "skin-color" "hair-color" "eyes" "extras"])))
;; Major hack to define all the parts of the avatar map and
;; work around a bug with spec/def
(doseq [ap avatar-spec-parts]
  #_(spec/def ap avatar-layer?) ; This does not work, treats "ap" as a literal ap
  #_(spec/def-impl ap (#'clojure.spec.alpha/res avatar-layer?) avatar-layer?)
  ((functionize spec/def) ap avatar-layer?))
(def avatar-spec-all
  "All parts of the avatar spec"
  (conj avatar-spec-parts :avatar/gender))

;; This doesn't work, treats avatar-spec-all as a literal
#_(spec/def ::avatar (spec/keys :req-un avatar-spec-all))
;; This does work though
(spec/def ::avatar ((functionize spec/keys) :req-un avatar-spec-all))

;; Tests
(spec/explain ::avatar {})
(spec/explain ::avatar {:gender :male :background 1 :body 3 :hair 2 :mouth 1 :nose 3 :skin-color 7 :hair-color 7 :eyes 3})
(spec/explain ::avatar {:gender :male :background 1 :body 3 :hair 2 :mouth 1 :nose 3 :skin-color 7 :hair-color 7 :eyes 3 :extras 77})
Comment by Alex Miller [ 04/Sep/17 7:50 AM ]

There are tradeoffs with using macros vs functions and we considered them at length in the spec definitions. The spec macros capture the form of the macro itself and use that for error reporting in spec/explain and form description in spec/form and spec/describe - those are considered essential features.

There are several workarounds. Using eval is a reasonable way to accomplish your goals although I would probably do something like:

(spec/def ::avatar (eval `(spec/keys :req-un ~avatar-spec-all)))

We may add function entry points in the future (for s/keys in particular as this seems to be the most common place people ask about it) so that is still under consideration.

And finally, we are working on specs for spec forms (CLJ-2112) which allow you to construct data representing a spec, then use spec/unform on it with the spec for spec forms to produce a spec.

Since this is the intended behavior and there are existing tickets and/or work going on for an alternative, I'm going to close this ticket.

Comment by Douglas Fields [ 04/Sep/17 9:38 AM ]

Thanks Alex Miller. Will investigate your other options and tickets and specs-for-spec-forms. Doug





[CLJ-2232] CompilerException can be too large which is thrown due to macro spec error Created: 01/Sep/17  Updated: 11/Sep/17  Resolved: 01/Sep/17

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

Type: Defect Priority: Major
Reporter: Shogo Ohta Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: errormsgs, spec


 Description   
(let [1 x] x)

;; Following error will be thrown

CompilerException clojure.lang.ExceptionInfo: Call to clojure.core/let did not conform to spec:                                                                 
In: [0 0] val: 1 fails spec: :clojure.core.specs.alpha/local-name at: [:args :bindings :binding :sym] predicate: simple-symbol?
In: [0 0] val: 1 fails spec: :clojure.core.specs.alpha/seq-binding-form at: [:args :bindings :binding :seq] predicate: vector?
In: [0 0] val: 1 fails spec: :clojure.core.specs.alpha/map-bindings at: [:args :bindings :binding :map] predicate: coll?
In: [0 0] val: 1 fails spec: :clojure.core.specs.alpha/map-special-binding at: [:args :bindings :binding :map] predicate: map?
:clojure.spec.alpha/spec  #object[clojure.spec.alpha$regex_spec_impl$reify__1200 0x50c628fc "clojure.spec.alpha$regex_spec_impl$reify__1200@50c628fc"]
:clojure.spec.alpha/value  ([1 x] x)
:clojure.spec.alpha/args  ([1 x] x)
 #:clojure.spec.alpha{:problems ({:path [:args :bindings :binding :sym], :pred clojure.core/simple-symbol?, :val 1, :via [:clojure.core.specs.alpha/bindings :clojure.core.specs.alpha/bindings clojure.core.specs.alpha/binding :clojure.core.specs.alpha/binding-form :clojure.core.specs.alpha/binding-form :clojure.core.specs.alpha/local-name], :in [0 0]} {:path [:args :bindings :binding :seq], :pred clojure.core/vector?, :val 1, :via [:clojure.core.specs.alpha/bindings :clojure.core.specs.alpha/bindings :clojure.core.specs.alpha/binding :clojure.core.specs.alpha/binding-form :clojure.core.specs.alpha/binding-form :clojure.core.specs.alpha/seq-binding-form], :in [0 0]} {:path [:args :bindings :binding :map], :pred 
clojure.core/coll?, :val 1, :via [:clojure.core.specs.alpha/bindings :clojure.core.specs.alpha/bindings :clojure.core.specs.alpha/binding :clojure.core.specs.alpha/binding-form :clojure.core.specs.alpha/binding-form :clojure.core.specs.alpha/map-binding-form :clojure.core.specs.alpha/map-bindings], :in [0 0]} {:path [:args :bindings :binding :map], :pred map?, :val 1, :via [:clojure.core.specs.alpha/bindings :clojure.core.specs.alpha/bindings :clojure.core.specs.alpha/binding :clojure.core.specs.alpha/binding-form :clojure.core.specs.alpha/binding-form :clojure.core.specs.alpha/map-binding-form :clojure.core.specs.alpha/map-special-binding], :in [0 0]}), :spec #object[clojure.spec.alpha$regex_spec_impl$reify__1200 0x50c628fc "clojure.spec.alpha$regex_spec_impl$reify__1200@50c628fc"], :value ([1 x] x), :args ([1 x] x)}, compiling:(/private/var/folders/zr/_mpf274n7mn0_5fyx84ypb280000gn/T/form-init2772666393313015405.clj:1:1)

Note that the last part of the error message is too large in this case.

Cause: A CompilerException thrown due to syntax error against a spec'ed macro has ExceptionInfo in it as its cause, and an ExceptionInfo contains the content of its ex-data in the error message if any.
The ex-data, in turn, has the entire explain-data describing the syntax error, which is likely to be large if the macro has a relatively complex syntax.



 Comments   
Comment by Alex Miller [ 01/Sep/17 8:29 AM ]

I don't understand why this is a problem. Tools (as always) should intercept and interpret CompilerExceptions in ways useful to users. Without data, tools can't make choices about what to do.

Comment by Shogo Ohta [ 02/Sep/17 4:07 AM ]

Sorry, I didn't explain it enough. Let me elaborate on it a little more.

I didn't mean to say it's a bad idea the CompilerException has detailed error information at all.
What I mean to say here is that the error message of the CE (which we can get by calling Throwable#getMessage) doesn't have to be messed up with the entire explain-data because the error message itself is just for humans, not for programs.
This behavior couldn't be changed for now even if you would set your own s/*explain-out*:

=> (set! s/*explain-out* (fn [_] (println "Something bad has happened!!")))

=> (let [1 x] x)
CompilerException clojure.lang.ExceptionInfo: Call to clojure.core/let did not conform to spec:                                                                 
Something's wrong!!
#:clojure.spec.alpha{:problems ({:path [:args :bindings :binding :sym],  ...
;; and the entire large explain-data will follow

=> (.getMessage *e)
"clojure.lang.ExceptionInfo: Call to clojure.core/let did not conform to spec:\nSomething's wrong!!\n #:clojure.spec.alpha{:problems ({:path [:args :bindings :binding :sym], ...
;; and the same long long explain-data will go here
Comment by Shogo Ohta [ 11/Sep/17 8:20 PM ]

Alex Miller Do you have any comments on this?

Comment by Alex Miller [ 11/Sep/17 8:30 PM ]

I don't think it's a problem.

Comment by Shogo Ohta [ 11/Sep/17 8:33 PM ]

OK, thanks.





[CLJ-2231] Remove dep lib exclusions Created: 30/Aug/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: Alex Miller Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: build

Attachments: Text File remove-exclusions.patch    
Patch: Code
Approval: Vetted

 Description   

Originally, the spec.alpha and core.specs.alpha lib deps pulled in an older version of Clojure itself as dependencies and they were excluded by Clojure.

These libs have been altered to rely on Clojure, etc as provided scope dependencies instead, so Clojure no longer needs to exclude them (as they are no longer transitive deps).

Note: before applying this patch, the pom must be updated to rely on a version of spec.alpha with these changes (but we haven't released one yet).

Patch: remove-exclusions.patch






[CLJ-2230] s/exercise and gen/generate ignore nil and false when spec is a set Created: 30/Aug/17  Updated: 30/Aug/17  Resolved: 30/Aug/17

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

Type: Defect Priority: Major
Reporter: David Chelimsky Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None


 Description   

(s/exercise #{nil false 1 2 3})
will never produce false or nil.



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

You should never use false or nil values in a set spec.

Comment by Leon Grapenthin [ 30/Aug/17 11:35 AM ]

Why?
I'm asking because that violates least surprise and there is no obvious benefit from that constraint.

Comment by Alex Miller [ 30/Aug/17 11:41 AM ]

This is not actually spec-specific. When sets are used as predicates, they will return the value if it exists in the set. That works fine for every case except if you have either of the two logically false values in the set (false and nil) - in those cases, the return value is logically false, and you do not get the answer you expect for a set.

You see the same issue in doing something like (some #{nil} [1 nil]). It will return nil, which is the nil it found, but is also logically false.

In spec, there are you should handle enumerated logically false values either with predicates like nil? or false? or with s/nilable.

Comment by Alex Miller [ 30/Aug/17 11:42 AM ]

One notable example that comes up is how to spec the set of values true, false, and nil. The best answer is (s/nilable boolean?).

Comment by Alex Miller [ 30/Aug/17 11:46 AM ]

In my opinion, the least surprising thing is to be logically consistent with the rest of the language.

Comment by Leon Grapenthin [ 30/Aug/17 1:02 PM ]

Thank you, that makes sense of course.

Having used Clojure and sets as functions a lot over the last couple of years, what mostly surprises me is that I didn't come up with your explanation myself.

So my explanation is that in the context of s/def, #{...} is seen by my mind as a syntax for enumeration specs, decoupled from Clojures setfunction behavior.

Regarding surprises its arguable that (s/valid? #{false} false) evaluating to true wouldn't surprise anyone who isn't told explicitly that set specs are supposed to work exactly like calling sets as functions.

Also it should be considered that the design appears to be intended as a syntax since there is no s/one-of, which certainly would support nil or false.

It could be worth considering a design exception here for good. Even though it doesn't work with sets as functions it can still work with sets as specs, especially since otherwise they don't have any other imaginable purpose for nil and false (that sets have).

Comment by Leon Grapenthin [ 30/Aug/17 1:10 PM ]

tldr If sets as specs is intended as a syntax (which likely it is), contains? logic would be more consistent than invocation logic (which has no purpose in spec).

Comment by Alex Miller [ 30/Aug/17 1:39 PM ]

No plans to change anything here. `contains?` makes sense for `valid?` but is wrong for `conform`.

Comment by Leon Grapenthin [ 30/Aug/17 1:49 PM ]

I didn't/don't argue that conform should return true or false as contains does.

Instead for the exact same reasons (s/conform #{false} false) returning false instead of ::s/invalid would "make sense".





[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-2228] Unroll constantly to improve performance of multi-arity calls Created: 28/Aug/17  Updated: 27/Sep/17

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

Type: Enhancement Priority: Minor
Reporter: Nathan Marz Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: performance

Attachments: Text File 0001-CLJ-2228-Improve-performance-of-constantly.patch    
Patch: Code and Test
Approval: Prescreened

 Description   

This was found to be a hot spot when testing some Specter use cases.

Review of common uses of constantly shows that the arity 1 case is the most common use of `constantly` in the wild, so only unrolled to two arguments.

Perf test:

(use 'criterium.core)
(def f (constantly 1))
(bench (dotimes [_ 1000] (f)))
(bench (dotimes [_ 1000] (f 1)))
(bench (dotimes [_ 1000] (f 1 2)))

;; Results:
;; Arity Before         After
;; 0     611.455589 ns  607.800747 ns    (no change was expected)
;; 1     3.098828 µs    611.116510 ns    (~5x improvement)
;; 2     3.508726 µs    620.415032 ns    (~5x improvement)

Patch: 0001-CLJ-2228-Improve-performance-of-constantly.patch

Prescreened by: Alex Miller



 Comments   
Comment by Alex Miller [ 29/Aug/17 7:30 AM ]

I want to call out to our workflow diagram at https://dev.clojure.org/display/community/JIRA+workflow for process here. I've triaged this, but it's unlikely to be reviewed for the next release at this point unless it's been prescreened, and that requires a patch. So it would be helpful if you could:

  • mention your use case where this made a difference (Specter)
  • attach a patch
  • include the benchmarks in the ticket description

One question I have is how many arities to unroll. 10 seems like a lot? My guess is that up to 2 or 3 args would cover 90+% of cases but could poke around in https://crossclj.info/fun/clojure.core/constantly.html or do some other research to get an idea.

Comment by Alex Miller [ 29/Aug/17 7:38 AM ]

What is the max arity for constantly you encounter in Specter?

Comment by Alex Miller [ 29/Aug/17 8:04 AM ]

I did some poking around and I suspect arity 1 is the 98% use case and arity 0 making up most of the rest. I failed to actually find an example of any arity >1 although I'm sure there are a few of them out there. I think unrolling to arity 2 would be more than sufficient.

Comment by Nathan Marz [ 27/Sep/17 3:37 PM ]

Specter uses constantly for arity 1, so this patch is sufficient for Specter.





[CLJ-2227] s/form fails to unfn #(...) forms occurring in a nested spec Created: 24/Aug/17  Updated: 24/Aug/17

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

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

Attachments: Text File CLJ-2227.patch    
Patch: Code

 Description   

s/form can unfn #(...) successfully in:

(s/form (s/and #(>= % 42)))
;=> (clojure.spec.alpha/and (clojure.core/fn [%] (clojure.core/>= % 42)))

But not in:

(s/form (s/and (s/and #(>= % 42))))
;=> (clojure.spec.alpha/and (clojure.spec.alpha/and (fn* [p1__1503#] (clojure.core/>= p1__1503# 42))))

; expected:
;  (clojure.spec.alpha/and (clojure.spec.alpha/and (clojure.core/fn [%] (clojure.core/>= % 42))))

The same goes for #(...) forms occurring in any nested specs.

Cause: clojure.spec.alpha/res calls unfn only when it's applied directly to a fn* form. So if a fn* form occurs in a nested spec form, c.s.a/res will do nothing and keep it as is.



 Comments   
Comment by Shogo Ohta [ 24/Aug/17 2:09 AM ]

Added a patch.





[CLJ-2226] AOT Bug regarding "dynamic" class imports Created: 22/Aug/17  Updated: 22/Aug/17  Resolved: 22/Aug/17

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

Type: Defect Priority: Minor
Reporter: Justin Conklin Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: aot, compiler
Environment:

Ubuntu 17.04



 Description   

When AOT compiling, class imports that are not "inline" with the namespace being compiled do not propagate correctly to the compiled classes.

For example, in some of my projects I have a imports.clj file on the classpath that all other namespaces load to avoid having to specify the same imports many times in each namespace declaration.

A simple, reproducible example of the bug is shown in this gist.



 Comments   
Comment by Justin Conklin [ 22/Aug/17 3:31 PM ]

Forgot to mention I'm on OpenJDK 1.8.0_131 64-Bit Server JVM.

Comment by Alex Miller [ 22/Aug/17 5:33 PM ]

This is a good repro, so thanks for that. If you pay careful attention to the stack trace, I think you'll see you are not accurately describing the error here. Note that the root cause error is an NPE in bar (it never even gets to the point where InputStream is used). `import` updates the ns-imports for the current namespace. In the stack trace, it is getting an NPE because the current namespace is actually nil (not foo1, which is what you're expecting). The reason for this is partially related to the same reasons as CLJ-2185 (why it starts as nil, not 'user), and partially related to how AOT generates the initialization code (where the load order is different).

Note also that the docs for gen-class specify "In all subsequent sections taking types, the primitive types can be referred to by their Java names (int, float etc), and classes in the java.lang package can be used without a package qualifier. All other classes must be fully qualified."

Because of the latter, I think I would say that what you're trying to do is not supported (because you must use fully-qualified names anyways in gen-classed code anyways).

Comment by Justin Conklin [ 22/Aug/17 6:08 PM ]

Thank you for your quick, insightful reply.

So the namespace is defined when loading bar.clj when the foo1 namespace is loaded and run via clojure.main, but not when running the AOT-compiled version? Can I infer from CLJ-2185 that the following quote from the compilation reference, "The static initializer for a loader class produces the same effects as does loading its source file", is not true wrt load?

Just to check, I made a new gist in which the foo namespace does not use gen-class and the result seems to be the same.





[CLJ-2225] clojure.core/assert docstring is incorrect Created: 22/Aug/17  Updated: 13/Sep/17

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

Type: Enhancement Priority: Minor
Reporter: Gordon Syme Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring

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

 Description   

The assert macro has two arities, documented as (assert x) and (assert x message)

The docstring is:

Evaluates expr and throws an exception if it does not evaluate to logical true.

This is quite misleading since assert actually throws an Error. It is tempting to use assert assuming that a failure will lead to a stacktrace in logs, like any other Exception, but the reality is that the JVM will terminate.

The behaviour is correct, assert should cause a program to exit if the asserted condition is true but I regularly come across incorrect uses.

I'll work up a patch if people agree this is an issue.



 Comments   
Comment by Alex Miller [ 22/Aug/17 5:54 AM ]

Error is a subclass of Throwable, just like Exception and has no special behavior (although it does have the special intended meaning that most programs should not catch it). Whether or not your program does catch it, or exit, or log, is totally up to your program and not a property of being an Error.

The only change that I think might make sense here is to be more specific about the exception type.

Comment by Gordon Syme [ 13/Sep/17 5:46 AM ]

clj-2225-20170913.patch updates the assert macro docstring. I've wrapped the docstring at 70 characters, it seems to be a fairly common width used throughout the rest of the file.





[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-2223] Add extra map argument to clojure.core/assert and clojure.spec/assert (like with ex-info) Created: 13/Aug/17  Updated: 14/Aug/17

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

Type: Feature Priority: Major
Reporter: Vitalie Spinu Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: assert, ex-info, spec

Attachments: Text File 0001-Add-extra-map-argument-to-clojure.spec-assert.patch     Text File 0001-Add-extra-map-argument-to-clojure.spec-assert.patch    
Approval: Triaged

 Description   

It's a common requirement to provide extra info about the context of the assertion. For instance one might want to include values of local or dynamic bindings into the assert report, or use `clojure.spec/assert` to leverage structured errors with specs outside of testing environments.

With clojure.core/assert one can splice extra data into formated message but with clojure.spec/assert there is no such feature. The message argument to clojure's assert was added in CLJ-774, and it was acknowledged there that the mechanism is not ideal. One notable benefit of passing reports as data is that editors can handle those gracefully in case of large data.

This proposal is related to CLJ-415 but has a broader scope. With an extra map one can pass any value (not just locals), and there is no danger of inadvertently flooding the REPL with large locals.



 Comments   
Comment by Alex Miller [ 13/Aug/17 12:41 PM ]

Can you start this with a use case for what you are trying to do where this would be useful? The title here is a solution but we find it is best to start with a problem, consider alternatives, and choose a solution.

Comment by Vitalie Spinu [ 13/Aug/17 1:44 PM ]

EDITED: Moved motivation into the issue's "Description".

Comment by Alex Miller [ 14/Aug/17 6:56 AM ]

Rewriting the ticket from this perspective would be a good start.

Comment by Vitalie Spinu [ 14/Aug/17 7:35 AM ]

I would be happy to rewrite, but Jira doesn't allow editing the description.

Comment by Alex Miller [ 14/Aug/17 7:58 AM ]

Sorry about that! I've given you edit rights.





[CLJ-2222] Add location metadata to specs Created: 13/Aug/17  Updated: 13/Aug/17  Resolved: 13/Aug/17

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

Type: Enhancement Priority: Minor
Reporter: Vitalie Spinu Assignee: Unassigned
Resolution: Duplicate Votes: 0
Labels: spec

Attachments: Text File 0001-Add-file-and-ns-metadata-to-specs.patch    
Patch: Code

 Description   

Currently it's not possible to access the location where specs were defined. This is needed for editors to be able to navigate to spec's definition and be able to provide ns/project narrowing as part of their spec inspectors. Currently the best option is to list all specs at once and let the user search for their project's specs.

If I am not mistaken, adding correct line and columns is likely to require changes to Clojure compiler, but adding file and ns is straightforward. Attaching a simple patch for this.



 Comments   
Comment by Alex Miller [ 13/Aug/17 6:31 AM ]

Dupe of CLJ-2037

Comment by Alex Miller [ 13/Aug/17 6:34 AM ]

Also, this is related to CLJ-2194.





[CLJ-2221] Allow destructuring of namespaced maps with unqualified symbols in :keys Created: 12/Aug/17  Updated: 12/Aug/17  Resolved: 12/Aug/17

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

Type: Enhancement Priority: Minor
Reporter: Vitalie Spinu Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None


 Description   

I might have missed it but I couldn't find facilities for generic (namespace-less) destructuring of maps in 1.9.

Intuitively, I would expect unqualified symbols in :keys to match only the name of the key in a map irrespective of the namespace of that key. So the following would work:

(defn tt [{:keys [a b]}] [a b])

(tt {:a 23 :b 33})
(tt #:ns1{:a 23 :b 33})
(tt #:ns2{:a 23 :b 33})

There is no discussion of such generic destructuring in CLJ-1919 nor in CLJ-1910. Would a special :*/keys be an option?



 Comments   
Comment by Alex Miller [ 12/Aug/17 7:46 AM ]

Thanks for the suggestion. In all cases currently, you are specifying a specific key to destructure. Destructuring is quite intentionally not a wildcard pattern matching system and there is no plan to change that as that would mean running code at runtime (not compile time) to extract data. While it seems like a small thing, this has big performance implications and we do not wish to go in that direction.





[CLJ-2220] Spec for ns :use clause incorrectly allows nested prefix lists Created: 09/Aug/17  Updated: 09/Aug/17  Resolved: 09/Aug/17

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

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

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

 Description   

Same as CLJ-2219, except for (ns (:use ...)).

Patch: clj-2220.patch - changes ::ns-use in same way as CLJ-2219 for ::ns-require and duplicates ::libspec and ::prefix-list to extend with the filter options allowed in :use.



 Comments   
Comment by Alex Miller [ 09/Aug/17 2:41 PM ]

Applied per Rich's ok





[CLJ-2219] Spec for ns :require clause incorrectly allows nested prefix lists Created: 09/Aug/17  Updated: 09/Aug/17  Resolved: 09/Aug/17

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

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

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

 Description   

In (ns (:require ...)), prefix lists (incorrectly) allow nesting. Per the require docstring, prefix lists are single level only. Should also make a spec representing the concept of libspec as mentioned in require docstring.

  • require contains only: libspec, prefix list, or flag
  • libspec - either a lib or vector of lib and options
  • prefix list - a prefix followed by libspecs

After patch:

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

(s/conform ::ccs/ns-require '(:require (clojure zip [set :as s])))
{:clause :require, :body [[:prefix-list {:prefix clojure, :libspecs [[:lib zip] [:lib+opts {:lib set, :options {:as s}}]]}]]}

(pprint (s/exercise ::ccs/ns-require 5))
([(:require :reload-all) {:clause :require, :body [[:flag :reload-all]]}]
 [(:require :verbose) {:clause :require, :body [[:flag :verbose]]}]
 [(:require :reload (q ?1. (k+5) Mo) (+- !I* (q-) b-))
  {:clause :require,
   :body
   [[:flag :reload]
    [:prefix-list
     {:prefix q,
      :libspecs [[:lib ?1.] [:lib+opts {:lib k+5}] [:lib Mo]]}]
    [:prefix-list
     {:prefix +-,
      :libspecs [[:lib !I*] [:lib+opts {:lib q-}] [:lib b-]]}]]}]
 [(:require (+!*c F! (g :refer :all)) CWR (f ynW QZ Dl!o))
  {:clause :require,
   :body
   [[:prefix-list
     {:prefix +!*c,
      :libspecs
      [[:lib F!] [:lib+opts {:lib g, :options {:refer [:all :all]}}]]}]
    [:libspec [:lib CWR]]
    [:prefix-list
     {:prefix f, :libspecs [[:lib ynW] [:lib QZ] [:lib Dl!o]]}]]}]
 [(:require :reload-all (U+.) :reload)
  {:clause :require,
   :body
   [[:flag :reload-all]
    [:libspec [:lib+opts {:lib U+.}]]
    [:flag :reload]]}])

Patch: clj-2219-2.patch



 Comments   
Comment by Alex Miller [ 09/Aug/17 11:13 AM ]

Applied on Rich's ok

Comment by Alex Miller [ 09/Aug/17 2:41 PM ]

reopened just to fix approval state.

Comment by Alex Miller [ 09/Aug/17 2:42 PM ]

re-closed





[CLJ-2218] Improving consistency of explain-data for instrument/macroexpand-check Created: 06/Aug/17  Updated: 15/Nov/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: 1
Labels: spec

Attachments: Text File CLJ-2218.patch    
Patch: Code

 Description   

Description

If you instrument a function, you may get a spec error like the following:

(defn f [x] (inc x))
(s/fdef f
  :args (s/cat :x (s/and integer? even?))
  :ret (s/and integer? odd?))

(t/instrument)

(f 3)
;; ExceptionInfo Call to #'user/f did not conform to spec:
;; In: [0] val: 3 fails at: [:args :x] predicate: even?
;; :clojure.spec.alpha/spec  #object[clojure.spec.alpha$regex_spec_impl$reify__1200
 0x19b3f9a "clojure.spec.alpha$regex_spec_impl$reify__1200@19b3f9a"]
;; :clojure.spec.alpha/value  (3)
;; :clojure.spec.alpha/args  (3)
;; :clojure.spec.alpha/failure  :instrument
;; :clojure.spec.test.alpha/caller  {:file "form-init3240393046310519022.clj", :lin
e 1, :var-scope user/eval1413}
;; clojure.core/ex-info (core.clj:4725)

(ex-data *e) 
;; {:clojure.spec.alpha/problems
;;   [{:path [:args :x],
;;     :pred clojure.core/even?,
;;     :val 3,
;;     :via [],
;;     :in [0]}],
;;  :clojure.spec.alpha/spec #object[clojure.spec.alpha$regex_spec_impl$reify__1200 0x19b3f9a "clojure.spec.alpha$regex_spec_impl$reify__1200@19b3f9a"],
;;  :clojure.spec.alpha/value (3),
;;  :clojure.spec.alpha/args (3),
;;  :clojure.spec.alpha/failure :instrument,
;;  :clojure.spec.test.alpha/caller {:file "form-init3240393046310519022.clj", :line 1, :var-scope user/eval1413}}

As you can see,

  • the explain-data has a regex (ie. the spec for the args of f) in it as ::s/spec
  • each problem contains :args in their :path

These facts can cause a confusion to spec error reporters because the spec for the args of f ((s/and integer? even?)) has no subspec corresponding to the key :args (I believe :path should only contains keys that is a clue to indicate which subspec to be chosen from a spec).

Possible resolutions

To resolve this confusing situation and improve the consistency of explain-data for instrument check, I think there are two options as follows:

  • Solution 1. removing :args from :path
  • Solution 2. modifying explain-data for instrument check so that they have fspec (rather than :args of it) as ::s/spec

Personally, I prefer Solution 2. since adding fspec in explain-data makes it possible to provide richer error information to *explain-out* implementors.

The same goes for macroexpand-check.



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

The fspec is the spec in question in here and it does have a component :args (the fspec instance supports key lookup via ILookup for :args as well). So while I would like to improve the error message and data here, I don't agree with removing :args from path. One thing that I think would be useful is for an instrumentation failure to better state the invocation (combining the function and the args into the original call). Right now those are separated and it take some mental work to knit the arg list back together.

Comment by Shogo Ohta [ 07/Aug/17 12:20 AM ]

So while I would like to improve the error message and data here, I don't agree with removing :args from path.

Yes, so once we decide to go with Solution 2., then I think there is no need to remove :args from :path.

One thing that I think would be useful is for an instrumentation failure to better state the invocation (combining the function and the args into the original call).

I totally agree that would be useful, though it sounds like it's somewhat beyond the scope of the ticket in terms of consistency improvement.

Comment by Shogo Ohta [ 24/Aug/17 5:05 AM ]

I've made a patch just to express what I meant. Any feedback will be appreciated.

Comment by Ben Brinckerhoff [ 15/Nov/17 9:02 PM ]

One thing that I think would be useful is for an instrumentation failure to better state the invocation (combining the function and the args into the original call). Right now those are separated and it take some mental work to knit the arg list back together.

Apologies if I'm missing something, but is it even possible to get the function from the explain-data? I don't see it in explain-data, but perhaps it is recoverable somehow? In any case, I would greatly appreciate it if this type of information was available, since it would make it possible to print clearer error messages.





[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-2216] Change *explain-out* to return string, instead of printing output Created: 31/Jul/17  Updated: 25/Aug/17  Resolved: 25/Aug/17

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

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

[org.clojure/clojure "1.9.0-alpha17"], [org.clojure/spec.alpha "0.1.123"]


Attachments: Text File CLJ-2216.patch    

 Description   

Currently, the API for the `explain-out` function is that it should take explain-data, print some output to STDOUT, and return nil.

In my experience writing a replacement for the default `explain-out` implementation (https://github.com/bhb/expound), I have found that development would be easier if `explain-out` returned a string that could be printed, instead of printing directly.

The main case where this comes up is when testing `explain-str` either on the REPL or in unit tests. `explain-str` is a useful function to test, since it is a representative use of the `explain-out` function without the extra complexity of `assert` or instrumentation. However, it is hard to use println debugging if anything goes wrong in a custom `explain-out`. For example:

(defn my-explain-printer1 [ed]
  ;; println debugging is not easy, since debug string is embedded in output
  ;; instead of printed on separate line. 
  ;; This is also annoying in tests, since adding debugging output will actually
  ;; make tests for `explain-str` fail (due to extra output)
  (prn "some output here")
  (prn ed))
  
(set! s/*explain-out* my-explain-printer1)
(s/explain-str string? 1)

(defn my-explain-printer2 [ed]
  (prn "some other output")
  ;; If an error is raised (for example, with the bad code below)
  ;; The above output is invisible, since the output is not printed!
  ;; This makes it tricky to debug errors.
  (+ :a 1)
  ;; more code here ...
  (prn ed))

(set! s/*explain-out* my-explain-printer2)
(s/explain-str string? 1)

Furthermore, this may actually simplify the implementation of clojure.spec. When I looked at the latest commit, I found that more often than not, callers to `explain-out` immediately wrapped the call in `with-out-str` to create a string. If `explain-out` returned a string, this would not be necessary.

Instances of `(with-out-str (explain-out ,,,))`

https://github.com/clojure/spec.alpha/blob/8f4e6f7d53db30b20d1f7b1c190f21f40d12ca49/src/main/clojure/clojure/spec/alpha.clj#L699
https://github.com/clojure/spec.alpha/blob/8f4e6f7d53db30b20d1f7b1c190f21f40d12ca49/src/main/clojure/clojure/spec/alpha.clj#L255
https://github.com/clojure/spec.alpha/blob/8f4e6f7d53db30b20d1f7b1c190f21f40d12ca49/src/main/clojure/clojure/spec/alpha.clj#L1925

Instances of `(explain-out )` — direct printing to STDOUT

https://github.com/clojure/spec.alpha/blob/8f4e6f7d53db30b20d1f7b1c190f21f40d12ca49/src/main/clojure/clojure/spec/alpha.clj#L255

I realize this is a breaking change, but since spec is still in alpha, I suspect custom implementations of `explain-out` are fairly rare still, and the upgrade path is simple (implementors simply need to insert an extra `with-out-str` in their printer function), I would greatly appreciate this change.



 Comments   
Comment by Alex Miller [ 31/Jul/17 10:33 PM ]

I'm marking this vetted just so we take a look at it before 1.9 releases but this does not reflect any opinion by Rich. Patch for consideration would help.

Comment by Ben Brinckerhoff [ 02/Aug/17 9:11 PM ]

This is my first patch to clojure. Please advise if I've done something incorrect in regards to submitting the patch.

Comment by Andy Fingerhut [ 03/Aug/17 1:39 AM ]

Ben: Looks like you have produced a patch in the expected format. A couple of tiny suggestions. The subject should include the JIRA issue number CLJ-2216 in it, and a more descriptive subject than "Refactor" would be good. Also I noticed that the date and time marked in the change was "Mon Sep 17 00:00:00 2001" – a slightly more accurate one is probably better.

I haven't done any more substantive checks on the contents of the change – a Clojure screener is best for that.

Comment by Alex Miller [ 03/Aug/17 2:53 PM ]

Agreed with Andy's comments.

To the content, I am in favor with the idea here of making the pluggable thing produce a string rather than print, but have a few comments:

1. explain-printer - seems like rather than printing and with-out-str, the string could be constructed directly instead.
2. explain-out - should print the explain message to out. The current changes return the string and do not print, so this is wrong. Should be (println (explain-out ed)) I think.
3. explain-str - should return a string explain message. It does that now, but only because it relies on explain-out which is wrong and it will be broken when you fix #2. I believe it should be (explain-out (explain-data spec x))
4. explain - works currently, but relies on faulty explain-str. Instead should be (explain-out (explain-data spec x))
5. macroexpand-check - should use explain-str, not explain-out
6. assert* - should use explain-str, not explain-out
5. explain-out and explain-printer are now poorly named, leading to some of the confusion above. Maybe something like explain-message and explain-message would be better? If we're going to break stuff, might as well break it better.

Comment by Ben Brinckerhoff [ 06/Aug/17 9:13 PM ]

Alex and Andy,

Thank you very much for the helpful feedback.

Andy - I have updated the Subject according to your feedback. I'm afraid I don't understand how to best resolve the issue with the timestamp. The "Mon Sep 17 00:00:00 2001" date appears to be added by git when I do {{git format-patch master --stdout -U15 > CLJ-2216.patch}} - this is apparently intentional according to https://git-scm.com/docs/git-format-patch#_discussion. Should I manually adjust this in my editor after generating the patch? Any advice is appreciated. I'm afraid I'm unfamiliar with the patch process.

Alex - I had failed to realize that 'explain-out' was a public function (I agree we should not change the API of that function!). I have added a few tests that helped me when making my changes, but I am happy to remove them if you prefer to not add these tests. Please note that these changes will require a change to Clojure on line https://github.com/clojure/clojure/blob/42a7fd42cfae973d2af16d4bed40c7594574b58b/src/clj/clojure/main.clj#L84. I believe that assert* and macroexpand-check need to use *explain-message* not explain-str, since only the former takes an 'explain-data' param.

Comment by Ghadi Shayban [ 06/Aug/17 9:21 PM ]

The patch date thing is innocuous, the date listed is hardcoded in git. See https://github.com/git/git/blob/e629a7d28a405e48fae6b064a781a10e885159fc/log-tree.c#L362 If you apply the patch it pulls the date from two lines below.

Comment by Shogo Ohta [ 24/Aug/17 4:27 AM ]

I'm afraid I don't agree with this proposal.

Some implementations of s/*explain-out* can have useful facilities like pretty-printing specs or input values when they are somewhat too large to display with prn or something. Using a nice pretty printer (e.g. Fipp), it's even possible to print those values very efficiently with a constant memory consumption.

If the ticket would make s/*explain-out* always return string, then its implementors couldn't take advantage of such a performance improvement.

Comment by Ben Brinckerhoff [ 24/Aug/17 7:04 PM ]

Does that performance improvement hold true if the printer is wrapped in with-out-str? As far as I can tell, only explain uses s/*explain-out* directly. All other calls will create the entire string before returning it.

Comment by Shogo Ohta [ 25/Aug/17 1:29 AM ]

Hm, certainly. I wasn't aware of that.

That said, what I meant to say was that the API for printing spec errors to STDOUT is more flexible and fundamental than the one for returning the result as string, in that the former can do what the latter cannot in some cases.

Comment by Alex Miller [ 25/Aug/17 1:48 PM ]

I've been thinking about this in the background. I think the important benefit of this change would be getting off the stream so that the explain message was built solely as a string. However, any explain printer will likely want to do some formatted printing, and those printers (whether Clojure's or others) are based on the printing apis against streams, not string building. As a result of all that and my general unease about where this has gone, I'm going to decline this ticket.





[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-2213] Allow multiple bindings for if-let, when-let, if-some, and when-some Created: 29/Jul/17  Updated: 04/Aug/17

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

Type: Enhancement Priority: Major
Reporter: Justin Spedding Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None

Attachments: Text File clojure.core.patch     Text File clojure-core v2 8-3-2017.patch     Text File core.specs.alpha.patch    
Patch: Code and Test
Approval: Triaged

 Description   

Description of issue:

Suppose I want to create multiple bindings with let and then execute a body. I can do that easily like so:

(let [a 1
      b (inc a)
      c (* b b)]
  [a b c])

But, if I want to do the same type of thing with if-let, I can only do so by nesting them because if-let only accepts one binding at a time.

(if-let [a 1]
  (if-let [b (inc a)]
    (if-let [c (* b b)]
      [a b c]
      "error")
    "error")
  "error")

This is very inelegant because:
1) It is not as simple to read as it would be if all of the bindings were next to each other on the same indentation
2) The else clause it duplicated multiple times.
3) The else clause is evaluated in a different context depending which binding failed. What if a was already bound to something? If the if-let shadows a, and b does not get bound, the else clause would be executed with a different value bound to a than if a was not shadowed in the first if-let. (see code below for example)

I want to be able to write this instead:

(if-let [a 1
         b (inc a)
         c (* b b)]
  [a b c]
  "error")
=> [1 2 4]

(let [a :original]
  (if-let [a :shadowed
           b false]
          a a))
=> :original

I also want to be able to do a similar thing with when-let, if-some, and when-some.

Proposed:

I re-wrote those macros to be able to handle multiple bindings. If supplied with just one binding, their behavior remains identical. If supplied with multiple bindings, they should only execute the body if every binding passed. In the case of some bindings passing and some failing in if-let or if-some, none of the bindings should leak into the else clause.

Patches:

  • clojure-core v2 8-3-2017.patch - Clojure patch with macro updates. For if-let and if-some, I had to add a bit of extra logic in order to prevent them from leaking bindings to the else clause in the case of some bindings passing and some failing. It also includes a few extra tests around each macro.
  • core.specs.alpha.patch - core.specs.alpha patch with equivalent updates to core specs


 Comments   
Comment by Alex Miller [ 29/Jul/17 4:40 PM ]

What's the relationship of this to CLJ-2007?

Comment by Justin Spedding [ 29/Jul/17 4:52 PM ]

I posted my solutions to that ticket as code in a comment. Then, you posted about the correct format of tickets and linked to the ticket creation guidelines. I figured that meant that you wanted a ticket to be made that followed the conventions.

Also, this ticket is about modifying the existing macros. CLJ-2007 was about creating 2 new macros: if-let* and when-let*.

Comment by Ghadi Shayban [ 03/Aug/17 3:30 PM ]

It is worth looking at what the JVM is intending on doing with test-and-destructure intrinsics. Brian Goetz covers this in a recent talk on pattern matching [1]

[1] https://www.youtube.com/watch?v=n3_8YcYKScw

Comment by Justin Spedding [ 03/Aug/17 9:44 PM ]

An updated patch that simplifies the generated code when 0 bindings are given to if-let and if-some





[CLJ-2212] docstring for `defmethod` is imprecise 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-2212.patch    
Patch: Code
Approval: Triaged

 Description   

arglist:

(defmethod multifn dispatch-val & fn-tail)

docstring:

Creates and installs a new method of multimethod associated with dispatch-value.

There are a few issues with this docstring that make it hard to understand:

1. The & fn-tail argument is not documented. What is the type / shape of & fn-tail. I don't see any mention in the reference guide either - https://clojure.org/reference/multimethods
2. The docstring and arglists conflict about the name multimethod vs. multifn.
3. The docstring and arglists conflict about the name dispatch-value vs. dispatch-val.

Aside: on clojuredocs.org there is a mention of an optional name being allows on the method https://clojuredocs.org/clojure.core/defmethod#example-542692c7c026201cdc3269cd but I don't see that documented anywhere official. I'm not sure if it supported.



 Comments   
Comment by Alex Miller [ 28/Jul/17 9:00 AM ]

fn-tail here means "additional args as could be passed to fn", which does include the function name, so that is supported.

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

Adding patch

Comment by Alex Miller [ 20/Aug/17 5:03 PM ]

Rather than changing the docstring from multimethod to multifn, I would change the code from multifn to multimethod. Otherwise looks good.





[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-2210] back referential expressions can cause exponential compilation times Created: 21/Jul/17  Updated: 06/Sep/17  Resolved: 06/Sep/17

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

Type: Defect Priority: Critical
Reporter: Nicola Mometto Assignee: Unassigned
Resolution: Completed Votes: 4
Labels: compiler, performance

Attachments: Text File 0001-CLJ-2210-cache-non-trivial-getJavaClass-hasJavaClass.patch    
Patch: Code
Approval: Ok

 Description   

Reported as causing problems in real world code: https://groups.google.com/forum/#!topic/clojure/Z91bhUvSB1g

With init.clj as :

(def expr '(fn [& {:keys [a b c d e f
                          map-0 map-1 map-2]}]
             (cond-> "foo"
               a (str a)
               b (str b)
               c (str c)
               d (str d)
               e (str e)
               f (str f)
               map-0 (cond->
                         (:a map-0) (str (:a map-0))
                         (:b map-0) (str (:b map-0))
                         (:c map-0) (str (:c map-0))
                         (:d map-0) (str (:d map-0)))
               map-1 (cond->
                         (:a map-1) (str (:a map-1))
                         (:b map-1) (str (:b map-1))
                         (:c map-1) (str (:c map-1))
                         (:d map-1) (str (:d map-1)))
               map-2 (cond->
                         (:a map-2) (str (:a map-2))
                         (:b map-2) (str (:b map-2))
                         (:c map-2) (str (:c map-2))
                         (:d map-2) (str (:d map-2))))))

(time (eval expr))

Before patch:

[~]> clj -i init.clj
"Elapsed time: 24233.193238 msecs"

After patch:

[~]> clj -i init.clj
"Elapsed time: 24.719472 msecs"

This is caused by every local bindings' type depending on the type of the previous binding, triggering an exponential number of calls to hasJavaClass and getJavaClass. By caching on first occurrence, the complexity becomes linear.

Patch: 0001-CLJ-2210-cache-non-trivial-getJavaClass-hasJavaClass.patch

Prescreened by: Alex Miller






[CLJ-2209] getting started documentation for 1.9.0-alpha uses 1.8.0.jar and does not work with 1.9.0-alpha17.jar due to missing specs related jars in the classpath Created: 20/Jul/17  Updated: 08/Oct/17  Resolved: 20/Jul/17

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

Type: Defect Priority: Major
Reporter: Michael A Wright Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None
Environment:

clojure-1.9.0-alpha17
Java 1.8.0_73-b02
Mac OS X 10.9.5



 Description   

in Https://clojure.org/guides/getting_started when I try:
java -cp clojure-1.8.0.jar clojure.main
it works (starts a REPL) as expected.
When I change to clojure-1.9.0-alpha17.jar I get an error
Caused by: java.io.FileNotFoundException: Could not locate clojure/spec/alpha__init.class or clojure/spec/alpha.clj on classpath.

In order to start a REPL this way with 1.9.0-alpha17, I had to use lein to fetch other dependencies and construct a command like this:

java -cp $HOME/.m2/repository/org/clojure/clojure/1.9.0-alpha17/clojure-1.9.0-alpha17.jar:$HOME/.m2/repository/org/clojure/spec.alpha/0.1.123/spec.alpha-0.1.123.jar:$HOME/.m2/repository/org/clojure/core.specs.alpha/0.1.10/core.specs.alpha-0.1.10.jar:. clojure.main

I also was able to unjar the three jar files and rejar into one jar file to simplify the classpath to one jar.

I'd like to see the getting started documentation address this somehow for Clojure newcomers (after 1.9.0 is released) or for experienced Clojure users that want a quicker way to start a REPL (instead of boot or lein) occasionally.

Perhaps there could be a way to bootstrap a minimal clojure repl without using lein or boot (and without requiring the downloading, and specifying the classpath for, three jar files? It would be nice for newcomers to kick the tires with minimal effort. Perhaps a simplified version of clojure.jar called clojure-repl.jar?



 Comments   
Comment by Michael A Wright [ 20/Jul/17 2:55 PM ]

https://groups.google.com/forum/#!msg/clojure/10dbF7w2IQo/ec37TzP5AQAJ has some discussion related to this. Is there another issue or plan somewhere that is tracking the work needed to update the Getting Started section to correctly work with a clojure-1.9.0.jar when it is release?

Comment by Alex Miller [ 20/Jul/17 9:40 PM ]

Hi Michael, I actually gave a talk about our plans for 1.9 in this area today at EuroClojure and I will have a better writeup for public consumption on that next week.

The Getting Started page is (always) targeted at the current stable release, currently 1.8, and that's something that we will update when Clojure 1.9 is released as part of our typical release process. In this case, it will be substantially updated based on the new stuff coming shortly. Because this is part of our standard release process, I don't need a ticket for it here so I will close this.

Comment by Michael A Wright [ 07/Oct/17 10:35 PM ]

FYI,
https://github.com/clojure/clojure/blob/clojure-1.9.0-beta2/readme.txt still says:

To run: java cp clojure${VERSION}.jar clojure.main

which still doesn't work for the downloaded clojure-1.9.0-beta2.jar

Where can I find the documentation on the recommended way to run clojure using a java command? (I have workarounds involving extracting/combining multiple jar files; in practice I use leiningen or boot to specify the version of the jar.)
Has a decision been made about what to change the README.txt file to say?
I keep seeing invitations to try the 1.9.x betas, but when I "try" them (by following the beta documentation), they don't work.
If I knew what to change the readme to, I wouldn't be asking.

Comment by Alex Miller [ 08/Oct/17 7:39 AM ]

If you use Leiningen or boot, all you should need to do is replace [org.clojure/clojure "1.8.0"] dep with [org.clojure/clojure "1.9.0-beta2"]. The latter states its dependencies, so just using it in any build system that transitively pulls dependencies should make this a non-issue.

If you want to use it from jars, you'll need three jars: clojure, spec.alpha, and core.specs.alpha so something like:

java -cp clojure-1.9.0-beta2.jar:spec.alpha-0.1.134.jar:core.specs.alpha-0.1.24.jar clojure.main

Another option is the new clojure scripts / tools.deps.alpha which you can find more info about at: https://clojure.org/guides/deps_and_cli

This allows you to install the scripts and then just use `clj` to start your repl. (Installers on a broader set of platforms are still a wip.)





[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-2207] Clojure reader should treat non-breaking space as whitespace character Created: 14/Jul/17  Updated: 27/Jul/17  Resolved: 25/Jul/17

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

Type: Enhancement Priority: Minor
Reporter: Nikita Prokopov Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: reader

Attachments: Text File clj-2207-nbsp.patch     Text File clj-2207-nbsp-v2.patch     Text File clj-2207-nbsp-v3.patch    
Patch: Code

 Description   

Right now, Clojure uses Character.isWhitespace(ch) || ch == ',' as the definition of whitespace in the reader. Character.isWhitespace, however, for obscure reasons (it has been defined long time ago), intentionally excludes U+00A0 (no-break space), U+2007 (figure space), U+202F (narrow no-break space). Logically, though, these characters should be treated as normal whitespace for all reasons except text formatting (e.g. the newer Character.isSpaceChar fixed that and does treat them as space chars).

Why is this important: if non-breaking space is inserted by accident (e.g. by pressing Option+Space on Mac), it'll be very hard to find the source of the error in a otherwise very innocent-looking code.

The attached patch implements Util.isWhitespace method that returns true for all characters treated as whitespace by Character.isWhitespace AND for those 3 exceptions. All cases where reading used Character.isWhitespace was referenced are modified to call new Util.isWhitespace instead.

Patch: clj-2207-nbsp-v3.patch

Prescreened by: Alex Miller



 Comments   
Comment by Alex Miller [ 14/Jul/17 10:12 AM ]

I think most of the locations that you changed are not relevant to your request and complicate the evaluation of this patch. Including support for these characters as whitespace in the reader seems fine to me. Determining whether these should be changed in formatting, string, xml, etc function requires more analysis and may not be worth doing so you should narrow this patch to just the changes in LispReader and EdnReader.

Also, the isWhitespace() method should not be in RT (which triggers loading of the Clojure runtime) and would be better in the Util class.

Comment by Nikita Prokopov [ 14/Jul/17 12:46 PM ]

Thanks Alex! Very valuable comments, thank you. See new patch attached. I took a freedom to alter main.clj as well because I feel like it should stay in sync with what ListReader/EdnReader do.

Comment by Alex Miller [ 14/Jul/17 1:44 PM ]

Why did you add the isWhitespace() method in LispReader too? Just make one method in Util and use it from everywhere.

Comment by Nikita Prokopov [ 14/Jul/17 5:52 PM ]

As you said, I didn’t want to change too much. But yes, if we’re not using it in string/xml/formatter it probably can be extracted. See new patch attached.

Comment by Phill Wolf [ 16/Jul/17 7:53 AM ]

javac does not accept non-breaking spaces. The message is

nonbreakingspace.java:4: error: illegal character: '\u00a0'

Do we really want a codebase with an uncontrolled variety of spaces and non-breaking spaces?

If non-breaking spaces are inadvertent, would an editor macro be more appropriate, or a Git hook, or a more pointed Clojure error message?

I am still smarting from tabs-vs-spaces. Java's \s does not match non-breaking spaces. I will be disappointed if things start hiding themselves from grep by using exotic spaces.

Overall - it seems to me that enlarging the category of invisible alternatives for whitespace in source code may not lead to a net improvement in quality of life.

Comment by Alex Miller [ 25/Jul/17 5:03 PM ]

Upon further reflection, I am compelled by Phill's argument. In general for stuff like this, we take our cue from Java and I think that seems like a reasonable approach here as well. I am declining this ticket.

Comment by Nikita Prokopov [ 26/Jul/17 5:43 AM ]

But the fact that we treat non-breaking space as a unique symbol rather than a generic space does give it special meaning. E.g. Clojure does not distinguish between tab and space (and it’s great!), it treats them both just as an empty separator, without assigning each one unique meaning.

What we have with non-breaking space is another invisible char that got its own treatmeant while looking exactly the same. Also, I believe it goes against Unicode intention: non-breaking space should be treated exactly as a space character for all purposes but word wrapping.

Comment by Andy Fingerhut [ 27/Jul/17 12:06 AM ]

Would it be reasonable to treat nonbreaking spaces in Clojure source as an error, unless it is inside of a string, comment, regex, or similar place (I can't think of any other similar places right now, but there may be some)?

Comment by Andy Fingerhut [ 27/Jul/17 12:13 AM ]

A quick test with Java shows that it allows non-breaking spaces inside of comments and strings, but not any of a few other places in a Java source file that I have tested with (e.g. in the middle of an identifier, in the middle of other white space).

Comment by Nikita Prokopov [ 27/Jul/17 12:08 PM ]

> Would it be reasonable to treat nonbreaking spaces in Clojure source as an error

It might work, but why? Why use of some whitespace characters is allowed but other, also whitespace, characters should be forbidden? Nothing magical or special about non-breaking spaces. The intent of them is that they are just like normal spaces. Treating them differently would just confuse people.

Comment by Andy Fingerhut [ 27/Jul/17 12:46 PM ]

Treating them as errors wouldn't be confusing at all – the compiler tells you where they are, and you change them to normal spaces in your Clojure source code and move on. How would that confuse people?

Comment by Nikita Prokopov [ 27/Jul/17 12:52 PM ]

It’s like saying you can’t use letter A in code, only in strings. What’s wrong with letter A? Why can’t I use it. If there’s special rule about it there better be a reason too.

Comment by Andy Fingerhut [ 27/Jul/17 12:55 PM ]

Maybe I can clarify my last question a little bit. Here are 3 alternatives (not intended to be exhaustive):

1. treat non-breaking spaces and similar characters as ones that can be part of var names and symbols

2. treat them as compiler errors, unless they are in comments, strings, or regexes

3. treat them as other whitespace characters are treated.

Alternative #3 is what this ticket proposed, and it was declined.

I think alternative #1 is where Clojure is now, and my guess is that among the 3 alternatives, it is the one most likely to cause confusion for people who accidentally introduce such a character in a Clojure source file.

I believe #2 is what the Java compiler does when compiling Java source files (based only on a few quick experiments, not complete knowledge of the subject). Alex Miller mentioned taking our cue from Java, so I thought I would propose it as an alternative to #1 and #3. If a separate JIRA ticket for this idea is desirable, I'm happy to create one.





[CLJ-2206] Facilities to alias (re-import) functions, variables and macros Created: 13/Jul/17  Updated: 13/Jul/17  Resolved: 13/Jul/17

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

Type: Feature Priority: Minor
Reporter: Vitalie Spinu Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None


 Description   

Organizing namespaces can be hard. Especially in Clojurescript where, due two stage macro compilation, creation of artificial namespaces leaves little room to move around. An often used re-def (def xyz ns/xyz) doesn't preserve metadata.

Zach Tellman's potemkin implements import facilities for functions, vars and macros in Clojure. Would be great to have something similar in Clojure(script) proper.



 Comments   
Comment by Alex Miller [ 13/Jul/17 2:04 PM ]

Thanks, but we do not expect to add var import functionality to Clojure. Namespaces exist to give context to vars and allowing them to be "imported" to other namespaces works against the use of namespaces for naming.

If ClojureScript has problems specific to ClojureScript, solutions can be considered there as needed.





[CLJ-2205] Consider security implications of proxy implementation Created: 12/Jul/17  Updated: 17/Jul/17  Resolved: 17/Jul/17

Status: Closed
Project: Clojure
Component/s: None
Affects Version/s: Release 1.2, Release 1.3, Release 1.4, Release 1.5, Release 1.6, Release 1.7, Release 1.8
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Alex Miller Assignee: Chouser
Resolution: Duplicate Votes: 1
Labels: proxy, security


 Description   

Per the explanation in CLJ-2204, AOT compiled proxies of Serializable/Externalizable classes are susceptible to misuse for deserialization attacks. We should consider modifications to proxy to detect and warn or prevent this kind of misuse.



 Comments   
Comment by Chouser [ 17/Jul/17 11:06 PM ]

Superceded by CLJ-2204





[CLJ-2204] Clojure classes can be used to craft a serialized object that runs arbitrary code on deserialization Created: 12/Jul/17  Updated: 06/Sep/17  Resolved: 06/Sep/17

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

Type: Defect Priority: Critical
Reporter: Alex Miller Assignee: Unassigned
Resolution: Completed Votes: 1
Labels: inspector, security

Attachments: Text File clj-2204-disable-proxy-serialization.patch     Text File clj-2204.patch    
Patch: Code and Test
Approval: Ok

 Description   

If a server can deserialize objects from an untrusted source, it is possible to craft a serialized object that runs arbitrary code on deserialization. Classes in the Clojure jar can be used for this purpose and this may lead to blacklisting the Clojure jar in some environments.

Demonstration of how to create such an object: https://github.com/frohoff/ysoserial/pull/68/files

The serialized class being constructed in the attack is clojure.inspector.proxy$javax.swing.table.AbstractTableModel$ff19274a, but clojure.core.proxy$clojure.lang.APersistentMap$ff19274a and proxy classes created by users are also susceptible. Clojure proxies contain a field (__clojureFnMap) that is a map of method names to proxy functions. Clojure AOT compiles the clojure.inspector and clojure.core.proxy code which generates the classes named above. These classes are then included inside the clojure jar.

The attack constructs an instance of one of these proxy classes and adds a "hashCode" proxy method to the proxy's table. The method is a Clojure function that can run arbitrary code. This instance is then put inside a HashMap and the whole thing is serialized. On deserialization, the HashMap will invoke hashCode() on the proxy object and cause the execution of the arbitrary code.

Note that most uses of proxy will create objects that cannot be serialized anyway, due to the common use of literal function bodies.

Proposed: Modify proxy so that the classes it generates are neither deserializable (to disable the attack) nor serializable (to avoid misleading users). The patch causes proxy classes to have readObject and writeObject methods that just throw an exception, when Serializable is in the inheritance tree of the proxy class. This is technically a breaking change since it is possible to construct a proxy that can be serialized, but such classes are unlikely in the wild and inherently insecure.

Patch: clj-2204-disable-proxy-serialization.patch

Prescreened by: Alex Miller

Background: Raised at https://groups.google.com/d/msg/clojure/WaL3hHzsevI/7zHU-L7LBQAJ and similar to issues reported on Apache Commons. See also:



 Comments   
Comment by Chouser [ 16/Jul/17 5:47 PM ]

I believe there are other proxies of a Serializable classes. I wrote a little script (included below) to dig through a target directory of AOT classes looking for suspicious ones. It may generate false positives, but one it found is generated from this line, and APersistentMap does inherit from java.io.Serializable. Using this clue, I was then able to change the gadget code to use clojure.core.proxy$clojure.lang.APersistentMap$ff19274a instead of the AbstractTableModel proxy and got equivalent results.

I'll keep looking for other classes, but I think we'll unfortunately need to do more than just skip AOT'ing the inspector namespace.

#!/bin/bash

java -cp $HOME/.m2/repository/org/clojure/spec.alpha/0.1.123/spec.alpha-0.1.123.jar:$HOME/.m2/repository/org/clojure/core.specs.alpha/0.1.10/core.specs.alpha-0.1.10.jar:target/classes clojure.main - <<"EOF"

(ns cc
  (:require [clojure.java.io :as io]
            [clojure.reflect :as reflect]
            [clojure.string :as str]
            [clojure.pprint :refer [pprint]]))

(defn ancestor-strs [classname]
 (set (map str (:ancestors (reflect/type-reflect (Class/forName classname) :ancestors true)))))

(->> (file-seq (io/file "target/classes")) ;; all files and dirs
 (keep #(second (re-matches #"target/classes/(.*)\.class" (str %)))) ;; class filenames
 (map #(str/replace % #"/" ".")) ;; classnames
 (filter #(let [as (conj (ancestor-strs %) %)]
	   (and
	    (contains? as "java.io.Serializable")
	    (some (fn [a] (re-find #"proxy" a)) as))))
 prn)

EOF
Comment by Chouser [ 16/Jul/17 9:22 PM ]

Here is the gadget code plus in-memory test harness written in Clojure; makes it a bit easier to test proxy classes for this issue:

(def flag (atom true))                                                         
                                                                               
(defn ok-proxy? [target-proxy]                                                 
  (let [baos (java.io.ByteArrayOutputStream.)                                  
        top (java.util.HashMap.                                                
              {(doto target-proxy                                              
                 (.__initClojureFnMappings {"hashCode" (constantly 0)})) 0})]  
    (.__initClojureFnMappings                                                  
      target-proxy                                                             
      {"hashCode" (comp eval (constantly '(do (reset! flag false) 0)))})       
    (.writeObject (java.io.ObjectOutputStream. baos) top)                      
    (let [payload (.toByteArray baos)]                                         
      (reset! flag true)                                                       
                                                                               
      ;; Deserializing the payload will execute hashCode method above          
      (-> payload                                                              
        java.io.ByteArrayInputStream. java.io.ObjectInputStream. .readObject)  
      @flag)))                                                                 
                                                                               
(prn :ok? (ok-proxy? (bean (Object.))))                                        
(prn :ok? (ok-proxy? (inspector/list-model nil)))                              
Comment by Chouser [ 16/Jul/17 10:03 PM ]

I think we should change the proxy code to generate classes that explicitly refuse to support deserialization. Proxy objects already generally fail to serialize due to the function objects in their __clojureFnMap, so this would not be removing any useful functionality that I'm aware of. I've got a proof of concept here in which proxy always supplies private readObject and writeObject methods, complaining usefully instead of writing, and absolutely refusing to read. Does this seem like a good approach?

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

Hey Chouser, thanks for working on this and finding my gaps! Making proxies nonserializable seems like a good approach to me. Should also cover writeExternal() and readExternal() for Externalizable I think?

Comment by Chouser [ 17/Jul/17 11:22 AM ]

Thanks, as always, for your diligence and expertise on so many of these issues.

It's not clear to me if we need to do the Externalizable methods (I'm only just now learning about these serialization features). Even if a proxied class does implement Externalizable, it would have to provide methods that explicitly serialize and deserialize the proxy class's FnMap – which of course nothing in Java core can do. If an application were to do this, it would not really be Clojure's responsibility to prevent it, even if it could, I think. Am I getting this right?

If we do implement Externailzable methods to prevent them from doing anything dangerous, it would only be for classes that inherit from Externalizable. This is in contrast to readObject/writeObject which are not methods of any interface, and so I believe are essentially harmless to provide at all times.

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

Yep, that makes sense. Externalizable classes would have explicitly made the choice to implement these methods and would not be serializing state of proxy subclass.

I suppose the only potential harm in providing readObject() and writeObject() would be in the (unlikely) case where the parent class was not Serializable but had those methods. I guess we could narrow to just providing them if the class implemented Serializable.

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

Also, I filed a separate ticket (CLJ-2205) re the broader proxy serialization issue which seems like where this is headed. It would probably make sense to rewrite the description here to cover the more general fix and to close CLJ-2205 as a duplicate.

Comment by Alex Miller [ 17/Jul/17 10:56 PM ]

Patch looks good to me - can you add a test with the harness to the patch too?

Comment by Chouser [ 17/Jul/17 11:15 PM ]

Thanks for reviewing it.

I will add a test; it could simply confirm that an attempt to serialize a proxy class fails. Do you think that's sufficient, or should it also include the attempted code execution? I guess the latter could fail for other more subtle reasons, causing the test to pass when perhaps we wouldn't want it to, so I'll go for the simpler test first.

Comment by Alex Miller [ 17/Jul/17 11:22 PM ]

I think the simpler test is sufficient.

Comment by Chouser [ 18/Jul/17 11:00 AM ]

Attached patch with fix and test. Note the test for prohibited deserialization is tricky because we also prevent the creation of the data needed for that test. Because of this, the serialized stream is included directly, along with instructions on how to recreate the data if needed in the future.

Comment by Chouser [ 18/Jul/17 11:54 AM ]

Alex, I just realized (by looking though this ticket's history) that I unintentionally overwrote your Description update last night. Not sure how that happened, but I apologize. Feel free to change it back or take bits from mine and add it to yours, or whatever else you feel is appropriate.





[CLJ-2203] Cannot create sets containing different empty collection, which prevents creating useful specs Created: 10/Jul/17  Updated: 10/Jul/17  Resolved: 10/Jul/17

Status: Closed
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: Declined Votes: 0
Labels: None
Environment:

[org.clojure/clojure "1.9.0-alpha17"]
[org.clojure/clojurescript "1.9.542"]



 Description   

In Clojure:

#{'() []}

Expected: No errors (the above works in Clojurescript)

Actual: "IllegalArgumentException Duplicate key: () clojure.lang.PersistentHashSet.createWithCheck (PersistentHashSet.java:56)"

Motivation: I am trying to define a spec for the "into" argument to "s/coll-of" e.g.

(require '[clojure.alpha.spec :as s])
(s/def :coll-of/into #{'() [] {}})

It is possible to define this spec in Clojurescript, but not Clojure. Using a set to define a spec is idiomatic, but if this is a corner case, it would be fine to use a new "one-of" spec operation that is more verbose than a new set, works the same way when used in a spec, but avoids these cases.



 Comments   
Comment by Alex Miller [ 10/Jul/17 1:22 PM ]

In Clojure, sequential collections are compared in the same "equality partition" - that is lists, vectors, sequences, and potentially other custom sequential collections compare as equal based on their contents, not based on the collection type. This design decision has many subtle tradeoffs but enables many important aspects of using Clojure collections and sequences together in seamless ways. There are no plans to change this, so I am declining this ticket, as it is working as designed.

Regarding ClojureScript, I would start with the typical expectation that its behavior should match Clojure, but also take into account the specifics of the host such that there might be good reasons why this behavior is different (but I don't know enough to say either way).

For this particular spec, I would recommend a spec like (s/and coll? empty?). But please note that it's not possible in most cases to actually instrument spec itself with specs because in many cases this will create an infinite recursion of checks. I have written specs for the spec namespace but in general I did not find it workable in practice to actually use them so we have chosen not to provide them in general. Specs for spec forms are a work in progress tracked in CLJ-2112.





[CLJ-2202] coll-of :min-count and :gen-max used together cause collections that are too large to be generated Created: 10/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: Sebastian Oberhoff Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: spec
Environment:

1.9.0-alpha16


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

 Description   

This should specify a spec whose generator always returns collections of size 4 or 5 but instead it generates collections of size 4 to 8:

(require '[clojure.spec.alpha :as s] '[clojure.spec.gen.alpha :as gen])
(map count (gen/sample (s/gen (s/coll-of char? :min-count 4 :gen-max 5))))
;; (4 7 8 8 4 7 4 5 5 7)

Cause: The max logic in s/every-impl is: (c/or max-count (max gen-max (c/* 2 (c/or min-count 0)))). If there is a max-count it's used, otherwise the larger of gen-max or 2*min-count is used. In this case, 2*min-count is 8. Seems like we should never generate more than gen-max though, so propose changing this logic to: (c/or max-count gen-max (c/* 2 (c/or min-count 0))).

Patch: clj-2202.patch






[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-2200] for macro does not support multiple body expr Created: 05/Jul/17  Updated: 05/Jul/17  Resolved: 05/Jul/17

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

Type: Enhancement Priority: Trivial
Reporter: Isaac Zeng Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None
Environment:

Clojure


Attachments: Text File 0001-let-macro-for-support-multiple-body-expr.patch    
Patch: Code

 Description   

it is not convenient for write multiple expression in for body,
from this now, we must explicit use `do` form, eg.

this form not supported

(for [i (range 10)]
  (prn i)
  (inc i))

we must use `do` wrap body

(for [i (range 10)]
  (do
    (prn i)
    (inc i)))


 Comments   
Comment by Daniel Stockton [ 05/Jul/17 5:10 AM ]

for is list comprehension. There is doseq for side effects: https://clojuredocs.org/clojure.core/doseq

Comment by Isaac Zeng [ 05/Jul/17 6:57 AM ]

@danielstockton

In fact, Many times, I found it will more convenient if for support this feature.

eg.
I want to see what happened in for loops(as normally, I use prn),

I'm also asked many folks, they thought it is better if for has this feature.

Comment by Nicola Mometto [ 05/Jul/17 8:34 AM ]

This is by design to discourage side-effects in its body, not only `for` is list comprehension and not a loop construct, but it's also lazy so if we were to facilitate side-effecty bodies in for, many would trip up on the laziness.

Comment by Alex Miller [ 05/Jul/17 8:41 AM ]

Declined, per the reasons already listed.





[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-2198] int-in-range? doesn't check for int-ness Created: 01/Jul/17  Updated: 01/Jul/17  Resolved: 01/Jul/17

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

Type: Defect Priority: Major
Reporter: Frederic Merizen Assignee: Unassigned
Resolution: Duplicate Votes: 0
Labels: spec
Environment:

Clojure 1.9.0-alpha17
Spec.alpha 0.1.123



 Description   
(require '[clojure.spec.alpha :as s])
(s/int-in-range? 1 4 1.5)
=> true

I expected the result to be false, because 1.5 is not an integer.

Cause of the bug: in the following snippet, int? is always truthy. It should read (int? val) instead.

(defn int-in-range?
  "Return true if start <= val, val < end and val is a fixed
  precision integer."
  [start end val]
  (c/and int? (<= start val) (< val end)))

The bug is fairly harmless in practice because int-in, the main client of int-in-range?, 'redundantly' checks for int-ness:

(defmacro int-in
  "Returns a spec that validates fixed precision integers in the
  range from start (inclusive) to end (exclusive)."
  [start end]
  `(spec (and int? #(int-in-range? ~start ~end %))
     :gen #(gen/large-integer* {:min ~start :max (dec ~end)})))


 Comments   
Comment by Alex Miller [ 01/Jul/17 2:37 PM ]

Dupe of https://dev.clojure.org/jira/browse/CLJ-2167 - fix should go in soon.





[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/instr