<< Back to previous view

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

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

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


 Description   

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



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

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

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

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

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

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

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

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





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

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

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

MacOS 10.12.6, current spec as of Sept 2 2017



 Description   

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

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

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

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

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

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

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

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



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

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

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

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

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

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

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

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

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

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

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

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

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

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

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





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

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

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


 Description   
(let [1 x] x)

;; Following error will be thrown

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

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

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



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

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

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

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

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

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

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

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

Alex Miller Do you have any comments on this?

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

I don't think it's a problem.

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

OK, thanks.





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

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

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


 Description   

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



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

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

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

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

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

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

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

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

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

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

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

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

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

Thank you, that makes sense of course.

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

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

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

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

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

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

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

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

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

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

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

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





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

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

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

Ubuntu 17.04



 Description   

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

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

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



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

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

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

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

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

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

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

Thank you for your quick, insightful reply.

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

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





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

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

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

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

 Description   

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

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



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

Dupe of CLJ-2037

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

Also, this is related to CLJ-2194.





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

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

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


 Description   

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

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

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

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

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



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

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





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

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

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

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

 Description   

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

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



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

Applied per Rich's ok





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

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

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

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

 Description   

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

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

After patch:

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

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

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

Patch: clj-2219-2.patch



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

Applied on Rich's ok

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

reopened just to fix approval state.

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

re-closed





[CLJ-2216] Change *explain-out* to return string, instead of printing output Created: 31/Jul/17  Updated: 25/Aug/17  Resolved: 25/Aug/17

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

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

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


Attachments: Text File CLJ-2216.patch    

 Description   

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

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

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

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

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

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

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

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

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

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

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

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



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

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

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

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

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

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

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

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

Agreed with Andy's comments.

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

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

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

Alex and Andy,

Thank you very much for the helpful feedback.

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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





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

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

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

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

 Description   

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

With init.clj as :

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

(time (eval expr))

Before patch:

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

After patch:

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

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

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

Prescreened by: Alex Miller






[CLJ-2209] getting started documentation for 1.9.0-alpha uses 1.8.0.jar and does not work with 1.9.0-alpha17.jar due to missing specs related jars in the classpath Created: 20/Jul/17  Updated: 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-2207] Clojure reader should treat non-breaking space as whitespace character Created: 14/Jul/17  Updated: 27/Jul/17  Resolved: 25/Jul/17

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

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

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

 Description   

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

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

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

Patch: clj-2207-nbsp-v3.patch

Prescreened by: Alex Miller



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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

Comment by Andy Fingerhut [ 27/Jul/17 12:06 AM ]

Would it be reasonable to treat nonbreaking spaces in Clojure source as an error, unless it is inside of a string, comment, regex, or similar place (I can't think of any other similar places right now, but there may be some)?

Comment by Andy Fingerhut [ 27/Jul/17 12:13 AM ]

A quick test with Java shows that it allows non-breaking spaces inside of comments and strings, but not any of a few other places in a Java source file that I have tested with (e.g. in the middle of an identifier, in the middle of other white space).

Comment by Nikita Prokopov [ 27/Jul/17 12:08 PM ]

> Would it be reasonable to treat nonbreaking spaces in Clojure source as an error

It might work, but why? Why use of some whitespace characters is allowed but other, also whitespace, characters should be forbidden? Nothing magical or special about non-breaking spaces. The intent of them is that they are just like normal spaces. Treating them differently would just confuse people.

Comment by Andy Fingerhut [ 27/Jul/17 12:46 PM ]

Treating them as errors wouldn't be confusing at all – the compiler tells you where they are, and you change them to normal spaces in your Clojure source code and move on. How would that confuse people?

Comment by Nikita Prokopov [ 27/Jul/17 12:52 PM ]

It’s like saying you can’t use letter A in code, only in strings. What’s wrong with letter A? Why can’t I use it. If there’s special rule about it there better be a reason too.

Comment by Andy Fingerhut [ 27/Jul/17 12:55 PM ]

Maybe I can clarify my last question a little bit. Here are 3 alternatives (not intended to be exhaustive):

1. treat non-breaking spaces and similar characters as ones that can be part of var names and symbols

2. treat them as compiler errors, unless they are in comments, strings, or regexes

3. treat them as other whitespace characters are treated.

Alternative #3 is what this ticket proposed, and it was declined.

I think alternative #1 is where Clojure is now, and my guess is that among the 3 alternatives, it is the one most likely to cause confusion for people who accidentally introduce such a character in a Clojure source file.

I believe #2 is what the Java compiler does when compiling Java source files (based only on a few quick experiments, not complete knowledge of the subject). Alex Miller mentioned taking our cue from Java, so I thought I would propose it as an alternative to #1 and #3. If a separate JIRA ticket for this idea is desirable, I'm happy to create one.





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

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

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


 Description   

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

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



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

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

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





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

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

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


 Description   

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



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

Superceded by CLJ-2204





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

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

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

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

 Description   

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

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

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

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

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

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

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

Prescreened by: Alex Miller

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



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

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

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

#!/bin/bash

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

Thanks for reviewing it.

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

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

I think the simpler test is sufficient.

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

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

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

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





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

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

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

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



 Description   

In Clojure:

#{'() []}

Expected: No errors (the above works in Clojurescript)

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

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

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

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



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

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

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

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





[CLJ-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-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-2188] clojure.core/slurp does not mark its return type as String Created: 23/Jun/17  Updated: 07/Sep/17  Resolved: 07/Sep/17

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

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

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

 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-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-2184] propagate metadata in doto forms Created: 20/Jun/17  Updated: 07/Sep/17  Resolved: 07/Sep/17

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

Type: Enhancement Priority: Minor
Reporter: Chouser Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: errormsgs

Attachments: Text File CLJ-2184-patch-00.patch    
Patch: Code
Approval: Ok

 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-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-2170] Several top-level forms have improperly-located docstrings Created: 29/May/17  Updated: 06/Sep/17  Resolved: 06/Sep/17

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

Type: Defect Priority: Minor
Reporter: Cameron Desautels Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: docstring

Attachments: Text File fix-improper-docstrings.patch    
Patch: Code
Approval: Ok

 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-2161] clojure.spec.alpha fails to reload Created: 02/May/17  Updated: 03/May/17  Resolved: 02/May/17

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

Type: Defect Priority: Critical
Reporter: Jeaye Wilkerson Assignee: Unassigned
Resolution: Completed Votes: 1
Labels: spec

Approval: Ok

 Description   

While looking to upgrade Orchestra to 1.9.0-alpha16, I found that I'm not able to reload clojure.spec.alpha more than once in the REPL. I've made a bare bones test case, linked below, which demonstrates the issue. I believe this is related to https://dev.clojure.org/jira/browse/CLJ-2098 but unrelated to autodoc, so I opted for a new ticket. That ticket is marked Critical, so I marked this similarly.

https://github.com/jeaye/clojure-spec-alpha-reload

The relevant Orchestra discussion is here: https://github.com/jeaye/orchestra/pull/3



 Comments   
Comment by Alex Miller [ 02/May/17 9:36 PM ]

Both of these problems are related to the current spec.alpha build not being AOT compiled. I've already made this change in spec.alpha for the next release and I believe that will fix the problem.

I've released a new version of spec.alpha (0.1.108) and I think if you update to that version, it should address the problem.

Comment by Daniel Compton [ 02/May/17 9:57 PM ]

Thanks for the quick turnaround, that version does seem to fix the problem.

Comment by Jeaye Wilkerson [ 03/May/17 12:06 AM ]

Thank you!





[CLJ-2156] Document char[] input support in clojure.java.io/copy Created: 22/Apr/17  Updated: 06/Sep/17  Resolved: 06/Sep/17

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

Type: Defect Priority: Trivial
Reporter: Yegor Timoshenko Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: docstring

Attachments: Text File copy-doc.patch    
Patch: Code
Approval: Ok

 Description   

https://github.com/clojure/clojure/blob/a26dfc1390c53ca10dba750b8d5e6b93e846c067/src/clj/clojure/java/io.clj#L393

copy actually supports char[] as input (see lines 373-380), so add to docstring.

Prescreened by: Alex Miller - tests already exist for this behavior, just not in docstring






[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-2151] clojure.spec: s/merge reports errors multiple times with qualified keywords Created: 11/Apr/17  Updated: 11/Apr/17  Resolved: 11/Apr/17

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

Type: Defect Priority: Major
Reporter: Tommi Reiman Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: spec
Environment:

1.9-alpha15



 Description   

With unqualified keys, the problems look as expected. With qualified keys, the problems are shown from all branches?

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

(s/def ::a int?)
(s/def ::b string?)

(s/explain-data
  (s/merge
    (s/keys :req-un [::a])
    (s/keys :req-un [::b]))
  {:a 1 :b 1})
; #:clojure.spec{:problems ({:path [:b], :pred string?, :val 1, :via [:user/b], :in [:b]})}

(s/explain-data
  (s/merge
    (s/keys :req [::a])
    (s/keys :req [::b]))
  {::a 1 ::b 1})
;#:clojure.spec{:problems ({:path [:user/b], :pred string?, :val 1, :via [:user/b], :in [:user/b]}
;                          {:path [:user/b], :pred string?, :val 1, :via [:user/b], :in [:user/b]})}


 Comments   
Comment by Alex Miller [ 11/Apr/17 3:42 PM ]

Spec explain reports every error it found - in this case it found that the required key ::b is missing in both branches of the s/merge. So I think this is the expected behavior.





[CLJ-2150] c.l Created: 11/Apr/17  Updated: 11/Apr/17  Resolved: 11/Apr/17

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

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


 Comments   
Comment by Tommi Reiman [ 11/Apr/17 7:35 AM ]

keybord typing error created multiple issues. Closing the unfinished duplicates.





[CLJ-2149] c.l Created: 11/Apr/17  Updated: 11/Apr/17  Resolved: 11/Apr/17

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

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


 Comments   
Comment by Tommi Reiman [ 11/Apr/17 7:35 AM ]

keybord typing error created multiple issues. Closing the unfinished duplicates.





[CLJ-2148] c.l Created: 11/Apr/17  Updated: 11/Apr/17  Resolved: 11/Apr/17

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

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


 Comments   
Comment by Tommi Reiman [ 11/Apr/17 7:35 AM ]

keybord typing error created multiple issues. Closing the unfinished duplicates.





[CLJ-2147] clojure.spec: s/keys* doesn't return a spec Created: 11/Apr/17  Updated: 11/Apr/17  Resolved: 11/Apr/17

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

Type: Defect Priority: Major
Reporter: Tommi Reiman Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: spec
Environment:

1.9.0-alpha15



 Description   
(s/spec? (s/keys* :req-un [::a ::b]))
; nil


 Comments   
Comment by Alex Miller [ 11/Apr/17 3:46 PM ]

This is pretty subtle but the regex ops return something that is a `regex?` but not a `spec?`. The reason for this has to do with being able to merge things that are regexes, but not merge across specs. (There is also some further history based in prior implementations.) In any case, this is currently the expected result.





[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-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-2140] surprising output from s/explain for a s/coll-of spec that encounters a map Created: 31/Mar/17  Updated: 31/Mar/17  Resolved: 31/Mar/17

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

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


 Description   

When a coll-of spec encounters a map, the explain output does not align with the validation issue:

(s/def :some/keywords (s/coll-of keyword?))
=> :some/keywords
(s/explain-data :some/keywords [:foo])
=> nil
(s/explain-data :some/keywords ["foo"])
=> #:clojure.spec{:problems ({:path [], :pred keyword?, :val "foo", :via [:some/keywords], :in [0]})}
(s/explain-data :some/keywords :foo)
=> #:clojure.spec{:problems [{:path [], :pred coll?, :val :foo, :via [:some/keywords], :in []}]}
(s/explain-data :some/keywords {:foo :bar})
=> #:clojure.spec{:problems ({:path [], :pred keyword?, :val [:foo :bar], :via [:some/keywords], :in [0]})}

I'd expect this from the last expression:

=> #:clojure.spec{:problems [{:path [], :pred coll?, :val {:foo :bar}, :via [:some/keywords], :in []}]}


 Comments   
Comment by Alex Miller [ 31/Mar/17 9:13 AM ]

Maps are collections and it is possible and occasionally useful to use s/coll-of or s-every to spec maps as collections of tuples. So, I think this is the expected and correct behavior.

There are some related cases in CLJ-2080 where this can go awry but the changes in the patch there would not affect this case.





[CLJ-2139] Couldn't satisfy such-that predicate after 100 tries error when using int? and int-in together Created: 29/Mar/17  Updated: 30/Mar/17  Resolved: 30/Mar/17

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

Type: Defect Priority: Minor
Reporter: Didier A. Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: newbie, spec
Environment:

Windows 7
Clojure 1.9.0-alpha15



 Description   

If you spec :args with (s/and int? (s/int-in x y)) you will get "ExceptionInfo Couldn't satisfy such-that predicate after 100 tries."

Here's code to reproduce:

Unable to find source-code formatter for language: clj. Available languages are: javascript, sql, xhtml, actionscript, none, html, xml, java
(ns spec-test.core
  (:require [clojure.spec :as s]
            [clojure.spec.test :as st]))

(defn aaa [a] (.charAt "abcdef" a))
(s/fdef aaa
        :args (s/cat :a (s/and int? (s/int-in 0 5)))
        :ret any?)

(st/check `aaa)


 Comments   
Comment by Alex Miller [ 30/Mar/17 8:59 AM ]

The problem here is that when you use (s/and int? (s/int-in 0 5)) as the spec, the generator will generate random ints from the first subspec, then filter based on the second one. Because most of the numbers don't fall in the range, it will fail to generate. You can find more info about and generators at http://blog.cognitect.com/blog/2016/8/24/combining-specs-with-and.

In this particular case, it's easy to adjust this by simply reducing your spec to (s/int-in 0 5) which is designed to generate only values in that range.





[CLJ-2138] Can't use an aliased keyword in the same form as alias definition Created: 25/Mar/17  Updated: 26/Mar/17  Resolved: 26/Mar/17

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

Type: Defect Priority: Major
Reporter: Yegor Timoshenko Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None
Environment:

java version "1.8.0_111"
Java(TM) SE Runtime Environment (build 1.8.0_111-b14)
Java HotSpot(TM) 64-Bit Server VM (build 25.111-b14, mixed mode)



 Description   

user=> (require '[clojure.string :as str])
nil
user=> ::str/hello
:clojure.string/hello
user=> (do (require '[clojure.string :as str2]) ::str2/hello)

RuntimeException Invalid token: ::str2/hello clojure.lang.Util.runtimeException (Util.java:221)
RuntimeException Unmatched delimiter: ) clojure.lang.Util.runtimeException (Util.java:221)

At the same time, creating an alias and using a function from the aliased namespace is possible:

user=> (do (require '[clojure.string :as str3]) (str3/blank? ""))
true



 Comments   
Comment by Alex Miller [ 26/Mar/17 11:05 AM ]

Autoresolved keywords are a feature of the reader, which reads one form at a time. In this case, the whole do block is read prior to being evaluated so the alias context does not yet exist when that keyword is read.





[CLJ-2137] Clojure REPL doesn't ask for new input if line contains a keyword with a colon Created: 25/Mar/17  Updated: 26/Mar/17  Resolved: 26/Mar/17

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

Type: Defect Priority: Major
Reporter: Yegor Timoshenko Assignee: Unassigned
Resolution: Not Reproducible Votes: 0
Labels: reader, repl
Environment:

java version "1.8.0_111"
Java(TM) SE Runtime Environment (build 1.8.0_111-b14)
Java HotSpot(TM) 64-Bit Server VM (build 25.111-b14, mixed mode)



 Description   
user=> :a
:a
user=> :a:z
:a:z
user=> [:a
  #_=> ]
[:a]
user=> [:a:z

RuntimeException EOF while reading, starting at line 1  clojure.lang.Util.runtimeException (Util.java:221)


 Comments   
Comment by Alex Miller [ 26/Mar/17 7:44 PM ]

This is not reproducible with the standard Clojure repl (invoking clojure.main). I can reproduce it with Leiningen so I'm guessing that's where you're seeing it. You can file a bug report for Leiningen at https://github.com/technomancy/leiningen.

Comment by Yegor Timoshenko [ 26/Mar/17 7:48 PM ]

Couldn't imagine that a build tool can affect the REPL. Thank you!





[CLJ-2136] Reloading multi-arity macros fails Created: 24/Mar/17  Updated: 24/Mar/17  Resolved: 24/Mar/17

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

Type: Defect Priority: Major
Reporter: Joel Kaasinen Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: macro
Environment:

Tested against clojure 1.7 and 1.8



 Description   

When a multi-arity macro is defined using &form and &env, reloading the namespace fails.

Steps to reproduce:

File macro.clj:

(ns macro)

(defmacro macro
  ([x] (macro &form &env x 2))
  ([x y] `(prn ~x ~y)))

REPL:

user=> (require 'macro)
nil
user=> (macro/macro 1)
1 2
nil
user=> (require 'macro :reload)

CompilerException clojure.lang.ArityException: Wrong number of args (4) passed to: macro/macro, compiling:(macro.clj:4:8)

PS. a workaround is to define the macro like

(defmacro macro
  ([x] `(macro ~x 2))
  ([x y] `(prn ~x ~y)))


 Comments   
Comment by Alex Miller [ 24/Mar/17 8:26 AM ]

This macro definition is incorrect as it's passing 4 args from one arity to the other, not 2 (which is what the error tells you). The "workaround" fixes that problem. I don't see a bug here.





[CLJ-2132] Maven pom requires artifact signing to install locally Created: 23/Mar/17  Updated: 20/Apr/17  Resolved: 20/Apr/17

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

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

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

 Description   

The recent pom changes inadvertently made artifact signing a requirement for locally installing a Clojure build via:

mvn clean install

The attached patch moves the GPG plugin back into a profile (named "sign").






[CLJ-2131] partition-with Created: 19/Mar/17  Updated: 19/Mar/17  Resolved: 19/Mar/17

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

Type: Enhancement Priority: Trivial
Reporter: Derek Troy-West Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None


 Description   

Any interest in introducing a partition fn that sits somewhere between partition-by and split-with?

(defn partition-with
"Applies f to each value in coll, splitting it each time f returns truthy
Returns a lazy seq of partitions."
[f coll]
(lazy-seq
(when-let [s (seq coll)]
(let [run (cons (first s) (take-while (complement f) (next s)))]
(cons run (partition-with f (seq (drop (count run) s))))))))

e.g

(partition-with #(= (rem % 3) 0) [1 2 3 6 7 8 9 12 13 15 16 17 18])
=> ((1 2) (3) (6 7 8) (9) (12 13) (15 16 17) (18))

I've used this occasionally and I notice it popped up on StackOverflow recently.

Not included thus far: the transducer arity or tests, but I'm happy to supply a patch if you're interested.



 Comments   
Comment by Derek Troy-West [ 19/Mar/17 6:31 AM ]

Apols for the formatting, I don't seem to be able to edit.

Comment by Derek Troy-West [ 19/Mar/17 7:12 AM ]

On reflection the special case of a seq of delimited sub-sequences is probably too narrow for core, which explains its current absence. Please ignore (I would kill the Jira myself, but..)

Comment by Alex Miller [ 19/Mar/17 12:32 PM ]

closed per request





[CLJ-2130] classof data-reader Created: 17/Mar/17  Updated: 17/Mar/17  Resolved: 17/Mar/17

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

Type: Enhancement Priority: Minor
Reporter: Greg Chapman Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None


 Description   

I think it would be useful to have a #classof default data-reader which would read the supplied form as a Java class. Examples:

#classof String -> java.lang.String
#classof java.util.Map -> java.util.Map
#classof [String] -> [Ljava.lang.String;
#classof long -> long (aka Long/TYPE)
#classof [long] -> [J
#classof [[int]] -> [[I

So the idea is that a symbol would be read as the class to which it resolves (with support for primitives), and vectors would be read as the class of arrays of the classof the first (and only) item in the vector.

The main reason for wanting this is to have a readable form for array classes.



 Comments   
Comment by Alex Miller [ 17/Mar/17 4:02 PM ]

This is a solution, not a problem. The description only lightly mentions the problem at the end but does not demonstrate (preferably in an example) where this problem is encountered or why the current solution for these problems is not sufficient. The suggestion here is one idea, but no other alternatives are suggested, and tradeoffs are not considered.

Comment by Greg Chapman [ 17/Mar/17 8:01 PM ]

This is a fair criticism, and I regret having posted the idea without exploring it more fully. Especially as the above idea will not work for type-hinting (for some reason, I thought instances of Class could be used in :tags, but I now see that only Symbols and Strings are allowed).

Anyway, the thing that got me thinking along these lines is the annoyance involved with extending protocols to arrays. In particular, extend-type uses its t parameter in two ways: 1) emitted as-is to be evaluated (resolving to a Class) and passed as the first parameter to extend, and 2) in the unemitted metadata to type-hint the protocol functions. I don't think there's a way to satisfy both these uses with a single form representing an array class, so you have to fall back on extend and do your own type-hinting.

But that's not really that big a deal. I apologize for wasting your time with this.

Comment by Alex Miller [ 17/Mar/17 10:05 PM ]

You might also want to keep an eye on CLJ-1381.





[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-2127] clojure.spec/def requires literal keyword as first argument Created: 15/Mar/17  Updated: 15/Mar/17  Resolved: 15/Mar/17

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

Type: Defect Priority: Minor
Reporter: Anders Hessellund Jensen Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None


 Description   

I expected the following to work:
(clojure.spec/def (keyword "foo" "bar") string?)

It fails, because the def macro does not evaluate the first argument. Is this intentional? If so, perhaps the documentation or error message could be updated to reflect that the macro only accepts literal keywords.

As an aside, as a clojure beginner I found the semantics of clojure.spec/def confusing. The regular def macro and its variants creates bindings in the current namespace. clojure.spec/def does not modify the namespace, it registers specs in the spec registry under the provided key, even though it also accepts a symbol as an argument. I would have been less confused if the function was named clojure.spec/register!, the first argument was evaluated, and registering a symbol would require quoting of the symbol. That would make it behave like a regular function where arguments are evaluated as usual.



 Comments   
Comment by Alex Miller [ 15/Mar/17 9:37 AM ]

The docstring starts "Given a namespace-qualified keyword or resolvable symbol k" which seems to say the accurate thing. The error seems pretty clear to me too:

user=> (clojure.spec/def (keyword "foo" "bar") string?)
AssertionError Assert failed: k must be namespaced keyword or resolvable symbol
(c/and (ident? k) (namespace k))

If you really need to do something like this, it's easy enough to wrap with another macro like:

user=> (defmacro mydef [ns n s] `(s/def ~(keyword ns n) ~s))
#'user/mydef
user=> (mydef "foo" "bar" string?)
:foo/bar

Rich considered all the ramifications of the name when he named it, so that's not likely to change.

Comment by Anders Hessellund Jensen [ 15/Mar/17 10:49 AM ]

I still fail to see how I can infer from the documentation that k has to be a literal qualified keyword, and that an expression evaluating to a qualified keyword is not accepted. I assume it is my lack of Clojure experience.

Thanks for your response, and apologise for wasting your time.

Comment by Alex Miller [ 15/Mar/17 11:18 AM ]

This is the situation with many core macros (which are after all taking code and returning code and thus are a bit more literal-minded than functions). No apologies necessary - while I'm declining here, it's still useful feedback and might affect decisions in the future.





[CLJ-2125] fspec doesn't generate pure functions. Created: 12/Mar/17  Updated: 12/Mar/17  Resolved: 12/Mar/17

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

Type: Enhancement Priority: Minor
Reporter: N/A Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: spec


 Description   

fspec doesn't generate pure functions.

(defn foo
[f]
(let [mf (memoize f)]
(= (mf 0) (mf 0))))

(s/fdef foo
:args (s/cat :f (s/fspec :args (s/cat :y int?)
:ret int?))
:ret true?)

(stest/check `foo)

=>
({:spec #object[clojure.spec$fspec_impl$reify__14282
0x20877912
"clojure.spec$fspec_impl$reify__14282@20877912"],
:clojure.spec.test.check/ret {:result true,
:num-tests 1000,
:seed 1489361298159},
:sym spike-spec.core/foo})

(defn bar
[f]
(= (f 0) (f 0)))

(s/fdef bar
:args (s/cat :f (s/fspec :args (s/cat :y int?)
:ret int?))
:ret true?)

(stest/check `bar)

=>
({:spec #object[clojure.spec$fspec_impl$reify__14282
0x56f3d7c7
"clojure.spec$fspec_impl$reify__14282@56f3d7c7"],
:clojure.spec.test.check/ret {:result #error{:cause "Specification-based check failed",
:data {:clojure.spec/problems [{:path [:ret],
:pred true?,
:val false,
:via [],
:in []}],
:clojure.spec.test/args (#object[clojure.spec$fspec_impl$reify_14282$fn_14285
0x7cf479f8
"clojure.spec$fspec_impl$reify_14282$fn_14285@7cf479f8"]),
:clojure.spec.test/val false,
:clojure.spec/failure :check-failed},
:via [{:type clojure.lang.ExceptionInfo,
:message "Specification-based check failed",
:data {:clojure.spec/problems [{:path [:ret],
:pred true?,
:val false,
:via [],
:in []}],
:clojure.spec.test/args (#object[clojure.spec$fspec_impl$reify_14282$fn_14285
0x7cf479f8
"clojure.spec$fspec_impl$reify_14282$fn_14285@7cf479f8"]),
:clojure.spec.test/val false,
:clojure.spec/failure :check-failed},
:at [clojure.core$ex_info
invokeStatic
"core.clj"
4725]}],
:trace [[clojure.core$ex_info
invokeStatic
"core.clj"
4725]
[clojure.spec.test$explain_check
invokeStatic
"test.clj"
279]
[clojure.spec.test$check_call
invokeStatic
"test.clj"
295]
[clojure.spec.test$quick_check$fn__13414
invoke
"test.clj"
308]
[clojure.lang.AFn
applyToHelper
"AFn.java"
154]
[clojure.lang.AFn
applyTo
"AFn.java"
144]
[clojure.core$apply
invokeStatic
"core.clj"
657]
[clojure.core$apply
invoke
"core.clj"
652]
[clojure.test.check.properties$apply_gen$fn_1500$fn_1501
invoke
"properties.cljc"
16]
[clojure.test.check.properties$apply_gen$fn__1500
invoke
"properties.cljc"
16]
[clojure.test.check.rose_tree$fmap
invokeStatic
"rose_tree.cljc"
78]
[clojure.test.check.rose_tree$fmap
invoke
"rose_tree.cljc"
74]
[clojure.test.check.generators$fmap$fn__629
invoke
"generators.cljc"
89]
[clojure.test.check.generators$gen_fmap$fn__579
invoke
"generators.cljc"
55]
[clojure.test.check.generators$call_gen
invokeStatic
"generators.cljc"
41]
[clojure.test.check.generators$call_gen
invoke
"generators.cljc"
38]
[clojure.test.check$quick_check
invokeStatic
"check.cljc"
62]
[clojure.test.check$quick_check
doInvoke
"check.cljc"
37]
[clojure.lang.RestFn
invoke
"RestFn.java"
425]
[clojure.lang.AFn
applyToHelper
"AFn.java"
156]
[clojure.lang.RestFn
applyTo
"RestFn.java"
132]
[clojure.core$apply
invokeStatic
"core.clj"
657]
[clojure.spec.gen$quick_check
invokeStatic
"gen.clj"
27]
[clojure.spec.gen$quick_check
doInvoke
"gen.clj"
27]
[clojure.lang.RestFn
applyTo
"RestFn.java"
137]
[clojure.core$apply
invokeStatic
"core.clj"
661]
[clojure.spec.test$quick_check
invokeStatic
"test.clj"
306]
[clojure.spec.test$check_1
invokeStatic
"test.clj"
334]
[clojure.spec.test$check$fn__13433
invoke
"test.clj"
410]
[clojure.core$pmap$fn_9385$fn_9386
invoke
"core.clj"
6897]
[clojure.core$binding_conveyor_fn$fn__6772
invoke
"core.clj"
2020]
[clojure.lang.AFn
call
"AFn.java"
18]
[java.util.concurrent.FutureTask
run
"FutureTask.java"
266]
[java.util.concurrent.ThreadPoolExecutor
runWorker
"ThreadPoolExecutor.java"
1142]
[java.util.concurrent.ThreadPoolExecutor$Worker
run
"ThreadPoolExecutor.java"
617]
[java.lang.Thread
run
"Thread.java"
745]]},
:seed 1489361392805,
:failing-size 0,
:num-tests 1,
:fail [(#object[clojure.spec$fspec_impl$reify_14282$fn_14285
0x7cf479f8
"clojure.spec$fspec_impl$reify_14282$fn_14285@7cf479f8"])],
:shrunk {:total-nodes-visited 0,
:depth 0,
:result #error{:cause "Specification-based check failed",
:data {:clojure.spec/problems [{:path [:ret],
:pred true?,
:val false,
:via [],
:in []}],
:clojure.spec.test/args (#object[clojure.spec$fspec_impl$reify_14282$fn_14285
0x7cf479f8
"clojure.spec$fspec_impl$reify_14282$fn_14285@7cf479f8"]),
:clojure.spec.test/val false,
:clojure.spec/failure :check-failed},
:via [{:type clojure.lang.ExceptionInfo,
:message "Specification-based check failed",
:data {:clojure.spec/problems [{:path [:ret],
:pred true?,
:val false,
:via [],
:in []}],
:clojure.spec.test/args (#object[clojure.spec$fspec_impl$reify_14282$fn_14285
0x7cf479f8
"clojure.spec$fspec_impl$reify_14282$fn_14285@7cf479f8"]),
:clojure.spec.test/val false,
:clojure.spec/failure :check-failed},
:at [clojure.core$ex_info
invokeStatic
"core.clj"
4725]}],
:trace [[clojure.core$ex_info
invokeStatic
"core.clj"
4725]
[clojure.spec.test$explain_check
invokeStatic
"test.clj"
279]
[clojure.spec.test$check_call
invokeStatic
"test.clj"
295]
[clojure.spec.test$quick_check$fn__13414
invoke
"test.clj"
308]
[clojure.lang.AFn
applyToHelper
"AFn.java"
154]
[clojure.lang.AFn
applyTo
"AFn.java"
144]
[clojure.core$apply
invokeStatic
"core.clj"
657]
[clojure.core$apply
invoke
"core.clj"
652]
[clojure.test.check.properties$apply_gen$fn_1500$fn_1501
invoke
"properties.cljc"
16]
[clojure.test.check.properties$apply_gen$fn__1500
invoke
"properties.cljc"
16]
[clojure.test.check.rose_tree$fmap
invokeStatic
"rose_tree.cljc"
78]
[clojure.test.check.rose_tree$fmap
invoke
"rose_tree.cljc"
74]
[clojure.test.check.generators$fmap$fn__629
invoke
"generators.cljc"
89]
[clojure.test.check.generators$gen_fmap$fn__579
invoke
"generators.cljc"
55]
[clojure.test.check.generators$call_gen
invokeStatic
"generators.cljc"
41]
[clojure.test.check.generators$call_gen
invoke
"generators.cljc"
38]
[clojure.test.check$quick_check
invokeStatic
"check.cljc"
62]
[clojure.test.check$quick_check
doInvoke
"check.cljc"
37]
[clojure.lang.RestFn
invoke
"RestFn.java"
425]
[clojure.lang.AFn
applyToHelper
"AFn.java"
156]
[clojure.lang.RestFn
applyTo
"RestFn.java"
132]
[clojure.core$apply
invokeStatic
"core.clj"
657]
[clojure.spec.gen$quick_check
invokeStatic
"gen.clj"
27]
[clojure.spec.gen$quick_check
doInvoke
"gen.clj"
27]
[clojure.lang.RestFn
applyTo
"RestFn.java"
137]
[clojure.core$apply
invokeStatic
"core.clj"
661]
[clojure.spec.test$quick_check
invokeStatic
"test.clj"
306]
[clojure.spec.test$check_1
invokeStatic
"test.clj"
334]
[clojure.spec.test$check$fn__13433
invoke
"test.clj"
410]
[clojure.core$pmap$fn_9385$fn_9386
invoke
"core.clj"
6897]
[clojure.core$binding_conveyor_fn$fn__6772
invoke
"core.clj"
2020]
[clojure.lang.AFn
call
"AFn.java"
18]
[java.util.concurrent.FutureTask
run
"FutureTask.java"
266]
[java.util.concurrent.ThreadPoolExecutor
runWorker
"ThreadPoolExecutor.java"
1142]
[java.util.concurrent.ThreadPoolExecutor$Worker
run
"ThreadPoolExecutor.java"
617]
[java.lang.Thread
run
"Thread.java"
745]]},
:smallest [(#object[clojure.spec$fspec_impl$reify_14282$fn_14285
0x7cf479f8
"clojure.spec$fspec_impl$reify_14282$fn_14285@7cf479f8"])]}},
:sym spike-spec.core/bar,
:failure #error{:cause "Specification-based check failed",
:data {:clojure.spec/problems [{:path [:ret],
:pred true?,
:val false,
:via [],
:in []}],
:clojure.spec.test/args (#object[clojure.spec$fspec_impl$reify_14282$fn_14285
0x7cf479f8
"clojure.spec$fspec_impl$reify_14282$fn_14285@7cf479f8"]),
:clojure.spec.test/val false,
:clojure.spec/failure :check-failed},
:via [{:type clojure.lang.ExceptionInfo,
:message "Specification-based check failed",
:data {:clojure.spec/problems [{:path [:ret],
:pred true?,
:val false,
:via [],
:in []}],
:clojure.spec.test/args (#object[clojure.spec$fspec_impl$reify_14282$fn_14285
0x7cf479f8
"clojure.spec$fspec_impl$reify_14282$fn_14285@7cf479f8"]),
:clojure.spec.test/val false,
:clojure.spec/failure :check-failed},
:at [clojure.core$ex_info invokeStatic "core.clj" 4725]}],
:trace [[clojure.core$ex_info invokeStatic "core.clj" 4725]
[clojure.spec.test$explain_check
invokeStatic
"test.clj"
279]
[clojure.spec.test$check_call
invokeStatic
"test.clj"
295]
[clojure.spec.test$quick_check$fn__13414
invoke
"test.clj"
308]
[clojure.lang.AFn applyToHelper "AFn.java" 154]
[clojure.lang.AFn applyTo "AFn.java" 144]
[clojure.core$apply invokeStatic "core.clj" 657]
[clojure.core$apply invoke "core.clj" 652]
[clojure.test.check.properties$apply_gen$fn_1500$fn_1501
invoke
"properties.cljc"
16]
[clojure.test.check.properties$apply_gen$fn__1500
invoke
"properties.cljc"
16]
[clojure.test.check.rose_tree$fmap
invokeStatic
"rose_tree.cljc"
78]
[clojure.test.check.rose_tree$fmap
invoke
"rose_tree.cljc"
74]
[clojure.test.check.generators$fmap$fn__629
invoke
"generators.cljc"
89]
[clojure.test.check.generators$gen_fmap$fn__579
invoke
"generators.cljc"
55]
[clojure.test.check.generators$call_gen
invokeStatic
"generators.cljc"
41]
[clojure.test.check.generators$call_gen
invoke
"generators.cljc"
38]
[clojure.test.check$quick_check
invokeStatic
"check.cljc"
62]
[clojure.test.check$quick_check
doInvoke
"check.cljc"
37]
[clojure.lang.RestFn invoke "RestFn.java" 425]
[clojure.lang.AFn applyToHelper "AFn.java" 156]
[clojure.lang.RestFn applyTo "RestFn.java" 132]
[clojure.core$apply invokeStatic "core.clj" 657]
[clojure.spec.gen$quick_check
invokeStatic
"gen.clj"
27]
[clojure.spec.gen$quick_check doInvoke "gen.clj" 27]
[clojure.lang.RestFn applyTo "RestFn.java" 137]
[clojure.core$apply invokeStatic "core.clj" 661]
[clojure.spec.test$quick_check
invokeStatic
"test.clj"
306]
[clojure.spec.test$check_1
invokeStatic
"test.clj"
334]
[clojure.spec.test$check$fn__13433
invoke
"test.clj"
410]
[clojure.core$pmap$fn_9385$fn_9386
invoke
"core.clj"
6897]
[clojure.core$binding_conveyor_fn$fn__6772
invoke
"core.clj"
2020]
[clojure.lang.AFn call "AFn.java" 18]
[java.util.concurrent.FutureTask
run
"FutureTask.java"
266]
[java.util.concurrent.ThreadPoolExecutor
runWorker
"ThreadPoolExecutor.java"
1142]
[java.util.concurrent.ThreadPoolExecutor$Worker
run
"ThreadPoolExecutor.java"
617]
[java.lang.Thread run "Thread.java" 745]]}})



 Comments   
Comment by Alex Miller [ 12/Mar/17 7:22 PM ]

The generated function uses the ret spec to generate results so I don't this does not seem like a goal.





[CLJ-2121] Parameter names in docstring for `pos?` Created: 04/Mar/17  Updated: 05/Mar/17  Resolved: 05/Mar/17

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

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

Attachments: Text File 0001-CLJ-2121-update-docstring-to-reflect-param-name.patch    
Patch: Code
Approval: Triaged

 Description   

The docstring for `pos?` is as follows:

Usage: (pos? x)
Returns true if num is greater than zero, else false

This should be either:

Usage: (pos? num)
Returns true if num is greater than zero, else false

or

Usage: (pos? x)
Returns true if x is greater than zero, else false


 Comments   
Comment by Marc O'Morain [ 04/Mar/17 8:50 AM ]

(`neg?` and `zero?` have the same issue)

Comment by Alex Miller [ 04/Mar/17 10:58 AM ]

Patch welcome - change should update docstring, not arg.

Comment by Erik Assum [ 04/Mar/17 2:32 PM ]

Duplicate of CLJ-1859?

Comment by Alex Miller [ 05/Mar/17 7:08 PM ]

Closed as duplicate





[CLJ-2120] (for) works in REPL, but not in a file Created: 28/Feb/17  Updated: 28/Feb/17  Resolved: 28/Feb/17

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

Type: Defect Priority: Major
Reporter: Vadim Liventsev Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None
Environment:

elemenary OS 0.4 (Ubuntu 16.04.2 LTS / Linux 4.4.0-64-generic)
Java HotSpot(TM) 64-Bit Server VM 1.8.0_121-b13
Loading clojure 1.8.0 with Leiningen



 Description   

I have the following file:

for.clj
(println "for.clj loaded")
(for [fruit ["apple" "orange" "watermelon"]] (println fruit))

REPL is running in the same folder.
Calling

(for [fruit ["apple" "orange" "watermelon"]] (println fruit))
yields the expected result:

apple
orange
watermelon
(nil nil nil)

Calling

(load "for")
yields:

for.clj loaded
nil

The same happens when the file is loaded by Leiningen. It is also not limited to just (println), I have a project with a bunch of (for), it works in REPL, but grinds to a halt when loaded from a file.



 Comments   
Comment by Alex Miller [ 28/Feb/17 7:12 AM ]

for is lazy and won't evaluate its body unless something uses the resulting lazy seq. In the repl, the printing will do so but when you load, nothing will be doing the forcing.

You can use (doall (for ...)) to force the lazy seq around the for to be evaluated. Also see dorun and run! for some other fns that are used to force side effects.





[CLJ-2119] clojure.core/extends? inconsistent (erroroneous) between 1.7, 1.8/1.9.0-alpha14 Created: 28/Feb/17  Updated: 01/Mar/17  Resolved: 28/Feb/17

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

Type: Defect Priority: Major
Reporter: Tom S Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: bug, extends?
Environment:

windows 7/ 10
java version "1.8.0_111"
Java(TM) SE Runtime Environment (build 1.8.0_111-b14)
Java HotSpot(TM) 64-Bit Server VM (build 25.111-b14, mixed mode)



 Description   

Up to clojure 1.7, clojure.core/extends? served as a predicate as an alternative to the much much slower clojure.core/satisfies? pred for relatively quick type-based operations. I recently migrated to 1.8, and testing showed this no longer holds; clojure.core/extends? appears to be unable to detect that an object actually extends a protocol.

Simple example (1.7):

;user=> (defn ikv? [obj] (extends? clojure.core.protocols/IKVReduce (type obj)))
;#'user/ikv?
;user=> (ikv? {:a 2})
;true

Called against a clojure.lang.PersistentArrayMap, which implements IKVReduce in its java class definition, this makes sense.

The same example reports false on clojure 1.8 (and 1.9.0-alpha14).
However, clojure.core/satisfies? is true in 1.8 (and 1.9.0-alpha14).

My hacked workaround is to use Alex Miller's example of memoizing the call to satisfies? (slightly slower than extends? but reasonable for my use cases.

However, the fundamental behavior of extends? seems to be broken, which may be a deeper issue going forward. Plus, it's a otherwise "silent" error, since it happily returns false, leading to silent, possibly obscure runtime errors.



 Comments   
Comment by Alex Miller [ 28/Feb/17 7:30 AM ]

I think you're relying on implementation details that are subject to change.

extends? can only catch protocol extensions that happen in the type definition, not ones that are applied later. So checking a protocol with satisfies? is the correct way to do it.

That said, I'll double check and make sure nothing is amiss.

Comment by Alex Miller [ 28/Feb/17 8:50 AM ]

Yeah, this change in what you're seeing is due to the refactoring in https://github.com/clojure/clojure/commit/684cd4117bb2cf463c95293855a0dff52134bdcd. Again, the fact that extends? worked before was relying on unreliable details of the specific way things are implemented and what you're really trying to check is something about the protocol, which requires satisfies?.

Comment by Tom S [ 01/Mar/17 4:11 AM ]

I completely missed the fact that clojure.lang.IKVReduce popped up during
the refactor, and is used as a fast-path for classes that implement it
during kv-reduce.

I chased my tail trying to figure out why maps don't extend
clojure.core.protocols/IKVReduce, which is simply because the interface
clojure.core.protocols.IKVReduce is no longer implemented, leaving
isAssignAbleFrom reflection call to return false now, while providing a
protocol implementation for clojure.lang.IKVReduce. The protocol
implementation delegates to the class's implementation of
clojure.lang.IKVReduce if possible.

So, satisfies? traces through superclasses trying to find a protocol
implementation, which is far more durable (depending on the protocol)
rather than class-specific interface implementations I was using to
cheat out a fastpath.

Just for grins, here's the microbenchmark associated with the "fast path"
vs cached method lookup:
(def the-map {:a 2})

;;note the clojure.lang.KVReduce
(defn ikv? [x] (instance? clojure.lang.KVReduce x))

(let [cache (java.util.HashMap.)]
(defn sat? [protocol x]
(let [c (class x)]
(if-let [res (.get cache c)] ;we know.
res
(let [res (satisfies? protocol x)
_ (.put cache c res)]
res)))))

;;Microbenchmarks.

;;(time (dotimes [i 1000000] (sat? clojure.core.protocols/IKVReduce the-map)))
;;"Elapsed time: 13.31948 msecs"

;;(time (dotimes [i 1000000] (ikv? the-map))
;;"Elapsed time: 6.816777 msecs"

Negligible cost for caching (preferably using a concurrent map in production).

Thanks for tracking down the refactoring change. Sorry for the false alarm.

Comment by Tom S [ 01/Mar/17 4:43 AM ]

;typo correction.
(defn ikv? [x] (instance? clojure.lang.IKVReduce x))





[CLJ-2118] extend-type doesn't identify that a protocol is a protocol Created: 23/Feb/17  Updated: 23/Feb/17  Resolved: 23/Feb/17

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

Type: Defect Priority: Major
Reporter: Maurício Eduardo Chicupo Szabo Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: protocols
Environment:

Ubuntu 16.04 x86_64, OpenJDK 8



 Description   

I'm trying to implement this tutorial: http://www.lucagrulla.com/posts/server-sent-events-with-ring-and-compojure/

In one part, I need to extend a core.async type with the following protocol: https://github.com/ring-clojure/ring/blob/master/ring-core/src/ring/core/protocols.clj#L19.

So, I've tried to implement something like this:

src/async_test/protocol_ext.clj
(ns async-test.protocol-ext
  (:require [clojure.core.async :as async]
            [clojure.java.io :as io])
  (:import [ring.core.protocols StreamableResponseBody]
           [clojure.core.async.impl.channels ManyToManyChannel]))

(extend-type ManyToManyChannel
  StreamableResponseBody
  (write-body-to-stream [channel response output-stream]
                        (async/go (with-open [writer (io/writer output-stream)]
                                    (async/loop []
                                                (when-let [msg (async/<! channel)]
                                                  (doto writer (.write msg) (.flush))
                                                  (recur)))))))

But this fails with interface ring.core.protocols.StreamableResponseBody is not a protocol.

Then, I tried to monkey-patch ring: opened a new file with the correct path, and added the following lines on the bottom of the protocol declaration, inside `extend-protocol` call:

src/ring/core/protocols.clj
ManyToManyChannel
  (write-body-to-stream [channel response output-stream]
                        (async/go (with-open [writer (io/writer output-stream)]
                                    (async/loop []
                                                (when-let [msg (async/<! channel)]
                                                  (doto writer (.write msg) (.flush))
                                                  (recur)))))))

Then, it worked. What's happening? Is Clojure ignoring that StreamableResponseBody is a protocol if there's already an `extend-protocol` call?



 Comments   
Comment by Maurício Eduardo Chicupo Szabo [ 23/Feb/17 3:00 PM ]

Okay, my bad. Seems that I need to `:require` the protocol, not `:import` it. Sorry about that...

Comment by Alex Miller [ 23/Feb/17 3:04 PM ]

The issue here is that you are importing StreamableResponseBody as a Java class (really an interface). While the protocol does generate a Java interface, you should use the protocol, not the interface, when you extend. This is confusing because they both have the same name.

So, your ns declaration should instead be:

(ns async-test.protocol-ext
  (:require [clojure.core.async :as async]
            [clojure.java.io :as io]
						[ring.core.protocols :refer [StreamableResponseBody]])
  (:import [clojure.core.async.impl.channels ManyToManyChannel]))

The monkeypatch you tried worked because you were properly referring to the protocol in that case.





[CLJ-2117] Support typical polymorphic maps in spec more directly Created: 23/Feb/17  Updated: 23/Feb/17  Resolved: 23/Feb/17

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

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


 Description   

Problem: A common scenario is that one wants to spec this:

(def a-poly-map
  {::type :t1
   <kvs required/optional in t1>
   <kvs required/optional in all>
   })

To do this, you usually write lots of boiler plate code like this:

(s/def ::type #{:t1 <more-types>})

(defmulti type-dispatch ::type)
(defmethod type-dispatch :t1 [_] (s/keys :req [<t1-keys>]))
<more defmethods for more-types>

(s/def ::poly-map (s/merge (s/keys :req [::type <poly-map-keys-in-all>])
                           (s/multi-spec type-dispatch ::type))

As a workaround one can (and we did) of course write a macro to write all that code.

From my experience this usecase is the 90% multi-spec usecase and thus I'd prefer if a more convenient to use spec existed.

Proposed solution: Implement this spec:

(s/def ::poly-map (s/merge (s/keys :req [<poly-map-keys-in-all>]
                           (s/poly-map ::type
                             :t1 {:req [<t1-keys>]} ; :req, :opt etc. as in s/keys
                             <more requirements for more types>)))

Limitations: This syntax does not support to reuse existing specs in for dispatch values (:t1, :t2, ...) intentionally, because it seems an non-existing usecase to spec incomplete s/keys for such maps separately anyway.

Further enhancement: While the <poly-map-keys-in-all> usecase is rare, this syntax could also directly support it:

(s/def ::poly-map (s/poly-map ::type
                    {:req [<poly-map-keys-in-all>]} ; optional
                    :t1 {:req [<t1-keys>]}
                    <more requirements for more types>))

This new spec would automatically check for the presence of ::type and conform/explain like a regular s/keys.



 Comments   
Comment by Alex Miller [ 23/Feb/17 11:50 AM ]

Thanks, but I don't think we expect to do anything like this.

Comment by Leon Grapenthin [ 23/Feb/17 12:03 PM ]

Why?

Comment by Alex Miller [ 23/Feb/17 12:51 PM ]

Because spec provides the tools to do it already.

Comment by Leon Grapenthin [ 23/Feb/17 1:55 PM ]

This ticket is asking for a syntactic simplification of a typical use case. I understand that there already is s/multi-spec. s/multi-spec however is a generic and in comparison quite advanced tool that is not tied to maps. Thus it requires a substantial amount of boilerplate to "do it".

After much practical experience with spec in my observation - and I do assume others would agree - this is the most common application for it.

There are many things in clojure.core which aren't the only "tools to do it" for the exact same reason, so please reconsider not closing this right away.

Comment by Alex Miller [ 23/Feb/17 2:32 PM ]

My job is to weigh the balance of several factors and decide if an enhancement request seems reasonably likely to remain as an open ticket and at some point be further evaluated.

Some factors I try to think about:

  • Is this a good problem? Does it describe a pain point that many people are likely to encounter? You claim it is "most common" use but seems way too early to judge this (from what I've seen, I wouldn't agree right now).
  • Is the pain significant enough that it prevents you from doing your work? In this case, clearly not - you've already built it and it would be easy to release it independently from core.
  • Is this a fundamental (simple) feature that belongs in core? Clojure has a small library and wishes to keep it that way. There are ample examples where this constraint was not considered enough in Clojure. From what I can tell, this is appears to be a composite of existing things, not a fundamental part.
  • If we were to solve this problem in core, is this the proposed solution a likely way we would do it? Based on things we've talked about during dev, my suspicion is, probably not.

On balance, it does not seem to be something worth leaving open in the tracker right now. But as the saying goes, "no is temporary, yes is forever". If a while down the road it's clear that this is a prominent problem that needs attention, there is nothing preventing us from considering it again (presumably with more data and experience at that time).

Comment by Leon Grapenthin [ 23/Feb/17 3:11 PM ]

Thanks for explaining the decision. I understand that you have priorities and probably a better plan to address this problem in the future.





[CLJ-2114] ::defn-args spec incorrectly parses map body as a prepost rather than function body Created: 16/Feb/17  Updated: 14/Mar/17  Resolved: 14/Mar/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: Nicola Mometto Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: spec

Attachments: Text File 0001-CLJ-2144-conform-map-fn-bodies-as-body-rather-than-a.patch    
Patch: Code
Approval: Ok

 Description   

Reported by Claire Alvis in #clojure-spec:

user> (s/conform :clojure.core.specs/defn-args
                 '(foo [bar] {:baz 42}))
{:name foo, :bs [:arity-1 {:args {:args [[:sym bar]]}, :prepost {:baz 42}}]}

The current spec conforms function bodies with single maps as prepost conditions rather than function bodies, after the patch:

user=> (s/conform :clojure.core.specs/defn-args '(foo [bar] {:baz 42}))
{:name foo, :bs [:arity-1 {:args {:args [[:sym bar]]}, :body [:body [{:baz 42}]]}]}
user=> (s/conform :clojure.core.specs/defn-args '(foo [bar] {:baz 42} 1))
{:name foo, :bs [:arity-1 {:args {:args [[:sym bar]]}, :body [:prepost+body {:prepost {:baz 42}, :body [1]}]}]}

Patch: 0001-CLJ-2144-conform-map-fn-bodies-as-body-rather-than-a.patch

Screened by: Alex Miller



 Comments   
Comment by Nicola Mometto [ 16/Feb/17 5:42 PM ]

An open question is whether we also want to make `:prepost` stricter as part of this patch, so that it will ensure that `:pre` and `:post` are a collection





[CLJ-2113] Update Clojure maven for latest on CI server Created: 16/Feb/17  Updated: 14/Mar/17  Resolved: 14/Mar/17

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

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

Attachments: Text File mvn3.patch    
Patch: Code
Approval: Ok

 Description   

Update maven build infrastructure to stop using oss-parent and use updated Maven 3 and sonatype plugins (similar to other changes made recently in all contrib projects).

  • Removed oss-parent parent pom. This has been deprecated for years and is no longer recommended for use.
  • Add snapshot repo (was previously pulled in via oss-parent)
  • maven-compiler-plugin - update to latest version
  • maven-release-plugin - update to latest version
  • add nexus-staging-maven-plugin - current recommended plugin for releases to maven central, replaces most of the maven-release-plugin's work (old version of this previously in oss-parent)
  • add maven-gpg-plugin for signing (previously in oss-parent)
  • remove old release profile which was activated by oss-parent pom

Patch: mvn3.patch

It's difficult to test this completely outside the context of actually building and deploying snapshots and releases but the changes are very similar to those made for all contrib projects recently.






[CLJ-2110] sorted map returns nil value for existing key Created: 14/Feb/17  Updated: 14/Feb/17  Resolved: 14/Feb/17

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

Type: Defect Priority: Major
Reporter: jonnik Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: sorted-map
Environment:

Oracle Java 1.8.0_112



 Description   

Then i try to get value by key from sorted map with comparator by value it returns nil.

(def tmap {1 {:v 1} 2 {:v 2} 3 {:v 3}})
(def tmap-sorted (apply sorted-map-by #(let [val-comp (- (compare (get-in tmap [%1 :v]) (get-in tmap [%2 :v])))]
                                         (if (= val-comp 0) 1 val-comp))
                        (flatten (vec tmap))))
; => {3 {:v 3} 2 {:v 2} 1 {:v 1}}
(get tmap-sorted 3)
;=> nil

Expected: {:v 3}
Actual: nil



 Comments   
Comment by Andy Fingerhut [ 14/Feb/17 11:31 AM ]

Your comparison function never returns 0, by the way you have written it. If it never returns 0, it will never 'think' that it has found an equal element when searching for a match using 'get'. By replacing (if (= val-comp 0) 1 val-comp) with val-comp, the get calls will work:

(def tmap-sorted (apply sorted-map-by #(let [val-comp (- (compare (get-in tmap [%1 :v]) (get-in tmap [%2 :v])))]
                                         val-comp)
                        (flatten (vec tmap))))
tmap-sorted
; => {3 {:v 3}, 2 {:v 2}, 1 {:v 1}}
(get tmap-sorted 3)
; => {:v 3}

You are getting a descending sorted order by negating the return value of compare. I would recommend to follow the advice on this page: https://clojure.org/guides/comparators particularly "Reverse the sort by reversing the order that you give the arguments to an existing comparator." That helps avoid corner cases with some integer values.

I would also recommend (into my-empty-sorted-map tmap) in place of your (apply my-empty-sorted-map (flatten (vec tmap)). Putting all of those recommendations together would result in code like this:

(def tmap-sorted2 (into (sorted-map-by #(compare (get-in tmap [%2 :v]) (get-in tmap [%1 :v])))
                        tmap))
tmap-sorted2
; => {3 {:v 3}, 2 {:v 2}, 1 {:v 1}}
(get tmap-sorted2 3)
; => {:v 3}
Comment by saintech [ 14/Feb/17 11:41 AM ]

Ok. But how about this?:

tmap-sorted
; => {3 {:v 3} 2 {:v 2} 1 {:v 1}}
(first tmap-sorted)
; => [3 {:v 3}]
(get tmap-sorted 3)
;=> nil

Is it OK?

Comment by Andy Fingerhut [ 14/Feb/17 12:43 PM ]

I believe that calling clojure.core/first on a sorted-map does not cause its comparison function to be called at all. It is already stored in a sorted tree in memory, and first just finds the earliest one.

clojure.core/get does call the comparison function, perhaps several times, to find an item with an equal key. The original comparison function given in the description never returns equality (i.e. the integer 0) when comparing two items.

Comment by Alex Miller [ 14/Feb/17 1:32 PM ]

Agreed with Andy, declining





[CLJ-2108] Loading core specs affects startup time Created: 13/Feb/17  Updated: 06/Sep/17  Resolved: 06/Sep/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: Alex Miller
Resolution: Completed Votes: 0
Labels: spec

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

 Description   

Adding the loading of spec itself + the clojure.core.specs namespace containing specs for core makes start time worse (and will only get longer as we add more core specs).

$ export REPO=$HOME/.m2/repository
$ time java -cp $REPO/org/clojure/clojure/1.8.0/clojure-1.8.0.jar clojure.main -e nil
real	0m0.842s

$ time java -cp $REPO/org/clojure/clojure/1.9.0-alpha19/clojure-1.9.0-alpha19.jar:$REPO/org/clojure/spec.alpha/0.1.123/spec.alpha-0.1.123.jar:$REPO/org/clojure/core.specs.alpha/0.1.24/core.specs.alpha-0.1.24.jar clojure.main -e nil
real	0m1.286s

Proposed: Turn off macro spec checking during RT initialization. On first macroexpand after initialization, lazily load clojure.spec.alpha and clojure.core.specs.alpha.

Patch details:

  • RT - add flag CHECK_SPECS, initialized to false. Set flag after init is done.
  • RT.doInit() - Don't preemptively load clojure.spec.alpha or clojure.core.specs.alpha.
  • Compiler.macroexpand1() - when macroexpanding, call checkSpecs()
  • Compiler.checkSpecs() - if RT.CHECK_SPECS and not MACRO_CHECK_LOADING, then get cached var for clojure.spec.alpha/macroexpand-check and invoke it with the form to check the spec
  • Compiler.ensureMacroCheck() - if MACRO_CHECK (the cached var) is null, then enter the loading block and check that again (double-checked locking). Set the MACRO_CHECK_LOADING flag while loading (this prevents reentrant checking during loading - where spec itself uses macros that can trigger this code). Load clojure.spec.alpha and clojure.core.specs.alpha. Set the cached var.
  • Compile - used in the Clojure build process. The problem that can occur during building is that the lazy load of clojure.core.specs.alpha causes it to get compiled (CLJ-322) and this leads to class loading problems later in the test phase (via CLJ-1544 ish stuff). So instead, eagerly load clojure.core.specs.alpha before compilation starts. Maybe this should happen in compile or elsewhere instead?
  • clojure.main - binds clojure.spec.alpha/explain-out, which requires an explicit load of clojure.spec.alpha - this was previously relying on the implicit load in RT. This has its own performance implications during startup, but can address that separately.

With just this patch, the impact of avoiding the load of clojure.core.specs.alpha can be seen:

$ time java -cp $REPO/org/clojure/clojure/1.9.0-master-SNAPSHOT/clojure-1.9.0-master-SNAPSHOT.jar:$REPO/org/clojure/spec.alpha/0.1.123/spec.alpha-0.1.123.jar:$REPO/org/clojure/core.specs.alpha/0.1.24/core.specs.alpha-0.1.24.jar clojure.main -e nil
real	0m0.990s

However, additional changes are required to avoid loading spec.alpha entirely - that relies on other tickets:

  • CLJ-1891 - clojure.core.server is being unnecessarily loaded even when no servers are specified (this is due to changes in 1.8) which loads clojure.main, which loads clojure.spec.alpha
  • TODO - clojure.main only loads spec.alpha to bind clojure.spec.alpha/explain-out - there are a couple ways to potentially remove this linkage

Applying CLJ-1891 and removing the load of spec.alpha in clojure.main gave me about 0m0.830s for the call above for comparison.

Patch: clj-2108.patch



 Comments   
Comment by Ghadi Shayban [ 13/Feb/17 4:58 PM ]

Locally,
alpha14 takes 1.02s

time java -jar clojure-1.9.0-alpha14.jar -e ':foo'

Master with this line commented out takes 0.92s

time java -jar target/clojure-1.9.0-master-SNAPSHOT.jar -e ':foo'




[CLJ-2107] s/explain of a predicate defined s/with-gen yields s/unknown Created: 07/Feb/17  Updated: 08/Feb/17  Resolved: 08/Feb/17

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

Type: Defect Priority: Major
Reporter: Tamas Herman Assignee: Unassigned
Resolution: Duplicate Votes: 0
Labels: generator, spec


 Description   

This explanation is clear:

(s/def ::some-string string?)
(s/explain ::some-string 123)

val: 123 fails spec: :boot.user/some-string predicate: string?

However if the failing spec has a custom generator, the explanation is not so helpful:

(s/def ::some-string-with-custom-generator (s/with-gen string? gen/string-alphanumeric))
(s/explain ::some-string-with-custom-generator nil)

=> :boot.user/some-string-with-custom-generator
val: nil fails spec: :boot.user/some-string-with-custom-generator predicate: :clojure.spec/unknown

I would expect the same predicate: string? explanation.

Based on the symptom I suspect this issue is related to http://dev.clojure.org/jira/browse/CLJ-2068



 Comments   
Comment by Tamas Herman [ 08/Feb/17 2:21 AM ]

Explanation of a workaround from Slack from @hiredman:

with-gen is a function, so it's arguments are evaluated, so the string? argument to with-gen is a function object, when spec is trying to find the name to report it does some stuff, which for symbols and keywords reports a good name, but for other Objects (including function objects) you get :clojure.spec/unknown.

wrapping with s/spec allows the spec macro to capture the meaningful, the symbol before evaluation:

(s/def ::some-string-with-custom-generator (s/with-gen (s/spec string?) gen/string-alphanumeric))
(s/explain ::some-string-with-custom-generator nil)

val: nil fails spec: :boot.user/some-string-with-custom-generator predicate: string?

I don't see any simple solution to make the original example yield a good explanation,
so maybe we should just explain how it works in https://clojure.org/guides/spec#_custom_generators
and wrap the with-gen examples with spec.

Comment by Alex Miller [ 08/Feb/17 8:49 AM ]

As you mentioned, this is a dup of CLJ-2068 and is fixed by the patch there.





[CLJ-2106] satisfies? is quite slow when returning false Created: 06/Feb/17  Updated: 09/Feb/17  Resolved: 09/Feb/17

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

Type: Enhancement Priority: Minor
Reporter: Mike Kaplinskiy Assignee: Unassigned
Resolution: Duplicate Votes: 0
Labels: performance


 Description   
(defprotocol SatisfiesTest (testThing [this]))
(defrecord MyRecord [] Object SatisfiesTest (testThing [this]))
(def r (MyRecord.))

(time (dotimes [_ 30000] (instance? user.SatisfiesTest r)))
"Elapsed time: 1.715 msecs"

(time (dotimes [_ 30000] (satisfies? SatisfiesTest r)))
"Elapsed time: 3.944 msecs"

(time (dotimes [_ 30000] (instance? user.SatisfiesTest {})))
"Elapsed time: 2.304 msecs"

(time (dotimes [_ 30000] (satisfies? SatisfiesTest {})))
"Elapsed time: 718.949 msecs"

It would be nice if satisfies? memoized negative return values by class (though that cache would need to be cleared by `extend-type` and friends).



 Comments   
Comment by Alex Miller [ 06/Feb/17 1:56 PM ]

This is covered in an existing ticket - http://dev.clojure.org/jira/browse/CLJ-1814

Comment by Alex Miller [ 06/Feb/17 2:02 PM ]

Actually, read too quickly - CLJ-1814 covers the positive case only. I don't think we're going to cache a negative return value though. Managing and cleaning that cache is likely to not be worth it. If you need this kind of thing, you should probably consider a different kind of conditional check before-hand.

Comment by Alex Miller [ 06/Feb/17 2:05 PM ]

For example, you can just memoize this call like this:

user=> (def check (memoize #(satisfies? SatisfiesTest %)))
#'user/check
user=> (time (dotimes [_ 30000] (check {})))
"Elapsed time: 3.512 msecs"
Comment by Alex Miller [ 06/Feb/17 2:13 PM ]

I tweaked the test to remove the use of range and the construction of the record so the numbers are more useful.

Comment by Nicola Mometto [ 08/Feb/17 6:52 PM ]

Alex Miller CLJ-1814 handles the negative cases too, and doesn't keep a cache of negative return values. It keeps a cache of all the protocols extended by a certain class and invalidates that cache on every call to extend, the benchmarks on the ticket description showcase both a positive and a negative test

Comment by Alex Miller [ 09/Feb/17 3:09 PM ]

Nicola - cool! I didn't realize that. Will mark this as a dupe then.





[CLJ-2104] Typo in pprint docstring Created: 29/Jan/17  Updated: 06/Sep/17  Resolved: 06/Sep/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: docstring, typo

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

 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-2101] Request for a way to forget specs Created: 24/Jan/17  Updated: 25/Jan/17  Resolved: 24/Jan/17

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

Type: Enhancement Priority: Trivial
Reporter: Johan Gall Assignee: Unassigned
Resolution: Duplicate Votes: 0
Labels: spec


 Description   

Hello,

I have a server that loads code, and specs, at runtime in generated namespaces.

Thus I have a spec leak.

It would be nice if there was an official API to forget specs.



 Comments   
Comment by Johan Gall [ 24/Jan/17 7:01 AM ]

(or better, a way to create a temporary spec world, but then that is probably not trivial)

Comment by Alex Miller [ 24/Jan/17 7:51 AM ]

Is this covered by http://dev.clojure.org/jira/browse/CLJ-2060 ?

Comment by Alex Miller [ 24/Jan/17 12:51 PM ]

Marking as a dup for now, but will re-open if this goes beyond what's in CLJ-2060.

Comment by Johan Gall [ 25/Jan/17 5:20 AM ]

ah sorry, indeed it's a dup





[CLJ-2100] s/nilable form should include the original spec, not the resolved spec Created: 19/Jan/17  Updated: 14/Mar/17  Resolved: 14/Mar/17

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

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

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

 Description   
=> (clojure.spec/def :test/name string?)
:test/name

=> (clojure.spec/form (clojure.spec/and :test/name))
(clojure.spec/and :test/name)

=> (clojure.spec/form (clojure.spec/nilable :test/name))
(clojure.spec/nilable clojure.core/string?)

In the final line, s/nilable form has the resolved spec rather than the original spec.

Proposed: Instead of getting the internal spec description, resolve the original spec form.

After:

user=> (clojure.spec/form (clojure.spec/nilable :test/name))
(clojure.spec/nilable :test/name)

Patch: CLJ-2100.patch






[CLJ-2099] Keywords with aliased namespaces cannot be read when the namespace is required in a reader conditional Created: 13/Jan/17  Updated: 25/Jan/17  Resolved: 13/Jan/17

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

Type: Defect Priority: Minor
Reporter: Viktor Magyari Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: keywords, reader, readerconditionals


 Description   

The title in itself isn't entirely true, but I couldn't find a way to describe it succintly (feel free to change).

The issue is easier to demonstrate with an example:

(ns foo
  #?(:cljs (:require [clojure.core :as c])))

#?(:cljs ::c/x)

When reading this in a :clj context, the reader cannot read ::c/x ("Invalid token: ::c/x"), despite the code being correct (presumably).
The same thing happens if the reader conditional branches are :clj and the source is read in a :cljs context.



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

This looks like expected behavior to me. Auto-resolved keywords rely on a resolution context and there isn't one at the point where ::c/x is being read.

Comment by Viktor Magyari [ 13/Jan/17 9:05 AM ]

To me it seems reasonable to expect the resolution context to include the clojure.core alias - more generally, include <platform> specific aliases in the <platform> branches of reader conditionals. Maybe consider this as an enhancement ticket?

Comment by Alex Miller [ 13/Jan/17 9:18 AM ]

To do this would require adding special handling specifically for ns or other things that create aliases, which implies conditional evaluation of some forms at read-time. You would also need some place (other than the current platform's namespace maps) to track other platform's namespace aliases. That's a lot of very special, custom stuff.

We're not going to add this.

Comment by Leon Grapenthin [ 25/Jan/17 2:13 PM ]

1. Why does the reader try to read the :cljs branch in Viktors example?
2. The original intent of reader conditionals was that the code could be read independently of the lang. Have aliases not been considered then due to a lack of popularity of ::a/n syntax?

My suggestion would be a knob that reads unresolvable alias kws as a special object #aliased-keyword{:alias "a", :name "n"}. This would solve both Viktors problem and also pay its debt to the reader conditional intent.

Comment by Alex Miller [ 25/Jan/17 5:16 PM ]

1. The reader reads everything, it's just selective about which parts of an expression gets returned. Without reading, it wouldn't know where the end of that form was.

2. I think this is a little more subtle in that reading the second thing in a conditional requires remembering something read and discarded in a prior conditional.

That said, maybe it would be possible to either read in a mode where this particular case doesn't throw or where this particular exception was known and discarded when inside a conditional.





[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-2094] clojure.spec: bug with protocols and extend-type Created: 01/Jan/17  Updated: 03/Jan/17  Resolved: 03/Jan/17

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

Type: Defect Priority: Major
Reporter: John Schmidt Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: spec


 Description   

I have the following two clj files (I've tried to come up with a minimal example):

core.clj
--------------
(ns spec-test.core
  (:require [clojure.spec :as s]))

(defprotocol Game
  (move [game]))

(s/def ::game1 #(satisfies? Game %))
(s/def ::game2 (partial satisfies? Game))

foo.clj
--------------
(ns spec-test.foo
  (:require [spec-test.core :refer [Game]]))

(defrecord Foo [])

(extend-type Foo
  Game
  (move [game]))

Here's a REPL session that explains my problem:

user=> (ns spec-test.core)
nil
spec-test.core=> (require 'spec-test.core :reload)
nil
spec-test.core=> (require 'spec-test.foo :reload)
nil
spec-test.core=> (satisfies? Game (spec-test.foo/->Foo))
true
spec-test.core=> ((partial satisfies? Game) (spec-test.foo/->Foo))
true
spec-test.core=> (s/explain ::game1 (spec-test.foo/->Foo))
Success!
nil
spec-test.core=> (s/explain ::game2 (spec-test.foo/->Foo))
val: #spec_test.foo.Foo{} fails spec: :spec-test.core/game2 predicate: (partial satisfies? Game) <---- WAAAAAT
nil

I would expect ::game1 and ::game2 to be equivalent, but somehow they're not.

Also see: https://groups.google.com/forum/#!topic/clojure/igBlMpqGU3A



 Comments   
Comment by Steve Miner [ 02/Jan/17 12:43 PM ]

I gave some incorrect advice on the mailing list so I'll try to correct myself here. Basically, the protocol is stored in a var, Game. If a predicate captures a particular state of the protocol, it won't respond correctly when additions are made to the protocol (with extend-type, etc.) So the problem with the usage of `partial` is that it evaluates the current value of the protocol at that point in time, before it has been extended to cover Foo.

The #(...) function definition would have used the var itself, not its current value. Naturally, the var allows the protocol to be updated such that the function sees the updated value. Basically, this is just normal Clojure evaluation. A `defn` style function would have worked fine as well. It's just the `partial` that evaluated its args that leads to the problem.

The same kind of issue could come up if you passed any var to partial and captured the current value of the var. Later changes to the var would not affect the result of partial.

I'll say the bug is the confusing error message. It seems that s/explain is implying it is using the var Game, but it really captured an "old" value of Game.

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

I think Steve's assessment is all correct.

I'm not sure that it's possible for spec to know what's happened here though wrt giving a better error message.

I don't think I really see anything that can be "fixed", so I'm going to mark this as declined.





[CLJ-2093] partial and fn behave differently with eval Created: 28/Dec/16  Updated: 31/Dec/16  Resolved: 31/Dec/16

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

Type: Defect Priority: Major
Reporter: N/A Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None


 Description   

(eval (list map (partial + 1) [0]))
CompilerException java.lang.ExceptionInInitializerError
(eval (list map (fn [x] (+ 1 x)) [0]))
=> (1)



 Comments   
Comment by Kevin Downey [ 29/Dec/16 9:22 PM ]

same issue as http://dev.clojure.org/jira/browse/CLJ-1206 and all the issues connected to that

Comment by Alex Miller [ 31/Dec/16 11:17 AM ]

You should not expect either of these to work as the expressions contain function objects (not function forms).

You should be doing something like this:

(eval (list 'map '(partial + 1) [0]))




[CLJ-2091] clojure.lang.APersistentVector#hashCode is not thread-safe Created: 24/Dec/16  Updated: 12/May/17  Resolved: 12/May/17

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

Type: Defect Priority: Major
Reporter: thurston n Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: collections, concurrency

Attachments: File clj-2091-0.diff     File clj-2091-default-initialization.diff    
Patch: Code
Approval: Ok

 Description   

Problem: clojure.lang.APersistentVector#hashCode contain a deliberate data race on hash computation. However, the code as written does not follow safe practices for the intended data race. Specifically, the problem arises because the hashCode() (and hasheq()) method make multiple reads of the (unsynchronized) _hash field. The JMM permits these reads to return different values. Specifically, the last read in the return may return the pre-computed value -1, which is not the desired hash value. This problem also applies to APersistentMap, APersistentSet, and PersistentQueue.

See: http://jeremymanson.blogspot.com/2008/12/benign-data-races-in-java.html for a good description of the problem.

Fix: The main fix is to read the cached hash field only once and return the value of the local computation, not the value of the field.

A secondary change that is also beneficial is to use the default initializer value (which has special ordering in the JMM to the beginning of the thread) rather than setting and using -1 as the sentinel value.

In both cases these changes follow the canonical idioms used in java.lang.String for lazy hash computation. The patch covers both.

Patch: clj-2091-default-initialization.diff - note that this patch will indicate whitespace errors when applied due to the wacky line endings in PersistentQueue. The problem here is really the PQ formatting, not the patch.

Prescreened by: Alex Miller

There are some hash-related tests already but I also spot-checked that hash computations are returning the same value with and without the patch for the collections in question.



 Comments   
Comment by thurston n [ 24/Dec/16 4:38 PM ]

I can of course provide a patch, but as this is my first issue and am generally unfamiliar with clojure's practices and because this issue is not restricted to APersistentVector#hashCode, I thought it best to hold off and let the stewards decide how to proceed and how I could best help

Comment by Alex Miller [ 31/Dec/16 12:47 PM ]

Patch welcome (but please sign the contributor's agreement first - http://clojure.org/community/contributing). Also, see the processes for developing patches at http://dev.clojure.org/display/community/Developing+Patches.

AFAICT, the affected classes are APersistentVector, APersistentMap, APersistentSet, and PersistentQueue? Would be ok by me to cover all of them in a single patch.

Comment by thurston n [ 03/Jan/17 4:00 PM ]

AFAICT, the affected classes are APersistentVector, APersistentMap, APersistentSet, and PersistentQueue?

  • Dunno. However my experience tells me that the broken idiom (racy cache/memoization) is likely elsewhere; but I know of no systematic way of finding them. Regardless, I'll just focus on those 4 classes.
  • My plan is to also fix #hasheq(). It's the same problem; if you don't want that then just let me know and I'll refrain.
  • I'm not planning to deal with the initialization of #_hash and #_hasheq (currently inline-initialized to -1); that's a separate (although related) thread-safety problem. Might they be just what we refer to as a "legacy idiosyncrasy"? If so, then they really should be changed to just be default-initialized. I did, as an experiment, change one to default initialization, and the tests passed - that should be enough, but given that the persistent classes are serializable, code-coverage, et al., I can't say for sure. So I'll leave it to others more familiar with the codebase to make that determination. I note that if it is in future determined to change them, then #hashCode() and #hasheq() will need to be modified (trivially) accordingly.

That's the plan. Sound good?

Comment by Alex Miller [ 03/Jan/17 9:49 PM ]

I thought the non-default initialization was part of what you were describing, so now I'm not sure we're on the same page. Maybe you can just patch one so we have something concrete to talk about.

Comment by thurston n [ 03/Jan/17 9:56 PM ]

I'm not sure what you mean by "patch one" - I just submitted a patch, did you look at that?

Comment by Alex Miller [ 03/Jan/17 10:13 PM ]

I meant one class - sorry, I didn't see the patch. I will look at it tomorrow with fresh eyes.

Comment by thurston n [ 03/Jan/17 11:02 PM ]

Sure.

To be clear, as I mentioned in today's earlier comment, I would advise removing the inline-initialization, viz.

int _hash = -1;
int _hasheq = -1;

with

int _hash;
int _hasheq;

As I wrote, the extant tests would pass (of course, changing #hashCode() and #hasheq() appropriately)

But the initialization issue is a different, although certainly not orthogonal, issue than the one my patch addresses.

Currently, (i.e. pre-patch), #hashCode() can return a spurious -1 even if an APersistentVector instance is safely published - my patch fixes that.

However, because of the inline-initialization, an APersistentVector instance that is not safely published could return a spurious 0 from #hashCode(), even with my patch.

Now if the inline-initialization is just a "legacy idiosyncrasy" (and we all do that at one time or another), then it could be safely replaced (along with the appropriate modification to my patch) and all APersistentVector instances (safely published or not), would have #hashCode() implementations that are correct.

Comment by Alex Miller [ 06/Jan/17 3:19 PM ]

Ok, I went over all this again and it makes sense to me. I think you should proceed and also make the initializer change (remove the -1 as sentinel and replace with no initializer and 0 for the comparison checks in the methods).

Comment by thurston n [ 06/Jan/17 11:36 PM ]

Combines the 2 commits into a single commit patch
So incorporates the original patch changes (single read) with default initialization and checks for zero
Don't know what to do with PersistentQueue's mixed line-endings – that I'll leave to you to deal with

Comment by thurston n [ 09/Jan/17 8:16 PM ]

Problem also in core.lang.ASeq#hashCode() and core.lang.ASeq#hasheq() - although thankfully without inline initialization

Surely not the last place either

Comment by Alex Miller [ 10/Jan/17 8:33 AM ]

Feel free to update the patch if you like





[CLJ-2088] 'into' can make maps from collection of lists, but vectors are ok. Created: 19/Dec/16  Updated: 20/Dec/16  Resolved: 19/Dec/16

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

Type: Defect Priority: Major
Reporter: Eugene Aksenov Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: bug
Environment:

OS: Ubuntu 16.04, x86_64, Linux 4.2.0-42-generic



 Description   

'into' can make maps from collection of lists, but vectors are ok.

Good behavior:
(into {} [[:a "a"] [:b "b"]])
;;=> {:a "a", :b "b"}

(into {} '([1 "a"] [2 "b"]))
;;=> {1 "a", 2 "b"}

Bad examples:
(into {} ['(:a "a") '(:b "b")])
;;=> ClassCastException clojure.lang.Keyword cannot be cast to java.util.Map$Entry clojure.lang.ATransientMap.conj (ATransientMap.java:44)

(into {} [(list [:a "a"]) [:b "b"]])
;;=> ClassCastException clojure.lang.PersistentVector cannot be cast to java.util.Map$Entry clojure.lang.ATransientMap.conj (ATransientMap.java:44)

(into {} ['(1 "a") '(2 "b")])
;;=> ClassCastException java.lang.Long cannot be cast to java.util.Map$Entry clojure.lang.ATransientMap.conj (ATransientMap.java:44)

(into {} '('(1 "a") '(2 "b")))
;;=> ClassCastException clojure.lang.Symbol cannot be cast to java.util.Map$Entry clojure.lang.ATransientMap.conj (ATransientMap.java:44)



 Comments   
Comment by Alex Miller [ 19/Dec/16 11:53 AM ]

into is built on conj (takes a sequential collection of elements to invoke with conj). conj for maps takes a map entry - a subclass of java.util.Map$MapEntry or a 2-element vector. Both of these choices can access keys and vals directly without "traversing" the entry (going through the key to get to the value).

We don't want to support that for performance reasons, so lists or seqs or not valid as map entries (and conj on a map does not support them).

into takes a sequential collection of these entries though, so vector, or list, or seq are all fine as the src collection for into.

So, all of this is working as intended and I am declining the ticket.

Comment by Eugene Aksenov [ 20/Dec/16 2:09 AM ]

Ok, live and learn
I've just added this code case and brief explanation to clojuredocs.org. Hope no one will be confused anymore.





[CLJ-2087] map does not work with core.async/<! Created: 17/Dec/16  Updated: 17/Dec/16  Resolved: 17/Dec/16

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

Type: Defect Priority: Major
Reporter: Roy Varghese Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None


 Description   

The construct
(map <! [chan1 chan2 chan3])

does not work for reasons outlined here https://github.com/clojure/core.async/wiki/Go-Block-Best-Practices

This leads to very obscure bugs.

If this cannot be fixed in map, please consider throwing an exception when it cannot handle the passed function.



 Comments   
Comment by Alex Miller [ 17/Dec/16 5:08 PM ]

It's unlikely that we're going to "fix" this as it part of the design of Clojure.

When I try your example I see:

AssertionError Assert failed: <! used not in (go ...) block




[CLJ-2086] A macro in a nested syntax-quote causes an exception Created: 15/Dec/16  Updated: 03/Jan/17  Resolved: 15/Dec/16

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

Type: Defect Priority: Major
Reporter: N/A Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None


 Description   

(defmacro foo
[x y]
``(~~x ~~y))

(foo and true)

CompilerException java.lang.RuntimeException: Can't take value of a macro: #'clojure.core/and



 Comments   
Comment by Alex Miller [ 15/Dec/16 10:15 PM ]
user=> (macroexpand '(foo and true))
(clojure.core/seq (clojure.core/concat (clojure.core/list and) (clojure.core/list true)))

What do you expect this to do?

Comment by Alex Miller [ 15/Dec/16 10:36 PM ]

Declining till more info about a) what problem you are trying to solve and b) what the expected behavior is.

Comment by N/A [ 22/Dec/16 8:30 PM ]

a) I'm trying to define a macro that defines a macro.
(defmacro make-defmacro
[macro-name macro]
`(defmacro ~macro-name
x# & more#
(do (~macro x#)
`(~~macro-name ~@more#))))

(make-defmacro bar and)
CompilerException java.lang.RuntimeException: Can't take value of a macro

b) (defmacro foo
[x y]
``(~~x ~~y))

(macroexpand '(foo and true))
=> (and true)

Comment by Viktor Magyari [ 03/Jan/17 10:52 AM ]

Try

(defmacro foo [x y]
  ``(~'~x ~~y))




[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-2084] Support MapEquivalence for defrecords Created: 13/Dec/16  Updated: 13/Dec/16  Resolved: 13/Dec/16

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

Type: Enhancement Priority: Major
Reporter: Herwig Hochleitner Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: defrecord


 Description   

defrecord equality implies matching types. This has been discussed and decided upon such that people should only bother with defrecords when they want an equality partition. There are, however, use cases for defrecords as small structmaps to save memory and those require map-based equivalence. Clojure has a marker interface for map equivalence: clojure.lang.MapEquivalence, which gets used in = to decide whether something qualifies as a persistent map. This can be reused to change the equiv implementation of a defrecord.

Current Behavior

When MapEquivalence is implemented in a defrecord, = becomes non-reflexive

(= {} (->Rec)), (not= (->Rec) {})

Proposed Behavior

Add clojure.lang.MapEquivalence to implemented interfaces in a defrecord, to treat it as a PersistenMap in equality, disregarding the type tag.
APersistentMap.equiv already does this, defrecord implementation can call APersistentMap/mapEquals



 Comments   
Comment by Alex Miller [ 13/Dec/16 10:27 AM ]

Wouldn't this be a breaking change for anyone relying on records to compare not equals based on type?

Comment by Herwig Hochleitner [ 13/Dec/16 4:35 PM ]

The key phrase is "Add clojure.lang.MapEquivalence to implemented interfaces". Any existing defrecords would not be affected. Au contraire, having a defrecord implement MapEquivalence right now results in a non-reflective-equals clojure-gigo scenario, so it's completely uncharted territory and ripe for definition. Sorry for not being clearer in the description, would you consider reopening?

Comment by Alex Miller [ 13/Dec/16 5:15 PM ]

It's not uncharted territory. Records intentionally compare different for equality based on type.

If you don't want that, don't use records. Or pour your record into a map before comparing via into or select-keys.

Comment by Herwig Hochleitner [ 13/Dec/16 5:51 PM ]

Yes, and I'm not proposing to change a single bit of that, agreed?
I am proposing the possibility to have specific defrecords implement map-like equality by using an option that a) naturally fits with what's already there b) compiles but yields GIGO right now:

(defrecord Rec []
  clojure.lang.MapEquivalence)

How is this already charted? If it is, then this ticket needs to be refiled as a Defect, because then the current behavior makes no sense.

So, if you're not declining this on technical grounds, what is it then? If it is not wanting to add use cases to defrecords for philosophical reasons, you're using the term "breaking change" very loosely here. There can't be any existing expectations on defrecords implementing MapEquivalence right now, because such defrecords are invalid programs right now.
Are you arguing some sort of "general expectation" on defrecords? One that would stand without looking at specific definitions? What use would that be to anybody. Who works with a defrecord without having a general idea of what it definition is?

Comment by Herwig Hochleitner [ 13/Dec/16 6:19 PM ]

Just to be clear, I did my due diligence on this and I'm aware that the current equality situation for records is completely intentional, hence I'm proposing growing it. The argument might still be that we don't want to grow it in that particular direction, but it's definitely not breakage.

Comment by Alex Miller [ 13/Dec/16 6:34 PM ]

I thought you were asking to add MapEquivalence to all records, which would be a breaking change.

The docstring for defrecord says:

{pre}
In addition, defrecord will define type-and-value-based =,
and will defined Java .hashCode and .equals consistent with the
contract for java.util.Map.{pre}

The important thing there is "type-and-value-based", and that's part of the design of records. Asking for MapEquivalence on a map means value-based only. These two things are in conflict and thus I would agree that asking for MapEquivalence on a record makes no sense.

deftype exists so that you can make your own types with whatever equality/hash/behavior you want - if you want something like this, then build it on top of deftype.

Comment by Herwig Hochleitner [ 13/Dec/16 6:41 PM ]

yeah, changing defrecord to implement MapEquivalence by default would make no sense.

However, declaring a (specific) defrecord to be in the MapEquivalence partition makes perfect sense, doesn't it?
That way, we can type our records an structmap them too.

Comment by Herwig Hochleitner [ 13/Dec/16 6:49 PM ]

Anyway, I'll build this on deftype for data.xml in any case, because we want this to work on current and older clojure versions. I just thought it would fit neatly with clojure itself (since there is already a marker interface for that purpose, only its use currently leads to breakage) and wanted to make you aware of the idea. I'll recycle it through the mailing list, as soon as the deftype based thing is in data.xml





[CLJ-2078] Add a maven profile to connect with an nrepl client Created: 07/Dec/16  Updated: 07/Dec/16  Resolved: 07/Dec/16

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

Type: Enhancement Priority: Trivial
Reporter: Herwig Hochleitner Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: development

Attachments: Text File clj-2078-0.patch    
Patch: Code

 Description   

Opening an nrepl port into a project is a popular development strategy with clojure projects. It should also work with the clojure project.

Maven profiles make it possible to add development tools without affecting the rest of the build.

clojure-maven-plugin has this tool built right in as the clojure:nrepl goal and cider provides middlewares for many advanced ide functionalities.

Attached patch provides and documents a -P cider profile that lets you jack into a clojure source checkout.



 Comments   
Comment by Herwig Hochleitner [ 07/Dec/16 7:38 PM ]

Assuming that non-contrib covered code is OK for optional dev tools.

Comment by Alex Miller [ 07/Dec/16 8:44 PM ]

Thanks, I don't think we're going to do this.





[CLJ-2077] Clojure can't be loaded from the boot classpath under java 9 Created: 06/Dec/16  Updated: 18/Sep/17  Resolved: 18/Sep/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: Toby Crawley Assignee: Unassigned
Resolution: Completed Votes: 6
Labels: java9

Attachments: Text File clj-2077-2.patch     Text File clj-2077-3.patch     Text File clj-2077-4.patch     Text File CLJ-2077.patch    
Patch: Code
Approval: Ok

 Description   

As part of the changes for the jigsaw module system in Java 9, the
java packages available to the boot classloader are now a subset of
the the full java distribution. This means that classes loaded via the
boot classloader cannot access any classes outside of that subset.

In the boot classloader only the java.base module is available. For a complete list of module/package listings see http://cr.openjdk.java.net/~mr/jigsaw/ea/module-summary.html

Clojure itself uses java.sql.Timestamp in clojure.instant to provide print-method and print-dup implementations for java.sql.Timestamp.

This can be seen with (using Clojure 1.4.0 or higher, and a early-access build
of Java 9, most recently tested with 9-ea+147):

java -Xbootclasspath/a:clojure.jar clojure.main -e "(require 'clojure.instant)"

This affects any clojure-based tool that puts itself on the boot
classpath in order to gain a startup time boost (both lein
and boot are affected currently).

Proposed: If java.sql.Timestamp is not available, do not load instant.clj or install it in the default data readers.

Patch: clj-2077-4.patch

Screener Notes: This looks correct and does not break normal usage. Should be tested in the bootclasspath scenarios people have problems with.



 Comments   
Comment by Toby Crawley [ 06/Dec/16 12:34 PM ]

More details on the underlying change that is triggering this are available at http://openjdk.java.net/jeps/261 (search for java.sql to find the relevant section).

Comment by Alex Miller [ 06/Dec/16 8:41 PM ]

Does this need to be a ticket here? Or is this really an issue for build tools?

Comment by Toby Crawley [ 08/Dec/16 4:30 PM ]

That depends on if we want using Clojure from the boot classpath to be an acceptable use case. If not, then I agree, it is just an issue for tooling.

Comment by Toby Crawley [ 09/Dec/16 2:21 PM ]

I realized today that this issue doesn't actually affect boot, since it doesn't use the bootclasspath. So lein is the only tooling I know of that is affected by this.

Comment by Ivan Pierre [ 12/Dec/16 4:59 AM ]

https://docs.oracle.com/javase/8/docs/api/java/sql/Timestamp.html
Would it be possible to use java.util.Date instead. Alas it's not possible to downcast :
Due to the differences between the Timestamp class and the java.util.Date class mentioned above, it is recommended that code not views Timestamp values generically as an instance of java.util.Date. The inheritance relationship between Timestamp and java.util.Date really denotes implementation inheritance, and not type inheritance.

Comment by Ivan Pierre [ 12/Dec/16 8:22 AM ]

The problem with Date is that it doesn't deal with nanoseconds. But Timestamp is created by a long giving the TimeDate value in milliseconds.
The use of setNano and getNano are only to store the SQL TIMESTAMP. Wouldn't it be better to deal with this value in another way?

The other way is to take just what we need from TimeStamp, and it's just a little thing, I'll try to compile with that to see if some other thing comes after...

Test code : https://gist.github.com/ivanpierre/b0ea937dac97d910a7c3c1e5774028e0

Comment by Ivan Pierre [ 12/Dec/16 1:13 PM ]

Ok, I pass to the GNU version of Timestamp. The code is neater. I mixed some of Sun's for more consistency. I dropped the string management of dates as Clojure will do it in clojure.instant.

It still works. I had a doubt...

If I type (clojure.lang.TimeStamp. 3678141) the response will be :
==> #inst "1970-01-01T01:01:18.141000000-00:00" with a nano of
141000000

But is if set nano to 1 : (.setNanos (clojure.lang.TimeStamp. 3678141) 1) the response is : #inst "1970-01-01T01:01:18.000000001-00:00"

This is correct, but it's a little disturbing to see my nice .141 disappear...

I put a fork on my GitHub. Last commit : https://github.com/ivanpierre/clojure/commit/749a0184ee7409290dad9ff353605fcaabd64f69

So, good, now pass to Leinigen...

Comment by Alex Miller [ 13/Dec/16 10:32 AM ]

I think the first question here is: do we expect that Clojure should be loadable from the bootclasspath?

While I realize this is a hack people use, my initial answer would be no (that was never a design constraint afaik). But will need to defer to Rich on that.

Comment by Ivan Pierre [ 13/Dec/16 10:41 AM ]

Well Leiningen do it, even with the lein repl. I test to see if TimeStamp is the only thing.
Alas Leinigen is not 1.9 compatible, so I have to go down to version 1.8 to make the tests. (problem of conflict between clojure.spec and hara library)
A funny thing would be to pass the whole Clojure test in bootstrap so we would know...

Comment by Ivan Pierre [ 13/Dec/16 1:06 PM ]

The java Timestamp could be directly integrated into clojure.instant as it's a new datatype. So no need to worry about copyright stuff, and integer it in a complete manner to accept SQL TIMESTAMP, and some correct protocols.
The worst is that looking across the DBs documentation doesn't help a lot and some are quite contradictory.

Comment by Phil Hagelberg [ 03/Apr/17 1:05 PM ]

> I think the first question here is: do we expect that Clojure should be loadable from the bootclasspath?

I don't know if it's the right thing for Clojure to be able to be loaded from the bootclasspath, but as an additional point of data: Leiningen takes 2.5 seconds to load on my machine from the bootclasspath on Java 8. Without the bootclasspath, it takes 3.4 seconds on Java 8 and 4.5 seconds on Java 9. (This is just for one Clojure instance, not launching a project subprocess.) So in this case it nearly doubles the time, which is consistently in the top 5 complaints about working with Clojure.

I would not be surprised if other Clojure-based tools (IDEs/editors perhaps) would want to use the bootclasspath but I don't have any data on that. I know there are people using the bootclasspath for production servers, but they probably wouldn't be as upset about adding a few seconds as people using it on their own machine.

I would be happy to write the few lines of code needed to defer all references to the inaccessible classes until runtime if it's decided that's the way to go.

Comment by Ghadi Shayban [ 03/Apr/17 3:13 PM ]

Phil - would you mind sharing your environment and testing code? I see a slowdown without bootclasspath but not nearly as dramatic as what you see. The Java 9 builds you are using probably have extra debugging enabled.

Comment by Phil Hagelberg [ 25/Apr/17 10:08 PM ]

Ghadi: I'm running on a Zulu build of Java 9 since I wasn't able to find binaries for Debian of the official OpenJDK ones. It's possible there's extra debugging overhead, but it's more likely it's just because I'm running on old hardware.

$ java -version
openjdk version "9-ea"
OpenJDK Runtime Environment (Zulu 9.0.0.10-linux64) (build 9-ea+155-pre-release-eval)
OpenJDK 64-Bit Server VM (Zulu 9.0.0.10-linux64) (build 9-ea+155, mixed mode, pre-release-eval)

Comment by Alex Miller [ 25/Apr/17 10:52 PM ]

I'm moving this into Clojure 1.9. Not implying any decisions, just so we look at it before any 1.9 release decisions.

Comment by Alex Miller [ 12/May/17 3:25 PM ]

Would like to see a patch for this that makes loading/definition of things not in java.base conditional such that this would not fail. If someone wants to work on that, would be happy to see it. If not, I will look at it eventually.

Comment by Ghadi Shayban [ 17/Aug/17 5:20 PM ]

Adding a patch that conditionally loads clojure.instant

test with:

java -Xbootclasspath/a:$PWD/target/clojure-1.9.0-master-SNAPSHOT.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
Comment by Rich Hickey [ 06/Sep/17 7:10 AM ]

Could we please get a function or macro that encapsulates the check/catch so we don't have N of these to maintain should there eventually be a better way?

Comment by Alex Miller [ 06/Sep/17 3:50 PM ]

Updated patch to encapsulate class existence check in "when-class" (and changed one existing usage of same pattern).

Comment by Alex Miller [ 08/Sep/17 10:19 AM ]

Added -3 patch that applies cleanly to latest master.

Comment by Alex Miller [ 08/Sep/17 12:51 PM ]

whoops, swapped the data readers in -3. fixed in -4.

Comment by Andrew Rosa [ 08/Sep/17 6:45 PM ]

Please correct me if I'm wrong, but as far as I can understand the issue is only related to code}}java.sql.Timestamp{{/code. The path removes the whole "instant.clj" file, but only a small portion of it is related to the problematic class.

Wouldn't be better to split it apart and conditionally load only the code}}java.sql.Timestamp{{/code portion of the code, like is already done with 1.8's code}}java.time.Instant{{/code on "code_instant18.clj"?

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

Hey Andrew, this is possible, and I went down this road a bit, but in the end I did not find the complexity of splitting it to be worth the effort. While you can work around the constructor-side of the reader parts as Timestamp is not used by default there, the Timestamp use is squarely in the middle of print support. A tagged literal that you can read but not print is of limited use so it seemed best to just omit the entire tag.

Comment by Alex Miller [ 18/Sep/17 1:33 PM ]

Released in 1.9.0-beta1.





[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-2071] Unexpected behavior with clojure.spec/tuple and clojure.spec.test/instrument Created: 01/Dec/16  Updated: 01/Dec/16  Resolved: 01/Dec/16

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

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

Clojure 1.9, JDK 8



 Description   

It looks like stest/instrument is comparing a sequence of actual args to a vector created by spec/tuple and it doesn't match, however clojure.spec.test/instrument appears to work fine. Reading the clojure.spec guide I would think the two approaches would be equivalent.

(ns sample
  (:require [clojure.spec.test :as stest]
            [clojure.spec :as spec]))

    (defn myinc [i]
      (inc i))

    (spec/fdef myinc
               :args (spec/tuple integer?) ; Fails with the error below
               ; :args (spec/cat :i integer?) ; This works
               :ret integer?)

    (stest/instrument `myinc)
    (myinc 1)
    ; clojure.lang.ExceptionInfo: Call to #'specific.core-spec/myinc did not conform to spec:
    ; val: (1) fails at: [:args] predicate: vector?
    ; :clojure.spec/args  (1)
    ; :clojure.spec/failure  :instrument
    ; :clojure.spec.test/caller  {:file "core_spec.clj", :line 28, :var-scope specific.core-spec/fn--6046}


 Comments   
Comment by Ben Rady [ 01/Dec/16 8:11 AM ]

Note that the namespace in this example was changed. It used to be specific.core-spec, which is shown in the error.

Comment by Alex Miller [ 01/Dec/16 10:22 AM ]

Spec will create a list or a seq of the args for checking the :args spec. Tuples can only be used on vectors (because they match by index). So, this is not currently expected to work. It is recommended that you use a regex spec (s/cat) instead.





[CLJ-2070] Faster clojure.core/delay implementation Created: 29/Nov/16  Updated: 06/Sep/17  Resolved: 06/Sep/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: dennis zhuang Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: performance
Environment:

macOS Sierra
intel Core i7 2.5G Hz
Memory 16GB

java version "1.8.0_66"
Java(TM) SE Runtime Environment (build 1.8.0_66-b17)
Java HotSpot(TM) 64-Bit Server VM (build 25.66-b17, mixed mode)


Attachments: Text File fast_delay_with_volatile_fn2.patch    
Patch: Code and Test
Approval: Ok

 Description   

clojure.lang.Delay uses a synchronized lock to protect the deref method, because it must make sure the Delay object is realized exactly once.

As we known synchronized lock plays worse performance under heavy concurrency. Instead, using volatile and double-checking lock in this situation improves the performance. The benchmark code is at test-delay.clj. The benchmark-delay function accepts thread count number as an argument, and it will start as many threads as the argument to access delay object concurrently as in (time (benchmark-delay 1)).

threads 1.9.0-alpha14 + patch % better
1 0.570196 ms 0.499905 ms 12
10 19.66194 ms 1.313828 ms 93
20 40.740032 ms 2.149794 ms 95
100 184.041421 ms 8.317295 ms 95

Patch: fast_delay_with_volatile_fn2.patch
Prescreened by: Alex Miller



 Comments   
Comment by Alex Miller [ 29/Nov/16 8:52 AM ]

A faster version of delay would be helpful - these are used extensively inside spec so would actually help the average case spec performance.

These whitespace errors need to be cleaned up...

$ git apply ~/Downloads/fast_delay.patch
/Users/alex/Downloads/fast_delay.patch:67: trailing whitespace.
	                try
/Users/alex/Downloads/fast_delay.patch:105: trailing whitespace.

/Users/alex/Downloads/fast_delay.patch:115: space before tab in indent.
        	    (fn []
/Users/alex/Downloads/fast_delay.patch:116: space before tab in indent.
          		    (.await barrier)
/Users/alex/Downloads/fast_delay.patch:117: space before tab in indent.
          		    (dotimes [_ 10000]
warning: squelched 8 whitespace errors
warning: 13 lines add whitespace errors.

More importantly, the double-check is on fn, so it's critical that fn is marked as volatile. You should re-run the perf test afterwards.

Comment by dennis zhuang [ 29/Nov/16 9:13 AM ]

Sorry, white spaces errors should be fixed before my attached.

But the fn doesn't need to be marked as volatile, because it's protected by synchronized in all blocks. And writing it to be null is fine here.

fn=null;

It's not like double-checking in singleton example, there is no reordering here.

Comment by Alex Miller [ 29/Nov/16 9:25 AM ]

fn is read at the top before the synchronized block - it needs to be volatile or one thread may not see the write inside the synchronized block from another thread.

Comment by dennis zhuang [ 29/Nov/16 9:41 AM ]

Yep ,but it's fine here.
If one thread can't see the writing null for fn at the top, it will enter the locking block.
The double-checking on fn!=null will make sure the fn is called at most once, and if the fn was called by another thread and was set to be null ,then current thread will fail on second checking on fn!=null and exit the locking to go on reading value or exception.

So, in the worst situation, maybe two or more threads would enter the locking block,but they all will fail on second checking on fn!=null except one thread of them success.

I don't want to declare fn to be volatile, because volatile also has a cost on reading. The fn variable may be flushed into main memory too late, but it's acceptable and safe here, and we avoid the cost of volatile reading.

Comment by Alex Miller [ 29/Nov/16 9:45 AM ]

I think you're wrong, and I'm not screening it without it being marked as volatile.

Comment by dennis zhuang [ 29/Nov/16 9:54 AM ]

The patch which mark fn volatile.

Comment by dennis zhuang [ 29/Nov/16 9:54 AM ]

The patch which does't mark fn volatile.

Comment by dennis zhuang [ 29/Nov/16 9:59 AM ]

Hi, alex.

I understand your opinion here. Though i still insist that fn doesn't need to be marked as volatile, but it's not a critical concern here.

I uploaded two patches, one is marked fn volatile, the other is not. All of them fixed the whitespace errors and update the benchmark result in ticket description.

Thanks.

Comment by dennis zhuang [ 29/Nov/16 10:15 AM ]

Rebase master.

Comment by Nicola Mometto [ 30/Nov/16 11:53 AM ]

dennis, here's an article describing why fn needs to be volatile: https://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html

Comment by dennis zhuang [ 30/Nov/16 6:01 PM ]

@Nicola

I knew the double-checking issue in old JVM memory model, but it is different here.
There is no instance constructed, it's only assigning fn to be null, so it doesn't have instruments reordering. And we don't have a partial constructed instance that escapes.

But it's not critical concern here, it seems that volatile doesn't compact performance of this patch.

Thanks.





[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-2062] Spec import and refer-clojure macros Created: 17/Nov/16  Updated: 14/Mar/17  Resolved: 14/Mar/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: core.specs, spec

Attachments: Text File import-referclj-2.patch    
Patch: Code
Approval: Ok

 Description   

Add specs for import and refer-clojure.

Patch:

  • Fixes some indentation of previous specs
  • Factors out ::filters spec from ::ns-refer-clojure
  • Factors out ::import-list from ::ns-import
  • Reuses ::filters in ::ns-refer
  • Reuses ::filters in ::use-prefix-list
  • Removes :ret any? in ::ns-use (no need for it)
  • Adds clojure.core/import spec
  • Adds clojure.core/refer-clojure spec

Patch: import-referclj-2.patch






[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-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-2058] s/keys doesn't check non-keyword elements in :req and :req-un vectors Created: 15/Nov/16  Updated: 15/Nov/16  Resolved: 15/Nov/16

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

Type: Defect Priority: Minor
Reporter: Eugene Pakhomov Assignee: Unassigned
Resolution: Not Reproducible Votes: 0
Labels: spec


 Description   

As can be seen here https://github.com/clojure/clojure/blob/master/src/clj/clojure/spec.clj#L430 the s/keys macro filters out any non-keyword element from :req and :req-un before checking it, but later it still uses them in the arguments to map-spec-impl.
Compare the behavior when passing non-keyword elements to :opt and :opt-un.



 Comments   
Comment by Alex Miller [ 15/Nov/16 9:29 AM ]

:req and :req-un support `and` and `or` connectives, :opt and :opt-un do not. So this all seems right to me.

I don't see any bug here?

Comment by Eugene Pakhomov [ 15/Nov/16 9:58 AM ]

I see your point. But how are `and` and `or` related to "all keys must be namespace-qualified keywords"?
And why to do the check at all for :opt and :opt-un that are not even used, when the check for :req and :req-un just allows any forms, not just documented `and` and `or`?

Comment by Alex Miller [ 15/Nov/16 10:08 AM ]

and and or are connectors, not keys.

I'd be fine if the ticket showed something invalid - no actual bug is being shown here. If you improve the ticket, I will re-open.





[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-2056] Efficient shortcut for (first (filter pred coll)) idiom Created: 11/Nov/16  Updated: 12/May/17  Resolved: 12/May/17

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

Type: Enhancement Priority: Major
Reporter: Nikita Prokopov Assignee: Unassigned
Resolution: Declined Votes: 39
Labels: None

Attachments: Text File clj-2056-clojure-core-seek-2.patch    
Patch: Code and Test

 Description   

It's a common task to look up for an item in a collection based on predicate. Currently Clojure has no direct support for that in clojure.core. Instead, our options are:

1. (first (filter pred coll)) will create intermediate lazy sequence and might evaluate pred up to 31 extra times in case of chunked sequence

2. (some #(when (pred %) %) coll) will short-circuit on first match, but won't catch false value in something like (some #(when false? %) [true false true])

Additionally, both of these workarounds a) obscure the purpose of the code, and b) do not handle custom not-found values.

Attached is a patch that makes use of efficiency of reduce-able collections, handles edge cases like looking for false? or nil?, and supports optional not-found value.

Examples:

(seek odd? (range)) => 1
(seek pos? [-1 1]) => 1
(seek pos? [-1 -2] ::not-found) => ::not-found
(seek nil? [1 2 nil 3] ::not-found) => nil

Patch: clj-2056-clojure-core-seek-2.patch

Prescreening notes: I think the general approach is good. Is it necessary to support nil? and false? preds? Or would a transduce formulation like the one in comments be sufficient.



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

Just as an interesting aside, the new halt-when transducer could actually be used to create something like this too (if you set aside the desire to support nil? and false? preds).

(transduce (comp (filter pred) (halt-when any?)) identity nil coll)

Patch has some trailing whitespace in the test code - could you clean that up?

Comment by Nikita Prokopov [ 12/Nov/16 3:46 AM ]

Attaching patch with trailing whitespace cleaned

Comment by Nikita Prokopov [ 12/Nov/16 3:46 AM ]

Thanks Alex! Attached new patch with whitespace cleaned

Comment by Moritz Heidkamp [ 30/Jan/17 12:34 PM ]

I had a similar train of thought today and arrived at the idea of adding transducer support to first, e.g. (first (filter pred) coll). This could potentially be as efficient as the special-purpose seek function proposed above but would of course not support the not-found value. However, it seems like a natural extension to first and could be useful for other purposes, too.

Comment by Alex Miller [ 12/May/17 3:34 PM ]

Upon review, we've decided that we do not wish to include this. Use of linear search (and in particular nested linear search) leads to poor performance - often it's better to use other kinds of data structures and that's why this functionality has not been included in the past.





[CLJ-2055] binding-form spec parses symbol-only maps incorrectly Created: 08/Nov/16  Updated: 14/Mar/17  Resolved: 14/Mar/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: Brandon Bloom Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: spec
Environment:

1.9.0-alpha14


Attachments: Text File CLJ-2055-01.patch    
Patch: Code
Approval: Ok

 Description   

The :clojure.core.specs/binding-form spec incorrectly treats some maps as sequential bindings.

Actual:

user=> (s/conform :clojure.core.specs/binding-form '{x y})
[:seq {:elems [[:seq {:elems [[:sym x] [:sym y]]}]]}]

Expected:

user=> (s/conform :clojure.core.specs/binding-form '{x y})
[:map {x y}]

Cause:

When there is no :keys, :strs, or :syms from :clojure.core.specs/map-special-binding, then :clojure.core.specs/seq-binding-form treats a map as sequential.

Proposed fix:

Include an (s/and vector? ...) check. See patch.

Patch: CLJ-2055-01.patch
Screened by: Alex Miller






[CLJ-2052] .class vs .clj isn't picked correctly when either .class or .clj are not in a jar Created: 03/Nov/16  Updated: 04/Nov/16  Resolved: 04/Nov/16

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

Type: Defect Priority: Major
Reporter: Mike Kaplinskiy Assignee: Unassigned
Resolution: Not Reproducible Votes: 0
Labels: None


 Description   

The code that figures out the last modification date (https://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/RT.java#L389) uses URLConnection.getLastModified. For `file://` URLs this always returns 0.

One of the resulting bugs: when both .class and .clj files are present on a directory-based classpath, .clj files are always preferred, regardless of modification time.



 Comments   
Comment by Alex Miller [ 03/Nov/16 6:15 PM ]

Can you provide OS and JVM info? This might be env-specific.

Comment by Mike Kaplinskiy [ 03/Nov/16 6:20 PM ]

Sure - I'm on macOS Sierra on Oracle Java 8:

$ java -version
java version "1.8.0_74"
Java(TM) SE Runtime Environment (build 1.8.0_74-b02)
Java HotSpot(TM) 64-Bit Server VM (build 25.74-b02, mixed mode)
$ uname -a
Darwin mikekap-mbp.local 16.1.0 Darwin Kernel Version 16.1.0: Thu Oct 13 21:26:57 PDT 2016; root:xnu-3789.21.3~60/RELEASE_X86_64 x86_64 i386 MacBookPro11,5 Darwin
Comment by Alex Miller [ 03/Nov/16 6:28 PM ]

It's prob not too fun but some example code would be awfully handy.

Comment by Mike Kaplinskiy [ 03/Nov/16 7:13 PM ]

Sorry this looks like it was my fault - the path I was looking at when testing didn't exist (seems .getLastModified always returns 0 on those instead of throwing an exception).

Sorry for the noise.

Comment by Alex Miller [ 04/Nov/16 9:26 AM ]

Closed per comments.





[CLJ-2051] Typo in clojure.instant/validated docstring Created: 03/Nov/16  Updated: 06/Sep/17  Resolved: 06/Sep/17

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

Type: Defect Priority: Trivial
Reporter: Greg Leppert Assignee: Unassigned
Resolution: Completed Votes: 1
Labels: docstring, typo

Attachments: Text File 58f6bca6ba64ca343b43585463eff1be74aeb965.patch    
Patch: Code
Approval: Ok

 Description   
  • "Return a function which constructs and instant by calling constructor
    + "Return a function which constructs an instant by calling constructor

Prescreened by: Alex Miller






[CLJ-2048] java.lang.ClassCastException: [Ljava.lang.Object; cannot be cast to [Ljava.lang.StackTraceElement Created: 21/Oct/16  Updated: 06/Sep/17  Resolved: 06/Sep/17

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

Type: Defect Priority: Major
Reporter: Gerrit Jansen van Vuuren Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJ-2048-b.patch    
Patch: Code
Approval: Ok

 Description   

clojure.core/throw-if creates an array to call Exception.setStracktrace() without specifying the array type. This works fine when passed at least one StackTraceElement, but in the case where passed no stack trace elements (all are filtered or stack traces are being elided by the JVM), this will be an Object[] which results in a ClassCastException:

Exception in thread "main" java.lang.ClassCastException: [Ljava.lang.Object; cannot be cast to [Ljava.lang.StackTraceElement;
        at clojure.core$throw_if.invokeStatic(core.clj:5649)
        at clojure.core$load_one.invokeStatic(core.clj:5698)
        at clojure.core$compile$fn__5682.invoke(core.clj:5903)
        at clojure.core$compile.invokeStatic(core.clj:5903)
        at clojure.core$compile.invoke(core.clj:5895)
        at clojure.lang.Var.invoke(Var.java:379)
        at clojure.lang.Compile.main(Compile.java:67)

This is tricky to reproduce because it involves stack trace filtering so there is no reproducing case here.

Patch: CLJ-2048-b.patch
Prescreened by: Alex Miller



 Comments   
Comment by Gerrit Jansen van Vuuren [ 21/Oct/16 4:18 AM ]

patch calls into-array with StackTraceElement type

Comment by Alex Miller [ 21/Oct/16 8:01 AM ]

How do you cause this to occur?

Comment by Alex Miller [ 21/Oct/16 8:11 AM ]

into-array will create a typed array based on the first element of the seq it is passed, so generally you should see a StackTraceElement[] here. I think the only time this would fail is if it was passed no stack trace elements.

Comment by Alex Miller [ 21/Oct/16 8:19 AM ]

I'd be happy to move this through screening, but the patch needs to be in the proper format (see http://dev.clojure.org/display/community/Developing+Patches).

Comment by Gerrit Jansen van Vuuren [ 21/Oct/16 8:42 AM ]

I'm trying to reproduce this in a way that can be presented here, but I got the compile error just after doing some serious package renaming, and can't reproduce it outside of the project itself.

Comment by Gerrit Jansen van Vuuren [ 21/Oct/16 8:45 AM ]

ok, I'll reformat the patch after reading (http://dev.clojure.org/display/community/Developing+Patches)

Comment by Gerrit Jansen van Vuuren [ 21/Oct/16 9:15 AM ]

I've created a new patch based on the guidelines, attached as file: CLJ-2048-b.patch.

Just to summarise:
The into-array returns the correct type if its provided with a none empty sequence, but if the sequence is empty it cannot know the type and then returns an object array. Because we set the array here to a java method Exception::setStackTrace passing it an object array causes a ClassCastException. One fix is to check for an empty sequence, but a less verbose way is just to pass the type which is known as part of the call to into-array, this is what is done in the patch CLJ-2048-b.patch.





[CLJ-2045] bean function not working Created: 17/Oct/16  Updated: 17/Oct/16  Resolved: 17/Oct/16

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

Type: Defect Priority: Major
Reporter: Roy Varghese Assignee: Unassigned
Resolution: Duplicate Votes: 0
Labels: None
Environment:

(defproject hello-jcloud "0.1.0-SNAPSHOT"
:description "FIXME: write description"
:url "http://example.com/FIXME"
:license {:name "Eclipse Public License"
:url "http://www.eclipse.org/legal/epl-v10.html"}
:dependencies [#_[org.clojure/clojure "1.8.0"]
[org.clojure/clojure "1.9.0-alpha13"]
[org.clojure/tools.logging "0.1.0"]
[org.apache.jclouds/jclouds-all "2.0.0-SNAPSHOT"]]
:repositories {"jclouds-snapshots" "https://repository.apache.org/content/repositories/snapshots"})



 Description   

user> (bean (java.util.Date.))
UnsupportedOperationException empty clojure.core.proxy$clojure.lang.APersistentMap$ff19274a.empty (:-1)
user>

It works when clojure/1.8.0 is uncommented.



 Comments   
Comment by Alex Miller [ 17/Oct/16 7:38 AM ]

This is a dupe of http://dev.clojure.org/jira/browse/CLJ-2027 which will probably be in the next alpha.





[CLJ-2043] s/form of conformer is broken Created: 14/Oct/16  Updated: 14/Mar/17  Resolved: 14/Mar/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: 3
Labels: spec

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

 Description   

s/form of s/conformer is wrong:

(s/form (s/conformer str))
=> str

Proposed: Fix the form for conformer to match the conformer call.

Patch: clj-2043.patch






[CLJ-2042] s/form of s/? does not resolve pred Created: 14/Oct/16  Updated: 28/Oct/16  Resolved: 28/Oct/16

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

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

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

 Description   
user=> (require '[clojure.spec :as s])
nil
user=> (s/form (s/? int?))
(clojure.spec/? int?)

Patch: clj-2042.patch






[CLJ-2039] typo in deftype doc string Created: 09/Oct/16  Updated: 06/Sep/17  Resolved: 06/Sep/17

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

Type: Enhancement Priority: Trivial
Reporter: Steve Miner Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: docstring, typo

Attachments: Text File CLJ-2039-deftype-typo.patch    
Patch: Code
Approval: Ok

 Description   

The doc string for deftype refers to "record class" where it should say "type".






[CLJ-2035] Bad s/form for collection specs Created: 07/Oct/16  Updated: 14/Mar/17  Resolved: 14/Mar/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: Johan Gall Assignee: Alex Miller
Resolution: Completed Votes: 6
Labels: spec

Attachments: Text File clj-2035-2.patch     Text File clj-2035.patch    
Patch: Code and Test
Approval: Ok

 Description   

There are several problems with s/form for collection specs (coll-of,map-of,every,every-kv):

1. coll spec forms expose implementation details of building on every:

(s/form (s/coll-of int?))
=> (clojure.spec/every int? :clojure.spec/cpred #object[user$eval16$fn__18 0xd506900 "user$eval16$fn__18@d506900"] :clojure.spec/kind-form nil :clojure.spec/conform-all true)

2. form does not resolve nested spec preds:

(s/def ::a (s/every (s/tuple ::b)))

(s/form ::a)
=> (clojure.spec/every (*s/tuple* :user/b) [ ... ])

(which impacts map-of and coll-of).

3. :kind fn is not resolved

(s/form (s/coll-of int? :kind vector?))
=> (clojure.spec/every int? :clojure.spec/cpred #object[user$eval4$fn__6 0x8fc095 "user$eval4$fn__6@8fc095"] :clojure.spec/kind-form clojure.core/vector? :kind #object[clojure.core$vector_QMARK___6428 0x6596f6ef "clojure.core$vector_QMARK___6428@6596f6ef"] :clojure.spec/conform-all true)

Ignoring the rest of the problems from #1, the :kind should be here but should be the resolved form (clojure.core/vector?).

Patch: clj-2035-2.patch



 Comments   
Comment by Johan Gall [ 10/Mar/17 3:58 PM ]

Thanks a lot!





[CLJ-2034] comment macro doesn't ignore namespace keyword Created: 04/Oct/16  Updated: 04/Oct/16  Resolved: 04/Oct/16

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

Type: Defect Priority: Minor
Reporter: Nuttanart Pornprasitsakul Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None


 Description   

In fresh started REPL:

user=> (comment (x/y))
nil
user=> (comment ::x/y)
RuntimeException Invalid token: ::x/y  clojure.lang.Util.runtimeException (Util.java:221)
RuntimeException Unmatched delimiter: )  clojure.lang.Util.runtimeException (Util.java:221)


 Comments   
Comment by Alex Miller [ 04/Oct/16 11:37 PM ]

This code is not valid because x is an invalid alias (not bound to anything).

This works for example:

user=> (alias 'x 'clojure.string)
nil
user=> (comment ::x/y)
nil

comment still reads the code, so code that is unable to be read is still invalid.

Comment by Alex Miller [ 04/Oct/16 11:38 PM ]

Note that CLJ-2030 would make this code work (by auto-creating x as a new namespace).





[CLJ-2033] s/conformer conversion loss in (very common) s/merge edge case Created: 04/Oct/16  Updated: 04/Oct/16  Resolved: 04/Oct/16

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

Type: Defect Priority: Major
Reporter: Alexander Kahl Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: spec
Environment:

1.9.0-alpha14



 Description   

When using s/conform on a spec generated by s/merge that uses non-namespaced keys from s/keys, keys that use a custom s/conformer lose their conversion.

Steps to reproduce:

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])
(defn convert [n] (if (double? n) n (double n)))
(s/def ::value (s/conformer convert))
(s/conform (s/keys :req [::value]) {::value 5}) => #:user{:value 5.0} ; correct
(s/conform (s/keys :req-un [::value]) {:value 5}) => {:value 5.0} ; correct
(s/conform (s/merge (s/keys :req [::value]) (s/keys)) {::value 5}) => #:user{:value 5.0} ; correct
(s/conform (s/merge (s/keys :req-un [::value]) (s/keys)) {:value 5}) => {:value 5} ; WRONG

While this seems like an edge case, it is very likely to occur since using s/merge with unqualified keys is a typical use case for configuration files. I first spotted this behavior in alpha13 and it still occurs in alpha14.



 Comments   
Comment by Alex Miller [ 04/Oct/16 8:36 AM ]

alpha14 isn't out yet, so thanks for traveling back in time to report this!

I think this is not a bug, just a subtlety in how merge conform works. Specifically, merge does not flow conformed results, you only get the conformed result of the last spec in the merge.

In this case:

(s/conform (s/merge (s/keys :req [::value]) (s/keys)) {::value 5})

the spec determining the conform value is (s/keys), which will conform all namespaced keys, including ::value.

In this case:

(s/conform (s/merge (s/keys :req-un [::value]) (s/keys)) {:value 5})

the spec determining the conform value is also (s/keys), but s/keys only conforms namespaced keys and there aren't any here, so you get the original map.

If you want the conformed value, you can swap the order in the merge because the first spec is conforming the unqualified key:

(s/conform (s/merge (s/keys) (s/keys :req-un [::value])) {:value 5})
;;=> {:value 5.0}
Comment by Alexander Kahl [ 04/Oct/16 8:50 AM ]

Oh geez, I meant 13 (and first observed in 12), wish I could actually travel back in time!

If what you're saying is right, why doesn't this work, as both (s/keys) use only unqualified keys?

(s/def ::other string?)
(s/conform (s/merge (s/keys :req-un [::value]) (s/keys :req-un [::other])) {:value 5 :other "5"}) => {:value 5, :other "5"}
Comment by Alexander Kahl [ 04/Oct/16 9:08 AM ]

I just read again your comment Alex Miller and finally started to understand how (s/keys) conforms all namespaced keys, so please disregard my other comment.
I still wish there was a solution that worked for multiple maps of unqualified keys. Otherwise, I'd have to expect my users to use qualified keys throughout configuration files which looks a lot like redundancy, unless I convert all the keys first.





[CLJ-2032] Confusing error conforming fspec with missing arg spec Created: 03/Oct/16  Updated: 28/Oct/16  Resolved: 28/Oct/16

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

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

1.9.0-alpha13


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

 Description   
(require '[clojure.spec :as s])
(def my-spec (s/fspec :ret string?))
(s/conform my-spec (fn [j] (str j)))
IllegalArgumentException No implementation of method: :specize* of protocol: #'clojure.spec/Specize found for class: nil  clojure.core/-cache-protocol-fn (core_deftype.clj:568)
	clojure.core/-cache-protocol-fn (core_deftype.clj:583)
	clojure.spec/fn--13560/G--13555--13569 (spec.clj:121)
	clojure.spec/specize (spec.clj:138)
	clojure.spec/gensub (spec.clj:262)
	clojure.spec/gen (spec.clj:275)
	clojure.spec/gen (spec.clj:275)
	clojure.spec/validate-fn (spec.clj:1664)
	clojure.spec/fspec-impl/reify--14270 (spec.clj:1686)
	clojure.spec/conform (spec.clj:150)
	clojure.spec/conform (spec.clj:146)

Proposed: When conforming, throw if no args spec specified for the fspec:

Can't conform fspec without args spec: (fspec :args nil :ret string? :fn nil)

Alternatives

  • absence of args spec always conforms ::invalid
  • absence of args spec always conforms any args
  • disallow fspecs without args (still potentially useful for other uses like documentation, so not sure we want to do that)

Patch: clj-2032.patch






[CLJ-2030] Auto-create alias namespaces Created: 28/Sep/16  Updated: 10/Mar/17  Resolved: 10/Mar/17

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

Type: Enhancement Priority: Major
Reporter: Herwig Hochleitner Assignee: Unassigned
Resolution: Declined Votes: 8
Labels: keywords, namespace, portability, spec

Attachments: Text File clj-2030-2.patch     Text File clj-2030-3-1.patch     Text File clj-2030-3-2.patch     Text File clj-2030.patch    
Patch: Code and Test
Approval: Triaged

 Description   

It is useful to name keywords in namespaces, without creating or requiring those namespaces. When wanting to do that via an ::alias/keyword, the aliased namespace has to actually exist, in order to be aliased.
Currently, in Clojure, this can be achieved dynamically, through a combination of create-ns and alias, Clojurescript requires a dummy file and a :require :as.

Proposals:

1. Extend clojure.core/alias to auto-create missing namespaces
2. Extend clojure.core/alias to accept varargs & {:as kvs}
3. Extend ns to accept (:alias ...) clauses

Patch:

  • clj-2030.patch does 1 (but not 2 or 3), and was screened by SDH
  • clj-2030-2.patch does 1+2 (but not 3)
  • clj-2030-3-1.patch does 1+3 (but not 2)
  • clj-2030-3-2.patch does 1+2+3

Before:

user=> (alias 'parts 'company.domain.parts)
java.lang.Exception: No namespace: company.domain.parts found

After:

user=> (alias 'parts 'company.domain.parts)
nil
user=> ::parts/widget
:company.domain.parts/widget


 Comments   
Comment by Alex Miller [ 28/Sep/16 10:04 PM ]

From original description:

My use case is a simplification of data.xml, which would benefit greatly from a uniform way to alias + auto-create namespaces within the ns clause.

I would like to support the syntax:

(ns foo.bar
  (:alias xh  #xml/ns "http://..<xhtml>.."
          svg #xml/ns "http://..<svg>.."))

{:tag ::xh/div
 :content [{:tag ::svg/g}]}

see https://github.com/bendlas/data.xml/commit/22cbe21181175d302c884b4ec9162bd5ebf336d7

Comment by Alex Miller [ 28/Sep/16 10:08 PM ]

Thanks for filing this, it is something we've looked at a bit already. I simplified the description a bit and moved the use case and syntax to comments. I don't really understand the ns :alias example given in your syntax proposal but I think it's very unlikely we would go that far.

Comment by Herwig Hochleitner [ 29/Sep/16 3:55 AM ]

My syntax example could already be implemented, if alias had the proposed behavior and was available in an ns clause. In the linked commit, I implemented a scheme to encode xml namespaces in clojure namespaces, by using percent-encoding. I could easily provide that reader tag, if clojure and clojurescript provided the proposed extensions to alias and ns.

Comment by Alex Miller [ 29/Sep/16 8:53 AM ]

Yeah, I get it now (sleep!). I think the particular example is distracting to understand the enhancement request though.

Comment by Alex Miller [ 18/Oct/16 9:13 AM ]

moving this to vetted just so we don't lose track of it, but Rich has not actually ok'ed this for 1.9 yet

Comment by Herwig Hochleitner [ 06/Dec/16 10:50 PM ]

added patches for 3

Comment by Alex Miller [ 10/Mar/17 8:07 AM ]

Based on conversations to date, we don't intend to make this change, although there are some other ideas in work.

Comment by Alex Miller [ 10/Mar/17 8:13 AM ]

See CLJ-2123 for tracking of alternative.

Comment by Brandon Bloom [ 10/Mar/17 7:07 PM ]

Could you share some design insights please?

My initial thought is that auto-creating a namespace would interact poorly with requiring namespaces and code loading as currently implemented.

Comment by Alex Miller [ 10/Mar/17 7:44 PM ]

Well you can look at the patch for the approach to what is suggested in this ticket. When you try to create an alias that isn't loaded, simply find or create it. If it does exist, it will be loaded. If it does not, then it will be created.

But kind of a moot point as we're not going to do this.





[CLJ-2029] (nth nil <anything>) does not throw an exception Created: 28/Sep/16  Updated: 28/Sep/16  Resolved: 28/Sep/16

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

Type: Defect Priority: Minor
Reporter: Hans Hübner Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None


 Description   

If `nth` is being passed `nil` as `coll` argument, no exception is thrown for arbitrary indices. This does not match the expected behavior (all indices should throw an "Index out of bounds" exception) and also not documented.



 Comments   
Comment by Alex Miller [ 28/Sep/16 9:21 AM ]

It is by design that nth works on nil to return nil for any index and changing this would likely break many existing programs. An enhancement for the docstring would be considered.





[CLJ-2028] Docstring error in clojure.core/filter, remove, and take-while Created: 26/Sep/16  Updated: 07/Sep/17  Resolved: 07/Sep/17

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

Type: Enhancement Priority: Minor
Reporter: Alan Thompson Assignee: Unassigned
Resolution: Completed Votes: 1
Labels: docstring
Environment:

All


Attachments: File clj-2028.diff    
Patch: Code
Approval: Ok

 Description   

The docstring for filter could be clearer about responding to logical true values:

​​Returns a lazy sequence of the items in coll for which
(pred item) returns true. pred must be free of side-effects.
Returns a transducer when no collection is provided.

should be corrected to read:

​Returns a lazy sequence of the items in coll for which
(pred item)​ ​​​returns logical true​. pred must be free of side-effects.​
Returns a transducer when  o collection is provided.

Similar changes could be applied to remove and take-while.

Patch: clj-2028.diff
Prescreened by: Alex Miller



 Comments   
Comment by Alex Miller [ 26/Sep/16 12:49 PM ]

"logical true" is the phrase used for this in other docstrings.

Comment by Jozef Wagner [ 27/Oct/16 7:13 AM ]

Added patch that updates docstrings for filter, filterv, remove and take-while





[CLJ-2027] bean printing regression from namespace map printing Created: 24/Sep/16  Updated: 28/Oct/16  Resolved: 28/Oct/16

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

Type: Defect Priority: Major
Reporter: Trey Sullivan Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: print, regression
Environment:

Clojure 1.9.0-alpha12


Attachments: Text File clj-2027.patch    
Patch: Code and Test
Approval: Ok

 Description   

The new namespace map printing is causing a failure in printing bean maps (which are proxies that don't support empty):

user=> (bean (java.util.Date.))
UnsupportedOperationException empty  clojure.core.proxy$clojure.lang.APersistentMap$ff19274a.empty (:-1)

user=> (pst *e)
UnsupportedOperationException empty
	clojure.core.proxy$clojure.lang.APersistentMap$ff19274a.empty (:-1)
	clojure.core/empty (core.clj:5151)
	clojure.core/lift-ns (core_print.clj:237)

Cause: The internal lift-ns function calls empty on the map too early (here it doesn't need to call it at all).

Proposed: Defer calling (empty m) until we know map has namespace keywords and namespace maps will be used for printing.

Patch: clj-2027.patch (note that into is not used in the change b/c it has not yet been defined at this point)






[CLJ-2024] Check should specize function specs before checking Created: 19/Sep/16  Updated: 28/Oct/16  Resolved: 28/Oct/16

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 Reeves Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: spec

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

 Description   

This code works fine in 1.9.0-alpha12:

(defn f [x] (+ x 1))
(s/def f (s/fspec :args (s/cat :x number?) :ret number?))
(stest/check `f)

But if we factor the fspec out into its own keyword:

(defn f [x] (+ x 1))
(s/def ::inc (s/fspec :args (s/cat :x number?) :ret number?))
(s/def f ::inc)
(stest/check `f)

The check fails with the exception:

({:failure #error {
 :cause "No :args spec"
 :data #:clojure.spec{:failure :no-args-spec}
 :via
 [{:type clojure.lang.ExceptionInfo
   :message "No :args spec"
   :data #:clojure.spec{:failure :no-args-spec}
   :at [...]}]
 :trace
 [...]}, :sym user/f, :spec :user/inc})

The check function doesn't seem to be resolving ::inc, when presumably it should.

Patch: clj-2024-2.patch



 Comments   
Comment by Rich Hickey [ 28/Oct/16 7:44 AM ]

this should be fixed in fspec, not its use by test

Comment by Alex Miller [ 28/Oct/16 8:23 AM ]

fspec is not the problem as far as I can tell - it is already making specs of its args.

The problem is that f is registered as an alias of ::inc. I don't think you want to resolve that at registration time (as ::inc might not exist yet).

The problem as far as I understand it is that at the time of use (by check), f is not resolved to it's final spec and that's what the patch does.

Comment by Alex Miller [ 28/Oct/16 8:43 AM ]

Added new patch that uses `spec` instead of private `specize` function.





[CLJ-2023] clojure.spec edge case failure Created: 14/Sep/16  Updated: 14/Sep/16  Resolved: 14/Sep/16

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

Type: Defect Priority: Major
Reporter: Brian Noguchi Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None
Environment:

Using clojure 1.9.0-alpha12



 Description   

I stumbled onto an odd edge case. The following is a minimal example you can run in the REPL:

```
(require '[clojure.spec :as s])
(s/def :tv/star (s/or :starring/ernie #{:char/ernie}
:starring/big-bird #{:char/big-bird}))

; The following behaves as expected.
(s/explain
(s/and (s/keys :req [:tv/star]
:opt [:tv/co-star])
(s/or
:ernie-and-bert (s/and #(do (println "!!!!!") (println %) (= (:tv/star %) :char/ernie))
#(= (:tv/co-star %) :char/bert))
:just-the-bird #(not (contains? % :tv/co-star))))
{:tv/star :char/big-bird})
; This outputs the following:
; !!!!!
; #:tv{:star [:starring/big-bird :char/big-bird]}
; Success!
; => nil

; The following is unexpected
(s/explain
(s/and (s/keys)
(s/or
:ernie-and-bert (s/and #(do (println "!!!!!") (println %) (= (:tv/star %) :char/ernie))
#(= (:tv/co-star %) :char/bert))
:just-the-bird #(not (contains? % :tv/co-star))))
{:tv/star :char/ernie :tv/co-star :char/bert})
; This outputs the following:
; !!!!!
; #:tv{:star [:starring/ernie :char/ernie], :co-star :char/bert}
; !!!!!
; #:tv{:star [:starring/ernie :char/ernie], :co-star :char/bert}
; !!!!!
; #:tv{:star [:starring/ernie :char/ernie], :co-star :char/bert}
; !!!!!
; #:tv{:star [:starring/ernie :char/ernie], :co-star :char/bert}
; val: #:tv{:star [:starring/ernie :char/ernie], :co-star :char/bert} fails at: [:ernie-and-bert] predicate: (do (println "!!!!!") (println %) (= (:tv/star %) :char/ernie))
; val: #:tv{:star [:starring/ernie :char/ernie], :co-star :char/bert} fails at: [:just-the-bird] predicate: (not (contains? % :tv/co-star))
; => nil
```

I would have expected the second `(s/explain ...)` to succeed, given my understanding of `clojure.spec` semantics. However, it seems as though the argument inside `#(do (println "!!!!!") (println %) (= (:tv/star %) :char/ernie))` does not resolve `%` to the original map `{:tv/star :char/ernie :tv/co-star :char/bert}` but rather the transformed map `#:tv{:star [:starring/ernie :char/ernie], :co-star :char/bert}` that seems to mix in the output of applying conform against the spec definition of `:tv/star`.

Miscellaneous edge case observations:

  • If I replace the sibling spec `(s/keys)` with a simple predicate like `some?`, then it succeeds.

```
(s/explain
(s/and some?
(s/or
:ernie-and-bert (s/and #(do (println "!!!!!") (println %) (= (:tv/star %) :char/ernie))
#(= (:tv/co-star %) :char/bert))
:just-the-bird #(not (contains? % :tv/co-star))))
{:tv/star :char/ernie :tv/co-star :char/bert})
```

  • If I elide the first, solitary registered clojure.spec definition `(s/def :tv/star ...)`, then it succeeds.

I haven't investigated a solution yet.



 Comments   
Comment by Brian Noguchi [ 14/Sep/16 4:20 AM ]

It looks like the observed behavior is the expected correct behavior. I just noticed that successive conformed values are supposed to propagate through the rest of the predicates.

https://github.com/clojure/clojure/blob/d920ada9fab7e9b8342d28d8295a600a814c1d8a/src/clj/clojure/spec.clj#L439-L440

Comment by Brian Noguchi [ 14/Sep/16 4:21 AM ]

This issue can be ignored and closed. Sorry!





[CLJ-2020] New prohibited field names (__hash __hasheq) break existing software Created: 07/Sep/16  Updated: 08/Sep/16  Resolved: 08/Sep/16

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

Type: Defect Priority: Major
Reporter: lvh Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: defrecord


 Description   

The most recent Clojure alpha (1.9.0-alpha12) contained a patch closing CLJ-1224. This had the unfortunate side effect of breaking some existing software, notably datascript. I've filed an issue upstream as well: https://github.com/tonsky/datascript/issues/176

It's not clear to me what the best resolution is; IIUC the behavior datascript was trying to accomplish is what records now do automagically, although I might have misunderstood. Ideally, datascript wouldn't have the serious performance regression on >=1.8.0, but it definitely should compile on 1.9.0, regardless of how that's resolved.



 Comments   
Comment by Alex Miller [ 07/Sep/16 9:02 PM ]

My initial reaction is that datascript should not be using fields starting with __ as that is at least implied to be "internal stuff" in the defrecord docstring. But, I reserve the right to think about it more.

Comment by Alex Miller [ 08/Sep/16 9:51 AM ]

After thinking about it more, I will double down to say that extending a defrecord to IHashEq is also bad form. Records are intended to have the hash semantics of maps and to be controlled by the language, not by a record extender. I did quite a bit of searching and have found no other project that does this.

Implementing IHashEq for deftypes is a normal and perfectly acceptable thing to do as deftypes are considered to be opaque from Clojure's point of view - you give it the semantics.

Comment by Alex Miller [ 08/Sep/16 9:58 AM ]

Rich concurs so I am declining. The lib code should be changed.

Comment by lvh [ 08/Sep/16 9:59 AM ]

IIUC the options for datascript are either moving to deftype, or removing the IHashEq impl (and living with the perf penalty). Is there an option that lets them keep the perf, conditionally implementing only on 1.9.0? (That may not be desirable at all.)

Comment by Alex Miller [ 08/Sep/16 10:41 AM ]

It's conceivable to conditionally load different versions of a namespace that defines the records based on Clojure version. I'm not sure whether that's worth doing or not (and how it plays with AOT). I don't know whether it even affects perf or by how much - seems like that's the first question to answer. If it doesn't matter, then just remove it.





[CLJ-2019] Loosen constraint between key name and spec name in clojure.spec/keys Created: 04/Sep/16  Updated: 15/May/17  Resolved: 15/May/17

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

Type: Feature Priority: Major
Reporter: Peter Schuck Assignee: Unassigned
Resolution: Declined Votes: 2
Labels: spec


 Description   

According to the Clojure Spec Rationale

Most systems for specifying structures conflate the specification of the key set (e.g. of keys in a map, fields in an object) with the specification of the values designated by those keys. I.e. in such approaches the schema for a map might say :a-key's type is x-type and :b-key's type is y-type. This is a major source of rigidity and redundancy.

Currently clojure.spec/keys does exactly the complecting the rationale says is a major source of rigidity and redundancy. clojure.spec/keys requires that any keys in it's key set have the name the have in the spec registry. For example:

(ns my.ns
  (:require
    [clojure.spec :as spec]))

(spec/def ::x-type integer?)
(spec/def ::y-type bool?)

;;The only map that can be checked for is {::x-type <x-type> ::y-type <y-type>}
(spec/def ::my-map (spec/keys :req [::x-type ::y-type]))

;;To validate a map like {::a-key <x-type> ::b-key <y-type>} You need to do this
(spec/def ::a-key ::x-type)
(spec/def ::b-key ::y-type)
(spec/def ::my-map (spec/keys :req [::a-key ::b-key]))

What clojure.spec/keys should allow you to do is this

(ns my.ns
  (:require
    [clojure.spec :as spec]))

(spec/def ::x-type integer?)
(spec/def ::y-type bool?)

;;The key set is now independent of the spec's names. You can validate
;;a map like {::a-key <x-type> ::b-key <y-type>}
(spec/def ::my-map (spec/keys :req {::a-key ::x-type ::b-key ::y-type}))

The exact implementation may vary from what I show here but the end result should be allowing users to check for x-type under ::a-key with out having to do the redundant step of (clojure.spec/def ::a-key (clojure.spec/spec <x-type>).



 Comments   
Comment by Laszlo Török [ 19/Sep/16 4:34 PM ]

Spec advocates to use namespaced keys to convey contextual semantics of the value.

Relevant quote from the Spec guide:

"These maps represent various sets, subsets, intersections and unions of the same keys, and in general ought to have the same semantic for the same key wherever it is used."

There may be different pieces of information that end up having the same representation (e.g. both are of type integer).

The ::x-type vs ::a-key nomenclature above is misleading. One should rather look a keyword-value pair in a map as an attribute-value pair, where you can specify the valid values of that attribute using a spec.

Comment by Alex Miller [ 15/May/17 3:42 PM ]

At this time, I'm going to decline this ticket as I think it's unlikely that we're going to do this. But I would not rule out the possibility that this idea rises from the dead at some future point.





[CLJ-2018] clojure.spec.gen lazy-loaded fns should contain wrapped thing's docstring Created: 03/Sep/16  Updated: 06/Sep/16  Resolved: 06/Sep/16

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

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


 Description   

Right now, using clojure.spec.gen's lazily loaded fns is harder than using the test.check versions because the former doesn't have useful docstrings and the latter does.

clojure.spec.gen lazy-loaded fns should contain docstring of the thing they refer to, or at least what it was at compile time, or at least a universally-understood reference to that thing that IDEs can follow. I don't think the latter exists, so perhaps the docstring at compile time should be embedded into the lazy-loaded fn if possible.

(As a general note, and I don't know if my experience generalizes, but I find myself grabbing for stuff in e.g. test.chuck so often that the "no runtime dependency" thing doesn't really work out anyway.)



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

I don't think they can contain those docstrings without actually loading the remote var to get its meta, which defies the whole point of lazy loading. So while I sympathize with the request, I'm not sure that I understand how it is possible to achieve?

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

I was hoping a macro would be able to load the var, but only at compile time (e.g. when building the jar), not affecting a runtime consumer of the same ns; the same way that some of my ClojureScript code is built with macros that read resource files at compile time. (Perhaps that is not technologically possible right now in Clojure.)

Comment by Alex Miller [ 06/Sep/16 11:20 AM ]

I'm going to decline this as I'm not sure how it would be done while still satisfying the other goals of this code. But if someone finds a reasonable way to do it, would reconsider.





[CLJ-2016] function contains? return value error Created: 30/Aug/16  Updated: 30/Aug/16  Resolved: 30/Aug/16

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

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

ubuntu 14.04



 Description   

hello.core=> (contains? [1 2 3] 1)
true
hello.core=> (contains? [1 2 3] 2)
true
hello.core=> (contains? [1 2 3] 3)
false

i am not sure, it's a bug or not, because it's so simple.
But, [1 2 3] should contains 3 right?!



 Comments   
Comment by Alex Miller [ 30/Aug/16 9:18 AM ]

This is a common question and this is the expected behavior.

contains? checks for containment of a key in an indexed collection. In a map, contains? works on keys. In a set, it works on the (hashed) elements. In a vector, it vector on the vector indexes (not the values).

So asking (contains? [1 2 3] 1) is asking whether there is an element at index 1 in [1 2 3], which there is (here it's 2). When you ask (contains? [1 2 3] 3) you are asking if [1 2 3] has index 3 (0-based), which it does not.

Hope that helps.

Comment by Alex Miller [ 30/Aug/16 9:27 AM ]

Also, more info here http://insideclojure.org/2015/01/27/clojure-tips-contains/





[CLJ-2014] (keyword "@type") can be printed, but not read Created: 26/Aug/16  Updated: 26/Aug/16  Resolved: 26/Aug/16

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

Type: Defect Priority: Minor
Reporter: David Smith Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None

Approval: Incomplete
Waiting On: Rich Hickey

 Description   

user=> (keyword "")
:
user=> (prn-str *1)
":\n"
user=> (read-string *1)
java.lang.RuntimeException: java.lang.Exception: Invalid token: : (NO_SOURCE_FILE:0)

This obviously isn't a huge defect, but I'd argue that anything that can be printed should be readable.



 Comments   
Comment by David Smith [ 26/Aug/16 5:02 AM ]

This is a clone of http://dev.clojure.org/jira/browse/CLJ-732 which it appears was closed with no explanation. I have recently come up against this when deserializing json. IMO it doesn't make sense for the keyword function to be able to produce non-valid keywords. What is the reason for rejecting this?

Comment by Alex Miller [ 26/Aug/16 7:47 AM ]

This is a feature used by a lot of Clojure programs. See:

http://clojure.org/guides/faq#unreadable_keywords

Comment by David Smith [ 26/Aug/16 7:49 AM ]

Thank you for the explanation, this can therefor be closed.





[CLJ-2012] ns spec breaks gen-class forms that use strings as class names Created: 24/Aug/16  Updated: 07/Sep/16  Resolved: 07/Sep/16

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

Type: Defect Priority: Major
Reporter: Nicola Mometto Assignee: Alex Miller
Resolution: Completed Votes: 0
Labels: regression, spec

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

 Description   

The following valid `ns` gen-class form is reported as invalid by spec:

(ns foo
  (:gen-class
     :name ^{org.apache.logging.log4j.core.config.plugins.Plugin
             {:name "LogstashConverter" :category "Converter"}
             org.apache.logging.log4j.core.pattern.ConverterKeys ["logstashJson"]} social.common.logging.LogstashPatternConverter
     :constructors {["[Ljava.lang.String;"] [