<< Back to previous view

[CLJ-1619] PersistentVector implements IReduce but the no init arity throws Created: 17/Dec/14  Updated: 17/Dec/14

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

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

Attachments: Text File 0001-Implement-no-init-arity-of-reduce-for-PersistentVect.patch    
Patch: Code

 Description   

The reduce arity of IReduce in PersistentVector is implemented as: "throw new UnsupportedOperationException()".
See: http://dev.clojure.org/jira/browse/CLJ-1572?focusedCommentId=36878&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-36878






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

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

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

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

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

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

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

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



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

Attached patch demonstrating problem (not a fix)

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

More investigation:

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

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

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

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

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

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

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





[CLJ-1614] Clojure does not start: ClassCastException Created: 12/Dec/14  Updated: 12/Dec/14

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

Type: Defect Priority: Minor
Reporter: Vladimir Tsichevski Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: compiler
Environment:

Eclipse RCP



 Description   

The clojure.lang.Compiler class static code throws the ClassCastException when reading compiler options from System properties (Compiler.java, line 260 in the git master release). When running Clojure from Eclipse RCP application the System properties may have non-string values.

Checking if the value is String and ignoring non-strings fixes this problem.






[CLJ-1613] :or defaults should refer to enclosing scope in map destructuring Created: 12/Dec/14  Updated: 12/Dec/14

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

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

Attachments: Text File 0001-CLJ-1613-evaluate-or-defaults-in-enclosing-scope-in-.patch    

 Description   

Michael Blume noticed that :or defaults can depend on the values of other keys, see https://groups.google.com/d/msg/clojure/6kOhpPOpHWM/ITjWwQFS_VQJ

Michael's Gist https://gist.github.com/MichaelBlume/4891dafdd31f0dcbc727 displays a case where an associative form involving :keys and :or compiles or not depending on the order of symbols in :keys. By tweaking that case one can arrive at expressions which always compile, but produce different values depending on :keys:

(let [foo 1
       bar 2
       {:keys [bar foo]
        :or {foo 3 bar (inc foo)}} {}]
  {:foo foo :bar bar})
;= {:foo 3, :bar 4}

(let [foo 1
      bar 2
      {:keys [foo bar]
       :or {foo 3 bar (inc foo)}} {}]
  {:foo foo :bar bar})
;= {:foo 3, :bar 2}

I believe that the most natural solution is to demand that :or defaults be evaluated in an enclosing scope where none of the destructuring-introduced locals are present. This approach is taken by the 0001 patch.



 Comments   
Comment by Michael Blume [ 12/Dec/14 2:27 AM ]

I suspect that this is the right thing to do but I think it's important to note that this will break existing code https://github.com/ngrunwald/ring-middleware-format/blob/master/src/ring/middleware/format_params.clj#L214





[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-1611] clojure.java.io/pushback-reader Created: 08/Dec/14  Updated: 08/Dec/14

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

Type: Enhancement Priority: Major
Reporter: Phill Wolf Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Whereas

  • clojure.core/read and clojure.edn/read require a PushbackReader;
  • clojure.java.io/reader produces a BufferedReader, which isn't compatible;
  • the hazard has tripped folks up for years[1];
  • clojure.java.io is pure sugar anyway (and would not be damaged by the addition of a little bit more);
  • clojure.java.io's very existence suggests suitability and fitness for use (wherein by the absence of a read-compatible pushback-reader it falls short);

i.e., in the total absence of clojure.java.io it would not seem "hard" to use clojure.edn, but in the presence of clojure.java.io and its "reader" function, amidst so much else in the API that does fit together, one keeps thinking one is doing it wrong;

and

  • revising the "read" functions to make their own Pushback was considered but rejected [2];

Therefore let it be suggested to add clojure.java.io/pushback-reader, returning something consumable by clojure.core/read and clojure.edn/read.

[1] The matter was discussed on Google Groups:

(2014, "clojure.edn won't accept clojure.java.io/reader?") https://groups.google.com/forum/#!topic/clojure/3HSoA12v5nc

with a reference to an earlier thread

(2009, "Reading... from a reader") https://groups.google.com/forum/#!topic/clojure/_tuypjr2M_A

[2] CLJ-82 and the 2009 message thread






[CLJ-1599] Add get-and-set! to expose AtomicReference.getAndSet() in atoms Created: 24/Nov/14  Updated: 24/Nov/14

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

Type: Enhancement Priority: Minor
Reporter: Steven Yi Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: enhancement

Attachments: File get-and-set.diff    
Patch: Code

 Description   

DESCRIPTION

This patch adds get-and-set! to core to allow getting the last value from an atom and setting it to a new value. This is useful for atomically draining an atom of its value and setting to a new value. The implementation delegates to Java's AtomicReference.getAndSet().

The equivalent operation in Clojure code would be:

(defn get-and-set! [atm newv]
(loop [oldv @atm]
(if (compare-and-set! atm oldv newv)
oldv
(recur @atm))))

This is close to a 1:1 translation of the Java code in sun.misc.Unsafe's getAndSetObject, used by AtomicReference (as of current JDK9 source code).

APPLICATIONS

  • User may want to check if an operation has occurred before by using an atom as a flag. I.e.,

(def has-run-once (atom false))
...
(when-not (get-and-set! has-run-once true)
(do something))

  • User may want to use an atom similarly to a java.util.concurrent.LinkedTransferQueue, for the case of pairing up adds by writers and drainTo by readers:

Thread 1: (swap! atm conj item1)
Thread 2: (swap! atm conj item2)
Thread 3: (let [new-vals (get-and-set! atm [])]
(do-something new-vals))

ALTERNATIVES

  • For case of atom as flag, user can use existing compare-and-set!:

(def has-run-once (atom false))
...
(when-not (compare-and-set! has-run-once false true)
(do something))

Argument: get-and-set! is a little clearer in intent as it is using the value of the atom, rather than the success of the cas operation. Also, this would not be applicable to situations where the value stored is not a boolean.

  • User can just go ahead and use LinkedTransferQueue.

Argument: User not fluent in Java may not be readily able to use this.

==

Argument for: This seems like a sufficiently primitive operation to include in core for atoms. I am unsure of the rationale, but assume it was vetted to include into Java's AtomicReference for a reason. Also, if users are using atoms and have this available, they are less likely to try to do this incorrectly, such as:

(let [vals @some-atom]
(reset! some-atom [])
(do-something-with vals))

Argument against: This may not be sufficiently primitive enough to include in core. Users have a workaround to implement the get-and-set! operation in user-code as given above.

Note: This request is similar to CLJ-1454 (http://dev.clojure.org/jira/browse/CLJ-1454), but differs in that this is not a swap! operation that accepts an IFn argument. Also, I looked to add a test in test/clojure/test_clojure/atoms.clj, but saw that the other operations weren't tested. (I assume this is due to the other operations delegating to AtomicReference and hence not deemed test-worthy.)






[CLJ-1597] Allow ISeq args to map conj Created: 22/Nov/14  Updated: 22/Nov/14

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

Type: Enhancement Priority: Minor
Reporter: Nicola Mometto Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: collections, maps

Attachments: Text File 0001-allow-ISeq-args-to-map-conj.patch    
Patch: Code and Test

 Description   

Currently conj on maps can take only maps or vectors, this patch allows it to take any ISeq:

user=> (conj {} '(1 2))
{1 2}





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

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.






[CLJ-1593] Use PAM for small maps when assigned to a var rather than always using PHMs Created: 15/Nov/14  Updated: 08/Dec/14

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

Type: Enhancement Priority: Minor
Reporter: Nicola Mometto Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: collections, compiler, maps

Attachments: Text File 0001-Use-PAM-rather-than-always-using-PHMs-for-small-maps.patch    
Patch: Code

 Description   

I'm reproposing the fix I implemented for http://dev.clojure.org/jira/browse/CLJ-944 a while ago as an enhancement rather than as a defect.

Currently when a map is used as the value of a `def` expression, unless it's an empty map, it will always be a PersistentHashMap even if it's a small map.

user=> (def a {:foo :bar})
#'user/a
user=> (class a)
clojure.lang.PersistentHashMap

The current patch makes makes small maps be compiled to PAMs, consistently with how it's handled in lexical contexts, only using PHMs when the number of elements is above the threshold

user=> (def a {:foo :bar})
#'user/a
user=> (class a)
clojure.lang.PersistentArrayMap
user=> (class (let [a {:foo :bar}] a))
clojure.lang.PersistentArrayMap
user=> (def a {0 0 1 1 2 2 3 3 4 4 5 5 6 6 7 7 8 8 9 9})
#'user/a
user=> (class a)
clojure.lang.PersistentHashMap


 Comments   
Comment by Alex Miller [ 15/Nov/14 12:17 PM ]

This might be subsumed under the small collections CLJ-1517, not sure.

Comment by Nicola Mometto [ 08/Dec/14 9:19 AM ]

This is now out of scope for CLJ-1517 now that's focused only on vectors.

Comment by Alex Miller [ 08/Dec/14 9:47 AM ]

We're just splitting the ticket apart, maps will be a separate ticket/patch.





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

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

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


 Description   

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

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

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

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

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

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



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

Duplicate with CLJ-1257 ?

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

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

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

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





[CLJ-1587] PersistentArrayMap's assoc doesn't respect HASHTABLE_THRESHOLD Created: 12/Nov/14  Updated: 12/Nov/14

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: 1
Labels: collections, data-structures, maps

Attachments: Text File 0001-PersistentArrayMap-s-assoc-doesn-t-respect-HASHTABLE.patch    
Patch: Code

 Description   

Currently a map with more than 8 elements will be converted from a PersistentArrayMap to a PersistentHashMap, but if using assoc, it will take 9 elements before the conversion happens:

user=>  (class {0 0 1 1 2 2 3 3 4 4 5 5 6 6 7 7})
clojure.lang.PersistentArrayMap
user=> (class {0 0 1 1 2 2 3 3 4 4 5 5 6 6 7 7 8 8})
clojure.lang.PersistentHashMap
user=>  (class (assoc {0 0 1 1 2 2 3 3 4 4 5 5 6 6 7 7} 8 8))
clojure.lang.PersistentArrayMap
user=>  (class (assoc {0 0 1 1 2 2 3 3 4 4 5 5 6 6 7 7} 8 8 9 9))
clojure.lang.PersistentHashMap

After patch:

user=> (class {0 0 1 1 2 2 3 3 4 4 5 5 6 6 7 7})
clojure.lang.PersistentArrayMap
user=> (class {0 0 1 1 2 2 3 3 4 4 5 5 6 6 7 7 8 8})
clojure.lang.PersistentHashMap
user=> (class (assoc {0 0 1 1 2 2 3 3 4 4 5 5 6 6 7 7} 8 8))
clojure.lang.PersistentHashMap
user=> (class (assoc {0 0 1 1 2 2 3 3 4 4 5 5 6 6 7 7} 8 8 9 9))
clojure.lang.PersistentHashMap





[CLJ-1583] Apply forces the evaluation of one element more than necessary Created: 07/Nov/14  Updated: 09/Nov/14

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

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

Attachments: Text File 0001-make-RT.boundedLength-lazier.patch    
Patch: Code

 Description   

Given a function with one fixed argument and a vararg, it should be sufficient to force evaluation of 2 elements for apply to know which arity it should select, however it currently forces 3:

user=> (defn x ([a & b]))
#'user/x
user=> (apply x (map println (iterate inc 0)))
0
1
2
nil

This makes lazy functions that use apply (for example mapcat) less lazy than they could be.
The proposed patch makes RT.boundedLength short-circuit immediately after the seq count is greater than the max fixed arity:

user=> (defn x ([a & b]))
#'user/x
user=> (apply x (map println (iterate inc 0)))
0
1
nil


 Comments   
Comment by Nicola Mometto [ 09/Nov/14 3:37 PM ]

The patch in this ticket slightly improves the issue reported at http://dev.clojure.org/jira/browse/CLJ-1218





[CLJ-1582] Overriding in-ns and ns is problematic Created: 07/Nov/14  Updated: 07/Nov/14

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

Type: Defect Priority: Major
Reporter: Nicola Mometto Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None

Attachments: Text File 0001-Allow-overriding-of-clojure.core-in-ns-and-clojure.c.patch    
Patch: Code

 Description   

Currently it is possible to override clojure.core/in-ns and clojure.core/ns, but it is not possible to refer to the namespace-specific vars without fully qualifying them:

user=> (ns foo (:refer-clojure :exclude [in-ns]))
nil
foo=> (def in-ns 1)
#'foo/in-ns
foo=> in-ns
#<clojure.lang.RT$1@76b5e4c5>

After this patch, overriding in-ns and ns works like for every other clojure.core var:

user=> (ns foo (:refer-clojure :exclude [in-ns]))
nil
foo=> (def in-ns 1)
#'foo/in-ns
foo=> in-ns
1


 Comments   
Comment by Reid McKenzie [ 07/Nov/14 11:46 AM ]

This is motivated by https://github.com/jonase/eastwood/issues/100





[CLJ-1577] Some hints accept both symbols and class objects, others only symbols Created: 30/Oct/14  Updated: 30/Oct/14

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

Type: Defect Priority: Minor
Reporter: Brandon Bloom Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: typehints


 Description   

In order to hint primitives, such as longs, you can hint with the symbol 'long. In some places, you can also use the class object java.lang.Long/TYPE. However, in some places, that doesn't work. This is particularly problematic when working with hints in macros, where subtle changes to when metadata is evaluated can lead to changes in whether or not hints are respected.

user=> (set! *unchecked-math* :warn-on-boxed)
:warn-on-boxed

user=> (defmacro mac []
         (let [field (with-meta 'x {:tag 'long})]
           (-> field meta :tag class prn)
           `(deftype Foo# [~field]
              clojure.lang.IDeref
              (deref [this#]
                (inc ~(with-meta field nil))))))
#'user/mac

user=> (mac)
clojure.lang.Symbol
#<java.lang.Class@1c76c583 class user.Foo__13651__auto__>

user=> (defmacro mac []
         (let [field (with-meta 'x {:tag java.lang.Long/TYPE})]
           (-> field meta :tag class prn)
           `(deftype Foo# [~field]
              clojure.lang.IDeref
              (deref [this#]
                (inc ~(with-meta field nil))))))
#'user/mac

user=> (mac)
java.lang.Class
Boxed math warning, /private/var/folders/43/mnwlkd2s7r1gbjwq6t__mgt40000gn/T/form-init5463347341158437534.clj:1:1 - call: public static java.lang.Number clojure.lang.Numbers.unchecked_inc(java.lang.Object).
#<java.lang.Class@74626b21 class user.Foo__13663__auto__>





[CLJ-1575] Using a (def ^:const instance) of a deftype that implements IPersistentCollection, triggers compiler errors Created: 29/Oct/14  Updated: 30/Oct/14

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

Type: Defect Priority: Minor
Reporter: Herwig Hochleitner Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

fresh repl


Attachments: Text File 0001-Test-for-analyzer-bug-CLJ-1575.patch    

 Description   

The compiler has a lot of assumptions about the possible types of IPersistentCollection literals and rightfully so. The strange thing with this case is, that taking the (constant) value works as soon as count is defined, but using it as an argument hits a closed dispatch for emitting the empty variants of the various literals.

> (deftype T [] clojure.lang.IPersistentCollection (count [_] 0)
> (def ^:const t (T.))
> (meta t)
java.lang.UnsupportedOperationException: Unknown Collection type
Compiler.java:2860 clojure.lang.Compiler$EmptyExpr.emit
Compiler.java:3632 clojure.lang.Compiler$InvokeExpr.emitArgsAndCall
...

EDIT updated the ticket after some investigation
NOTE attached test patch doesn't even implement (count []) for the deftype, which just triggers a rightful AbstractMethodError



 Comments   
Comment by Herwig Hochleitner [ 29/Oct/14 10:00 PM ]

The test had a typo, sorry

Comment by Alex Miller [ 30/Oct/14 7:14 AM ]

Looks like a variant of CLJ-1093.





[CLJ-1573] Support (Java) transient fields in deftype, e.g. for hashcodes Created: 26/Oct/14  Updated: 26/Oct/14

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

Type: Enhancement Priority: Minor
Reporter: Ghadi Shayban Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: compiler, deftype

Attachments: Text File 0001-transient-field-deftype.patch    
Patch: Code and Test

 Description   

Enhance deftypes to allow fields to be marked ACC_TRANSIENT.

strawman syntax:
(deftype AType [^:transient hash])

Came across this need while experimenting with a reified range written in a deftype, not in Java.

Patch doesn't include docstring change, but has a test.






[CLJ-1570] Core clojure code mixes tabs with spaces Created: 20/Oct/14  Updated: 20/Oct/14

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

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


 Description   

A handful of functions in clojure.core, clojure.core-proxy, clojure.inspector, clojure.xml, clojure.pprint, clojure.stacktrace, clojure.set, and clojure.test switch partway through from indenting with spaces to indenting with tabs. This may cause them to display incorrectly depending on how the developer's editor is configured.

(not sure if this should be marked defect or task)



 Comments   
Comment by Andy Fingerhut [ 20/Oct/14 1:41 PM ]

Some similarities to CLJ-1026, although this problem does not cause the same issues with warnings on git patches as CLJ-1026 does, as far as I know.

One similarity is that if it is of interest (I don't know if it is), Alex or other Clojure screeners may want a procedure to clean them all up, and perhaps repeat that process periodically, e.g. before each major release.





[CLJ-1566] Documentation for clojure.core/require does not document :rename Created: 16/Oct/14  Updated: 19/Oct/14

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.





[CLJ-1563] How About Default Implementations on Protocols Created: 11/Oct/14  Updated: 12/Oct/14

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

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


 Description   

Consider this example

user=> (defprotocol Foo (foo [x] x))
Foo
user=> (defrecord Bar [gaz waka] Foo)
user.Bar
user=> (def bar (Bar. 1 2))
#'user/bar
user=> (.foo bar)

AbstractMethodError user.Bar.foo()Ljava/lang/Object;  sun.reflect.NativeMethodAccessorImpl.invoke0 (NativeMethodAccessorImpl.java:-2)
user=>

What about the default implementation.



 Comments   
Comment by David Williams [ 11/Oct/14 8:48 PM ]

As it stands you have to workaround with this

http://stackoverflow.com/questions/15039431/clojure-mix-protocol-default-implementation-with-custom-implementation

Comment by Jozef Wagner [ 12/Oct/14 1:01 AM ]

I don't think we need it. What's the rationale behind extending some protocol, not implementing its methods, and then calling those methods, expecting them not to throw. Be explicit about what yout type should do, whether it is a default or custom behavior. You basically have three options

(defn default-foo 
  [this] 
  :foo)

(defprotocol P
  (-foo [this]))

(deftype T
  P
  (-foo [this] (default-foo))

(defn foo 
  [x]
  (-foo x))

or

(defprotocol P
  (-foo [this]))

(deftype T)

(defn foo 
  [x]
  (if (satisfies? P x)
    (-foo x)
    :foo))

or

(defprotocol P
  (-foo [this]))

(extend-protocol P
  java.lang.Object
  (-foo [this] :foo))

(deftype T)

(defn foo 
  [x]
  (-foo x))

I think however that my first approach is unidiomatic and you should prefer the latter ones.

Comment by David Williams [ 12/Oct/14 12:36 PM ]

I agree, this is a low priority enhancement. I think it could make the Protocol experience more DWIMy, and Java 8 has default implementations on interfaces for the same kind of convenience.





[CLJ-1560] Forbid closing over mutable fields completely Created: 10/Oct/14  Updated: 10/Oct/14

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

Type: Defect Priority: Major
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





[CLJ-1556] Add instance check functions to defrecord/deftype Created: 09/Oct/14  Updated: 09/Oct/14

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

Type: Enhancement Priority: Minor
Reporter: Nicola Mometto Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: defrecord, deftype

Attachments: Text File 0001-CLJ-1556-Generate-type-functions-with-instance-check.patch    
Patch: Code

 Description   

It is often necessarty to test for instance? on deftypes/defrecords, this patch makes the two macros automatically generate a type? function implemented as (fn [x] (instance? type x)), to complement ->type and map->type
Example:

user=>(deftype x [])
user.x
user=>(x? (x.))
true


 Comments   
Comment by Jozef Wagner [ 09/Oct/14 9:11 AM ]

What about camel cased types? predicate SomeType? does not look like an idiomatic type predicate. I suggest to have this type predicate function and its name optional, through e.g. :predicate metadata on a type name. Moreover, it is far more useful to have such predicate on protocols, rather than types.

Comment by Nicola Mometto [ 09/Oct/14 9:17 AM ]

I don't think camel cased types should pose any issue. we use ->SomeType just as fine, I don't see why SomeType? should be problematic.

I disagree that it's more useful to have a predicate for protocols since protocols are already regular Vars and it's just a matter of (satisfies? theprotocol x), the value of the predicate on types/record is to minimize the necessity of having to import the actual class





[CLJ-1550] Classes generated by deftype and defrecord don't play nice with .getPackage Created: 07/Oct/14  Updated: 07/Oct/14

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

Type: Defect Priority: Major
Reporter: Bozhidar Batsov Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: bug


 Description   
(.getPackage String)
;; => #<Package package java.lang, Java Platform API Specification, version 1.7>
(deftype T [])
(.getPackage T)
;; => nil

This seems like a bug to me as it's not obvious why the class generated by deftype should exhibit different behaviour.



 Comments   
Comment by Alex Miller [ 07/Oct/14 8:54 AM ]

According to http://docs.oracle.com/javase/7/docs/api/java/lang/Class.html#getPackage() this method returns the package information found by the class loader or null if there is none. Its not clear to me that the current behavior is wrong per the spec. I would need to experiment more to see if this is unusual or not.

Comment by Bozhidar Batsov [ 07/Oct/14 9:05 AM ]

A bit of background for the issue. I'm no expert on the topic, but being able to procure all the class information except its package definitely looks strange to me.

Comment by Kevin Downey [ 07/Oct/14 11:46 AM ]

if you AOT compile(generate a class file on disk for a deftype), getPackage works fine, which suggests to me it is a jvm issue

Comment by Kevin Downey [ 07/Oct/14 11:49 AM ]

actually, it must just be that dynamicclassloader doesn't define a package for classes it loads

Comment by Alex Miller [ 07/Oct/14 12:13 PM ]

Yep, I believe that's correct.





[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: 0
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: 01/Oct/14

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

Type: Enhancement Priority: Major
Reporter: Kuldeep Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: enhancement, macro

Attachments: File condp-threading-macros-25sept2014.diff    
Patch: Code

 Description   

After introduction of cond-> and cond->> macros in 1.5. It makes sense to have condp-> and condp->> macros in the core library.

(condp-> {}
(complement :a) (assoc :a 1)
:a (assoc :b 2)) ;=> {:b 2, :a 1}

In the above example the result of each expr which was evaluated is being passed to the next predicate.



 Comments   
Comment by Andy Fingerhut [ 01/Oct/14 6:37 PM ]

Kuldeep, I cannot comment on whether this change is of interest to the Clojure developers, because I do not know.

I can say that the patch you have attached is not in the expected format. See the page below for instructions on creating a patch in the expected format:

http://dev.clojure.org/display/community/Developing+Patches





[CLJ-1532] pr-str captures stdout from printing side-effects of lazily evaluated expressions. Created: 23/Sep/14  Updated: 23/Sep/14

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

Type: Defect Priority: Minor
Reporter: Silas Davis Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: print
Environment:

Linux



 Description   

Because clojure.core/pr-str uses with-out-str to capture the output of pr (and pr cannot be parsed a writable thing - just uses out).

If you pr-str the result of something lazy you can get side-effects written to stdout with println interspersed with the output. For example in my case I was extracting benchmarks from the library criterium and trying to print the data structure to the file. The solution would be to provide an overload of pr/pr-str that takes a writer. I note that pr-on provides some of the functionality but it is private.

This is an ugly bug when you're trying to persist program output in EDN, because the randomly interspersed stdout messages make it invalid for read-string. We shouldn't need our functions to be pure for pr-str to work as expected.

I've omitted a patch because although I think a fix is straight-forward I'm not sure quite where it should go (e.g. make pr-on public, change pr, change pr-str)






[CLJ-1530] Make foo/bar/baz unreadable Created: 22/Sep/14  Updated: 28/Oct/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-fix-LispReader-and-EdnReader-so-that-foo-bar-baz-is-.patch    
Patch: Code and Test

 Description   

Currently keywords and symbols containing more than one slash are disallowed by the spec, but allowed by the readers.
This trivial patch makes them unreadable by the readers too.

Pre:

user=> :foo/bar/baz
:foo/bar/baz

Post:

user=> :foo/bar/baz
RuntimeException Invalid token: :foo/bar/baz  clojure.lang.Util.runtimeException (Util.java:221)


 Comments   
Comment by Andy Fingerhut [ 22/Sep/14 12:14 PM ]

Perhaps overlap with CLJ-1527 ?

Comment by Thomas Engelschmidt [ 28/Oct/14 4:36 AM ]

Please notice that keywords with more than one slash has a different hashcode across clojure version 1.5 and 1.6

This creates a problem when using a datomic version that works with clojure 1.5 under clojure 1.6 and the schema have one or more keys with more than one slash.





[CLJ-1526] clojure.core/> inconsistent behavior wrt to documentation. Created: 17/Sep/14  Updated: 22/Sep/14

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

Type: Enhancement Priority: Minor
Reporter: Phillip Lord Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: math


 Description   

The > function is inconsistent wrt to their behaviour for 0 arity.

user> (doc >)
-------------------------
clojure.core/>
([x] [x y] [x y & more])
  Returns non-nil if nums are in monotonically decreasing order,
  otherwise false.
nil
user> (> 3 2)
true
user> (> 3)
true
user> (>)
ArityException Wrong number of args (0) passed to: core/>  clojure.lang.AFn.throwArity (AFn.java:429)

This is mostly likely to become problematic when using > via apply where

(or (= 0 (count l))
    (apply > l))

It seems that the documentation should be updated, 0-arg case should return true, or the 1-arg case should also throw an exception.

This affects the other comparators also.



 Comments   
Comment by Robert Tweed [ 17/Sep/14 9:48 AM ]

As per my original post on this (here: https://groups.google.com/d/msg/clojure/8zkpO9FBN64/u2LAQsR93IgJ), while the question of whether an empty set has monotonic order perhaps has more than one answer in theory, from a purely pragmatic engineering perspective, it makes the most sense to evaluate to true here.

This /should/ not be a breaking change. Therefore it is fairly safe to introduce into a minor revision. It's a also a trivial fix. But it is possible (though highly unlikely) that someone could have code that depends on the exception being raised at runtime (as it does now) to handle empty lists in some special way. Such code is horrible and ought to be rewritten, so should not be seen as justification for retaining the current behaviour, which limits the general usefulness of these functions and may be responsible for subtle bugs in existing production code.

However such a change should probably not be backported to existing 1.6.x branches, just to be 100% safe, since it is not a security issue. My suggestion therefore would be to add a note to the docs in existing maintenance branches (any future 1.6.x) and evaluate to true in future versions (1.7+).





[CLJ-1523] Add 'doseq' like macro for transducers Created: 08/Sep/14  Updated: 09/Sep/14

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

Type: Enhancement Priority: Trivial
Reporter: Timothy Baldridge Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: None

Attachments: File doreduced2.diff     File doreduced.diff    
Patch: Code and Test

 Description   

Doseq is currently a good way to execute a lazy sequence and perform side-effects. It would be nice to have a matching macro for transducers.

Approach: The included patch simply calls transduce with the provided xform, collection, and a reducing function that throws away the accumulated value at each step. The value from each reducing step is bound to the provided symbol. A shorter arity is provided for those cases when no xform is desired, but fast doseq-like semantics are still wanted.

Patch: doreduced2.diff



 Comments   
Comment by Jozef Wagner [ 09/Sep/14 4:19 AM ]

How about making xform parameter optional? And you have a typo in docstring example, doseq -> doreduced.

Comment by Timothy Baldridge [ 09/Sep/14 7:52 AM ]

Good point, fixed typeo, added other arity.





[CLJ-1522] Enhance multimethods metadata Created: 08/Sep/14  Updated: 09/Sep/14

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

Type: Enhancement Priority: Minor
Reporter: Bozhidar Batsov Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: metadata


 Description   

I think that multimethod metadata can be extended a bit with some property indicating the var in question is referring to a multimethod (we have something similar for macros) and some default arglists property.

I'm raising this issue because as a tool writer (CIDER) I'm having hard time determining if something is a multimethod (I have to resort to code like (instance? clojure.lang.MultiFn obj) which is acceptable, but not ideal I think (compared to macros and special forms)). There's also the problem that I cannot provide the users with eldoc (function signature) as it's not available in the metadata (this issue was raised on the mailing list as well https://groups.google.com/forum/#!topic/clojure/crje_RLTWdk).

I feel that we really have a problem with the missing arglist and we should solve it somehow. I'm not sure I'm suggesting the best solution and I'll certainly take any solution.



 Comments   
Comment by Bozhidar Batsov [ 09/Sep/14 4:24 AM ]

Btw, I failed to mention this as I thought it was obvious, but I think we should use the dispatch function's arglist in the multimethod metadata.





[CLJ-1521] A little improvement for parsing let expr Created: 07/Sep/14  Updated: 08/Sep/14

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

Type: Enhancement Priority: Trivial
Reporter: dennis zhuang Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: let, parser
Environment:

Mac OSX 10.9.4
java version "1.7.0_17"
Java(TM) SE Runtime Environment (build 1.7.0_17-b02)
Java HotSpot(TM) 64-Bit Server VM (build 23.7-b01, mixed mode)


Attachments: File improve_parse_let_expr.diff    
Patch: Code

 Description   

The recurMismatches vector in LetExpr parser as see in

https://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/Compiler.java#L6062-6065

There is not necessary to add initialize value 'false' into it when it is not a loop expression.

We can rewrite it into:

if(isLoop)
			    {
				for (int i = 0; i < bindings.count()/2; i++)
				    {
				    recurMismatches = recurMismatches.cons(RT.F);
				    }				
			    }

It's a little improvement for parsing let expression.



 Comments   
Comment by Andy Fingerhut [ 07/Sep/14 11:16 AM ]

Dennis, you might want to clarify the description a little bit, if I understand this ticket correctly. The proposed change would be no change to the behavior of the compiler, except a small speed improvement during compilation?

Comment by dennis zhuang [ 08/Sep/14 2:36 AM ]

Yep,the patch doesn't change the behavior of the compiler.All test is fine.

The recurMismatches vector in LetExpr parser as see in

https://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/Compiler.java#L6062-6065

is only used when detecting type mismatch for loop special form,it's not necessary to be initialized for let special form.So i just added a if(isLoop) clause before initializing it.





[CLJ-1520] assoc-in with empty key path assoc-es to nil Created: 05/Sep/14  Updated: 05/Sep/14

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

Type: Defect Priority: Major
Reporter: Francis Avila Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   
(assoc-in {} [] 1) ;=> {nil 1}

This should probably throw an exception.

CLJ-373 has a patch (CLJ-373-nested-ops.patch) which fixes this (by throwing an exception on empty key paths), the related broken behavior of update-in, and documents empty key path behavior in get-in et al. I can pull just the assoc-in stuff out of that into a separate patch, but I am really hoping that all the issues in the patch addresses are resolved at once, I.e.:

(get-in {} [] :notfound) ;=> {} ; ok
(get-in {nil 1} [] :notfound) ;=> {nil 1} ; ok
(assoc-in {} [] 1) ;=> {nil 1} ; wat?
(assoc-in {nil 0} [] 1) ;=> {nil 1} ; wat?
(update-in {} [] identity) ;=> {nil nil} ; wat?
(update-in {nil 0} [] inc) ;=> {nil 1} ; wat?





[CLJ-1519] Added extra arity to clojure.core/ns-* fns Created: 04/Sep/14  Updated: 10/Sep/14

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

Type: Enhancement Priority: Major
Reporter: Alex Baranosky Assignee: Unassigned
Resolution: Unresolved Votes: 3
Labels: enhancement, patch

Attachments: Text File new-ns-arity.patch    
Patch: Code and Test

 Description   

Hello,

Adds another arity where the "ns" parameter is set to a default value of *ns* in these fns:

ns-unmap, ns-resolve, ns-name, ns-map, ns-publics, ns-imports, ns-interns, ns-refers, ns-aliases, ns-unalias

I find I very often use ns-unalias and ns-unmap from the repl, and passing the *ns* arg gets a little tedious.






[CLJ-1514] Use qualified class names for return type hints of standard Clojure functions Created: 28/Aug/14  Updated: 28/Aug/14

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

Type: Enhancement Priority: Minor
Reporter: Tassilo Horn Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: enhancement, interop, patch, typehints

Attachments: Text File 0001-Use-fully-qualified-class-names-for-return-type-hint.patch    
Patch: Code

 Description   

The attached patch converts all function return type hints to spell out the class name fully qualified. There are two reasons for doing this:

1. Simple names in return type hints cause the issue described in http://dev.clojure.org/jira/browse/CLJ-1232. That's usually not a problem with return type hints referring to java.lang-classes because those are always imported. However, using `ns-unmap` you can remove them. For example, after `(ns-unmap ns 'String)` in my namespace, `(.length (format "foo = %s") 1)` throws an IllegalArgumentException: Unable to resolve classname: String. By using fully-qualified class names, that problem goes away.

2. tools.analyzer (used by the Clojure lint tool Eastwood) crashes when encountering such a simple-named return type hint. So currently, I cannot lint parts of my project because there's code that calls `clojure.core/format`.



 Comments   
Comment by Alex Miller [ 28/Aug/14 9:34 AM ]

1. that seems like a pretty weird thing to do
2. sounds like an issue with tools.analyzer, not with Clojure?

Comment by Nicola Mometto [ 28/Aug/14 10:46 AM ]

Just to clarify, tools.analyzer(.jvm) can analyze just fine forms in the form (defn x ^Class []) as long as Class is resolvable, whereas it will throw an exception if that function is then used in a namespace where that class is no longer resolvable, which is similar to what Clojure already does, except tools.analyzer.jvm will throw an exception even if the type hint is not used.

Since version 0.5.1 there's an handler that can be provided to change that behaviour, see https://github.com/clojure/tools.analyzer.jvm/blob/master/src/main/clojure/clojure/tools/analyzer/passes/jvm/validate.clj#L232

Comment by Nicola Mometto [ 28/Aug/14 11:02 AM ]

Now a comment regarding this ticket: the patch in this ticket is just a work-around for the issue exposed in http://dev.clojure.org/jira/browse/CLJ-1232, IMHO the correct move would be to actually recognize that issue as a bug rather than as an accepted "limitation" as Rich's comment seems to suggest so that a fix might be commited.

Comment by Tassilo Horn [ 28/Aug/14 1:29 PM ]

@Alex: 1. is not as weird as it sounds at first. For example, consider you have macros that generate complete APIs for something into some new namespace. Then it can make sense to use a real vanilla namespace, i.e., without referring clojure.core and importing java.lang. With 2. I side with Nicola and consider CLJ-1232 a bug.

@Nicola: Today I've used Eastwood (0.1.4) to lint my project. It crashed when it encountered this definition:

(defmacro error
  "Throws an exception with the given message and cause."
  ([msg]
     `(error ~msg nil))
  ([msg cause]
     `(throw (java.lang.Exception. ~msg ~cause))))

(defmacro errorf
  "Throws an exception with the given `msg` and `objs` passed to `format`.
  `msg` is a format string."
  [msg & objs]
  `(error (format ~msg ~@objs)))  ;; This is line 112 where the crash occurs

The message was:

Exception thrown during phase :analyze+eval of linting namespace funnyqt.tg-test
A function, macro, protocol method, var, etc. named clojure.core/format has been used here:
{:file "funnyqt/utils.clj",
 :end-column 19,
 :column 12,
 :line 112,
 :end-line 112}
Wherever it is defined, or where it is called, it has a type of String
This appears to be a Java class name with no package path.
Library tools.analyzer, on which Eastwood relies, cannot analyze such files.
If this definition is easy for you to change, we recommend you prepend it with
a full package path name, e.g. java.net.URI
Otherwise import the class by adding a line like this to your ns statement:
    (:import (java.net URI))

An exception was thrown while analyzing namespace funnyqt.tg-test 
Lint results may be incomplete.  If there are compilation errors in
your code, try fixing those.  If not, check above for info on the
exception.

So it seems it crashes because `format` has a `^String` return type hint. The namespace containing the `errorf` macro above has no modified ns-imports, i.e., all java.lang classes are imported there, too.

Comment by Nicola Mometto [ 28/Aug/14 1:46 PM ]

Tassilo, since `errorf` is a macro, that error is probably caused at the expansion point of that macro in a namespace that unmaps 'String.
If that's not the case, please open a ticket in the eastwood repo

Comment by Tassilo Horn [ 28/Aug/14 2:16 PM ]

Nicola, you are correct. As I've explained above to Alex, I generate APIs in fresh namespaces that don't refer clojure.core and also ns-unmap all java.lang classes, and the generated code also contains `errorf`-forms.

Well, since `ns-unmap` is there, I think it's legit to use it. So that makes CLJ-1232 even more important. But until that gets fixed which requires a common agreement that it is indeed a bug, I'd be very happy if this patch could be accepted. I mean, when it cannot do any harm and doesn't obscure anything but helps at least one person, then why not do it?





[CLJ-1513] Enhancing reader Created: 25/Aug/14  Updated: 25/Aug/14

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

Type: Enhancement Priority: Trivial
Reporter: Anton Rambold Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: edn, reader


 Description   

Attach "character start" and "character end" to the meta information of read forms produced by clojure.lang.EdnReader and clojure.lang.LispReader.
This will allows for better code inspection by linters for example. Currently only line number and column are attached to the meta information.



 Comments   
Comment by Andy Fingerhut [ 25/Aug/14 4:59 PM ]

I am not certain, but perhaps the EDN and regular reader in the tools.reader contrib library already do what you want here? That is, besides :line and :column metadata, they also have :end-line and :end-column metadata for the end of the expression.





[CLJ-1508] Supplied-p parameter in clojure Created: 18/Aug/14  Updated: 18/Aug/14

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

Type: Enhancement Priority: Minor
Reporter: dennis zhuang Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: destructuring
Environment:

Mac OSX 10.9.4

java version "1.7.0_17"
Java(TM) SE Runtime Environment (build 1.7.0_17-b02)
Java HotSpot(TM) 64-Bit Server VM (build 23.7-b01, mixed mode)


Attachments: File supplied_p.diff    
Patch: Code and Test

 Description   

As see in https://groups.google.com/forum/?hl=en#!topic/clojure/jWc51JOkvsA

I think we can add a ? option for destructure ,then we can write a test like :

(deftest supplied-p-in-destructuring
  (let [{:keys [a b c d] :p? {a a-p? b b-p? c c-p? d d-p?} :or {a 1}} {:b 2 :c 3 }]
    (is (= a 1))
    (is (false? a-p?))
    (is (= 2 b))
    (is (true? b-p?))
    (is (= 3 c))
    (is (true? c-p?))
    (is (nil? d))
    (is (false? d-p?))))

Even if the a var has a default value 1 by :or option,but the a-p? is still false.
Just like the supplied-p-parameter in Commons LISP.

The patch is attached with code and test.



 Comments   
Comment by Steve Miner [ 18/Aug/14 8:24 AM ]

As mentioned on the mailing list, you could use {:as arg} destructuring to get same information. Here's a slightly modified example that works in the current Clojure:

(deftest supplied-p-in-destructuring
  ;; (let [{:keys [a b c d] :p? {a a-p? b b-p? c c-p? d d-p?} :or {a 1}} {:b 2 :c 3 }]
  (let [{:keys [a b c d] :or {a 1} :as argmap} {:b 2 :c 3 }
        supplied? (partial contains? argmap)
        a-p? (supplied? :a)
        b-p? (supplied? :b)
        c-p? (supplied? :c)
        d-p? (supplied? :d)]
    (is (= a 1))
    (is (false? a-p?))
    (is (= 2 b))
    (is (true? b-p?))
    (is (= 3 c))
    (is (true? c-p?))
    (is (nil? d))
    (is (false? d-p?))))




[CLJ-1507] Throw NPE in eval reader Created: 16/Aug/14  Updated: 16/Aug/14

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

Type: Defect Priority: Minor
Reporter: dennis zhuang Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: eval-reader
Environment:

Mac OSX 10.9.4
java version "1.7.0_17"
Java(TM) SE Runtime Environment (build 1.7.0_17-b02)
Java HotSpot(TM) 64-Bit Server VM (build 23.7-b01, mixed mode)


Attachments: File fix_npe_eval_reader.diff    
Patch: Code

 Description   
Clojure 1.7.0-master-SNAPSHOT
user=> #=(var a)
NullPointerException   clojure.lang.Symbol.hashCode (Symbol.java:84)
user=> (.printStackTrace *e)
clojure.lang.LispReader$ReaderException: clojure.lang.LispReader$ReaderException: java.lang.NullPointerException
	at clojure.lang.LispReader.read(LispReader.java:218)
	at clojure.core$read.invoke(core.clj:3580)
	at clojure.core$read.invoke(core.clj:3578)
	at clojure.core$read.invoke(core.clj:3576)
	at clojure.core$read.invoke(core.clj:3574)
	at clojure.main$repl_read.invoke(main.clj:139)
	at clojure.main$repl$read_eval_print__6807$fn__6808.invoke(main.clj:237)
	at clojure.main$repl$read_eval_print__6807.invoke(main.clj:237)
	at clojure.main$repl$fn__6816.invoke(main.clj:257)
	at clojure.main$repl.doInvoke(main.clj:257)
	at clojure.lang.RestFn.invoke(RestFn.java:421)
	at clojure.main$repl_opt.invoke(main.clj:323)
	at clojure.main$main.doInvoke(main.clj:421)
	at clojure.lang.RestFn.invoke(RestFn.java:397)
	at clojure.lang.Var.invoke(Var.java:375)
	at clojure.lang.AFn.applyToHelper(AFn.java:152)
	at clojure.lang.Var.applyTo(Var.java:700)
	at clojure.main.main(main.java:37)
Caused by: clojure.lang.LispReader$ReaderException: java.lang.NullPointerException
	at clojure.lang.LispReader.read(LispReader.java:218)
	at clojure.lang.LispReader$CtorReader.invoke(LispReader.java:1164)
	at clojure.lang.LispReader$DispatchReader.invoke(LispReader.java:609)
	at clojure.lang.LispReader.read(LispReader.java:183)
	... 17 more
Caused by: java.lang.NullPointerException
	at clojure.lang.Symbol.hashCode(Symbol.java:84)
	at java.util.concurrent.ConcurrentHashMap.hash(ConcurrentHashMap.java:332)
	at java.util.concurrent.ConcurrentHashMap.get(ConcurrentHashMap.java:987)
	at clojure.lang.Namespace.findOrCreate(Namespace.java:173)
	at clojure.lang.RT.var(RT.java:341)
	at clojure.lang.LispReader$EvalReader.invoke(LispReader.java:1042)
	at clojure.lang.LispReader$DispatchReader.invoke(LispReader.java:616)
	at clojure.lang.LispReader.read(LispReader.java:183)
	... 20 more

If the var symbol doesn't contains namespace ,it will throw the NPE exception in above code.Instead,i think it should use Compiler.currentNS() when doesn't find the var's namespace.

The patch is attached, after patched:

Clojure 1.7.0-master-SNAPSHOT
user=> #=(var a)
#'user/a





[CLJ-1506] A little improvement when reading syntax quote form Created: 16/Aug/14  Updated: 30/Aug/14

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

Type: Enhancement Priority: Minor
Reporter: dennis zhuang Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: syntax-quote
Environment:

Mac OSX 10.9.4
java version "1.7.0_17"
Java(TM) SE Runtime Environment (build 1.7.0_17-b02)
Java HotSpot(TM) 64-Bit Server VM (build 23.7-b01, mixed mode)


Attachments: File fast_syntax_quote_reader.diff    
Patch: Code

 Description   

When reading syntax quote on keyword,string or number etc,it returns the form as result directly. Read it in:
https://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/LispReader.java#L844-847

else if(form instanceof Keyword
       || form instanceof Number
       || form instanceof Character
       || form instanceof String)
   ret = form;

But missing check if it is a nil,regular pattern or boolean constants.
After patched:

else if(form == null
       || form instanceof Keyword
       || form instanceof Number
       || form instanceof Character
       || form instanceof Pattern
       || form instanceof Boolean
       || form instanceof String)
    ret = form;

It's a little patch, i am not sure if it is worth a try.






[CLJ-1504] Add :inline to most core predicates Created: 15/Aug/14  Updated: 15/Aug/14

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

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

Attachments: Text File 0001-add-inline-to-some-core-predicates.patch    
Patch: Code

 Description   

This will allow instance? predicates calls to be emitted using the instanceof JVM bytecode and will also allow tools like core.typed or tools.analyzer.jvm to infer the type of a var/local on a per branch basis without having to special-case all the core predicates.



 Comments   
Comment by Jozef Wagner [ 15/Aug/14 1:32 PM ]

Related ticket CLJ-1227 and related quote from Alex:

definline is considered to be an experimental feature and Rich would like to discourage its use as the hope is to remove it in the future. The desired replacement is something like common lisp compiler macros that could allow the compiler to detect special situations and optimize the result but leave behind a function invocation for the case where no special behavior is available.

Comment by Nicola Mometto [ 15/Aug/14 1:42 PM ]

This patch uses "manual" :inline metadata on functions, it's used by many other core functions (like +,- et), not definline so Rich's comment doesn't apply.





[CLJ-1502] Clojure Inspector navigation error Created: 12/Aug/14  Updated: 15/Aug/14

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

Type: Defect Priority: Minor
Reporter: Dan Campbell Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: bug, inspector, navigation
Environment:

Windows 7 and 8, Java 7, Clojure repl


Attachments: Text File clj-1502-v1.patch    
Patch: Code

 Description   

With Clojure 1.6.0 on some platforms (details below), if you create an object such as

(def nst (vec '((3 7 22) 99 (123 18 225 437))))

and then you inspect the tree representing the object

(inspect-tree nst)

Most of the navigation with the keyboard proceeds fine. However, when you point to an individual value - e.g. the 99 or the 437 - and press the right arrow key, there is an error

Exception in thread "AWT-EventQueue-0" java.lang.UnsupportedOperationException: count not supported on this type: Long
	at clojure.lang.RT.countFrom(RT.java:556)
	at clojure.lang.RT.count(RT.java:530)
	at clojure.inspector$fn__6907.invoke(inspector.clj:40)
	at clojure.lang.MultiFn.invoke(MultiFn.java:227)
	at clojure.inspector$tree_model$fn__6929.invoke(inspector.clj:63)
	at clojure.inspector.proxy$java.lang.Object$TreeModel$775afa87.getChildCount(Unknown Source)
	at javax.swing.plaf.basic.BasicTreeUI$Actions.traverse(BasicTreeUI.java:4395)
	at javax.swing.plaf.basic.BasicTreeUI$Actions.actionPerformed(BasicTreeUI.java:4052)
	at javax.swing.SwingUtilities.notifyAction(SwingUtilities.java:1662)
	at javax.swing.JComponent.processKeyBinding(JComponent.java:2878)
	at javax.swing.JComponent.processKeyBindings(JComponent.java:2925)
	at javax.swing.JComponent.processKeyEvent(JComponent.java:2841)
	at java.awt.Component.processEvent(Component.java:6282)
	at java.awt.Container.processEvent(Container.java:2229)
	at java.awt.Component.dispatchEventImpl(Component.java:4861)
	at java.awt.Container.dispatchEventImpl(Container.java:2287)
	at java.awt.Component.dispatchEvent(Component.java:4687)
	at java.awt.KeyboardFocusManager.redispatchEvent(KeyboardFocusManager.java:1895)
	at java.awt.DefaultKeyboardFocusManager.dispatchKeyEvent(DefaultKeyboardFocusManager.java:762)
	at java.awt.DefaultKeyboardFocusManager.preDispatchKeyEvent(DefaultKeyboardFocusManager.java:1027)
	at java.awt.DefaultKeyboardFocusManager.typeAheadAssertions(DefaultKeyboardFocusManager.java:899)
	at java.awt.DefaultKeyboardFocusManager.dispatchEvent(DefaultKeyboardFocusManager.java:727)
	at java.awt.Component.dispatchEventImpl(Component.java:4731)
	at java.awt.Container.dispatchEventImpl(Container.java:2287)
	at java.awt.Window.dispatchEventImpl(Window.java:2719)
	at java.awt.Component.dispatchEvent(Component.java:4687)
	at java.awt.EventQueue.dispatchEventImpl(EventQueue.java:735)
	at java.awt.EventQueue.access$200(EventQueue.java:103)
	at java.awt.EventQueue$3.run(EventQueue.java:694)
	at java.awt.EventQueue$3.run(EventQueue.java:692)
	at java.security.AccessController.doPrivileged(Native Method)
	at java.security.ProtectionDomain$1.doIntersectionPrivilege(ProtectionDomain.java:76)
	at java.security.ProtectionDomain$1.doIntersectionPrivilege(ProtectionDomain.java:87)
	at java.awt.EventQueue$4.run(EventQueue.java:708)
	at java.awt.EventQueue$4.run(EventQueue.java:706)
	at java.security.AccessController.doPrivileged(Native Method)
	at java.security.ProtectionDomain$1.doIntersectionPrivilege(ProtectionDomain.java:76)
	at java.awt.EventQueue.dispatchEvent(EventQueue.java:705)
	at java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:242)
	at java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:161)
	at java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:150)
	at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:146)
	at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:138)
	at java.awt.EventDispatchThread.run(EventDispatchThread.java:91)

Environments where this has been reproduced:
+ Windows 7 Enterprise, SP1, Oracle JDK 1.7.0_51, Clojure 1.6.0
+ Ubuntu Linux 14.04.1, Oracle JDK 1.7.0_65, Clojure 1.6.0

Environments where the same sequence of events does not cause an exception:
+ Mac OS X 10.8.5, Oracle JDK 1.7.0_51, Clojure 1.6.0



 Comments   
Comment by Andy Fingerhut [ 13/Aug/14 6:08 PM ]

Patch clj-1502-v1.patch avoids the exception in the situation reported. Tested manually on OS X, Linux, and Windows 7 versions mentioned in the patch comment. I suspect it is not worth the effort to write an automated test for this.

Comment by Dan Campbell [ 15/Aug/14 6:40 PM ]

Thanks, Andy

  • DC




[CLJ-1496] Added a new arity to 'ex-info' that only accepts a message. Created: 08/Aug/14  Updated: 11/Aug/14

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

Type: Enhancement Priority: Minor
Reporter: dennis zhuang Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: ex-info, exceptions
Environment:

java version "1.7.0_17"
Java(TM) SE Runtime Environment (build 1.7.0_17-b02)
Java HotSpot(TM) 64-Bit Server VM (build 23.7-b01, mixed mode)

Mac OSX 10.9.4


Attachments: File ex_info_arity.diff    
Patch: Code

 Description   

We often use 'ex-info' to throw a custom exception.But ex-info at least accepts two arguments: a string message and a data map.
In most cases,but we don't need to throw a exception that taken a data map.
So i think we can add a new arity to ex-info:

(ex-info "the exception message")

That created a ExceptionInfo instance carries empty data.

I am not sure it's useful for other people,but it's really useful for our developers.

The patch is attached.






[CLJ-1489] Implement var-symbol Created: 02/Aug/14  Updated: 06/Aug/14

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

Type: Enhancement Priority: Trivial
Reporter: Reid McKenzie Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File 0001-Implement-var-symbol.patch    

 Description   

var-symbol provides the obvious complement operation to resolve. Where resolve maps from a symbol to a var by resolving it in the environment, var-symbol allows a user to recover the root binding symbol from a var if the var is named. If the var is not named, var-symbol returns nil.

This is related to CLJ-1488 in that it handles the common case of symbolically manipulating Vars in terms of the Symbols they bind without requiring that users manually reconstruct the bound symbol. Futhermore this patch nicely handles the non-obvious implementation consequent case of an unnamed var.

Depends on CLJ-1488



 Comments   
Comment by Andy Fingerhut [ 06/Aug/14 2:30 PM ]

Patch 0001-Implement-var-symbol.patch dated Aug 2 2014 does not apply cleanly. I haven't checked whether it used to apply cleanly before some commits made to Clojure master earlier today, but if it did, then those commits have made this patch become 'stale'.

See the section "Updating stale patches" at http://dev.clojure.org/display/community/Developing+Patches for suggestions on how to update patches.





[CLJ-1486] Make fnil var-arg Created: 31/Jul/14  Updated: 18/Sep/14

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

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

Attachments: Text File 0001-make-fnil-vararg.patch    
Patch: Code and Test

 Description   

Currently fnil is defined only for 1 to 3 args, this patch makes it var-arg






[CLJ-1482] Replace a couple of (filter (complement ...) ...) usages with (remove ...) Created: 27/Jul/14  Updated: 27/Jul/14

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

Type: Enhancement Priority: Trivial
Reporter: Bozhidar Batsov Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: enhancement

Attachments: Text File 0001-Replace-a-couple-of-filter-complement-usages-with-re.patch    
Patch: Code

 Description   

The title basically says it all - remove exists so we can express our intentions more clearly.






[CLJ-1471] Option to print type info Created: 21/Jul/14  Updated: 21/Jul/14

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

Type: Enhancement Priority: Minor
Reporter: Pascal Germroth Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: pprint


 Description   

I've had an issue with defrecord-types being converted into ordinary maps somewhere, which was relatively hard to track down inside a deep structure since they are pprinted as the same thing by default.
The following code patches into the pprint dispatch and prints the type around values; it turned out to be quite useful, but feels hackish.
Maybe something like that would be useful to integrate into clojure.pprint directly (there are a number of cosmetic options already), i.e. into clojure.pprint/write-out.

Only printing (type) may not be enough in some cases; so an option to print all metadata would be nice.
Maybe something like :metadata nil as default, :metadata :type to print types (but also for non-IMetas, using (type) and :metadata true to print metadata for IMetas using (meta).

(defn pprint-with-type
  ([object] (pprint object *out*))
  ([object writer]
   ; keep original dispatch.
   ; calling it directly will print only that object,
   ; but return to our dispatch for subobjects.
   (let [dispatch clojure.pprint/*print-pprint-dispatch*]
     (binding [clojure.pprint/*print-pprint-dispatch*
               (fn [obj]
                 (if (instance? clojure.lang.IMeta obj)
                   (do (print "^{:type ")
                       (dispatch (type obj))
                       (print "} ")
                       (clojure.pprint/pprint-newline :fill)
                       (dispatch obj))
                   (do (print "(^:type ")
                       (dispatch (type obj))
                       (print " ")
                       (clojure.pprint/pprint-newline :fill)
                       (dispatch obj)
                       (print ")"))))]
       (clojure.pprint/pprint object writer)))))





[CLJ-1470] Make Atom and ARef easy to subclass Created: 20/Jul/14  Updated: 23/Jul/14

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

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

Attachments: Text File CLJ-1470-v1.patch    
Patch: Code

 Description   

Atom is currently defined as final and ARef.validate() is package-private. This makes it impossible to define a subclass of an Atom and difficult subclass ARef (if validate() needs to be called).

I propose removing the final modifier from Atom, making ARef.validate() protected and also making Atom.state protected (it is currently package-private).

I'm not sure if there is a specific reason why Atom is final - if this is for performance reasons or to prevent someone from doing strange things with Atom's, but I can see a use case for sub-classing it.

One use-case is to create reactive Atom that allows derefs to be tracked (as in reagent). I have some Clojure (not Clojurescript) code where I'm trying to play with this idea and I've had to copy the entire Atom class (because it's sealed) and place it in the clojure.lang package (because ARef.validate() is package-private): https://github.com/aaronc/freactive/blob/master/src/java/clojure/lang/ReactiveAtom.java. In addition, I need to copy the defns for swap! and reset! into my own namespace. This seems a bit inconvenient.



 Comments   
Comment by Kevin Downey [ 23/Jul/14 12:55 AM ]

related to http://dev.clojure.org/jira/browse/CLJ-803





[CLJ-1469] Emit KeywordInvoke callsites only when keyword is not namespaced Created: 18/Jul/14  Updated: 22/Oct/14

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

Type: Enhancement Priority: Trivial
Reporter: Ghadi Shayban Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File kwinvoke.patch    
Patch: Code

 Description   

Summary: Don't emit KeywordLookup thunks and machinery for namespaced keyword access

Description: When the compiler sees a keyword at the beginning of a sexpr, (:foo x), it emits some machinery that takes into account that 'x' could be a defrecord with a defined 'foo' field. This exists to fast-path it into a field lookup. Here is the supporting code from the target defrecord: https://github.com/clojure/clojure/blob/master/src/clj/clojure/core_deftype.clj#L185-L198
The compiler currently emits the same machinery for (:foo/bar x), a namespaced keyword access, but defrecords don't have any fast path field access for that. This trivial patch turns that scenario into a normal invocation.

Here is the disassembly for (fn [x] (:foo/bar x))
https://gist.github.com/anonymous/d94fc56fba4a1665f73f

There are two static fields on the IFn also for every kw access.

With the trivial patch, it turns into a normal invoke. (emit the fn aka the namespaced keyword, then the args Aka the target, and call IFn invoke: kw.invoke(target))






[CLJ-1467] Implement Comparable in PersistentList Created: 17/Jul/14  Updated: 14/Nov/14

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

Type: Enhancement Priority: Major
Reporter: Pascal Germroth Assignee: Unassigned
Resolution: Unresolved Votes: 4
Labels: collections

Attachments: Text File 0001-first-try-for-adding-compare.patch    

 Description   

PersistentVector implements Comparable already.



 Comments   
Comment by Bart Kastermans [ 13/Nov/14 11:17 AM ]

Patch for this issue; done with Jeroen van Dijk and Razvan Petruescu at a clojure meetup. Any feedback welcome; the learning for me here is not the fix, but learning how to deal with ant and jira etc.

Comment by Andy Fingerhut [ 13/Nov/14 12:31 PM ]

Looks like you have navigated the steps for creating a patch in the desired format, and attaching it to a JIRA ticket, just fine. I see your name on the list of contributors, which is a precondition before a patch can be committed to Clojure or a contrib library.

You've gotten past what are actually the easier parts. There is still the issue of whether this ticket is even considered by the Clojure core team to be an enhancement worth making a change to Clojure. Take a look at the JIRA workflow here if you haven't seen it already and are curious: http://dev.clojure.org/display/community/JIRA+workflow

If you like Pascal think that this is a change you really want to see in Clojure, you may vote on this or any other JIRA ticket (except ones you create yourself – the creator is effectively the 0th voter for a ticket). Log in and click on the Vote link near the top right, and/or Watch to get email updates of changes.

Comment by Bart Kastermans [ 14/Nov/14 3:12 AM ]

Andy, thanks for the info. I was not aware of the JIRA workflow.





[CLJ-1463] Providing own ClassLoader for eval is broken Created: 10/Jul/14  Updated: 10/Jul/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.2, Release 1.3, Release 1.5, Release 1.6
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Volkert Oakley Jurgens Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: compiler
Environment:

Clojure 1.6.0



 Description   

clojure.lang.Compiler has a method with the signature

public static Object eval(Object form, boolean freshLoader)

but the freshLoader argument is ignored since https://github.com/clojure/clojure/commit/2c2ed386ed0f6f875342721bdaace908e298c7f3

Is there a good reason this still needs to be "hotfixed" like this?

We would like to provide our own ClassLoader for eval to manage the lifecycle of the generated classes.






[CLJ-1462] cl-format throws ClassCastException: Writer cannot be cast to Future/IDeref Created: 07/Jul/14  Updated: 09/Jul/14

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

Type: Defect Priority: Minor
Reporter: Pascal Germroth Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: print


 Description   

Using ~I and ~_ etc fails in many situations, the most trivial one being:

Clojure 1.6.0 and 1.5.1:

user=> (clojure.pprint/cl-format true "~I")
ClassCastException java.io.PrintWriter cannot be cast to java.util.concurrent.Future  clojure.core/deref-future (core.clj:2180)
user=> (clojure.pprint/cl-format nil "~I")
ClassCastException java.io.StringWriter cannot be cast to java.util.concurrent.Future  clojure.core/deref-future (core.clj:2180)
user=> (clojure.pprint/cl-format nil "~_")
ClassCastException java.io.StringWriter cannot be cast to java.util.concurrent.Future  clojure.core/deref-future (core.clj:2180)

Clojure 1.4.0

user=> (clojure.pprint/cl-format true "~I")
ClassCastException java.io.OutputStreamWriter cannot be cast to clojure.lang.IDeref  clojure.core/deref (core.clj:2080)
user=> (clojure.pprint/cl-format nil "~I")
ClassCastException java.io.StringWriter cannot be cast to clojure.lang.IDeref  clojure.core/deref (core.clj:2080)
user=> (clojure.pprint/cl-format nil "~_")
ClassCastException java.io.StringWriter cannot be cast to clojure.lang.IDeref  clojure.core/deref (core.clj:2080)

These work in other implementations, i.e. clisp, creating empty output in these trivial cases:

> (format t "~I")
NIL
> (format nil "~I")
""
> (format nil "~_")
""


 Comments   
Comment by Andy Fingerhut [ 07/Jul/14 11:01 AM ]

The tilde-underscore sequence is for "conditional newline", according to the CLHS here: http://www.lispworks.com/documentation/lw51/CLHS/Body/22_cea.htm

Tilde-capital-letter-I is for indent: http://www.lispworks.com/documentation/lw51/CLHS/Body/22_cec.htm

Comment by Pascal Germroth [ 07/Jul/14 12:09 PM ]

Ah, didn't think to try that. It fails without cl-format as well:

user=> (clojure.pprint/pprint-newline :linear)
ClassCastException java.io.PrintWriter cannot be cast to java.util.concurrent.Future  clojure.core/deref-future (core.clj:2180)
user=> (clojure.pprint/pprint-indent :block 0)
ClassCastException java.io.PrintWriter cannot be cast to java.util.concurrent.Future  clojure.core/deref-future (core.clj:2180)

Manually creating a pretty writer does work though:

user=> (binding [*out* (clojure.pprint/get-pretty-writer *out*)] (clojure.pprint/pprint-newline :linear))
nil

In the get-pretty-writer doc it says:

Generally, it is unnecessary to call this function, since pprint,
write, and cl-format all call it if they need to.

Which appears to not be true for cl-format, and it would be nice if it would be applied automatically for all functions that need a pretty writer.

Comment by Pascal Germroth [ 09/Jul/14 6:37 PM ]

More bad news!
Manually creating a pretty-writer doesn't do the trick either, because it is not being properly flushed:

user=> (binding [*out* (get-pretty-writer *out*)] (cl-format true "hello ~_world~%"))
hello world
nil
user=> (binding [*out* (get-pretty-writer *out*)] (cl-format true "hello ~_world"))
hellonil
user=> (binding [*out* (get-pretty-writer *out*)] (cl-format true "hello ~_world") (.ppflush *out*))
hello worldnil

The ~% inserts an unconditional newline like \n, which also works as expected.

Insert ~_ before and it only prints up to that one. But I've also managed to get it to abort at other ~_ s, maybe because other commands flushed it.

Manually flushing it, like the inexplicably private with-pretty-writer macro does works though.
I don't understand why get-pretty-writer is exposed but not the macro that is needed to use it properly. Also all functions using pretty-writer facilities should use with-pretty-writer, that's what it appears to be specifically designed for. Then there's no need to expose it (or get-pretty-writer).





[CLJ-1459] records should support transient Created: 05/Jul/14  Updated: 06/Jul/14

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

Type: Enhancement Priority: Major
Reporter: Yongqian Li Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: defrecord


 Description   

user=> (defrecord R [a])
user.R
user=> (transient (->R nil))
ClassCastException user.R cannot be cast to clojure.lang.IEditableCollection clojure.core/transient (core.clj:3060)






[CLJ-1455] Postcondition in defrecord: Compiler unable to resolve symbol % Created: 28/Jun/14  Updated: 29/Jun/14

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

Type: Enhancement Priority: Minor
Reporter: Phill Wolf Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: defrecord


 Description   

Clojure's postconditions[1] are a splendiferous, notationally
idiot-proof way to scrutinize a function's return value without
inadvertently causing it to return something else.

Functions (implementing protocols) for a record type may be defined in
its defrecord or with extend-type. In functions defined in
extend-type, postconditions work as expected. Therefore, it is a
surprise that functions defined in defrecord cannot use
postconditions.

Actually it appears defrecord sees a pre/postcondition map as ordinary
code, so the postcondition runs at the beginning of the function (not
the end) and the symbol % (for return value) is not bound.

The code below shows a protocol and two record types that implement
it. Type "One" has an in-the-defrecord function definition where the
postcondition does not compile. Type "Two" uses extend-type and the
postcondition works as expected.

Unable to find source-code formatter for language: clojure. Available languages are: javascript, sql, xhtml, actionscript, none, html, xml, java
(defprotocol ITimesThree
  (x3 [a]))

;; defrecord with functions inside cannot use postconditions.
(defrecord One
    []
  ITimesThree
  (x3 [a]
    {:pre [(do (println "One x3 pre") 1)] ;; (works fine)
     :post [(do (println "One x3 post, %=" %) 1)]
     ;; Unable to resolve symbol: % in this context.
     ;; With % removed, it compiles but runs at start, not end.
     }
    (* 1 3)))

;; extend-type can add functions with postconditions to a record.
(defrecord Two
    [])
(extend-type Two
  ITimesThree
  (x3 [a]
    {:pre [(do (println "Two x3 pre") 1)] ;; (works fine)
     :post [(do (println "Two x3 post, %=" %) 1)] ;; (works fine)
     }
    (* 2 3)))

(defn -main
  "Main"
  []
  (println (x3 (->One)))
  (println (x3 (->Two))))

[1] http://clojure.org/special_forms, in the fn section.






[CLJ-1447] Make proxy work with protocols directly (like reify does) Created: 18/Jun/14  Updated: 18/Jun/14

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

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


 Description   

Currently Proxy only supports interfaces and abstract classes. While protocols are supported via the protocol's interface, this means that the method names must be java mangled. E.g. the method name for set-value! becomes set_value_BANG_. However, the only possible way to subclass abstract classes in Clojure is currently via gen-class (doesn't work from the REPL) or proxy.






[CLJ-1446] (def v) with no init supplied destroys #'v metadata Created: 13/Jun/14  Updated: 13/Jun/14

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

Type: Defect Priority: Major
Reporter: Nahuel Greco Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

(def a) destroys #'a metadata, check this:

(def ^:mykey a 1)

(meta #'a)              ;; ok, :mykey is present

(let [v (def a)]
   [(meta v)            ;; NO :mykey present, metadata destroyed
    (identical? v #'a)  ;; true, we are talking of the same var
   ])

(meta #'a)              ;; NO :mykey present

If this is not a bug but a "feature", then we have at least two problems:

1- The def special form documentation doesn't state this behaviour at all, it needs to be clarified. With the current documentation it seems as doing a def with no init supplied will not make any side-effect at all, and this is not true for the var metadata.

2- defmulti uses this form to lookup the var and check if it already binds to a MultiFn, if that is the case then defmulti does nothing... but it really does something, defmulti will destroy the original var metadata in the (supposedly non-destructive) check. This is the involved defmulti fragment:

(let [v# (def ~mm-name)]
  (when-not (and (.hasRoot v#) (instance? clojure.lang.MultiFn (deref v#)))
   ...


 Comments   
Comment by Alex Miller [ 13/Jun/14 4:14 PM ]

I think this is mostly a dupe of CLJ-1148 but I'll leave it as it states the specific problem more precisely.

Comment by Nahuel Greco [ 13/Jun/14 7:35 PM ]

Alex Miller: It seems CLJ-1148 is an special case where this problem shows, but the patches in CLJ-1148 only fixes the issues for defonce, not generally for def, not for defmulti and not clarifies this behaviour in the def special form documentation.





[CLJ-1445] pprint prints some metadata when *print-meta* bound to true, but not all Created: 13/Jun/14  Updated: 13/Jun/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: 3
Labels: None

Attachments: File clj-1445-workaround-v2.clj    

 Description   

Short example illustrating the behavior:

user=> *clojure-version*
{:major 1, :minor 6, :incremental 0, :qualifier nil}

user=> (def f1 '(defn foo [^Integer x] ^{:bar 8} (inc x)))
#'user/f1

;; pr shows all metadata, as expected

user=> (binding [*print-meta* true] (pr f1))
^{:line 2, :column 10} (defn foo [^Integer x] ^{:bar 8, :line 2, :column 33} (inc x))nil

;; pprint shows some metadata, but not all

user=> (binding [*print-meta* true] (clojure.pprint/pprint f1))
(defn foo [^Integer x] (inc x))
nil

I have not dug into the details yet, but it appears that this may be because pprint uses pr to show symbols, but not to show collections. Thus pprint shows metadata on symbols, but not collections.

It would be nice if pprint could instead show all metadata, as pr does, when print-meta is bound to true.



 Comments   
Comment by Andy Fingerhut [ 13/Jun/14 11:30 AM ]

Attached file clj-1445-workaround-v1.clj is a function that pprints with more metadata than clojure.pprint does. As noted in the comments, it may not show metadata on other metadata. Please update with an enhanced version if you create one.

Comment by Andy Fingerhut [ 13/Jun/14 12:26 PM ]

Attached file clj-1445-workaround-v2.clj supersedes the earlier one, which I will delete.

The included function pprint-meta appears to be a correct way to pprint values with all metadata, even if the metadata maps themselves have metadata on them.





[CLJ-1444] Fix unquote splicing for empty seqs Created: 11/Jun/14  Updated: 12/Nov/14

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: reader, syntax-quote

Attachments: Text File 0001-Fix-unquote-splicing-for-empty-seqs-This-required-ma.patch    
Patch: Code and Test

 Description   

Current behaviour:

user=> `(~@())
nil
user=> `[~@()]
[]

Expected behaviour:

user=> `(~@())
()
user=> `[~@()]
[]


 Comments   
Comment by Andy Fingerhut [ 06/Aug/14 2:21 PM ]

Patch 0001-Fix-unquote-splicing-for-empty-seqs.patch dated Jun 11 2014 no longer applies cleanly to latest Clojure master due to some changes committed earlier today. I haven't checked whether this patch is straightforward to update.

Comment by Nicola Mometto [ 06/Aug/14 2:31 PM ]

Updated patch to apply to HEAD

Comment by Nicola Mometto [ 12/Nov/14 10:07 AM ]

This patch requires the patch at http://dev.clojure.org/jira/browse/CLJ-1586 to be applied first otherwise some compile-time metdata might get lost.





[CLJ-1443] reduce docstring partly incorrect with reducers. Created: 10/Jun/14  Updated: 10/Jun/14

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

Type: Defect Priority: Minor
Reporter: Greg Chapman Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring, reducers


 Description   

The docstring for reduce includes this: "If val is not supplied, returns the result of applying f to the first 2 items in coll". This is true if coll is a sequence, but not if it is a reducer. For example:

user=> (->> (range 0 10 2) (reduce (fn[x y] (+ x y))))
20
user=> (->> (range 0 10 2) (r/map #(/ % 2)) (reduce (fn[x y] (+ x y))))
ArityException Wrong number of args (0)

The docstring should be updated to make it clear that reducers (used without an initial seed value) require the reducing function to support a 0 arity overload returning the identity value for the reduction operation.






[CLJ-1442] Tag gensym sourced symbols with metadata Created: 09/Jun/14  Updated: 11/Aug/14

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

Type: Enhancement Priority: Trivial
Reporter: Reid McKenzie Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: None

Attachments: Text File 0003-Annotate-generated-symbols-with-metadata.patch    
Patch: Code

 Description   

For static analysis tools derived from TANAL it is frequently useful to determine whether a symbol is user defined or the result of code generation. As tools analyzer depends on the Clojure core for evaluation and symbol generation a user wishing to annotate generated symbols must currently provide a binding replacing clojure.core/gensym with a snippet equivalent to the following patch. Such overloading is not appropriate for TANAL, TE* or user code as it is a redefinition of clojure.core behavior which should be standard rather than subjected to users with crowbars.



 Comments   
Comment by Gary Trakhman [ 09/Jun/14 2:11 PM ]

This could eventually help with filtering out def'd symbols like 't131045 coming from reify in CLJS. I've been seeing this behavior with core.async namespaces in an autodoc-cljs proof-of-concept, which could eventually target tools.analyzer.

Comment by Alex Miller [ 09/Jun/14 2:57 PM ]

Re the patch, why not call the Symbol constructor that takes meta instead of with-meta? For performance, it might also be useful to use the same constant map as well.

Comment by Reid McKenzie [ 09/Jun/14 3:10 PM ]

Because the compiler will emit the meta map as a static field the patch as-is will share the same map instance between all annotated symbols. Calling the metadata constructor is reasonable, I'll update the patch.

Comment by Reid McKenzie [ 09/Jun/14 3:28 PM ]

So the metadata constructor of Symbol is private, see https://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/Symbol.java#L100. Without changing this directly constructing symbols with metadata is not possible from the core. If you're worried about escaping the var indirection cost of adding metadata via with-meta inlining with-meta is an option, however then we're building two symbols for no good reason. Exposing the currently private metadata constructor is probably the right fix, abet its own ticket.

Comment by Andy Fingerhut [ 01/Jul/14 6:41 PM ]

From the comments above it appears that this is not planned to be a final version of this patch, but FYI some automated scripts I have found that patch 0001-Annotate-generated-symbols-with-metadata.patch dated Jun 9 2014 applies cleanly to the latest Clojure master as of Jul 1 2014, but Clojure fails to build.

Comment by Reid McKenzie [ 02/Jul/14 1:37 AM ]

Thanks Andy, I'll rework and test it in the morning

Comment by Reid McKenzie [ 07/Jul/14 9:49 AM ]

Because of the work that clojure.lang.Symbol/intern does, exposing and using the metadata constructor directly makes no sense. The updated patch directly invokes clojure.lang.Symbol/withMeta rather than indirecting through clojure.core/with-meta and taking the performance hit of calling through a Var. Builds cleanly on my system.

Comment by Andy Fingerhut [ 01/Aug/14 8:49 PM ]

Reid, although JIRA can handle multiple attachments with the same name, it can be a bit confusing for people, and for some scripts I have for determining which patches apply and test cleanly. Would you mind renaming one of your patches?

Comment by Reid McKenzie [ 01/Aug/14 10:55 PM ]

3rd and final cut at this patch.





[CLJ-1441] Provide docs on how to reference imports that conflict with default ns class imports Created: 07/Jun/14  Updated: 07/Jun/14

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

Type: Enhancement Priority: Minor
Reporter: Howard Lewis Ship Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: documentation


 Description   

This is related to CLJ-1440; a name clash on class "Compiler" between clojure.lang and another package.

The documentation does not address how to handle this cleanly; specifically, refer would appear to allow a way to exclude clojure.lang.Compiler, but does not.



 Comments   
Comment by Alex Miller [ 07/Jun/14 7:56 PM ]

refer is all about symbols that refer to Var. refer's docstring seems pretty clear on that to me.

Your conflict is on symbols that refer to a Class, which is the domain of import and has no exclusion facilities. The set of default imports is defined in RT.DEFAULT_IMPORTS and includes clojure.lang.Compiler along with everything in java.lang.*.

You can always fully-qualify any class you want to use in your ns, so that is one workaround available. Another is what Nicola suggested in CLJ-1440 - post-modify the ns after load.

Either ns or import could theoretically document more explicitly the list of auto-imports and recommend a solution to this problem. I'm not sure whether this is worth doing or would be accepted given the infrequency of the use case and availability of workarounds.

I tweaked http://clojure.org/namespaces to mention this.





[CLJ-1438] bit-* functions don't check for overflow Created: 05/Jun/14  Updated: 05/Jun/14

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

Type: Enhancement Priority: Minor
Reporter: Pascal Germroth Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: math


 Description   

The bit* functions, in contrast to the other numerical functions, don't appear to check for overflow, i.e. (bit-test 13 200000) returns true.

It would be nice if the behaviour would fit the other numerical operators, i.e. throw on overflow and provide a variant that doesn't, and one that works with arbitrary precision, also not currently supported:
(bit-test (bigint 13) 20000), (bit-test (biginteger 13) 20000) throw IllegalArgumentException.






[CLJ-1435] 'numerator and 'denominator fail to handle integral values (i.e. N/1) Created: 30/May/14  Updated: 30/May/14

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

Type: Defect Priority: Major
Reporter: Aaron Brooks Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None


 Description   

Because ratio values reduce to lowest terms and, for integral values where the lowest term is N/1, are auto-converted to BigInts (and formerly Longs), the current behavior of clojure.core/numerator and clojure.core/denominator yield unexpected results.

user=> (numerator 1/3)
1
user=> (numerator (+ 1/3 2/3))

ClassCastException clojure.lang.BigInt cannot be cast to clojure.lang.Ratio  clojure.core/numerator (core.clj:3306)
user=> (denominator 1/3)
3
user=> (denominator (+ 1/3 2/3))

ClassCastException clojure.lang.BigInt cannot be cast to clojure.lang.Ratio  clojure.core/denominator (core.clj:3314)
user=>

The auto-conversion to Longs is not really the problem in my mind. I'd like to see numerator return the original value when presented with a BigInt and denominator always return 1 when presented with a BigInt. It seems reasonable to request the same for Longs.

If desired, I'd be happy to produce a patch.



 Comments   
Comment by Andy Fingerhut [ 30/May/14 6:35 PM ]

I don't know the official stance on this ticket, but will add some notes.

Aaron, numerator and denominator are pretty clearly documented to work on Ratio types only.

It is pretty easy to write my-numerator and my-denominator that work exactly as you wish, checking for the type of arg and using numerator, denominator for Ratio types, and doing whatever you think is correct for other numeric types.

Comment by Aaron Brooks [ 30/May/14 7:44 PM ]

I'm aware that they are documented as such. Part of my point is that you can be working entirely with Ratio types and, via arithmetic operations between them, sometimes wind up with a non-Ratio number unexpectedly.

Also consider:

user=> (numerator 2/1)
ClassCastException java.lang.Long cannot be cast to clojure.lang.Ratio  clojure.core/numerator (core.clj:3238)

You're then left either implementing a try/catch correction or always checking the type before using numerator or denominator which is a loss in performance.

The patch I have in mind is creating a protocol, extended to Ratio, BigInt and Long which calls the appropriate method (Ratios) or returns either the given number or 1 (numerator/denominator) for the integral types. I expect this to maintain the current level of performance in the cases where it works and behave properly in the cases currently not handled.





[CLJ-1434] The doc string for `trampoline` suggests that it applies to mutual recursion in general. It doesn't: it applies to mutual *tail* recursion only. Created: 29/May/14  Updated: 29/May/14

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

Type: Task Priority: Trivial
Reporter: Colin Hastie Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring, patch,
Environment:

Any.



 Description   

The doc-string for `trampoline` starts "trampoline can be used to convert algorithms requiring mutual
recursion without stack consumption. ". This is inaccurate: `trampoline` applies only to mutual tail recursion.

Replace this with "trampoline can be used to convert algorithms employing mutual tail recursion into a form that does not consume stack".






[CLJ-1433] proxy-super calls generally use reflection Created: 28/May/14  Updated: 28/May/14

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

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


 Description   

For example:

user=> (proxy [java.util.BitSet] []
  (flip [bitIndex]
    (proxy-super flip bitIndex)))
Reflection warning, NO_SOURCE_PATH:73:5 - call to method flip can't be resolved (target class is unknown).

I believe this issue might be fixed by simply adding type-hint metadata to the 'this symbol emitted by the proxy macro. I have not tried this change, but this macro seems to indicate it should work:

(defmacro proxy-super-cls [cls meth & args]
  (let [thissym (with-meta (gensym) {:tag cls})]
    `(let [~thissym ~'this]
      (proxy-call-with-super (fn [] (. ~thissym ~meth ~@args)) ~thissym ~(name meth))
    )))
;;;;;;;;;;;;;;;;;;;;;;
user=> (proxy [java.util.BitSet] []
  (flip [bitIndex]
    (proxy-super-cls java.util.BitSet flip bitIndex)))
#<BitSet$ff19274a {}>





[CLJ-1432] NullPointerException on function with primitive result declaration Created: 26/May/14  Updated: 30/May/14

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

Type: Enhancement Priority: Minor
Reporter: Gunnar Völkel Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: primitives, typehints


 Description   

The following minimal example shows the error:

(defn f ^double [])
(f)
=> NullPointerException

When decompiling the function `f` I found the following return expression:

return null.doubleValue();

This happened in a Java interop scenario where the called Java method had no return value but was in the return position of the primitive Clojure function.
The compiler should check for `null` on compilation.

Another example - calling a method with void return as the last expression fails in a similar way:

(defn f ^double [^SomeClassToAvoidRuntimeReflection obj, x, y]
  (.someMethod obj, x, y))
(f obj, x, y)
=> NullPointerException


 Comments   
Comment by Alex Miller [ 26/May/14 11:19 PM ]

What do you expect to happen in this case? You declared a function as returning a double but didn't return one.

Comment by Gunnar Völkel [ 27/May/14 8:48 AM ]

Since this is only the minimal example the error is relatively easy to spot.
Consider the following small example with Java interop:

(defn f ^double [^SomeClassToAvoidRuntimeReflection obj, x, y]
  (.someMethod obj, x, y))
(f obj, x, y)
=> NullPointerException

In this example it is much harder to find the reason for the NPE because you'd first suspect `obj` to be `null`.

I expect a check in the compiler at the point where "return null.doubleValue();" is emitted, followed by an error message, e.g. "Primitive return value of type 'double' expected, but no value is returned.".

Comment by Jozef Wagner [ 28/May/14 2:15 AM ]

Your second example seems perfectly OK to me, compiler should not report any error and NPE check must be at runtime.

Comment by Gunnar Völkel [ 28/May/14 2:46 AM ]

@Jozef: No, you are wrong. The compiler infers via reflection at compile time that the called method does not return a value and emits "return null.doubleValue()". So this can and should be reported as explicit error at compile time. I added a typehint to make it clear that there is no runtime reflection involved.
You would be right, if the compiler emitted something like "return somevar.doubleValue();" because then at compile time there is no knowledge about a possible "null" value.

Comment by Andy Fingerhut [ 28/May/14 10:00 AM ]

Gunnar, in your example, is the method 'someMethod' declared to return void, or something else? Adding that info to your example might help clarify it.

Comment by Jozef Wagner [ 29/May/14 2:26 AM ]

Gunnar, the second example was ambiguous and strayed away the discussion. Anyway, whether returning wrong type is through the native method or not, it is a user error in the first place. Right now it is reported at runtime. For me this ticket should be a minor enhancement instead of defect.

Comment by Gunnar Völkel [ 30/May/14 4:40 AM ]

Yes, the reason is a user error. But one that is harder to debug than necessary.
Also, it is clearly a defect since emitting 'null.doubleValue()' can not be considered as a valid compilation.

Andy, yes 'someMethod' is declared to return void. I'd edit the original ticket text to add the example and the java method return value information, but it seems jira does not let me.

Comment by Alex Miller [ 30/May/14 8:35 AM ]

I added the second example (with clarifying void comment) to the description.





[CLJ-1431] Switch from MurmurHash3 to SipHash to prevent DoS collision attack (hash flooding) Created: 25/May/14  Updated: 26/May/14

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

Type: Enhancement Priority: Major
Reporter: James Thornton Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: security


 Description   

Clojure is using Murmur3 throughout:
https://github.com/clojure/clojure/commit/dff9600387b962f16fc78e6477e10e34651fd366

DJB, Jean-Philippe Aumasson, and Martin Boßlet have shown that Murmur3 is not resilient against hash collision attacks:
http://www.ocert.org/advisories/ocert-2012-001.html
https://131002.net/siphash/

"Hash-flooding DoS reloaded: attacks and defenses" talk by DJB, Jean-Philippe Aumasson, and Martin Boßlet
http://media.ccc.de/browse/congress/2012/29c3-5152-en-hashflooding_dos_reloaded_h264.html

"Breaking Murmur: Hash-flooding DoS Reloaded"
http://emboss.github.io/blog/2012/12/14/breaking-murmur-hash-flooding-dos-reloaded/

Python, Ruby, JRuby, Haskell, Rust, Perl, Redis... have all switched to SipHash
https://en.wikipedia.org/wiki/SipHash

Last year Google dropped CityHash from Guava and replaced it with SipHash
https://code.google.com/p/guava-libraries/issues/detail?id=1232

SipHash Guava Implementation
https://code.google.com/p/guava-libraries/source/browse/guava/src/com/google/common/hash/SipHashFunction.java

SipHash Java reference implementation
https://github.com/emboss/siphash-java/blob/master/src/main/java/com/github/emboss/siphash/SipHash.java



 Comments   
Comment by Alex Miller [ 26/May/14 12:56 AM ]

Thanks, we've talked about this issue and some possible things we could do, but didn't have a ticket for it yet.

Comment by Alex Miller [ 26/May/14 1:08 AM ]

While the Java 7 approach relied on (attempting) to properly seed hash maps with string hash codes, that was all dropped in Java 8, which addressed DoS collision hash attacks by instead improving the data structure to switch from linear collisions to a red/black tree (log-time) for collisions. It's possible a similar approach could work in Clojure as well.

One workaround that could be used now is to wrap map keys in a custom type that implements IHashEq and implements an alternate hash function.





[CLJ-1428] restart-agent is ignored inside an fn passed to set-error-handler. Created: 19/May/14  Updated: 19/May/14

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

Type: Defect Priority: Minor
Reporter: Rafik NACCACHE Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: agents
Environment:

Linux, jdk 1.7, emacs / cider



 Description   

If I pass a function containing start-agent to set-error-handler of an agent, if an exception occurs the restart-agent is ignored.
for example:

(def a (agent 0))

(set-error-handler! a (fn [the-agent the-exception] (restart-agent the-agent)) )

If I now issue : (send! a #(/ 1 0)), I still have a failed agent. It did not restart.

I know I can set the error-mode to the agent to :continue to have my agent up after a crash, but I wished I could fix the conditions that caused the exception in the first-place then restart the agent programmatically in the set-error-handler.

Maybe it is a known beahviour, but then it is not documented ?






[CLJ-1425] Defer literal map construction of syntax-quoted maps to allow for semantically valid unquote splicing Created: 16/May/14  Updated: 15/Oct/14

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

Type: Enhancement Priority: Minor
Reporter: Jon Distad Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: reader
Environment:

Any


Attachments: Text File 0001-Fix-map-unquote-splicing.patch    
Patch: Code and Test

 Description   

At present one cannot unquote-splice into a map literal unless the map contains an even number of literal forms, even if one of them is a null unquote (~@[]).

E.g.: `{~@[1 2]} ;=> RuntimeException Map literal must contain an even number of forms clojure.lang.Util.runtimeException (Util.java:219)

However, within the context of a syntax-quote, it is not essential that the map literal be represented internally as a map since the syntax-quote emits code to build the map and not the map itself. The syntaxQuote method on SyntaxQuoteReader does not even operate the map, but rather a flattened sequence of interleaved keys and values.

With the aid of metadata and a LispReader-global Var, we can track that a collection of elements within a syntax quote will become a map, and emit the proper code forms from the SyntaxQuoteReader. There is a small edge case in metadata literals, but an with additional piece of metadata containing the proto-map we can still generate the appropriate (with-meta ...) form at syntax-quote emission time.

Importantly, none of the hand-waving involved ever escapes the reader, and the eval/compile environment is none the wiser.

This allows the following:

`{~@[1 2]} ;=> after eval: {1 2}
`^{~@[:foo :bar]} sym ;=> metadata of 'sym after eval: {:foo :bar}

But not:
`~{1} ;=> RuntimeException ...

Or:{1} ;=> RuntimeException ...

And `{~@[1]} has the same semantics as the currently required `{~@[1] ~@[]}
;=> IllegalArgumentException No value supplied for key: 1 clojure.lang.PersistentHashMap.create (PersistentHashMap.java:77)

The changes in my patch pass all existing tests and include an additional test for the newly-supported map unquote-splicing form.



 Comments   
Comment by Jon Distad [ 16/May/14 5:57 PM ]

Modified from this morning- more tests, plus bugfix for the new cases caught.

Comment by Jon Distad [ 17/May/14 10:47 AM ]

Updated patch.

Now uses two distinct paths for adding metadata. Old version potentially stacked with-meta calls, which could result in lost keys.

Comment by Kevin Downey [ 14/Oct/14 1:36 PM ]

It seems like this is a bad idea, it sort of makes sense from purely a macro writing perspective, but syntax quote is used outside of macros, in which case this just becomes a circumvention of the duplicate key checks that were added, I think some time around 1.3 maybe 1.4

http://dev.clojure.org/display/design/Allow+duplicate+map+keys+and+set+elements

Comment by Jon Distad [ 14/Oct/14 5:55 PM ]

Actually, unquote-splicing already circumvents the duplicate key check because it expands to an (apply hash-map ...) call.

In Clojure 1.7.0-alpha2

user> `{~@[:foo :bar :foo :bar] ~@[]}
;=> {:foo :bar}
user> '`{~@[:foo :bar :foo :bar] ~@[]}
;=> (clojure.core/apply clojure.core/hash-map (clojure.core/seq (clojure.core/concat [:foo :bar :foo :bar] [])))

Comment by Kevin Downey [ 14/Oct/14 7:15 PM ]

yeah, sorry, I was confusing this implementation with a related issue that was closed. do you have a motivating example for this? I write a fair bit of clojure and have not found it to be an issue in practice, and I am leery of relaxing these sort of constraints. if we allow this behavior, then syntax quote can definitely never be pulled out of the reader(there may be other behavior that already makes this hard to impossible, I am not sure), effectively syntax quote would have to operate on data before it makes out of the reader, were as if maps used in syntax quote are "well formed" in may be possible to move syntax quote (a source of a lot of complexity in the reader) out of the reader and have it operate on data that has already been read in.

I am almost 100% sure making syntax quote a post reader macro is not a priority in any shape or form, but I just mention it as the sort of follow on thing that could have the door shut on it due to these kind of changes, I've general begun to think of basically anything related to syntax quote as adding syntax above beyond just data, which seems a negative.

So anyway, I don't feel much pain from this behavior and it seems like the "fix" could have some follow on consequences, so a solid motivating example would be good.

just to warn you away from spending time coming up with a motivating example, every feature I have railed against has been committed, so if you just ignore me there is a real chance you'll make it in

Comment by Jon Distad [ 15/Oct/14 8:16 AM ]

To be honest, I had forgotten I submitted this. I suppose it boils down to prioritizing principles- do the literal semantics of a map take precedence over the conceptual semantics? At this point I'm against my former position and I think the literal semantics of the should take precedence, as they currently do. Especially since this is in the reader.





[CLJ-1423] Applying a var to an infinite arglist consumes all available memory Created: 15/May/14  Updated: 15/May/14

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

Type: Defect Priority: Minor
Reporter: Alan Malloy Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: None

Attachments: Text File apply-var.patch    
Patch: Code and Test

 Description   

It is possible to apply a function to an infinite argument list: for example, (apply distinct? (repeat 1)) immediately returns false, after realizing just a few elements of the infinite sequence (repeat 1). However, (apply #'distinct? (repeat 1)) attempts to realize all of (repeat 1) into memory at once.

This happens because Var.applyTo delegates to AFn.applyToHelper to decide which arity of Var.invoke to dispatch to; but AFn doesn't expect infinite arglists (mostly those use RestFn). So it uses RT.seqToArray, which doesn't work well in this case.

Instead, Var.applyTo(args) can just dispatch to deref().applyTo(args), and let the function being stored figure out what to do with the arglist.

I've changed Var.applyTo to do this, and added a test (which fails before my patch is applied, and passes afterwards).






[CLJ-1422] Recur around try boxes primitives Created: 14/May/14  Updated: 28/Jul/14

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

Type: Enhancement Priority: Minor
Reporter: Kyle Kingsbury Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: compiler, performance, typehints


 Description   

Primitive function and recur variables can't pass through a (try) cleanly; they're boxed to Object instead. This causes reflection warnings for fns or loops that use primitive types.

user=> (set! *warn-on-reflection* true)
true
 
user=> (fn [] (loop [t 0] (recur t)))
#<user$eval676$fn__677 user$eval676$fn__677@3d80023a>
 
user=> (fn [] (loop [t 0] (recur (try t))))
NO_SOURCE_FILE:1 recur arg for primitive local: t is not matching primitive, had: Object, needed: long
Auto-boxing loop arg: t
#<user$eval680$fn__681 user$eval680$fn__681@5419323a>

user=> (fn [^long x] (recur (try x)))
NO_SOURCE_FILE:1 recur arg for primitive local: x is not matching primitive, had: Object, needed: long

CompilerException java.lang.IllegalArgumentException:  recur arg for primitive local: x is not matching primitive, had: Object, needed: long, compiling:(NO_SOURCE_PATH:1:1)


 Comments   
Comment by David James [ 15/Jun/14 10:27 PM ]

Without commenting on the most desirable behavior, the following code does not cause reflection warnings:

user=> (set! *warn-on-reflection* true)
true
user=> (fn [] (loop [t 0] (recur (long (try t)))))
#<user$eval673$fn__674 user$eval673$fn__674@4e56c411>
Comment by Nicola Mometto [ 16/Jun/14 6:33 AM ]

Similar ticket http://dev.clojure.org/jira/browse/CLJ-701

Comment by Kevin Downey [ 21/Jul/14 6:59 PM ]

try/catch in the compiler only implements Expr, not MaybePrimitiveExpr, looking at extending TryExpr with MaybePrimitiveExpr it seems simple enough, but it turns out recur analyzes it's arguments in the statement context, which causes (try ...) to essentially wrap itself in a function like ((fn [] (try ...))), at which point it is an invokeexpr which is much harder to add maybeprimitiveexpr too and it reduces to the same case as CLJ-701

Comment by Kevin Downey [ 22/Jul/14 9:27 PM ]

http://dev.clojure.org/jira/browse/CLJ-701 has a patch that I think solves this

Comment by Alex Miller [ 28/Jul/14 1:56 PM ]

Should I dupe this to CLJ-701?

Comment by Kevin Downey [ 28/Jul/14 5:22 PM ]

if you want the fixes for try out of the return context to be part of CLJ-701 then yes it is a dupe, if you are unsure or would prefer 701 to stay more focused (my patch may not be acceptable, or may be too large and doing too much) then no it wouldn't be a dupe. I sort of took it on myself to solve both in the patch on CLJ-701 because I came to CLJ-701 via Nicola's comment here, and the same compiler machinery can be used for both.

I think the status is pending on the status of CLJ-701.





[CLJ-1419] Report errors on missing param list or return type of methods in gen-class and gen-interface Created: 10/May/14  Updated: 12/May/14

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

Type: Enhancement Priority: Trivial
Reporter: Nathan Zadoks Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: errormsgs, gen-class

Attachments: Text File 0001-CLJ-1419-default-to-void-return-type-in-gen-interfac.patch     Text File 0001-CLJ-1419-map-nil-to-void-in-prim-class.patch     File clj1419.clj     Text File fail.log    
Patch: Code

 Description   

The following are invalid and should produce errors when invoked on gen-class or gen-interface:

(gen-interface :name clj1419.IFail :methods [[myMethod java.lang.String]])  ;; no params, throws error
(gen-interface :name clj1419.IFail :methods [[myMethod []]]) ;; no return type
(gen-interface :name clj1419.IFail :methods [[myMethod]])  ;; no params or return type

The first example throws an error. The second and third do not but will generate an invalid class, verify with:

(.getMethods clj1419.IFail)
ClassNotFoundException java.lang.  java.net.URLClassLoader$1.run (URLClassLoader.java:366)

Add checks to prevent these errors.



 Comments   
Comment by Nathan Zadoks [ 10/May/14 1:34 PM ]

I've implemented both fixes, and attached them as patches.

Comment by Nathan Zadoks [ 10/May/14 1:40 PM ]

I'd argue that the behaviour of asm-type is at fault here (it can output an invalid type name when passed a nil argument), so I prefer that fix over the purely symptomatic generate-interface fix.

Comment by Andy Fingerhut [ 10/May/14 2:33 PM ]

Nathan, were you planning on submitting a signed Clojure Contributor's Agreement, or already have? Details here if you have not: http://clojure.org/contributing

Patches from non-contributors cannot be committed to Clojure.

Note: I cannot promise you that one of your patches will be accepted into Clojure if you sign a CA – only that it will not if you do not sign one.

Comment by Alex Miller [ 10/May/14 4:19 PM ]

Please add an example of how this happens and the current error.

Comment by Nathan Zadoks [ 11/May/14 3:45 AM ]

Andy — Yep, I've read up on that. My CA will be underway to Rich soon. (filled in, signed, in an envelope, just need to await the arrival of those bloody international stamps…)

Alex Miller — Tahdah!

A demonstration of the issue, both attached and as a gist: https://gist.github.com/nathan7/3a7e3a09e458f1354cbb

Comment by Nathan Zadoks [ 11/May/14 3:48 AM ]

and here's log of the compiler crash that results (also added to the gist now)

Comment by Nathan Zadoks [ 11/May/14 4:27 AM ]

Whoops, both of my patches were rather broken due to a misunderstanding on my side.
I forgot entirely that asm-type takes a symbol, not a string.
Modifying asm-type was definitely a bad idea, that check just looks whether it should defer to prim->class.
Adding nil to prim->class would work (and I've attached my patch for that too), but it's starting to look rather inelegant compared to just patching gen-interface.
(on a side note: I'm having a lot of fun exploring the Clojure codebase! thanks for that, humans!)

Comment by Alex Miller [ 11/May/14 7:26 AM ]

My reading of the docstring of gen-interface is that method declarations must specify a parameter list and a valid return type. I would expect all of these to be invalid:

(gen-interface :name clj1419.IFail :methods [[fail nil]])
(gen-interface :name clj1419.IFail :methods [[fail [] nil]])
(gen-interface :name clj1419.IFail :methods [[fail []]])

"nil" is not a valid type - you can use "void" for this purpose and this works fine:

(gen-interface :name clj1419.IFail :methods [[fail [] void]])

If this ticket is (as the title states) a request to allow omitting the return type or using "nil" as a return type, then I think the answer is no. If the ticket is a request to improve the error reporting of the failure cases above, then I think we can consider that but it will be very low priority.

Comment by Nathan Zadoks [ 12/May/14 8:19 AM ]

The code seems to suggest otherwise though, seeing the explicit extra branch for pclasses being nil.
As much as I like PL trivia, I haven't run into `void` in Clojure anywhere else yet, and I'm surprised to see it here.
Maintaining the principle of least surprise seems preferable to pedantry about whether nil is a type: (= "nil" (str (type (.methodReturningVoid obj)))

Comment by Alex Miller [ 12/May/14 8:26 AM ]

The two places to look for words to rely on are docstrings and the http://clojure.org/documentation pages. Implementation details are just that.

"nil" is not a type. "void" is a documented type identifier indicating the absence of a return value - http://clojure.org/java_interop#Java%20Interop-Aliases

Comment by Nathan Zadoks [ 12/May/14 8:27 AM ]

Okay. Better error-checking in asm-type then?

Comment by Alex Miller [ 12/May/14 8:49 AM ]

I have updated the title and description based on my understanding of what this ticket should be, which is enhanced error-checking on the method specs for gen-class and gen-interface. I'm not sure if that's in asm-type or somewhere earlier.





[CLJ-1412] Add 2-arity version of `cycle` that takes the numer of times to "repeat" the coll Created: 28/Apr/14  Updated: 18/Sep/14

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

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

Attachments: Text File 0001-Add-2-arity-version-of-cycle-that-takes-the-number-o.patch    
Patch: Code

 Description   

There are already similar arities for repeat/repeatedly and similar functions, this patch adds a 2-arity version of cycle that behaves like this:

user> (cycle 0 '(1 2))
()
user> (cycle -1 '(1 2))
()
user> (cycle 3 '(1 2))
(1 2 1 2 1 2)
user> (cycle 1 '(1 2))
(1 2)


 Comments   
Comment by Andy Fingerhut [ 06/Aug/14 2:19 PM ]

Patch 0001-Add-2-arity-version-of-cycle-that-takes-the-number-o.patch dated Apr 28 2014 no longer applies cleanly to latest Clojure master due to some changes committed earlier today. This appears trivial to update, as it is likely only a couple of lines of diff context that have changed.

Comment by Nicola Mometto [ 06/Aug/14 2:36 PM ]

Updated patch to apply to HEAD





[CLJ-1411] Special symbols can be shadowed inconsistently Created: 28/Apr/14  Updated: 29/Apr/14

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

Type: Defect Priority: Minor
Reporter: Volkert Oakley Jurgens Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: compiler


 Description   

The compiler does not complain about let binding (or def-ing) special symbols, but the binding only works if not used at the beginning of a list:

These work:

(let [try :a]
  try)
=> :a
(let [try (constantly :a)]
  (apply try :b))
=> :a

This doesn't work:

(let [try (constantly :a)]
  (try :b))
=> :b

This is true for all special symbols, not just publicly exposed ones like try and new, but also internal ones like fn*.

I would expect consistent behaviour: either the compiler does not permit shadowing special symbols at all, or shadowing them works in all cases.



 Comments   
Comment by Nicola Mometto [ 28/Apr/14 10:01 AM ]

I don't think that shadowing special symbols is a good idea, but probably having all the special symbols namespace qualified (clojure.core/import* is the only one ns-qualified atm) along with checking for the symbol in the locals env first and fallbacking to the special symbols map after that, would probably help in those scenarios

Comment by Volkert Oakley Jurgens [ 29/Apr/14 12:48 AM ]

I think that shadowing special symbols is a bad idea. If that was possible, we'd have to change most macros in clojure.core to make them safe (i.e. explicitly add a namespace to each special symbol usage). And how would we handle special symbols that are not just implementation specific, like try and new? Every 3rd party macro that uses those might become unsafe.

My personal preference would be to prohibit the shadowing of special symbols.

Comment by Nicola Mometto [ 29/Apr/14 5:37 AM ]

That won't be the case since what I'm proposing includes making syntax-quote aware of the namespaced special symbols.
`def would expand to 'clojure.core/def for example.

Comment by Volkert Oakley Jurgens [ 29/Apr/14 5:58 AM ]

That's true, but macros don't have to use the syntax quote. See for example the definition of when.





[CLJ-1409] Add support for marking gen-class methods as native Created: 21/Apr/14  Updated: 21/Apr/14

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

Type: Enhancement Priority: Minor
Reporter: Alex Miller Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: gen-class, interop


 Description   

As far as I know, there is no support for creating a Java instance in Clojure with native methods. Everything else needed exists, but there is no way to get the right annotation on the method right now (similar to static).

Here's an example (http://benchmarksgame.alioth.debian.org/u64q/program.php?test=pidigits&lang=clojure&id=4) from Alioth perf tests where ASM is being used directly to generate a class with native methods where gen-class would have been perfectly adequate with this enhancement. (Equivalent Java: http://benchmarksgame.alioth.debian.org/u64q/program.php?test=pidigits&lang=java&id=2).

Suggested implementation is to mark ^{:native true} on a method and omit the body.






[CLJ-1407] Recur mismatch might cause multiple evaluation Created: 17/Apr/14  Updated: 17/Apr/14

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: compiler, macro


 Description   

Since mismatching recurs cause the loop body to be re-analyzed, macroexpansion in the loop body might happen more than once, causing any side effects that happen during macroexpansion to be evaluated potentially multiple times

Clojure 1.7.0-master-SNAPSHOT
user=> (defmacro x [] (println "foo"))
#'user/x
user=> (fn [] (loop [y 1] (x) (recur (Integer. 1))))
foo
foo
#<user$eval6$fn__7 user$eval6$fn__7@71687585>


 Comments   
Comment by Andy Fingerhut [ 17/Apr/14 6:59 PM ]

This is not a question about whether the behavior in the description is a bug or not, but rather a curiosity about how often people write macros that have side effects at macroexpansion time. I think the following in Clojure itself do, but there may be others:

  • gen-class, and also ns because it uses gen-class
  • gen-interface, and also definterface because it uses gen-interface
  • clojure.core/compile-if (private) calls eval on its expr arg, but as used now doesn't cause macroexpansion-time side effects
  • doc seems to have one case that prints at macroexpansion time
  • I am not sure whether defprotocol or deftype have macroexpansion time side effects, or whether they are limited to run time
Comment by Nicola Mometto [ 17/Apr/14 9:20 PM ]

Andy, I don't think there are that many macros that side-effect at macroexpansion time and I haven't discovered this bug in real code but while thinking about how loop locals invalidation was implemented in Compiler.java.

Because there are a really a small number of side-effecting macros, this is unlikely to cause problems in real code, so I changed the priority to minor.





[CLJ-1406] Libs are blindly added into loaded-libs even if an error occurs during loading Created: 17/Apr/14  Updated: 17/Apr/14

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

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

Attachments: Text File 0001-modify-clojure.core-load-lib-so-that-it-removes-the-.patch    
Patch: Code

 Description   

Suppose you have a lib that causes some errors during loading, like the following:

(ns broken-lib)

(} ; this line will cause a reader error

And then, if you require the lib, it would be added into loaded-libs in spite of the reader error, which makes require succeed silently after that.

user=> (contains? (loaded-libs) 'broken-lib)
false
user=> (require 'broken-lib)
CompilerException java.lang.RuntimeException: Unmatched delimiter: }, compiling:(broken_lib.clj:3:3) 
user=> (contains? (loaded-libs) 'broken-lib)
true
user=> (require 'broken-lib)
nil
user=>

Cause:
The patch for CLJ-1116 made the ns macro blindly add the lib being defined into loaded-libs even if an error occurs during loading.

Approach:
Modify clojure.core/load-lib so that it removes the lib from loaded-libs on error.



 Comments   
Comment by Alex Miller [ 17/Apr/14 9:07 AM ]

This patch seems somewhat removed from the cause - is there some way to instead prevent the lib from being added to loaded-libs in the first place?

Comment by OHTA Shogo [ 17/Apr/14 9:21 AM ]

To do so, I think we need to revert CLJ-1116.





[CLJ-1403] ns-resolve might throw ClassNotFoundException but should return nil Created: 14/Apr/14  Updated: 02/Oct/14

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

Attachments: Text File 0001-CLJ-1403-ns-resolve-returns-nil-if-class-is-not-foun.patch    
Patch: Code and Test

 Description   

The doc of ns-resolve states that in case the symbol cannot be resolved, it should return nil.

user=> (doc ns-resolve)
-------------------------
clojure.core/ns-resolve
([ns sym] [ns env sym])
  Returns the var or Class to which a symbol will be resolved in the
  namespace (unless found in the environment), else nil.  Note that
  if the symbol is fully qualified, the var/Class to which it resolves
  need not be present in the namespace.
nil

However if the symbol contains dots and is not a resolvable Class, a ClassNotFoundException is thrown

user=> (ns-resolve *ns* 'foo.bar)
ClassNotFoundException foo.bar  java.net.URLClassLoader$1.run (URLClassLoader.java:372)
user=> (pst *e)
ClassNotFoundException foo.bar
	java.net.URLClassLoader$1.run (URLClassLoader.java:372)
	java.net.URLClassLoader$1.run (URLClassLoader.java:361)
	java.security.AccessController.doPrivileged (AccessController.java:-2)
	java.net.URLClassLoader.findClass (URLClassLoader.java:360)
	clojure.lang.DynamicClassLoader.findClass (DynamicClassLoader.java:61)
	java.lang.ClassLoader.loadClass (ClassLoader.java:424)
	java.lang.ClassLoader.loadClass (ClassLoader.java:357)
	java.lang.Class.forName0 (Class.java:-2)
	java.lang.Class.forName (Class.java:340)
	clojure.lang.RT.classForName (RT.java:2065)
	clojure.lang.Compiler.maybeResolveIn (Compiler.java:6963)
	clojure.core/ns-resolve (core.clj:4026)
nil

The attached patch makes ns-resolve return nil in that case instead of throwing an exception



 Comments   
Comment by Alex Miller [ 14/Apr/14 2:07 PM ]

Can you include the (pst *e) ?

Comment by Nicola Mometto [ 14/Apr/14 2:10 PM ]

Added result of (pst *e) in the description

Comment by Andy Fingerhut [ 02/Oct/14 11:36 AM ]

Nicola, the patch 0001-CLJ-1403-ns-resolve-returns-nil-if-class-is-not-foun.patch dated 31 Aug 2014 applies cleanly to latest Clojure master as of Oct 1 2014, but fails to compile with JDK8. I haven't checked whether it compiles cleanly with other JDK versions yet.

Comment by Nicola Mometto [ 02/Oct/14 11:48 AM ]

Updated the patch so that it compiles fine on JDK8





[CLJ-1402] sort-by calls keyfn more times than is necessary Created: 11/Apr/14  Updated: 11/Apr/14

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

Type: Enhancement Priority: Minor
Reporter: Steve Kim Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: performance


 Description   

clojure.core/sort-by evaluates keyfn for every pairwise comparison. This is wasteful when keyfn is expensive to compute.

user=> (def keyfn-calls (atom 0))
#'user/keyfn-calls
user=> (defn keyfn [x] (do (swap! keyfn-calls inc) x))
#'user/keyfn
user=> @keyfn-calls
0
user=> (sort-by keyfn (repeatedly 10 rand))
(0.1647483850582695 0.2836687590331822 0.3222305842748623 0.3850390922996001 0.41965440953966326 0.4777580378736771 0.6051704988802923 0.659376178201709 0.8459820304223701 0.938863131161208)
user=> @keyfn-calls
44


 Comments   
Comment by Steve Kim [ 11/Apr/14 11:46 AM ]

CLJ-99 is a similar issue





[CLJ-1399] missing field munging when recreating deftypes serialized in to byte code Created: 02/Apr/14  Updated: 02/Apr/14

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

Type: Defect Priority: Major
Reporter: Kevin Downey Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: None

Attachments: File clj-1399.diff    
Patch: Code

 Description   

to embed deftypes in the bytecode the compiler emits the value of each field, then emits a call to the deftypes underlying class's constructor.

to get a list of fields the compiler calls .getBasis.

the getBasis fields are the "clojure" level field names of the deftype, which the actual "jvm" level field names have been munged (replacing - with _, etc), so the compiler tries to generate code to set values on non-existent fields

https://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/Compiler.java#L4579

https://www.refheap.com/70731

you can work around this by using field names that don't require munging. a solution might be just calling munge in the emission of field sets of ITypes



 Comments   
Comment by Kevin Downey [ 02/Apr/14 4:26 PM ]

reproducing case

$ rlwrap java -server -Xmx1G -Xms1G -jar /Users/hiredman/src/clojure/target/clojure-1.6.0-master-SNAPSHOT.jar
Clojure 1.6.0-master-SNAPSHOT
user=> (deftype Foo [hello-world])
user.Foo
user=> (alter-var-root #'default-data-readers assoc 'foo (fn [x] (Foo. x)))
{foo #<user$eval6$fn__7 user$eval6$fn__7@2f953efd>, inst #'clojure.instant/read-instant-date, uuid #'clojure.uuid/default-uuid-reader}
user=> #foo "1"
CompilerException java.lang.IllegalArgumentException: No matching field found: hello-world for class user.Foo, compiling:(NO_SOURCE_PATH:0:0)
user=>
Comment by Kevin Downey [ 02/Apr/14 4:39 PM ]

this patch fixes the issue on the latest master for me

Comment by Chas Emerick [ 02/Apr/14 4:57 PM ]

FWIW, this was precipitated by real experience (I think I created the refheap paste). The workaround is easy (don't use dashes in field names of deftypes you want to return from data reader functions), but I wouldn't expect anyone to guess that that wasn't already oversensitized to munging edge cases.





[CLJ-1398] Update URLs in javadoc.clj Created: 02/Apr/14  Updated: 09/May/14

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

Type: Enhancement Priority: Trivial
Reporter: Eli Lindsey Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None

Attachments: Text File 0001-update-apache-commons-javadoc-location.patch     Text File 0002-add-javadoc-lookup-for-guava-and-apache-commons-lang.patch     Text File 0003-add-javadoc-lookup-for-jdk8.patch    
Patch: Code

 Description   

Three minor fixes/enhancements to javadoc.clj:

0001 corrects the URLs for apache commons javadoc (the ones used in javadoc.clj no longer resolve).
0002 adds javadoc lookup for guava and apache commons lang3.
0003 adds javadoc lookup for jdk8.

(Note: contributor agreement is in the mail)



 Comments   
Comment by Andy Fingerhut [ 04/Apr/14 11:22 AM ]

Eli, thanks for the patches. It appears that you are not currently on the list of Clojure contributors here: http://clojure.org/contributing

It is the policy of the Clojure team only to incorporate patches submitted by people who have signed and submitted a Clojure CA. Were you interested in doing that?

Comment by Eli Lindsey [ 04/Apr/14 11:27 AM ]

> It is the policy of the Clojure team only to incorporate patches submitted by people who have signed and submitted a Clojure CA. Were you interested in doing that?

Yup! I mailed off the CA to Rich on Wednesday when this was filed; should be arriving shortly.

Comment by Eli Lindsey [ 09/May/14 8:18 PM ]

Just to note - Clojure CA went through and I'm listed on the contributors page now.





[CLJ-1394] Print multi method dispatch values in the exception messages. Created: 31/Mar/14  Updated: 01/Apr/14

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

Type: Enhancement Priority: Trivial
Reporter: Roman Scherer Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: errormsgs

Attachments: File pr-str-dispatch-value-safe.diff    
Patch: Code and Test

 Description   

The error messages of defmulti are at the moment not as helpful as they could be under certain circumstances. Calling this multi method with a lazy seq as it's dispatch argument raises the following exception:

(defmulti test-multi identity)
(defmethod test-multi 1 [arg] true)

(test-multi (map identity [:x]))
;=> java.lang.IllegalArgumentException: No method in multimethod 'test-multi' for dispatch value: clojure.lang.LazySeq@3c6f1187

Sometimes it would be useful to actually see which values are in the lazy seq being dispatched on. A better error message could look like
this for example:

(test-multi (map identity [:x]))
;=> java.lang.IllegalArgumentException: No method in multimethod 'test-multi' for dispatch value (:x) of class clojure.lang.LazySeq

This patch addresses this issue by formatting the dispatch value via `pr-str` and printing the class before it is passed to the exception constructor. The same is also done for the methods in MultiFn.java that throw a dispatch value as part of their exception message.



 Comments   
Comment by Nicola Mometto [ 31/Mar/14 8:22 PM ]

What if the value is infinite lazy-seq?

Comment by Roman Scherer [ 01/Apr/14 2:50 AM ]

Nicola, I forgot those. But I think infinite sequences could be handled with:

(set! *print-length* 10)

I'll try it out and will update the patch later.

Any other edge cases in mind?

Comment by Roman Scherer [ 01/Apr/14 2:28 PM ]

After having read "Controlling run-away trains, onions, and exercise
bikes" [1] I now bind print-length and print-size when building
the error message. This helps when not being able to dispatch on this
for example:

(test-multi (let [x (atom 0)] (reset! x {:deeper x})))

However I'm not sure if this helps in the following case, where
dispatching would fail on an infinite seq.

(test-multi (iterate inc 0))

The above doesn't terminate in Clojure 1.6.0, nor does it when binding
print-length like the attached patch does.

[1] http://blog.n01se.net/blog-n01se-net-p-85.html





[CLJ-1391] Allow logical operators on assert expressions Created: 26/Mar/14  Updated: 26/Mar/14

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

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


 Description   

With current code, it is not possible to express logical operators on some clojure.test assert expressions. For example, this will work:

(is (thrown? Exception <some-expression>))

however, here will fail:

(is (not (thrown? Exception <some-expression>)))

since '(thrown?)' is not an ordinary function, but looks like. This also adds confusion which is hard to explain to others unless '(is)' code was shown first.

Also, if the one would like to implement macro (e.g. 'is-not-thrown?') in form:

(defmacro is-not-thrown? [e expr]
  `(is (not ('thrown? ~e ~expr))))

which could be even more confusing for a person not knowing how 'thrown?' is implemented.






[CLJ-1389] Re-loading a namespace ignores metadata specified for the namespace Created: 20/Mar/14  Updated: 20/Mar/14

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

Type: Defect Priority: Minor
Reporter: Howard Lewis Ship Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: metadata, namespace, repl


 Description   

Using the REPL I added some metadata to a namespace and reloaded it.

(ns io.aviso.rook-test5)

to

(ns io.aviso.rook-test5
  "A testing namespace"
  {:inherted   :namespace
   :overridden :namespace})

But requesting the meta data yields nil:

(-> 'io.aviso.rook-test5 find-ns meta)
=> nil

I have tested a few variations, such as putting the metadata on the symbol instead of providing an attribute map. In all cases, the metadata from before the load persists.

Using remove-ns before re-loading the namespace does the right thing ... the metadata shows up as expected.






[CLJ-1385] Docstrings for `conj!` and `assoc!` should suggest using the return value; effect not always in-place Created: 16/Mar/14  Updated: 06/Aug/14

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

Type: Enhancement Priority: Minor
Reporter: Pyry Jahkola Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: collections, docstring

Attachments: Text File CLJ-1385-reword-docstrings-on-transient-update-funct-2.patch     Text File CLJ-1385-reword-docstrings-on-transient-update-funct.patch    
Patch: Code

 Description   

The docstrings of both `assoc!` and `conj!` say "Returns coll.", suggesting the transient edit happens always in-place, `coll` being the first argument.

However, the fact that the following example omits the key `8` in its result proves that in-place edits aren't always the case:

(let [a (transient {})]
      (dotimes [x 9]
        (assoc! a x :ok))
      (persistent! a))
    ;;=> {0 :ok, 1 :ok, 2 :ok, 3 :ok, 4 :ok, 5 :ok, 6 :ok, 7 :ok}

Instead, programmers should be guided towards using constructs like `reduce` with transients:

(persistent! (reduce #(assoc! %1 %2 :ok)
                 (transient {})
                 (range 9)))
    ;;=> {0 :ok, 1 :ok, 2 :ok, 3 :ok, 4 :ok, 5 :ok, 6 :ok, 7 :ok, 8 :ok}

The easiest way to achieve this is by changing the docstrings of (at least) `conj!` and `assoc!` to not read "Returns coll." but instead tell that the change is destructive.



 Comments   
Comment by Alex Miller [ 16/Mar/14 8:49 AM ]

When modifying transient collections, it is required to use the collection returned from functions like assoc!. The ! here indicates its destructive nature. The transients page (http://clojure.org/transients) describes the calling pattern pretty explicitly: "You must capture and use the return value in the next call."

I do not agree that we should be guiding programmers away from using functions like assoc! – transients are used as a performance optimization and using assoc! or conj! in a loop is often the fastest version of that. However I do think it would be helpful to make the docstring more explicit.

Comment by Gary Fredericks [ 05/Apr/14 10:23 AM ]

Alex I think you must have misread the ticket – the OP is suggesting guiding toward using the return value of assoc!, not avoiding assoc! altogether.

And the docstring is not simply inexplicit, it's actually incorrect specifically in the case that the OP pointed out. conj! and assoc do not return coll at the point where array-maps transition to hash-maps, and the fact that they do otherwise is supposed to be an implementation detail as far as I understand it.

Comment by Alex Miller [ 05/Apr/14 11:55 AM ]

@Gary - you're right, I did misread that.

assoc and conj both explicitly say "return a new collection" whereas assoc! and conj! say "Returns coll." I read that as "returns the modified collection" without regard to whether it's the identical instance, but I can read it your way too.

Would saying "Returns updated collection." transmit the right idea? Using "collection" instead of "coll" removes the concrete tie to the variable and "updated" hints more strongly that you should use the return value.

Comment by Pyry Jahkola [ 05/Apr/14 12:47 PM ]

@Alex, that update makes it sound right to me, FWIW.

Comment by Gary Fredericks [ 05/Apr/14 2:37 PM ]

Yeah, I think that's better. Thanks Alex. I'd be happy to submit a patch for that but I'm assuming patches are too heavy for this kind of change?

Comment by Andy Fingerhut [ 06/Apr/14 3:35 PM ]

Patches are exactly what has been done in the past for this kind of change, if it is in a doc string and not on the clojure.org web page.

Comment by Alex Miller [ 06/Apr/14 4:13 PM ]

Yup, patch desired.

Comment by Gary Fredericks [ 06/Apr/14 5:32 PM ]

Glad I asked.

Patch is attached that also updates the docstring for pop! which had the same issue, though arguably it's less important since afaik pop! does always return the identical collection (but I don't think this is part of the contract).

Comment by Andy Fingerhut [ 06/Aug/14 2:14 PM ]

Patch CLJ-1385-reword-docstrings-on-transient-update-funct.patch dated Apr 6 2014 no longer applies to latest Clojure master cleanly, due to some changes committed earlier today. I suspect it should be straightforward to update the patch to apply cleanly, given that they are doc string changes, but there may have been doc string changes committed to master, too.

Comment by Gary Fredericks [ 06/Aug/14 3:04 PM ]

Attached a new patch.





[CLJ-1383] Should name throw on nil? Created: 14/Mar/14  Updated: 15/Mar/14

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

Type: Enhancement Priority: Minor
Reporter: John Chijioke Assignee: Jozef Wagner
Resolution: Unresolved Votes: 0
Labels: None

Attachments: File clj-1383.diff    
Patch: Code and Test

 Description   

The name function throws NullPointerException on nil. Since the name function is about obtaining the string form of a specific object it should not throw on nil. It should just return the nil object as the str fn does.



 Comments   
Comment by Jozef Wagner [ 15/Mar/14 3:23 AM ]

added patch with test





[CLJ-1381] Improve support for extending protocols to primitive arrays Created: 13/Mar/14  Updated: 18/Sep/14

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

Type: Defect Priority: Major
Reporter: Alex Miller Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: protocols


 Description   

It is possible to extend protocols to primitive arrays but specifying the class for the type is a little tricky:

(defprotocol P (p [_]))
(extend-protocol P (Class/forName "[B") (p [_] "bytes"))
(p (byte-array 0))   ;; => "bytes"

However, things go bad if you try to do more than one of these:

(extend-protocol P 
  (Class/forName "[B") (p [_] "bytes")
  (Class/forName "[I") (p [_] "ints"))
CompilerException java.lang.UnsupportedOperationException: nth not supported on this type: Character, compiling:(NO_SOURCE_PATH:1:1)
	clojure.lang.Compiler.analyze (Compiler.java:6380)
	clojure.lang.Compiler.analyze (Compiler.java:6322)
	clojure.lang.Compiler$MapExpr.parse (Compiler.java:2879)
	clojure.lang.Compiler.analyze (Compiler.java:6369)
	clojure.lang.Compiler.analyze (Compiler.java:6322)
	clojure.lang.Compiler$InvokeExpr.parse (Compiler.java:3624)
	clojure.lang.Compiler.analyzeSeq (Compiler.java:6562)
	clojure.lang.Compiler.analyze (Compiler.java:6361)
	clojure.lang.Compiler.analyze (Compiler.java:6322)
	clojure.lang.Compiler$BodyExpr$Parser.parse (Compiler.java:5708)
	clojure.lang.Compiler$FnMethod.parse (Compiler.java:5139)
	clojure.lang.Compiler$FnExpr.parse (Compiler.java:3751)
Caused by:
UnsupportedOperationException nth not supported on this type: Character
	clojure.lang.RT.nthFrom (RT.java:857)
	clojure.lang.RT.nth (RT.java:807)
	clojure.core/emit-hinted-impl/hint--5951/fn--5953 (core_deftype.clj:758)
	clojure.core/map/fn--4207 (core.clj:2487)
	clojure.lang.LazySeq.sval (LazySeq.java:42)
	clojure.lang.LazySeq.seq (LazySeq.java:60)
	clojure.lang.RT.seq (RT.java:484)
	clojure.lang.RT.countFrom (RT.java:537)
	clojure.lang.RT.count (RT.java:530)
	clojure.lang.Cons.count (Cons.java:49)
	clojure.lang.Compiler.analyze (Compiler.java:6352)
	clojure.lang.Compiler.analyze (Compiler.java:6322)

The code in {parse-impls} is seeing the second {(Class/forName "[I")} as a function, not as a new type. One workaround for this is to only extend the protocol to one type at a time.

It would be even better (moving into enhancement area) if there was a syntax here to specify primitive array types - we already have the syntax of {bytes, ints, longs}, etc for type hints and those seem perfectly good to me.



 Comments   
Comment by Nahuel Greco [ 18/Sep/14 6:08 PM ]

It also breaks when extending only one array type:

(extend-protocol P
  String               (p [_] "string")
  (Class/forName "[B") (p [_] "ints") 
  )

;=> CompilerException java.lang.UnsupportedOperationException: nth not supported on this type ...

But changing the declaration order fixes it:

(extend-protocol P
  (Class/forName "[B") (p [_] "ints") 
  String               (p [_] "string")
  )

;=> OK




[CLJ-1380] Three-arg ExceptionInfo constructor permits nil data Created: 13/Mar/14  Updated: 27/Oct/14

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

Type: Defect Priority: Minor
Reporter: Gordon Syme Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: File clj-1380.diff    
Patch: Code and Test

 Description   

The argument check in the two-arg clojure.lang.ExceptionInfo constructor isn't present in the three-arg constructor so it's possible to create an ExceptionInfo with arbitrary (or nil) data.

E.g.:

user=> (clojure-version)
"1.5.1"

user=> (ex-info "hi" nil)
IllegalArgumentException Additional data must be a persistent map: null  clojure.lang.ExceptionInfo.<init> (ExceptionInfo.java:26)

user=> (ex-info "hi" nil (Throwable.))
NullPointerException   clojure.lang.ExceptionInfo.toString (ExceptionInfo.java:40)


 Comments   
Comment by Gordon Syme [ 13/Mar/14 10:47 AM ]

Sorry, didn't meant to classify as "major" and I don't have permissions to edit.

Comment by Gordon Syme [ 13/Mar/14 11:11 AM ]

Patch + tests

I'm not at all familiar with the project so may have put tests in the wrong language and/or wrong place.

The ex-info-works test is a bit dorky but shows that both constructors are equivalent (and passes without the patch to ExceptionInfo).

Comment by Alex Miller [ 13/Mar/14 12:18 PM ]

No worries on the classification - I adjust most incoming tickets in some way or another.

Thanks for the patch, however it cannot be considered unless you complete the Clojure Contributor's Agreement - http://clojure.org/contributing. This is an important step in the process that keeps the Clojure codebase on a sound legal basis.

Someone else could develop a clean room patch implementation for this ticket later, but of course it would be ideal if you could become a contributor!

Comment by Gordon Syme [ 13/Mar/14 1:15 PM ]

Hi Alex,

sure, that makes sense. I'll get the contributor's agreement in the post. It may take a while to arrive since I'm based in Europe.

Comment by Gordon Syme [ 25/Mar/14 10:03 AM ]

I just checked http://clojure.org/contributing, looks like my CCA made it through

Comment by Andy Fingerhut [ 01/Oct/14 6:48 PM ]

Gordon, I do not know if your patch is of interest to the Clojure developers, so I can't comment on that aspect of this ticket.

Instructions for creating a patch in the expected format is given on the wiki page below. Your patch is not in the expected format.

http://dev.clojure.org/display/community/Developing+Patches

Comment by Gordon Syme [ 27/Oct/14 5:30 AM ]

Whoops, sorry Andy.

I've rebased against master and added a correctly formatted patch.





[CLJ-1379] Quoting of :actual form is incorrect in clojure.test :pass type maps Created: 12/Mar/14  Updated: 12/Mar/14

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

Type: Defect Priority: Minor
Reporter: Hugo Duncan Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: test
Environment:

All clojure versions


Attachments: File fix-quoting-in-pass-case.diff    
Patch: Code

 Description   

The function symbol is not correctly quoted in the construction of the :actual value in a :pass type map for clojure.test.

It currently produces (list = 1 1) instead of (list '= 1 1) for an (is (= 1 1)) test.

I haven't been able to come up with a workaround for this.






[CLJ-1376] Initialize internal maps to more efficient version Created: 11/Mar/14  Updated: 11/Mar/14

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

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


 Description   

In reviewing some hashing stuff, I noticed that there are many places internal to Clojure that use maps initialized with PersistentHashMap.EMPTY. Many of these maps are likely to have a small number of entries such that a PersistentArrayMap might be more efficient.

These are the candidates:

src/jvm/clojure/lang/ARef.java
19:private volatile IPersistentMap watches = PersistentHashMap.EMPTY;

src/jvm/clojure/lang/Compiler.java
3009:				IPersistentMap m = PersistentHashMap.EMPTY;
3819:					       KEYWORDS, PersistentHashMap.EMPTY,
3820:					       VARS, PersistentHashMap.EMPTY,
3964:	IPersistentMap closes = PersistentHashMap.EMPTY;
3977:	IPersistentMap keywords = PersistentHashMap.EMPTY;
3978:	IPersistentMap vars = PersistentHashMap.EMPTY;
5121:                            ,CLEAR_SITES, PersistentHashMap.EMPTY
7259:			       KEYWORDS, PersistentHashMap.EMPTY,
7260:			       VARS, PersistentHashMap.EMPTY
7418:			IPersistentMap opts = PersistentHashMap.EMPTY;
7475:			IPersistentMap fmap = PersistentHashMap.EMPTY;
7522:					       KEYWORDS, PersistentHashMap.EMPTY,
7523:					       VARS, PersistentHashMap.EMPTY,
7912:                            ,CLEAR_SITES, PersistentHashMap.EMPTY

src/jvm/clojure/lang/LispReader.java
755:					RT.map(GENSYM_ENV, PersistentHashMap.EMPTY));

src/jvm/clojure/lang/MultiFn.java
39:	this.methodTable = PersistentHashMap.EMPTY;
41:	this.preferTable = PersistentHashMap.EMPTY;
49:		methodTable = methodCache = preferTable = PersistentHashMap.EMPTY;

src/jvm/clojure/lang/Var.java
48:	final static Frame TOP = new Frame(PersistentHashMap.EMPTY, null);
175:	setMeta(PersistentHashMap.EMPTY);
341:	IPersistentMap ret = PersistentHashMap.EMPTY;

Approach: Two possible approaches - initialize to PersistentArrayMap.EMPTY or call RT.map(). The latter requires function invocation so is slightly slower, but has the benefit of localizing map construction into a single place.






[CLJ-1375] Remove Util.pcequiv() and stop pretending Java colls are equiv to Clojure colls Created: 11/Mar/14  Updated: 11/Mar/14

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

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

Attachments: Text File clj-1375-v1.patch    
Patch: Code

 Description   

The Util.pcequiv() method

static public boolean pcequiv(Object k1, Object k2){
	if(k1 instanceof IPersistentCollection)
		return ((IPersistentCollection)k1).equiv(k2);
	return ((IPersistentCollection)k2).equiv(k1);
}

tries to get equiv semantics (cross-class number equality) for cases of mixed Clojure/Java collection comparison. However, this is not a sustainable direction and we would like to stop doing this.

Attached patch removes this and changes calling code to only call equiv when both collections are IPersistentCollection.






[CLJ-1371] divide(Object, Object) with (NaN, 0) does not return NaN Created: 07/Mar/14  Updated: 07/Mar/14

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

Type: Defect Priority: Trivial
Reporter: Yongqian Li Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: math


 Description   

user=> (def x Double/NaN)
#'user/x
user=> (/ x 0)

ArithmeticException Divide by zero clojure.lang.Numbers.divide (Numbers.java:156)
user=> (/ Double/NaN 0)
Double/NaN



 Comments   
Comment by Alex Miller [ 07/Mar/14 7:50 AM ]

As per the Java Language Specification (http://docs.oracle.com/javase/specs/jls/se7/html/jls-4.html#jls-4.2.4),

"All numeric operations with NaN as an operand produce NaN as a result."

Comment by Yongqian Li [ 07/Mar/14 7:54 AM ]

But in the first example it produces an ArithmeticException.

Comment by Alex Miller [ 07/Mar/14 9:27 AM ]

Ah, I see the question now.

Here we are dividing a double by a long. In the first case, this is parsed as divide(Object, long) which then calls divide(Object, Object), which throws ArithmeticException if the second arg is 0 (regardless of the first arg).

In the second case it's parsed as divide(double, long) which just relies on Java to properly upcast the primitive long to a double to do the divide.

Note that making this call with 2 doubles does return NaN:

user=> (def x Double/NaN)
#'user/x
user=> (/ x 0.0)
NaN

or type hinting x to a double works as well:

user=> (def x Double/NaN)
#'user/x
user=> (/ ^double x 0.0)
NaN

I think one option to "fix" this behavior would be to add checks in divide(Object, Object) to check whether x is NaN and instead return NaN.





[CLJ-1368] Document usage for case with non-readable constants Created: 02/Mar/14  Updated: 02/Mar/14

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

Type: Enhancement Priority: Minor
Reporter: Herwig Hochleitner Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docs, interop


 Description   

Problem

It is pretty obscure how to get constant-time dispatch for e.g. Enums, even if user knows about case.

Proposal

The possibility to dispatch to arbitrary constants with case, by wrapper macro, should be documented.

Wording

  • Should it warn against doing that with unstable values?
  • Should it mention anything else than java Enums?

Case Techniques

Case is documented for accepting all readable forms as test-constants. However, it can also be made to use any compile-time-known constants as test-constants, by wrapping it in another macro.

Sometimes this is appropriate, e.g. when dispatching on a java Enum.
Other times, less so, e.g. when dispatching on objects whose hash changes when the vm is restarted (breaks AOT).

Implications

This technique is an application of a more general technique: Passing non-literals to a macro from another macro.
Are there other macros that have use cases like this?

References

https://groups.google.com/d/topic/clojure/3yGjDO2YnjQ/discussion



 Comments   
Comment by Herwig Hochleitner [ 02/Mar/14 11:25 AM ]

This is a duplicate of http://dev.clojure.org/jira/browse/CLJ-1367

Actually, it's an alternate solution





[CLJ-1367] Allow case statement to compare java constants Created: 02/Mar/14  Updated: 02/Mar/14

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

Type: Enhancement Priority: Minor
Reporter: Adam Clements Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: interop


 Description   

As raised on the mailing list: https://groups.google.com/forum/#!topic/clojure/3yGjDO2YnjQ

It's not possible to use java constants in a case statement. condp = could be used in this case but these are things which could be used in a java switch statement and so it's annoying to give up constant time dispatch. For example:

(case (.getActionMasked event)
MotionEvent/ACTION_POINTER_DOWN :down
MotionEvent/ACTION_UP :up
MotionEvent/ACTION_POINTER_UP :up
MotionEvent/ACTION_MOVE :move
MotionEvent/ACTION_CANCEL :cancel
MotionEvent/ACTION_OUTSIDE :outside
:none))

Doesn't work, but there is no reason this couldn't be resolved at compile time and dispatched in constant time.



 Comments   
Comment by Herwig Hochleitner [ 02/Mar/14 11:32 AM ]

Another solution for this problem: http://dev.clojure.org/jira/browse/CLJ-1368





[CLJ-1366] The empty map literal is read as a different map each time Created: 01/Mar/14  Updated: 02/Mar/14

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

Type: Enhancement Priority: Minor
Reporter: OHTA Shogo Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: memory, reader

Attachments: Text File 0001-make-the-reader-return-the-same-empty-map-when-it-re.patch     Text File 0002-make-the-reader-return-the-same-empty-map-when-it-re.patch    
Patch: Code

 Description   

As reported here (https://groups.google.com/forum/?hl=en#!topic/clojure-dev/n83hlRFsfHg), the empty map literal is read as a different map each time.

user=> (identical? (read-string "{}") (read-string "{}"))
false

Making the reader return the same empty map when it reads an empty map is expected to improve some memory efficiency, and also lead to consistency with the way other collection literals are read in.

user=> (identical? (read-string "()") (read-string "()"))
true
user=> (identical? (read-string "[]") (read-string "[]"))
true
user=> (identical? (read-string "#{}") (read-string "#{}"))
true

Cause: LispReader calls RT.map() with an empty array when it reads an empty map, and RT.map() in turn makes a new map unless its argument given is null.

Approach: make RT.map() return the same empty map when the argument is an empty array as well, not only when null



 Comments   
Comment by OHTA Shogo [ 01/Mar/14 2:59 AM ]

Sorry, the patch 0001-make-the-reader-return-the-same-empty-map-when-it-re.patch didn't work.

The updated patch 0002-make-the-reader-return-the-same-empty-map-when-it-re.patch works, but I'm afraid it'd be beyond the scope of this ticket since it modifies RT.map() behavior a bit.





[CLJ-1364] Primitive VecSeq does not implement equals or hashing methods Created: 19/Feb/14  Updated: 19/Feb/14

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

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


 Description   

VecSeq (as produced by (seq (vector-of :int 1 2 3))) does not implements equals, hashCode, or hasheq and does not play with any other Clojure collections or sequences appropriately in this regard.

user=> (def rs (range 3))
user=> (def vs (seq (vector-of :int 0 1 2)))
user=> rs
(0 1 2)
user=> vs
(0 1 2)
user=> (.equals rs vs)
true
user=> (.equals vs rs)    ;; expect: true
false
user=> (.equiv rs vs)
true
user=> (.equiv vs rs)
true
user=> (.hashCode rs)
29824
user=> (.hashCode vs)     ;; expect to match (.hashCode rs)
2081327893
user=> (System/identityHashCode vs)  ;; show that we're just getting Object hashCode
2081327893
user=> (.hasheq rs)
29824
user=> (.hasheq vs)       ;; expect same as (.hasheq rs) but not implemented at all
IllegalArgumentException No matching field found: hasheq for class clojure.core.VecSeq  clojure.lang.Reflector.getInstanceField (Reflector.java:271)





[CLJ-1360] clojure.string/split strips trailing delimiters Created: 18/Feb/14  Updated: 18/Feb/14

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

Type: Defect Priority: Major
Reporter: Tim McCormack Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None


 Description   

clojure.string/split and clojure.string/split-lines inherit the bizarre default behavior of java.lang.String#split(String,int) in stripping trailing consecutive delimiters:

(clojure.string/split "banana" #"an")
⇒ ["b" "" "a"]
(clojure.string/split "banana" #"na")
⇒ ["ba"]
(clojure.string/split "nanabanana" #"na")
⇒ ["" "" "ba"]

In the case of split-lines, processing a file line by line and rejoining results in truncation of trailing newlines in the file. In both cases, the behavior is surprising and cannot be inferred from the docstrings.

This behavior should either be fixed or documented.



 Comments   
Comment by Andy Fingerhut [ 18/Feb/14 10:51 AM ]

Probably documenting would be safer than changing the behavior at this point, given that some people may actually rely on the current behavior after testing, deploying, etc.

I don't currently have a suggestion for a modified doc string, but note that there are examples of this behavior and how one can use an extra "-1" limit argument at the end to get all split strings: http://clojuredocs.org/clojure_core/clojure.string/split





[CLJ-1358] doc macro does not expand special cases properly Created: 17/Feb/14  Updated: 17/Feb/14

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

Type: Defect Priority: Minor
Reporter: Chad Taylor Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: repl

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

 Description   

The doc macro supports three special cases, mapping & to fn, catch to try, and finally to try. However, the macro does not currently expand these cases - it executes them like a function instead. This is evident if you use the following at a REPL:

user> (macroexpand '(doc try))   ;; ok
((var clojure.repl/print-doc) ((var clojure.repl/special-doc) (quote try)))

user> (macroexpand '(doc catch)) ;; broken
;; -- unexpectedly prints try doc -- ;;
nil

user> (= (with-out-str (doc catch)) (with-out-str (doc try))) ;; broken, expect true
;; -- unexpectedly prints try doc -- ;;
false

Workaround: Call doc with the symbol to which the special case is mapped, fn or try.

Cause: Incorrect quoting when handling special cases in doc macro

Solution: Update special case quoting approach to match the other cases.

Patch: CLJ-1358.patch



 Comments   
Comment by Chad Taylor [ 17/Feb/14 10:41 PM ]

Adding a patch with code and test.





[CLJ-1347] finalize won't work in reified objects - document Created: 10/Feb/14  Updated: 01/Mar/14

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

Type: Enhancement Priority: Minor
Reporter: Jozef Wagner Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

java 7



 Description   

Finalize is called for reified objects even when they are still reachable. It gets called second time at proper time.

user=> (def x (reify Object (finalize [o] (println "OH MY!"))))
#'user/x
user=> (System/gc)
nil
OH MY!
user=> x
#<user$reify__1496 user$reify__1496@53fb35af>
user=> (System/gc)
nil
user=> (def x nil)
#'user/x
user=> (System/gc)
nilOH MY!

Deftype seems to work fine

user=> (deftype T [] Object (finalize [o] (println "great success")))
user.T
user=> (def y (->T))
#'user/y
user=> (System/gc)
nil
user=> (def y nil)
#'user/y
user=> (System/gc)
great success


 Comments   
Comment by Alex Miller [ 10/Feb/14 8:38 AM ]

Just a note: the calls to System/gc don't necessarily cause finalizers to run on the first try - sometimes it took more than one for that to succeed for me. You'd think System/runFinalizers would do it but I had no luck at all with that.

Comment by Gary Fredericks [ 13/Feb/14 10:01 PM ]

reify actually creates two objects – the first is created by reify*, and then reify immediately calls with-meta on it, creating a copy.

The docstring sort of describes this behavior: "reify always implements clojure.lang.IObj and transfers meta data of the form to the created object."

Comment by Jozef Wagner [ 14/Feb/14 5:01 AM ]

Oh, so finalizer is a no-no in reify. Should be mentioned in docs IMO.

Comment by Gary Fredericks [ 14/Feb/14 6:28 AM ]

Just for fun you could do something tricksy like:

^::second-object
(reify Object
  (finalize [self]
    (when (::second-object (meta self))
      ...)))

(have not actually run this)

Comment by Gary Fredericks [ 01/Mar/14 1:36 PM ]

It looks like the class generated by reify always has a constructor that takes a metadata argument, so it doesn't seem out of the question to eliminate the extra object altogether.

I'll try to keep digging on this.





[CLJ-1346] clojure.core.VecSeq does not implement method equals Created: 09/Feb/14  Updated: 09/Feb/14

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

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


 Description   

.equals is asymmetric for seq's of primitive vectors and PersistentVectors, because clojure.core.VecSeq does not implement Java's equals method. It implements equiv, so clojure.core/= is fine:

user=> (def v1 [1 2 3])
#'user/v1
user=> (def v2 (vector-of :long 1 2 3))
#'user/v2
user=> (= v1 v2)
true
user=> (.equals v1 v2)
true
user=> (= (seq v1) (seq v2))
true
user=> (.equals (seq v1) (seq v2))
true
user=> (= v2 v1)
true
user=> (.equals v2 v1)
true
user=> (= (seq v2) (seq v1))
true
;; This is the one that is not like the others, and a symptom of the problem
user=> (.equals (seq v2) (seq v1))
false





[CLJ-1342] Byte comparison boxes both bytes and converts to longs to compare (which is slow) Created: 06/Feb/14  Updated: 06/Feb/14

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: compiler

Attachments: File bytebox.clj    

 Description   

This came up in a much more complicated example but consider a case like this:

(defn simple []
  (let [b (byte-array [(byte 0)])
        m (byte 0)]
    (= m (aget b 0))))

In the compiled bytecode, both m and (aget b 0) are known to be bytes, but both are boxed using Byte.valueOf(), then cast using RT.uncheckedLongCast() and finally compared as longs:

26: iload_2
  27: invokestatic  #69  // Method java/lang/Byte.valueOf:(B)Ljava/lang/Byte;
  30: checkcast     #81  // class java/lang/Number
  33: invokestatic  #85  // Method clojure/lang/RT.uncheckedLongCast:(Ljava/lang/Object;)J

In a tight loop manipulating and matching against byte arrays, this boxing is significant for performance.

Attached is a test that demonstrates the performance difference between the byte[] and long[] performance to get an idea of the difference.



 Comments   
Comment by Nicola Mometto [ 06/Feb/14 9:10 PM ]

The description states that Util.equiv() has a byte/byte comparison variant but it doesn't look like it actually exists.

Comment by Nicola Mometto [ 06/Feb/14 9:17 PM ]

By the way, tools.emitter.jvm uses i2l to cast the byte to a long instead of boxing && unboxing to a long

Comment by Alex Miller [ 06/Feb/14 9:39 PM ]

Thanks Nicola - I must have confused it with the boolean/boolean version.





[CLJ-1341] keyword function returns nil on bad input Created: 05/Feb/14  Updated: 14/Feb/14

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

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

Attachments: Text File keyword-1341-2014-02-12.2.patch     Text File keyword-1341-2014-02-12.patch    
Patch: Code and Test

 Description   

The keyword function should throw an exception on bad input rather than return nil.

user=> (keyword 5)
nil
user=> (keyword [])
nil

Cause: The keyword function is defined as a cond with cases for keywords, symbols, and strings. There is no :else so all other cases return nil.

Proposal: Add an :else branch and throw an exception in keyword.

Patch:



 Comments   
Comment by Eric Normand [ 12/Feb/14 7:17 PM ]

The keyword function should throw an IllegalArgumentException on wrong argument type rather than return nil. For consistency, the two-argument case should throw an IllegalArgumentException if not both arguments are strings.

The find-keyword function should behave similarly to maintain the same signature.

Current behavior:

user=> (keyword 5)
nil
user=> (keyword [])
nil
user=> (keyword 1 1)
java.lang.ClassCastException: java.lang.Long cannot be cast to java.lang.String

Cause: The keyword function is defined as a cond with cases for keywords, symbols, and strings. There is no :else so all other cases return nil. The same goes for find-keyword, which should behave in the same way. The two-argument case does throw an exception, but the message is not very helpful.

Proposal: I have added an :else branch to the cond that throws an IllegalArgumentException with a message that indicates the acceptable types and prints the actual argument. I made the same change to find-keyword. There are also simple tests.

Patch: keyword-1341-2014-02-12.patch

Note: This change does not check for all bad input, just the type. For instance, it is still possible to pass in a string with "illegal" keyword characters.

Comment by Alex Miller [ 12/Feb/14 9:20 PM ]

Hey Eric, thanks for the patch! The 1 arg change looks good.

On the 2 arg change I have a concern - I'm worried that we are adding new checks into a pretty hot code path (keyword creation). The 2 arg path is not a silent failure as you'll get a ClassCastException so I do not think adding these checks here is worth it. In the 1-arg case you've already fallen through the else, so there's no additional cost.

Comment by Eric Normand [ 12/Feb/14 9:35 PM ]

Understood. I'll remove the two-argument case.

Comment by Eric Normand [ 12/Feb/14 9:51 PM ]

The keyword function should throw an IllegalArgumentException on wrong argument type rather than return nil. The two-argument case already throws an exception.

The find-keyword function should behave similarly to maintain the same signature.

Current behavior:

user=> (keyword 5)
nil
user=> (keyword [])
nil
user=> (keyword 1 1)
java.lang.ClassCastException: java.lang.Long cannot be cast to java.lang.String

Cause: The keyword function is defined as a cond with cases for keywords, symbols, and strings. There is no :else so all other cases return nil. The same goes for find-keyword, which should behave in the same way.

Proposal: I have added an :else branch to the cond that throws an IllegalArgumentException with a message that indicates the acceptable types and prints the actual argument. I made the same change to find-keyword. There are also simple tests.

Alternatives: Adding checks for the two-argument case was considered but it was feared that adding the extra overhead was not worth it since it already threw an exception. No significant overhead is added in the single-argument case since it will only affect erroneous input.

Patch: keyword-1341-2014-02-12.2.patch

Note: This change does not check for all bad input, just the type. For instance, it is still possible to pass in a string with "illegal" keyword characters.





[CLJ-1340] Emit unboxed cohercions from int/long to float/double Created: 05/Feb/14  Updated: 05/Feb/14

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

Type: Enhancement Priority: Minor
Reporter: Christophe Grand Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: enhancement, performance

Attachments: File primitive-cohercion.diff    
Patch: Code

 Description   

Currently when an int or long is used where a float or double is expected, boxed conversion happens instead of emitting [IL]2[FD] instructions.






[CLJ-1337] defprotocol's docstring is out of date Created: 04/Feb/14  Updated: 04/Feb/14

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

Type: Defect Priority: Trivial
Reporter: OHTA Shogo Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring

Attachments: Text File 0001-Update-defprotocol-s-docstring-to-remove-an-out-of-d.patch    
Patch: Code

 Description   

The docstring of defprotocol has an out-of-date description as follows:

defprotocol is dynamic, has no special compile-time effect, and defines no new types or classes.

Indeed, it used to have no compile-time effect. Today, however, it does generate a corresponding interface via gen-interface at compile time.

The patch just removes this description.






[CLJ-1333] Documentation for "=" is misleading Created: 30/Jan/14  Updated: 14/Feb/14

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

Type: Enhancement Priority: Minor
Reporter: George Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docs
Environment:

linux 2.6.32-431.el6.x86_64



 Description   

Document for clojure.core/= says it compares numbers in a type-independent manner. In reality the comparission is made in a type dependent manner. If the above statement was true then (= 1 1.0) would eval to true not false;

clojure.core/=
([x] [x y] [x y & more])
Equality. Returns true if x equals y, false if not. Same as
Java x.equals except it also works for nil, and compares
numbers and collections in a type-independent manner. Clojure's immutable data
structures define equals() (and thus =) as a value, not an identity,
comparison.



 Comments   
Comment by Kevin Downey [ 02/Feb/14 4:58 PM ]

I think this is a little more complex than described.

= does compare things in a jvm type independent manner, but it does use what people have taken to calling "equality classes"

(= [1 2] '(1 2))

(= {:a 1} (doto (java.util.HashMap.) (.put :a 1)))

etc.

now for numbers, it seems logical to me, to have floating point and precise numbers in distinct equality classes

in which case, 1.0 and 1 are in distinct equality classes, so not equal.

Comment by Gary Fredericks [ 13/Feb/14 10:16 PM ]

The docstring is definitely misleading for people unfamiliar with this sort of thing though. Numbers are probably the first thing that the words "type independent manner" bring to mind. A brief pointer to the == function might be useful.

Comment by George [ 14/Feb/14 5:47 AM ]

I find == function to be confusing
For example
(== 1 1.0) => true
(== 1 1.0M) => false ; what is wrong with this comparison?

and doc says:
Returns non-nil if nums all have the equivalent value (type-independent)

Comment by Alex Miller [ 14/Feb/14 7:56 AM ]

@George - that last example (== 1 1.0M) is actually a bug that is fixed in 1.6 where it will return true.





[CLJ-1332] Exceptions are not cached in lazy seqs Created: 29/Jan/14  Updated: 13/Feb/14

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

Type: Enhancement Priority: Minor
Reporter: Yongqian Li Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None


 Description   

It is confusing that exceptions will only be thrown once when it is possible to iterate over a seq many times.

user=> (def a (for [i (reverse (range 2))] (/ 1 i)))
#'user/a
user=> (println a)

ArithmeticException Divide by zero clojure.lang.Numbers.divide
(Numbers.java:156)
(user=> (println a)
(1)
nil
user=> (println a)
(1)
nil



 Comments   
Comment by Gary Fredericks [ 13/Feb/14 10:29 PM ]

The cause of this is the lazy-seq macro which uses :once metadata to signal to the compiler that the thunk it creates will only be called once.

When the evaluation of a lazy seq throws an exception, trying to walk the seq again causes the function to be called a second time. Since its closed over values have likely been cleared by that point, you get different behavior.

Glancing at LazySeq.java made me pretty convinced you can't cache exceptions without adding an extra check somewhere in the standard codepath for lazy seq traversal.

Comment by Yongqian Li [ 13/Feb/14 11:38 PM ]

Btw, I ran into this issue while trying to evaluate a lazy-seq in a future in order to do some processing concurrently in the background. Any suggestions for workarounds?





[CLJ-1327] Clojure Primitives extend Serializable without serialVersionUID Created: 20/Jan/14  Updated: 20/Jan/14

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

Type: Defect Priority: Minor
Reporter: Kuldeep Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Linux



 Description   

Clojure keywords for instance are serializable but do not define a serialVersionUID.






[CLJ-1326] Inconsistent reflection warnings when target is a literal Created: 19/Jan/14  Updated: 05/Feb/14

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

Type: Enhancement Priority: Trivial
Reporter: Nicola Mometto Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: errormsgs


 Description   
user=> (set! *warn-on-reflection* true)
true
user=> (.get {} 0)
Reflection warning, NO_SOURCE_PATH:2:1 - call to get can't be resolved.
nil
user=> (.get {1 1} 0)
nil
user=> (.get ^:foo {1 1} 0)
Reflection warning, NO_SOURCE_PATH:4:1 - call to get can't be resolved.
nil
user=> (.get {1 (inc 0)} 0)
Reflection warning, NO_SOURCE_PATH:5:1 - call to get can't be resolved.
nil

Similar issues apply to other literals (vector literals, list literals)






[CLJ-1324] Allow leading slashes in unqualified symbol names Created: 15/Jan/14  Updated: 02/Aug/14

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

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

All


Attachments: Text File clj-1324-1.patch    
Patch: Code and Test

 Description   

The proposal is to allow the reader to accept symbol names with leading slashes.

Problem: Leading slashes are frequently useful, e.g. in mathematical operators like "/" or "/="

Currently, only "/" is allowed as a special case, and is used for the division operator in clojure.core

This could be extended to allow all symbols to have names starting with a leading slash.

There should be no ambiguity with namespace-qualified symbols:
1) In the case of a leading slash, a symbol should be interpreted as an unqualified symbol e.g. "/="
2) In the case of a slash anywhere except in leading position, it should considered as namespace qualified, e.g. "clojure.core/+"
3) In the case of multiple non-leading slashes, the first slash is the namespace separator, e.g. "clojure.core.matrix.operators//="

Optionally, it also would be possible to allow multiple slashes after the leading slash in a name. This would allow symbols such as "/src/main/clojure" to become valid.



 Comments   
Comment by Paavo Parkkinen [ 10/Feb/14 7:32 AM ]

Attached patch to allow leading slashes in symbol names.

The patch changes the regexp pattern used to match symbols to accept characters after a slash in symbol names.

Tests pass, and the patch also adds a couple of new special cases to the symbol tests.

Comment by Andy Fingerhut [ 01/Aug/14 9:26 PM ]

Paavo, a commit made to Clojure master earlier today causes your patch clj-1324-1.patch to no longer apply cleanly. I haven't investigated in detail, but it might be straightforward to update the patch so that it applies cleanly again.

Comment by Paavo Parkkinen [ 02/Aug/14 7:37 PM ]

Attached updated patch.

Comment by Andy Fingerhut [ 02/Aug/14 8:40 PM ]

It would be less confusing if you could name the patches differently, or remove the older one.





[CLJ-1323] AsmReflector throws exceptions on JDK8 Created: 13/Jan/14  Updated: 23/Mar/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: None

Attachments: File clj-1323-disable.diff    

 Description   

After the commit of the updated ASM library for CLJ-713, Clojure builds and passes all tests except for one, compare-reflect-and-asm in reflect.clj.

This can be narrowed down somewhat to a difference in behavior of the following 2 forms evaluated with the latest Clojure and JDK8:

;; The following two lines work with the latest (Jan 11 2014) Clojure 1.6.0-master-SNAPSHOT
;; if run on JDK 6 or JDK 7, but throw an exception with JDK 8.

(import '[clojure.asm ClassReader ClassVisitor Type Opcodes])
(def r (ClassReader. "java.lang.Object"))

I am not certain, but from a bit of Google searching it appears that this may be a limitation of the ASM library version 4 – it throws exceptions when attempting to read class files produced by JDK 8, because of a newer classfile version number. Links that seem to support this conclusion:

http://mail-archive.ow2.org/asm/2013-02/msg00000.html

http://forge.ow2.org/tracker/index.php?func=detail&aid=316375&group_id=23&atid=350023

A couple of alternatives are:

(1) update ASM again to one that supports JDK 8 class files

(2) disable the compare-reflect-and-asm test. Clojure itself does not use the AsmReflector for anything except this unit test. The Java reflector is the default one.



 Comments   
Comment by Alex Miller [ 13/Jan/14 8:16 AM ]

1) There is no released ASM that supports JDK 8 yet. ASM 5 will but it will not be final till JDK 8 is in the final stages of release.

2) Probably more likely.

Comment by Nicola Mometto [ 19/Mar/14 9:01 AM ]

As of now, both JDK8 and ASM5 are out.
I just tried compiling clojure on JDK8 with ASM5 and all compiles fine

Comment by Alex Miller [ 19/Mar/14 9:07 AM ]

How are you running this test?

Comment by Nicola Mometto [ 19/Mar/14 9:11 AM ]

I downloades ASM5, replaced the bundled ASM that comes with clojure with that one after changing che package name to "clojure.asm" and run `mvn install`, all the tests pass.

Comment by Alex Miller [ 19/Mar/14 9:24 AM ]

I was actually talking about the JDK 8 change - was curious about exactly what was being changed?

Comment by Alex Miller [ 19/Mar/14 10:04 AM ]

In particular, I'm assuming that you're not altering the build.xml to change the compilation -source or -target and running with JAVA_HOME / path set to JDK 8.

We don't have any plans to actually build Clojure with JDK 8 any time soon, so I'm not overly concerned about that. But it does appear that the embedded ASM 4 cannot read newer class files from JDK 8. Afaik, the only place that happens is in clojure.reflect.java in the AsmReflector, which is not the default reflector. The JavaReflector will properly reflect the Java 8 classes.

ASM 5 has only been out a couple days and already has at least one serious bug reported - I'd like that to see more use before we switch to it, so maybe this is a good target for the release after Clojure 1.6.

Comment by Alex Miller [ 23/Mar/14 12:17 PM ]

Patch to temporarily disable the failing test until we have an ASM that supports JDK 8.





[CLJ-1321] Documentation improvement for clojure.walk, to note use of recursion that can easily blow the JVM stack Created: 09/Jan/14  Updated: 09/Jan/14

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

Type: Enhancement Priority: Minor
Reporter: Lee Spector Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: documentation
Environment:

JVM



 Description   

prewalk and postwalk are use recursion in ways that will blow the stack for sufficiently deep nested structures.

I suggest that this be noted in various clojure.walk documentation strings since this kind of recursion/limit seems to be rare in Clojure, and hence will be unexpected.

(It'd be even better to remove the recursion/limit via something like zippers and iteration, but this issue is just a suggestion for improvement of the documentation.)






[CLJ-1319] array-map fails lazily if passed an odd number of arguments Created: 08/Jan/14  Updated: 30/Jan/14

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

Type: Enhancement Priority: Minor
Reporter: Stuart Sierra Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: errormsgs

Attachments: Text File 0001-CLJ-1319-Throw-on-odd-arguments-to-PersistentArrayMa.patch    
Patch: Code and Test

 Description   

If called with an odd number of arguments, array-map does not throw an exception until the map is realized, when it throws the confusing ArrayIndexOutOfBoundsException.

Example, in 1.5.1 and 1.6.0-alpha3:

user=> (def m (array-map :a 1 :b))
#'user/m

user=> (prn m)
ArrayIndexOutOfBoundsException 3
  clojure.lang.PersistentArrayMap$Seq.first
  (PersistentArrayMap.java:313)


 Comments   
Comment by Alex Miller [ 08/Jan/14 11:01 AM ]

PersistentArrayMap.createAsIfByAssoc could check length is even to catch this

Comment by Jason Felice [ 27/Jan/14 1:01 PM ]

A better error message would be nice... this is the best I could think of.





[CLJ-1317] clojure.zip/seq-zip returns spurious nils during traversal Created: 31/Dec/13  Updated: 05/Feb/14

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

Type: Defect Priority: Minor
Reporter: Michał Marczyk Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: zip

Attachments: Text File 0001-CLJ-1317-fix-seq-zip-to-avoid-spurious-nils.patch    
Patch: Code

 Description   

Problem reported by Lee Spector on the mailing list:

https://groups.google.com/d/msg/clojure/8TL7IGmE7N0/u1xfgTOLDRgJ

Here's a quote from Lee's post describing the problem:

Here's an illustration, stepping through '(() 0) with next and printing the node at each step: 

(loop [z (zip/seq-zip '(() 0))] 
  (if (zip/end? z) 
    :done 
    (do (println (zip/node z)) 
      (recur (zip/next z))))) 

That produces: 

(() 0) 
() 
nil 
0 
:done 

I don't expect the nil to be there. 

The underlying cause is that seq-zip passes identity as the children argument to zipper. Applied to (), this returns (), which is truthy, leading zipper to descend into a non-existent subtree.

One natural solution would be to use seq in place of identity:

(defn seq-zip [root]
  (zipper seq?
          seq  ;; changed
          (fn [node children] (with-meta children (meta node)))
          root))

With this change, no nil is produced in the example above. Patch with this change forthcoming.



 Comments   
Comment by Michał Marczyk [ 31/Dec/13 5:52 PM ]

Note that the docstring of clojure.zip/zipper asks that the children argument return a seq of children. The rest of clojure.zip, however, expects nil to be returned when there are no children, as evidenced by this problem.

One could argue that this behaviour of the rest of clojure.zip should be fixed, but I think it makes sense and is convenient. Perhaps the docstring should be adjusted, though.





[CLJ-1314] Correct placement of doc string for function bubble-max-key Created: 23/Dec/13  Updated: 05/Feb/14

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

Type: Enhancement Priority: Trivial
Reporter: Andy Fingerhut Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring

Attachments: File clj-1314-v2.diff    
Patch: Code

 Description   

It is private, defined with defn-, so perhaps the doc string is superfluous, but someone wrote one, and it is in the wrong place relative to the args:

(defn- bubble-max-key [k coll]
  "Move a maximal element of coll according to fn k (which returns a number) 
   to the front of coll."
  (let [max (apply max-key k coll)]
    (cons max (remove #(identical? max %) coll))))

Found using a pre-release version of the Eastwood Clojure lint tool.



 Comments   
Comment by Andy Fingerhut [ 23/Dec/13 4:11 PM ]

Patch clj-1314-v1.diff simply corrects the location of the doc string for bubble-max-key.

Comment by Andy Fingerhut [ 02/Jan/14 11:05 AM ]

Patch clj-1314-v2.diff is same as the previous clj-1314-v1.diff, except it leaves out some unintended changes.





[CLJ-1311] gen-interface uses DynamicClassLoader when not compiling, gen-class doesn't Created: 20/Dec/13  Updated: 05/Feb/14

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

Type: Enhancement Priority: Minor
Reporter: Joel Kaasinen Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring, gen-class, interop


 Description   

The documentation for both gen-class and gen-interface says: "When not compiling, does nothing."

However, gen-interface does the right thing and uses DynamicClassLoader.defineClass when not compiling. This means e.g. that gen-interface works from the repl.

I don't see a reason why gen-class couldn't do the same. Obviously, the docstrings would need to be updated too.






[CLJ-1309] Bindings after :as in list destructuring should throw error Created: 19/Dec/13  Updated: 05/Feb/14

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

Type: Enhancement Priority: Minor
Reporter: ben wolfson Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: Compiler, errormsgs


 Description   

If you try to define a vector binding with anything at all after an :as parameter, you do not get a compiler error, and the binding is silently swallowed:

user> ((fn [[:as y z]] y) [1 2])
[1 2]

If you try to actually use the binding, there will be a compiler error (the compiler will complain that there's no binding for the symbol), but the actual error has already happened, and should be reported earlier.






[CLJ-1308] extend-type doesn't type-hint correctly as promised by the doc when the class is determined at run-time Created: 15/Dec/13  Updated: 05/Feb/14

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: typehints


 Description   

extend-type works with non-constant expressions as its type:

(extend-type (class 1) proto (protof [this]))

However, in this case this will get tagged with `(class 1)`, which is clearly wrong; the doc explicitely states that the args will be proberly type-hinted: "[..] Propagates the class as a type hint on the first argument of all fns."

I don't know if extend-type is not supposed to work with non-constant Classes, in which case it should be stated in the doc or if the current behaviour is wrong.






[CLJ-1300] take-while with n<1 behaves like (repeat 1) Created: 22/Nov/13  Updated: 22/Nov/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.1, Release 1.2, Release 1.3, Release 1.4, Release 1.5, Release 1.6
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Yaron Peleg Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   
user=> (take-nth -1 [1 3 4])
; hangs
user=> (take 5 (take-nth -1 [5 9 14]))
(1 1 1 1 1)

I understand this behavior may be intentionally undefined,
but raising the issue on IRC didn't yield an answer on whether
this is a bug or grey area.






[CLJ-1296] locking expressions cause vars to be dereferenced, even if not executed, unless wrapped in let Created: 17/Nov/13  Updated: 18/Apr/14

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

Type: Enhancement Priority: Minor
Reporter: Andy Fingerhut Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: performance


 Description   

Description of one example with poor performance discovered by Michał Marczyk in the discussion thread linked below.

The difference between the compiled versions of:

(defn foo [x]
  (if (> x 0)
    (inc x)
    (locking o
      (dec x))))

and

(defn bar [x]
  (if (> x 0)
    (inc x)
    (let [res (locking o
                (dec x))]
      res)))

is quite significant. foo gets compiled to a single class, with invocations handled by a single invoke method; bar gets compiled to a class for bar + an extra class for an inner function which handles the (locking o (dec x)) part – probably very similar to the output for the version with the hand-coded locking-part (although I haven't really looked at that yet). The inner function is a closure, so calling it involves an allocation of a closure object; its ctor receives the closed-over locals as arguments and stores them in two fields (lockee and x). Then they get loaded from the fields in the body of the closure's invoke method etc.

Note: The summary line may be too narrow a description of the root cause, and simply the first example of a case where this issue was noticed and examined. Please make the summary and this description more accurate if you diagnose this issue.

See discussion thread on Clojure group here: https://groups.google.com/forum/#!topic/clojure/x86VygZYf4Y



 Comments   
Comment by Kevin Downey [ 18/Apr/14 2:17 AM ]

maybe it is already clear to others, but this was not immediately clear to me:

the reason

(defn bar [x]
  (if (> x 0)
    (inc x)
    (let [res (locking o
                (dec x))]
      res)))

generates a second class is locking is a macro that contains a try/finally form in it's expansion.

binding the result of a try/finally form to a result (as in the let) would require some real tricky code gen without adding the extra function, so of course the clojure compile adds the extra function.





[CLJ-1292] print docstring should specify nil return value Created: 01/Nov/13  Updated: 06/Nov/14

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

Type: Enhancement Priority: Trivial
Reporter: Phill Wolf Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring, print

Attachments: File clj-1292.diff    

 Description   

The docstring for print does not mention its return value. The docstring should clarify whether print dependably returns nil or shouldn't be depended on to (lest, for example, something leak out as the inadvertent return value of print's caller).



 Comments   
Comment by Édipo L Féderle [ 06/Nov/14 7:46 PM ]

Hi, I just add to docstring the mention to fact that it return nil





[CLJ-1291] struct-map docstring incomplete, inconsistent Created: 01/Nov/13  Updated: 05/Feb/14

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

Type: Enhancement Priority: Trivial
Reporter: Phill Wolf Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring


 Description   

The docstring for struct-map refers to "structure-basis" and "keyvals" while the parameters are "s" and "inits". The docstring says "keyvals can also contain keys not in the basis" but does not say what happens in that case.






[CLJ-1286] Fix reader spec and regex to match code for keywords starting with digits Created: 31/Oct/13  Updated: 10/Dec/14

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

Type: Defect Priority: Minor
Reporter: Alex Miller Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: reader


 Description   

The reader page at http://clojure.org/reader states that symbols (and keywords) cannot start with a number and the regex used in LispReader (and EdnReader) also has this intention. CLJ-1252 addressed this by fixing the broken reader regex to match the spec. However, that broke some existing code so we rolled back the change. There is still a disconnect here and this ticket serves to decide what to do instead.

I presume that we are effectively deciding that keywords like :5 are ok to read. If so, we should alter the regex to more accurately capture that intent - right now it allows these purely by accident due to backtracking. A secondary question is whether the Clojure and EDN reader spec should also explicitly allow these as valid. My preference would be to have the reader and the spec match, so I would lobby to loosen the reader spec.

Related ticket: CLJ-1527



 Comments   
Comment by Nicola Mometto [ 12/Nov/13 4:50 PM ]

what about keywords like :1/1 or :1/a? Clojure currently accepts the latter but not the former.

Comment by Francis Avila [ 02/Jul/14 3:13 PM ]

There's more discussion of this problem (and symbol/keyword parsing in general) in the context of cljs.reader at CLJS-677.

Comment by Andy Fingerhut [ 02/Jul/14 3:27 PM ]

Francis, can you double-check that ticket number? The one you mention (CLJS-667) doesn't seem to have any discussion of this problem.

Comment by Francis Avila [ 02/Jul/14 3:39 PM ]

Sincere apologies, it's CLJS-677. (Original post corrected too.)

Comment by Nicola Mometto [ 10/Dec/14 10:08 AM ]

From a discussion in #clojure, it emerged that while :foo/1 is currently not allowed, ::1 is.





[CLJ-1284] Clojure functions and reified objects should expose a public static field to identify their proper Clojure name Created: 24/Oct/13  Updated: 29/Aug/14

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

Type: Enhancement Priority: Minor
Reporter: Howard Lewis Ship Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: errormsgs

Attachments: Text File CLJ-1284-store-demunged-names.patch    

 Description   

There are several examples of frameworks that attempt to de-mangle a Java class name into a Clojure symbol (including namespace); this is useful for writing out an improved, Clojure-specific stack trace when reporting exceptions.

Existing libraries are based on regular expression matching and guesswork, and can occasionally give incorrect results, such as when a namespace or function name actually contains an underscore.

It would be helpful for authors of such frameworks if Clojure would expose a static final field on such classes with the proper name that should appear in the stack trace; libraries would then be able to use reflection to access the proper name of the field, without the current guesswork.

I would suggest CLOJURE_SOURCE_NAME as a reasonable name for such a field.

Other Clojure class constructs beyond functions, such as reified types and protocol implementations, would also benefit, though it is less obvious what exact string value would properly and unambiguously identify what purpose the class plays.



 Comments   
Comment by Alex Miller [ 24/Oct/13 8:31 PM ]

FYI, there is a patch on the way in for 1.6 that contains a new demunge function in Compiler. However, the munged name is not always reversible so having the original around is a good idea.

Comment by Andy Fingerhut [ 25/Oct/13 11:10 AM ]

The patch Alex is referring to is attached to CLJ-1083.

Comment by Andy Fingerhut [ 25/Oct/13 11:13 AM ]

Howard, there seems to be some overlap in the intent between this ticket and CLJ-1278. I guess either of them could be done without the other, but wanted to check.

Comment by Daniel Solano Gómez [ 20/Aug/14 2:17 PM ]

Here's an initial stab at adding this feature.

Some notes:

  • This will tag emitted classes from deftype and fn
  • This will handle fn}}s that are enclosed, but the output will be slightly different from the standard {{demunge function: only the initial $ is transformed to a /.
  • Unfortunately, because the defn for type/record constructor occurs in a let form, the generated symbol doesn't match what it should be.
  • There is no exposed API to get the demunged symbol from the class. Perhaps demunge should check if the given name corresponds to a class with this field?

I welcome any input on how this should really work. In particular, any ideas on how to best deal with {{defn}}s that are not top-level forms.

Comment by Andy Fingerhut [ 29/Aug/14 4:42 PM ]

Patch CLJ-1284-store-demunged-names.patch dated Aug 20 2014 does not apply cleanly to latest master after some commits were made to Clojure on Aug 29, 2014. I have not checked whether it applied cleanly before that day, nor have I checked how easy or difficult it might be to update this patch.





[CLJ-1283] Method clojure.lang.RT.lastModified(URL, String) fails if 'jar' protocol is not handled by JarURLConnector. Created: 24/Oct/13  Updated: 24/Oct/13

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

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

Attachments: Text File fix.patch    
Patch: Code

 Description   

Method clojure.lang.RT.lastModified(URL, String) throws a ClassCastException if it is called in environment, where 'jar' protocol is not handled by standard JarURLConnector. The provided patch just adds a type check for that case. I ran into this problem when I tried to add a Clojure REPL to existing Java application.



 Comments   
Comment by Alex Miller [ 24/Oct/13 8:26 AM ]

Hi TV, thanks for the report! Just a note, that we can't accept your patch unless you have signed a Contributor's Agreement as per http://clojure.org/contributing.

Just out of curiosity, is this an internal environment where you've customized the jar protocol or some kind of app server someone else would be likely to encounter?

Comment by TV [ 24/Oct/13 8:44 AM ]

It is an internal environment and so I set the priority to minor.





[CLJ-1280] Create reusable exception that can carry file/line/col info Created: 18/Oct/13  Updated: 18/Apr/14

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

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


 Description   

This concept already exists in multiple places in Clojure - Compiler$CompilerException and the Exception classes buried in EdnReader and LispReader. It would also be useful in other places where IllegalArgument or other other exceptions are thrown.

For example, this protocol exception throws an IllegalArgumentException and could transmit the file, line, and column info at the location of the error but it seems weird to use any of the existing exceptions for this purpose.

(defprotocol Bar (m [this]) (m [this arg]))


 Comments   
Comment by Kevin Downey [ 18/Apr/14 12:09 AM ]

seems like ExceptionInfo can do this





[CLJ-1279] Fix confusing macroexpand1 ArityException handling Created: 16/Oct/13  Updated: 05/Feb/14

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

Type: Defect Priority: Major
Reporter: Alex Coventry Assignee: Unassigned
Resolution: Unresolved Votes: 3
Labels: Compiler, errormsgs, macro

Attachments: Text File 0001-Edit-macro-ArityException-in-AFn.patch     Text File 0001-Fix-macroexpand1-s-handling-of-ArityException.patch    
Patch: Code and Test

 Description   

macros can give very confusing error messages when they execute a form which generates an ArityException. clojure.lang.Compiler.macroexpand1 assumes that any ArityException comes from the call to the macro itself, which need not be the case. For instance:

user> (do (defmacro f [] (assoc)) (f))
ArityException Wrong number of args (-2) passed to: core$assoc clojure.lang.Compiler.macroexpand1 (Compiler.java:6488)
user> (use 'clojure.repl) (pst)
nil
ArityException Wrong number of args (-2) passed to: core$assoc
clojure.lang.Compiler.macroexpand1 (Compiler.java:6488)
clojure.lang.Compiler.macroexpand (Compiler.java:6544)
clojure.lang.Compiler.eval (Compiler.java:6618)
clojure.lang.Compiler.eval (Compiler.java:6624)
clojure.lang.Compiler.eval (Compiler.java:6597)
clojure.core/eval (core.clj:2864)
clojure.main/repl/read-eval-print-6596/fn-6599 (main.clj:260)
clojure.main/repl/read-eval-print--6596 (main.clj:260)
clojure.main/repl/fn--6605 (main.clj:278)
clojure.main/repl (main.clj:278)
clojure.tools.nrepl.middleware.interruptible-eval/evaluate/fn--1251 (interruptible_eval.clj:56)
clojure.core/apply (core.clj:617)
nil

Easy enough to see the source of the problem in this case, but because both the number of arguments actually passed is off by two, and the stacktrace element for the call to assoc has been dropped, this shortcut by macroexpand1 can get super confusing.

The attached patch corrects this behavior. E.g.

user=> (do (defmacro f [] (assoc)) (f))
ArityException Wrong number of args (0) passed to: core$assoc clojure.lang.AFn.throwArity (AFn.java:437)
user=> (use 'clojure.repl) (pst)
nil
ArityException Wrong number of args (0) passed to: core$assoc
user/f (NO_SOURCE_FILE:1)
clojure.lang.Var.invoke (Var.java:419)
clojure.lang.Var.applyTo (Var.java:532)
clojure.lang.Compiler.macroexpand1 (Compiler.java:6507)
clojure.lang.Compiler.macroexpand (Compiler.java:6580)
clojure.lang.Compiler.eval (Compiler.java:6654)
clojure.lang.Compiler.eval (Compiler.java:6660)
clojure.lang.Compiler.eval (Compiler.java:6633)
clojure.core/eval (core.clj:2864)
clojure.main/repl/read-eval-print-6594/fn-6597 (main.clj:260)
clojure.main/repl/read-eval-print--6594 (main.clj:260)
clojure.main/repl/fn--6603 (main.clj:278)
nil



 Comments   
Comment by Alex Coventry [ 17/Oct/13 11:01 AM ]

Patch with test

Comment by Alex Coventry [ 23/Oct/13 11:42 PM ]

Amended patch to deal more gracefully with unexpected stack trace structure.

Comment by Alex Miller [ 24/Oct/13 12:09 AM ]

Also see CLJ-397 and CLJ-383.

Comment by Alex Coventry [ 24/Oct/13 2:46 PM ]

Thanks, Alex. It would be easy enough to move most of the logic into ArityException, which would be a compromise between Stu's[1] options 1 and 2. Is that worth doing?

Amending clojure.lang.AFn.throwArity to check whether "this" is a macro and adjust the arg count there accordingly might be the simplest way. I can see why Rich prefers all the logic to go into ArityException, but since ArityExceptions are used for things other than macros, I don't see a way to make an honest error message there without groveling the stack trace.

[1] http://dev.clojure.org/jira/browse/CLJ-397?focusedCommentId=24090&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-24090

Comment by Alex Miller [ 24/Oct/13 2:57 PM ]

I would have to take more time than I have to make an informed opinion but I can say that from a general point of view inspecting StackTraceElements does not seem like the right solution to (almost) any problem.

Comment by Alex Coventry [ 24/Oct/13 10:26 PM ]

This patch causes Var.setMacro to set instance attribute AFn.macrop to true, so that AFn.throwArity can reduce the number of arguments reported.

I'm not used to negotiating java class hierarchies, so it's possible there's a cleaner way. Since Var.fn() returns an IFn, I added macrop handling methods IFn.setMacro and IFn.isMacro. These then needed to be implemented in Ref and Keyword, as well as AFn (where I wanted them) because they implement the IFn interface but don't inherit from AFn.

The real drawback I see with this approach is the duplicated state, though: ^{:macro true} vs AFn.macrop==true.

Comment by Andy Fingerhut [ 25/Oct/13 6:33 PM ]

I have not investigated the reason yet, but neither patch applies cleanly after the latest commits to Clojure master on Oct 25 2013. Given that what kinds of solution methods would be acceptable for this issue, it sounds like more thinking and code changes are probably needed anyway before worrying too much about that.





[CLJ-1278] Identify Clojure namespace and function name in a compiled function's toString() Created: 10/Oct/13  Updated: 26/Nov/14

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

Type: Enhancement Priority: Minor
Reporter: Howard Lewis Ship Assignee: Unassigned
Resolution: Unresolved Votes: 9
Labels: errormsgs, interop

Attachments: Text File CLJ-1278-2.patch     Text File CLJ-1528--function-tostring.patch    
Patch: Code and Test

 Description   

For debugging purposes, it would be useful to have Clojure-oriented toString() for functions.

novate.core.processing.async/locate-destination(async.clj:231)

instead of:

novate.core.processing.async$locate_destination@2690d691

This would remove the frustration of mentally de-mangling the function name from the Java class name.

This will have a relatively insignificant change to the generated code: An implementation of the toString() method. This will add a few dozen bytes to the size of a compiled Clojure function (for the additional bytecodes, and the constant string).



 Comments   
Comment by Howard Lewis Ship [ 10/Oct/13 8:39 PM ]

Contains changes and updated tests. I don't have any details on if this affects compiler performance or generated code size in any significant or even measurable way.

Comment by Andy Fingerhut [ 11/Oct/13 4:06 PM ]

Howard, sorry I do not have more useful comments on the changes you make in your patch. Right now I only have a couple of minor comments on its form. The preferred format for patches is that created using the instructions shown on this wiki page: http://dev.clojure.org/display/community/Developing+Patches

Also, there are several parts of your patch that appear to only make changes in the whitespace of lines. It would be best to leave such changes out of a proposed patch.

Comment by Howard Lewis Ship [ 11/Oct/13 5:00 PM ]

Yes, I didn't notice the whitespace changes until after; I must have hit reformat at some point, despite my best efforts. I'll put together a new patch shortly.

Comment by Howard Lewis Ship [ 11/Oct/13 6:26 PM ]

Clean patch

Comment by Howard Lewis Ship [ 25/Nov/14 6:00 PM ]

FYI, it's been a year. The correct file is CLJ-1278-2.patch.

Comment by Howard Lewis Ship [ 25/Nov/14 6:07 PM ]

... hm, something's changed in recent times.

     [java] FAIL in (fn-toString) (fn.clj:83)
     [java] nested functions
     [java] expected: (= (simple-name (.toString (factory-function))) (str "clojure.test-clojure.fn/" "factory-function/fn"))
     [java]   actual: (not (= "clojure.test-clojure.fn/factory-function/fn__7565" "clojure.test-clojure.fn/factory-function/fn"))
     [java]
     [java] FAIL in (fn-toString) (fn.clj:83)
     [java] nested functions
     [java] expected: (= (simple-name (.toString (named-factory-function))) (str "clojure.test-clojure.fn/" "named-factory-function/a-function-name"))
     [java]   actual: (not (= "clojure.test-clojure.fn/named-factory-function/a-function-name__7568" "clojure.test-clojure.fn/named-factory-function/a-function-name"))

I'd be willing to update my patch if there's any indication that it will ever be picked up. It's been over a year since last update.

Comment by Andy Fingerhut [ 26/Nov/14 10:30 AM ]

The change in behavior you are seeing is most likely due to a fix for ticket CLJ-1330.

And in case you were wondering, no, I am not the person who knows what tickets are of interest. I know that this one has gotten a fair number of votes, and by votes is one of the top ranked enhancement suggestions - look under "enhancements" on this report, or search for 1330: http://jafingerhut.github.io/clj-ticket-status/CLJ-top-tickets-by-weighted-vote.html

The features going into Clojure 1.7 are pretty well decided upon, and a fair number of other fixes and enhancements were delayed to 1.8. A longer than 1 year wait is not unusual, especially for enhancements.

Comment by Howard Lewis Ship [ 26/Nov/14 3:06 PM ]

Thanks for the info; don't want to come off as whiny but The Great Silence is off putting to someone who wants to help improve things.

I'll update my patch, and hope to see some motion for 1.8.

Comment by Alex Miller [ 26/Nov/14 3:43 PM ]

There are ~400 open tickets for Clojure. As a printing enhancement, this is generally considered lower priority than defects. Additionally, the proposal changes the compiler, bytecode generation code, and adds fields to generated objects, which has unassessed and potentially wide impacts. The combination of these things means it might be a while before we get around to looking at it.

Things that you could do to help:
1) Simplify the description. Someone coming to this ticket (screeners and ultimately Rich) want to look at the description and get the maximal understanding with the minimal effort. We have some guidelines on this at http://dev.clojure.org/display/community/Creating+Tickets if you haven't seen it. For an enhancement, a short (1-2 sentence) description of the problem and an example I can run in the repl is best. Then a proposal (again, as short as possible). Examples: CLJ-1529, CLJ-1325, CLJ-1378. For an enhancement like this, seeing (succinct) before/after versions where a user will see this is often the quickest way for a screener to understand the benefit.

2) Anticipate and remove blockers. As I mentioned above, you are changing the size of every function object. What is the impact on size and construction time? Providing data and/or a test harness saves a screener from doing this work. It's better to leave details in attachments or comments and refer to it in the description if it's lengthy.

3) Have others screen (per http://dev.clojure.org/display/community/Screening+Tickets ) - while that is the job a screener (often me) will have to re-do, having more eyeballs on it early helps. Ask on #clojure for someone else to take a look, try it, etc. If there are open questions, leaving those in the description helps guide my work.

Comment by Howard Lewis Ship [ 26/Nov/14 4:09 PM ]

Alex, thanks for the advice. I'll follow through. Some of that data is already present, but I can make it more prominent.

I know that I'm overwhelmed by the number of issues (including enhancements and minor improvements) on the Tapestry issue list, so I'm understanding of problem space.





[CLJ-1275] print-dup's handling of metadata typehint is unreadable in some circumstances Created: 02/Oct/13  Updated: 29/Aug/14

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

Type: Defect Priority: Minor
Reporter: Alex Coventry Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: metadata, print

Attachments: Text File 0001-Don-t-use-shorthand-for-typehints-when-print-dup.patch    
Patch: Code and Test

 Description   

With print-dup true, if an object being printed has a metadata map with only a :tag key, the printer renders it as "^value". This can cause an IllegalArgumentException if you try to read the printed string back in, in some circumstances. E.g.

user=> (read-string (let [ge (with-meta (gensym) {:tag Object})] (binding [*print-dup* true] (pr-str ge))))
  IllegalArgumentException Metadata must be Symbol,Keyword,String or Map  clojure.lang.LispReader$MetaReader.invoke (LispReader.java:732)

This is causing problems with sleight/riddley's [1] handling of the (case) macro, which drops a type-hint on a gensym it incorporates in the form it returns. When sleight tries to reserialize a macroexpanded (case) form from riddley, it fails as demonstrated above. E.g.

user=> (read-string (binding [*print-dup* true] (pr-str (macroexpand '(case 1 1 1)))))
  user=> IllegalArgumentException Metadata must be Symbol,Keyword,String or Map  clojure.lang.LispReader$MetaReader.invoke (LispReader.java:732)

The attached patch corrects this by making core_print.clj's print-meta always print out the full metadata map if print-dup is true. The patch also contains a test for this case.

[1] https://github.com/ztellman/sleight https://github.com/ztellman/riddley



 Comments   
Comment by Alex Coventry [ 02/Oct/13 10:28 PM ]

Corresponding bug on sleight: https://github.com/ztellman/sleight/issues/5

Comment by Andy Fingerhut [ 29/Aug/14 4:39 PM ]

Patch 0001-Don-t-use-shorthand-for-typehints-when-print-dup.patch dated Oct 2 2013 no longer applied cleanly to latest master after some commits were made to Clojure on Aug 29, 2014. It did apply cleanly before that day.

I have not checked how easy or difficult it might be to update this patch.





[CLJ-1272] Agent thread executors do not use the global uncaught exception handler Created: 01/Oct/13  Updated: 05/Feb/14

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

Type: Enhancement Priority: Major
Reporter: David Greenberg Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: agents


 Description   

If you use Thread.setDefaultUncaughtExceptionHandler to catch all application exceptions, and then throw an exception in a future, that exception will get swallowed up in deployment environments that don't watch stdout. It seems that the agent's executors ought to delegate to the global handler.

This issue bit us, in that we deploy and monitor our system only through its logs and metrics, and never actually saw that exceptions were being thrown in futures.






[CLJ-1266] Better primitive support for floats Created: 26/Sep/13  Updated: 05/Feb/14

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

Type: Enhancement Priority: Minor
Reporter: Christophe Grand Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: performance

Attachments: File floats.diff     File floats-intrinsics.diff    
Patch: Code

 Description   

Clojure offers optimized arithmetic functions for long, int and doubles but none for floats.
Plus converting from integers (ints or longs) to floating point numbers (float or double) doesn't use the specialized bytecode.
This patch adds float-add/subtract/multiply/divide and more efficient coversion from integers to floating points numbers.



 Comments   
Comment by Alex Miller [ 05/Feb/14 12:29 PM ]

I think it's unlikely the arithmetic float ops will be accepted.
However, the intrinsics changes could be useful - could you split those into a new ticket?

Comment by Christophe Grand [ 05/Feb/14 4:20 PM ]

I attached a new patch with only intrinsics and more comprehensive primitive coercion. Is it the split you expected?

Comment by Alex Miller [ 05/Feb/14 5:22 PM ]

No, but totally my fault for saying the wrong words.

I think the changes in Compiler to get access to I2D, L2D, I2F, and L2F are potentially useful (most particularly L2D) - these would make sense in a new ticket.

The other changes in Intrinsics and Numbers to support float math are unlikely to be accepted.

Comment by Christophe Grand [ 05/Feb/14 5:40 PM ]

Godd thing I had already split coercions in a sparate commit then

See http://dev.clojure.org/jira/browse/CLJ-1340





[CLJ-1263] Allow static compilation of function invocations Created: 14/Sep/13  Updated: 07/Nov/13

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


 Description   

This proposal is to allow metadata on functions to prevent a fully dynamic var deref to be used whenever the function is called.

When the function is invoked, JVM "invokevirtual" instruction will be used, which is faster than the current implementation (var deref + IFn cast + invokinterface) and has less restrictions (no need to predefine interfaces to match the function parameters). The JVM is generally able to compile such invokevirtual instructions into extremely efficient code - effectively as fast as pure Java.

This is intended to pave the way to better support for statically compiled, high performance code. In particular, it allow:

  • Supporting arbitrary JVM primitives (float, int, byte, char etc.) as well as just double/long.
  • Supporting typed return values e.g. "String". This could eliminate many casts and type checks.
  • Supporting typed reference arguments (e.g. String).

Suggested usage:

(defn ^:static foo ^int [^String a ^String b]
(+ (count a) (Integer/parseInt b)))

Existing code / semantics should not be affected



 Comments   
Comment by Alex Fowler [ 18/Sep/13 5:08 AM ]

Very nice! That is what would really improve experience with certain tasks. I think it will also make possible to work with primitive arrays without the conversions?

Comment by Mike Anderson [ 19/Sep/13 5:44 PM ]

Hi Alex - which aspect of "work with primitive arrays" are you referring to? This feature would certainly help with passing primitive arguments to/from functions that use primitive arrays. It would also potentially help avoid some casts of primitive array arguments themselves. I don't think it helps in any other way - perhaps a separate issue would be appropriate if there is another thing you are trying to do?

Comment by Kevin Downey [ 29/Oct/13 11:50 AM ]

this issue is confusing, because there was/is a :static feature in clojure(which seems to be disabled in the compiler at the moment) and this proposal doesn't mention the existing work at all.

I also think this proposal is begging the question, there is no discussion of other possible solutions to the performance problem (whatever that is) that this is trying to solve.

the (var.deref()(IFn)).invoke(...) is pretty fundamental to the feel of clojure, in fact the existing :static keyword seems to be disabled in the compiler exactly because it complicates those semantics. so we should have a very clear proposal (not a wishlist) if we want to change that with some very clear wins.

maybe an optimizing clojure compiler would be a better approach.

Comment by Mike Anderson [ 30/Oct/13 11:01 PM ]

Hi Kevin,

This is partly in response to this discussion on Clojure-Dev, where we discussed there are quite a lot of performance issues around the way that Clojure passes arguments currently:
https://groups.google.com/d/topic/clojure/H5P25eYKBj4/discussion

Also I believe it reinstates the original intention of "^:static": I can't find where this is/was officially documented, but Arthur's answer in this SO question suggests that this was the case:
http://stackoverflow.com/questions/7552632/what-does-static-do-in-clojure

I think the proposal is relatively clear: it's probably the minimal change required to get static/direct (i.e. not via an indirect var reference / IFn) function invocations without affecting any of the semantics of current code.

This is sufficiently important for me that it's preventing me from shifting some performance-critical code to Clojure (even with primitive type hints). e.g. here's a simple case with a small primitive function:

(defn ^long foo [^long x]
(inc x))

(c/quick-bench (dotimes [i 100] (foo i))) ;; c = criterium
=> Execution time mean : 194.334293 ns

(c/quick-bench (dotimes [i 100] (inc i)))
=> Execution time mean : 71.539048 ns

i.e. the indirect function invocation is costing us nearly 170% overhead. In Java the equivalent functions perform identically: the overhead is zero because with static function invocation the JVM JIT is able to eliminate all the function call overhead.

In the long term, I agree that a proper optimising compiler would be the best way forward (perhaps Clojure 2.0/CinC can give us this?) but in the meantime I think this is a pragmatic way to improve performance with minimal impact on existing code. Even with an optimising compiler, I think we' would need some way to specifiy the "optimised" semantics rather than the indirect var deref behaviour, and "^:static" seems like a reasonable way to do so (unless anyone has a better idea?)

Comment by Kevin Downey [ 04/Nov/13 3:58 PM ]

have you looked at the definition of int and how it uses :inline/definline to avoid the call overhead?

Comment by Mike Anderson [ 05/Nov/13 4:27 AM ]

Good point Kevin - :inline and definline seem like a good approach in many cases (although it's marked as "experimental" - does that mean we can't rely on it to work in future releases?).

This proposal is still somewhat different: the inline solutions and its variants are effectively doing macro expansion to generate code without a function call on the Clojure side. The approach in this proposal would still emit a function call in bytecode, but do so in a way that the JVM can subsequently inline and optimise much more efficiently. Both have their uses, I think?

Commented edited Nov 7 2013 by Andy Fingerhut: Regarding definline marked as experimental, it has been so marked since Clojure 1.0's release, and the plan is to keep it marked that way in the pending Clojure 1.6 release. See discussion thread on CLJ-1281. No plans to remove it that I am aware of.

Comment by Kevin Downey [ 06/Nov/13 2:06 PM ]

my point is your benchmark above is not a comparison of clojure's current deref + cast + invoke vs. invokevirtual, inc is being inlined in to a static method call there

Comment by Kevin Downey [ 06/Nov/13 2:32 PM ]

I've been noodling around this, and it is entirely possible to generate and invoke code in clojure right now without paying the extra deref() cost:

 (defn ^long fib [^long n]
   (case n
     0 0
     1 1
     (+ (fib (dec n))
        (fib  (dec (dec n))))))

can be written as

(declare TheR1798)

(definterface I1797
  (^long fib_Invoke_1 [^long n]))

(deftype R1798 []
  I1797
  (^long fib_Invoke_1
    [this1799 ^long n]
    (case n
      0 0
      1 1
      (+ (.fib_Invoke_1 this1799 (dec n))
         (.fib_Invoke_1 this1799 (dec (dec n)))))))

(def TheR1798 (new R1798))

(defn ^long fib [^long n]
  (.fib_Invoke_1 TheR1798  n)))

now the recursive calls are invokeinterfaces, and the resulting function seems to have mean execution time about 5 times smaller using criterium to bench mark

it is entirely possible to write a macro that translates one in to other, and the weird names in the above are because I have a little proof of concept that does that.

the body of the bytecode for the regular fib function first shown looks something like:

  public final java.lang.Object invokePrim(long);
    Code:
       0: lload_1       
       1: lstore_3      
       2: lload_3       
       3: l2i           
       4: tableswitch   { // 0 to 1
                     0: 28
                     1: 40
               default: 52
          }
      28: lconst_0      
      29: lload_3       
      30: lcmp          
      31: ifne          52
      34: getstatic     #33                 // Field const__1:Ljava/lang/Object;
      37: goto          94
      40: lconst_1      
      41: lload_3       
      42: lcmp          
      43: ifne          52
      46: getstatic     #37                 // Field const__3:Ljava/lang/Object;
      49: goto          94
      52: getstatic     #57                 // Field const__5:Lclojure/lang/Var;
      55: invokevirtual #70                 // Method clojure/lang/Var.getRawRoot:()Ljava/lang/Object;
      58: checkcast     #6                  // class clojure/lang/IFn$LO
      61: lload_1       
      62: invokestatic  #75                 // Method clojure/lang/Numbers.dec:(J)J
      65: invokeinterface #77,  3           // InterfaceMethod clojure/lang/IFn$LO.invokePrim:(J)Ljava/lang/Object;
      70: getstatic     #57                 // Field const__5:Lclojure/lang/Var;
      73: invokevirtual #70                 // Method clojure/lang/Var.getRawRoot:()Ljava/lang/Object;
      76: checkcast     #6                  // class clojure/lang/IFn$LO
      79: lload_1       
      80: invokestatic  #75                 // Method clojure/lang/Numbers.dec:(J)J
      83: invokestatic  #75                 // Method clojure/lang/Numbers.dec:(J)J
      86: invokeinterface #77,  3           // InterfaceMethod clojure/lang/IFn$LO.invokePrim:(J)Ljava/lang/Object;
      91: invokestatic  #81                 // Method clojure/lang/Numbers.add:(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Number;
      94: areturn       
    LineNumberTable:
      line 243: 0
      line 244: 2
      line 247: 52
      line 247: 52
      line 247: 61
      line 248: 70
      line 248: 79
      line 248: 79
    LocalVariableTable:
      Start  Length  Slot  Name   Signature
             2      92     3 G__301   J
             0      94     0  this   Ljava/lang/Object;
             0      94     1     n   J

  public java.lang.Object invoke(java.lang.Object);
    Code:
       0: aload_0       
       1: aload_1       
       2: checkcast     #89                 // class java/lang/Number
       5: invokestatic  #93                 // Method clojure/lang/RT.longCast:(Ljava/lang/Object;)J
       8: invokeinterface #77,  3           // InterfaceMethod clojure/lang/IFn$LO.invokePrim:(J)Ljava/lang/Object;
      13: areturn       

the body of the "optimized" version looks like:

  public long fib_Invoke_1(long);
    Code:
       0: lload_1       
       1: lstore_3      
       2: lload_3       
       3: l2i           
       4: tableswitch   { // 0 to 1
                     0: 28
                     1: 38
               default: 48
          }
      28: lconst_0      
      29: lload_3       
      30: lcmp          
      31: ifne          48
      34: lconst_0      
      35: goto          80
      38: lconst_1      
      39: lload_3       
      40: lcmp          
      41: ifne          48
      44: lconst_1      
      45: goto          80
      48: aload_0       
      49: checkcast     #6                  // class com/thelastcitadel/kernel/I2364
      52: lload_1       
      53: invokestatic  #53                 // Method clojure/lang/Numbers.dec:(J)J
      56: invokeinterface #55,  3           // InterfaceMethod com/thelastcitadel/kernel/I2364.fib_Invoke_1:(J)J
      61: aload_0       
      62: checkcast     #6                  // class com/thelastcitadel/kernel/I2364
      65: lload_1       
      66: invokestatic  #53                 // Method clojure/lang/Numbers.dec:(J)J
      69: invokestatic  #53                 // Method clojure/lang/Numbers.dec:(J)J
      72: invokeinterface #55,  3           // InterfaceMethod com/thelastcitadel/kernel/I2364.fib_Invoke_1:(J)J
      77: invokestatic  #59                 // Method clojure/lang/Numbers.add:(JJ)J
      80: lreturn       
    LineNumberTable:
      line 251: 0
      line 251: 2
      line 251: 48
      line 251: 48
      line 251: 52
      line 251: 61
      line 251: 65
      line 251: 65
    LocalVariableTable:
      Start  Length  Slot  Name   Signature
             2      78     3 G__2363   J
             0      80     0  this   Lcom/thelastcitadel/kernel/R2365;
             0      80     1     n   J

so the calls are not invokevirtual (due to the way clojure compiles stuff, you cannot type anything inside a record as being that record's type), but the interface is unique and only has one instance, so I think the jvm's class hierarchy analysis makes short work of that.

if I have time I may try and complete my macro and release it as a library, but given tools.analyzer.jvm someone should be able to do better than my little proof of concept very quickly.

Comment by Andy Fingerhut [ 07/Nov/13 12:48 PM ]

I don't know if my editing of Mike Anderson's Nov 5 2013 comment is notified to people watching this ticket, so adding a new comment so those interested in definline's experimental status can know to go back and re-read it.





[CLJ-1257] Suppress warnings when clojure.core symbols are explicitly replaced with "use" or "refer" Created: 06/Sep/13  Updated: 29/Aug/14

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

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

Attachments: File clj-1257-2.diff     File clj-1257.diff    
Patch: Code

 Description   

Problem: Libraries that provide DSLs (such as core.matrix) often replace or extend functions in core (such as "+", "==", "zero?"), since it is desirable to use the best / most idiomatic names.

Currently importing such libraries with "use" causes unwanted warnings like "WARNING: + already refers to: #'clojure.core/+ in namespace: test.blank, being replaced by: #'clojure.core.matrix/+".

Avoiding these warnings requires extra user effort and boilerplate code, which is frustrating for users since they have already explicitly asked for the full library to be imported into the current namespace (i.e. via "use" or ":refer :all").

Proposed solution is to introduce a new var warn-on-replace similar to warn-on-reflection which allows this warning to be controlled.



 Comments   
Comment by Mike Anderson [ 06/Sep/13 10:40 PM ]

Basic patch and test attached.

Comment by Andy Fingerhut [ 07/Sep/13 3:22 PM ]

I have no idea whether this idea will be vetted or not, but if it is, I have some comments on the proposed patch.

The new symbol warn-on-replace should have doc and metadata on it. See add-doc-and-meta for warn-on-reflection in core.clj for an example to copy and edit.

You check WARN_ON_REFLECTION before issuing the warning in Namespace.java, instead of WARN_ON_REPLACE.

Possible typo in the test description in ns_libs.clj: Maybe "symbol in clojure.core" instead of "symbol is clojure.core"?

If someone wants warnings from :use statements in ns forms, it seems the only way to do that with patch clj-1257.diff would be to do (set! warn-on-replace true) in a file before the ns form. That would not work well with the current version of tools.namespace, which assumes that if there is an ns form, it is the first form in the file. One can argue that tools.namespace should not make such an assumption, but it does right now.

Perhaps there should be a command line option clojure.compile.warn-on-replace like there is for clojure.compile.warn-on-reflection (search for warn-on-replace in Compile.java)?

Comment by Mike Anderson [ 07/Sep/13 11:09 PM ]

Thanks Andy for the feedback! I'll post an updated patch shortly.

It occurs to me that we should probably implement a more general approach to warnings in Clojure. Adding new vars and command line options for each warning doesn't seem like a good long-term strategy. I think that's beyond the scope of this patch though.

Comment by Andy Fingerhut [ 08/Sep/13 12:49 AM ]

Actually, there is something called compiler-options (search for the variations compiler-options, COMPILER_OPTIONS, and compiler_options for all related occurrences) that is a map where each key/value pair is a different option. That might be preferable for warn-on-replace, if it is in fact desired.

Comment by Mike Anderson [ 08/Sep/13 1:47 AM ]

Updated patch attached.

Compiler-options looks like it may indeed be a better place for this, if that is the preferred strategy for controlling warnings. But I'll wait for more feedback / confirmation on the approach before making that change.

Comment by Alex Miller [ 09/Sep/13 1:43 PM ]

Is (:refer-clojure :exclude [+ = zero?]) a valid workaround? Or are you really concerned with the consumers of the library?

Comment by Mike Anderson [ 09/Sep/13 7:18 PM ]

I'm mainly concerned with consumers of the library.

So while (:refer-clojure :exclude [+ = zero?]) is possible as a temporary workaround, it's very inconvenient for users. We should really fix this in Clojure itself. Users have enough trouble with ns forms already without adding to their woes

As an alternative solution: I personally wouldn't mind it if the library author could add some metadata to symbols (e.g. "^:replace-symbol") to signal that a library function is intended to replace something in core. That's a slightly different approach (and I think a bit trickier to implement) but it should also work.

Comment by Mike Anderson [ 22/May/14 4:43 AM ]

Example issue reported by a user because of this:

https://github.com/mikera/vectorz-clj/issues/23

Comment by Andy Fingerhut [ 29/Aug/14 4:37 PM ]

As before, I can't comment on whether there is interest in this ticket or these patches, but I can say that all patches dated Sep 7 2013 and earlier no longer applied cleanly to latest master after some commits were made to Clojure on Aug 29, 2014. They did apply cleanly before that day.

I have not checked how easy or difficult it might be to update this patch.

Comment by Mike Anderson [ 29/Aug/14 7:00 PM ]

I'm happy to update the patch, just need feedback on which approach / solution to this problem is preferred.

I'd really like to see this in 1.7!





[CLJ-1256] Support type-hinted overrides of function parameters Created: 06/Sep/13  Updated: 09/Sep/13

Status: