<< Back to previous view

[CLJ-2207] Clojure reader should treat non-breaking space as whitespace character Created: 14/Jul/17  Updated: 26/Jul/17  Resolved: 25/Jul/17

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

Type: Enhancement Priority: Minor
Reporter: Nikita Prokopov Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: reader

Attachments: Text File clj-2207-nbsp.patch     Text File clj-2207-nbsp-v2.patch     Text File clj-2207-nbsp-v3.patch    
Patch: Code

 Description   

Right now, Clojure uses Character.isWhitespace(ch) || ch == ',' as the definition of whitespace in the reader. Character.isWhitespace, however, for obscure reasons (it has been defined long time ago), intentionally excludes U+00A0 (no-break space), U+2007 (figure space), U+202F (narrow no-break space). Logically, though, these characters should be treated as normal whitespace for all reasons except text formatting (e.g. the newer Character.isSpaceChar fixed that and does treat them as space chars).

Why is this important: if non-breaking space is inserted by accident (e.g. by pressing Option+Space on Mac), it'll be very hard to find the source of the error in a otherwise very innocent-looking code.

The attached patch implements Util.isWhitespace method that returns true for all characters treated as whitespace by Character.isWhitespace AND for those 3 exceptions. All cases where reading used Character.isWhitespace was referenced are modified to call new Util.isWhitespace instead.

Patch: clj-2207-nbsp-v3.patch

Prescreened by: Alex Miller



 Comments   
Comment by Alex Miller [ 14/Jul/17 10:12 AM ]

I think most of the locations that you changed are not relevant to your request and complicate the evaluation of this patch. Including support for these characters as whitespace in the reader seems fine to me. Determining whether these should be changed in formatting, string, xml, etc function requires more analysis and may not be worth doing so you should narrow this patch to just the changes in LispReader and EdnReader.

Also, the isWhitespace() method should not be in RT (which triggers loading of the Clojure runtime) and would be better in the Util class.

Comment by Nikita Prokopov [ 14/Jul/17 12:46 PM ]

Thanks Alex! Very valuable comments, thank you. See new patch attached. I took a freedom to alter main.clj as well because I feel like it should stay in sync with what ListReader/EdnReader do.

Comment by Alex Miller [ 14/Jul/17 1:44 PM ]

Why did you add the isWhitespace() method in LispReader too? Just make one method in Util and use it from everywhere.

Comment by Nikita Prokopov [ 14/Jul/17 5:52 PM ]

As you said, I didn’t want to change too much. But yes, if we’re not using it in string/xml/formatter it probably can be extracted. See new patch attached.

Comment by Phill Wolf [ 16/Jul/17 7:53 AM ]

javac does not accept non-breaking spaces. The message is

nonbreakingspace.java:4: error: illegal character: '\u00a0'

Do we really want a codebase with an uncontrolled variety of spaces and non-breaking spaces?

If non-breaking spaces are inadvertent, would an editor macro be more appropriate, or a Git hook, or a more pointed Clojure error message?

I am still smarting from tabs-vs-spaces. Java's \s does not match non-breaking spaces. I will be disappointed if things start hiding themselves from grep by using exotic spaces.

Overall - it seems to me that enlarging the category of invisible alternatives for whitespace in source code may not lead to a net improvement in quality of life.

Comment by Alex Miller [ 25/Jul/17 5:03 PM ]

Upon further reflection, I am compelled by Phill's argument. In general for stuff like this, we take our cue from Java and I think that seems like a reasonable approach here as well. I am declining this ticket.

Comment by Nikita Prokopov [ 26/Jul/17 5:43 AM ]

But the fact that we treat non-breaking space as a unique symbol rather than a generic space does give it special meaning. E.g. Clojure does not distinguish between tab and space (and it’s great!), it treats them both just as an empty separator, without assigning each one unique meaning.

What we have with non-breaking space is another invisible char that got its own treatmeant while looking exactly the same. Also, I believe it goes against Unicode intention: non-breaking space should be treated exactly as a space character for all purposes but word wrapping.





[CLJ-2204] Clojure classes can be used to craft a serialized object that runs arbitrary code on deserialization Created: 12/Jul/17  Updated: 25/Jul/17

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

Type: Defect Priority: Critical
Reporter: Alex Miller Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: inspector, security

Attachments: Text File clj-2204-disable-proxy-serialization.patch     Text File clj-2204.patch    
Patch: Code and Test
Approval: Prescreened

 Description   

If a server can deserialize objects from an untrusted source, it is possible to craft a serialized object that runs arbitrary code on deserialization. Classes in the Clojure jar can be used for this purpose and this may lead to blacklisting the Clojure jar in some environments.

Demonstration of how to create such an object: https://github.com/frohoff/ysoserial/pull/68/files

The serialized class being constructed in the attack is clojure.inspector.proxy$javax.swing.table.AbstractTableModel$ff19274a, but clojure.core.proxy$clojure.lang.APersistentMap$ff19274a and proxy classes created by users are also susceptible. Clojure proxies contain a field (__clojureFnMap) that is a map of method names to proxy functions. Clojure AOT compiles the clojure.inspector and clojure.core.proxy code which generates the classes named above. These classes are then included inside the clojure jar.

The attack constructs an instance of one of these proxy classes and adds a "hashCode" proxy method to the proxy's table. The method is a Clojure function that can run arbitrary code. This instance is then put inside a HashMap and the whole thing is serialized. On deserialization, the HashMap will invoke hashCode() on the proxy object and cause the execution of the arbitrary code.

Note that most uses of proxy will create objects that cannot be serialized anyway, due to the common use of literal function bodies.

Proposed: Modify proxy so that the classes it generates are neither deserializable (to disable the attack) nor serializable (to avoid misleading users). The patch causes proxy classes to have readObject and writeObject methods that just throw an exception, when Serializable is in the inheritance tree of the proxy class. This is technically a breaking change since it is possible to construct a proxy that can be serialized, but such classes are unlikely in the wild and inherently insecure.

Patch: clj-2204-disable-proxy-serialization.patch

Prescreened by: Alex Miller

Background: Raised at https://groups.google.com/d/msg/clojure/WaL3hHzsevI/7zHU-L7LBQAJ and similar to issues reported on Apache Commons. See also:



 Comments   
Comment by Chouser [ 16/Jul/17 5:47 PM ]

I believe there are other proxies of a Serializable classes. I wrote a little script (included below) to dig through a target directory of AOT classes looking for suspicious ones. It may generate false positives, but one it found is generated from this line, and APersistentMap does inherit from java.io.Serializable. Using this clue, I was then able to change the gadget code to use clojure.core.proxy$clojure.lang.APersistentMap$ff19274a instead of the AbstractTableModel proxy and got equivalent results.

I'll keep looking for other classes, but I think we'll unfortunately need to do more than just skip AOT'ing the inspector namespace.

#!/bin/bash

java -cp $HOME/.m2/repository/org/clojure/spec.alpha/0.1.123/spec.alpha-0.1.123.jar:$HOME/.m2/repository/org/clojure/core.specs.alpha/0.1.10/core.specs.alpha-0.1.10.jar:target/classes clojure.main - <<"EOF"

(ns cc
  (:require [clojure.java.io :as io]
            [clojure.reflect :as reflect]
            [clojure.string :as str]
            [clojure.pprint :refer [pprint]]))

(defn ancestor-strs [classname]
 (set (map str (:ancestors (reflect/type-reflect (Class/forName classname) :ancestors true)))))

(->> (file-seq (io/file "target/classes")) ;; all files and dirs
 (keep #(second (re-matches #"target/classes/(.*)\.class" (str %)))) ;; class filenames
 (map #(str/replace % #"/" ".")) ;; classnames
 (filter #(let [as (conj (ancestor-strs %) %)]
	   (and
	    (contains? as "java.io.Serializable")
	    (some (fn [a] (re-find #"proxy" a)) as))))
 prn)

EOF
Comment by Chouser [ 16/Jul/17 9:22 PM ]

Here is the gadget code plus in-memory test harness written in Clojure; makes it a bit easier to test proxy classes for this issue:

(def flag (atom true))                                                         
                                                                               
(defn ok-proxy? [target-proxy]                                                 
  (let [baos (java.io.ByteArrayOutputStream.)                                  
        top (java.util.HashMap.                                                
              {(doto target-proxy                                              
                 (.__initClojureFnMappings {"hashCode" (constantly 0)})) 0})]  
    (.__initClojureFnMappings                                                  
      target-proxy                                                             
      {"hashCode" (comp eval (constantly '(do (reset! flag false) 0)))})       
    (.writeObject (java.io.ObjectOutputStream. baos) top)                      
    (let [payload (.toByteArray baos)]                                         
      (reset! flag true)                                                       
                                                                               
      ;; Deserializing the payload will execute hashCode method above          
      (-> payload                                                              
        java.io.ByteArrayInputStream. java.io.ObjectInputStream. .readObject)  
      @flag)))                                                                 
                                                                               
(prn :ok? (ok-proxy? (bean (Object.))))                                        
(prn :ok? (ok-proxy? (inspector/list-model nil)))                              
Comment by Chouser [ 16/Jul/17 10:03 PM ]

I think we should change the proxy code to generate classes that explicitly refuse to support deserialization. Proxy objects already generally fail to serialize due to the function objects in their __clojureFnMap, so this would not be removing any useful functionality that I'm aware of. I've got a proof of concept here in which proxy always supplies private readObject and writeObject methods, complaining usefully instead of writing, and absolutely refusing to read. Does this seem like a good approach?

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

Hey Chouser, thanks for working on this and finding my gaps! Making proxies nonserializable seems like a good approach to me. Should also cover writeExternal() and readExternal() for Externalizable I think?

Comment by Chouser [ 17/Jul/17 11:22 AM ]

Thanks, as always, for your diligence and expertise on so many of these issues.

It's not clear to me if we need to do the Externalizable methods (I'm only just now learning about these serialization features). Even if a proxied class does implement Externalizable, it would have to provide methods that explicitly serialize and deserialize the proxy class's FnMap – which of course nothing in Java core can do. If an application were to do this, it would not really be Clojure's responsibility to prevent it, even if it could, I think. Am I getting this right?

If we do implement Externailzable methods to prevent them from doing anything dangerous, it would only be for classes that inherit from Externalizable. This is in contrast to readObject/writeObject which are not methods of any interface, and so I believe are essentially harmless to provide at all times.

Comment by Alex Miller [ 17/Jul/17 12:47 PM ]

Yep, that makes sense. Externalizable classes would have explicitly made the choice to implement these methods and would not be serializing state of proxy subclass.

I suppose the only potential harm in providing readObject() and writeObject() would be in the (unlikely) case where the parent class was not Serializable but had those methods. I guess we could narrow to just providing them if the class implemented Serializable.

Comment by Alex Miller [ 17/Jul/17 12:49 PM ]

Also, I filed a separate ticket (CLJ-2205) re the broader proxy serialization issue which seems like where this is headed. It would probably make sense to rewrite the description here to cover the more general fix and to close CLJ-2205 as a duplicate.

Comment by Alex Miller [ 17/Jul/17 10:56 PM ]

Patch looks good to me - can you add a test with the harness to the patch too?

Comment by Chouser [ 17/Jul/17 11:15 PM ]

Thanks for reviewing it.

I will add a test; it could simply confirm that an attempt to serialize a proxy class fails. Do you think that's sufficient, or should it also include the attempted code execution? I guess the latter could fail for other more subtle reasons, causing the test to pass when perhaps we wouldn't want it to, so I'll go for the simpler test first.

Comment by Alex Miller [ 17/Jul/17 11:22 PM ]

I think the simpler test is sufficient.

Comment by Chouser [ 18/Jul/17 11:00 AM ]

Attached patch with fix and test. Note the test for prohibited deserialization is tricky because we also prevent the creation of the data needed for that test. Because of this, the serialized stream is included directly, along with instructions on how to recreate the data if needed in the future.

Comment by Chouser [ 18/Jul/17 11:54 AM ]

Alex, I just realized (by looking though this ticket's history) that I unintentionally overwrote your Description update last night. Not sure how that happened, but I apologize. Feel free to change it back or take bits from mine and add it to yours, or whatever else you feel is appropriate.





[CLJ-2210] back referential expressions can cause exponential compilation times Created: 21/Jul/17  Updated: 24/Jul/17

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

Type: Defect Priority: Critical
Reporter: Nicola Mometto Assignee: Unassigned
Resolution: Unresolved Votes: 4
Labels: compiler, performance

Attachments: Text File 0001-CLJ-2210-cache-non-trivial-getJavaClass-hasJavaClass.patch    
Patch: Code
Approval: Triaged

 Description   

Reported as causing problems in real world code: https://groups.google.com/forum/#!topic/clojure/Z91bhUvSB1g

With init.clj as :

(def expr '(fn [& {:keys [a b c d e f
                          map-0 map-1 map-2]}]
             (cond-> "foo"
               a (str a)
               b (str b)
               c (str c)
               d (str d)
               e (str e)
               f (str f)
               map-0 (cond->
                         (:a map-0) (str (:a map-0))
                         (:b map-0) (str (:b map-0))
                         (:c map-0) (str (:c map-0))
                         (:d map-0) (str (:d map-0)))
               map-1 (cond->
                         (:a map-1) (str (:a map-1))
                         (:b map-1) (str (:b map-1))
                         (:c map-1) (str (:c map-1))
                         (:d map-1) (str (:d map-1)))
               map-2 (cond->
                         (:a map-2) (str (:a map-2))
                         (:b map-2) (str (:b map-2))
                         (:c map-2) (str (:c map-2))
                         (:d map-2) (str (:d map-2))))))

(time (eval expr))

Before patch:

[~]> clj -i init.clj
"Elapsed time: 24233.193238 msecs"

After patch:

[~]> clj -i init.clj
"Elapsed time: 24.719472 msecs"

This is caused by every local bindings' type depending on the type of the previous binding, triggering an exponential number of calls to hasJavaClass and getJavaClass. By caching on first occurrence, the complexity becomes linear

Patch: 0001-CLJ-2210-cache-non-trivial-getJavaClass-hasJavaClass.patch






[CLJ-2007] if-let* & when-let* with multiple bindings implementation Created: 21/Aug/16  Updated: 24/Jul/17

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

Type: Feature Priority: Major
Reporter: Ertuğrul Çetin Assignee: Unassigned
Resolution: Unresolved Votes: 5
Labels: None


 Description   

I think Clojure programs will be more elegant if we use let versions of if & when with multiple bindings.

;; if-let* imp.

(defmacro if-let*
  "Multiple binding version of if-let"
  ([bindings then]
   `(if-let* ~bindings ~then nil))
  ([bindings then else]
   (when (seq bindings)
     (assert-args
       (vector? bindings) "a vector for its binding"
       (even? (count bindings)) "exactly even forms in binding vector"))
   (if (seq bindings)
     `(if-let [~(first bindings) ~(second bindings)]
        (if-let* ~(vec (drop 2 bindings)) ~then ~else)
        ~(if-not (second bindings) else))
     then)))

;;Example if-let*

(if-let* [a 1
          b (+ a 1) ]
          b)

;;=> 2

(if-let* [a 1
          b (+ a 1)
          c false] ;;false or nil - does not matter
          b
          a)

;;=> 1

;; when-let* imp.

(defmacro when-let*
  "Multiple binding version of when-let"
  [bindings & body]
  (when (seq bindings)
    (assert-args
      (vector? bindings) "a vector for its binding"
      (even? (count bindings)) "exactly even forms in binding vector"))
  (if (seq bindings)
    `(when-let [~(first bindings) ~(second bindings)]
       (when-let* ~(vec (drop 2 bindings)) ~@body))
    `(do ~@body)))

;;Example when-let*

  (when-let* [a 1 
             b 2 
             c (+ a b)]
             (println "yeah!")
             c)
  ;;=>yeah!
  ;;=>3

  (when-let* [a 1 
             b nil 
             c 3]
             (println "damn! b is nil")
             a)
  ;;=>nil


 Comments   
Comment by Vitalie Spinu [ 01/Jul/17 3:47 PM ]

Very useful and pleasant feature. Even emacs-lisp has these nowadays.

Isn't these backward compatible? Should then be when-let and if-let if performance permits.

Comment by Michael Blume [ 06/Jul/17 1:41 PM ]

This turns out to be just slightly subtle to define. Question is – in the else form, should the bindings which evaluated truthy still exist?

That is, should

(let [a :original]
  (if-let [a :shadowed
           b false]
    (exception)
    a))

evaluate to :original or :shadowed? If the bindings do exist – if you get :shadowed – then you have a single else branch that's being evaluated in a different context depending on when you hit falsey. That seems inelegant (correct me if I'm wrong). If you try to evaluate the else in the outer context, without the truthy bindings – if you try to get :original – the implementation becomes a bit nontrivial. I think you wind up needing sentinel values and so on.

Comment by Ertuğrul Çetin [ 06/Jul/17 2:12 PM ]

I think implementation should be changed like this(bindings should not be leaked to else form ever):

(defmacro when-let*
  "Multiple binding version of when-let"
  [bindings & body]
  (when (seq bindings)
    (assert-all
      (vector? bindings) "a vector for its binding"
      (even? (count bindings)) "exactly even forms in binding vector"))
  (if (seq bindings)
    `(when-let [~(first bindings) ~(second bindings)]
       (when-let* ~(vec (drop 2 bindings)) ~@body))
    `(do ~@body)))

(defmacro if-let*
  "Multiple binding version of if-let"
  ([bindings then]
   `(if-let* ~bindings ~then nil))
  ([bindings then else]
   (when (seq bindings)
     (assert-all
       (vector? bindings) "a vector for its binding"
       (even? (count bindings)) "exactly even forms in binding vector"))
   (if (seq bindings)
     `(if-let [~(first bindings) ~(second bindings)]
        (if-let* ~(vec (drop 2 bindings)) ~then ~else)
        ~else)
     then)))
Comment by Michael Blume [ 06/Jul/17 2:18 PM ]

That will leak bindings to the else form though – not the falsy one that led you to the else form but the outer truthy ones

Comment by Michael Blume [ 06/Jul/17 2:19 PM ]

And if you try to capture the else form in a closure and bind that you break calls to recur.

Comment by Michael Blume [ 06/Jul/17 2:36 PM ]

My example under your implementation expands to

(let [a :original]
  (let [temp__4655__auto__ :shadowed]
    (if temp__4655__auto__
      (let [a temp__4655__auto__]
        (let [temp__4655__auto__ false]
          (if temp__4655__auto__
            (let
             [b temp__4655__auto__]
              (exception))
            a)))
      a)))

and evaluates to :shadowed

Comment by Justin Spedding [ 22/Jul/17 3:47 PM ]

I wrote an alternate version of if-let* and if-some* that do not leak bindings into the else clause:

(defmacro if-let*
  "Multiple binding version of if-let"
  ([bindings then]
   `(if-let* ~bindings ~then nil))
  ([bindings then else]
   (assert-args
     (vector? bindings) "a vector for its binding"
     (even? (count bindings)) "exactly even forms in binding vector")
   (let [if-let-else (keyword (name (gensym "if_let_else__")))
         inner (fn inner [bindings]
                 (if (seq bindings)
                   `(if-let [~(first bindings) ~(second bindings)]
                      ~(inner (drop 2 bindings))
                      ~if-let-else)
                   then))]
     `(let [temp# ~(inner bindings)]
        (if (= temp# ~if-let-else) ~else temp#)))))

(defmacro if-some*
  "Multiple binding version of if-some"
  ([bindings then]
   `(if-some* ~bindings ~then nil))
  ([bindings then else]
   (assert-args
     (vector? bindings) "a vector for its binding"
     (even? (count bindings)) "exactly even forms in binding vector")
   (let [if-some-else (keyword (name (gensym "if_some_else__")))
         inner (fn inner [bindings]
                 (if (seq bindings)
                   `(if-some [~(first bindings) ~(second bindings)]
                      ~(inner (drop 2 bindings))
                      ~if-some-else)
                   then))]
     `(let [temp# ~(inner bindings)]
        (if (= temp# ~if-some-else) ~else temp#)))))
Comment by Justin Spedding [ 23/Jul/17 12:38 PM ]

After giving it some more thought, I do not see much of a reason to have separate macros for if-let and if-let*. Why not just extend the base if-let, if-some, when-let, and when-some to accept multiple bindings?

These macros produce the same output if used with just one binding, but are also capable of handling multiple bindings:

(defmacro if-let
  "Multiple binding version of if-let"
  ([bindings then]
   `(if-let ~bindings ~then nil))
  ([bindings then else]
   (assert-args
     (vector? bindings) "a vector for its binding"
     (even? (count bindings)) "exactly even forms in binding vector")
   (if (== 2 (count bindings))
     `(let [temp# ~(second bindings)]
        (if temp#
          (let [~(first bindings) temp#]
            ~then)
          ~else))
     (let [if-let-else (keyword (name (gensym "if_let_else__")))
           inner (fn inner [bindings]
                   (if (seq bindings)
                     `(if-let [~(first bindings) ~(second bindings)]
                        ~(inner (drop 2 bindings))
                        ~if-let-else)
                     then))]
       `(let [temp# ~(inner bindings)]
          (if (= temp# ~if-let-else) ~else temp#))))))

(defmacro if-some
  "Multiple binding version of if-some"
  ([bindings then]
   `(if-some ~bindings ~then nil))
  ([bindings then else]
   (assert-args
     (vector? bindings) "a vector for its binding"
     (even? (count bindings)) "exactly even forms in binding vector")
   (if (== 2 (count bindings))
     `(let [temp# ~(second bindings)]
        (if (nil? temp#)
          ~else
          (let [~(first bindings) temp#]
            ~then)))
     (let [if-some-else (keyword (name (gensym "if_some_else__")))
           inner (fn inner [bindings]
                   (if (seq bindings)
                     `(if-some [~(first bindings) ~(second bindings)]
                        ~(inner (drop 2 bindings))
                        ~if-some-else)
                     then))]
       `(let [temp# ~(inner bindings)]
          (if (= temp# ~if-some-else) ~else temp#))))))

(defmacro when-let
  "Multiple binding version of when-let"
  [bindings & body]
  (assert-args
    (vector? bindings) "a vector for its binding"
    (even? (count bindings)) "exactly even forms in binding vector")
  (if (== 2 (count bindings))
    `(let [temp# ~(second bindings)]
       (when temp#
         (let [~(first bindings) temp#]
           ~@body)))
    (if (seq bindings)
      `(when-let [~(first bindings) ~(second bindings)]
                 (when-let ~(vec (drop 2 bindings)) ~@body))
      `(do ~@body))))

(defmacro when-some
  "Multiple binding version of when-some"
  [bindings & body]
  (assert-args
    (vector? bindings) "a vector for its binding"
    (even? (count bindings)) "exactly even forms in binding vector")
  (if (== 2 (count bindings))
    `(let [temp# ~(second bindings)]
       (if (nil? temp#)
         nil
         (let [~(first bindings) temp#]
           ~@body)))
    (if (seq bindings)
      `(when-some [~(first bindings) ~(second bindings)]
                 (when-some ~(vec (drop 2 bindings)) ~@body))
      `(do ~@body))))
Comment by Alex Miller [ 24/Jul/17 9:42 AM ]

This ticket description starts with a solution. Please instead follow the guidelines for tickets that is written up at https://dev.clojure.org/display/community/Creating+Tickets.

The description should include (in this order):

  • One or two examples demonstrating the cases where existing if-let or when-let are cumbersome
  • A description (NOT code) of the proposed enhancement that will improve this. Describe the semantics of the proposed changes. For example, what happens if only some of the bindings are true? Are previous local bindings available to later local bindings? etc. If there is more than one reasonable answer to these questions, describe the options.
  • State which patch should be considered

Also as an aside, note that if-let and when-let now have specs in core.specs.alpha - if you modify these, you must also include a patch that updates the specs. If you add new macros, you should include a patch that adds the corresponding specs. As the specs are on a different repo, the spec change changes should be in a separate patch.





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

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

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

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


Attachments: Text File clj-2116.patch    

 Description   

Problem

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

Details

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

Examples:

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

Here is a more complete example:

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

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

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

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

Suggestion

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

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

Using it would look like this:

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

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

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

Alternative

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

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



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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

initial work as patch.





[CLJ-2209] getting started documentation for 1.9.0-alpha uses 1.8.0.jar and does not work with 1.9.0-alpha17.jar due to missing specs related jars in the classpath Created: 20/Jul/17  Updated: 20/Jul/17  Resolved: 20/Jul/17

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

Type: Defect Priority: Major
Reporter: Michael A Wright Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None
Environment:

clojure-1.9.0-alpha17
Java 1.8.0_73-b02
Mac OS X 10.9.5



 Description   

in Https://clojure.org/guides/getting_started when I try:
java -cp clojure-1.8.0.jar clojure.main
it works (starts a REPL) as expected.
When I change to clojure-1.9.0-alpha17.jar I get an error
Caused by: java.io.FileNotFoundException: Could not locate clojure/spec/alpha__init.class or clojure/spec/alpha.clj on classpath.

In order to start a REPL this way with 1.9.0-alpha17, I had to use lein to fetch other dependencies and construct a command like this:

java -cp $HOME/.m2/repository/org/clojure/clojure/1.9.0-alpha17/clojure-1.9.0-alpha17.jar:$HOME/.m2/repository/org/clojure/spec.alpha/0.1.123/spec.alpha-0.1.123.jar:$HOME/.m2/repository/org/clojure/core.specs.alpha/0.1.10/core.specs.alpha-0.1.10.jar:. clojure.main

I also was able to unjar the three jar files and rejar into one jar file to simplify the classpath to one jar.

I'd like to see the getting started documentation address this somehow for Clojure newcomers (after 1.9.0 is released) or for experienced Clojure users that want a quicker way to start a REPL (instead of boot or lein) occasionally.

Perhaps there could be a way to bootstrap a minimal clojure repl without using lein or boot (and without requiring the downloading, and specifying the classpath for, three jar files? It would be nice for newcomers to kick the tires with minimal effort. Perhaps a simplified version of clojure.jar called clojure-repl.jar?



 Comments   
Comment by Michael A Wright [ 20/Jul/17 2:55 PM ]

https://groups.google.com/forum/#!msg/clojure/10dbF7w2IQo/ec37TzP5AQAJ has some discussion related to this. Is there another issue or plan somewhere that is tracking the work needed to update the Getting Started section to correctly work with a clojure-1.9.0.jar when it is release?

Comment by Alex Miller [ 20/Jul/17 9:40 PM ]

Hi Michael, I actually gave a talk about our plans for 1.9 in this area today at EuroClojure and I will have a better writeup for public consumption on that next week.

The Getting Started page is (always) targeted at the current stable release, currently 1.8, and that's something that we will update when Clojure 1.9 is released as part of our typical release process. In this case, it will be substantially updated based on the new stuff coming shortly. Because this is part of our standard release process, I don't need a ticket for it here so I will close this.





[CLJ-2205] Consider security implications of proxy implementation Created: 12/Jul/17  Updated: 17/Jul/17  Resolved: 17/Jul/17

Status: Closed
Project: Clojure
Component/s: None
Affects Version/s: Release 1.2, Release 1.3, Release 1.4, Release 1.5, Release 1.6, Release 1.7, Release 1.8
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Alex Miller Assignee: Chouser
Resolution: Duplicate Votes: 1
Labels: proxy, security


 Description   

Per the explanation in CLJ-2204, AOT compiled proxies of Serializable/Externalizable classes are susceptible to misuse for deserialization attacks. We should consider modifications to proxy to detect and warn or prevent this kind of misuse.



 Comments   
Comment by Chouser [ 17/Jul/17 11:06 PM ]

Superceded by CLJ-2204





[CLJ-1189] Map-destructuring :or fumble needs compiler warning Created: 31/Mar/13  Updated: 16/Jul/17

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

Type: Enhancement Priority: Minor
Reporter: Phill Wolf Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: destructuring, errormsgs

Attachments: Text File CLJ-1189-p1.patch    
Patch: Code and Test

 Description   

Here is a map-destructuring blunder that I wish the compiler warned about:

(defn server
  [{servlet ::servlet type ::type :or {::type :jetty} :as service-map}])

It would be splendid to get a warning that :or keys that are not symbols being bound have no effect.

The incomplete code snippet above comes from Pedestal.service 0.1.0.

Here is a complete one-line example with the coding error:

user> (defn picnic [{botulism :botulism :or {:botulism 6}}] botulism) 
#'user/picnic 
user> (picnic {}) 
nil 
user> ;; I intended 6.


 Comments   
Comment by Gary Fredericks [ 26/May/13 8:25 AM ]

Should this be a warning or an exception? I don't know of any similar example of the compiler giving a warning, so I would expect the latter.

Comment by Gary Fredericks [ 26/May/13 9:54 AM ]

Added a patch that throws an exception when :or is not a map or its keys are not symbols. Also some tests.

Comment by Phill Wolf [ 16/Jul/17 8:51 AM ]

Still an issue in 1.8 - which I hope may be addressed in the course of other destructuring enhancements in 1.9. How may I update the "versions" field?

Comment by Alex Miller [ 16/Jul/17 3:54 PM ]

These are compile errors in Clojure 1.9 due to the new core specs on defn (and other let destructuring locations).





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

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

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

Approval: Triaged

 Description   

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

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

For example, given:

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

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

Workarounds:

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

Proposed:

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



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

I forgot to add this proposal:

Proposal

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

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

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





[CLJ-1814] Make `satisfies?` as fast as a protocol method call Created: 11/Sep/15  Updated: 14/Jul/17

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

Type: Enhancement Priority: Critical
Reporter: Nicola Mometto Assignee: Unassigned
Resolution: Unresolved Votes: 10
Labels: performance, protocols

Attachments: Text File 0001-CLJ-1814-cache-protocol-impl-satisfies-as-fast-as-me.patch     Text File 0001-CLJ-1814-cache-protocol-impl-satisfies-as-fast-as-me-v2.patch     Text File 0001-CLJ-1814-cache-protocol-impl-satisfies-as-fast-as-me-v3.patch     Text File 0001-CLJ-1814-cache-protocol-impl-satisfies-as-fast-as-me-v4.patch    
Patch: Code
Approval: Triaged

 Description   

Currently `satisfies?` doesn't use the same impl cache used by protocol methods, making it too slow for real world usage.

With:

(defprotocol p (f [_]))
(deftype x [])
(deftype y [])
(extend-type x p (f [_]))

Before patch:

(let [s "abc"] (bench (instance? CharSequence s))) ;; Execution time mean : 1.358360 ns
(let [x (x.)] (bench (satisfies? p x))) ;; Execution time mean : 112.649568 ns
(let [y (y.)] (bench (satisfies? p y))) ;; Execution time mean : 2.605426 µs

Cause: `satisfies?` calls `find-protocol-impl` to see whether an object implements a protocol, which checks for whether x is an instance of the protocol interface or whether x's class is one of the protocol implementations (or if its in an inheritance chain that would make this true). This check is fairly expensive and not cached.

Proposed: Extend the protocol's method impl cache to also handle (and cache) instance checks (including negative results).

After patch:

(let [x (x.)] (bench (satisfies? p x))) ;; Execution time mean : 79.321426 ns
(let [y (y.)] (bench (satisfies? p y))) ;; Execution time mean : 77.410858 ns

Patch: 0001-CLJ-1814-cache-protocol-impl-satisfies-as-fast-as-me-v4.patch



 Comments   
Comment by Michael Blume [ 11/Sep/15 4:17 PM ]

Nice. Honeysql used to spend 80-90% of its time in satisfies? calls before we refactored them out.

Comment by Michael Blume [ 24/Sep/15 3:55 PM ]

I realize this is a deeply annoying bug to reproduce, but if I clone core.match, point its Clojure dependency to 1.8.0-master-SNAPSHOT, start a REPL, connect to the REPL from vim, and reload clojure.core.match, I get

|| java.lang.Exception: namespace 'clojure.tools.analyzer.jvm.utils' not found, compiling:(clojure/tools/analyzer/jvm.clj:9:1)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5647| clojure.core$throw_if.invokeStatic
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5733| clojure.core$load_lib.invokeStatic
|| clojure.core$load_lib.doInvoke(core.clj)
|| clojure.lang.RestFn.applyTo(RestFn.java:142)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|647| clojure.core$apply.invokeStatic
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5765| clojure.core$load_libs.invokeStatic
|| clojure.core$load_libs.doInvoke(core.clj)
|| clojure.lang.RestFn.applyTo(RestFn.java:137)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|647| clojure.core$apply.invokeStatic
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5787| clojure.core$require.invokeStatic
|| clojure.core$require.doInvoke(core.clj)
|| clojure.lang.RestFn.invoke(RestFn.java:703)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/tools.analyzer.jvm/0.6.5/tools.analyzer.jvm-0.6.5.jar::clojure/tools/analyzer/jvm.clj|9| clojure.tools.analyzer.jvm$eval4968$loading__5561__auto____4969.invoke
zipfile:/Users/michael.blume/.m2/repository/org/clojure/tools.analyzer.jvm/0.6.5/tools.analyzer.jvm-0.6.5.jar::clojure/tools/analyzer/jvm.clj|9| clojure.tools.analyzer.jvm$eval4968.invokeStatic
|| clojure.tools.analyzer.jvm$eval4968.invoke(jvm.clj)
|| clojure.lang.Compiler.eval(Compiler.java:6934)
|| clojure.lang.Compiler.eval(Compiler.java:6923)
|| clojure.lang.Compiler.load(Compiler.java:7381)
|| clojure.lang.RT.loadResourceScript(RT.java:372)
|| clojure.lang.RT.loadResourceScript(RT.java:363)
|| clojure.lang.RT.load(RT.java:453)
|| clojure.lang.RT.load(RT.java:419)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5883| clojure.core$load$fn__5669.invoke
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5882| clojure.core$load.invokeStatic
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5683| clojure.core$load_one.invokeStatic
|| clojure.core$load_one.invoke(core.clj)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5728| clojure.core$load_lib$fn__5618.invoke
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5727| clojure.core$load_lib.invokeStatic
|| clojure.core$load_lib.doInvoke(core.clj)
|| clojure.lang.RestFn.applyTo(RestFn.java:142)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|647| clojure.core$apply.invokeStatic
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5765| clojure.core$load_libs.invokeStatic
|| clojure.core$load_libs.doInvoke(core.clj)
|| clojure.lang.RestFn.applyTo(RestFn.java:137)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|647| clojure.core$apply.invokeStatic
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5787| clojure.core$require.invokeStatic
|| clojure.core$require.doInvoke(core.clj)
|| clojure.lang.RestFn.invoke(RestFn.java:457)
src/main/clojure/clojure/core/match.clj|1| clojure.core.match$eval4960$loading__5561__auto____4961.invoke
src/main/clojure/clojure/core/match.clj|1| clojure.core.match$eval4960.invokeStatic
|| clojure.core.match$eval4960.invoke(match.clj)
|| clojure.lang.Compiler.eval(Compiler.java:6934)
|| clojure.lang.Compiler.eval(Compiler.java:6923)
|| clojure.lang.Compiler.load(Compiler.java:7381)
|| clojure.lang.RT.loadResourceScript(RT.java:372)
|| clojure.lang.RT.loadResourceScript(RT.java:363)
|| clojure.lang.RT.load(RT.java:453)
|| clojure.lang.RT.load(RT.java:419)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5883| clojure.core$load$fn__5669.invoke
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5882| clojure.core$load.invokeStatic
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5683| clojure.core$load_one.invokeStatic
|| clojure.core$load_one.invoke(core.clj)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5728| clojure.core$load_lib$fn__5618.invoke
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5727| clojure.core$load_lib.invokeStatic
|| clojure.core$load_lib.doInvoke(core.clj)
|| clojure.lang.RestFn.applyTo(RestFn.java:142)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|647| clojure.core$apply.invokeStatic
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5765| clojure.core$load_libs.invokeStatic
|| clojure.core$load_libs.doInvoke(core.clj)
|| clojure.lang.RestFn.applyTo(RestFn.java:137)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|647| clojure.core$apply.invokeStatic
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5787| clojure.core$require.invokeStatic
|| clojure.core$require.doInvoke(core.clj)
|| clojure.lang.RestFn.invoke(RestFn.java:421)
|| clojure.core.match$eval4949.invokeStatic(form-init2494799382238714928.clj:1)
|| clojure.core.match$eval4949.invoke(form-init2494799382238714928.clj)
|| clojure.lang.Compiler.eval(Compiler.java:6934)
|| clojure.lang.Compiler.eval(Compiler.java:6897)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|3096| clojure.core$eval.invokeStatic
|| clojure.core$eval.invoke(core.clj)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/main.clj|240| clojure.main$repl$read_eval_print__7404$fn__7407.invoke
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/main.clj|240| clojure.main$repl$read_eval_print__7404.invoke
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/main.clj|258| clojure.main$repl$fn__7413.invoke
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/main.clj|258| clojure.main$repl.invokeStatic
|| clojure.main$repl.doInvoke(main.clj)
|| clojure.lang.RestFn.invoke(RestFn.java:1523)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/tools.nrepl/0.2.10/tools.nrepl-0.2.10.jar::clojure/tools/nrepl/middleware/interruptible_eval.clj|58| clojure.tools.nrepl.middleware.interruptible_eval$evaluate$fn__637.invoke
|| clojure.lang.AFn.applyToHelper(AFn.java:152)
|| clojure.lang.AFn.applyTo(AFn.java:144)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|645| clojure.core$apply.invokeStatic
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|1874| clojure.core$with_bindings_STAR_.invokeStatic
|| clojure.core$with_bindings_STAR_.doInvoke(core.clj)
|| clojure.lang.RestFn.invoke(RestFn.java:425)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/tools.nrepl/0.2.10/tools.nrepl-0.2.10.jar::clojure/tools/nrepl/middleware/interruptible_eval.clj|56| clojure.tools.nrepl.middleware.interruptible_eval$evaluate.invokeStatic
|| clojure.tools.nrepl.middleware.interruptible_eval$evaluate.invoke(interruptible_eval.clj)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/tools.nrepl/0.2.10/tools.nrepl-0.2.10.jar::clojure/tools/nrepl/middleware/interruptible_eval.clj|191| clojure.tools.nrepl.middleware.interruptible_eval$interruptible_eval$fn__679$fn__682.invoke
zipfile:/Users/michael.blume/.m2/repository/org/clojure/tools.nrepl/0.2.10/tools.nrepl-0.2.10.jar::clojure/tools/nrepl/middleware/interruptible_eval.clj|159| clojure.tools.nrepl.middleware.interruptible_eval$run_next$fn__674.invoke
|| clojure.lang.AFn.run(AFn.java:22)
|| java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
|| java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
|| java.lang.Thread.run(Thread.java:745)

Same thing with reloading a namespace in my own project which depends on clojure.core.match

Comment by Nicola Mometto [ 24/Sep/15 3:59 PM ]

is it possible that AOT is involved?

Comment by Michael Blume [ 24/Sep/15 5:31 PM ]

Narrowed it down a little, if I check out tools.analyzer.jvm, open a REPL, and do (require 'clojure.tools.analyzer.jvm.utils) I get

|| java.lang.ClassCastException: java.lang.Class cannot be cast to clojure.asm.Type, compiling:(utils.clj:260:13)
|| clojure.lang.Compiler$InvokeExpr.eval(Compiler.java:3642)
|| clojure.lang.Compiler$InvokeExpr.eval(Compiler.java:3636)
|| clojure.lang.Compiler$DefExpr.eval(Compiler.java:450)
|| clojure.lang.Compiler.eval(Compiler.java:6939)
|| clojure.lang.Compiler.load(Compiler.java:7381)
|| clojure.lang.RT.loadResourceScript(RT.java:372)
|| clojure.lang.RT.loadResourceScript(RT.java:363)
|| clojure.lang.RT.load(RT.java:453)
|| clojure.lang.RT.load(RT.java:419)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5883| clojure.core$load$fn__5669.invoke
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5882| clojure.core$load.invokeStatic
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5683| clojure.core$load_one.invokeStatic
|| clojure.core$load_one.invoke(core.clj)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5728| clojure.core$load_lib$fn__5618.invoke
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5727| clojure.core$load_lib.invokeStatic
|| clojure.core$load_lib.doInvoke(core.clj)
|| clojure.lang.RestFn.applyTo(RestFn.java:142)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|647| clojure.core$apply.invokeStatic
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5765| clojure.core$load_libs.invokeStatic
|| clojure.core$load_libs.doInvoke(core.clj)
|| clojure.lang.RestFn.applyTo(RestFn.java:137)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|647| clojure.core$apply.invokeStatic
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|5787| clojure.core$require.invokeStatic
|| clojure.core$require.doInvoke(core.clj)
|| clojure.lang.RestFn.invoke(RestFn.java:421)
|| clojure.tools.analyzer.jvm.utils$eval4392.invokeStatic(form-init8663423518975891793.clj:1)
|| clojure.tools.analyzer.jvm.utils$eval4392.invoke(form-init8663423518975891793.clj)
|| clojure.lang.Compiler.eval(Compiler.java:6934)
|| clojure.lang.Compiler.eval(Compiler.java:6897)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|3096| clojure.core$eval.invokeStatic
|| clojure.core$eval.invoke(core.clj)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/main.clj|240| clojure.main$repl$read_eval_print__7404$fn__7407.invoke
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/main.clj|240| clojure.main$repl$read_eval_print__7404.invoke
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/main.clj|258| clojure.main$repl$fn__7413.invoke
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/main.clj|258| clojure.main$repl.invokeStatic
|| clojure.main$repl.doInvoke(main.clj)
|| clojure.lang.RestFn.invoke(RestFn.java:1523)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/tools.nrepl/0.2.10/tools.nrepl-0.2.10.jar::clojure/tools/nrepl/middleware/interruptible_eval.clj|58| clojure.tools.nrepl.middleware.interruptible_eval$evaluate$fn__637.invoke
|| clojure.lang.AFn.applyToHelper(AFn.java:152)
|| clojure.lang.AFn.applyTo(AFn.java:144)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|645| clojure.core$apply.invokeStatic
zipfile:/Users/michael.blume/.m2/repository/org/clojure/clojure/1.8.0-master-SNAPSHOT/clojure-1.8.0-master-SNAPSHOT.jar::clojure/core.clj|1874| clojure.core$with_bindings_STAR_.invokeStatic
|| clojure.core$with_bindings_STAR_.doInvoke(core.clj)
|| clojure.lang.RestFn.invoke(RestFn.java:425)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/tools.nrepl/0.2.10/tools.nrepl-0.2.10.jar::clojure/tools/nrepl/middleware/interruptible_eval.clj|56| clojure.tools.nrepl.middleware.interruptible_eval$evaluate.invokeStatic
|| clojure.tools.nrepl.middleware.interruptible_eval$evaluate.invoke(interruptible_eval.clj)
zipfile:/Users/michael.blume/.m2/repository/org/clojure/tools.nrepl/0.2.10/tools.nrepl-0.2.10.jar::clojure/tools/nrepl/middleware/interruptible_eval.clj|191| clojure.tools.nrepl.middleware.interruptible_eval$interruptible_eval$fn__679$fn__682.invoke
zipfile:/Users/michael.blume/.m2/repository/org/clojure/tools.nrepl/0.2.10/tools.nrepl-0.2.10.jar::clojure/tools/nrepl/middleware/interruptible_eval.clj|159| clojure.tools.nrepl.middleware.interruptible_eval$run_next$fn__674.invoke
|| clojure.lang.AFn.run(AFn.java:22)
|| java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
|| java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
|| java.lang.Thread.run(Thread.java:745)

I don't see where AOT would be involved?

Comment by Nicola Mometto [ 27/Sep/15 2:28 PM ]

Michael Blume The updated patch should fix the issue you reported

Comment by Michael Blume [ 28/Sep/15 12:39 PM ]

Cool, thanks =)

New patch no longer deletes MethodImplCache, which is not used – is that deliberate?

Comment by Alex Miller [ 02/Nov/15 3:08 PM ]

It would be cool if there was a bulleted list of the things changed in the patch in the description. For example: "Renamed MethodImplCache to ImplCache", etc. That helps makes it easier to review.

Comment by Nicola Mometto [ 02/Nov/15 3:35 PM ]

Attached is an updated patch that doesn't replace MethodImplCache with ImplCache but simply reuses MethodImplCache, reducing the impact of this patch and making it easier (and safer) to review.

Comment by Alex Miller [ 07/Jun/16 11:42 AM ]

Bumping priority as this is used in new inst? predicate - see https://github.com/clojure/clojure/commit/58227c5de080110cb2ce5bc9f987d995a911b13e

Comment by Alex Miller [ 29/Jun/17 2:31 PM ]

I ran the before and after tests with the v3 patch. Before times matched pretty closely but I could not replicate the after results. I got this which is actually much worse in the not found case:

(let [x (x.)] (bench (satisfies? p x))) ;; Execution time mean : 76.833504 ns
(let [y (y.)] (bench (satisfies? p y))) ;; Execution time mean : 20.570007 µs
Comment by Nicola Mometto [ 29/Jun/17 4:09 PM ]

v4 patch fixes the regression on the not-found case, not sure how that happened, apologies.
Here are the timings I'm getting now:

clojure master:

user=> (let [x (x.)] (bench (satisfies? p x)))
Evaluation count : 604961580 in 60 samples of 10082693 calls.
             Execution time mean : 112.649568 ns
    Execution time std-deviation : 12.216782 ns
   Execution time lower quantile : 99.299203 ns ( 2.5%)
   Execution time upper quantile : 144.265205 ns (97.5%)
                   Overhead used : 1.898271 ns

Found 3 outliers in 60 samples (5.0000 %)
	low-severe	 2 (3.3333 %)
	low-mild	 1 (1.6667 %)
 Variance from outliers : 73.7545 % Variance is severely inflated by outliers
nil
user=> (let [y (y.)] (bench (satisfies? p y)))
Evaluation count : 22676100 in 60 samples of 377935 calls.
             Execution time mean : 2.605426 µs
    Execution time std-deviation : 141.100070 ns
   Execution time lower quantile : 2.487234 µs ( 2.5%)
   Execution time upper quantile : 2.873045 µs (97.5%)
                   Overhead used : 1.898271 ns

Found 1 outliers in 60 samples (1.6667 %)
	low-severe	 1 (1.6667 %)
 Variance from outliers : 40.1251 % Variance is moderately inflated by outliers
nil

master + v4:

user=> (let [x (x.)] (bench (satisfies? p x)))
Evaluation count : 731759100 in 60 samples of 12195985 calls.
             Execution time mean : 79.321426 ns
    Execution time std-deviation : 3.959245 ns
   Execution time lower quantile : 75.365187 ns ( 2.5%)
   Execution time upper quantile : 87.986479 ns (97.5%)
                   Overhead used : 1.905711 ns

Found 1 outliers in 60 samples (1.6667 %)
	low-severe	 1 (1.6667 %)
 Variance from outliers : 35.2614 % Variance is moderately inflated by outliers
nil
user=> (let [y (y.)] (bench (satisfies? p y)))
Evaluation count : 771220140 in 60 samples of 12853669 calls.
             Execution time mean : 77.410858 ns
    Execution time std-deviation : 1.407926 ns
   Execution time lower quantile : 75.852530 ns ( 2.5%)
   Execution time upper quantile : 80.759226 ns (97.5%)
                   Overhead used : 1.897646 ns

Found 4 outliers in 60 samples (6.6667 %)
	low-severe	 3 (5.0000 %)
	low-mild	 1 (1.6667 %)
 Variance from outliers : 7.7866 % Variance is slightly inflated by outliers

So to summarize:
master found = 112ns
master not-found = 2.6us

master+v4 found = 79ns
master+v4 not-found = 77ns

Comment by Michael Blume [ 14/Jul/17 5:12 PM ]

For records that have been declared with an implementation of a particular protocol, and which therefore implement the corresponding interface, would it make sense to use an (instance?) check against that interface as a fast path?





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

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

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


 Description   

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

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

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

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

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

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

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

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


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

run.sh in the linked github repo throws:

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

and fails to compile the required java source

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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





[CLJ-2206] Facilities to alias (re-import) functions, variables and macros Created: 13/Jul/17  Updated: 13/Jul/17  Resolved: 13/Jul/17

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

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


 Description   

Organizing namespaces can be hard. Especially in Clojurescript where, due two stage macro compilation, creation of artificial namespaces leaves little room to move around. An often used re-def (def xyz ns/xyz) doesn't preserve metadata.

Zach Tellman's potemkin implements import facilities for functions, vars and macros in Clojure. Would be great to have something similar in Clojure(script) proper.



 Comments   
Comment by Alex Miller [ 13/Jul/17 2:04 PM ]

Thanks, but we do not expect to add var import functionality to Clojure. Namespaces exist to give context to vars and allowing them to be "imported" to other namespaces works against the use of namespaces for naming.

If ClojureScript has problems specific to ClojureScript, solutions can be considered there as needed.





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

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

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

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



 Description   

Repro:

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

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

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

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

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



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

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

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

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

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





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

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

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


 Description   

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

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



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

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

It certainly harms discoverability of these fns.

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

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

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

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

I think this is very, very unlikely to ever happen

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

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

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

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

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

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

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

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





[CLJ-2165] #clojure/var tag for transmitting var identity Created: 22/May/17  Updated: 11/Jul/17

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

Type: Feature Priority: Major
Reporter: Alex Miller Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: print, reader, var

Attachments: Text File vartag2.patch    
Approval: Vetted

 Description   

Currently one can't send vars around in edn. #' is clojure reader specific. Objective is to transmit var identity and bind to same-named var on reading side (a la var serialization support).

Proposed: This is not generic enough to add to edn, so use #clojure/var for tag. Printing may print #clojure/var instead of #' (perhaps via a flag) - needs more assessment. #clojure/var tag reader should be installed in data readers.

Patch: vartag2.patch



 Comments   
Comment by Christophe Grand [ 11/Jul/17 10:14 AM ]

Should unnamed vars (eg created by with-local-vars) print to #clojure/var nil or throw an exception? (exception is the print-dup behavior)





[CLJ-790] Primitive type hints on function names should print error message Created: 10/May/11  Updated: 10/Jul/17

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

Type: Enhancement Priority: Major
Reporter: Alan Dipert Assignee: Alan Dipert
Resolution: Unresolved Votes: 2
Labels: errormsgs, typehints

Approval: Triaged

 Description   

Functions returning primitives are hinted with metadata on the argument list, not on the function name. Using a primitive type hint on a function name should print an error message.

Currently, misplaced primitive hints are read without error.



 Comments   
Comment by Andy Fingerhut [ 23/Feb/15 9:20 PM ]

One can type hint a primitive value on a Var naming a function, or any value one wants, like so:

(def {:tag 'long} foo 17)

(defn {:tag 'double} bar [x y]
(* 2.0 x y))

I think it is odd that one must use {:tag 'long} instead of ^long, since trying to use ^long ends up giving the useless type hint that is the value of the function clojure.core/long.

However, the Clojure compiler will use the primitive type hints as shown in the examples above to avoid reflection in appropriate Java interop calls, so making them an error seems undesirable.

Comment by lvh [ 02/Jul/16 10:07 AM ]

Alternatively, perhaps the compiler could simply use the type hint? While ^long is useless now, its intent seems unambiguous.

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

Description could use some examples





[CLJ-1863] Bad type hints on a defn cause the compiler to throw a NPE Created: 04/Dec/15  Updated: 10/Jul/17

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

Type: Defect Priority: Major
Reporter: Timothy Baldridge Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: compiler, errormsgs, typehints

Approval: Triaged

 Description   

After CLJ-1232 was committed to master, it is possible for the Clojure compiler to throw a NPE if a defn is type hinted with a invalid type. This surfaces in CLJS where the defn macro is re-used by the ClojureScript compiler, but I think it raises the question: "Should a bad type hint result in a compiler exception?"

The offending line can be found here on GitHub: https://github.com/clojure/clojure/blob/master/src/clj/clojure/core.clj#L247



 Comments   
Comment by Alex Miller [ 18/Dec/15 8:12 AM ]

This is basically the same as CLJ-1868, but I think what you are asking here is whether bad type hints should be ignored or throw any exception, right?

(Whereas CLJ-1868 is about which exception/message is thrown)

Comment by Timothy Baldridge [ 18/Dec/15 8:22 AM ]

Agreed. I think another possible solution would be to update CLJS to not use the CLJ defn, but I still think that a bad type hint should just be ignored.

Comment by Nicola Mometto [ 18/Dec/15 8:29 AM ]

I don't agree that we shoud ignore bad type hints.
If the compiler knows that something is wrong, it should tell the user immediately rather than silently ignoring and potentially failing at runtime later

Comment by Alex Miller [ 10/Jul/17 2:09 PM ]

Description could use some examples





[CLJ-2203] Cannot create sets containing different empty collection, which prevents creating useful specs Created: 10/Jul/17  Updated: 10/Jul/17  Resolved: 10/Jul/17

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

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

[org.clojure/clojure "1.9.0-alpha17"]
[org.clojure/clojurescript "1.9.542"]



 Description   

In Clojure:

#{'() []}

Expected: No errors (the above works in Clojurescript)

Actual: "IllegalArgumentException Duplicate key: () clojure.lang.PersistentHashSet.createWithCheck (PersistentHashSet.java:56)"

Motivation: I am trying to define a spec for the "into" argument to "s/coll-of" e.g.

(require '[clojure.alpha.spec :as s])
(s/def :coll-of/into #{'() [] {}})

It is possible to define this spec in Clojurescript, but not Clojure. Using a set to define a spec is idiomatic, but if this is a corner case, it would be fine to use a new "one-of" spec operation that is more verbose than a new set, works the same way when used in a spec, but avoids these cases.



 Comments   
Comment by Alex Miller [ 10/Jul/17 1:22 PM ]

In Clojure, sequential collections are compared in the same "equality partition" - that is lists, vectors, sequences, and potentially other custom sequential collections compare as equal based on their contents, not based on the collection type. This design decision has many subtle tradeoffs but enables many important aspects of using Clojure collections and sequences together in seamless ways. There are no plans to change this, so I am declining this ticket, as it is working as designed.

Regarding ClojureScript, I would start with the typical expectation that its behavior should match Clojure, but also take into account the specifics of the host such that there might be good reasons why this behavior is different (but I don't know enough to say either way).

For this particular spec, I would recommend a spec like (s/and coll? empty?). But please note that it's not possible in most cases to actually instrument spec itself with specs because in many cases this will create an infinite recursion of checks. I have written specs for the spec namespace but in general I did not find it workable in practice to actually use them so we have chosen not to provide them in general. Specs for spec forms are a work in progress tracked in CLJ-2112.





[CLJ-2202] coll-of :min-count and :gen-max used together cause collections that are too large to be generated Created: 10/Jul/17  Updated: 10/Jul/17

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

Type: Defect Priority: Minor
Reporter: Sebastian Oberhoff Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: spec
Environment:

1.9.0-alpha16


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

 Description   

This should specify a spec whose generator always returns collections of size 4 or 5 but instead it generates collections of size 4 to 8:

(require '[clojure.spec.alpha :as s] '[clojure.spec.gen.alpha :as gen])
(map count (gen/sample (s/gen (s/coll-of char? :min-count 4 :gen-max 5))))
;; (4 7 8 8 4 7 4 5 5 7)

Cause: The max logic in s/every-impl is: (c/or max-count (max gen-max (c/* 2 (c/or min-count 0)))). If there is a max-count it's used, otherwise the larger of gen-max or 2*min-count is used. In this case, 2*min-count is 8. Seems like we should never generate more than gen-max though, so propose changing this logic to: (c/or max-count gen-max (c/* 2 (c/or min-count 0))).

Patch: clj-2202.patch






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

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

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


 Description   
  1. Spec metadata support

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

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

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

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

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

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

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

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



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

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

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

Changed example as per Nicolas' comment.

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

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

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

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





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

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

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

Attachments: Text File gen-let.patch    

 Description   

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

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



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

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

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

With this patch they could write this as:

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

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

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

e.g.

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




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

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

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


 Description   

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

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






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

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

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

Approval: Vetted

 Description   

Minimal failing case:

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

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






[CLJ-2200] for macro does not support multiple body expr Created: 05/Jul/17  Updated: 05/Jul/17  Resolved: 05/Jul/17

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

Type: Enhancement Priority: Trivial
Reporter: Isaac Zeng Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None
Environment:

Clojure


Attachments: Text File 0001-let-macro-for-support-multiple-body-expr.patch    
Patch: Code

 Description   

it is not convenient for write multiple expression in for body,
from this now, we must explicit use `do` form, eg.

this form not supported

(for [i (range 10)]
  (prn i)
  (inc i))

we must use `do` wrap body

(for [i (range 10)]
  (do
    (prn i)
    (inc i)))


 Comments   
Comment by Daniel Stockton [ 05/Jul/17 5:10 AM ]

for is list comprehension. There is doseq for side effects: https://clojuredocs.org/clojure.core/doseq

Comment by Isaac Zeng [ 05/Jul/17 6:57 AM ]

@danielstockton

In fact, Many times, I found it will more convenient if for support this feature.

eg.
I want to see what happened in for loops(as normally, I use prn),

I'm also asked many folks, they thought it is better if for has this feature.

Comment by Nicola Mometto [ 05/Jul/17 8:34 AM ]

This is by design to discourage side-effects in its body, not only `for` is list comprehension and not a loop construct, but it's also lazy so if we were to facilitate side-effecty bodies in for, many would trip up on the laziness.

Comment by Alex Miller [ 05/Jul/17 8:41 AM ]

Declined, per the reasons already listed.





[CLJ-2198] int-in-range? doesn't check for int-ness Created: 01/Jul/17  Updated: 01/Jul/17  Resolved: 01/Jul/17

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

Type: Defect Priority: Major
Reporter: Frederic Merizen Assignee: Unassigned
Resolution: Duplicate Votes: 0
Labels: spec
Environment:

Clojure 1.9.0-alpha17
Spec.alpha 0.1.123



 Description   
(require '[clojure.spec.alpha :as s])
(s/int-in-range? 1 4 1.5)
=> true

I expected the result to be false, because 1.5 is not an integer.

Cause of the bug: in the following snippet, int? is always truthy. It should read (int? val) instead.

(defn int-in-range?
  "Return true if start <= val, val < end and val is a fixed
  precision integer."
  [start end val]
  (c/and int? (<= start val) (< val end)))

The bug is fairly harmless in practice because int-in, the main client of int-in-range?, 'redundantly' checks for int-ness:

(defmacro int-in
  "Returns a spec that validates fixed precision integers in the
  range from start (inclusive) to end (exclusive)."
  [start end]
  `(spec (and int? #(int-in-range? ~start ~end %))
     :gen #(gen/large-integer* {:min ~start :max (dec ~end)})))


 Comments   
Comment by Alex Miller [ 01/Jul/17 2:37 PM ]

Dupe of https://dev.clojure.org/jira/browse/CLJ-2167 - fix should go in soon.





[CLJ-2195] integer (32-bit) bit operations not available Created: 30/Jun/17  Updated: 30/Jun/17  Resolved: 30/Jun/17

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

Type: Enhancement Priority: Minor
Reporter: Phillip Lord Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: interop


 Description   

It would be nice if Clojure provided implementations for all Java
operators, using primitive types. While Clojures numbers handling is
nice, there are algorithms for which it is a pain.

Clojure currently provides a number of functions to deal with this:
unchecked-add and unchecked-add-int, for instance. But, not all of the
Java operators are accessible in this way, at least to my knowledge;
specifically, the "&" and "|" operators have no function
equivalents. Actually, they are still present in source (Numbers.java)
but have been commented out. I can't really understand the reason for
this omission.



 Comments   
Comment by Andy Fingerhut [ 30/Jun/17 9:17 AM ]

Are these functions in clojure.core not what you are hoping to have?

user=> (pprint (apropos "bit-"))
(clojure.core/bit-and
clojure.core/bit-and-not
clojure.core/bit-clear
clojure.core/bit-flip
clojure.core/bit-not
clojure.core/bit-or
clojure.core/bit-set
clojure.core/bit-shift-left
clojure.core/bit-shift-right
clojure.core/bit-test
clojure.core/bit-xor
clojure.core/unsigned-bit-shift-right)

Comment by Alex Miller [ 30/Jun/17 9:32 AM ]

Although it supports all primitives for the purposes of Java interoperability, pure Clojure uses only long and double primitives. Clojure will promote ints to longs and floats to doubles as needed.

There are any plans to ever offer bit ops for 32-bit integers. You can use longs and the existing functions to get primitive-level long bit ops using `bit-and`, `bit-or`, `bit-xor`, `bit-not`, `bit-shift-left`, `bit-shift-right`, `unsigned-bit-shift-right`, etc.

Comment by Phillip Lord [ 30/Jun/17 10:25 AM ]

@Andy Fingerhut

user> (type (bit-and (int 0)(int 0)))
java.lang.Long

@Alex Miller Presume you mean "There are not any plans"...

I don't understand the logic though; there are already explicit functions both in clojure.core supporting int operations for add in both Numbers.java and core.clj, functions in Number.java for shiftLeftInt, shiftRightInt, and unsignedShiftRight. But &, |, ^ and ~ cannot be replicated, at least as far as I can tell. It seems a bit half-and-half. Why do some of operators in clojure, some only in Java, and some not at all?

We can work around this, of course, but if we have to port these operations into Java, it's probably easier to port the entire algorithm.

Comment by Alex Miller [ 30/Jun/17 11:18 AM ]

Sorry, yes - "there are not any plans".

It is difficult for me to go back in time (before I worked on Clojure) and summarize the results of months of design effort on Rich's part which presumably considered many tradeoffs and criteria. The end result though is: pure Clojure uses only long and double primitives and that's where we provide operations. There are a few cases where int versions of some functionality is available - those are likely either vestigial, or are used inside the Clojure implementation somewhere but if they're not surfaced in core, they're not part of the API.

Comment by Phillip Lord [ 30/Jun/17 11:36 AM ]

They are in the API. unchecked-add-int, unchecked-multiply-int are all surfaced in core. Just the arithmetic operators, but not the bitwise.

Anyway, thanks for the explanation; at least, now the situation is clear. I can either port the entire algorithms or just these operators out to Java.

Comment by Alex Miller [ 30/Jun/17 11:53 AM ]

The Java classes are the implementation, not the API.

Comment by Phillip Lord [ 30/Jun/17 12:42 PM ]

user=> clojure.core/unchecked-add-int
#object[clojure.core$unchecked_add_int 0x479395d7 "clojure.core$unchecked_add_int@479395d7"]
user=> (clojure.core/unchecked-add-int 1 2)
3
user=> (type (clojure.core/unchecked-add-int 1 2))
java.lang.Integer

So, int operations in the API, yes? But just for the arithmetic operators and not for the bitwise ones.

Comment by Ghadi Shayban [ 30/Jun/17 1:59 PM ]

Symmetry is not one of the stronger API design motivations

Comment by Alex Miller [ 30/Jun/17 2:59 PM ]

Sorry, yes - some int operations are in the Clojure api. There are no plans to add the int bit ops.





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

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

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


 Description   

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

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

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

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

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

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

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

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

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

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

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

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

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

;; And confirm BadYer instances will throw when called.

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


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

;; Confirm generator works as expected:

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

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

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

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

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





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

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

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

Approval: Triaged

 Description   

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

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

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

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

I would like to write a spec like:

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

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

The current workaround:

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

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






[CLJ-1074] Read/print round-trip for +/-Infinity and NaN Created: 21/Sep/12  Updated: 30/Jun/17

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

Type: Defect Priority: Major
Reporter: Colin Jones Assignee: Unassigned
Resolution: Unresolved Votes: 7
Labels: print, reader

Attachments: Text File 0001-Read-Infinity-and-NaN.patch     Text File clj-1074-2.patch     Text File clj-1074-3.patch     Text File clj-1074-read-infinity-and-nan-patch-v2-plus-edn-reader.patch    
Patch: Code and Test
Approval: Prescreened

 Description   

A few float-related forms (namely, Double/POSITIVE_INFINITY, Double/NEGATIVE_INFINITY, Double/NaN) are not eval-able after a round-trip via

(read-string (binding [*print-dup* true] (pr-str f))

The two options I see are to provide print-method implementations for these and their Float cousins, or to make Infinity, -Infinity, +Infinity, and NaN readable values. Since it sounds like edn may want to provide a spec for these values (see https://groups.google.com/d/topic/clojure-dev/LeJpOhHxESs/discussion and https://github.com/edn-format/edn/issues/2), I think making these values directly readable as already printed is preferable. Something like Double/POSITIVE_INFINITY seems too low-level from edn's perspective, as it would refer to a Java class and constant.

I'm attaching a patch implementing reader support for Infinity, -Infinity, +Infinity, and NaN.

Patch: clj-1074-3.patch
Prescreened: Alex Miller



 Comments   
Comment by Timothy Baldridge [ 03/Dec/12 11:34 AM ]

Please bring this up on clojure-dev. We'll be able to vet this ticket after that.

Comment by Colin Jones [ 03/Dec/12 1:18 PM ]

Should I respond to my original clojure-dev post about this (linked in the issue description above), or start a new one?

Comment by Andy Fingerhut [ 24/May/13 1:11 PM ]

Patch clj-1074-read-infinity-and-nan-patch-v2.txt dated May 24 2013 is identical to 0001-Read-Infinity-and-NaN.patch dated Sep 21 2012, except it applies cleanly to latest master. The older patch conflicts with a recent commit made for CLJ-873.

Comment by Nicola Mometto [ 25/May/13 11:55 AM ]

clj-1074-read-infinity-and-nan-patch-v2-plus-edn-reader.patch is the same as clj-1074-read-infinity-and-nan-patch-v2.txt except it patches EdnReader too, but it must be applied after #CLJ-873 0001-Fix-CLJ-873-for-EdnReader-too.patch get merged

Comment by Andrew Tarzwell [ 12/Feb/15 12:01 PM ]

We're running into this bug now, applying the patch clj-1074-read-infinity-and-nan-patch-v2-plus-edn-reader.patch seems to resolve it on 1.7 master, but it would be an improvement to not depend on a patched version.

Is there a fix in the works? Or a more up to date conversation on why this hasn't been addressed?

Thanks,
Andrew

Comment by Andy Fingerhut [ 12/Feb/15 12:23 PM ]

Andrew, the tools.reader library provides this enhancement today, if you would prefer using unpatched software, and if it meets your needs. There are a few other small differences between it and the reader built into Clojure, which you can see near the end of its README: https://github.com/clojure/tools.reader

As far as why it hasn't been addressed yet, I think a short accurate answer is that the Clojure developers have been working on other issues, and this one has not been high enough priority to be addressed yet (disclaimer: This is in no way an official answer, just the best guess of an observer who watches this stuff too much).

I see you have voted on the ticket. Good. More votes can in some cases influence the Clojure developers to respond to a ticket earlier rather than later. You may try to persuade others to vote on it, too, if you wish.

If there is some production use of Clojure hindered by the lack of a fix, bringing this up in the Clojure or Clojure Dev Google groups couldn't hurt (signed CA on file required for Clojure Dev group membership – see http://clojure.org/contributing )

Comment by Andrew Tarzwell [ 12/Feb/15 1:46 PM ]

Andy,

Thank you for the quick response, I was unaware of tools.reader having this fixed. That should do us for now.

Comment by Alex Miller [ 24/Oct/16 9:43 AM ]

Refreshed patch to apply to current master. No semantic changes, attribution retained.

Comment by Alex Miller [ 30/Jun/17 9:03 AM ]

Updated patch so it would apply cleanly, no other changes, attribution retained.





[CLJ-2060] Add support for undefining a spec Created: 16/Nov/16  Updated: 30/Jun/17

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

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

Attachments: Text File clj-2060-2.patch     Text File clj-2060-3.patch     Text File clj-2060-4.patch     Text File clj-2060.patch    
Patch: Code and Test
Approval: Screened

 Description   

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

Proposed: Make s/def take nil as a way to "clear" a spec:

user=> (s/def ::a string?)
:user/a
user=> (s/def ::a nil)
:user/a
user=> (s/get-spec ::a)
nil

Patch: clj-2060-4.patch



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

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

Comment by Alex Miller [ 10/May/17 1:03 PM ]

Updated patch to apply to spec.alpha

Comment by Alex Miller [ 29/Jun/17 4:25 PM ]

Update s/def docstring as well in -4 patch.





[CLJ-2171] Default explain printer prints root val and spec Created: 31/May/17  Updated: 30/Jun/17

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

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

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

 Description   

In CLJ-2085 (added in spec.alpha 0.1.123 / Clojure 1.9.0-alpha17) we added new attributes to the explain-data map. However, explain-printer reports all non-problem attributes (previously was used for reporting the arguments in an instrument failure) so we are now getting a lot more printing than desired.

Example (last 2 lines here are new):

user=> (s/explain #{1 2} 5)
val: 5 fails predicate: :clojure.spec.alpha/unknown
:clojure.spec.alpha/spec  #{1 2}
:clojure.spec.alpha/value  5

Proposed: I think we should only print the ::problems and the instrument explain should have some more work done on it anyways to improve it separately.

Patch: clj-2171.patch






[CLJ-2167] int-in-range? throws exception when val not an int Created: 26/May/17  Updated: 30/Jun/17

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

Type: Defect Priority: Major
Reporter: David Bürgin Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: spec
Environment:

Clojure 1.9.0-alpha16
spec.alpha 0.1.123


Attachments: Text File int-in-range.patch    
Patch: Code
Approval: Screened

 Description   

The spec predicate int-in-range? throws an exception when not passed an int? value.

(require '[clojure.spec.alpha :as s])
(s/int-in-range? 0 10 :x)
ClassCastException clojure.lang.Keyword cannot be cast to java.lang.Number

Expected result: false

Cause: int-in-range? uses int? instead of (int? val), so that condition always evaluates true regardless of input.

Solution: Use (int? val) instead.

Patch: int-in-range.patch

Screened by: Alex Miller



 Comments   
Comment by Ghadi Shayban [ 26/May/17 1:05 PM ]

It's totally a typo & you should sign the CA. Sooner the better because there seems to be some spec bug fixing activity today





[CLJ-2183] `cat` specs should verify value is sequential Created: 08/Jun/17  Updated: 30/Jun/17

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

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

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


Attachments: Text File clj-2183.patch    
Patch: Code and Test
Approval: Screened

 Description   

Regex op specs can currently pass with maps or sets (which are unordered) but may give confusing errors.

(require '[clojure.spec.alpha :as s])
(s/def ::kv (s/cat :k keyword? :v any?))
(s/explain ::kv {"foo" "bar"})
In: [0] val: ["foo" "bar"] fails spec: :user/kv at: [:k] predicate: keyword?

Cause: Conforming fails because the first value of the map (the tuple pair `["foo" "bar"]`) is not a keyword

Proposed: Regex op specs currently check `coll?`, which will pass unordered collections like sets or maps - this is unlikely to be useful for positional regex specs. Propose to narrow that check to `sequential?`. On failure, use an explain pred that describes the actual check (the current one just repeats the regex spec instead).

With the patch, the message actually tells you the actual predicate that is failing (the sequential? check):

val: {"foo" "bar"} fails spec: :user/kv predicate: (or (nil? %) (sequential? %))

Patch: clj-2183.patch

Screened by: Chouser - while making unordered colls invalid is the intent of the patch, a gray area is that of sorted colls (sorted sets, etc). These could have been matched with the prior impl, but will not be after the change. See comments for more examples.



 Comments   
Comment by Chouser [ 20/Jun/17 11:44 PM ]

Note, I believe this is a breaking change. Before the patch, this works:

(s/def ::dt (s/cat :datep (s/spec (s/cat :datek #{::date}, :datev number?))
                   :timep (s/spec (s/cat :timek #{::time}, :timev number?))))

(s/explain ::dt {::date 10, ::time 20})
;; Success!

After the patch, it fails:

(s/explain ::dt {::date 10, ::time 20})
;; val: #:user{:date 10, :time 20} fails spec: :user/dt predicate: (or (nil? %) (sequential? %))
;; :clojure.spec.alpha/spec  :user/dt
;; :clojure.spec.alpha/value  #:user{:date 10, :time 20}

However, even without the patch such specs will match only as long as the map (or set) entries are in the expected order. For hash-maps and hash-sets (or collections like array-maps that may be promoted to hash-maps), this is unpredictable and the patch arguably only "breaks" things that were at risk of breaking anyway.

Sorted collections are a little dicier. Without the patch, this arguably reasonable spec works reliably for sorted sets:

(s/def ::ab (s/cat :a (s/? #{"a"}) :b (s/? #{"b"})))
(s/explain ::ab (sorted-set "a" "b"))
;; Success!

With the patch, the sorted-set doesn't match because it's not a sequential collection, thus a breaking change.

Fortunately spec is still alpha so breaking changes are ok, and specs that intend to match sorted collections have several other options for doing so, especially the spec macros meant for non-sequential collections, like so:

(s/def ::ab (s/coll-of #{"a" "b"}))
Comment by Alex Miller [ 21/Jun/17 1:39 AM ]

Yes, the idea here is that this constrains the scope of what regex op specs support, so the "breaking" part of it is intentional.

Sorted colls is an interesting question, seems like a gray area. Personally, I'd be happy to rule it out, but maybe Rich would disagree. For something like this, it's best to note it as screener notes in the description as Rich doesn't necessarily read all the comments. I'll add it as an example.





[CLJ-2103] s/coll-of and s/every gen is very slow if :kind specified without :into Created: 28/Jan/17  Updated: 29/Jun/17

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

Type: Defect Priority: Critical
Reporter: Leon Grapenthin Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: generator, spec
Environment:

alpha14


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

 Description   

An s/coll-of with :kind but without :into takes a very long time to generate. You will mostly notice this when using stest/check but here is an example that replicates the important bits:

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

;; Basic coll-of - fast
(time (dorun (gen/sample (s/gen (s/coll-of int?)) 1000)))
;; "Elapsed time: 49.10318 msecs"

;; coll-of with :into - fast
(time (dorun (gen/sample (s/gen (s/coll-of int? :into ())) 1000)))
;; "Elapsed time: 50.169762 msecs"

;; coll-of with :kind - SLOW
(time (dorun (gen/sample (s/gen (s/coll-of int? :kind list?)) 1000)))
;; "Elapsed time: 14922.123514 msecs"

;; coll-of with :kind and :into - fast
(time (dorun (gen/sample (s/gen (s/coll-of int? :kind list? :into ())) 1000)))
"Elapsed time: 54.188577 msecs"

Cause: If :into is not provided, s/every and s/coll-of has to generate a collection (via gen of list?) to call empty on it, to then fill it up. This is quite clearly documented in the docstring of `s/every`:

:kind - a pred/spec that the collection type must satisfy, e.g. vector?
         (default nil) Note that if :kind is specified and :into is
         not, this pred must generate in order for every to generate.

Assumedly the list? generates quite large vectors at a certain point which significantly slows down the generation. The responsible code is in gen* of every-impl.

Proposed: Use standard empty coll for persistent collection :kind preds (list? vector? set? map?).

In the patch:

  • Create lookup table `empty-coll` for kind-form (resolved pred) to empty coll.
  • When possible (if :into coll is specified or kind-form is one of these special cases), determine the empty gen-into collection once at the beginning.
  • If empty gen-into collection is available, start with that in the generator. Otherwise, fall back to prior behavior for kind.

Performance-wise, the times after the patch are comparable to times using :into.

Patch: clj-2103.patch



 Comments   
Comment by Leon Grapenthin [ 14/Feb/17 3:03 PM ]

I see two approaches to improve this behavior:

1. The gen uses the gen of kind to generate one value with the smallest size, calls empty on it to determine :into. This would lead to a surprise when your :kind is e. g. (s/or :a-vec vector? :a-list list?) (which currently throws, anyway)
2. We use an internal lookup table to assume :into. {clojure.core/vector? [], clojure.core/set? #{} ...}





[CLJ-2182] s/& does not check preds if regex matches empty collection Created: 08/Jun/17  Updated: 29/Jun/17

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

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

Attachments: Text File clj-2182-2.patch     Text File clj-2182.patch    
Patch: Code and Test
Approval: Screened

 Description   

In the following example, the ::x key is required in the map but it will conform without error for an empty collection:

(s/def ::x integer?)
(s/conform (s/keys* :req-un [::x]) [])
;; nil, expected error
(s/explain (s/keys* :req-un [::x]) [])
;; Success!, expected explanation

Cause: At the moment, (s/keys* :req-un [::x]) is expanded to a form equivalent to the following one:

(s/& (s/* (s/cat ::s/k keyword? ::s/v))
     ::s/kvs->map
     (s/keys :req-un [::x]))

The issue seems to be in the implementation of s/&, specifically the ::amp branch of accept-nil? which expects that either the conformed regex returns empty or that the preds are not invalid. This seems like a false assumption that an empty conformed regex ret does not require the s/& preds to be valid. In this case we are using a conformer to transform the input and do another check, so it's certainly not valid here.

Proposed: Modify s/& to always validate the preds when accepting nil.

user=> (s/conform (s/keys* :req-un [::x]) [])
:clojure.spec.alpha/invalid

user=> (-> (s/explain-data (s/keys* :req-un [::x]) []) ::s/problems first :pred)
(clojure.core/fn [%] (clojure.core/contains? % :x))

Patch: clj-2182-2.patch



 Comments   
Comment by Alex Miller [ 29/Jun/17 4:20 PM ]

-2 patch includes a test





[CLJ-2176] s/tuple explain-data :pred problems Created: 05/Jun/17  Updated: 29/Jun/17

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

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

Attachments: Text File clj-2176.patch    
Patch: Code and Test
Approval: Screened

 Description   

As reported in CLJ-2175, s/tuple has some issues with reporting valid resolved preds in explain-data:

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

(-> (s/explain-data (s/tuple int?) :a) ::s/problems first :pred)
;;=> vector?   
;;EXPECTED: clojure.core/vector?

(-> (s/explain-data (s/tuple int?) []) ::s/problems first :pred)
;;=> (clojure.core/= (clojure.core/count %) 1)  
;;EXPECTED: (clojure.core/fn [%] (clojure.core/= (clojure.core/count %) 1))

Patch: clj-2176.patch






[CLJ-2177] s/keys explain-data :pred problem Created: 05/Jun/17  Updated: 29/Jun/17

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

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

Attachments: Text File clj-2177.patch    
Patch: Code and Test
Approval: Screened

 Description   

As reported in CLJ-2175, s/keys has an issue with reporting a valid resolved pred in explain-data:

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

(-> (s/explain-data (s/keys :req [::x]) :a) ::s/problems first :pred)
;;=> map?   
;;EXPECTED: clojure.core/map?

Patch: clj-2177.patch



 Comments   
Comment by Shogo Ohta [ 05/Jun/17 6:31 PM ]

Missing the patch?





[CLJ-2178] s/& explain-data :pred problem Created: 05/Jun/17  Updated: 29/Jun/17

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

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

Attachments: Text File clj-2178.patch    
Patch: Code and Test
Approval: Screened

 Description   

As reported in CLJ-2175, s/& has some issues with reporting valid resolved preds in explain-data:

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

(-> (s/explain-data (s/& int? even?) []) ::s/problems first :pred)
;;=> #function[clojure.core/int?]
;;EXPECTED: clojure.core/int?

(-> (s/explain-data (s/& int? even?) [0 2]) ::s/problems first :pred)
;;=> (clojure.spec.alpha/& #function[clojure.core/int?] clojure.core/even?)
;;EXPECTED: (clojure.spec.alpha/& clojure.core/int? clojure.core/even?)

Problem: s/& was not capturing the re form. Added a new :amp key to carry the re form (similar to :maybe key in s/?) and used that for describe when necessary.

Patch: clj-2178.patch






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

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

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

Approval: Triaged

 Description   

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

For example:

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

(defn a [x])

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

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

;; should pass
(stest/check `a)

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

;; Similar cases which should fail:

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

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





[CLJ-1551] Consider transducer support for primitives Created: 07/Oct/14  Updated: 28/Jun/17

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

Type: Feature Priority: Major
Reporter: Alex Miller Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: transducers

Approval: Vetted

 Description   

Need to consider how we can support primitives for transducers. In particular it may be that IFn needs overloading for L/D in addition to O.






[CLJ-2111] Clarify s/every docstring for :kind Created: 14/Feb/17  Updated: 28/Jun/17

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

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

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

 Description   

Docstring for `s/every` in the `:kind` option says "a pred/spec that the collection type must satisfy, e.g. vector?" but using a spec for `:kind` will fail:

user=> (s/valid? (s/every number? :kind (s/or :vector vector? :list list?))
          [])
ClassCastException clojure.spec$or_spec_impl$reify__13891 cannot be cast to clojure.lang.IFn  dev/eval44499/fn--44501 (form-init3178965928127409998.clj:22)

user=> (pst *e)
ClassCastException clojure.spec$or_spec_impl$reify__13903 cannot be cast to clojure.lang.IFn
	user/eval20/fn--22 (NO_SOURCE_FILE:13)
	clojure.spec/every-impl/reify--14039 (spec.clj:1225)
	clojure.spec/valid? (spec.clj:744)
	clojure.spec/valid? (spec.clj:740)

Proposed: The intent here was just to support a predicate function. Change docstring to say just "pred" rather than "pred/spec".

Patch: clj-2111.patch



 Comments   
Comment by Alex Miller [ 14/Feb/17 5:06 PM ]

Certainly a function like this works (s/every number? :kind #(or (vector? %) (list? %))). The question is whether the s/every doc that states "pred/spec" means only a predicate function or "predicate function OR spec". I'm not sure what the intention was. Certainly the code in every seems to be wrapping the kind into a function and then invoking it in every-impl, so it's not written to accept a spec currently.

Comment by Alex Miller [ 14/Feb/17 5:06 PM ]

Marking vetted to either resolve, update docstring, or decline. Need more info from Rich.





[CLJ-1743] Avoid compile-time static initialization of classes when using inheritance Created: 02/Jun/15  Updated: 27/Jun/17

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

Type: Enhancement Priority: Critical
Reporter: Abe Fettig Assignee: Unassigned
Resolution: Unresolved Votes: 5
Labels: aot, compiler, interop

Attachments: Text File 0001-Avoid-compile-time-class-initialization-when-using-g.patch     Text File clj-1743-2.patch     Text File clj-1743-3.patch    
Patch: Code
Approval: Triaged

 Description   

I'm working on a project using Clojure and RoboVM. We use AOT compilation to compile Clojure to JVM classes, and then use RoboVM to compile the JVM classes to native code. In our Clojure code, we call Java APIs provided by RoboVM, which wrap the native iOS APIs.

But we've found an issue with inheritance and class-level static initialization code. Many iOS APIs require inheriting from a base object and then overriding certain methods. Currently, Clojure runs a superclass's static initialization code at compile time, whether using ":gen-class" or "proxy" to create the subclass. However, RoboVM's base "ObjCObject" class [1], which most iOS-specific classes inherit from, requires the iOS runtime to initialize, and throws an error at compile time since the code isn't running on a device.

CLJ-1315 addressed a similar issue by modifying "import" to load classes without running static initialization code. I've written my own patch which extends this behavior to work in ":gen-class" and "proxy" as well. The unit tests pass, and we're using this code successfully in our iOS app.

Patch: clj-1743-2.patch

Here's some sample code that can be used to demonstrate the current behavior (Full demo project at https://github.com/figly/clojure-static-initialization):

Demo.java
package clojure_static_initialization;

public class Demo {
  static {
    System.out.println("Running static initializers!");
  }
  public Demo () {
  }
}
gen_class_demo.clj
(ns clojure-static-initialization.gen-class-demo
  (:gen-class :extends clojure_static_initialization.Demo))
proxy_demo.clj
(ns clojure-static-initialization.proxy-demo)

(defn make-proxy []
  (proxy [clojure_static_initialization.Demo] []))

[1] https://github.com/robovm/robovm/blob/master/objc/src/main/java/org/robovm/objc/ObjCObject.java



 Comments   
Comment by Alex Miller [ 18/Jun/15 3:01 PM ]

No changes from previous, just updated to apply to master as of 1.7.0-RC2.

Comment by Alex Miller [ 18/Jun/15 3:03 PM ]

If you had a sketch to test this with proxy and gen-class, that would be helpful.

Comment by Abe Fettig [ 22/Jun/15 8:31 AM ]

Sure, what form would you like for the sketch code? A small standalone project? Unit tests?

Comment by Alex Miller [ 22/Jun/15 8:40 AM ]

Just a few lines of Java (a class with static initializer that printed) and Clojure code (for gen-class and proxy extending it) here in the test description that could be used to demonstrate the problem. Should not have any dependency on iOS or other external dependencies.

Comment by Abe Fettig [ 01/Jul/15 8:49 PM ]

Sample code added, let me know if I can add anything else!

Comment by Abe Fettig [ 27/Jul/15 2:21 PM ]

Just out of curiosity, what are the odds this could make it into 1.8?

Comment by Alex Miller [ 27/Jul/15 6:06 PM ]

unknown.

Comment by Didier A. [ 20/Nov/15 7:11 PM ]

I'm affected by this bug too. A function in a namespace calls a static Java variable which is initialized in place. Another namespace which is genclassed calls that function. Now at compile time, the static java is initialized and it makes building fail, because that static java initialization needs resources which don't exist on the build machine.

Comment by Michael Blume [ 13/Mar/17 10:00 PM ]

Refreshing patch so it applies to master, no changes, keeping attribution.

Comment by Alex Miller [ 27/Jun/17 5:22 PM ]

I am confused by the patch making changes in RT.loadClassForName() but the changes in Compiler are calls to RT.classForNameNonLoading()? Is this patch drift or what's up?





[CLJ-2190] clojure.spec.alpha/exercise-fn should emit a bettor error message when no implementation is found for a symbol Created: 27/Jun/17  Updated: 27/Jun/17

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

Type: Enhancement Priority: Minor
Reporter: Abhirag Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: errormsgs, spec
Environment:

Clojure 1.9.0-alpha17
test.check 0.10.0-alpha2


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

 Description   

Here we get a NullPointerException because although we do have a spec for my-reverse, we don't have an implementation for it, a more descriptive error message would help.

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

(s/fdef foo :args (s/cat :x int?) :ret int?)
=> user/foo

(s/exercise-fn `foo)
NullPointerException   clojure.core/apply (core.clj:657)

Proposed: Check for a nil function and throw.

Patch: clj-2190.patch






[CLJ-2191] Docstring clarifications for clojure.spec.test.alpha/check Created: 27/Jun/17  Updated: 27/Jun/17  Resolved: 27/Jun/17

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

Type: Enhancement Priority: Trivial
Reporter: Michael Nygard Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: docstring


 Description   

The docstring for `check` refers to `::stc/opts` but that alias is only visible in the source code for clojure.spec.test.alpha. I recommend expanding the alias in the docstring so it says `:clojure.spec.test.check/opts`.

Likewise for the reference to `::stc/ret` in the same docstring.



 Comments   
Comment by Alex Miller [ 27/Jun/17 11:10 AM ]

The check docstring says "The opts map includes the following optional keys, where stc
aliases clojure.spec.test.check:"

Comment by Alex Miller [ 27/Jun/17 11:17 AM ]

Closing as unnecessary per separate conversation.

Comment by Michael Nygard [ 27/Jun/17 11:17 AM ]

Yes, it definitely does say that. I didn't read closely enough. Feel free to close.





[CLJ-2189] gen-class does not preserve parameter names of overridden methods Created: 25/Jun/17  Updated: 25/Jun/17  Resolved: 25/Jun/17

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

Type: Enhancement Priority: Minor
Reporter: Maxim Neverov Assignee: Unassigned
Resolution: Not Reproducible Votes: 0
Labels: interop

Attachments: PNG File ScreenShot.png    
Approval: Triaged

 Description   

Parameters names are not preserved during classes generation. It relates to interfaces and abstract classes clojure class inherited from as well as the methods declared in :methods part of the gen-class.

It would be useful to preserve names so that java programmers that use clojure libraries wouldn't be confused.

Steps to reproduce:
1. Declare java interface or abstract class with methods to implement in clojure.
2. Use gen-class to generate jar:

(ns foo.bar.core
  (:gen-class
    :name foo.bar.ClientImpl
    :extends foo.bar.AbstractClient
    :main false
    :methods [[method [String] void]]))

3. Add resulted jar in other java project.
4. Methods parameters looks like s1, aLong1 etc (like it shown in the attached screen shot).

Complete example is here



 Comments   
Comment by Maxim Neverov [ 25/Jun/17 1:46 PM ]

:uberjar did the trick. Ticket can be closed. Sorry for inconvenience.





[CLJ-2166] instrument exception doesn't contain function name in ex-data Created: 24/May/17  Updated: 24/Jun/17

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

Type: Enhancement Priority: Minor
Reporter: James Reeves Assignee: Unassigned
Resolution: Unresolved Votes: 4
Labels: spec

Attachments: Text File clj2166.patch    
Patch: Code
Approval: Screened

 Description   

When an instrumented function fails, it throws an IExceptionInfo. The ex-data of this exception contains the arguments that failed, but not the function that was called.

(require '[clojure.spec.alpha :as s] '[clojure.spec.test.alpha :as stest])
(defn foo [x] (+ x 1))
(s/fdef foo :args (s/cat :x number?))
(stest/instrument `foo)
(foo "x")  ;; ExceptionInfo Call to #'user/foo did not conform to spec:
(ex-data *e)

{:clojure.spec.alpha/problems
 [{:path [:args :x], :pred number?, :val "x", :via [], :in [0]}],
 :clojure.spec.alpha/args ("x"),
 :clojure.spec.alpha/failure :instrument,
 :clojure.spec.test.alpha/caller
 {:file "form-init1493151512736136730.clj",
  :line 101,
  :var-scope user/eval23284}}

*Proposed: Add an extra key clojure.spec.alpha/fn that has the symbol of the var under instrumentation.

After:

user> (ex-data *e)
=>
{:clojure.spec.alpha/problems [{:path [:args :x],
                                :pred clojure.core/number?,
                                :val "x",
                                :via [],
                                :in [0]}],
 :clojure.spec.alpha/spec #object[clojure.spec.alpha$regex_spec_impl$reify__1210
                                  0x4db4a004
                                  "clojure.spec.alpha$regex_spec_impl$reify__1210@4db4a004"],
 :clojure.spec.alpha/value ("x"),
 :clojure.spec.alpha/fn user/foo,
 :clojure.spec.alpha/args ("x"),
 :clojure.spec.alpha/failure :instrument,
 :clojure.spec.test.alpha/caller {:file "form-init2105027941758372775.clj",
                                  :line 1,
                                  :var-scope user/eval1050}}

Patch: clj2166.patch

Screened by: Alex Miller



 Comments   
Comment by Josh Jones [ 23/Jun/17 3:58 PM ]

I've done a small patch to add this, but is there a consensus on whether the value for the newly-added key should be the var itself (referenced by a :clojure.spec.alpha/var key), or the symbol (referenced by a :clojure.spec.alpha/fn key)?

Comment by Alex Miller [ 23/Jun/17 4:29 PM ]

Symbol

Comment by Josh Jones [ 23/Jun/17 6:22 PM ]

I've attached a one-liner patch, and welcome any comments or issues.

Note that although I signed the CA today (Rich also signed) and can thus officially submit, I am not yet on "Signed Contributor Agreement" list at https://clojure.org/community/contributors . I can't edit posts on Jira yet, which I why I did not edit to mark that a patch is present.

I've read the contrib guide but this is my first clojure patch, so please bear with me if I have made any errors in the workflow or otherwise here. Thanks!

Using the example in the original ticket, the new output of ex-data is:

user> (ex-data *e)
=>
{:clojure.spec.alpha/problems [{:path [:args :x],
                                :pred clojure.core/number?,
                                :val "x",
                                :via [],
                                :in [0]}],
 :clojure.spec.alpha/spec #object[clojure.spec.alpha$regex_spec_impl$reify__1210
                                  0x4db4a004
                                  "clojure.spec.alpha$regex_spec_impl$reify__1210@4db4a004"],
 :clojure.spec.alpha/value ("x"),
 :clojure.spec.alpha/fn user/foo,
 :clojure.spec.alpha/args ("x"),
 :clojure.spec.alpha/failure :instrument,
 :clojure.spec.test.alpha/caller {:file "form-init2105027941758372775.clj",
                                  :line 1,
                                  :var-scope user/eval1050}}
Comment by Alex Miller [ 24/Jun/17 7:29 AM ]

Thanks, looks good. I've also added the groups for you to have edit rights.





[CLJ-2188] clojure.core/slurp does not mark its return type as String Created: 23/Jun/17  Updated: 24/Jun/17

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

Type: Enhancement Priority: Minor
Reporter: Michael Blume Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None

Attachments: Text File CLJ-2188-v1.patch    
Patch: Code
Approval: Prescreened

 Description   

Given that slurp always returns a string (or throws), a user might expect that calling a string method on it will not reflect. However, its return type is not hinted, so calling, say, (.getBytes (slurp "foo")) will reflect unless the user hints. If we hint in clojure.core, the user won't have to.

user=> (set! *warn-on-reflection* true)
true
user=> (.length (slurp "pom.xml"))
Reflection warning, NO_SOURCE_PATH:3:1 - reference to field length can't be resolved.
9218

Patch: CLJ-2188-v1.patch

Prescreened by: Alex Miller






[CLJ-2186] ::s/map-bindings definition is underspecified Created: 22/Jun/17  Updated: 23/Jun/17

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

Type: Defect Priority: Major
Reporter: Allen Rohner Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: core.specs, spec

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

 Description   

:clojure.core.specs.alpha/map-bindings` is less strict than expected. It specifies `:into {}`, but not `:kind map?` which means non-maps conform.

(s/conform ::clojure.core.specs.alpha/map-binding-form '[[foo bar]])
=>
[[foo bar]]

This is also an issue because of the next line:

(s/def ::map-binding-form (s/merge ::map-bindings ::map-special-binding))

s/merge takes two maps, and ::map-bindings is not required to be a map.

Patch: clj-2186.patch

Screened by: Alex Miller (apply to core.specs.alpha)



 Comments   
Comment by Sameer Rahmani [ 23/Jun/17 11:01 AM ]

On clojure 1.9.0-alpha17 :

user=> (s/conform ::clojure.core.specs.alpha/map-binding-form '[[foo bar]])
:clojure.spec.alpha/invalid




[CLJ-2187] if-seq and if-pred for more flexible conditional assignment Created: 23/Jun/17  Updated: 23/Jun/17  Resolved: 23/Jun/17

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

Type: Feature Priority: Major
Reporter: Michael Zavarella Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None


 Description   

The current `if` macros are very useful but are limited in their use case. I wrote a couple of macros that I've found useful since writing them and I think other would too.

if-seq has nice to have for when you have a collection and want 'else' to evaluate if the collection is false-y or empty.

if-pred has been nice as a 'go-to' and is one of my most used functions/macros.

(defmacro if-seq
  "bindings => binding-form test

  If test is a seq, evaluates then with binding-form bound to the value of
  test, if not, yields else"
  ([bindings then]
   `(if-seq ~bindings ~then nil))
  ([bindings then else & oldform]
   (assert-args
    (vector? bindings) "a vector for its binding"
    (nil? oldform) "1 or 2 forms after binding vector"
    (= 2 (count bindings)) "exactly 2 forms in binding vector")
   (let [form (bindings 0) tst (bindings 1)]
     `(let [temp# ~tst]
        (if (seq temp#)
          (let [~form temp#]
            ~then)
          ~else)))))

(defmacro if-pred
  ([pred bindings then]
   `(if-pred ~pred ~bindings ~then nil))
  ([pred bindings then else & oldform]
   (assert-args
    (vector? bindings) "a vector for its binding"
    (nil? oldform) "1 or 2 forms after binding vector"
    (= 2 (count bindings)) "exactly 2 forms in binding vector")
   (let [form (bindings 0) tst (bindings 1)]
     `(let [temp# ~tst]
        (if (~pred temp#)
          (let [~form temp#]
            ~then)
          ~else)))))


 Comments   
Comment by Alex Miller [ 23/Jun/17 9:00 AM ]

Hi Michael, thanks for the suggestion. I don't think we'll add these to core, but seems like a good addition to a utility library.





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

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

Approval: Triaged

 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-2185] Standardize the running context of all Java to Clojure entry points. Created: 22/Jun/17  Updated: 22/Jun/17

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

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

Approval: Triaged

 Description   

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

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

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

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

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

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

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

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

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

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






[CLJ-2163] Add test for var serialization Created: 19/May/17  Updated: 20/Jun/17

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

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

Attachments: Text File var-test.patch    
Patch: Code and Test
Approval: Screened

 Description   

Add some tests for var serialization.

Screened by: Chouser






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

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

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

Approval: Vetted

 Description   

Generator overrides for spec aliases are not respected.

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

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

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

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


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

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

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

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

(require '[clojure.spec :as s])
(s/def ::original number?)
(s/def ::alias ::original)
(@#'clojure.spec/spec-name (s/get-spec ::alias))
;; => :user/original
Comment by Charles Despointes [ 20/Jun/17 1:19 PM ]

I've a somewhat similar issue. I think it is related.
I'm trying to do something like :

(require '[clojure.spec.alpha :as s])
(require '[clojure.spec.gen.alpha :as gen])
(s/def ::bar any?)
(s/def ::foo (s/with-gen any? (fn [] (s/gen ::bar))))
(gen/generate (s/gen ::foo {::bar (fn [] (s/gen int?))}))

I'm somewhat expecting it generates me an integer like it would have with a direct aliasing to ::bar in ::foo definition. But it doesn't and keep the with-gen binded generator.
Is that the same issue or is that an expected behaviour or should i fill a new issue ?





[CLJ-2184] propagate metadata in doto forms Created: 20/Jun/17  Updated: 20/Jun/17

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

Type: Enhancement Priority: Minor
Reporter: Chouser Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: errormsgs

Attachments: Text File CLJ-2184-patch-00.patch    
Patch: Code
Approval: Prescreened

 Description   

The doto macro currently produces lists without metadata, which among other things means errors and warnings are misleading or less specific than they could be. For example, this code generates reflection warnings, but the warnings point to the beginning of the doto block itself:

(set! *warn-on-reflection* true)

(def a "a")
(def b (int \b))

(doto "abc"
  (.indexOf a)
  (.indexOf b))

Note the line and column numbers in these warnings:

Reflection warning, t1.clj:6:1 - call to method indexOf on java.lang.String can't be resolved (argument types: unknown).
Reflection warning, t1.clj:6:1 - call to method indexOf on java.lang.String can't be resolved (argument types: unknown).

A more specific and accurate output would look like this:

Reflection warning, t1.clj:7:3 - call to method indexOf on java.lang.String can't be resolved (argument types: unknown).
Reflection warning, t1.clj:8:3 - call to method indexOf on java.lang.String can't be resolved (argument types: unknown).

Similar macros like -> take pains to propagate metadata from the users forms to the macro's output. This is more important for those macros because of how they interact with type hints.
Since the return values of the interior doto forms are ignored, the metadata is somewhat less important, but the example above shows the the propagation would still be valuable.

Patch: CLJ-2184-patch-00.patch

Prescreened by: Alex Miller



 Comments   
Comment by Alex Miller [ 20/Jun/17 11:20 AM ]

Patches welcome....

Comment by Chouser [ 20/Jun/17 11:24 AM ]

Attached "CLJ-2184-patch-00.patch" which uses a single with-meta call in doto to copy the metadata from each user-supplied form to the output form.





[CLJ-941] NullPointerException possible with seq-zip Created: 26/Feb/12  Updated: 19/Jun/17

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

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

Approval: Triaged

 Description   

For example:

Clojure 1.3.0
user=> (require '[clojure.zip :as z])
nil
user=> (-> (z/seq-zip (list 1)) z/down z/remove)
NullPointerException clojure.core/with-meta (core.clj:211)

Possibly the make-node function for seq-zip should be:

(fn [node children] (with-meta (or children ()) (meta node)))



 Comments   
Comment by Greg Chapman [ 26/Feb/12 5:54 PM ]

Also the docstring for zipper should probably be updated to indicate that the children parameter can be nil.

Comment by Christopher Brown [ 19/Jun/17 8:43 PM ]

4+ years later, I also ran into this.

Is there a workaround? I.e., is there a way to remove elements from singleton branches?

Comment by Alex Miller [ 19/Jun/17 9:35 PM ]

Would be happy to see a patch (although it also needs to backfill some tests too).

One workaround would be something like:

(-> (z/seq-zip (list 1)) (z/edit #(drop 1 %)) z/node)

Or you could use another zipper library like hara.zip which doesn't have remove, but works with delete-right:

(-> (z/seq-zip (list 1)) z/down z/delete-right z/up z/node)




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

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

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

any


Approval: Triaged

 Description   

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

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





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

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

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

Approval: Triaged

 Description   

Description

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

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

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

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

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

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

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

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

Proposal

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

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

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






[CLJ-1903] Provide a transducer for reductions Created: 17/Mar/16  Updated: 07/Jun/17

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

Type: Feature Priority: Major
Reporter: Pierre-Yves Ritschard Assignee: Unassigned
Resolution: Unresolved Votes: 4
Labels: transducers

Attachments: Text File 0001-clojure.core-add-reductions-stateful-transducer.patch     Text File 0002-clojure.core-add-reductions-with-for-init-passing-va.patch    
Approval: Triaged

 Description   

Reductions does not currently provide a transducer when called with a 1-arity.

Proposed:

  • A reductions transducer
  • Similar to seequence reductions, initial state is not included in reductions
(assert (= (sequence (reductions +) nil) []))
(assert (= (sequence (reductions +) [1 2 3 4 5]) [1 3 6 10 15]))

A second patch proposes a variant which allows explicit initialization values: reductions-with

(assert (= (sequence (reductions-with + 0) [1 2 3 4 5]) [1 3 6 10 15])))

Patch: 0001-clojure.core-add-reductions-stateful-transducer.patch
Patch: 0002-clojure.core-add-reductions-with-for-init-passing-va.patch



 Comments   
Comment by Steve Miner [ 17/Mar/16 3:47 PM ]

The suggested patch gets the "init" value for the reductions by calling the function with no args. I would like a "reductions" transducer that took an explicit "init" rather than relying on a nullary (f).

If I remember correctly, Rich has expressed some regrets about supporting reduce without an init (ala Common Lisp). My understanding is that an explicit init is preferred for new Clojure code.

Unfortunately, an explicit init arg for the transducer would conflict with the standard "no-init" reductions [f coll]. In my own code, I've used the name "accumulations" for this transducer. Another possible name might be "reductions-with".

Comment by Pierre-Yves Ritschard [ 17/Mar/16 4:38 PM ]

Hi Steve,

I'd much prefer for init values to be explicit as well, unfortunately, short of testing the 2nd argument in the 2-arity variant - which would probably be even more confusing, there's no way to do that with plain "reductions".

I like the idea of providing a "reductions-with" variant that forced the init value and I'm happy to augment the patch with that if needed.

Comment by Pierre-Yves Ritschard [ 18/Mar/16 3:35 AM ]

@Steve Miner I added a variant with reductions-with.

Comment by Pierre-Yves Ritschard [ 24/May/16 6:40 AM ]

Is there anything I can help to move this forward?
@alexmiller any comments on the code itself?

Comment by Alex Miller [ 24/May/16 7:31 AM ]

Haven't had a chance to look at it yet, sorry.

Comment by Pierre-Yves Ritschard [ 24/May/16 7:36 AM ]

@alexmiller, if the upshot is getting clojure.spec, I'll take this taking a bit of time to review

Comment by Steve Miner [ 25/May/16 3:21 PM ]

For testing, I suggest you compare the output from the transducer version to the output from a simliar call to the sequence reductions. For example,

(is (= (reductions + 3 (range 20)) (sequence (reductions-with + 3) (range 20)))

I would like to see that equality hold. The 0002 patch doesn't handle the init the same way the current Clojure reductions does.

Comment by Pierre-Yves Ritschard [ 07/Sep/16 4:29 PM ]

@alexmiller I'm tempting one more nudge to at least get an idea on the patch and the reductions-with variant since 1.9 seems to be getting closer to a release.

Comment by Alex Miller [ 08/Sep/16 10:43 AM ]

Sorry, don't know that I'll get to it soon or that it will be considered for 1.9. I also don't know that it won't, just ... don't know.

Comment by Pierre-Yves Ritschard [ 08/Sep/16 10:48 AM ]

@alexmiller, Thanks for the prompt reply. I'm trying to make sure i'll be around when feedback comes to be able to act quickly on it. Cheers!

Comment by Pierre-Yves Ritschard [ 07/Jun/17 9:03 AM ]

@alexmiller, any additonal insight into wether this has a chance of going in or if I can adapt/perfect the code in any way?

Comment by Alex Miller [ 07/Jun/17 9:30 AM ]

I will try to prescreen it when I get a chance.





[CLJ-2179] s/inst-in and s/int-in generators should have uniform distribution not biased towards min value Created: 06/Jun/17  Updated: 06/Jun/17

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

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

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

 Description   

The s/inst-in and s/int-in generators are based on gen/large-integer* which grows from 0.

(require '[clojure.spec.alpha :as s] '[clojure.spec.gen.alpha :as gen])
(gen/sample (s/gen (s/int-in 0 100)))
;;=> (1 0 1 1 1 0 1 1 72 1)

(gen/sample (s/gen (s/inst-in #inst "2001-01-01" #inst "2001-12-31")))
;;=> (#inst "2001-01-01T00:00:00.000-00:00" #inst "2001-01-01T00:00:00.000-00:00" #inst "2001-01-01T00:00:00.001-00:00" #inst "2001-01-01T00:00:00.001-00:00" ...)

Proposed: Instead, s/inst-in should use a uniform distribution generator:

After on same:

(26 16 65 96 63 37 31 4 94 9)

(#inst "2001-03-03T04:51:43.702-00:00" 
 #inst "2001-07-25T07:13:03.224-00:00" 
 #inst "2001-03-31T18:28:41.625-00:00" 
 #inst "2001-04-17T19:33:14.176-00:00" 
 #inst "2001-01-14T07:03:08.521-00:00" 
 #inst "2001-06-06T09:52:03.421-00:00" ...)

Patch: clj-2179.patch






[CLJ-2175] Some collection specs have inconsistent forms of :pred in their explain data Created: 02/Jun/17  Updated: 05/Jun/17  Resolved: 05/Jun/17

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

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

Approval: Vetted

 Description   

Here are the results of my investigation (you can reproduce them by (s/explain-data <spec> <input>)):

# <spec> <input> <expected> :pred
1. (s/tuple integer?) :a clojure.core/vector? vector?
2. (s/tuple integer?) [] (clojure.core/fn [%] (clojure.core/= (clojure.core/count %) 1)) (clojure.core/= (clojure.core/count %) 1)
3. (s/keys :req [::x]) :a clojure.core/map? map?
4. (s/keys :req [::x]) {} (clojure.core/fn [%] (clojure.core/contains? % :user/x)) (ditto)
5. (s/coll-of integer?) :a clojure.core/coll? (ditto)
6. (s/? integer?) :a (clojure.spec.alpha/? clojure.core/integer?) (ditto)
7. (s/? integer?) [1 2] (clojure.spec.alpha/? clojure.core/integer?) (ditto)
8. (s/* integer?) :a (clojure.spec.alpha/* clojure.core/integer?) (ditto)
9. (s/+ integer?) :a (clojure.spec.alpha/+ clojure.core/integer?) (ditto)
10. (s/+ integer?) [] clojure.core/integer? (ditto)
11. (s/& integer? even?) [] clojure.core/integer? #function[clojure.core/integer?]
12. (s/& integer? even?) [0 2] (clojure.spec.alpha/& clojure.core/integer? clojure.core/even?) (clojure.spec.alpha/& #function[clojure.core/integer?] clojure.core/even?)
13. (s/cat :i integer?) [] clojure.core/integer? (ditto)
14. (s/cat :i integer?) [1 2] (clojure.spec.alpha/cat :i clojure.core/integer?) (ditto)
15. (s/alt :i integer? :s string?) [] (clojure.spec.alpha/alt :i clojure.core/integer? :s clojure.core/string?) (ditto)
16. (s/alt :i integer? :s string?) [1 2] (clojure.spec.alpha/alt :i clojure.core/integer? :s clojure.core/string?) (ditto)

I think there are 4 different types of problems to be resolved here (marked with ):

  • Problem 1 (1. & 3.): symbols representing fn name should be resolved (as in 5.)
  • Problem 2 (2.): compound expressions should be wrapped to fn forms (as in 4.)
  • Problem 3 (11. & 12.): functions shouldn't be put into :pred as they are


 Comments   
Comment by Alex Miller [ 02/Jun/17 8:22 AM ]

Thanks, this is great. It would be helpful to have an "expected" column as well that lists what the :pred should be.

On 10 and 13 (Problem 3) - I don't agree that these are a problem. If you look at the problem, it's a complaint about insufficient input and the pred is being used to indicate the kind of input that was expected.

Comment by Shogo Ohta [ 02/Jun/17 7:42 PM ]

It would be helpful to have an "expected" column as well that lists what the :pred should be.

Sure. I will.

On 10 and 13 (Problem 3) - I don't agree that these are a problem. If you look at the problem, it's a complaint about insufficient input and the pred is being used to indicate the kind of input that was expected.

Yes, I think that's the most controversial point. Both the options look reasonable to me. But I'm not sure the results of the case 10. and 13. are really consistent with that of 15. from your stance.

Comment by Alex Miller [ 05/Jun/17 7:43 AM ]

10 and 13 are not controversial - these are the expected results. I think it is consistent with 15 - in both cases something is expected and we are stating the most specific spec/pred that is expected.

Comment by Alex Miller [ 05/Jun/17 8:34 AM ]

I've logged:

Closing this now as a duplicate of those 3 isolated issues.

Comment by Shogo Ohta [ 05/Jun/17 9:13 AM ]

OK, thank you for your time!





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

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

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

clojure-1.9.0-alpha14 and clojure-1.8.0 tested.



 Description   

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

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

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



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

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

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

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

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

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

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

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

Comment by Daniel Balke [ 02/Jun/17 9:45 AM ]

I ran into this while processing byte arrays and expected it to work because (= (byte 1) 1).
To address your points, Alex:

I would argue it's unintentional because the error message only indicates that the argument is not an integer, which bytes and shorts by definition of clojure.core/integer? and clojure.core/int? clearly are.
nth does work while using the vector as a function does not.

As for the performance impact: It would only slow down if you use bytes or shorts as arguments, else it would short-circuit, so old code would be unchanged in raw performance.

If you still think this is too unimportant to fix, that's also fine, just wanted to hear your opinions on this.





[CLJ-2168] clojure.spec: :pred in explain for coll-of should have resolved symbols Created: 26/May/17  Updated: 01/Jun/17

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

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

0.1.123


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

 Description   

:pred should be resolved in explain problems like:

s/coll-of and s/every-kv should have resolved :pred functions if it's values aren't valid:

(::s/problems (s/explain-data (s/coll-of (fn [x] (pos? x))) [-1]))
({:path [], :pred (fn [x] (pos? x)), :val -1, :via [], :in [0]})

should be

(::s/problems (s/explain-data (s/coll-of (fn [x] (pos? x))) [-1]))
({:path [], :pred (clojure.core/fn [x] (clojure.core/pos? x)), :val -1, :via [], :in [0]})

Other examples:

;; same with every
(::s/problems (s/explain-data (s/every (fn [x] (pos? x))) [-1]))
({:path [], :pred (fn [x] (pos? x)), :val -1, :via [], :in [0]})

;; :distinct option pred is not resolved:
(::s/problems (s/explain-data (s/coll-of pos? :distinct true) [-1 -1]))
[{:path [], :pred distinct?, :val [-1 -1], :via [], :in []}]

map-of and every-kv do not have this issue. The :count, :min-count, :max-count, and :kind options do correctly produce resolved :preds.

Patch: clj-2168.patch



 Comments   
Comment by Shogo Ohta [ 31/May/17 2:19 AM ]

The same problem happens with s/every.

Comment by Shogo Ohta [ 01/Jun/17 9:02 PM ]

Oh, sorry. I meant s/every-kv, not s/every.

BTW, after looking into things around this, I found some other spec macros were putting inconsistent forms of :pred in their explain data.

For example,

(s/explain-data (s/tuple integer?) []) => (clojure.core/= (clojure.core/count %) 1)
(s/explain-data (s/& integer? even?) []) => #function[clojure.core/integer?] (not a symbol)

I'll file that as another ticket later.





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

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

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

All


Approval: Triaged

 Description   

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

Clojure 1.7.0

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

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

Clojure 1.9.0-master-SNAPSHOT

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

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

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

And in 1.9:

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

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

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



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

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

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

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

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





[CLJ-1544] AOT code cannot see on-the-fly compiled classes Created: 01/Oct/14  Updated: 01/Jun/17

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

Type: Enhancement Priority: Critical
Reporter: Allen Rohner Assignee: Unassigned
Resolution: Unresolved Votes: 15
Labels: aot

Attachments: Text File 0001-CLJ-1544-force-reloading-of-namespaces-during-AOT-co.patch     Text File 0001-CLJ-1544-force-reloading-of-namespaces-during-AOT-co-v2.patch     Text File 0001-CLJ-1544-force-reloading-of-namespaces-during-AOT-co-v3.patch     Text File 0001-CLJ-1641-disallow-circular-dependencies-even-if-the-.patch    
Patch: Code
Approval: Incomplete

 Description   

AOTed Clojure code cannot see on-the-fly Clojure classes (vars are fine), because of the rules of Java classloader delegation. The on-the-fly code is loaded by a DynamicClassLoader, which delegates up to the classpath loader for the AOTed code. This is a particular problem when AOT code wants to consume a protocol from on-the-fly source, because protocols necessitate a class-level (not just var-level) relationship.

A minimal reproducible case is described in the following comment: http://dev.clojure.org/jira/browse/CLJ-1544?focusedCommentId=36734&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-36734

Other examples of the bug:
https://github.com/arohner/clj-aot-repro
https://github.com/methylene/class-not-found

A real issue triggered by this bug: https://github.com/cemerick/austin/issues/23

Related ticket: CLJ-1641 contains descriptions and comments about some potentially unwanted consequences of applying proposed patch 0001-CLJ-1544-force-reloading-of-namespaces-during-AOT-co-v3.patch

Approach: The approach taken by the attached patch is to force reloading of namespaces during AOT compilation if no matching classfile is found in the compile-path or in the classpath

Patch: 0001-CLJ-1544-force-reloading-of-namespaces-during-AOT-co-v3.patch

Screened by: Alex Miller



 Comments   
Comment by Alex Miller [ 04/Dec/14 12:45 PM ]

Possibly related: CLJ-1457

Comment by Nicola Mometto [ 05/Dec/14 4:51 AM ]

Has anyone been able to reproduce this bug from a bare clojure repl? I have been trying to take lein out of the equation for an hour but I don't seem to be able to reproduce it – this makes me think that it's possible that this is a lein/classlojure/nrepl issue rather than a compiler/classloader bug

Comment by Nicola Mometto [ 06/Dec/14 4:20 PM ]

I was actually able to reproduce and understand this bug thanks to a minimal example reduced from a testcase for CLJ-1413.

>cat error.sh
#!/bin/sh

rm -rf target && mkdir target

java -cp src:clojure.jar clojure.main - <<EOF
(require 'myrecord)
(set! *compile-path* "target")
(compile 'core)
EOF

java -cp target:clojure.jar clojure.main -e "(use 'core)"

> cat src/core.clj
(in-ns 'core)
(clojure.core/require 'myrecord)
(clojure.core/import myrecord.somerecord)

>cat src/myrecord.clj
(in-ns 'myrecord)
(clojure.core/defrecord somerecord [])

> ./error.sh
Exception in thread "main" java.lang.ExceptionInInitializerError
	at java.lang.Class.forName0(Native Method)
	at java.lang.Class.forName(Class.java:344)
	at clojure.lang.RT.classForName(RT.java:2113)
	at clojure.lang.RT.classForName(RT.java:2122)
	at clojure.lang.RT.loadClassForName(RT.java:2141)
	at clojure.lang.RT.load(RT.java:430)
	at clojure.lang.RT.load(RT.java:411)
	at clojure.core$load$fn__5403.invoke(core.clj:5808)
	at clojure.core$load.doInvoke(core.clj:5807)
	at clojure.lang.RestFn.invoke(RestFn.java:408)
	at clojure.core$load_one.invoke(core.clj:5613)
	at clojure.core$load_lib$fn__5352.invoke(core.clj:5653)
	at clojure.core$load_lib.doInvoke(core.clj:5652)
	at clojure.lang.RestFn.applyTo(RestFn.java:142)
	at clojure.core$apply.invoke(core.clj:628)
	at clojure.core$load_libs.doInvoke(core.clj:5691)
	at clojure.lang.RestFn.applyTo(RestFn.java:137)
	at clojure.core$apply.invoke(core.clj:630)
	at clojure.core$use.doInvoke(core.clj:5785)
	at clojure.lang.RestFn.invoke(RestFn.java:408)
	at user$eval212.invoke(NO_SOURCE_FILE:1)
	at clojure.lang.Compiler.eval(Compiler.java:6767)
	at clojure.lang.Compiler.eval(Compiler.java:6730)
	at clojure.core$eval.invoke(core.clj:3076)
	at clojure.main$eval_opt.invoke(main.clj:288)
	at clojure.main$initialize.invoke(main.clj:307)
	at clojure.main$null_opt.invoke(main.clj:342)
	at clojure.main$main.doInvoke(main.clj:420)
	at clojure.lang.RestFn.invoke(RestFn.java:421)
	at clojure.lang.Var.invoke(Var.java:383)
	at clojure.lang.AFn.applyToHelper(AFn.java:156)
	at clojure.lang.Var.applyTo(Var.java:700)
	at clojure.main.main(main.java:37)
Caused by: java.io.FileNotFoundException: Could not locate myrecord__init.class or myrecord.clj on classpath.
	at clojure.lang.RT.load(RT.java:443)
	at clojure.lang.RT.load(RT.java:411)
	at clojure.core$load$fn__5403.invoke(core.clj:5808)
	at clojure.core$load.doInvoke(core.clj:5807)
	at clojure.lang.RestFn.invoke(RestFn.java:408)
	at clojure.core$load_one.invoke(core.clj:5613)
	at clojure.core$load_lib$fn__5352.invoke(core.clj:5653)
	at clojure.core$load_lib.doInvoke(core.clj:5652)
	at clojure.lang.RestFn.applyTo(RestFn.java:142)
	at clojure.core$apply.invoke(core.clj:628)
	at clojure.core$load_libs.doInvoke(core.clj:5691)
	at clojure.lang.RestFn.applyTo(RestFn.java:137)
	at clojure.core$apply.invoke(core.clj:628)
	at clojure.core$require.doInvoke(core.clj:5774)
	at clojure.lang.RestFn.invoke(RestFn.java:408)
	at core__init.load(Unknown Source)
	at core__init.<clinit>(Unknown Source)
	... 33 more

This bug also has also affected Austin: https://github.com/cemerick/austin/issues/23

Essentially this bug manifests itself when a namespace defining a protocol or a type/record has been JIT loaded and a namespace that needs the protocol/type/record class is being AOT compiled later. Since the namespace defining the class has already been loaded the class is never emitted on disk.

Comment by Nicola Mometto [ 06/Dec/14 6:51 PM ]

I've attached a tentative patch fixing the issue in the only way I found reasonable: forcing the reloading of namespaces during AOT compilation if the compiled classfile is not found in the compile-path or in the classpath

Comment by Nicola Mometto [ 06/Dec/14 7:30 PM ]

Updated patch forces reloading of the namespace even if a classfile exists in the compile-path but the source file is newer, mimicking the logic of clojure.lang.RT/load

Comment by Nicola Mometto [ 06/Dec/14 7:39 PM ]

Further testing demonstrated that this bug is not only scoped to deftypes/defprotocols but can manifest itself in the general case of a namespace "a" requiring a namespace "b" already loaded, and AOT compiling the namespace "a"

Comment by Tassilo Horn [ 08/Dec/14 4:46 AM ]

I'm also affected by this bug. Is there some workaround I can apply in the meantime, e.g., by dictating the order in which namespaces are going to be loaded/compiled in project.clj?

Comment by Nicola Mometto [ 15/Dec/14 10:58 AM ]

Tassilo, if you don't have control over whether or not a namespace that an AOT namespace depends on has already been loaded before compilation starts, requiring those namespaces with :reload-all should be enough to work around this issue

Comment by Tassilo Horn [ 15/Dec/14 11:36 AM ]

Nicola, thanks! But in the meantime I've switched to using clojure.java.api and omit AOT-compilation. That works just fine, too.

Comment by Michael Blume [ 15/Dec/14 5:05 PM ]

Tassilo, that's often a good solution, another is to use a shim clojure class

(ns myproject.main-shim (:gen-class))

(defn -main [& args]
  (require 'myproject.main)
  ((resolve 'myproject.main) args))

then your shim namespace is AOT-compiled but nothing else in your project is.

Comment by Tassilo Horn [ 16/Dec/14 1:07 AM ]

Thanks Michael, that's a very good suggestion. In fact, I've always used AOT only as a means to export some functions to Java-land. Basically, I did as you suggest but required the to-be-exported fn's namespace in the ns-form which then causes AOT-compilation of that namespace and its own deps recursively. So your approach seems to be as convenient from the Java side (no need to clojure.java.require `require` in order to require the namespace with the fn I wanna call ) while still omitting AOT. Awesome!

Comment by Nicola Mometto [ 06/Jan/15 6:07 PM ]

I'm marking this as incomplete to prevent further screening until the bug reported here: http://dev.clojure.org/jira/browse/CLJ-1620?focusedCommentId=37232&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-37232 is figured out

Comment by Nicola Mometto [ 07/Jan/15 4:43 AM ]

Fixed the patch, I'm re marking the tickets as Vetted as it was before.

Comment by Alex Miller [ 16/Jan/15 12:54 PM ]

This patch is being rolled back for 1.7.0-alpha6 pending further investigation into underlying problems and possible solutions.

Comment by Colin Fleming [ 19/Jan/15 4:41 AM ]

I'm not 100% sure, but this looks a lot like Cursive issue 369. It had a case that I could reproduce with JDK 7 but not JDK 8, has the same mysterious missing namespace class symptom, and involves mixed AOT/non-AOT namespaces. However it's happening at runtime, not at compile time, which doesn't seem consistent.

Comment by Alex Miller [ 19/Jan/15 7:29 AM ]

My error report above was incorrectly tied to this issue (see CLJ-1636). I will delete the comment.

Comment by Nicola Mometto [ 29/Jan/15 12:23 PM ]

Since ticket CLJ-1641 has been closed, I'll repost here a comment I posted in that ticket + the patch I proposed, arguing why I think the patch I proposed for this ticket should not have been reverted:

Zach, I agree that having different behaviour between AOT and JIT is wrong.

But I also don't agree that having clojure error out on circular dependencies should be considered a bug, I would argue that the way manifold used to implement the circular dependency between manifold.stream and manifold.stream.graph was a just a hack around lack of validation in require.

My proposal to fix this disparity between AOT and JIT is by making require/use check for circular dependencies before checking for already-loaded namespaces.

This way, both under JIT and AOT code like

(ns foo.a (:require foo.b))
(ns foo.b)
(require 'foo.a)

will fail with a circular depdenency error.

This is what the patch I just attached (0001-CLJ-1641disallow-circular-dependencies-even-if-the.patch) does.

Comment by Stuart Halloway [ 15/Aug/16 1:35 PM ]

This is actually a runtime problem, not a compilation problem.

AOTed Clojure code cannot see on-the-fly Clojure classes (vars are fine), because of the rules of Java classloader delegation. The on-the-fly code is loaded by a DynamicClassLoader, which delegates up to the classpath loader for the AOTed code. This is a particular problem when AOT code wants to consume a protocol from source, because protocols necessitate a class-level (not just var-level) relationship.

The person who runs a particular Clojure app can solve this problem by making sure their own consumption of AOT compilation is "infectious", i.e. if you want to AOT-compile library A which uses library B, then you need to AOT-compile library B as well.

I think that attempts to have the Clojure compiler magically implement the "infectious" rule above will cause more problems than they solve, and that we should close this ticket and provide good guidance for tools like lein and boot.

Comment by Michael Sperber [ 15/Aug/16 2:24 PM ]

This problem occurs within the compilation of a single library/project, so I don't think this can be solved by simple usages of Leiningen or Boot.

Comment by John Szakmeister [ 27/Oct/16 4:54 AM ]

I just spent quite a bit of time tracking down what I thought might be a variant of this problem. I've been trying to use Colin Fleming's new gradle-clojure plugin and was running into issues with the resulting shadow jar (the equivalent of an uberjar). At the time I believed it to be a problem in the gradle-clojure plugin, but it turned out to be a different issue. The shadow plugin was not preserving the last modified time on files extracted from dependencies, and it resulted in some source files looking newer than the class files. I suspect that Clojure was then recompiling the class, thinking it was out-of-date. This was nasty to track down and quite unexpected, but I can see the sense in the behavior now that I know what's going on. I'm not sure if anything should be done on the Clojure side, but it points to a problem with including the source and AOT files together--you really need to make sure the timestamps are kept intact and I can see that fact being easily overlooked. In this case, it was a plugin completely unrelated to Clojure that had to be fixed. I should also add that taking the infectious approach Stuart mentions is probably an issue in the Gradle and (possibly?) Maven environments, since there are separate plugins for packaging the uberjar.

FWIW, I have a fair amount of information in the ticket for gradle-clojure about the failure mode and the steps I went through to try and track down the problem: https://github.com/cursive-ide/gradle-clojure/issues/8. Also, I've put a pull request in on the shadow plugin to help keep the timestamps intact: https://github.com/johnrengelman/shadow/pull/260. There is also an issue in the shadow plugin describing the problem too: https://github.com/johnrengelman/shadow/issues/259.

Comment by Mike Rodriguez [ 14/Mar/17 6:15 PM ]

I'm confused by http://dev.clojure.org/jira/browse/CLJ-1544?focusedCommentId=36734&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-36734

Isn't the issue just that the "clj" source files also need to be included on the runtime classpath in order to be JITed when needed - since they were not AOTed?

i.e.

cp src/myrecord.clj target/

java -cp target:clojure.jar clojure.main -e "(use 'core)"

Why would the expectation be that "target" contains everything needed by the classpath when the AOT compilation was not done on everything? I don't get a failure when I add this step here.

The AOTed code looks fine. It just calls a `require` on the `myrecord` ns to JIT it. So I'm guessing I'm not seeing something else about this general problem statement.





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

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

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

Any


Approval: Triaged

 Description   

The messages in the exceptions thrown by the readers would be much more informative if they included readily available information. There are many instances of this, but to name a few specific instances (all from LispReader.java, though there in most cases there are corresponding problems in EdnReader.java):

  • If the RegexReader class hits an unexpected EOF, it reports "EOF while reading regex". It would be helpful if the message included the first few characters of the regex it was trying to read – available in sb – as a guide to the person trying to locate the problem.
  • The same logic applies to StringReader.
  • In NamespaceMapReader, the error thrown if the namespaced map is not in fact a map could include the namespace symbol.
  • Whenever an odd number of elements in a map is detected, the exception could at least report the number of elements that the bad map did include, something like: "Map literal cannot contain 7 forms. Map literals must contain an even number of forms." Even better would be the first few forms.
  • The "Metadata can only be applied to IMetas" exception in MetaReader is not nearly as helpful as it could be. At the very least it should report the class of the thing that is not an IMeta.
  • With an additional argument, readDelimitedList could report the kind of thing that it was reading in the event that it hit the EOF. Without the additional argument it still report that it hit an EOF while trying to read the first or 4th or 29 element of a collection.





[CLJ-2172] Error message for non integer index into vector could be improved. Created: 31/May/17  Updated: 31/May/17

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

Type: Enhancement Priority: Minor
Reporter: Russ Olsen Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: errormsgs

Approval: Triaged

 Description   

The exception generated in APersistentVector/invoke and also in APersistentVector/assoc when the index is not an integer would be better if it included the class of the object actually passed in and would be better still if it included the actual value.



 Comments   
Comment by Alex Miller [ 31/May/17 7:25 PM ]

Needs assessment of current paths through core that reach this (and whether the change in question would be better or worse when that happens).





[CLJ-2170] Several top-level forms have improperly-located docstrings Created: 29/May/17  Updated: 30/May/17

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

Type: Defect Priority: Minor
Reporter: Cameron Desautels Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring

Attachments: Text File fix-improper-docstrings.patch    
Patch: Code
Approval: Prescreened

 Description   

A number of top-level forms have docstrings which are improperly-located within the defining form (viz. defn / defn- / defmacro), and thus are discarded rather than attached as proper metadata. I believe I have fixed all (10) instances within the project with my patch.

The following code demonstrates the problem and the efficacy of the patch:

(def doc-syms
  "Symbols missing documentation because it's incorrectly located
  within the defn / defn- / defmacro form."
  [#'clojure.core/group-by-sig
   #'clojure.pprint/setf
   #'clojure.pprint/unzip-map
   #'clojure.pprint/tuple-map
   #'clojure.pprint/rtrim
   #'clojure.pprint/ltrim
   #'clojure.pprint/prefix-count
   #'clojure.pprint/prerr
   #'clojure.pprint/prlabel
   #'clojure.set/bubble-max-key])

;; before patch
(every? nil?    (map (comp :doc meta) doc-syms)) ; => true

;; after patch
(every? string? (map (comp :doc meta) doc-syms)) ; => true

Prescreened by: Alex Miller



 Comments   
Comment by Andy Fingerhut [ 29/May/17 10:26 PM ]

If your 'after patch' expression truly does evaluate to false, it doesn't give much confidence that your change fixed them all, does it? Shouldn't it evaluate to true? Perhaps it isn't true because of some behavior related to defn- and doc strings. I have not checked.

CLJ-1314 has a change like this for clojure.set/bubble-max-key only. If this change is committed, that issue could be closed, too.

Comment by Cameron Desautels [ 29/May/17 10:55 PM ]

Ha. Apologies. This is a typo. It evaluates to true, as I encourage anyone to test.

I'll amend it.

Comment by Cameron Desautels [ 29/May/17 11:01 PM ]

Hmm, perhaps I don't have permission to edit. The typo stands, for now.

To clarify for anyone reading: the issue description has a typo, not the patch.

Comment by Alex Miller [ 30/May/17 8:59 AM ]

Cameron Desautels I gave you edit rights.

Comment by Cameron Desautels [ 30/May/17 10:47 AM ]

Thank you. I have corrected the issue description.





[CLJ-2104] Typo in pprint docstring Created: 29/Jan/17  Updated: 29/May/17

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

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

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

 Description   

Typo in pprint ns docstring: "complete documentation on the the clojure web site on github"



 Comments   
Comment by Cameron Desautels [ 29/May/17 12:00 PM ]

While we're at it, might as well fix the orthography of "clojure" (=> "Clojure") and "github" (=> "GitHub").





[CLJ-2169] conj has out-of-date :arglists Created: 27/May/17  Updated: 27/May/17

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

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

Attachments: Text File 0001-CLJ-2169-add-nullary-unary-overloads-of-conj-to-its-.patch     Text File 0002-CLJ-2169-add-nullary-unary-overloads-of-conj-to-its-.patch    
Patch: Code
Approval: Triaged

 Description   

conj has had nullary and unary overloads since 1.7.0, but its :arglists still only list [coll x] and [coll x & xs].

Prescreened by: Alex Miller



 Comments   
Comment by Michał Marczyk [ 27/May/17 6:05 PM ]

It occurs to me that perhaps the docstring could be updated too to explain (conj).

The new 0002-… patch includes the :arglists change of the 0001-… patch and adds the sentence "(conj) returns []." to the docstring immediately after "(conj nil item) returns (item).".





[CLJ-2141] New qualified-* predicates can return true, nil, and false Created: 31/Mar/17  Updated: 26/May/17  Resolved: 26/May/17

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

Type: Defect Priority: Major
Reporter: Daniel Compton Assignee: Unassigned
Resolution: Completed Votes: 91
Labels: function

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

 Description   

As of Clojure 1.9-alpha15, the new qualifed-keyword?, qualified-symbol?, and qualified-ident? functions can return true, nil, or false depending on the input passed to them, e.g.:

=> (qualified-keyword? ::abc)
true
=> (qualified-keyword? :abc)
nil
=> (qualified-keyword? 'abc)
false

Returning nil rather than false for unqualified keywords has the following issues:

  • Many sources of Clojure documentation note that a trailing '?' in a Clojure function generally implies a true/false return so this violates a common expectation.
  • Returning true/false/nil complicates the ability of ClojureScript to optimize boolean returns. Theoretically, this might affect Clojure JIT optimization as well, turning a call taking the return value into polymorphic rather than bimorphic.
  • Returning true/false/nil can yield confusing results for functions that partition by value, like group-by

The prior implementation during development used `boolean` to coerce the return value to a null and that was removed for performance reasons (would require a call through another var).

Alternatives:

Perf benchmark (w/ criterium bench, Java 1.8, AOT, direct-linked)
Example: (bench (dotimes [_ 100] (qualified-keyword? :a)))

Impl :a :a/b note
(and (keyword? x) (namespace x) true) 96.749127 ns 96.412551 ns current impl (returns nil for :a)
(and (keyword? x) (some? (namespace x))) 99.014677 ns 96.149567 ns uses existing preds
(and (keyword? x) (not (nil? (namespace x)))) 97.512916 ns 95.008061 ns nil? inlines
(if (and (keyword? x) (namespace x)) true false) 98.617369 ns 97.866673 ns if-based
(if (keyword? x) (if (namespace x) true false) false) 99.000727 ns 99.474754 ns  

Note that these vary by less than 1 ns per invocation, so perf is not significantly different.

Approach: After discussion with Rich, have decided to use a boolean wrapper around the check: (boolean (and (keyword? x) (namespace x))).
Also, needed to move boolean implementation higher to colocate the qualified fns near the other fns.

Patch: clj-2141-3.patch



 Comments   
Comment by Didier A. [ 03/Apr/17 5:14 AM ]

Agreed, either make it truthy/falsey and drop the ?, or keep the convention, one of the fee ones that still does not have an exception to its rule.

Comment by Alex Miller [ 03/Apr/17 12:19 PM ]

Moving to vetted so we remember to look at this before 1.9 releases.

Comment by Nikita Prokopov [ 04/Apr/17 2:38 AM ]

Just wanted to add my 2cents:

  • Even if nobody ever promised explicitly that predicates strictly return true/false, it has become an established convention, people rely on it.
  • Performance benefits are debatable at best. The function itself might become slightly faster but the coercion to boolean is still happening at the call site (e.g. inside if/when condition).
  • I think problem is important enough that it must get at least a second look. If there’s no good reason to break the convention, it’ll be better for everybody if we keep following it.
  • Having predicates that return three possible values is, well, weird by all means.
  • Even worse is that they look exactly like the old predicates. There’s no way to tell. It adds nuance and complexity to otherwise pretty simple and straightforward part of Clojure

Sorry but I really think this was overlooked. I only wish best for Clojure ind its future. Please at least consider this.

Thanks!

Nikita.

Comment by Ertuğrul Çetin [ 04/Apr/17 6:25 AM ]

Totally agreed!

Comment by Murat Berk [ 05/Apr/17 9:38 AM ]

It should NOT violate a common expectation/convention.

Comment by Bozhidar Batsov [ 06/Apr/17 1:50 AM ]

Btw, this is also an opportunity to improve the docstrings of those functions. Apart from the fact that for the millionth time they're missing a final `.`, now we can add something like `otherwise it returns false` or something like this. I assume this was originally omitted due the possibility to return either nil or false.

Comment by Alex Miller [ 07/Apr/17 3:29 PM ]

Re the docstrings, no plans to update those. The "otherwise..." was omitted because it's omitted on all of the other predicate docstrings as well. That makes sense (if the common implication is that the return value is false).

Comment by Bozhidar Batsov [ 11/Apr/17 2:22 AM ]

Perhaps. On a more general note - I really think some docstring standard should be imposed at some point. I find it odd that some docstrings are terminated with `.`, some are not. Not to mention it's really hard to figure out in docstrings what's a reference to a parameter, what's a reference to some other var, etc. And inconsistent docstrings are hard to format/present - e.g. in Elisp and CL it's common to have the first line of the docstring as a separate sentence, so you could have a good one-line overview of the thing it describes. Anyways, that's completely off-topic. One day I might blog/speak about this.

Comment by Chris Oakman [ 26/May/17 1:12 PM ]

Just adding a note to say that I agree with Nikita Prokopov's comments above.

Comment by Alex Miller [ 26/May/17 3:28 PM ]

This change is included in 1.9.0-alpha17.





[CLJ-2128] spec error during macroexpand no longer throws compiler exception with location Created: 16/Mar/17  Updated: 26/May/17  Resolved: 26/May/17

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

Type: Defect Priority: Major
Reporter: Alex Miller Assignee: Unassigned
Resolution: Completed Votes: 1
Labels: error-reporting, regression, spec
Environment:

1.9.0-alpha12-1.9.0-alpha15


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

 Description   

This used to work but got out of sync with the commit https://github.com/clojure/clojure/commit/b3d3a5d6ff0a2f435bb6a5326da2b960038adad4, which changed the IllegalArgumentException to an ex-info, but didn't change the corresponding catch in the Compiler. That change is visible as of 1.9.0-alpha12.

Before alpha12:

...
Exception in thread "main" java.lang.IllegalArgumentException: Call to clojure.core/ns did not conform to spec:
In: [1] val: ((:required clojure.string)) fails at: [:args] predicate: (cat :docstring (? string?) :attr-map (? map?) :clauses :clojure.core.specs/ns-clauses),  Extra input
:clojure.spec/args  (th.core (:required clojure.string))
, compiling:(th/core.clj:1:1)
...

^^ note the "th/core.clj:1:1"

After alpha12:

...
Exception in thread "main" clojure.lang.ExceptionInfo: Call to clojure.core/ns did not conform to spec:
In: [1] val: ((:required clojure.string)) fails at: [:args] predicate: (cat :docstring (? string?) :attr-map (? map?) :clauses :clojure.core.specs/ns-clauses),  Extra input
:clojure.spec/args  (th.core (:required clojure.string))
 #:clojure.spec{:problems [{:path [:args], :reason "Extra input", :pred (cat :docstring (? string?) :attr-map (? map?) :clauses :clojure.core.specs/ns-clauses), :val ((:required clojure.string)), :via [], :in [1]}], :args (th.core (:required clojure.string))}, compiling:(/private/var/folders/7r/_1fj0f517rgcxwx79mn79mfc0000gn/T/form-init4120559363887828149.clj:1:125)
	at clojure.lang.Compiler.load(Compiler.java:7441)
...

Patch: clj-2128.patch






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

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

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

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

 Description   

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

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

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

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

Patch: clj-2057-2.patch



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

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

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

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

Comment by Alex Miller [ 10/May/17 12:50 PM ]

Updated patch to apply to spec.alpha





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

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

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

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

 Description   

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

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

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

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

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

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

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

Patch: clj-2059-2.patch

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


 Comments   
Comment by Alex Miller [ 10/May/17 12:46 PM ]

Updated patch to apply to spec.alpha





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

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

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

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

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

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

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

user=> (s/exercise-fn str)
Exception No :args spec found, can't generate  clojure.spec/exercise-fn (spec.clj:1811)

user=> (s/fdef clojure.core/str :ret string?)
user=> (s/exercise-fn str)
Exception No :args spec found, can't generate  clojure.spec/exercise-fn (spec.clj:1811)

Patch: clj-2061-3.patch



 Comments   
Comment by Alex Miller [ 10/May/17 1:01 PM ]

Updated patch to apply to spec.alpha





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

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

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

Attachments: Text File clj-2063-2.patch     Text File clj-2063-3.patch     Text File longest-explain.patch    
Patch: Code
Approval: Ok

 Description   

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

Before:

(require '[clojure.spec.alpha :as s])
(s/def ::ex (s/alt :a (s/* keyword?) 
                   :b (s/cat :b1 keyword?) 
                   :c (s/cat :c1 (s/alt :c2 keyword? :c3 int?))))
									 
(s/explain ::ex ["c"])

In: [0] val: "c" fails spec: :user/ex at: [:a] predicate: keyword?
In: [0] val: "c" fails spec: :user/ex at: [:b :b1] predicate: keyword?
In: [0] val: "c" fails spec: :user/ex at: [:c :c1 :c2] predicate: keyword?
In: [0] val: "c" fails spec: :user/ex at: [:c :c1 :c3] predicate: int?

Proposed: Sort the explain problems with longest path first.

After:

In: [0] val: "c" fails spec: :user/ex at: [:c :c1 :c2] predicate: keyword?
In: [0] val: "c" fails spec: :user/ex at: [:c :c1 :c3] predicate: int?
In: [0] val: "c" fails spec: :user/ex at: [:b :b1] predicate: keyword?
In: [0] val: "c" fails spec: :user/ex at: [:a] predicate: keyword?

Patch: clj-2063-3.patch



 Comments   
Comment by Alex Miller [ 10/May/17 12:58 PM ]

Updated patch to apply to spec.alpha

Comment by Alex Miller [ 26/May/17 8:56 AM ]

Minor code cleanup in clj-2063-3.patch to use unary minus.





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

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

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

Attachments: Text File clj-2076-2.patch     Text File clj-2076-3.patch     Text File clj-2076-4.patch     Text File clj-2076.patch    
Patch: Code and Test
Approval: Ok

 Description   

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

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

Expected: [1 2 "blah"]

Cause: every-impl unform* just returns x

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

Patch: clj-2076-4.patch



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

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

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

Added tests, ready to screen

Comment by Alex Miller [ 07/Apr/17 3:49 PM ]

Added patch -3, only updated to apply cleanly to master.

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

Updated patch to apply to spec.alpha instead, no other changes.





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

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

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

Attachments: Text File clj-2085-2.patch     Text File explain-data.patch    
Patch: Code
Approval: Ok

 Description   

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

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

Patch: clj-2085-2.patch



 Comments   
Comment by David Chelimsky [ 03/Apr/17 9:26 AM ]

We're using explain-data to generate non-dev-readable messages in a browser. Access to the root structure would be helpful in cases where understanding the output requires specific knowledge of Clojure's data structures e.g. when a spec for a collection of maps is used to validate a map, in which case the :val key is bound to a MapEntry.

Comment by Alex Miller [ 10/May/17 12:10 PM ]

Updated patch to apply to spec.alpha





[CLJ-2153] Documentation for int-in and int-in-range? does not mention that they're limited to fixed precision integers Created: 12/Apr/17  Updated: 26/May/17  Resolved: 26/May/17

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

Type: Enhancement Priority: Trivial
Reporter: Rovanion Luckey Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: docstring, spec

Attachments: Text File clj-2153-4.patch     Text File fixed-precision-doc-3.patch     Text File fixed-precision-doc.patch     Text File fixed-precision-doc.patch    
Patch: Code
Approval: Ok

 Description   

The documentation for clojure.spec/int-in and clojure.spec/int-in-range? does not explicitly mention that they only accept and produce fixed precision integers as opposed to all integer types including BigInt.

Patch: clj-2153-4.patch

Screened by: Alex Miller



 Comments   
Comment by Alex Miller [ 12/Apr/17 10:00 AM ]

To consider the patch, you need to sign the contributors agreement, which you can find here: https://clojure.org/community/contributing

Comment by Alex Miller [ 13/Apr/17 8:31 AM ]

Thanks for signing the CA. Please update the patch so it applies cleanly without whitespace warnings:

$ git apply ~/Downloads/fixed-precision-doc.patch
/Users/alex/Downloads/fixed-precision-doc.patch:32: trailing whitespace.
  "Return true if start <= val, val < end and val is a fixed
/Users/alex/Downloads/fixed-precision-doc.patch:40: trailing whitespace.
  "Returns a spec that validates fixed precision integers in the
warning: 2 lines add whitespace errors.

While this is admittedly not consistent within this file, I think it's best if the docstring lines are indented (with 2 spaces) after the first line, and that's what I'd like to see in the patch. Text changes are fine.

Comment by Rovanion Luckey [ 13/Apr/17 8:40 AM ]

Docstring now indented with two spaces after the first line.

Comment by Rovanion Luckey [ 13/Apr/17 8:42 AM ]

Now with even less trailing whitespaces.

Comment by Alex Miller [ 10/May/17 12:21 PM ]

Updated patch to apply to spec.alpha, no other changes, attribution retained.





[CLJ-2142] Namespace map syntax prevents duplicate key check Created: 02/Apr/17  Updated: 26/May/17  Resolved: 26/May/17

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

Type: Defect Priority: Major
Reporter: Gregg Reynolds Assignee: Unassigned
Resolution: Completed Votes: 2
Labels: reader

Attachments: Text File 0001-CLJ-2142-throw-on-duplicate-keys-in-namespaced-map-l.patch     Text File 0001-CLJ-2142-throw-on-duplicate-keys-in-namespaced-map-l-v2.patch     Text File clj-2142-3.patch    
Patch: Code and Test
Approval: Ok

 Description   
user> #::{:a 1 :a 2}
#:user{:a 2}

Cause: In the namespace map reader, a map is built by repeated assoc rather than via createWithCheck. Thus, assoc of same key replaces prior key rather than throwing an error.

Approach: Build an array and invoke RT.map(a), to echo same code path without namespace map literal syntax.

After:

user=> #::{:a 1 :a 2}
IllegalArgumentException Duplicate key: :user/a  clojure.lang.PersistentArrayMap.createWithCheck (PersistentArrayMap.java:71)

Patch: clj-2142-3.patch



 Comments   
Comment by Nicola Mometto [ 02/Apr/17 12:06 PM ]

updated patch also fixes EdnReader

Comment by Gregg Reynolds [ 02/Apr/17 1:23 PM ]

wow, that was fast, Nicola!





[CLJ-1860] 0.0 and -0.0 compare equal but have different hash values Created: 01/Dec/15  Updated: 26/May/17  Resolved: 26/May/17

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

Type: Defect Priority: Minor
Reporter: Patrick O'Brien Assignee: Unassigned
Resolution: Completed Votes: 2
Labels: math

Attachments: Text File clj-1860-2.patch     Text File clj-1860-make-equals-false-for-pos-neg-0.0-v1.patch     Text File CLJ-1860-negative-zero-hash-eq-fix.patch    
Patch: Code and Test
Approval: Ok

 Description   

0.0 and -0.0 compare as equal but have different hash values:

user=> (= 0.0 -0.0)
true
user=> (hash -0.0)
-2147483648
user=> (hash 0.0)
0

This causes problems as the equality/hashing assumption is violated.

user=> #{[1 2 0.0] [1 2 -0.0]}
#{[1 2 -0.0] [1 2 0.0]}

user=> (hash-map 0.0 1 -0.0 2)
{0.0 2}

user=> (hash-map [0.0] 1 [-0.0] 2)
{[0.0] 1, [-0.0] 2}

user=> (array-map [0.0] 1 [-0.0] 2)
{[0.0] 2}

user=> (hash-set [0.0] [-0.0])
#{[0.0] [-0.0]}

Cause: The source of this is due to some differences in Java. Java primitive double 0.0 and -0.0 == but the boxed Double is NOT .equals(). See also: http://docs.oracle.com/javase/7/docs/api/java/lang/Double.html#equals%28java.lang.Object%29

Double equality is checked with == in Clojure, which will report true. Hashing falls through to .hashCode(), which returns different values (but is consistent with the .equals() result on the boxed form).

Approach: While there are times when 0.0 and -0.0 being different are useful (see background below), most Clojure users expect these to compare equal. IEEE 754 says that they should compare as equals as well. So the approach to take here is to leave them as equal but to modify the hash for -0.0 to be the same as 0.0 so that `=` and `hash` are consistent. The attached patch takes this approach.

Patch: clj-1860-2.patch

Screened: Alex Miller

Alternative: Make 0.0 != -0.0. This approach affects a much larger set of code as comparison operators etc may be affected. The patch clj-1860-make-equals-false-for-pos-neg-0.0-v1.patch may be one way to implement this approach, and seems fairly small in the quantity of code affected (2 methods).

Background: https://en.wikipedia.org/wiki/Signed_zero



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

Just to summarize, it seems like this functionality in Clojure is the same as it is in Java:

0.0 == -0.0: true
new Double(0.0).hashCode(): 0
new Double(-0.0).hashCode(): -2147483648
new Double(-0.0).equals(new Double(0.0)): false

I can see pros and cons to both of the aforementioned approaches, as well as just leaving this one be. Does anyone else have any input on this one? Is this issue something we should rectify, or by changing it will we end up creating more problems than we solved?

Comment by Andy Fingerhut [ 10/Feb/16 12:38 AM ]

The Java behavior you demonstrate shows that in this case, they are not equals, so there is no need for the hashCode() values to be the same in order to satisfy the hash consistency property of equals and hashCode.

Clojure currently violates the hash consistency property that should ideally hold between clojure.core/= and clojure.core/hash, for 0.0 and -0.0.

Changing clojure.core/= so it is false would restore the hash consistency property for these values. Keeping (clojure.core/== 0.0 -0.0) true is hopefully something that will be maintained across any change, but that does not violate hash consistency, because that property has nothing to say about the value of clojure.core/==

Comment by Stephen Hopper [ 10/Feb/16 7:10 AM ]

Thanks for the explanation, Andy. That makes sense to me. I'll put together some tests and a possible solution for evaluation.

Comment by Stephen Hopper [ 10/Feb/16 11:20 AM ]

After diving into the source code a bit more, my preference is to modify the hash calculation for -0.0 to be the same as the hash calculation for 0.0. This will restore the hash consistency property without breaking other mathematical operations. Basically, if we update clojure.core/= to return false for (= 0.0 -0.0), we will need to update other functions (clojure.core/<, clojure.core/>, etc.) so that -0.0 and 0.0 still follow basic numerical properties surrounding equality and ordering. I'll add some tests and a possible patch to hash calculation for numbers for consideration.

Comment by Andy Fingerhut [ 10/Feb/16 12:40 PM ]

Someone, perhaps Patrick O'Brien , brought up a preference that making (= 0.0 -0.0) false would help in some applications, e.g. numerical applications involving normal vectors where it was beneficial if (= 0.0 -0.0) was false. I have no knowledge whether this preference will determine what change will be made to Clojure, if any. If someone finds a link to the email discussion that was in one of the Clojure or Clojure Dev Google groups, that would be a useful reference.

Comment by Stephen Hopper [ 11/Feb/16 9:47 PM ]

I've added a pair of patches for review on this one (one contains test updates and the other contains my proposed updates to the hash calculation). I realize that this is different than the approach proposed earlier in the ticket, but I think it should be the preferred approach. As I mentioned earlier, merely changing clojure.core/= to return false for (= 0.0 -0.0) would require also updating other numeric equality functions (clojure.core/<, clojure.core/>, etc.). More importantly, I feel that this behavior would be different than what most Clojure developers would expect.

For this reason, the patches I've updated merely modify the calculation of hashes with Numbers.java for Floats and Doubles for which isZero returns true to return the hashCode for positive 0.0 instead of negative 0.0.

Is this acceptable?

EDIT:
With these patches applied, we get the following behavior in the REPL:

user=> (= 0.0 -0.0)
true
user=> (hash 0.0)
0
user=> (hash -0.0)     
0
user=> (hash (float 0.0))
0
user=> (hash (float -0.0))
0
Comment by Andy Fingerhut [ 12/Feb/16 3:48 AM ]

Stephen, please see here http://dev.clojure.org/display/community/Developing+Patches for the commands used to create patches in the desired format.

Only a screener or Rich can say whether the patch is acceptable in the ways that matter for committing into Clojure.

Comment by Stephen Hopper [ 12/Feb/16 7:42 AM ]

I've corrected the format on my patch files and resubmitted them as one patch. Let me know if you see any issues with this. Also, it's worth pointing out that the first two patch files can be ignored / deleted.

Comment by Andy Fingerhut [ 14/Feb/16 1:30 PM ]

There are also instructions on that page for deleting old attachments, in the section titled "Removing patches", if you wished to do that.

A very minor comment, as the email address you use in your patches is completely up to you (as far as I know), but the one you have in your patch doesn't look like one that others could use to send you a message. If that was intentional on your part, no problem.

Comment by Stephen Hopper [ 14/Feb/16 1:48 PM ]

I've corrected the email address snafoo and removed the outdated patches. Thanks for walking me through this stuff, Andy.

Comment by Steve Miner [ 23/Feb/16 3:43 PM ]

The suggested fix is to make (hash -0.0) return the same as (hash 0.0). With the proposed patch, both will return 0. That happens to be the same as (hash 0). Not wrong at all, but maybe slightly less good than something else. Since you're making a change anyway, why not go the other way and use the -0.0 case as the common result?

I'm thinking that it would be a useful property if the hash of a long N is not equal to the hash of the corresponding (double N). In Clojure 1.8, zero is the only value I could quickly find where the long and the double equivalents have the same hash value.

The patch could be slightly tweaked to make (hash 0.0) and (hash -0.0)

return new Double(-0.0).hashCode()

The only change to the patch is to add the negative sign. The new hash result is -2147483648.

Admittedly this is an edge case, not a real performance issue. People probably don't mix longs and doubles in sets anyway. On the other hand, zeroes are kind of common. Since you're proposing a change, I thought it's worth considering a slight tweak.

Comment by Steve Miner [ 23/Feb/16 3:46 PM ]

Same for the float case, of course.

Comment by Alex Miller [ 24/Feb/16 1:36 PM ]

Looking at this agin, I'm ok with the approach and agree it's definitely a smaller change. Few additional changes needed in the patch and then I'll move it along:

  • Rather than `new Double(0.0).hashCode()` and `new Float(0.0).hashCode()`, move those values into `private static final int` constants and just return them.
  • In the comparison tests, throw -0.0M in there as well
  • Squash the patch into a single commit

Re Steve's suggestion, I do not think it's critical that long and double 0 hash differently and would prefer that double hashes match Java hashCode, so I would veto that suggestion.

Comment by Stephen Hopper [ 24/Feb/16 1:52 PM ]

Thank you, Alex. I'll make those updates.

Comment by Stephen Hopper [ 24/Feb/16 2:43 PM ]

Alex, I've attached the new patch file (CLJ-1860-negative-zero-hash-eq-fix.patch) to address the points from your previous post. Let me know if I missed anything.

Thanks!

Comment by Mike Anderson [ 25/Feb/16 7:44 PM ]

I may be late to the party since I have only just seen this, but I have a strong belief that 0.0 and -0.0 should be == but not =.

Reasons:

  • Anyone doing numerical comparison should use ==, so you want the result to be true
  • Anyone doing value comparison should use =, so you want the result to be false because these are different IEE784 double values. This includes set membership tests etc.

i.e. the Java code is doing it right, and we should be consistent with this.

Comment by Alex Miller [ 25/Feb/16 11:52 PM ]

I think there is consensus that == should be true, so we can set that aside and focus on =.

The reality is that there is no easy way to compare two collections with == (for example comparing [5.0 0.0 1.0] and [5.0 -0.0 1.0]). This is an actual use case that has been problematic for multiple people. While I grant there are use cases where 0.0 and -0.0 are usefully differentiable, I do not know of a real case in the community where this is the desired behavior, so I would rather err on the side of satisfying the intuition of the larger (and currently affected) population.

Also note that the Java code is doing it BOTH ways (primitive doubles are equal, boxed doubles are not), so I think that's a weak argument.

Comment by Alex Miller [ 26/Feb/16 8:44 AM ]

(after sleeping more on this...) It's possible that a better answer here is to expand what can be done with ==. I trust that when Rich looks at this ticket he will have his opinions which may or may not match up to mine and if so, we'll go in a different direction.

Comment by Andy Fingerhut [ 27/Feb/16 10:15 AM ]

Attachment clj-1860-make-equals-false-for-pos-neg-0.0-v1.patch dated Feb 27 2016 is a first cut at implementing a change where = returns false when comparing positive and negative 0.0, float or double.

As far as I can tell, there is no notion of positive and negative 0 for BigDecimal, so no change in behavior there.

Comment by Alex Miller [ 17/Nov/16 4:16 PM ]

Rich says: "0.0=-0.0, make hash the same"





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

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

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


 Description   

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



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

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





[CLJ-2164] case fails when a single single clause with an empty test seq is used Created: 22/May/17  Updated: 22/May/17

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

Type: Defect Priority: Minor
Reporter: Chris Blom Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None

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

 Description   

It is not possible to use case with a single empty seq of options, or with a single seq of options and a default clause.

I would expect

(case 1 () :a :none)

to return :none, instead it fails with an uninformative exception: "Unhandled clojure.lang.ArityException: Wrong number of args (-2) passed to: core/max"

I would expect (case 1 () :a) to fail with "java.lang.IllegalArgumentException: No matching clause", but instead it also fails with
"Unhandled clojure.lang.ArityException: Wrong number of args (-2) passed to: core/max"

This seems inconsistent, as passing an empty list of options is fine when there are other alternatives:

(case 1 () :a 2 :b :none)

returns :none, as expected

The attached patch removes the test-clause pairs with empty test lists before further conversion to case*, and adds tests.



 Comments   
Comment by Chris Blom [ 22/May/17 8:54 AM ]

oops, typo in the title (duplicated "single")

Comment by Alex Miller [ 22/May/17 8:55 AM ]

An empty match list in case seems like it should be an error - maybe this should fail to compile rather than ignoring?

Comment by Chris Blom [ 22/May/17 9:07 AM ]

An empty list of options is currently supported when multiple clauses are given, so failing to compile on empty lists would be a breaking change.

This works in 1.8:

(case 1
() :never-happens
1 :ok
:default)
=> :ok

But this does not:

(case 1
() :never-happens
:default)
=> throws clojure.lang.ArityException: Wrong number of args (-2) passed to: core/max

Only failing when no other clauses are given seems very inconsistent to me.

Comment by Nicola Mometto [ 22/May/17 9:39 AM ]

an empty list doesn't make any sense in case as the correct way to match a literal empty list is `(case () (()) :empty)`. I don't see any value in making it not throw and my vote is to have the `case` macro complain at compile time every time a `()` ise used

Comment by Alex Miller [ 22/May/17 9:57 AM ]

() would never match anything now, so failing on () would not break any existing case that matches.

The one case I can imagine might exist though is a macro that creates a case and could potentially programmatically create an empty case list? Something like this:

(defmacro make-case [xs] `(defn ~'foo [e#] (case e# ~xs "matched" "nope")))
Comment by Chris Blom [ 22/May/17 10:00 AM ]

I'm not using this to match empty lists, I ran into this corner case when generating case statements from a DSL.
While it is a pathological case, I disagree that is does not make any sense, an empty list here simply represents no alternatives,
so the clause wil never match and its result-expr will never run.

My point is that now (in clojure 1.8) this is allowed:

(case a
() :never-happens
1 :a
2 :b
:default)

it is equivalent to (as an empty list never matches)

(case a
1 :a
2 :b
:default)

But

(case a
() :never-happens
:default)

gives an uninformative error.

I argue that it should be equivalent to

(case a
:default)

as rejecting empty lists in case statements in general would be a breaking change,
and only rejecting empty lists when no other clauses are present is inconsistent.

Comment by Chris Blom [ 22/May/17 10:11 AM ]

() would never match anything now, so failing on () would not break any existing case that matches.

As () in case is allowed in clojure =<1.8 (just not when its the only clause), letting the compiler reject it would potentially break existing code.

The one case I can imagine might exist though is a macro that creates a case and could potentially programmatically create an empty case list?

That is exactly how i ran into this problem





[CLJ-1965] clojure.spec/def should support an optional doc-string Created: 19/Jun/16  Updated: 18/May/17

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

Type: Feature Priority: Minor
Reporter: Alexander Kiel Assignee: Unassigned
Resolution: Unresolved Votes: 44
Labels: spec

Approval: Vetted

 Description   

Like clojure.core/def clojure.spec/def should support an optional doc string because one usually likes to describe specs in more detail as one could through keyword naming.



 Comments   
Comment by Moritz Heidkamp [ 03/Nov/16 4:23 PM ]

Building on this idea, I suggest to add first-class metadata support to registered specs and implement doc strings in terms of that (i.e. the same way as with vars).

Comment by Josh Brandoff [ 01/Apr/17 5:02 PM ]

Hi! Was just discussing the potential for a feature like this with a colleague. What's the current status? Was thinking of potentially working on it but wanted to get feedback and guidance from the community first.

Comment by Alex Miller [ 03/Apr/17 9:32 AM ]

We don't have a recommended approach to this yet so not looking for a patch at this time.





[CLJ-1758] xf overload for vec and set Created: 17/Jun/15  Updated: 16/May/17

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

Type: Feature Priority: Major
Reporter: Leon Grapenthin Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None


 Description   

Having (vec xf coll) and (set xf coll) overloads seem useful as opposed to writing (into [] ...).

One might also consider these as variadic overloads, like the sequence function has. I am unsure about that since into doesn't have one and I know too little about multiple input transducers.



 Comments   
Comment by Leon Grapenthin [ 16/May/17 1:36 PM ]

After two more years with Clojure I don't see this as improvement anymore. (into [] ...) provides more expressiveness. Unless other parties are interest, I'd recommend to close this.

We still lack functionality to use the variadic map transducer with sets, but I have never needed it, so I wouldn't open a ticket for that myself.

Comment by Alex Miller [ 16/May/17 4:03 PM ]

I'm still interested.





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

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

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

Attachments: Text File clj-2075-add-three-arities-to-comparisons-3.patch    
Patch: Code
Approval: Triaged

 Description   

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

(< 0 temp 100)

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

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

(defn <
  "Returns non-nil if nums are in monotonically increasing order,
  otherwise false."
  {:inline (fn [x y] `(. clojure.lang.Numbers (lt ~x ~y)))
   :inline-arities #{2}
   :added "1.0"}
  ([x] true)
  ([x y] (. clojure.lang.Numbers (lt x y)))
  ([x y & more]
    (if (< x y)
     (if (next more)
       (recur y (first more) (next more))
       (< y (first more)))
     false)

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

(defn <
  "Returns non-nil if nums are in monotonically increasing order,
  otherwise false."
  {:inline (fn [x y] `(. clojure.lang.Numbers (lt ~x ~y)))
   :inline-arities #{2}
   :added "1.0"}
  ([x] true)
  ([x y] (. clojure.lang.Numbers (lt x y)))
  ([x y z] (and (. clojure.lang.Numbers (lt x y))
                (. clojure.lang.Numbers (lt y z))))
  ([x y z & more]
   (if (< x y)
     (let [nmore (next more)]
       (if nmore
         (recur y z (first more) nmore)
         (< y z (first more))))
     false)))

The performance gains are quite significant:

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

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

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

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

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

Results are good:

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

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

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



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

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

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

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

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

Use this patch if CLJ-1912 would be applied first

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

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

Comment by Alex Miller [ 15/May/17 3:31 PM ]

Dupe of CLJ-1912

Comment by Nikita Prokopov [ 15/May/17 3:43 PM ]

Alex Miller It is a duplicate, but my patch is waaaaaaaaay faster. Just look at the numbers (70-80% improvement vs 5-10%). It’s because I introduced a real arity so that intermediate collection is not created and is not destructured in case of 3 arguments.

Comment by Jozef Wagner [ 15/May/17 11:55 PM ]

There's a quite serious bug in the supplied patch(es), that causes e.g. (= 3 3 2) to return true. Because of this the benchmarks are flawed too I guess.

Comment by Nikita Prokopov [ 16/May/17 3:13 PM ]

Jozef Wagner thanks for spotting this! Attaching an updated path. Benchmark wasn’t flawed too much because perf gain comes not from doing one less/one more comparison but from not having an overhead of calling a fn with unknown arity.





[CLJ-1610] Unrolled small maps Created: 08/Dec/14  Updated: 15/May/17

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

Type: Feature Priority: Major
Reporter: Alex Miller Assignee: Zach Tellman
Resolution: Unresolved Votes: 10
Labels: collections


 Description   

Placeholder for unrolled small maps enhancement (companion for vectors at CLJ-1517).



 Comments   
Comment by Andy Fingerhut [ 09/Jul/15 10:59 PM ]

Is there an expectation that these would perform better that PersistentArrayMap?

Comment by Zach Tellman [ 09/Jul/15 11:53 PM ]

Yes, in some cases significantly so, for three reasons (in rough order of importance):

  • positional constructors, without any need for array instantiation/population
  • short-circuiting equality checks using hash comparisons
  • no iteration on any operation

There are a series of benchmarks at https://github.com/ztellman/cambrian-collections/blob/master/test/cambrian_collections/map_test.clj#L64-L148, which compare operations against maps with both keywords (which don't benefit from the hash comparisons) and symbols (which do). The 7-entry map cases cause the unrolled maps to overflow, so they only exist to test the overflow mechanism.

I've run the benchmark suite on my laptop, and the results are at https://gist.github.com/ztellman/961001e1a77e4f76ee1d. Some notable results:

The rest of the benchmarks are marginally faster due to unrolling, but most of the performance benefits are from the above behaviors. In a less synthetic benchmark, I found that Cheshire JSON decoding (which is 33% JSON lexing and 66% data structure building) was sped up roughly 30-40%.





[CLJ-2] Scopes Created: 15/Jun/09  Updated: 15/May/17

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

Type: Feature Priority: Major
Reporter: Assembla Importer Assignee: Rich Hickey
Resolution: Unresolved Votes: 2
Labels: None

Attachments: Text File scopes-spike.patch    

 Description   

Add the scope system for dealing with resource lifetime management



 Comments   
Comment by Assembla Importer [ 24/Aug/10 11:43 AM ]

Converted from http://www.assembla.com/spaces/clojure/tickets/2

Comment by Assembla Importer [ 24/Aug/10 11:43 AM ]

richhickey said: Updating tickets (#8, #42, #113, #2, #20, #94, #96, #104, #119, #124, #127, #149, #162)

Comment by Stuart Halloway [ 12/Jul/11 8:26 AM ]

Patch demonstrates idea, not ready for prime time.

Comment by Tassilo Horn [ 23/Dec/11 7:37 AM ]

I think the decision of having to specify either a Closeable resource or a close function for an existing non-Closeable resource in with-open is quite awkward, because they have completely different meaning.

  (let [foo (open-my-custom-resource "foo.bar")]
    (with-open [r (reader "foo.txt")
                foo #(.terminate foo)]
      (do-stuff r foo)))

I think a CloseableResource protocol that can be extended to custom types as implemented in the patch to CLJ-308 is somewhat easier to use. Extend it once, and then you can use open-my-custom-resource in with-open just like reader/writer and friends...

That said, Scopes can still be useful, but I'd vote for handling the "how should that resource be closed" question by a protocol. Then the with-open helper can simply add

(swap! *scope* conj (fn [] (clojure.core.protocols/close ~(bindings 0))))

and cleanup-scope only needs to apply each fn without having to distinguish Closeables from fns.





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

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

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

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

 Description   

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

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



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

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

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

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





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

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

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


 Description   

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



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

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

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

Related: CLJ-1527





[CLJ-366] Multiplatform command-line clojure launcher Created: 28/May/10  Updated: 15/May/17  Resolved: 15/May/17

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

Type: Enhancement Priority: Major
Reporter: Assembla Importer Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None


 Description   

Clojure needs a lower barrier of entry, long java commands scare people away! We need a script that conveniently launches a clojure repl or executes clojure files, much like the ruby/python/perl/other-favorite-interpreted-language behavior.

NOTES:

From Russ Olson (regarding Dejure/Dejour):

  • I just fixed a bunch of bugs in the script, so make sure you get the latest from download from: http://github.com/russolsen/dejour
  • After looking at jruby, scala, and groovy, it seems that the only way to do this right on windows is to write a C or C++ program and have a .exe.


 Comments   
Comment by Assembla Importer [ 24/Aug/10 8:21 AM ]

Converted from http://www.assembla.com/spaces/clojure/tickets/366

Comment by Assembla Importer [ 24/Aug/10 8:21 AM ]

stu said: Updating tickets (#370, #366, #374)

Comment by Aaron Bedra [ 10/Dec/10 10:13 AM ]

Design page is at http://dev.clojure.org/display/design/CLJ+Launcher and should be the basis for all future discussion

Comment by Kevin Downey [ 17/Apr/14 10:22 PM ]

is the priority on this, as the ticket says, Major?

the last commit on https://github.com/russolsen/dejour was 2 years ago.
the last edit to the wikipage was 3 years ago.

Comment by Alex Miller [ 15/May/17 4:46 PM ]

May do something like this in the future, but won't work it through here.





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

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

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


 Description   

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



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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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





[CLJ-1552] Consider kv support for transducers (similar to reducers fold) Created: 07/Oct/14  Updated: 15/May/17

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

Type: Feature Priority: Major
Reporter: Alex Miller Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: transducers


 Description   

In reducers, fold over a map has special support for kv. Consider whether/how to add this for transducers.



 Comments   
Comment by Marshall T. Vandegrift [ 16/Dec/14 11:13 AM ]

We don't have a JIRA "unvote" feature, but I'd like to register my vote against this proposed enhancement. As a heavy user of clojure.core.reducers, I consider the switch to k-v semantics when reducing a map to be a significant mis-feature. As only an initial transformation function applied directly to a map is able to receive the k-v semantics (a limitation I can’t see how would not carry over to transducers), this behavior crops up most frequently when re-ordering operations and discovering that an intermediate map has now caused an airity error somewhere in the middle of a chain of threaded transformations. I’ve never found cause to invoke it intentionally.

Comment by Ghadi Shayban [ 21/Jan/16 9:17 PM ]

Marshall, there really isn't a proposed enhancement, yet. So there's nothing to be against! Your input is valuable. (Regarding c.c.reducers, that is a separate problem – yes that behavior is surprising)

Considering kv-support for transducers:
Is it useful to have some functions that transform reduce-kv style reducing functions (fn [result k v])?

Ignore naming:
map-key
map-val
map-keyval
filter-*

These could be mechanically generated. You wouldn't have to have a kv-version for every transducer currently in core. Some like map or filter could specifically apply to the key and ignore the val, or v.v.

Some things like map's transducer would be arity-incompatible (map's transducer has a varargs arity).





[CLJ-1366] The empty map literal is read as a different map each time Created: 01/Mar/14  Updated: 15/May/17

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

Type: Enhancement Priority: Major
Reporter: Shogo Ohta 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
Approval: Prescreened

 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

Patch: 0002-make-the-reader-return-the-same-empty-map-when-it-re.patch

Prescreened by: Alex Miller



 Comments   
Comment by Shogo Ohta [ 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-1309] Bindings after :as in list destructuring should throw error Created: 19/Dec/13  Updated: 15/May/17  Resolved: 15/May/17

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

Type: Enhancement Priority: Minor
Reporter: ben wolfson Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: Compiler, destructuring, errormsgs

Approval: Ok

 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.



 Comments   
Comment by Alex Miller [ 15/May/17 4:40 PM ]

In 1.9, this now throws an error as the spec is violated.





[CLJ-1754] Destructuring with :merge Created: 16/Jun/15  Updated: 15/May/17  Resolved: 15/May/17

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

Type: Enhancement Priority: Major
Reporter: Henrik Heine Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: destructuring


 Description   

Destructuring with :or {...} does not add those defaults to :as binding. Destructuring with :merge adds this.
Usefull when you're wrapping calls to functions and want to add defaults that your callers need not pass in.

(defn foo [& {:merge {:c "C" :d "D"}
:as opt-args}]
opt-args)

should behave like:

(defn foo [& {:keys [c d]
:or {c "C" d "D"}
:as opt-args}]
(let [opt-args (merge {:c c :d d} opt-args)]
opt-args))

Options:
(a) the bindings for c and d in the example may be usefull or not. For the :merge example above they are not needed.
(b) :merge could use keywords or symbols. keywords make it look like the (merge) and symbols make it look like :keys/:or.

Suggestion: using symbols will build a binding for those names and using keywords will not. So users can get the bindings if they need them.

see also https://groups.google.com/forum/#!folder/Clojure$20Stuff/clojure/gG6Tzssn9Nw



 Comments   
Comment by Alex Miller [ 16/Jun/15 7:14 AM ]

Destructuring is about extracting parts of a composite input value. This seems to go a step beyond that into transformation of the input value. Can't say I am a fan of that but I will leave it open.





[CLJ-1951] bigint? predicate and generator Created: 08/Jun/16  Updated: 15/May/17

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

Type: Feature Priority: Minor
Reporter: Alex Miller Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: generator

Approval: Vetted

 Description   

Add bigint? and spec.gen support.

This part is easy:

(defn bigint?
  "Returns true if n is a BigInt"
  {:added "1.9"}
  [n] (instance? clojure.lang.BigInt n))

The generator is the tricky bit. test.check doesn't have a generator for bigints, just large-integer for things in long range. I think we'd want numbers beyond long range in a bigint generator (as that's a likely place where bugs might lie). Making a really high-quality bigint generator (with good growth and shrinking characteristics) is something that needs more thought.

http://clojure.github.io/test.check/clojure.test.check.generators.html#var-large-integer



 Comments   
Comment by Gary Fredericks [ 17/Dec/16 6:17 PM ]

In case I don't get around to making a patch, I think a generator along these lines would be a decent start:

(def gen-bigint 
  (gen/sized 
   (fn [size] 
     (let [large-integer (gen/resize size gen/large-integer)] 
       ;; scaling gives us relatively small vectors, but using  
       ;; the resized large-integer above means the numbers in 
       ;; the small vectors will still be big 
       (gen/scale #(+ 2 (/ % 20))
                  (gen/fmap (fn [xs] (+ (bigint (first xs)) (reduce * (map bigint (rest xs))))) 
                            (gen/not-empty (gen/vector large-integer))))))))
Comment by Gary Fredericks [ 17/Dec/16 6:21 PM ]

If anything seems inadequate about the sizing in the above generator, I should point out that sizing in test.check is rather subtle but the requirements are also not well-defined. I'd be happy to discuss in detail.

Comment by Gary Fredericks [ 15/May/17 4:39 PM ]

As an update, I've put together a marvelous bigint generator that this margin is too small to contain. It might need tweaking but is probably fine for these purposes. It's not on test.check master yet but I expect it will be soon.





[CLJ-107] GC Issue 103: bit-count function Created: 17/Jun/09  Updated: 15/May/17

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

Type: Feature Priority: Trivial
Reporter: Andy Fingerhut Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: File clj-107-v1.diff    

 Description   
Reported by a...@thened.net, Apr 08, 2009

I posted this small patch to the mailing list last week but received no
feedback, so I'm attaching it here to make sure it doesn't get lost.  I
have submitted a CA.

http://groups.google.com/group/clojure/browse_thread/thread/4345f76a12bac6fe/

The new function bit-count returns the count of 1-bits in a number, like
C's popcount or Common Lisp's logcount.


 Comments   
Comment by Assembla Importer [ 24/Aug/10 3:45 AM ]

Converted from http://www.assembla.com/spaces/clojure/tickets/107
Attachments:
bit-count.diff - https://www.assembla.com/spaces/clojure/documents/dqone2w4er3RbzeJe5afGb/download/dqone2w4er3RbzeJe5afGb

Comment by Assembla Importer [ 24/Aug/10 3:45 AM ]

oranenj said: [file:dqone2w4er3RbzeJe5afGb]

Comment by Assembla Importer [ 24/Aug/10 3:45 AM ]

richhickey said: Updating tickets (#8, #19, #30, #31, #126, #17, #42, #47, #50, #61, #64, #69, #71, #77, #79, #84, #87, #89, #96, #99, #103, #107, #112, #113, #114, #115, #118, #119, #121, #122, #124)

Comment by Andy Fingerhut [ 15/Nov/12 8:40 PM ]

clj-107-add-bit-count-v1.txt is probably a correct updated version of the old patch linked above. Added a couple of unit tests.

Comment by Andy Fingerhut [ 22/Oct/13 7:56 PM ]

Patch clj-107-v1.diff is identical to earlier clj-107-add-bit-count-v1.txt, but with .diff suffix for easier diff-style viewing in some editors.

Comment by Steve Miner [ 03/Jan/16 1:17 PM ]

Work-around: Use Java interop Long/bitCount or Integer/bitCount





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

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

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

Attachments: Text File CLJ-1871.patch    

 Description   

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

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

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


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

patch





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

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

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


 Description   

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

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

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

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

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

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






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

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

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

-



 Description   

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

Thoughts?






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

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

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

Attachments: Text File clj-1806-1.patch    

 Description   

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



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

patch w/ described changes





[CLJ-1787] Add executables to distribution (suggested changes included) Created: 27/Jul/15  Updated: 15/May/17  Resolved: 15/May/17

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

Type: Enhancement Priority: Major
Reporter: Morten Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None
Environment:

Linix, OS X and Windows


Patch: Code

 Description   

Being new to clojure I was surprised that there was not a executable included in the download. It makes Clojure appear less proff then it is and is unnecessary complex to remember the exact java cmd line to use so I suggest that shell executable files are added to the distibution for the next version of Clojure. They could look as suggest below for Windows and OS X:

1) Linux and OS X:
#!/bin/bash
java -jar `dirname "$0"`/clojure-1.8.0.jar $@

2) Windows:
java -jar %~dp0\clojure-1.8.0.jar %1 %2 %3 %4 %5 %6 %7 %8






[CLJ-1300] take-while with n<1 behaves like (repeat 1) Created: 22/Nov/13  Updated: 15/May/17

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: Enhancement 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.



 Comments   
Comment by Stuart Halloway [ 21/Jul/15 5:06 PM ]

This is probably even worse now – i.e. likely different behavior with transducer arity.

Comment by Alex Miller [ 15/May/17 4:37 PM ]
user=> (into [] (comp (take-nth -1) (take 5)) [5 9 14])
[5 9 14]




[CLJ-1026] Mixed end-of-line endings in the source code Created: 17/Jul/12  Updated: 15/May/17  Resolved: 15/May/17

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

Type: Enhancement Priority: Minor
Reporter: John Szakmeister Assignee: Unassigned
Resolution: Declined Votes: 2
Labels: None

Attachments: Text File 0001-Introduce-end-of-line-normalization.patch     File clj-1026-bash-script.sh    
Patch: Code

 Description   

Some source files in Clojure have the DOS/Windows carriage return/line feed line endings, whereas the majority use the Linux/Unix line feed only line endings.

The main ongoing issue with this is that if there is a correctly formatted git patch for such a file, then using the recommended 'git am' command to apply such a patch always gives warnings. Some screeners see such warnings, and immediately disregard the patch, asking for the warnings to be fixed before they will consider it. As far as I can tell, this is impossible. Thus patches modifying files with CR/LF line endings are nearly in a deadlock state, requiring unusual effort from non-screeners to remind screeners that such patches are as good as they can be, and would they please ignore those warnings because there is no way around them.

If someone can find a way to apply git patches to such files without such warnings, please update this description or add a comment.

Stu Halloway asks: Wouldn't eliminating all CR/LF endings in Clojure source files invalidate many unapplied patches? My answer: Please do it anyway. Others will step in to fix those patches, if they become broken.

The attached file clj-1026-bash-script.sh does several things, which should be easy to verify by reading through the script itself, which is quite simple:

1. find all files with CR/LF endings, and remove the CR characters.
2. Does 'git status .' to show the files that have been modified
3. Does 'git diff .' and 'git diff -w .', showing the number of lines that have been modified, and the number of lines that have been modified in a way that changes something besides whitespace, which should be exactly 0 lines, because only whitespace has been changed.

Original description when this ticket was created:

While examining some of the Clojure source code, I discovered that some files had mixed line endings, or CRLF line endings on a non-Windows box. Using .gitattributes, we can correct that so that files have the right endings for the platform that it's on.



 Comments   
Comment by John Szakmeister [ 17/Jul/12 2:26 PM ]

Patch to fix line endings and introduce .gitattributes.

Comment by Stuart Halloway [ 20/Jul/12 4:47 PM ]

This looks like a change to every line in the world, which makes it hard to vet. Also: will it render incompatible all other outstanding patches at the time it is applied?

Comment by John Szakmeister [ 21/Jul/12 6:52 AM ]

You can use git diff -w $(git merge-base HEAD master) to see the actual diff minus the line ending change. Here it is inline:

:: git diff -w $(git merge-base HEAD master)
diff --git a/.gitattributes b/.gitattributes
new file mode 100644
index 0000000..7b89cfa
--- /dev/null
+++ b/.gitattributes
@@ -0,0 +1,6 @@
+*.txt           text
+*.clj           text
+*.html          text
+*.js            text
+*.css           text
+*.java          text diff=java

Also, no, it won't render all the outstanding patches incompatible. For one, it seems like the files that have the eols mixed or in CRLF aren't touched much. I also went through the majority of outstanding patches and couldn't fix one that conflicts. Secondly, format-patch emits the patch inline and is intended to be sent via email. SMTP says that all lines must end with CRLF, so line endings are effectively lost. So git will convert it to the right line ending on application.

It can conflict with any outstanding branches that you may have. Supplying the renormalization option on merge can help:

git merge -X renormalize <branch-name>

Or, you can enable this by default for your repository:

git config --local merge.renormalize true

If you think it's too much trouble, let's just drop it though.

Comment by Stuart Halloway [ 10/Aug/12 1:15 PM ]

Patch does not apply on my working copy of Clojure. I am using

git am -s ...
/Users/stu/repos/clojure/.git/rebase-apply/patch:344: trailing whitespace.
  p {  	
/Users/stu/repos/clojure/.git/rebase-apply/patch:350: space before tab in indent.
  	margin-left: 0.5in;
error: patch failed: epl-v10.html:1
error: epl-v10.html: patch does not apply
error: patch failed: src/jvm/clojure/asm/AnnotationVisitor.java:1
error: src/jvm/clojure/asm/AnnotationVisitor.java: patch does not apply
error: patch failed: src/jvm/clojure/asm/AnnotationWriter.java:1
error: src/jvm/clojure/asm/AnnotationWriter.java: patch does not apply

I am willing to do this, just inept.

Comment by Andy Fingerhut [ 10/Aug/12 1:21 PM ]

Stuart, I updated this page http://dev.clojure.org/display/design/JIRA+workflow a while back when I had trouble applying some patches involving files with carriage return line endings. I did some Googling on git docs and found the option '--keep-cr' that seems to help in such cases. Try that out.

Comment by Alex Miller [ 04/Sep/13 12:36 PM ]

I think in this case, we should not provide a patch but a process. And I would like to see this cleaned up.

Comment by Andy Fingerhut [ 04/Sep/13 1:14 PM ]

I am not sure what kind of process you mean, Alex, but here are some thoughts.

There are some patches attached to tickets right now that modify Java source files that have CR/LF line endings throughout. Typically those patches remove some lines, and add new ones, and those new lines typically have CR/LF line endings, too, because whatever editor the person used while making the patch saw the file have CR/LF line endings, and preserved that on new lines of text they added to the file.

So if you want to get rid of CRs throughout the Clojure source code, and keep them out, one way would be to:

(1) do a big commit to get rid of all of the current CR characters in source files.

(2) update the few pending patches that introduce lines with CRs (I checked. There are only 15 tickets with such patches right now.)

(3) I can automate a process of detecting patches that have CRs and flag them for CR removal.

In fact (2) can be done as a result of (3).

Comment by Alex Miller [ 04/Sep/13 2:29 PM ]

I just mean that any patch for this ticket is doomed to be continuously broken until it is applied, so it would be better to instead have a set of steps that someone can follow to fix all line endings, than a patch. That is #1 on your list should be a set of steps. #2 and 3 would be nice as well.

Comment by Andy Fingerhut [ 04/Sep/13 7:48 PM ]

clj-1026-bash-script.sh is not a patch, but a bash script that can be run in an unmodified Clojure source tree, preferably freshly cloned, that will delete carriage returns characters from all files except those in the .git directory, and directories.

It also prints out the results of a few commands to help you gain confidence that those are the only changes it made.

Tested on Mac OS X 10.8.4 and Ubuntu Linux 12.04.3.

Comment by Andy Fingerhut [ 31/Aug/14 3:32 PM ]

Shell script clj-1026-bash-script.sh has updated 'comments' printed with the latest stats of number of files and lines containing carriage return characters as of Aug 31 2014.

Comment by Michael Blume [ 07/Jul/15 5:16 PM ]

Would now be a good time to come back to this?

(I'd love to see the indentation in the Java source standardized too, but probably that's another ticket)

Comment by Andy Fingerhut [ 07/Jul/15 7:35 PM ]

I must say that it seems that since Alex Miller has been on the job, the times where patches are summarily rejected for various minor reasons like warnings, spurious white space changes, etc. has gone way down, since he often cleans them up himself. Kudos to Alex!

Comment by Alex Miller [ 15/May/17 4:32 PM ]

I'm not saying that we will or will not change this eventually, but if so, it won't be via a patch on a ticket.