<< Back to previous view

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

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

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

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.





[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-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-2197] instrument :stub doesn't use :gen override Created: 30/Jun/17  Updated: 30/Jun/17

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

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


 Description   

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

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

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

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

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

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

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

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

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

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

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

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

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

;; And confirm BadYer instances will throw when called.

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


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

;; Confirm generator works as expected:

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

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

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

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

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





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

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

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


 Description   
  1. Spec metadata support

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

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

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

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

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

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

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

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



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

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

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

Changed example as per Nicolas' comment.

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

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

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

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





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

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

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

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



 Description   

Repro:

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

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

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

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

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



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

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

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

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

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

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

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

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

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

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

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

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





[CLJ-2162] condp should accept macros as first argument. Also would allow proper inlining Created: 07/May/17  Updated: 15/May/17

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

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


 Description   

Currently condp takes the value of the first argument pred. This has two disadvantages:

  1. It won't allow macros to be used as the first argument
  2. It will be less efficient since the function calls can't be inlined anymore (common idiom: condp = x ...)

If pred is a symbol the let binding can be avoided. This would properly inline = which is common.



 Comments   
Comment by Alex Miller [ 07/May/17 3:30 PM ]

Concrete examples would help.

Comment by A. R [ 08/May/17 12:36 AM ]

In my example I have a macro that matches routes. Since they're static this can be optimized during compile time:

(condp route-match? x [:comments :view] 1 [:posts :edit] 2 [_ :delete] 3)

Would also provide a faster assoc for `records` which generates a `condp` with `identical`.





[CLJ-2159] Disambiguate behavior of def with doc-string Created: 23/Apr/17  Updated: 23/Apr/17

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

Type: Enhancement Priority: Trivial
Reporter: Christopher Brown Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring, documentation
Environment:

REPL


Attachments: Text File clarify-def-forms.patch    
Patch: Code

 Description   

As far as I can tell, it's impossible to use `def` to create a var that's unbound but has `:doc` metadata (or to change the `:doc` metadata of an existing var without also binding / changing the bound value).

This change clarifies the possible usages of `def`; i.e., if you supply `doc-string`, you must also supply `init`.






[CLJ-2157] multi-spec doesn't generate possible tags from hierarchy Created: 22/Apr/17  Updated: 22/Apr/17

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

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


 Description   

Problem: Even though multi-spec supports hierarchy dispatch of multi-methods, its generator only generates tags that have direct method implementations.

Proposed solution: It should also generate from hierarchy.






[CLJ-2144] clojure.walk/keywordize-keys wants ns support for clojure.spec utility Created: 08/Apr/17  Updated: 15/May/17

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

Type: Feature Priority: Major
Reporter: Aaron Brooks Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File 0001-CLJ-2144-Add-namespace-arg-to-walk-keywordize-keys.patch    

 Description   

keywordize-keys currently takes a single argument, a nested structure presumably containing maps, turning all string keys into un-namespaced keys. I've found that I've needed to maintain my own modified version of keywordize-keys that allows me to pass a namespace so I can import JSON objects and use them with clojure.spec (which strongly prefers namespaced keys).

The addition of an additional 2-arity invocation with a namespace string argument is a non-breaking change. I'll attach a patch once I have a JIRA number.



 Comments   
Comment by Aaron Brooks [ 08/Apr/17 1:51 PM ]

This patch also includes a test but I accidentally selected "Code" when I opened the ticket. I don't think I can change that. Can a maintainer update the "Patch" field to "Code and test"?

Comment by Alex Miller [ 11/Apr/17 4:10 PM ]

Another route to go with this btw is to first do CLJ-1899 to build on.





[CLJ-2133] Clarify documentation for the satisfies? function. Created: 23/Mar/17  Updated: 23/Mar/17

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

Type: Enhancement Priority: Minor
Reporter: David Chelimsky Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: docstring
Environment:

N/A



 Description   

The docs for satisfies? says "Returns true if x satisfies the protocol", but does not define the meaning of "satisfies". The function returns true when type and protocol are referenced in the same call to either extend-type or extend-protocol even when none of the protocol functions are implemented. I think the doc should be specific about this to avoid confusion.






[CLJ-2116] Support for selective conforming with clojure.spec Created: 22/Feb/17  Updated: 04/Oct/17

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

Type: Feature Priority: Major
Reporter: Tommi Reiman Assignee: Unassigned
Resolution: Unresolved Votes: 23
Labels: spec
Environment:

[org.clojure/clojure "1.9.0-alpha14"]


Attachments: Text File clj-2116.patch    

 Description   

Problem

using clojure.spec in runtime border validation supporting multiple exchange formats is hard.

Details

Currently in clojure.spec (alpha-14), conformers are attached to Spec instances at creation time and they are invoked on every conform. This is not very useful in system border validation, where conforming/coercion functions should be selected based on runtime data, e.g. the exchange format.

Examples:

  • a keyword? spec:
    • with EDN, no coercion should be done (it can present Keywords)
    • with JSON, String->Keyword coercion should be applied
    • with String-based formats (CSV, query-params, ...), String->Keyword coercion should be applied
  • a integer? spec:
    • with EDN, no coercion should be done (it can present numbers)
    • with JSON, no coercion should be done (it can present numbers)
    • with String-based formats (CSV, query-params, ...), String->Long coercion should be applied

Here is a more complete example:

(s/def ::id integer?)
(s/def ::name string?)
(s/def ::title keyword?)
(s/def ::person (s/keys :opt [::id], :req-un [::name ::title]))

;; this is how we see the data over different exchange formats
(def edn-person {::id 1, :name "Tiina", :title :boss})
(def json-person {::id 1, :name "Tiina", :title "boss"})
(def string-person {::id "1", :name "Tiina", :title "boss"})

;; here's what we want
(def conformed-person edn-person)

To use this today, one needs to manually create new border specs with different conformers for all different exchange formats. Non-qualified keywords could be mapped in s/keys to work (e.g. ::title => ::title$JSON), but this wont work if fully qualified keys are exposed over the border (like ::id in the example) - one can't register multiple, differently conforming version of the spec with same name.

Suggestion

Support selective conforming in the Spec Protocol with a new 3-arity conform* and clojure.spec/conform, both taking a extra user-provided callback/visitor function. If the callback is provided, it's called from within the Specs conform* with the current spec as argument and it will return either nil or a 2-arity conformer function that should be used for the actual confrom.

Actual conforming-matcher implementations can be maintained in 3rd party libraries, like spec-tools[1].

Using it would look like this:

;; edn
(assert (= conformed-person (s/conform ::person edn-person)))
(assert (= conformed-person (s/conform ::person edn-person nil)))

;; json
(assert (= conformed-person (s/conform ::person json-person json-conforming-matcher)))

;; string
(assert (= conformed-person (s/conform ::person string-person string-conforming-matcher)))

Alternative

Another option to support this would be to allow Specs to be extended with Protocols. 3rd party libs could have a new Conforming protocol with 3-arity conform and add implementations for it on all current specs. Currently this is not possible.

[1] https://github.com/metosin/spec-tools



 Comments   
Comment by Alex Miller [ 22/Feb/17 3:33 PM ]

I don't think we are interested in turning spec into a transformation engine via conformers, so I suspect we are probably not interested. However, I'll leave it for Rich to assess.

Comment by Tommi Reiman [ 23/Feb/17 1:26 AM ]

Currently, Plumatic Schema is the tool used at the borders. Now, people are starting to move to Spec and it would really bad for the Clojure Web Developement Story if one had to use two different modelling libraries for their apps. If Spec doesn't want to be a tranformation engine via conformers, I hope for the Alternative suggestion to allow 3rd parties to write this kind of extensions: exposing Specs as Records/Types instead of reified protocols would do the job?

Comment by ken restivo [ 28/Feb/17 9:43 PM ]

I could see why the Clojure core developers might not want Spec to support this kind of coercion, but the practical reality is that someone will have to. If it isn't in Spec itself, it'll have to be done libraries built upon it like Tommi's.

The use case here is: I have a conf file that is YAML. I'm parsing the YAML using a Clojure library, turning it into a map. Now I have to validate the map, but YAML doesn't support keywords, for example, and the settings structure goes directly into Component/Mount/etc as part of the app state, so it makes sense to run s/conform on it as the first step in app startup after reading configuration. Add to this the possibility of other methods of merging in configuration (env vars, .properties files, etc) and this coercion will be necessary somewhere.

Comment by Tommi Reiman [ 08/May/17 1:03 PM ]

Any news on assessing this? I would be happy to provide a patch or a link to a modified clojure.spec with samples on usage with the 3-arity conform in it. Some thinking aloud: http://www.metosin.fi/blog/clojure-spec-as-a-runtime-transformation-engine/

Comment by Alex Miller [ 09/May/17 10:10 AM ]

Rich hasn't looked at it yet. My guess is still that we're not interested in this change. While I think some interesting problems are described in the post, I don't agree with most of the approaches being taken there.

Comment by Simon Belak [ 10/May/17 4:39 AM ]

Why not just use s/or (or s/alt) and then dispatch on the tag. Something like:

(s/def ::id (s/and (s/or :int integer?
                         :str string?)
                   (s/conformer (fn [[tag x]]
                                  (case tag
                                    :int x
                                    :str (Integer/parseInt x))))))

I use that pattern quite a bit in https://github.com/sbelak/huri and with a bit of syntactic sugar it works quite well.

Comment by Imre Kószó [ 12/May/17 3:46 AM ]

Simon that will not work if you are trying to conform to specs from third parties though. One of the points of this suggestion is that third parties would be able to write their own conformers to existing specs without redefining those specs.

Comment by Tommi Reiman [ 08/Jun/17 1:40 AM ]

Thanks for the comments. I would be happy to provide a patch / sample repo with the changed needed for this, in hope that it would help to decide if this could end up in the spec or not. What do you think?

Below is a sample of initial spec-integration into ring/http libs, using spec-tools. For now, one needs to wrap specs into spec records to enable the 3-arity conforming. This is boilerplate I would like to see removed. With this change, it should work out-of-box for all (3rd party) specs.

(require '[compojure.api.sweet :refer :all])
(require '[clojure.spec.alpha :as s])
(require '[spec-tools.core :as st])

;; to enable 3-arity conforming
(defn enum [values]
  (st/spec (s/and (st/spec keyword?) values)))

(s/def ::id int?)
(s/def ::name string?)
(s/def ::description string?)
(s/def ::size (enum #{:L :M :S}))
(s/def ::country (st/spec keyword?) ;; to enable 3-arity conforming
(s/def ::city string?)
(s/def ::origin (s/keys :req-un [::country ::city]))
(s/def ::new-pizza (st/spec (s/keys :req-un [::name ::size ::origin] :opt-un [::description])))
(s/def ::pizza (st/spec (s/keys :req [::id] :req-un [::name ::size ::origin] :opt-un [::description])))

;; emits a ring-handler with input & output validation (& swagger-docs)
;; select conforming based on request content-type (e.g. json/edn) + strip-extra keys from maps
(context "/spec" []
  (resource
    {:coercion :spec
     :parameters {:body-params ::new-pizza}
     :responses {200 {:schema ::pizza}}
     :post {:handler (fn [{new-pizza :body-params}]
                       (ok (assoc new-pizza ::id 1))}}))
Comment by Tommi Reiman [ 21/Jul/17 4:13 AM ]

Intended to create internal PR in my fork of clojure.spec, but ended up doing a real DUMMY PR for the actual repo. Well, here it is anyway:

https://github.com/clojure/spec.alpha/pull/1

Happy to finalize & create a patch into Jira if this goes any further.

Comment by Tommi Reiman [ 21/Jul/17 4:14 AM ]

comments welcome. here's a sample test for it:

(deftest conforming-callback-test
  (let [string->int-conforming
        (fn [spec]
          (condp = spec
            int? (fn [_ x _]
                   (cond
                     (int? x) x
                     (string? x) (try
                                   (Long/parseLong x)
                                   (catch Exception _
                                     ::s/invalid))
                     :else ::s/invalid))
            :else nil))]

    (testing "no conforming callback"
      (is (= 1 (s/conform int? 1)))
      (is (= ::s/invalid (s/conform int? "1"))))

    (testing "with conforming callback"
      (is (= 1 (s/conform int? 1 string->int-conforming)))
      (is (= 1 (s/conform int? "1" string->int-conforming))))))
Comment by Tommi Reiman [ 22/Jul/17 2:12 AM ]

initial work as patch.

Comment by Tommi Reiman [ 04/Oct/17 1:02 AM ]

Any news on this?





[CLJ-2115] Support data conveying conform errors (alternative/complement to :clojure.spec/invalid) Created: 22/Feb/17  Updated: 15/May/17

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

Type: Feature Priority: Major
Reporter: Max Penet Assignee: Unassigned
Resolution: Unresolved Votes: 5
Labels: spec


 Description   

At the moment if a conform calls fails (returning :clojure.spec/invalid) there is no way to supply extra information about why it failed. We do have the possibility to get explain-data, but at best this would return a spec form, the original value and some metadata.

While this is fine in most cases, some conformer functions upon failure can provide extra data that'd be useful to the consumer. A practical example we had was a spec that can contain values for a String based DSL (think SQL like), that would conform these values to their parsed AST. When the conform wrapped function fails it would throw an ex-info with line/col info and more metadata about the failure. But all this data was lost since we can only return :clojure.spec/invalid. All this happened inside a rule engine schema, that can contain hundreds of these; re-parsing all the failing values for error reporting is something we wanted to avoid.

The proposal would be to to support a new return value that'd allow conveying data about the conform failure, or to support both this new value and :clojure.spec/invalid.
This could take the form of (explain-info {..}) potentially returned by conformer function for later consumption by explain, to match clojure semantics with exceptions (ex-info/ex-data, explain-info/explain-data).

A more naive implementation could just allow to throw inside the conformer function and have the error merged/assoced into the explain map (but that might be a bit too invasive in my opinion).






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

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

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


 Description   

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

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

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

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


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

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





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

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

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

clojure 1.9.0-alpha14



 Description   

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

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

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

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

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

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

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

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



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

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

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

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

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

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

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

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





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

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

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


 Description   

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



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

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

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

Related: CLJ-1527





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

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

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


 Description   

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



 Comments   
Comment by Martin Clausen [ 24/May/17 1:33 AM ]

The potential recursive behaviour of clojure.walk/walk depends on the inner function passed to it and is not inherent to clojure.walk/walk itself. If you look at the source code for clojure.walk/prewalk and clojure.walk/postwalk, they are both implemented using walk, but passed a recursive inner function.





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

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

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

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

 Description   

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

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

This feels like an implementation detail leak.



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

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

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

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

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

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

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

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

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





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

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

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

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


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

 Description   

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

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

an exception will be thrown:

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

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






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

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

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


 Description   

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

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

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

Instead, this has to be written as

(def ^Character/TYPE x \space)

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

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

Primitive type aliases should be handled consistently.

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



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

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

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

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





[CLJ-2022] Add fixed arities for mapcat Created: 13/Sep/16  Updated: 13/Sep/16

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

Type: Enhancement Priority: Minor
Reporter: Robert C Faber Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: performance

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

 Description   

Add fixed arities for mapcat, in the pattern of map.

Same change as CLJS-1776.



 Comments   
Comment by Alex Miller [ 13/Sep/16 10:29 AM ]

Presumably this is to improve performance. Please include a benchmark showing the difference.

Comment by Ghadi Shayban [ 13/Sep/16 12:47 PM ]

Please consider interactions with apply and laziness CLJ-1218





[CLJ-2021] case where spec/conform -> spec/unform -> spec/conform gives invalid result Created: 12/Sep/16  Updated: 28/Jan/17

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

Type: Defect Priority: Major
Reporter: Jeroen van Dijk Assignee: Unassigned
Resolution: Unresolved Votes: 3
Labels: spec
Environment:

clojure 1.9, mac osx, java 1.8



 Description   

The example belows shows a case where a conform-ed form, does not conform any after an unform. It would be my expectation that you can repeat conform -> unform -> conform endlessly and get the same result.

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

(s/def ::defn-macro (s/cat :type #

Unknown macro: {'defn}
:definition :clojure.core.specs/defn-args))

(let [form '(defn foo "bar" ([a & b] a a c) ([a b] a))]

(-> form
(->> (s/conform ::defn-macro))) ;;=> {:type defn, :definition {:name foo, :docstring "bar", :bs [:arity-n {:bodies [{:args {:args [[:sym a]], :varargs {:amp &, :form [:sym b]}}, :body [a a c]} {:args {:args [[:sym a] [:sym b]]}, :body [a]}]}]}}

;; Unforming returns the function definition, but with the args in a list instead of a vector:
(->> form
(s/conform ::defn-macro)
(s/unform ::defn-macro)) ;;=> (defn foo "bar" ((a (& b)) a a c) ((a b) a)))

;; Conforming after unforming doesn't work anymore
(->> form
(s/conform ::defn-macro)
(s/unform ::defn-macro)
(s/conform ::defn-macro)) ;;=> :clojure.spec/invalid

)



 Comments   
Comment by Jeroen van Dijk [ 12/Sep/16 8:22 AM ]

This gist shows the above code with better formatting https://gist.github.com/jeroenvandijk/28c6cdd867dbc9889565dca92673a531

Comment by Leon Grapenthin [ 28/Jan/17 4:49 PM ]

This can quickly be traced down to :clojure.core.specs/arg-list which is speced as a (s/and <regex> vector?). When unforming, it doesn't create a vector.

Thinking about it, a vcat would be nice for this and similar cases.





[CLJ-2017] with-gen should specify if the generator should return conformed or unformed data Created: 03/Sep/16  Updated: 04/Sep/16

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

Type: Enhancement Priority: Major
Reporter: lvh Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring, spec


 Description   

I think the answer is "unformed", but this isn't very clear from the docstring.



 Comments   
Comment by Alex Miller [ 03/Sep/16 6:46 PM ]

The answer is definitely unconformed. Conforming only happens when you call conform. This doesn't seem confusing to me, but maybe it should be clearer. I suspect it would be better to clarify this in a reference documentation page though.

Comment by lvh [ 04/Sep/16 10:29 AM ]

I agree that a reference documentation change would be most helpful.

I rely heavily on my environment showing me docstrings, so a small point (maybe just unconformed/unformed/whatever in parens) in the docstring would still be helpful.





[CLJ-2011] clojure.walk.macroexpand-all will not properly expand macros that depend on &env Created: 23/Aug/16  Updated: 23/Aug/16

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

Type: Defect Priority: Major
Reporter: Collin Bell Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: macro, walk
Environment:

MacOSX, Clojure 1.9.0-alpha10, Java 1.8.0_45, CIDER 0.13.0snapshot (package: 20160602.809), nREPL 0.2.12



 Description   

(clojure.walk/macroexpand-all '(defn foo [a] (go [] a)))

Unhandled clojure.lang.ExceptionInfo
Could not resolve var: a
{:var a}

This is because go depends on &env and macroexpand-all does not handle &env.

The reason this issue is important is because it breaks the cider debugger for async.






[CLJ-1997] Macros cannot reliably detect usage of locals Created: 02/Aug/16  Updated: 02/Aug/16

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

Type: Enhancement Priority: Major
Reporter: Gary Fredericks Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: macro


 Description   

Problem

The motivating problem is the implementation of gen/let in test.check (see also TCHECK-98).

A common usage of gen/let might look something like this:

(gen/let [a gen-a
          b gen-b]
  (f a b))

The crucial characteristic of this code is that the generator for b does not depend on the value a (though in general it could). Because of this independence, the ideal expansion is:

(gen/fmap 
  (fn [[a b]] (f a b)) 
  (gen/tuple gen-a gen-b))

However, because gen/let cannot, in general, tell whether or not the expression for the generator for b depends on a, it needs to fallback to a more general expansion:

(gen/fmap
  (fn [[a b]] (f a b))
  (gen/bind 
    gen-a
    (fn [a]
      (gen/tuple (gen/return a) gen-b))))

Using gen/bind greatly reduces shrinking power, and so it's best to avoid it when possible.

A knowledgeable user could get around this by using gen/tuple explicitly, e.g.:

(gen/let [[a b] (gen/tuple gen-a gen-b)]
  (f a b))

But I think most users would prefer not to have to think about these things.

Possible Solutions

tools.analyzer

tools.analyzer is probably adequate, but is a large dependency for a library.

a subset of tools.analyzer

Nicola has mentioned the idea of carving out some subset of the analyzer that would be sufficient for this case, and that might be the best option.

a mechanism for macroexpanding a macro body

I believe if there were a robust mechanism for a macro to fully macroexpand an expression that this problem would be easier (clojure.core/macroexpand and friends have a few known incorrectnesses) – a simple tree-seq over the expanded expression could prove that a local is not used (though a naive approach might falsely conclude that a local *is* used, which might be an acceptable compromise for the test.check case, and otherwise a robust code walker should not be difficult to implement on expanded code).

I believe zach's riddley library does something like this, and depending on riddley would probably be the best option for a non-contrib library, but is not an acceptable dependency for a contrib library.






[CLJ-1996] clojure.spec stubs don't cooperate with clojure.spec.test/check Created: 31/Jul/16  Updated: 31/Jul/16

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

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


 Description   

This is just like CLJ-1949, but for stubs instead of higher-order-function arguments.

The solution is more difficult, though, since cst/check and cst/instrument can be called/used seperately.

My only idea is to have a dynamic var where the two can coordinate. Stubs would use gen/generate when not called during testing, but in the context of a call to cst/check the dynamic var would contain an alternate implementation that works similarly to the patch in CLJ-1949.

I'd be happy to prepare a patch with that implementation (or any other) if desired.






[CLJ-1989] `let` ported from `test.check/let` to `clojure.spec.gen` Created: 24/Jul/16  Updated: 06/Jul/17

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

Type: Feature Priority: Minor
Reporter: Matthew Wampler-Doty Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: spec

Attachments: Text File gen-let.patch    

 Description   

When using `clojure.spec` for elaborate specifications and `clojure.spec.gen` for generative testing, developers often find themselves writing code which heavily relies on `clojure.spec.gen/fmap`. This is sometimes unnatural and difficult to read.

To make writing custom generators easier, this patch ports `test.check/let` to `clojure.spec.gen`. Now developers can write generators more simply.



 Comments   
Comment by Matthew Wampler-Doty [ 24/Jul/16 5:55 PM ]

For example, if a user wanted to make a generator of vectors with length between 5 and 11 or 20 to 40 elements, consisting of keywords which were either `:a` or `:b`, they would have to write something like:

(gen/fmap (fn [[n gens]] (take n gens)) 
          (gen/tuple (spec/gen (spec/or :short (int-in 5 11) 
                                        :long (int-in 20 40)))
                     (gen/vector (gen/elements #{:a :b}) 40)))

With this patch they could write this as:

(gen/let [length (spec/or :short (int-in 5 11)
                          :long  (int-in 20 40))]
  (repeat length #{:a :b}))
Comment by Rick Moynihan [ 06/Jul/17 6:57 AM ]

Ran into the lack of this useful macro recently. And spoke to @gfredricks about it on slack. He mentioned that the supplied patch looks to be a complete reimplementation of gen/let with different behaviour.

He suggested an alternative might be to have the gen/let macro make a require call, which when succeeds just expands to use the test.check let implementation. If the require fails the macro can expand into code that raises an appropriate error. Presumably indicating that test.check needs to be included as a dependency.

e.g.

 (defmacro let [& args] (try (require ...) `(...) (catch Exception e `(throw ...)))) 




[CLJ-1984] clojure.spec/double-in should allow strict greater-than, less-than tests Created: 20/Jul/16  Updated: 21/Jul/16

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

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


 Description   

clojure.spec/double-in defines a spec that tests whether a double is greater than or equal to a minimum value and less than or equal to a maximum value. This seems like an arbitrary choice from the point of view of mathematics and practical concerns. Sometimes you need to test whether a double is greater than a minimum or less than a maximum. Example: The application will divide by the tested double later.

Of course we can add tests to double-in, e.g. like

(s/and (s/double-in :min 0.0 :max 1.0) #(not= 0.0 %))}}

but

#(and (> % 0.0) (<= % 1))

might be clearer if double-in's NaN and Infinity tests aren't needed.

Why not have a common interface to all four interval tests? Rather than four different spec functions, which is one option, I suppose, I suggest adding two keywords to double-in. When true, these would change the >= or <= tests to > or < tests:

:min-greater

(or? :min+, :min-greater-than, :greater-than-min, :strict-min, :min-open, or possibly :infinmum, :inf, but that could be misleading)

:max-less

(or :max- :max-less-than, :less-than-max, :strict-max, :max-open, or possibly :supremum, :sup etc.)

For example,

(s/valid? (s/double-in :min 0.0 :max 0.1 :min-greater true) 0.0)

would return false, but

(s/valid? (s/double-in :min 0.0 :max 0.1 :min-greater false) 0.0)

would return true.

Default values for these keywords should probably be false, for compatibility with the current definition of double-in.






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

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

Type: Defect Priority: Major
Reporter: Alexander Kiel Assignee: Unassigned
Resolution: Unresolved Votes: 3
Labels: spec


 Description   

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

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

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



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

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

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

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

(defn parse-long [s]
  (try
    (Long/parseLong s)
    (catch Exception _
      ::s/invalid)))
Comment by Alexander Kiel [ 12/Jul/16 10:01 AM ]

No change in alpha 10 with the removal of :clojure.spec/any and introduction of any?.

Comment by Sean Corfield [ 12/Sep/16 4:06 PM ]

Another example from Slack, related to this:

(if-let [a 1]
  ::s/invalid)

Fails compilation (macroexpansion) because ::s/invalid causes the spec for if-let to think the then form is non-conforming.

Workaround:

(if-let [a 1]
  '::s/invalid)
Comment by Ambrose Bonnaire-Sergeant [ 05/Sep/17 3:41 PM ]

Another example from the wild: https://github.com/pjstadig/humane-test-output/pull/23

A macro rewriting

(is (= ::s/invalid ..))

to

(let [a ::s/invalid] ...)

resulted in some very strange errors.

Comment by Alexander Kiel [ 13/Sep/17 6:34 AM ]

The macro issues can be solved by just not using ::s/invalid in code directly. I think in general, it better to use the predicate s/invalid?.

Instead of writing:

(= ::s/invalid ...)

one should use

(s/invalid? ...)

But I have no idea to solve the issue where you have ::s/invalid in data which is validated. The function spec for identical? is a good example.

(s/fdef clojure.core/identical?
  :args (s/cat :x any? :y any?)
  :ret boolean?)

world not work.





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

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

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


 Description   

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

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

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

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

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

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

The same also happens if all arguments are lists:



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

More odd side effects.

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

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

See comments on CLJ-1953





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

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

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

Not Relevant



 Description   

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

If all inputs are sets, it works.

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

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

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

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

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


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

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

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

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

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

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

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

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

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

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

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

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





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

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

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

All



 Description   

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






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

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

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


 Description   

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



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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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





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

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

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

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



 Description   

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

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

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

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

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



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

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

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

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

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

Both of these return false on both JDKs:

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

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

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

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

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

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

IMHO the only sane approach would be to:

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

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

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

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

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

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





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

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

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


 Description   

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

Here is the naive approach:

(shutdown-agents)

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

Here is the proof:

(def my-agent (agent 1))

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

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

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

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

Here is the output:

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

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

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

A similar naive approach works perfectly well in Java:

ExecutorService executor = Executors.newSingleThreadExecutor();

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

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

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

Output:

Shutdown requested
Action 1 complete
Action 2 complete

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

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

(await my-agent)
(shutdown-agents)

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

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

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






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

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

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


 Description   

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

Repro:

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

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

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



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

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

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





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

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

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


 Description   

Two issues regarding the documentation of core.reducers

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





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

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

Type: Feature Priority: Minor
Reporter: Kevin Downey Assignee: Ghadi Shayban
Resolution: Unresolved Votes: 5
Labels: None

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

 Description   

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

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

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

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

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

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

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

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

(import '(java.util UUID))

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

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

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

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

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

and the result would be true.

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

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


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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

Now I'm partial to the name successions.

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

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

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

(def numbers (doall (range 1000)))

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

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

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

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

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

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

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

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

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

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

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

updated patch to apply cleanly to core

Comment by Ghadi Shayban [ 18/Sep/16 11:40 PM ]

I'm not sure I'm sold on this anymore, and have suggested a different approach on the mailing list https://groups.google.com/d/msg/clojure-dev/89RNvkLdYc4/PAJh8gfmDAAJ

Comment by Ghadi Shayban [ 17/Apr/17 1:40 PM ]

I have been marinating upon two generator functions that cover use-cases including the one listed above.

One of them is similar to Scheme's unfold but with some deviations more appropriate to Clojure. The other function takes a side-effecting producer and a sentinel value.

Ignore the naming and examine the semantics. https://gist.github.com/ghadishayban/902373e247e920855139902912d237f0





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

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

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


 Description   

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

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

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

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

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

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

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

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


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

run.sh in the linked github repo throws:

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

and fails to compile the required java source

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

Comment by Arnout Roemers [ 14/Jul/17 6:47 AM ]

I'm not sure the issue is (only) due to having an AOT compiled library. I started rewriting parts of the slf4j-timbre library, in order to remove the need for AOT [1]. But I noticed the problem in that version remains when the TimbreLoggerAdapter loads the slf4j-timbre.adapter namespace, instead of a Clojure namespace, during compilation. So as soon as a SLF4J log statement is performed during compilation, the `_thunk_` issue still ensued when running the result.

A workaround however is to require the slf4j-timbre.adapter namespace as one of the first in the AOT compiled namespaces. This also works with the current AOT compiled version of slf4j-timbre library. So maybe solving this issue has more to do with transitive loading of namespaces through Java classes during AOT compilation, instead of with AOT compiled libraries per se?

[1] https://github.com/hellodata-org/slf4j-timbre/blob/f9a2f4469fd92063c261276479593de39fffbee3/src-java/com/github/fzakaria/slf4j/TimbreLoggerAdapter.java





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

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

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

Attachments: Text File rand_with_two_params1.patch    

 Description   

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

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

(+ a (rand (- b a))

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

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






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

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

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


 Description   

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






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

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

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

Crappy Linux VM running RHEL6

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



 Description   

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

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

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

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

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

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

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

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



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

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





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

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

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

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

 Description   

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

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

(ns unmap-test.core)

(def a :test-1)

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

(def a :test-2)

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

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

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

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

(ns unmap-test.core)

(def a :test-1)

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

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

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

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

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

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

Relevant links:

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



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

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

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

was this fixed by clj-1604?

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

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





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

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

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

Attachments: Text File CLJ-1871.patch    

 Description   

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

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

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


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

patch





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

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

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

Attachments: Text File clj-1866.patch    

 Description   

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

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



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

Example code where this is an issue?

Benchmark before/after ?

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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





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

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

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


 Description   

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

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

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

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

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

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

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

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

Redefining the same function twice makes it work.

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

(defn a [x]
  (a x))

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

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

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

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

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


 Comments   
Comment by Nicola Mometto [ 06/Jan/17 10:12 AM ]

I just took a quick look at this, not an easy one to fix as handling recursive calls would likely require 2 passes over the whole defn AST, one to determine whether the defn is direct-linkable and the other one to build the AST using StaticInvokeExpr rather than InvokeExpr (using a stub Method using the analysis info rather than reflecting)

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

Yeah, Rich is aware of this and it's not done yet due to the issues you mentioned. It's hard!





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

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

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


 Description   

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






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

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

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


 Description   

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

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

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

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

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

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






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

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

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

Attachments: File CLJ_1857_split_docs_update.diff    

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

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

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


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

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

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

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

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

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

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





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

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

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

tested on CentOS 6



 Description   

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

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

(def myf (nestfn 100 "body"))

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



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

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

Comment by Philipp Neumann [ 16/Oct/17 10:53 AM ]

I tried clojure.core.match with 13 patterns and the compiliation failed under Windows. I assume this problem is the root cause of it.





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

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

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

-



 Description   

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

Thoughts?






[CLJ-1821] Move map-invert from clojure.set to clojure.core Created: 28/Sep/15  Updated: 28/Sep/15

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

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


 Description   

map-invert is hard to find when it lives in the clojure.set namespace, because it acts on maps and not sets. To my eyes set/map-invert also looks strange when reading code, but this is the preferred way to bring in the clojure.set namespace.

This is one of the minor warts I'd like to see fixed in clojure 2.0.






[CLJ-1820] Move rename-keys from clojure.set to clojure.core Created: 28/Sep/15  Updated: 12/Jul/17

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

Type: Enhancement Priority: Trivial
Reporter: Lars Andersen Assignee: Unassigned
Resolution: Unresolved Votes: 5
Labels: None


 Description   

rename-keys is hard to find when it lives in the clojure.set namespace, because it acts on maps and not sets. To my eyes set/rename-keys also looks strange when reading code, but this is the preferred way to bring in the clojure.set namespace.

This is one of the minor warts I'd like to see fixed in clojure 2.0.



 Comments   
Comment by Gordon Syme [ 04/Jul/17 4:56 AM ]

It's not just rename-keys, there are a few fns in clojure.set that don't make sense being there, at first glance anyway.

It certainly harms discoverability of these fns.

I've come across at least one re-implementation of rename-keys and map-invert during my day job because the author didn't know these fns exist.

I'd argue for breaking the relational and map fns out of clojure.set into their own namespaces and deffing some vars in clojure.set for backwards compatibility.
Those compatibility vars could be deleted in 1.10.

I'm happy to do this (or another approach) but would like some buy in on the approach from the core team first.

Comment by Nicola Mometto [ 04/Jul/17 6:25 AM ]

I think this is very, very unlikely to ever happen

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

We will not delete/move existing vars as this would break existing programs.

Comment by Marc O'Morain [ 12/Jul/17 4:49 AM ]

> We will not delete/move existing vars as this would break existing programs.

This issue can be addressed without deletion as per Gordon's suggestion:

A new var could be added to clojure.core named clojure.core/rename-keys. Then the rename-keys var in clojure.set can be defined as:

(def rename-keys clojure.core/rename-keys)
Comment by Alex Miller [ 12/Jul/17 6:08 AM ]

I am aware of that, which is why I did not close the issue. I was just stating one possible resolution that is off the table.





[CLJ-1813] Improve use-fixtures docstring Created: 10/Sep/15  Updated: 13/Oct/15

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

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

Attachments: Text File fixture-docstring.patch    

 Description   

The docstring for use-fixtures says

Wrap test runs in a fixture function to perform setup and teardown. 
Using a fixture-type of :each wraps every test individually, 
while: once wraps the whole run in a single function

I think it would be helpful to explain what a fixture function is and how it performs setup and teardown. I know because I've looked at examples, but I don't think the docstring explains this at all.

Is this something Core is interested in taking a patch on?



 Comments   
Comment by Alex Miller [ 10/Sep/15 9:08 PM ]

It's explained in the clojure.test ns docstring in more depth:

Fixtures are attached to namespaces in one of two ways.  \"each\"
   fixtures are run repeatedly, once for each test function created
   with \"deftest\" or \"with-test\".  \"each\" fixtures are useful for
   establishing a consistent before/after state for each test, like
   clearing out database tables.

   \"each\" fixtures can be attached to the current namespace like this:
   (use-fixtures :each fixture1 fixture2 ...)
   The fixture1, fixture2 are just functions like the example above.
   They can also be anonymous functions, like this:
   (use-fixtures :each (fn [f] setup... (f) cleanup...))

   The other kind of fixture, a \"once\" fixture, is only run once,
   around ALL the tests in the namespace.  \"once\" fixtures are useful
   for tasks that only need to be performed once, like establishing
   database connections, or for time-consuming tasks.

   Attach \"once\" fixtures to the current namespace like this:
   (use-fixtures :once fixture1 fixture2 ...)

I'm not really answering your question, just wondering if you saw that and whether it changes your question.

Comment by Daniel Compton [ 10/Sep/15 9:15 PM ]

Perhaps just pointing the user towards the ns docstring would be a good alternative? I had forgotten about the docstring on the ns, and I'm not sure whether duplicating the docs about fixtures in use-fixtures is a great idea.

Perhaps something like this?

Wrap test runs in a fixture function to perform setup and teardown. 
Using a fixture-type of :each wraps every test individually, 
while :once wraps the whole run in a single function

See the clojure.test docstring for more details.




[CLJ-1806] group-by as reducer / reduction fn Created: 29/Aug/15  Updated: 15/May/17

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

Type: Feature Priority: Minor
Reporter: Karsten Schmidt Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: reducers, transducers

Attachments: Text File clj-1806-1.patch    

 Description   

Whilst working on a query engine heavily based on transducers, I noticed that it'd be great to have group-by able to be used as reduction fn. The attached patch adds a single arity version to group-by which enables this use case and also includes a few tests.



 Comments   
Comment by Karsten Schmidt [ 29/Aug/15 8:08 PM ]

patch w/ described changes





[CLJ-1799] Replace refs in pprint Created: 13/Aug/15  Updated: 28/Sep/17

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

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

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

 Description   

I noticed that pprint uses refs and dosync transactions in a number of places, which seems unlikely to be necessary. It seems like these could be replaced by atoms, or even volatiles, given that printing typically happens in a single thread. Presumably this would improve performance of pprint significantly.



 Comments   
Comment by dennis zhuang [ 14/Aug/15 11:28 AM ]

I develop a patch to fix this issue.I run all the tests in clojure and clojure.data.json, and no one fails.

Use criterium to do a simple benchmark as below:

(use 'criterium.core)
(require '[clojure.data.json :as json])
(bench (json/write-str 
  {:a 1 :b 2 :c (range 10) :d "hello world"
   :e (apply hash-set (range 10))}))

before patch:

Evaluation count : 6180060 in 60 samples of 103001 calls.
             Execution time mean : 10.302604 µs
    Execution time std-deviation : 597.958933 ns
   Execution time lower quantile : 9.631444 µs ( 2.5%)
   Execution time upper quantile : 11.618551 µs (97.5%)
                   Overhead used : 1.724553 ns

After patch:

Evaluation count : 6000900 in 60 samples of 100015 calls.
             Execution time mean : 10.212543 µs
    Execution time std-deviation : 564.874941 ns
   Execution time lower quantile : 9.528383 µs ( 2.5%)
   Execution time upper quantile : 11.334033 µs (97.5%)
                   Overhead used : 1.827143 ns




[CLJ-1792] array-map and hash-map differ in handling of keys comparing identical but not equal Created: 04/Aug/15  Updated: 04/Aug/15

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

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

Attachments: Text File 0001-do-pointer-check-in-Util.EquivPred.patch    
Patch: Code

 Description   
user=> (let [a (array-map Double/NaN 1)] (assoc a (key (first a)) "foo"))
{NaN 1, NaN "foo"}
user=> (let [a (hash-map Double/NaN 1)] (assoc a (key (first a)) "foo"))
{NaN "foo"}

Approach: Array-map's comparison skips identity-check and always delegates to .equals calls. The attached patch alignes array-map behaviour with hash-map by doing the appropriate pointer checks before delegating to .equals






[CLJ-1791] Issue defining a defrecord protocol method named "clear" Created: 04/Aug/15  Updated: 01/Apr/16

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

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


 Description   

There seems to be a problem in trying to define a protocol with a method named "clear"

(defprotocol PClear
(clear [o]))
=> PClear

(defrecord Foo []
PClear
(clear [o] o))
=> CompilerException java.lang.ClassFormatError: Duplicate method name&signature in class file xxxx/Foo, compiling:(NO_SOURCE_PATH:1:1)

I assume this is due to a name conflict with the Java method Collection.clear() in the underlying implementation. However the error is very unclear about this, and the potential for conflict appears to be undocumented as far as I can see.

There seem to be two possible approaches to fixing this:
a) Disallow the use of "clear" as a protocol method name (in which case the error should be more informative, and the rule should be documented)
b) Find a way to support this in the class file format (possibly by overloading on JVM return types, since Collection.clear() returns void??)



 Comments   
Comment by Nicola Mometto [ 04/Aug/15 6:58 AM ]

Mike, the jvm doesn't support return type overloading so your second suggestion is not technically possible.

Reading the doc for defrecord

The class will have implementations of several (clojure.lang)
interfaces generated automatically: IObj (metadata support) and
IPersistentMap, and all of their superinterfaces.

Perharps java.util.Collection (or even better, java.util.Map) should be mentioned here.

Comment by Alex Miller [ 04/Aug/15 7:46 AM ]

I think this should be a doc enhancement request.

Comment by Shogo Ohta [ 08/Aug/15 3:42 AM ]

It might be out of the scope of this ticket, but protocol method conflicts can cause some other kinds of errors:

user=> (defprotocol P1 (finalize [this]))
P1
user=> (defrecord R1 [] P1 (finalize [this]))

CompilerException java.lang.VerifyError: (class: user/R1, method: finalize signature: ()Ljava/lang/Object;) Unable to pop operand off an empty stack, compiling: ...
user=> (defprotocol P2 (wait [this]))
P2
user=> (defrecord R2 [] P2 (wait [this]))
user.R2
user=> (def r (->R2))
#'user/r
user=> (wait r)
CompilerException java.lang.IllegalArgumentException: No single method: wait of interface: user.P2 found for function: wait of protocol: P2, compiling: ...
user=>

IMHO it would be nicer if defprotocol would warn method conflicts with a more informative message.

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

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

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

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

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

Ah, yes of course, thanks.





[CLJ-1789] Use transients with select-keys if possible Created: 28/Jul/15  Updated: 04/Oct/17

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

Attachments: Text File 0001-CLJ-1789-Use-transients-and-reduce-with-select-keys.patch    
Patch: Code

 Description   

Problem: Currently select-keys uses conj to add entries. If the map is editable, conj! could be used instead to improve select-keys performance.

Additionally keyseq is traversed as a seq but could be traversed via reduce instead, which might be faster.

Approach 1: Use a transient map and conj!, keeping loop/recur
Approach 2: Reimplement select-keys to use reduce instead of loop/recur
Approach 3: Combine approach one and two

selected key size loop/recur transient reduce transient + reduce
1 243 ns 256 ns 161 ns 188 ns
7 1.1 ms
885 ns 454 ns

From these numbers, approach 3 was chosen.

Note: In order to implement select-keys in terms of reduce, select-keys needed to be moved until after the definition of reduce. This forced a (declare select-keys) since it's used before the definition of reduce

Patch: 0001-CLJ-1789-Use-transients-and-reduce-with-select-keys.patch



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

Standard Clojure select-keys:

(bench (clojure.core/select-keys {:a "b" :c "d"} [:a]))
Evaluation count : 246382440 in 60 samples of 4106374 calls.
             Execution time mean : 243.245536 ns
    Execution time std-deviation : 2.714803 ns
   Execution time lower quantile : 238.473675 ns ( 2.5%)
   Execution time upper quantile : 248.544255 ns (97.5%)
                   Overhead used : 1.845047 ns

With transients:

(bench (select-keys {:a "b" :c "d"} [:a]))
Evaluation count : 232727220 in 60 samples of 3878787 calls.
             Execution time mean : 256.937568 ns
    Execution time std-deviation : 10.025123 ns
   Execution time lower quantile : 249.951872 ns ( 2.5%)
   Execution time upper quantile : 276.251590 ns (97.5%)
                   Overhead used : 1.845047 ns

Found 5 outliers in 60 samples (8.3333 %)
	low-severe	 3 (5.0000 %)
	low-mild	 2 (3.3333 %)
 Variance from outliers : 25.4503 % Variance is moderately inflated by outliers

With reduce

(bench (select-keys {:a "b" :c "d"} [:a]))
Evaluation count : 364807860 in 60 samples of 6080131 calls.
             Execution time mean : 161.582833 ns
    Execution time std-deviation : 2.212659 ns
   Execution time lower quantile : 158.027524 ns ( 2.5%)
   Execution time upper quantile : 167.673682 ns (97.5%)
                   Overhead used : 1.845047 ns

Found 3 outliers in 60 samples (5.0000 %)
	low-severe	 3 (5.0000 %)
 Variance from outliers : 1.6389 % Variance is slightly inflated by outliers

Reduce + transient

(bench (select-keys {:a "b" :c "d"} [:a]))
Evaluation count : 318075720 in 60 samples of 5301262 calls.
             Execution time mean : 188.656164 ns
    Execution time std-deviation : 3.024952 ns
   Execution time lower quantile : 183.867285 ns ( 2.5%)
   Execution time upper quantile : 195.466784 ns (97.5%)
                   Overhead used : 1.845047 ns

Found 4 outliers in 60 samples (6.6667 %)
	low-severe	 4 (6.6667 %)
 Variance from outliers : 1.6389 % Variance is slightly inflated by outliers

On bigger map/selection

(bench (clojure.core/select-keys {:a "b" :c "d" :b "b" :d "d" :e "e" :f "f" :g "g"} [:a :c :b :d :e :f :g]))
Evaluation count : 56147160 in 60 samples of 935786 calls.
             Execution time mean : 1.104653 µs
    Execution time std-deviation : 36.366516 ns
   Execution time lower quantile : 1.048257 µs ( 2.5%)
   Execution time upper quantile : 1.142031 µs (97.5%)
                   Overhead used : 1.845047 ns

Found 5 outliers in 60 samples (8.3333 %)
	low-severe	 4 (6.6667 %)
	low-mild	 1 (1.6667 %)
 Variance from outliers : 19.0389 % Variance is moderately inflated by outliers

reduce

(bench (select-keys {:a "b" :c "d" :b "b" :d "d" :e "e" :f "f" :g "g"} [:a :c :b :d :e :f :g]))
Evaluation count : 67723500 in 60 samples of 1128725 calls.
             Execution time mean : 885.840664 ns
    Execution time std-deviation : 11.503115 ns
   Execution time lower quantile : 864.403495 ns ( 2.5%)
   Execution time upper quantile : 905.721942 ns (97.5%)
                   Overhead used : 1.845047 ns

Transient + reduce

(bench (select-keys {:a "b" :c "d" :b "b" :d "d" :e "e" :f "f" :g "g"} [:a :c :b :d :e :f :g]))
Evaluation count : 134119380 in 60 samples of 2235323 calls.
             Execution time mean : 454.587795 ns
    Execution time std-deviation : 15.681611 ns
   Execution time lower quantile : 439.822498 ns ( 2.5%)
   Execution time upper quantile : 485.797378 ns (97.5%)
                   Overhead used : 1.845047 ns

Found 3 outliers in 60 samples (5.0000 %)
	low-severe	 3 (5.0000 %)
 Variance from outliers : 20.6393 % Variance is moderately inflated by outliers

The attached patch is using both transients and reduce

Comment by Alex Miller [ 04/Oct/17 8:54 AM ]

The proposed approach seems good to me. The description needs to reflect what was considered and chosen better.





[CLJ-1784] Reflector.getMethods should be cached Created: 21/Jul/15  Updated: 11/Dec/15

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

Type: Enhancement Priority: Major
Reporter: Vladimir Sitnikov Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File clj-1784.patch    

 Description   

Currently Reflector.getMethods performs expensive logic that includes java.lang.reflect.Method copying.
See: https://github.com/clojure/clojure/blob/b8607d5870202034679cda50ec390426827ff692/src/jvm/clojure/lang/Reflector.java#L373

In our application I see the following back-traces:

at Reflector.copyMethods
at Reflector.invokeInstanceMethod
at ...

These kind of backtraces are second top consumers of all the heap allocation.

JDK cannot cache Methods / Fields since they are mutable (e.g. user can call setAccessible here and there).
However, for the purposes of Clojure, I believe it should be fine to cache Methods and Fields.

What do you think?
E.g. WeakHashMap<Class, WeakReference<List<Method>>> or more sophisticated structure to account String name.



 Comments   
Comment by Alex Miller [ 23/Jul/15 8:19 AM ]

If you are seeing Reflector as a hot spot in your application, you should probably turn on warn-on-reflection and use type hints to avoid reflection.

Comment by Vladimir Sitnikov [ 28/Jul/15 6:10 AM ]

Do you mean there is absolutely no reason to use reflection in Clojure ever?
I do understand that if developer gives enough type hints the reflection would go away.

However:
1) I just do not know if it is easily doable (in other words, if it is possible at all, maintainable, etc)
2) I'm not sure if "always use type hints" is considered a best practice. For instance, warn-on-reflection documentation page says nothing like "always use type hints"
3) Caching copyMethods seems to be a low-hanging fruit here, so it would shave cpu cycles for those who omitted type hints

PS. I'm a java performance engineer, not a Clojure engineer (as in "my Clojure knowledge is somewhere near (+ x y)"), so I kindly beg on your forgiveness for me not doing RTFM.

Comment by Alex Miller [ 28/Jul/15 7:44 AM ]

No, I'm saying that if reflection is a hotspot in your application, usually it's worth investing a few minutes to add type hints in those hotspot areas and this is common advice for Clojure apps. Once that minimal work is done, few Clojure apps are bound by reflection.

Caching seems like an easy solution until you consider all of the management aspects. How does the cache get cleaned? Are the instances mutable and able to be reused? Are there cases where class loaders or code reloading create unexpected side effects? What are the concurrency effects of putting a shared resource in the invocation path? What is the memory impact of a cache and is it configurable?

Those are all things that would need to be investigated, meaning that this is not low-hanging fruit.

Comment by Mike Anderson [ 08/Dec/15 8:39 PM ]

Patch for simple caching of Reflector.getMethods calls for small arities

Comment by Mike Anderson [ 08/Dec/15 8:46 PM ]

I created a small patch to add very simple (fixed size, 1 element for each arity) caching for Reflector.getMethods calls. The aim is to keep this super simple to avoid issues like concurrency effects and having a variable-sized cache.

This helps a small amount in my tests (about 15-20%) on reflection calling the same method in a loop, which is probably the common case where people actually care about reflection performance.

Performance could certainly be improved further due to the fact that I think most of the overhead is actually is the `invokeMatchingMethod`, but that is an orthogonal issue. This patch opens the way for further performance optimisation in that area.

;; clojure 1.8.0-RC3
user=> (let [v (identity 1)] (time (dotimes [i 1000] (.doubleValue v))))
"Elapsed time: 1.598779 msecs"

;; with cached arities
user=> (let [v (identity 1)] (time (dotimes [i 1000] (.doubleValue v))))
"Elapsed time: 1.359888 msecs"

Comment by Vladimir Sitnikov [ 09/Dec/15 2:24 AM ]

Mike Anderson, I wonder if your patch results in a performance regression for concurrent workload.

You've created a single point of contention as lots of threads will try to update private static InstanceMethodCache[] instanceMethodCache entries, so it will hit both "true sharing" and "false sharing" problems.

Should instanceMethodCache be final and written in capital letters?

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

This ticket needs a better problem definition. That is: "I am doing ____" (with an example) that shows Reflector.getMethods() as the bottleneck.

If I guess at what the problem is, I remain unconvinced that this is the best solution.

A ThreadLocal is likely the cache solution with the lowest concurrency impact.

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

This shouldn't have any noticeable concurrency impact: no locking is required for this very simple approach. Most of the time it is simply an unlocked read from an array on the heap, the Java memory model is enough to guarantee correct behaviour. That's cheaper than even a threadlocal, e.g. there's some evidence here that this is 10-20x faster: http://stackoverflow.com/questions/609826/performance-of-threadlocal-variable

At the very least, any concurrency impact is so tiny it will be dwarfed by the benefits of often avoiding the getMethods calls, which are expensive. The cost of array access is a few nanoseconds compared to the cost of getMethods which appears from the benchmark above to be a few hundred nanoseconds.

The worst concurrency case I can think of is the case where two different threads are calling getMethods on different methods at a high rate and these calls are perfectly interleaved so that they always invalidate the cache. But even in that case, it's probably not measurably worse than the current code.

@Vladimir yes, insntanceMethodCache could be final. Might help the JVM very marginally, I guess.

@Alex, I proposed this patch because it is an improvement over what is currently there, I certainly don't think it will be the "best possible solution". In the spirit of open source and making incremental progress, I'd like you to consider accepting it, even if this issue stays open for future consideration. This is also linked to clj-1866, I'm trying to make the "fast path" for reflection better in a few different ways. If you'd rather have a single large patch with a whole bunch of improvements I can certainly do that, I has under the impression that smaller, more "obvious" patches would be easier for you to review but happy either way.

Comment by Vladimir Sitnikov [ 10/Dec/15 1:18 AM ]

Mike Anderson, you are missing the point.

Please check here: http://shipilev.net/blog/2014/jmm-pragmatics/#_benchmarks, slide 77/100 "SC-DRF: Writes"

Alexey Shipilev: This reinforces the idea that data sharing is what you should avoid in the first place, not volatiles

Having ThreadLocal cache would eliminate "shared update" problem.

This ticket needs a better problem definition. That is: "I am doing ____" (with an example) that shows Reflector.getMethods() as the bottleneck

That is true.
My particular case was somehow resolved by development team.
I just thought some basic cache would make Clojure do the right thing by default and require less type specialization written manually.

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

I see a lot of "should" type statements in there. The whole point is that no change like this is going to go in until we know that there aren't impacts. But more importantly I'm not even going to mark it triaged until it's a good ticket that starts with a problem statement.

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

Alex, what do you mean by "know there aren't impacts"? That seems an absurd position on the face of it, it is a perfectly acceptable trade-off to allow minor regressions in rare corner cases if you are improving the fast path / common case significantly.

Also, this definitely isn't a standard that is universally applied for changes to Clojure. Plenty of changes go into Clojure which cause performance regressions in other areas, you only need to look at Andy Fingerhut's excellent benchmarking efforts here to see that: https://jafingerhut.github.io/clojure-benchmarks-results/Clojure-expression-benchmark-graphs.html

Problem statement is IMHO obvious for anything performance related: "Performance of X is sub-optimal, which hurts users who are doing X." If you want a new ticket / changed description that says something like that then I'm happy to do it, but that seriously just feels like bureaucratic box ticking. Please consider this as constructive feedback on your contribution process.

What exactly (i.e. which benchmarks) do you need to see as a valid demonstration of improvement in performance-related issues like this?

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

Similar to my comments in CLJ-1866, the title of this ticket is "Reflector.getMethods should be cached". That is again a solution, not a problem. What I'm looking for is a title like "Repeated reflection in a loop is slow" and a description that starts with some example code demonstrating the problem. Without a good problem statement, I cannot triage this ticket. I may still consider the priority of the problem to be low enough that it's not worth triaging at this time - I'll withhold judgement though until the ticket is improved.

The fact that prior changes have had unexpected performance impacts only lends additional credence to my suggestion that this (performance-oriented) ticket should validate its claims. You have added code, which makes the "miss" path of this code slower than it was before. How much slower? It should make the "hit" path faster - how much faster? In typical code, how often do we encounter hit vs miss paths? My presumption is that the example will demonstrate a case where the hit path is common. These are the kinds of questions I, as a screener, must ask to evaluate any proposed solution.

Additionally, you are introducing concurrency concerns and some additional work is required to verify both correctness (the current patch has visibility issues) and that you have not introduced contention or memory issues. These are typical problems for any caching-related optimization and I could point you to any number of prior tickets that have wrestled with them.

Comment by Mike Anderson [ 11/Dec/15 9:13 PM ]

Thanks Alex for explaining your concerns.

I agree that a problem-oriented approach to patches is better, so I suggest the following:

  • We close both this issue and clj-1866
  • I create a separate problem-focused ticket for reflection performance
  • I'll benchmark the changes as whole for a number of different cases
  • You'll triage the patch on the assumption that we can demonstrate noticeable improvement in the common cases, all tests pass as before, and no major regressions occur in the corner cases (concurrent access, frequent cache misses etc.)

If you want a problem-oriented issue then I don't think it makes much sense to create separate tickets / patches for each "solution" (although some OSS projects choose to do it that way, they usually have have a much more streamlined process for minor changes / optimisations which probably doesn't suit the Clojure dev process)

Agreed?

Comment by Alex Miller [ 11/Dec/15 9:45 PM ]

As I said, we would prefer small focused tickets and patches, rather than one big patch.

I will reiterate that I'm not convinced doing any of this work makes sense if the scenario is one where a type hint would solve the problem.





[CLJ-1782] Hide local IDE files in .gitignore Created: 21/Jul/15  Updated: 21/Jul/15

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

Type: Enhancement Priority: Trivial
Reporter: Mike Anderson Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None

Attachments: Text File clj-1782.patch    

 Description   

Several IDEs (e.g. Eclipse/CCW, IntelliJ IDEA/Cursive) create local files in project workspaces which should normally be ignored for the purposes of source control.

This patch proposes to add some common files that should be ignored to the .gitignore






[CLJ-1779] Optimise compiler usage of getMethod calls Created: 17/Jul/15  Updated: 17/Jul/15

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

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


 Description   

Currently the Clojure compiler makes multiple redundant calls to Method.getMethod(...) while emitting code, e.g.

gen.invokeStatic(Type.getType(Long.class), Method.getMethod("Long valueOf(long)"));

It seems to be the case that that:

a) These getMethod calls are effectively returning equivalent, immutable constant values
b) getMethod is moderately expensive (performs string analysis and quite a few object allocations)
c) These calls are very common during compilation of typical Clojure code

The proposed enhancement is to replace all of these getMethod calls with constant static values. This should improve compilation performance noticeably with no effect on behaviour.






[CLJ-1777] Add function version of vswap! Created: 13/Jul/15  Updated: 15/May/17

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

Type: Feature Priority: Major
Reporter: Nicola Mometto Assignee: Unassigned
Resolution: Unresolved Votes: 4
Labels: None

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

 Description   

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

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

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



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

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

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

Sorry, I mixed the fix version with the affected field

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

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

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

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

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

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

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

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

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

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





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

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

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

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

 Description   

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



 Comments   
Comment by Daniel Compton [ 09/Feb/17 8:34 PM ]

Is there a compiler change required as well here?

Comment by Alex Miller [ 09/Feb/17 10:56 PM ]

I think he is saying that the compiler is fine but that if it weren't, no test would tell you that.

Comment by Michael Blume [ 09/Oct/17 6:30 PM ]

Yes, that.

Comment by Andy Fingerhut [ 09/Oct/17 10:21 PM ]

Is it an accident that the Clojure compiler accepts 'code' like this? If I saw Clojure code like this in a project, I think I would assume it was an error.

Comment by Alex Miller [ 09/Oct/17 10:24 PM ]

Seems like it should be valid code to me (although there is not much reason to ever do this unless it's the last expression in a body).

Comment by Andy Fingerhut [ 09/Oct/17 10:35 PM ]

[facepalm] Yes, of course this should be valid in return position, for returning such a vector or map. Away from return position is the odd-looking thing, but as you say, no reason the compiler should reject it.

Comment by Alex Miller [ 10/Oct/17 8:43 AM ]

Note that the elements of a collection will be evaluated, so side effects triggered by code in the collection will be executed.

user=> (let [] 
         [(future (println "hi")) (future (println "there"))] 
         1)
hi
there
1




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

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

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


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

(defrecord UUIDWrapper [^UUID uuid])

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

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

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

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

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



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

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

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

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

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

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

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

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

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

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

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





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

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

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


 Description   

The documentation for sort seems to be incomplete:

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

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



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

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

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

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

I was thinking that a neat fix here would be to just state that sort takes a comparator function, and let the user refer to the reference manual for details of comparator functions. I could not find anything in the reference manual, but the Clojure guide has a good description of comparators - https://clojure.org/guides/comparators. I'm thinking of writing a small entry for the reference manual to document comparators - how does that sound?

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

This issue should be rewritten to remove the first issue. For the second one, I think it would be ok to add a clause/sentence to cover that a comparator can be a 2-arg function that returns either a boolean or a number.

If you want to update the web site, you should do that as an issue or PR on https://github.com/clojure/clojure-site.





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

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

Type: Feature Priority: Major
Reporter: Leon Grapenthin Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None


 Description   

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

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



 Comments   
Comment by Leon Grapenthin [ 16/May/17 1:36 PM ]

After two more years with Clojure I don't see this as improvement anymore. (into [] ...) provides more expressiveness. Unless other parties are interest, I'd recommend to close this.

We still lack functionality to use the variadic map transducer with sets, but I have never needed it, so I wouldn't open a ticket for that myself.

Comment by Alex Miller [ 16/May/17 4:03 PM ]

I'm still interested.





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

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

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


 Description   

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

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






[CLJ-1747] eduction's printer requires/promises that its source collection is Iterable Created: 07/Jun/15  Updated: 12/Apr/17

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

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

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

 Description   

eduction expects its source collection to be Iterable [1], and its print-method goes through print-sequential [2]. This implies a promise that may restrict the use-case of an eduction over a virtual collection, e.g. a pure IReduceInit source that may be backed by I/O or some other resource. I have found it useful to construct these I/O reducibles and wrap them with an eduction. But when interacting at the REPL, printing out an eduction-wrapped IReduceInit will fail. Does the print-method impl for eduction require too much? This is a only minor inconvenience more than anything else, obviously I could create my own flavor of eduction.

Totally hypothetical example:

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

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

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

Proposed Approach:
Since we have #object tagged literals, let eduction print like an opaque #object






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

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

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


 Description   

I find myself repeat code like this

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

and just think why not something like:

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



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

I agree this is very annoying.

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





[CLJ-1734] Display more descriptive error message when trying to use reader conditionals in a non-cljc file Created: 19/May/15  Updated: 06/Oct/16

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

Type: Enhancement Priority: Minor
Reporter: Daniel Compton Assignee: Unassigned
Resolution: Unresolved Votes: 3
Labels: errormsgs, reader


 Description   

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

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

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

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



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

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

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

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

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

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

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

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

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

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

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

Comment by Marshall Abrams [ 06/Oct/16 10:09 AM ]

I encountered the problem with using reader conditionals in a .clj file. I would have had no idea of how to fix the error meant except for Daniel Compton's blog post about it. I understand that it's not just a filename issue, and that there can be other sources of this error, but when it is a filename issue, it's very mysterious, and this seems like an easy mistake to make. A contributing context is that Clojurescript macros can be defined in a .clj file, so it can be natural to use that extension as a reminder about when the macros will be compiled.

I do think that something like "Reader conditionals not allowed ..." would be a lot easier to understand than "Conditional read not allowed". That would at least make it obvious that it's the reader conditional that's causing the problem. Once you know that this is what "conditional read" means, it's obvious, but "conditional read" is not a common phrase in the Clojure world, afaik, and I wondered why it didn't say "reader conditional" if that's what the message was about.

(An option would be to add a comment in the error string stating that the cause might be reader conditionals used outside of a .cljc file. Not sure if you want to go down that road of "might be"/"maybe" in error messages, since that kind of thing could get out of hand and lead to long error messages. However, this sort of thing has been done elsewhere, and it might be very helpful in this case; mistakenly using .clj with a reader conditional seems like something that would occur often.)





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

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

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


 Description   

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

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



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

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

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

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

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

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

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

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





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

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

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

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

 Description   

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






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

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

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

Attachments: Text File CLJ-1720_v01.patch     Text File CLJ-1720_v02.patch    

 Description   

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



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

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

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

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





[CLJ-1715] Use AFn.applyToHelper rather than IFn.applyTo in InvokeExpr.eval Created: 23/Apr/15  Updated: 23/Apr/15

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

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

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

 Description   

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

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

Example of currently failing code that will work with patch:

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





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

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

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


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

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






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

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

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


 Description   

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

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






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

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

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


 Description   

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

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

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

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

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

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



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

This may be a duplicate of CLJ-1613

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

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





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

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

Type: Defect Priority: Major
Reporter: Dr. Christian Betz Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

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

This is the actual test:

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

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

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

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

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

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

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



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

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

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

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

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

Hi,

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

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

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

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

Regards

Chris

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

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

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

public volatile MethodImplCache __methodImplCache;

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

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

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





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

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

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

Attachments: Text File clj-1690.patch    

 Description   

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

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



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

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

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

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

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

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

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

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

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

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

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

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

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

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

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





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

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

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


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

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






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

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

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


 Description   

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

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

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

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

unless I also type-hint x.



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

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





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

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

 Description   

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

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

After this patch, comparisons like

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

Patch: CLJ-1679-v3.patch



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

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

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

like so =)

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

Makes sense, thanks for the updated patch

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

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

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

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

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

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

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

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





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

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

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

-


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

 Description   

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

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






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

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

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

OSX, Clojure 1.6.0



 Description   

Saving the below snippet and running it like

java -jar clojure-1.6.0.jar snippet.clj

Produces

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

The snippet:

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

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

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

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


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

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

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

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

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

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

I understand why the fact that

(defn ^boolean foo [] true)

and

(defn foo ^boolean [] true)

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

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

Thanks for clarifying Nicola, you are indeed correct.

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

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

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

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

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





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

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

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

OpenJDK 1.7 on GNU/Linux



 Description   

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

Example:

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

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

Similar result if passing in another function; for example:

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

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


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

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





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

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

Type: Defect Priority: Minor
Reporter: Michael Blume Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: numerics, reflection


 Description   
(import 'java.io.DataOutputStream)
(import 'java.io.ByteArrayOutputStream)

(defn- ->bytes
  "Convert a Java primitive to its byte representation."
  [write v]
  (let [output-stream (ByteArrayOutputStream.)
        data-output (DataOutputStream. output-stream)]
    (write data-output v)
    (seq (.toByteArray output-stream))))

(defn int->bytes [n]
  (->bytes 
    #(.writeInt ^DataOutputStream %1 %2)
    n))

(defn int->bytes-ref [n]
  (->bytes 
    #(.writeInt %1 %2)
    n))

user=> (int->bytes 5)
(0 0 0 5)
user=> (int->bytes-ref 5)
(0 0 0 5)
user=> (int->bytes (inc Integer/MAX_VALUE))

IllegalArgumentException Value out of range for int: 2147483648  clojure.lang.RT.intCast (RT.java:1115)
user=> (int->bytes-ref (inc Integer/MAX_VALUE))
(-128 0 0 0)

So it looks like type-hinting the DataOutputStream results in bytecode calling RT.intCast, which throws because the value is too large. In the reflective case, we locate the method writeInt at runtime, and then do not call RT.intCast, but instead allow the long to be downcast without bounds checking.

It seems like we should be calling RT.intCast in both cases?






[CLJ-1655] Dorun's behavior when called with two argument's is both unintuitive and undocumented. Created: 04/Feb/15  Updated: 11/Jun/16

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Michael Blume Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Dorun can be called as (dorun n coll). When called this way, dorun will force n+1 elements from coll, which seems unintuitive. I can't necessarily call this a defect, though. It doesn't deviate from the documented behavior because there is no documented behavior – the two-argument arity is not mentioned in the docstring.

user=> (defn printing-range [n] (lazy-seq (println n) (cons n (printing-range (inc n)))))
#'user/printing-range
user=> (dorun 0 (printing-range 1))
1
nil
user=> (dorun 3 (printing-range 1))
1
2
3
4
nil


 Comments   
Comment by Stuart Halloway [ 11/Jun/16 5:36 PM ]

I do not think this is a bug, as it is caused by the pervasive semantics of seq, not just of dorun. Consider

(def x (seq (printing-range 0)))
0

i.e. just calling seq consumes the first item. I am leaving open as a feature request for improved docstring.





[CLJ-1651] Erroneous error message when using into to create a map. Created: 29/Jan/15  Updated: 29/Jan/15

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.7
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Justin Glenn Smith Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: error-reporting


 Description   

If you provide a sequence instead of a vector type for the entries provided to into for creating a hash-map, the error message is misleading.

org.noisesmith.orsos=> (into {} '((:a 0) (:b 1)))

ClassCastException clojure.lang.Keyword cannot be cast to java.util.Map$Entry clojure.lang.ATransientMap.conj (ATransientMap.java:44)

As we see, it reports the type of the first item in the entry, rather than the actual error, the type of the entry itself, which can be particularly confusing if the key in the entry is actually a valid type to be an entry:

=> (into {} '((["a" 1] ["b" 2]) (["c" 3] ["d" 4])))

ClassCastException clojure.lang.PersistentVector cannot be cast to java.util.Map$Entry clojure.lang.ATransientMap.conj (ATransientMap.java:44)






[CLJ-1628] Accept list as lib specification in clojure.core/require and clojure.core/use Created: 28/Dec/14  Updated: 30/Dec/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Petr Gladkikh Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File NS-macro-accept-lists-as-libspecs.patch    
Patch: Code

 Description   

Currently function clojure.core/load-libs treats '(a.namespace.name) as prefix list so this construct has no effect at all (as if it was prefix list without suffixes). At the same time '[a.namespace.name] causes require call (require 'a.namespace.name). In other cases functions require or use are ambivalent about differences between [] and (). In this particular case there is difference between no-op and lib loading. E.g. Clojure validation tool Eastwood includes rule for this case since the behavior of (require '(a.namespace.name)) is not obvious.

The suggested change lets to avoid this special case in require or use calls (including ones that stem from ns macro expansion). Accepting both list and vector as library specification makes behavior uniform and similar to the way suffix items are handled in prefix lists.

The patch is minimal in order to avoid reordering sequential? functions in clojure/core.clj
Should I include tests for these cases also?



 Comments   
Comment by Petr Gladkikh [ 30/Dec/14 2:39 AM ]

If on the other hand representing prefix lists as Clojure lists is intentional and list-for-prefix, vector-for-libspec-or-suffix should be distinguishing feature then we should issue error when

  • prefix list is enclosed in Clojure vector
  • libspec or suffix is in Clojure list

If backwards compatibility is important then one may at least write a warning in ':verbose' mode.

Also there should be error or warning if prefix list is empty.





[CLJ-1626] ns macro: compare ns name during macroexpansion. Created: 23/Dec/14  Updated: 02/Jul/15

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.6, Release 1.7
Fix Version/s: None

Type: Enhancement Priority: Trivial
Reporter: Petr Gladkikh Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None

Attachments: File compare-ns-name-at-macroexpansion.diff    

 Description   

Macroexpansion of 'ns' produces 'if' form that is executed at runtime. However comparison can be done during macroexpansion phase producing clearer resulting form in most cases.

Patch for suggested change is in attachment.



 Comments   
Comment by Andy Fingerhut [ 02/Jul/15 3:53 PM ]

Petr, I do not know whether this change is of interest to the Clojure core team or not. I do know that it is not in the expected format for a patch. See this link for instructions on creating a patch in the expected format: http://dev.clojure.org/display/community/Developing+Patches

Same comment applies to your patch for CLJ-1628





[CLJ-1625] Cannot implement protocol methods of the same name inline Created: 23/Dec/14  Updated: 14/Nov/16

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.6, Release 1.7
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Tassilo Horn Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: protocols


 Description   

One major benefit of protocols (IMHO) is that the protocol methods are properly namespace qualified. Thus I can have multiple protocols in different namespaces that define a foo method and extend them all (or a subset of them) upon existing types. However, that's not true with extending protocols inline with defrecord and deftype, or with extending protocols on the Java side by implementing their interfaces.

Example:

;; file: protocoltest/foo.clj
(ns prototest.foo)
(defprotocol Foo
  (mymethod [this]))

;; file: protocoltest/bar.clj
(ns prototest.bar)
(defprotocol Bar
  (mymethod [this]))

;; file: protocoltest/core.clj
(ns prototest.core
  (:require [prototest.foo :as foo]
            [prototest.bar :as bar]))

;; inline extension of both mymethod methods doesn't work
(defrecord MyRec [x]
  foo/Foo
  (mymethod [this] :foo)
  bar/Bar
  (mymethod [this] :bar))
;;=> java.lang.ClassFormatError
;;   Duplicate method name&signature in class file prototest/core/MyRec

;; I have to resort to either half-inline-half-dynamic...
(defrecord MyRec [x]
  foo/Foo
  (mymethod [this] :foo))
(extend-type MyRec
  bar/Bar
  (mymethod [this] :bar))

;; ... or fully dynamic extension.
(defrecord MyRec [x])
(extend-type MyRec
  foo/Foo
  (mymethod [this] :foo)
  bar/Bar
  (mymethod [this] :bar))

;; Then things work just fine.
(foo/mymethod (->MyRec 1))
;;=> :foo
(bar/mymethod (->MyRec 1))
;;=> :bar

I know that I get the error because both the Foo and the Bar interfaces backing the protocols have a mymethod method and thus they cannot be implemented both at once (at least not with different behavior).

But why throw away the namespacing advantages we have with protocols? E.g., why is the protocoltest.foo.Foo method not named protocoltest$foo$mymethod (or some other munged name) in the corresponding interface? That way, both methods can be implemented inline where you gain the speed advantage, and you can do the same even from the Java side. (Currently, invoking clojure.core.extend from the Java side using clojure.java.api is no fun because you have to construct maps, intern keywords, define functions, etc.)

Of course, the ship of changing the default method naming scheme has sailed long ago, but maybe a :ns-qualified-method-names option could be added to defprotocol.






[CLJ-1615] transient set "keys" and "values" wind up with different metadata Created: 12/Dec/14  Updated: 06/Oct/17

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.6
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Michael Blume Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: collections, meta, transient

Attachments: Text File 0001-CLJ-1615-ensure-transient-set-keys-and-values-have-c.patch     Text File 0001-demonstrate-CLJ-1615.patch     Text File CLJ-1615-entryAt.patch     Text File clj-1615-v3.patch     Text File clj-1615-v4.patch    
Patch: Code and Test

 Description   
(let [s (-> #{} 
          transient 
          (conj! (clojure.core/with-meta [-7] {:mynum 0}))
          (conj! (clojure.core/with-meta [-7] {:mynum -1})) 
          persistent!)]
  [(meta (s [-7])) (meta (first s))])
=> [{:mynum -1} {:mynum 0}]

basically it looks like the "key" (the value we get by seqing on the set) retains the metadata from the first conj! but the "value" (what we get by calling invoke with the "key") carries the metadata from the second conj!. This does not match the behavior if we don't use transients:

(let [s (-> #{} 
          (conj (clojure.core/with-meta [-7] {:mynum 0}))
          (conj (clojure.core/with-meta [-7] {:mynum -1})))]
  [(meta (s [-7])) (meta (first s))])
=> [{:mynum 0} {:mynum 0}]

(found playing with zach tellman's collection-check)



 Comments   
Comment by Michael Blume [ 12/Dec/14 5:07 PM ]

Attached patch demonstrating problem (not a fix)

Comment by Michael Blume [ 12/Dec/14 5:40 PM ]

More investigation:

The difference between "keys" and "vals" arises from the fact that clojure sets use maps under the covers.

The difference between persistent and transient seems to be because PersistentHashSet.cons short-circuits on contains (https://github.com/clojure/clojure/blob/clojure-1.6.0/src/jvm/clojure/lang/PersistentHashSet.java#L97) and ATransientSet.conj does not (https://github.com/clojure/clojure/blob/clojure-1.6.0/src/jvm/clojure/lang/ATransientSet.java#L27)

Adding a contains check to ATransientSet.conj makes the behavior consistent and passes the attached test, but I imagine this could cause a performance hit. Thoughts?

Comment by Michael Blume [ 12/Dec/14 5:43 PM ]

Attached proposed fix – note that this may cause a performance hit for transient sets.

Comment by Michael Blume [ 13/Dec/14 2:40 PM ]

Attaching an alternative fix – instead of doing a contains check on every transient conj, back set.get with entryAt. More invasive but possibly faster.





[CLJ-1612] clojure.core.reducers/mapcat can call f1 with undefined arity of 0 arguments? Created: 10/Dec/14  Updated: 10/Dec/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.6, Release 1.7
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Andy Fingerhut Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: reducers


 Description   

I have not run across this with running code, so perhaps it is impossible for reasons I have not understood. Also not sure whether fixing issues with reducers is of any importance, given transducers. This was found while testing the Eastwood lint tool on some Clojure namespaces, including clojure.core.reducers.

(defcurried mapcat
  "Applies f to every value in the reduction of coll, concatenating the result
  colls of (f val). Foldable."
  {:added "1.5"}
  [f coll]
  (folder coll
   (fn [f1]
     (let [f1 (fn
                ([ret v]
                  (let [x (f1 ret v)] (if (reduced? x) (reduced x) x)))
                ([ret k v]
                  (let [x (f1 ret k v)] (if (reduced? x) (reduced x) x))))]
       (rfn [f1 k]
            ([ret k v]
               (reduce f1 ret (f k v))))))))

The definition of macro rfn expands to a (fn ...) that can call f1 with no arguments, which is not a defined arity for f1.






[CLJ-1610] Unrolled small maps Created: 08/Dec/14  Updated: 15/May/17

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.6
Fix Version/s: None

Type: Feature Priority: Major
Reporter: Alex Miller Assignee: Zach Tellman
Resolution: Unresolved Votes: 10
Labels: collections


 Description   

Placeholder for unrolled small maps enhancement (companion for vectors at CLJ-1517).



 Comments   
Comment by Andy Fingerhut [ 09/Jul/15 10:59 PM ]

Is there an expectation that these would perform better that PersistentArrayMap?

Comment by Zach Tellman [ 09/Jul/15 11:53 PM ]

Yes, in some cases significantly so, for three reasons (in rough order of importance):

  • positional constructors, without any need for array instantiation/population
  • short-circuiting equality checks using hash comparisons
  • no iteration on any operation

There are a series of benchmarks at https://github.com/ztellman/cambrian-collections/blob/master/test/cambrian_collections/map_test.clj#L64-L148, which compare operations against maps with both keywords (which don't benefit from the hash comparisons) and symbols (which do). The 7-entry map cases cause the unrolled maps to overflow, so they only exist to test the overflow mechanism.

I've run the benchmark suite on my laptop, and the results are at https://gist.github.com/ztellman/961001e1a77e4f76ee1d. Some notable results:

The rest of the benchmarks are marginally faster due to unrolling, but most of the performance benefits are from the above behaviors. In a less synthetic benchmark, I found that Cheshire JSON decoding (which is 33% JSON lexing and 66% data structure building) was sped up roughly 30-40%.





[CLJ-1595] Nested doseqs leak with sequence of huge lazy-seqs Created: 20/Nov/14  Updated: 25/May/15

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Andrew Rudenko Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: File doseq_leaks.diff    
Patch: Code

 Description   

Hello!

This little snippet demonstrates the problem:

(doseq [outer-seq (list (range)) inner-seq outer-seq])

That's it. It is not just eats my processor, but also eats all available memory. Practically it can affect (and it is) at consuming of complex lazy structures like huge XML-documents.

I think this is at least non trivial behaviour.

It can be fixed by this small patch. We can get next element before current iteration, not after, so outer loop will not hold reference to the head of inner-seq.

This patch doesn't solve all problems, for example this code:

(doseq [outer-seq [(range)] inner-seq outer-seq])

leaks. Because chunked-seqs (vector in this case) holds current element by design.



 Comments   
Comment by Andy Fingerhut [ 25/May/15 3:15 PM ]

Andrew, sorry but I do not know whether this ticket is of interest to the Clojure core team.

I do know that patches are only considered for inclusion in Clojure if the submitter has signed the contributor agreement (CA). If you were interested in doing that, you can do it fairly quickly on-line here: http://clojure.org/contributing





[CLJ-1592] Ability to suppress warnings on name conflict with clojure.core Created: 14/Nov/14  Updated: 15/May/17

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.6, Release 1.7
Fix Version/s: None

Type: Feature Priority: Minor
Reporter: Mike Anderson Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

In numerical code, it is often useful and idiomatic to replace clojure.core functions with augmented versions (e.g. clojure.core.matrix.operators defines + in a way that works with whole arrays, not just scalar numbers)

Currently there seems to be no way to avoid a warning in client code when a library does this, e.g.:

;; library namespace
(ns foo
  (:refer-clojure :exclude [+]))
(def + clojure.core/+)

;; later on, in some other namespace
(require '[foo :refer :all])
=> WARNING: + already refers to: #'clojure.core/+ in namespace: bar, being replaced by: #'foo/+

A workaround exists by using (:refer-clojure :exclude ...) in the user namespace, however this creates unnecessary work for the user and requires maintenance of boilerplate code.

Proposed solution is to allow vars to be annotated with additional metadata (e.g. ^:replace-var ) that when added to library functions will suppress this warning. This will allow library authors to specify that a function should work as a drop-in replacement for clojure.core (or some other namespace), and that a warning is therefore not required.



 Comments   
Comment by Andy Fingerhut [ 14/Nov/14 9:46 PM ]

Duplicate with CLJ-1257 ?

Comment by Mike Anderson [ 14/Nov/14 9:53 PM ]

Hi Andy, it refers to the same warning - but the scope of the solution is different:

  • CLJ-1257 is more like a global way to turn off this warning
  • CLJ-1592 is for suppressing this warning on specific vars

If CLJ-1257 is implemented and the warning is off be default, CLJ-1592 becomes mostly unnecessary. Without CLJ-1257 or if the warning defaults to on, CLJ-1592 is needed.





[CLJ-1591] Symbol not being bound in namespace when name clashes with clojure.core Created: 14/Nov/14  Updated: 18/Aug/16

Status: Reopened
Project: Clojure
Component/s: None
Affects Version/s: Release 1.6, Release 1.7
Fix Version/s: None

Type: Defect Priority: Trivial
Reporter: Mike Anderson Assignee: Unassigned
Resolution: Unresolved Votes: 5
Labels: None


 Description   

The following code fails (both in 1.6 and latest 1.7-alpha4):

user=> (ns foo)
nil
foo=>  (def inc inc)
WARNING: inc already refers to: #'clojure.core/inc in namespace: foo, being replaced by: #'foo/inc
#'foo/inc

;; Note inc is unbound at this point, which causes the exception below
foo=> inc
#<Unbound Unbound: #'foo/inc>
foo=> (ns bar)
nil
bar=> (require ['foo :refer ['inc]])
WARNING: inc already refers to: #'clojure.core/inc in namespace: bar, being replaced by: #'foo/inc
nil
bar=> (inc 8)

IllegalStateException Attempting to call unbound fn: #'foo/inc  clojure.lang.Var$Unbound.throwArity (Var.java:43)

Further investigation shows that foo/inc is unbound:

foo/inc
=> #<Unbound Unbound: #'foo/inc>

Further investigation also shows that replacing the (def inc inc) with almost anything else, e.g. (def inc dec), (def inc clojure.core/inc), or (def inc (fn [n] (+ n 1))), causes no exception (but the warnings remain).

I would expect:
a) foo/inc should be bound and have the same value as clojure.core/inc
b) No error when requiring foo/inc
c) bar/inc should be bound to foo/inc



 Comments   
Comment by Nicola Mometto [ 14/Nov/14 10:04 PM ]

The second error should be expected, the right syntax should be (require ['foo :refer ['inc]]) (note the leading quote before inc)

Comment by Mike Anderson [ 14/Nov/14 10:20 PM ]

Thanks for the catch Nicola - I've edited the description. Still get the same error however (just with a slightly different message)

Comment by Alex Miller [ 14/Nov/14 10:22 PM ]

See comment...

Comment by Mike Anderson [ 14/Nov/14 10:24 PM ]

@Alex what comment? Note that the error still occurs even with the right syntax....

Comment by Mike Anderson [ 14/Nov/14 10:26 PM ]

Appears to have been closed prematurely

Comment by Alex Miller [ 14/Nov/14 10:39 PM ]

I can't reproduce with the correct syntax:

Clojure 1.7.0-master-SNAPSHOT
user=> (ns foo)
nil
foo=> (def inc inc)
WARNING: inc already refers to: #'clojure.core/inc in namespace: foo, being replaced by: #'foo/inc
#'foo/inc
foo=> (ns bar)
nil
bar=> (require ['foo :refer ['inc]])
WARNING: inc already refers to: #'clojure.core/inc in namespace: bar, being replaced by: #'foo/inc
nil
Comment by Mike Anderson [ 14/Nov/14 10:55 PM ]

The problem is that the var is still unbound and causes e.g. the following error:

=> (foo/inc 8)
IllegalStateException Attempting to call unbound fn: #'foo/inc clojure.lang.Var$Unbound.throwArity (Var.java:43)

I don't think that should be expected - or am I missing something?

Comment by Alex Miller [ 14/Nov/14 10:57 PM ]

Ah, will take a look. But not right now.

Comment by Andy Fingerhut [ 15/Nov/14 1:09 PM ]

Updated the description with a few more details. The exception goes away if you do (def inc (fn [n] (+ n 1))) instead of (def inc inc), for example. The warnings remain.

Comment by Tom Crayford [ 20/Nov/14 11:07 AM ]

Unsure if this is the same issue (I think it might be?), but I reproduced the exact same error message with AOT compilation involved:

reproduced in this git repository: https://github.com/yeller/compiler_update_not_referenced_bug

clone it, run `lein do clean, uberjar, test`, and that error message will show up every time for me

Comment by Andy Fingerhut [ 20/Nov/14 5:43 PM ]

Mike, I think replacing (def inc inc) in your example with (def inc clojure.core/inc) should be considered as a reasonable workaround for this issue, unless you have some use case where you need to def inc to something that is not in clojure.core (and if so, why?)

The reason (def inc inc) behaves this way is, if not absolutely necessary, at least commonly used in Clojure programs to define recursive functions, e.g. (defn fib [n] (if (<= n 1) 1 (+ (fib (dec n)) (fib (- n 2))))), so that the occurrences of fib in the body are resolved to the fib being defined.

Comment by Alex Miller [ 22/Nov/14 9:05 AM ]

Moving to 1.7 until I can look at this more deeply.

Comment by Mike Anderson [ 23/Nov/14 6:08 PM ]

Andy - yes the workaround is fine for me right now.

I don't think this is an urgent issue but it may be exposing a subtle complexity regarding assumptions about the state of the namespace at different times. Perhaps the semantics should be something like:

  • The def statement itself should be run before the var is interned. e.g. (def inc (inc 5)) should result in (def inc 6)
  • Anything complied / deferred to run after completion of the def statement should use the new var (i.e. the new var should be referenced by fns, lazy sequences etc.)
Comment by Andy Fingerhut [ 23/Nov/14 6:36 PM ]

I'm not sure what your proposal means in a case like this:

(def inc (fn [x] (inc x)))

Is the second inc to be interpreted/resolved before or after the new inc is created? Because it is (fn ...) it should be the after-behavior? What else besides fn should cause the after-behavior, rather than the before-behavior?

Even more fun (not saying that people often write code like this, but the compiler can handle it today):

(def inc (if (> (inc y) 5)
           (fn [x] (inc x))
           (fn [x] (dec x))))

I think the current compiler behavior of 'in the body of a def, the def'd symbol always refers to the new var, not any earlier def'd vars' is fairly straightforward to explain.

Comment by Tom Crayford [ 23/Nov/14 9:15 PM ]

Should I file the AOT issue reproduced in that thing as a new issue?

Comment by Andy Fingerhut [ 24/Nov/14 5:16 PM ]

Tom: Alex Miller or another screener would be best to say whether the AOT issue should be a separate ticket, but my best guess would be "go for it". I tried to look at the link you gave but it seems not to point to anything. Could you double-check that link?

Comment by Tom Crayford [ 24/Nov/14 6:48 PM ]

Andy,

Great. I'll write one up tomorrow sometime. I accidentally left that repo as private, should be visible now.

Comment by Andy Fingerhut [ 24/Nov/14 8:11 PM ]

This comment is really most relevant for ticket CLJ-1604, where it has been copied:

Tom, looked at your project. Thanks for that. It appears not to have anything like (def inc inc) in it. It throws exception during test step of 'lein do clean, uberjar, test' consistently for me, too, but compiles with only warnings and passes tests with 'lein do clean, test'. I have more test results showing in which Clojure versions these results change. To summarize, the changes to Clojure that appear to make the biggest difference in the results are below (these should be added to the new ticket you create – you are welcome to do so):

Clojure 1.6.0, 1.7.0-alpha1, and later changes up through the commit with description "CLJ-1378: Allows FnExpr to override its reported class with a type hint": No errors or warnings for either lein command above.

Next commit with description "Add clojure.core/update, like update-in but takes a single key" that adds clojure.core/update: 'lein do clean, test' is fine, but 'lein do clean, uberjar' throws exception during compilation, probably due to CLJ-1241.

Next commit with description "fix CLJ-1241": 'lein do clean, test' and 'lein do clean, uberjar' give warnings about clojure.core/update, but no errors or exceptions. 'lein do clean, uberjar, test' throws exception during test step that is same as the one I see with Clojure 1.7.0-alpha4. Debug prints of values of clojure.core/update and int-map/update (in data.int-map and in Tom's namespace compiler-update-not-referenced-bug.core) show things look fine when printed inside data.int-map, and in Tom's namespace when not doing the uberjar, but when doing the uberjar, test, int-map/update is unbound in Tom's namespace.

In case it makes a difference, my testing was done with Mac OS X 10.9.5, Leiningen 2.5.0 on Java 1.7.0_45 Java HotSpot(TM) 64-Bit Server VM

Comment by Nicola Mometto [ 25/Nov/14 3:44 PM ]

Tom, I've opened a ticket with a patch fixing the AOT issue: http://dev.clojure.org/jira/browse/CLJ-1604

Comment by Kevin Downey [ 03/Jun/16 5:09 AM ]

the consequences of this bug can be very hard to track back to this bug. it would be really nice to get it fixed in someway.

(defmulti update identity)
... pages of other code ...
(defmethod update :foo [_])

will throw a compiler error on the defmethod saying update is unbound

Comment by Sean Corfield [ 18/Aug/16 4:02 PM ]

This looks very similar to the bug I ran into recently with Encore and an early Alpha of Clojure 1.9.0 where a new core predicate had been introduced, causing a warning from Encore which defined a function with the same name – but use of the code produced an error that the var was unbound. In that case it was a series of defn forms inside a do form – done to put a reader conditional around a group of function definitions. I lifted the conflicting functions out of the do and the error went away (but the warning remained, as expected). You can see the PR I submitted to Encore showing the code rearrangement: https://github.com/ptaoussanis/encore/pull/26/commits/040bf1be99eee79cbbcb7cc10ed37aa0a1e7ec17

Adding those functions to :refer-clojure :exclude instead solved the problem "properly" (and made the warning go away, obviously).





[CLJ-1585] Report boxed math warning on function that boxes primitive return value Created: 11/Nov/14  Updated: 14/Dec/15

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.7
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Alex Miller Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: errormsgs, math

Attachments: Text File clj-1585.patch    
Patch: Code

 Description   

With the new :warn-on-boxed (CLJ-1325), these examples do not report a boxed math warning although they each do boxing:

user=> (defn f1 [^long x] (inc x))
f1
user=> (defn f2 [x] (aget (long-array [1 2]) 0))
f2
user=> (defn f3 [x] (aget (int-array [1 2]) 0))
f3
user=> (defn f4 [^String s] (.indexOf s "a"))

Cause: emitBoxReturn has a hard-coded call to box a prim return value.

Solution: If *unchecked-math* is set to :warn-on-boxed, emit a warning on boxing of primitive numeric return types.

Patch:



 Comments   
Comment by Alex Miller [ 12/Nov/14 12:39 AM ]

Attached patch does the job, but from trying it out on some real code, it finds both problematic cases and lots of cases that could safely be ignored and/or where there is no obvious way to fix the warning. I think it may need some more tuning to reduce the rate of unfixable things a bit.





[CLJ-1583] Apply forces the evaluation of one element more than necessary Created: 07/Nov/14  Updated: 09/Nov/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Nicola Mometto Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File 0001-make-RT.boundedLength-lazier.patch    
Patch: Code

 Description   

Given a function with one fixed argument and a vararg, it should be sufficient to force evaluation of 2 elements for apply to know which arity it should select, however it currently forces 3:

user=> (defn x ([a & b]))
#'user/x
user=> (apply x (map println (iterate inc 0)))
0
1
2
nil

This makes lazy functions that use apply (for example mapcat) less lazy than they could be.
The proposed patch makes RT.boundedLength short-circuit immediately after the seq count is greater than the max fixed arity:

user=> (defn x ([a & b]))
#'user/x
user=> (apply x (map println (iterate inc 0)))
0
1
nil


 Comments   
Comment by Nicola Mometto [ 09/Nov/14 3:37 PM ]

The patch in this ticket slightly improves the issue reported at http://dev.clojure.org/jira/browse/CLJ-1218





[CLJ-1582] Overriding in-ns and ns is problematic Created: 07/Nov/14  Updated: 07/Nov/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Nicola Mometto Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None

Attachments: Text File 0001-Allow-overriding-of-clojure.core-in-ns-and-clojure.c.patch    
Patch: Code

 Description   

Currently it is possible to override clojure.core/in-ns and clojure.core/ns, but it is not possible to refer to the namespace-specific vars without fully qualifying them:

user=> (ns foo (:refer-clojure :exclude [in-ns]))
nil
foo=> (def in-ns 1)
#'foo/in-ns
foo=> in-ns
#<clojure.lang.RT$1@76b5e4c5>

After this patch, overriding in-ns and ns works like for every other clojure.core var:

user=> (ns foo (:refer-clojure :exclude [in-ns]))
nil
foo=> (def in-ns 1)
#'foo/in-ns
foo=> in-ns
1


 Comments   
Comment by Reid McKenzie [ 07/Nov/14 11:46 AM ]

This is motivated by https://github.com/jonase/eastwood/issues/100





[CLJ-1577] Some hints accept both symbols and class objects, others only symbols Created: 30/Oct/14  Updated: 30/Oct/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.7
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Brandon Bloom Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: typehints


 Description   

In order to hint primitives, such as longs, you can hint with the symbol 'long. In some places, you can also use the class object java.lang.Long/TYPE. However, in some places, that doesn't work. This is particularly problematic when working with hints in macros, where subtle changes to when metadata is evaluated can lead to changes in whether or not hints are respected.

user=> (set! *unchecked-math* :warn-on-boxed)
:warn-on-boxed

user=> (defmacro mac []
         (let [field (with-meta 'x {:tag 'long})]
           (-> field meta :tag class prn)
           `(deftype Foo# [~field]
              clojure.lang.IDeref
              (deref [this#]
                (inc ~(with-meta field nil))))))
#'user/mac

user=> (mac)
clojure.lang.Symbol
#<java.lang.Class@1c76c583 class user.Foo__13651__auto__>

user=> (defmacro mac []
         (let [field (with-meta 'x {:tag java.lang.Long/TYPE})]
           (-> field meta :tag class prn)
           `(deftype Foo# [~field]
              clojure.lang.IDeref
              (deref [this#]
                (inc ~(with-meta field nil))))))
#'user/mac

user=> (mac)
java.lang.Class
Boxed math warning, /private/var/folders/43/mnwlkd2s7r1gbjwq6t__mgt40000gn/T/form-init5463347341158437534.clj:1:1 - call: public static java.lang.Number clojure.lang.Numbers.unchecked_inc(java.lang.Object).
#<java.lang.Class@74626b21 class user.Foo__13663__auto__>





[CLJ-1575] Using a (def ^:const instance) of a deftype that implements IPersistentCollection, triggers compiler errors Created: 29/Oct/14  Updated: 24/Dec/16

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.6, Release 1.7
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Herwig Hochleitner Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

fresh repl


Attachments: Text File 0001-Test-for-analyzer-bug-CLJ-1575.patch    

 Description   

The compiler has a lot of assumptions about the possible types of IPersistentCollection literals and rightfully so. The strange thing with this case is, that taking the (constant) value works as soon as count is defined, but using it as an argument hits a closed dispatch for emitting the empty variants of the various literals.

> (deftype T [] clojure.lang.IPersistentCollection (count [_] 0)
> (def ^:const t (T.))
> (meta t)
java.lang.UnsupportedOperationException: Unknown Collection type
Compiler.java:2860 clojure.lang.Compiler$EmptyExpr.emit
Compiler.java:3632 clojure.lang.Compiler$InvokeExpr.emitArgsAndCall
...

EDIT updated the ticket after some investigation
NOTE attached test patch doesn't even implement (count []) for the deftype, which just triggers a rightful AbstractMethodError



 Comments   
Comment by Herwig Hochleitner [ 29/Oct/14 10:00 PM ]

The test had a typo, sorry

Comment by Alex Miller [ 30/Oct/14 7:14 AM ]

Looks like a variant of CLJ-1093.

Comment by Herwig Hochleitner [ 24/Dec/16 8:27 PM ]

This bug is still present in 1.8, even though CLJ-1093 has been marked fixed for 1.8.





[CLJ-1573] Support (Java) transient fields in deftype, e.g. for hashcodes Created: 26/Oct/14  Updated: 15/May/17

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Feature Priority: Minor
Reporter: Ghadi Shayban Assignee: Unassigned
Resolution: Unresolved Votes: 4
Labels: compiler, deftype

Attachments: Text File 0001-transient-field-deftype.patch    

 Description   

Enhance deftypes to allow fields to be marked ACC_TRANSIENT.

strawman syntax:
(deftype AType [^:transient hash])

Came across this need while experimenting with a reified range written in a deftype, not in Java.

Patch doesn't include docstring change, but has a test.



 Comments   
Comment by Adrian Medina [ 29/Dec/14 11:54 AM ]

Perhaps ^:transient-mutable would be a more appropriate modifier name to be consistent with the ^:unsynchronized-mutable and ^:volatile-mutable field modifiers. In any event, this feature would eliminate the need to drop down to Java for types that require transient fields.

Comment by Andy Fingerhut [ 29/Dec/14 12:07 PM ]

Roberto, there is a "Vote" word you can click on to actually vote for tickets, and ticket wranglers actually look at those votes at times to examine popular ones sooner. +1 comments don't do that.





[CLJ-1566] Documentation for clojure.core/require does not document :rename Created: 16/Oct/14  Updated: 02/Jul/15

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: James Laver Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File refer.patch    
Patch: Code

 Description   

By contrast, clojure.core/use does mention :rename.

I attach a patch



 Comments   
Comment by Andy Fingerhut [ 16/Oct/14 1:33 PM ]

James, your patch removes any mention of the :all keyword, and that keyword is not mentioned in the doc string for clojure.core/refer.

I haven't checked whether refer can take :all as an argument, but clojure.core/require definitely can.

Comment by James Laver [ 16/Oct/14 1:39 PM ]

Ah, you're quite right. Fixed now. See updated patch in a sec.

Comment by Andy Fingerhut [ 16/Oct/14 8:16 PM ]

For sake of reduced confusion, it would be better if you could either name your patches differently, or delete obsolete ones with identical names as later ones. JIRA allows multiple patches to have the same names, without replacing the earlier ones.

Comment by James Laver [ 17/Oct/14 12:44 AM ]

Okay, that's done. The JIRA interface is a bit tedious in places.

Comment by Bozhidar Batsov [ 19/Oct/14 1:34 AM ]

Seems to me the sentence should end with a dot.

Comment by James Laver [ 19/Oct/14 4:36 AM ]

Added a dot.

Comment by Andy Fingerhut [ 02/Jul/15 3:56 PM ]

James, I do not know whether this change is of interest to the Clojure core team, but see this link for instructions on creating patches in the expected format (yours is not in that format): http://dev.clojure.org/display/community/Developing+Patches





[CLJ-1563] How About Default Implementations on Protocols Created: 11/Oct/14  Updated: 15/May/17

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Feature Priority: Minor
Reporter: David Williams Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Consider this example

user=> (defprotocol Foo (foo [x] x))
Foo
user=> (defrecord Bar [gaz waka] Foo)
user.Bar
user=> (def bar (Bar. 1 2))
#'user/bar
user=> (.foo bar)

AbstractMethodError user.Bar.foo()Ljava/lang/Object;  sun.reflect.NativeMethodAccessorImpl.invoke0 (NativeMethodAccessorImpl.java:-2)
user=>

What about the default implementation.



 Comments   
Comment by David Williams [ 11/Oct/14 8:48 PM ]

As it stands you have to workaround with this

http://stackoverflow.com/questions/15039431/clojure-mix-protocol-default-implementation-with-custom-implementation

Comment by Jozef Wagner [ 12/Oct/14 1:01 AM ]

I don't think we need it. What's the rationale behind extending some protocol, not implementing its methods, and then calling those methods, expecting them not to throw. Be explicit about what yout type should do, whether it is a default or custom behavior. You basically have three options

(defn default-foo 
  [this] 
  :foo)

(defprotocol P
  (-foo [this]))

(deftype T
  P
  (-foo [this] (default-foo))

(defn foo 
  [x]
  (-foo x))

or

(defprotocol P
  (-foo [this]))

(deftype T)

(defn foo 
  [x]
  (if (satisfies? P x)
    (-foo x)
    :foo))

or

(defprotocol P
  (-foo [this]))

(extend-protocol P
  java.lang.Object
  (-foo [this] :foo))

(deftype T)

(defn foo 
  [x]
  (-foo x))

I think however that my first approach is unidiomatic and you should prefer the latter ones.

Comment by David Williams [ 12/Oct/14 12:36 PM ]

I agree, this is a low priority enhancement. I think it could make the Protocol experience more DWIMy, and Java 8 has default implementations on interfaces for the same kind of convenience.





[CLJ-1560] Forbid closing over mutable fields completely Created: 10/Oct/14  Updated: 19/Jul/15

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.6
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Jozef Wagner Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Closing over mutable fields should be forbidden completely (and generate compiler exception), not just when trying to set! them. As the change of the mutable field does not propagate into closed over ones, this leads to surprising bugs:

(defprotocol P 
  (-set [this]) 
  (-get [this]) 
  (-get-fn [this]))

(deftype T [^:unsynchronized-mutable val] 
  P 
  (-set [this] (set! val :bar)) 
  (-get [this] val) 
  (-get-fn [this] (fn [] val)))

(def x (->T :foo))

(def xf (-get-fn x))

user> (-set x)
:bar
user> (-get x)
:bar
user> (xf)
:foo ;; should be :bar !!!


 Comments   
Comment by Jozef Wagner [ 10/Oct/14 1:42 PM ]

related issue CLJ-274

Comment by Ghadi Shayban [ 28/Dec/14 10:42 PM ]

In the given example, the close-over happens before the set!, so the closure gets the value, not an assignable container. This is consistent with the rest of the language (pass-by-value not by mutable container)

Comment by Jozef Wagner [ 29/Dec/14 2:21 AM ]

Thanks for explanation. The ticket is about a proposal that closing over a mutable field should result in error being thrown, an not in a value. If value is desired, an explicit let binding will have to be used. So far, I haven't found a valid use case where closing over mutable field and getting the value closed over is the intended and wanted behavior.





[CLJ-1553] Parallel transduce Created: 07/Oct/14  Updated: 15/May/17

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.7
Fix Version/s: None

Type: Feature Priority: Major
Reporter: Alex Miller Assignee: Unassigned
Resolution: Unresolved Votes: 11
Labels: transducers


 Description   

Consider how to create a parallel path for transducers, similar to reducers fold.






[CLJ-1552] Consider kv support for transducers (similar to reducers fold) Created: 07/Oct/14  Updated: 15/May/17

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.7
Fix Version/s: None

Type: Feature Priority: Major
Reporter: Alex Miller Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: transducers


 Description   

In reducers, fold over a map has special support for kv. Consider whether/how to add this for transducers.



 Comments   
Comment by Marshall T. Vandegrift [ 16/Dec/14 11:13 AM ]

We don't have a JIRA "unvote" feature, but I'd like to register my vote against this proposed enhancement. As a heavy user of clojure.core.reducers, I consider the switch to k-v semantics when reducing a map to be a significant mis-feature. As only an initial transformation function applied directly to a map is able to receive the k-v semantics (a limitation I can’t see how would not carry over to transducers), this behavior crops up most frequently when re-ordering operations and discovering that an intermediate map has now caused an airity error somewhere in the middle of a chain of threaded transformations. I’ve never found cause to invoke it intentionally.

Comment by Ghadi Shayban [ 21/Jan/16 9:17 PM ]

Marshall, there really isn't a proposed enhancement, yet. So there's nothing to be against! Your input is valuable. (Regarding c.c.reducers, that is a separate problem – yes that behavior is surprising)

Considering kv-support for transducers:
Is it useful to have some functions that transform reduce-kv style reducing functions (fn [result k v])?

Ignore naming:
map-key
map-val
map-keyval
filter-*

These could be mechanically generated. You wouldn't have to have a kv-version for every transducer currently in core. Some like map or filter could specifically apply to the key and ignore the val, or v.v.

Some things like map's transducer would be arity-incompatible (map's transducer has a varargs arity).





[CLJ-1548] primitive type hints on protocol methods break call sites Created: 04/Oct/14  Updated: 04/Oct/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.6
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Brandon Bloom Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: None


 Description   
user=> (defprotocol P (f [this ^long x]))
P
user=> (deftype T [] P (f [_ x] x))
#<java.lang.Class class user.T>
user=> (f (T.) 5)

ClassCastException user$eval7289$fn__7290$G__7280__7297 cannot be cast to clojure.lang.IFn$OLO  user/eval7313 (NO_SOURCE_FILE:1)





[CLJ-1543] Type tags on argument vector appear to help avoid reflection when used with defn, but not with def foo (fn ...) Created: 30/Sep/14  Updated: 02/Oct/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.6
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Andy Fingerhut Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: interop, typehints


 Description   

I would have expected that both of the Java interop calls below would avoid reflection, but only the first involving f1 does.

Clojure 1.6.0
user=> (set! *warn-on-reflection* true)
true
user=> (defn f1 ^java.util.LinkedList [coll] (java.util.LinkedList. coll))
#'user/f1
user=> (def f2 (fn ^java.util.LinkedList [coll] (java.util.LinkedList. coll)))
#'user/f2
user=> (.size (f1 [2 3 4]))
3
user=> (.size (f2 [2 3 4]))
Reflection warning, NO_SOURCE_PATH:5:1 - reference to field size can't be resolved.
3

Not sure if this has anything to do with CLJ-1232, but was discovered when testing variants of that issue.



 Comments   
Comment by Andy Fingerhut [ 30/Sep/14 9:08 PM ]

What a nice number for a ticket, 1543. The year Copernicus's most celebrated book was published: http://en.wikipedia.org/wiki/Nicolaus_Copernicus

Comment by Jozef Wagner [ 01/Oct/14 4:05 AM ]

Isn't type hinting of arg vector meant only for primitive type hints? AFAIK non-primitive type hints should be on a function name, everything else is non idiomatic.

Comment by Nicola Mometto [ 01/Oct/14 7:05 AM ]

This isn't an issue of arg vector hinting vs function name hinting.
The issue here is that return type hinting cannot be put on anonymous functions but only on defns as the :arglists will be added by defn on the Var's metadata.

This is one of the reasons why I'd like to have that information as a field on the fn rather than as metadata on the Var

Comment by Andy Fingerhut [ 01/Oct/14 10:55 AM ]

Jozef, you may be correct that non-primitive type hints on the argument vector are non idiomatic. Do you have any source for that I could read?

Comment by Tassilo Horn [ 02/Oct/14 12:19 AM ]

Only the version with hints on the argument vectors is documented at http://clojure.org/java_interop#Java Interop-Type Hints. However, in the case you have just one arity (or all arities return a value of the same type) the hint on the var name also works. But the two versions seem to have different semantics. Have a look at CLJ-1232.

Comment by Jozef Wagner [ 02/Oct/14 5:48 AM ]

Type hinting is a very intricate part of Clojure but you can almost always apply a 'place hint on a symbol' idiom. Type hinting on an arg vector must be done only in two cases:

  • primitive hints
  • different return classes for different arities

In the first case, compiler needs type hints when compiling fn* (see [1]), not later, thus you must specify them on arg vector.

Second case, which is the issue discussed here, must be used only when defining with defn. Compiler first looks for the tag in the metadata of a var, and if it does not find one, it has a special case in which it looks for a return class inside :arglist metadata. This is clearly a very special case [2] to handle situations where you have different return classes for different arities. Obviously, using def instead of defn won't create an :arglist metadata for you thus you see a reflection warning. Example:

user=> (def f2 (fn ^java.util.LinkedList [coll] (java.util.LinkedList. coll)))
#'user/f2
user=> (.size (f2 [2 3 4]))
Reflection warning, /tmp/form-init.clj:1:1 - reference to field size can't be resolved.
3
user=> (alter-meta! #'f2 assoc :arglists '(^java.util.LinkedList [coll]))
{:ns #<Namespace user>, :name f2, :file "/tmp/form-init.clj", :column 1, :line 1, :arglists ([coll])}
user=> (.size (f2 [2 3 4]))
3

BTW CLJ-1491 has a discussion slightly relevant to this topic.

[1] https://github.com/clojure/clojure/blob/03cd9d159a2c49a21d464102bb6d6061488b4ea2/src/jvm/clojure/lang/Compiler.java#L5134
[2] https://github.com/clojure/clojure/blob/03cd9d159a2c49a21d464102bb6d6061488b4ea2/src/jvm/clojure/lang/Compiler.java#L3572

Comment by Jozef Wagner [ 02/Oct/14 7:15 AM ]

Andy, I've found sources that speak against my recommendations See CLJ-811 and [1].

[1] https://groups.google.com/d/msg/clojure/b005zQCPxOQ/6G0AlWKKKa0J





[CLJ-1538] Set literal duplicate check occurs too early. Created: 27/Sep/14  Updated: 09/Oct/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.6, Release 1.7
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Chhi'mèd Künzang Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: reader


 Description   

I cannot use literal syntax to create a set/map with unique members/keys if the elements are generated with an identical form. Examples of such legal forms: (rand), (read), (clojure.core.async/<!!), etc. I will use (rand) in these examples.

user=> #{(rand) (rand)}
IllegalArgumentException Duplicate key: (rand)  clojure.lang.PersistentHashSet.createWithCheck (PersistentHashSet.java:68)

user=> {(rand) 1, (rand) 2}

IllegalArgumentException Duplicate key: (rand)  clojure.lang.PersistentArrayMap.createWithCheck (PersistentArrayMap.java:70)

It appears that the input is being checked for duplicates before the arguments to the collection constructors are evaluated. However, this doesn't prevent the need to run the check again later.

Note that duplicates are still (correctly) detected, after evaluation, even if duplicates do not appear as literals in the source:

user=> #{(+ 1 1) 2}

IllegalArgumentException Duplicate key: 2  clojure.lang.PersistentHashSet.createWithCheck (PersistentHashSet.java:56)
user=> {(+ 1 1) :a, 2 :b}

IllegalArgumentException Duplicate key: 2  clojure.lang.PersistentArrayMap.createWithCheck (PersistentArrayMap.java:70)

The first duplicate check therefore seems to be both redundant and incorrect.

Note that this eager duplicate-checking seems to have higher precedence even than the syntax-quote reader macro.

user=> `#{~(rand) ~(rand)}

IllegalArgumentException Duplicate key: (clojure.core/unquote (rand))  clojure.lang.PersistentHashSet.createWithCheck (PersistentHashSet.java:68)

user=> `{~(rand) 1, ~(rand) 2}

IllegalArgumentException Duplicate key: (clojure.core/unquote (rand))  clojure.lang.PersistentArrayMap.createWithCheck (PersistentArrayMap.java:70)

This is odd – since syntax-quote should not realize a collection at all at read time:

For Lists/Vectors/Sets/Maps, syntax-quote establishes a template of the corresponding data structure. Within the template, unqualified forms behave as if recursively syntax-quoted, but forms can be exempted from such recursive quoting by qualifying them with unquote or unquote-splicing, in which case they will be treated as expressions and be replaced in the template by their value, or sequence of values, respectively. (http://clojure.org/reader)

Definitions aside, based on the apparent expansion of syntax-quote, I would expect the previous to have worked correctly.

If I fake the expected macroexpansion by manually substituting the desired inputs, I get the expected results:

user=> '`#{~:a ~:b}
(clojure.core/apply clojure.core/hash-set (clojure.core/seq (clojure.core/concat (clojure.core/list :b) (clojure.core/list :a))))
user=> (clojure.core/apply clojure.core/hash-set (clojure.core/seq (clojure.core/concat (clojure.core/list (rand)) (clojure.core/list (rand)))))
#{0.27341896385866227 0.3051522362827035}
user=> '`{~:a 1, ~:b 2}
(clojure.core/apply clojure.core/hash-map (clojure.core/seq (clojure.core/concat (clojure.core/list :a) (clojure.core/list 1) (clojure.core/list :b) (clojure.core/list 2))))
user=> (clojure.core/apply clojure.core/hash-map (clojure.core/seq (clojure.core/concat (clojure.core/list (rand)) (clojure.core/list 1) (clojure.core/list (rand)) (clojure.core/list 2))))
{0.12476921225204185 2, 0.5807961046096718 1}

It seems to me that there is a superfluous duplicate check being run before the set/map reader macros evaluate their arguments. This check should seemingly be removed. Even if the check did not catch some false-positive duplicates (as it does), it would be unnecessary since the apparent second post-evaluation check would catch all true duplicates.

All that said, it's unclear that this check should happen at all. If I try to create sets/map with duplicate members/keys, I don't get an error. The duplicates are silently removed or superseded.

user=> (set (list 1 1))
#{1}
user=> (hash-map 1 2 1 3)
{1 3}

It seems it would be most consistent for literals constructed by the reader syntax to do the same.

I can see the argument that a literal representation is not a 'request to construct' but rather an attempt to simulate the printed representation of a literal data object. From that perspective, disallowing 'illegal' printed representations seems reasonable. Unfortunately, the possibility of evaluated forms inside literal vectors, sets, and maps (since lists are evaluated at read time) already breaks this theory. That is, the printed representation of such collections is not an accurately readable form, so read-time duplicate checking still cannot prevent seeming inconsistencies in print/read representations:

user=> '#{(+ 1 1) 2}
#{(+ 1 1) 2}
user=> #{(+ 1 1) 2}

IllegalArgumentException Duplicate key: 2  clojure.lang.PersistentHashSet.createWithCheck (PersistentHashSet.java:56)

Given that the problem cannot be completely avoided at all, it seems simplest and most consistent to treat reader literal constructors like their run-time counterparts, as syntax quote would in the absence of the spurious duplicate check.



 Comments   
Comment by Alex Miller [ 09/Oct/14 8:04 AM ]

Also see CLJ-1555

Comment by Nicola Mometto [ 09/Oct/14 8:09 AM ]

Potentially related: http://dev.clojure.org/jira/browse/CLJ-1425





[CLJ-1536] Remove usage of sun.misc.Signal (which may not be available in Java 9) Created: 26/Sep/14  Updated: 26/Sep/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.7
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Alex Miller Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

It looks like Java 9 will not continue to provide access to "internal" classes like sun.misc.Signal. Clojure currently uses this in the REPL to trap ctrl-c (SIGINT) and cancel current evaluation instead of process shutdown.

There is a page of alternatives here:
https://wiki.openjdk.java.net/display/JDK8/Java+Dependency+Analysis+Tool

But there is no suggested alternative for sun.misc.Signal and I'm not aware of a portable solution to it.






[CLJ-1534] Adding condp-> and condp->> macros to core library Created: 24/Sep/14  Updated: 15/May/17

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.7
Fix Version/s: None

Type: Feature Priority: Major
Reporter: Kuldeep Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: enhancement, macro

Attachments: File clj_1534.diff     File condp-threading-macros-25sept2014.diff    

 Description   

After introduction of cond-> and cond->> macros in 1.5. It makes sense to have condp-> and condp->> macros in the core library.

(condp-> {}
(complement :a) (assoc :a 1)
:a (assoc :b 2)) ;=> {:b 2, :a 1}

In the above example the result of each expr which was evaluated is being passed to the next predicate.



 Comments   
Comment by Andy Fingerhut [ 01/Oct/14 6:37 PM ]

Kuldeep, I cannot comment on whether this change is of interest to the Clojure developers, because I do not know.

I can say that the patch you have attached is not in the expected format. See the page below for instructions on creating a patch in the expected format:

http://dev.clojure.org/display/community/Developing+Patches

Comment by Kuldeep [ 28/Jan/15 11:31 AM ]

Rebased against master and generated patch as described in wiki.

Comment by Vitalie Spinu [ 12/Jul/16 5:21 AM ]

This is a very common pattern for me.

This is one way of dealing with such state-dependent conditionals:

(-> x
  (as-> y (if (:foo y) (assoc y :boo 0) y))
  ...)

The proposed `condp->` is much more readable:

(-> x
  (condp-> :foo (assoc :boo 0))
  ...)

BTW, `condp->` is not exactly the counterpart of `condp`. So maybe shorter `pred->` or `p->` are better names for this.





[CLJ-1532] pr-str captures stdout from printing side-effects of lazily evaluated expressions. Created: 23/Sep/14  Updated: 14/Nov/16

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.6
Fix Version/s: